Correctly append ADDITIONAL_CA_CERT_BUNDLE contents when analyzer is run multiple times
Problem to solve
While working on Allow git to use cacert I came across the following behaviour with regards to CA Certificate handling:
-
run a secure analyzer
Docker
image locally:docker run -it --rm -v "$PWD:/code" \ -e ADDITIONAL_CA_CERT_BUNDLE="$(cat test.crt)" \ -e GEMNASIUM_DB_REMOTE_URL=https://gitlab-airgap-test.us-west1-b.c.group-secure-a89fe7.internal/analyzers/gemnasium-db \ -e SECURE_LOG_LEVEL=debug \ registry.gitlab.com/gitlab-org/security-products/analyzers/gemnasium /bin/sh
-
execute the analyzer multiple times:
/ # cd /code /code # /analyzer run /code # /analyzer run
-
The analyzer ends up appending the
ADDITIONAL_CA_CERT_BUNDLE
contents to the/etc/ssl/certs/ca-cert-additional-gitlab-bundle.pem
without any newlines, which causes an invalid certificate:/code # cat /etc/ssl/certs/ca-cert-additional-gitlab-bundle.pem -----BEGIN CERTIFICATE----- MIIEQzCCAyugAwIBAgIUe5OYnWvcwt2MgCpVSUgvFa8E3D0wDQYJKoZIhvcNAQEL IPcVVawMK/ChUcbktFylIAu9ohWrJHU5KuDrzhEOyG+0hEFGFnzYfpJSADIHvNNS Gtpf/YEZclLD7wHrkhbeIThnU/Z9q270dm15wEGO9MLACEob6DZo -----END CERTIFICATE----------BEGIN CERTIFICATE----- MIIEQzCCAyugAwIBAgIUe5OYnWvcwt2MgCpVSUgvFa8E3D0wDQYJKoZIhvcNAQEL IPcVVawMK/ChUcbktFylIAu9ohWrJHU5KuDrzhEOyG+0hEFGFnzYfpJSADIHvNNS Gtpf/YEZclLD7wHrkhbeIThnU/Z9q270dm15wEGO9MLACEob6DZo -----END CERTIFICATE-----
The above certificate file is invalid and will cause other tools that try to use it, such as
git
, to fail. The certificate should instead be formatted as follows, with a newline between each certificate:-----BEGIN CERTIFICATE----- MIIEQzCCAyugAwIBAgIUe5OYnWvcwt2MgCpVSUgvFa8E3D0wDQYJKoZIhvcNAQEL IPcVVawMK/ChUcbktFylIAu9ohWrJHU5KuDrzhEOyG+0hEFGFnzYfpJSADIHvNNS Gtpf/YEZclLD7wHrkhbeIThnU/Z9q270dm15wEGO9MLACEob6DZo -----END CERTIFICATE----- -----BEGIN CERTIFICATE----- MIIEQzCCAyugAwIBAgIUe5OYnWvcwt2MgCpVSUgvFa8E3D0wDQYJKoZIhvcNAQEL IPcVVawMK/ChUcbktFylIAu9ohWrJHU5KuDrzhEOyG+0hEFGFnzYfpJSADIHvNNS Gtpf/YEZclLD7wHrkhbeIThnU/Z9q270dm15wEGO9MLACEob6DZo -----END CERTIFICATE-----
This behaviour doesn't happen when you specify the ADDITIONAL_CA_CERT_BUNDLE
in a CI variable, which you can see in the following test.
It only seems to occur when you pass the certificate file as a commandline Docker environment value such as
-e ADDITIONAL_CA_CERT_BUNDLE="$(cat test.crt)"
It also only occurs when you run the analyzer
more than once, which should only happen for developers running the analyzer locally to possibly debug issues.
Intended users
User experience goal
A user should be able to run the /analyzer run
command more than once without corrupting the CA Certificate file.
Proposal
There are a few different possible solutions, we need to figure out the best approach:
- Figure out the correct way to read the certificate file into the
ADDITIONAL_CA_CERT_BUNDLE
environment variable when running Docker from the commandline, because using-e ADDITIONAL_CA_CERT_BUNDLE="$(cat test.crt)"
doesn't work correctly, even when I tried adding newlines into thetest.crt
file - Switch to using
fmt.Fprintln(f, b.content)
so that newlines are automatically inserted - I've tested this and it doesn't seem to alter the current behaviour when passing theADDITIONAL_CA_CERT_BUNDLE
value as a CI environment variable - Update the logic for writing the CA certificate file so it checks to see if the certificate has already been written to the
/etc/ssl/certs/ca-cert-additional-gitlab-bundle.pem
file, and if so, it doesn't append another copy of it - Do nothing, but this means that developers testing this locally will only be able to execute the
/analyzer run
command once before it results in an invalid certificate file
Further details
@dsearles also experienced the problem described in this issue and had the following to say:
I remember running into a similar error when I was working through changing the behavior to append rather than override. I'm not sure how to solve
1.
, but that would be a nice approach. When I noticed the same behavior, then I switched to2.
, but noticed that it added an extra newline between the certs and stuck with this approach as it's more common for a single run to happen in an analyzer than multiple runs. From my perspective,3.
would be ideal, but likely quite difficult to achieve. In the end, I'm open to any of the options above as I'd like to eventually move away from using common for certs in the groupstatic analysis analyzers (for more info, see: #218840 (closed)).
Implementation plan
Since the git config write in the same package already uses fmt.Fprintln
to achieve the desired result, it will be consistent to continue to use this: https://gitlab.com/gitlab-org/security-products/analyzers/common/-/blob/master/cacert/bundle.go#L82
-
update https://gitlab.com/gitlab-org/security-products/analyzers/common/-/blob/master/cacert/bundle.go#L54 with fmt.Fprintln
-
test by building an analyzer with the new version of common and ensuring that running /analyzer run
multiple times does not corrupt the ca cert inDefaultBundlePath = "
/etc/ssl/certs/ca-cert-additional-gitlab-bundle.pem` -
update all analyzers that depend on common and release new versions
What does success look like, and how can we measure that?
The /analyzer run
command can be executed multiple times without resulting in a corrupt CA Certificate file.
What is the type of buyer?
Enterprise Edition GitLab Ultimate
Is this a cross-stage feature?
Yes, this affects all analyzers that provide the ADDITIONAL_CA_CERT_BUNDLE
flag
Links / references
gitlab-org/security-products/analyzers/common!112 (comment 395342055)