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 <mrashish@redhat.com>

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

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,

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()