From 4c41a14e7452da1b97877af38795528d410f48a2 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Fri, 20 Oct 2023 11:26:35 -0400 Subject: [PATCH] Merge branch '21086-cert-source' fixes #21086 Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- lib/crunchrun/crunchrun.go | 23 +++++++++++---- lib/crunchrun/crunchrun_test.go | 10 +++---- sdk/go/arvados/tls_certs.go | 23 +++++++++++++++ sdk/go/arvados/tls_certs_test.go | 32 +++++++++++++++++++++ sdk/go/arvados/tls_certs_test_showenv.go | 22 +++++++++++++++ sdk/go/arvadosclient/arvadosclient.go | 36 +----------------------- 6 files changed, 100 insertions(+), 46 deletions(-) create mode 100644 sdk/go/arvados/tls_certs.go create mode 100644 sdk/go/arvados/tls_certs_test.go create mode 100644 sdk/go/arvados/tls_certs_test_showenv.go diff --git a/lib/crunchrun/crunchrun.go b/lib/crunchrun/crunchrun.go index ef04551883..0e0d3c43e4 100644 --- a/lib/crunchrun/crunchrun.go +++ b/lib/crunchrun/crunchrun.go @@ -46,6 +46,8 @@ import ( type command struct{} +var arvadosCertPath = "/etc/arvados/ca-certificates.crt" + var Command = command{} // ConfigData contains environment variables and (when needed) cluster @@ -494,7 +496,7 @@ func (runner *ContainerRunner) SetupMounts() (map[string]bindmount, error) { } } - if bind == "/etc/arvados/ca-certificates.crt" { + if bind == arvadosCertPath { needCertMount = false } @@ -644,10 +646,19 @@ func (runner *ContainerRunner) SetupMounts() (map[string]bindmount, error) { } if needCertMount && runner.Container.RuntimeConstraints.API { - for _, certfile := range arvadosclient.CertFiles { - _, err := os.Stat(certfile) - if err == nil { - bindmounts["/etc/arvados/ca-certificates.crt"] = bindmount{HostPath: certfile, ReadOnly: true} + for _, certfile := range []string{ + // Populated by caller, or sdk/go/arvados init(), or test suite: + os.Getenv("SSL_CERT_FILE"), + // Copied from Go 1.21 stdlib (src/crypto/x509/root_linux.go): + "/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 + } { + if _, err := os.Stat(certfile); err == nil { + bindmounts[arvadosCertPath] = bindmount{HostPath: certfile, ReadOnly: true} break } } @@ -1996,7 +2007,7 @@ func (command) RunCommand(prog string, args []string, stdin io.Reader, stdout, s time.Sleep(*sleep) if *caCertsPath != "" { - arvadosclient.CertFiles = []string{*caCertsPath} + os.Setenv("SSL_CERT_FILE", *caCertsPath) } keepstore, err := startLocalKeepstore(conf, io.MultiWriter(&keepstoreLogbuf, stderr)) diff --git a/lib/crunchrun/crunchrun_test.go b/lib/crunchrun/crunchrun_test.go index 30f3ea4bb4..c533821351 100644 --- a/lib/crunchrun/crunchrun_test.go +++ b/lib/crunchrun/crunchrun_test.go @@ -1413,11 +1413,11 @@ func (am *ArvMountCmdLine) ArvMountTest(c []string, token string) (*exec.Cmd, er return nil, nil } -func stubCert(temp string) string { +func stubCert(c *C, temp string) string { path := temp + "/ca-certificates.crt" - crt, _ := os.Create(path) - crt.Close() - arvadosclient.CertFiles = []string{path} + err := os.WriteFile(path, []byte{}, 0666) + c.Assert(err, IsNil) + os.Setenv("SSL_CERT_FILE", path) return path } @@ -1432,7 +1432,7 @@ func (s *TestSuite) TestSetupMounts(c *C) { realTemp := c.MkDir() certTemp := c.MkDir() - stubCertPath := stubCert(certTemp) + stubCertPath := stubCert(c, certTemp) cr.parentTemp = realTemp i := 0 diff --git a/sdk/go/arvados/tls_certs.go b/sdk/go/arvados/tls_certs.go new file mode 100644 index 0000000000..db52781339 --- /dev/null +++ b/sdk/go/arvados/tls_certs.go @@ -0,0 +1,23 @@ +// Copyright (C) The Arvados Authors. All rights reserved. +// +// SPDX-License-Identifier: Apache-2.0 + +package arvados + +import "os" + +// Load root CAs from /etc/arvados/ca-certificates.crt if it exists +// and SSL_CERT_FILE does not already specify a different file. +func init() { + envvar := "SSL_CERT_FILE" + certfile := "/etc/arvados/ca-certificates.crt" + if os.Getenv(envvar) != "" { + // Caller has already specified SSL_CERT_FILE. + return + } + if _, err := os.ReadFile(certfile); err != nil { + // Custom cert file is not present/readable. + return + } + os.Setenv(envvar, certfile) +} diff --git a/sdk/go/arvados/tls_certs_test.go b/sdk/go/arvados/tls_certs_test.go new file mode 100644 index 0000000000..7900867715 --- /dev/null +++ b/sdk/go/arvados/tls_certs_test.go @@ -0,0 +1,32 @@ +// Copyright (C) The Arvados Authors. All rights reserved. +// +// SPDX-License-Identifier: Apache-2.0 + +package arvados + +import ( + "os" + "os/exec" + + check "gopkg.in/check.v1" +) + +type tlsCertsSuite struct{} + +var _ = check.Suite(&tlsCertsSuite{}) + +func (s *tlsCertsSuite) TestCustomCert(c *check.C) { + certfile := "/etc/arvados/ca-certificates.crt" + if _, err := os.Stat(certfile); err != nil { + c.Skip("custom cert file " + certfile + " does not exist") + } + out, err := exec.Command("bash", "-c", "SSL_CERT_FILE= go run tls_certs_test_showenv.go").CombinedOutput() + c.Logf("%s", out) + c.Assert(err, check.IsNil) + c.Check(string(out), check.Equals, certfile+"\n") + + out, err = exec.Command("bash", "-c", "SSL_CERT_FILE=/dev/null go run tls_certs_test_showenv.go").CombinedOutput() + c.Logf("%s", out) + c.Assert(err, check.IsNil) + c.Check(string(out), check.Equals, "/dev/null\n") +} diff --git a/sdk/go/arvados/tls_certs_test_showenv.go b/sdk/go/arvados/tls_certs_test_showenv.go new file mode 100644 index 0000000000..f2622cf11d --- /dev/null +++ b/sdk/go/arvados/tls_certs_test_showenv.go @@ -0,0 +1,22 @@ +// Copyright (C) The Arvados Authors. All rights reserved. +// +// SPDX-License-Identifier: Apache-2.0 + +//go:build ignore + +// This is a test program invoked by tls_certs_test.go + +package main + +import ( + "fmt" + "os" + + "git.arvados.org/arvados.git/sdk/go/arvados" +) + +var _ = arvados.Client{} + +func main() { + fmt.Println(os.Getenv("SSL_CERT_FILE")) +} diff --git a/sdk/go/arvadosclient/arvadosclient.go b/sdk/go/arvadosclient/arvadosclient.go index 516187c0e6..461320eca9 100644 --- a/sdk/go/arvadosclient/arvadosclient.go +++ b/sdk/go/arvadosclient/arvadosclient.go @@ -9,16 +9,12 @@ package arvadosclient import ( "bytes" "crypto/tls" - "crypto/x509" "encoding/json" "errors" "fmt" "io" - "io/ioutil" - "log" "net/http" "net/url" - "os" "strings" "sync" "time" @@ -121,40 +117,10 @@ type ArvadosClient struct { RequestID string } -var CertFiles = []string{ - "/etc/arvados/ca-certificates.crt", - "/etc/ssl/certs/ca-certificates.crt", // Debian/Ubuntu/Gentoo etc. - "/etc/pki/tls/certs/ca-bundle.crt", // Fedora/RHEL -} - // MakeTLSConfig sets up TLS configuration for communicating with // Arvados and Keep services. func MakeTLSConfig(insecure bool) *tls.Config { - tlsconfig := tls.Config{InsecureSkipVerify: insecure} - - if !insecure { - // Use the first entry in CertFiles that we can read - // certificates from. If none of those work out, use - // the Go defaults. - certs := x509.NewCertPool() - for _, file := range CertFiles { - data, err := ioutil.ReadFile(file) - if err != nil { - if !os.IsNotExist(err) { - log.Printf("proceeding without loading cert file %q: %s", file, err) - } - continue - } - if !certs.AppendCertsFromPEM(data) { - log.Printf("unable to load any certificates from %v", file) - continue - } - tlsconfig.RootCAs = certs - break - } - } - - return &tlsconfig + return &tls.Config{InsecureSkipVerify: insecure} } // New returns an ArvadosClient using the given arvados.Client -- 2.30.2