Skip to content

Forcibly load X509 cert

Robert May requested to merge x509-cert-loading into master

What does this MR do?

We seem to set the following two constants:

OpenSSL::X509::DEFAULT_CERT_DIR
OpenSSL::X509::DEFAULT_CERT_FILE

I believe probably via ENV vars.

When loading custom root certificates for X509 commit signing verification, the certificate is bundled into that directory, and the file constant set to point to it.

OpenSSL then seemingly completely ignores OpenSSL::X509::DEFAULT_CERT_FILE, and fails to pick up the cert file from inside OpenSSL::X509::DEFAULT_CERT_DIR, even if you forcibly add that path to the store again with store.add_path(OpenSSL::X509::DEFAULT_CERT_DIR).

However if you add the OpenSSL::X509::DEFAULT_CERT_FILE with store.add_cert(OpenSSL::X509::DEFAULT_CERT_FILE), it does work, and it can successfully verify the commits.

I'm not sure if this is an upstream bug in the openssl ruby library or not, but this patch forcibly loads the cert when testing for commit signing, which fixes that feature in some circumstances.

I found another instance of us force-loading this file: https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/gitaly_client.rb#L65

Related #293697 (closed)

Screenshots (strongly suggested)

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Robert May

Merge request reports