Bug #21086
closedsdk/go/arvados should use TLS certificates from /etc/arvados/ca-certificates.crt if it exists
Description
Move the "choose a cert source" logic from sdk/go/arvadosclient
to sdk/go/arvados
and make sure both libraries use it.
Updated by Tom Clegg over 1 year ago
Out of the box, Go checks the following locations:
// Possible certificate files; stop after finding one.
var certFiles = []string{
"/etc/ssl/certs/ca-certificates.crt", // Debian/Ubuntu/Gentoo etc.
"/etc/pki/tls/certs/ca-bundle.crt", // Fedora/RHEL 6
"/etc/ssl/ca-bundle.pem", // OpenSUSE
"/etc/pki/tls/cacert.pem", // OpenELEC
"/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem", // CentOS/RHEL 7
"/etc/ssl/cert.pem", // Alpine Linux
}
// Possible directories with certificate files; all will be read.
var certDirectories = []string{
"/etc/ssl/certs", // SLES10/SLES11, https://golang.org/issue/12139
"/etc/pki/tls/certs", // Fedora/RHEL
"/system/etc/security/cacerts", // Android
}
SSL_CERT_FILE and SSL_CERT_DIR can be used to override these locations.
Currently, the arvadosclient library modifies its http client to effectively replace certFiles with its own list ("/etc/arvados/ca-certificates.crt" + debian + fedora paths), drop the other default locations, disable the SSL_CERT_FILE
and SSL_CERT_DIR
env var mechanism, and also disable loading certs from the default certDirectories. I suspect disabling the env vars and certDirectories was accidental -- in fact, it seems our customization (cc1dbda8f559ab43f326c77595d4af87e8ca7a33) predates those stdlib features (https://cs.opensource.google/go/go/+/e83bcd95a4a86e4caf2faa78158170d512dd9de5) by a few months.
Also, the way the current code modifies the http client doesn't align very well with the way the arvados
package lets the caller provide its own http client.
So I'm thinking another way to load from our custom location, which would be more compatible with the other OSes and other local customizations, would be to automatically set SSL_CERT_FILE
to /etc/arvados/ca-certificates.crt
only if that file exists and SSL_CERT_FILE
is not already set to a different readable file. This would also have a side effect that child processes that support those env vars would also use the certificates, which seems desirable if anything.
As an aside, is there even a reason why we use the custom path /etc/arvados/ca-certificates.crt
instead of adding certs to /etc/ssl/certs/
where unmodified SSL clients would already know how to find them?
Updated by Lucas Di Pentima over 1 year ago
Tom Clegg wrote in #note-2:
So I'm thinking another way to load from our custom location, which would be more compatible with the other OSes and other local customizations, would be to automatically set
SSL_CERT_FILE
to/etc/arvados/ca-certificates.crt
only if that file exists andSSL_CERT_FILE
is not already set to a different readable file. This would also have a side effect that child processes that support those env vars would also use the certificates, which seems desirable if anything.
I agree this would be good to have.
As an aside, is there even a reason why we use the custom path
/etc/arvados/ca-certificates.crt
instead of adding certs to/etc/ssl/certs/
where unmodified SSL clients would already know how to find them?
The only thing I can think could be a reason is to avoid the standard ca-certificates.crt overwrite the customized one when installing the related package.
Updated by Tom Clegg over 1 year ago
21086-cert-source @ eccb6ba135ebe1bc73bdeeed89150d3c4ee26126 -- developer-run-tests: #3868
- All agreed upon points are implemented / addressed.
- ✅ now both Go SDKs will load /etc/arvados/ca-certificates.crt if it exists. (arvadosclient imports arvados, so the init() func will run if a program imports either arvadosclient or arvados)
- Anything not implemented (discovered or discussed during work) has a follow-up story.
- N/A
- Code is tested and passing, both automated and manual, what manual testing was done is described
- ✅ There's a test that only works if /etc/arvados/ca-certificates.crt exists on the test host (otherwise it gets skipped). Created that file & tested manually on my dev machine.
- Documentation has been updated.
- N/A
- Behaves appropriately at the intended scale (describe intended scale).
- N/A
- Considered backwards and forwards compatibility issues between client and server.
- N/A
- Follows our coding standards and GUI style guidelines.
- ✅
Updated by Tom Clegg over 1 year ago
- % Done changed from 0 to 100
- Status changed from In Progress to Resolved
Applied in changeset arvados|7b51c206b53b1c542be66bd0e277ff2ad87894c0.