From 5875f0ea9f128e5df56236c98d58a7f3cf27c8e2 Mon Sep 17 00:00:00 2001 From: Maya Rashish Date: Sun, 4 Apr 2021 19:24:05 +0300 Subject: [PATCH] Restore compatibility in specifying custom CAs After this, our logic matches the old one. - If a user specifies a certificate, the system certs are still valid. - A certificate can be any filename, not just tls.crt (which was used in the tests, so slipped under the radar) XXX untested: this likely fixes the use of a custom CA for an importer proxy. Unfortunately, curl's behaviour with the --cacert and --capath aren't directly usable for these circumstances, so we need to do some work to get the same behaviour out of them. Create a single file containing the system CAs, proxy CAs, and user-specified CA in one, and pass it as a single file to the curl nbdkit plugin. Signed-off-by: Maya Rashish --- pkg/common/common.go | 2 + pkg/image/nbdkit.go | 5 +-- pkg/importer/http-datasource.go | 75 +++++++++++++++++---------------- 3 files changed, 42 insertions(+), 40 deletions(-) diff --git a/pkg/common/common.go b/pkg/common/common.go index 786f36e370..3b019d9d22 100644 --- a/pkg/common/common.go +++ b/pkg/common/common.go @@ -49,6 +49,8 @@ const ( ImporterS3Host = "s3.amazonaws.com" // ImporterCertDir is where the configmap containing certs will be mounted ImporterCertDir = "/certs" + // ImporterCertFile is the single file containing all the certificates to be trusted by the importer + ImporterCertFile = "all-certs.crt" // DefaultPullPolicy imports k8s "IfNotPresent" string for the import_controller_gingko_test and the cdi-controller executable DefaultPullPolicy = string(v1.PullIfNotPresent) // ImportProxyConfigMapName provides the name of the ConfigMap in the cdi namespace containing a CA certificate bundle diff --git a/pkg/image/nbdkit.go b/pkg/image/nbdkit.go index a684fdf379..3603094eba 100644 --- a/pkg/image/nbdkit.go +++ b/pkg/image/nbdkit.go @@ -13,6 +13,7 @@ import ( "github.com/pkg/errors" "k8s.io/klog/v2" + "kubevirt.io/containerized-data-importer/pkg/common" ) const ( @@ -78,9 +79,7 @@ func NewNbdkit(plugin NbdkitPlugin, nbdkitPidFile string) *Nbdkit { func NewNbdkitCurl(nbdkitPidFile, certDir, socket string) NbdkitOperation { var pluginArgs []string args := []string{"-r"} - if certDir != "" { - pluginArgs = append(pluginArgs, fmt.Sprintf("cainfo=%s/%s", certDir, "tls.crt")) - } + pluginArgs = append(pluginArgs, fmt.Sprintf("cainfo=%s", common.ImporterCertFile)) return &Nbdkit{ NbdPidFile: nbdkitPidFile, diff --git a/pkg/importer/http-datasource.go b/pkg/importer/http-datasource.go index aaae3fa7fc..481071df4b 100644 --- a/pkg/importer/http-datasource.go +++ b/pkg/importer/http-datasource.go @@ -25,6 +25,7 @@ import ( "io/ioutil" "net/http" "net/url" + "os" "path" "path/filepath" "strconv" @@ -85,6 +86,10 @@ func NewHTTPDataSource(endpoint, accessKey, secKey, certDir string, contentType if err != nil { return nil, errors.Wrapf(err, fmt.Sprintf("unable to parse endpoint %q", endpoint)) } + err = createCAFile(certDir) + if err != nil { + return nil, errors.Wrapf(err, "Unable to generate certificate file") + } ctx, cancel := context.WithCancel(context.Background()) httpReader, contentLength, brokenForQemuImg, err := createHTTPReader(ctx, ep, accessKey, secKey, certDir) if err != nil { @@ -204,58 +209,54 @@ func (hs *HTTPDataSource) Close() error { return err } -func createHTTPClient(certDir string) (*http.Client, error) { - client := &http.Client{ - // Don't set timeout here, since that will be an absolute timeout, we need a relative to last progress timeout. - } - - if certDir == "" { - return client, nil - } +// CURL either supports a hashed-directory of certificates or a single file certificate. +// Create a single file containing all our certificates. +func createCAFile(certDir string) error { + systemCertDir := "/etc/pki/tls/certs/" + certDirs := []string{certDir, systemCertDir, common.ImporterProxyCertDir} - // let's get system certs as well - certPool, err := x509.SystemCertPool() + certFile, err := os.Create(common.ImporterCertFile) if err != nil { - return nil, errors.Wrap(err, "Error getting system certs") + klog.Errorf("Failed to create %s, got %v", common.ImporterCertFile, err) } + defer certFile.Close() - // append the user-provided trusted CA certificates bundle when making egress connections using proxy - if files, err := ioutil.ReadDir(common.ImporterProxyCertDir); err == nil { + for dir := range certDirs { + files, err := ioutil.ReadDir(certDir) + if err != nil { + klog.V(3).Infof("Error %v listing files in %s", err, dir) + break + } for _, file := range files { if file.IsDir() || file.Name()[0] == '.' { continue } - fp := path.Join(common.ImporterProxyCertDir, file.Name()) - if certs, err := ioutil.ReadFile(fp); err == nil { - certPool.AppendCertsFromPEM(certs) - } - } - } - // append server CA certificates - files, err := ioutil.ReadDir(certDir) - if err != nil { - return nil, errors.Wrapf(err, "Error listing files in %s", certDir) - } + fp := path.Join(certDir, file.Name()) + klog.Infof("Attempting to get certs from %s", fp) - for _, file := range files { - if file.IsDir() || file.Name()[0] == '.' { - continue + certData, err := ioutil.ReadFile(fp) + if err != nil { + return errors.Wrapf(err, "Error reading file %s", fp) + } + certFile.Write(certData) } + } - fp := path.Join(certDir, file.Name()) - - klog.Infof("Attempting to get certs from %s", fp) + return nil +} - certs, err := ioutil.ReadFile(fp) - if err != nil { - return nil, errors.Wrapf(err, "Error reading file %s", fp) - } +func createHTTPClient(certDir string) (*http.Client, error) { + client := &http.Client{ + // Don't set timeout here, since that will be an absolute timeout, we need a relative to last progress timeout. + } - if ok := certPool.AppendCertsFromPEM(certs); !ok { - klog.Warningf("No certs in %s", fp) - } + certData, err := ioutil.ReadFile(common.ImporterCertFile) + if err != nil { + return nil, err } + certPool := x509.NewCertPool() + certPool.AppendCertsFromPEM(certData) // the default transport contains Proxy configurations to use environment variables and default timeouts transport := http.DefaultTransport.(*http.Transport).Clone()