Update certificate support

Remove `rootCAs` from the `NewDefaultUnpacker` API, the argument is no longer used for HTTP transport. Add `CertificateData` to the ImageSource struct. This is PEM-encoded data (straight from a Secret[tls.crt]) to be used to validate the certificate used to access an image regidstry (works along side the `InsecureSkipTLSVerify` option). Signed-off-by: Todd Short <[email protected]>
operator-framework · Jun 18, 2024 · e1d8b0e · e1d8b0e
1 parent d2888ec
commit e1d8b0e
Show file tree

Hide file tree

Showing 9 changed files with 51 additions and 20 deletions.
diff --git a/api/v1alpha2/bundle_types.go b/api/v1alpha2/bundle_types.go
@@ -67,6 +67,8 @@ type ImageSource struct {
 	// This should not be used in a production environment.
 	// +optional
 	InsecureSkipTLSVerify bool `json:"insecureSkipTLSVerify,omitempty"`
+	// CertificateData contains the PEM data of the certificate that is to be used for the TLS connection
+	CertificateData string `json:"certificateData,omitempty"`
 }
 
 type GitSource struct {

diff --git a/cmd/core/main.go b/cmd/core/main.go
@@ -199,7 +199,7 @@ func main() {
 		os.Exit(1)
 	}
 
-	unpacker, err := source.NewDefaultUnpacker(mgr, systemNamespace, unpackCacheDir, rootCAs)
+	unpacker, err := source.NewDefaultUnpacker(mgr, systemNamespace, unpackCacheDir)
 	if err != nil {
 		setupLog.Error(err, "unable to setup bundle unpacker")
 		os.Exit(1)

diff --git a/cmd/helm/main.go b/cmd/helm/main.go
@@ -189,7 +189,7 @@ func main() {
 		os.Exit(1)
 	}
 
-	unpacker, err := source.NewDefaultUnpacker(mgr, systemNamespace, unpackCacheDir, rootCAs)
+	unpacker, err := source.NewDefaultUnpacker(mgr, systemNamespace, unpackCacheDir)
 	if err != nil {
 		setupLog.Error(err, "unable to setup bundle unpacker")
 		os.Exit(1)

diff --git a/manifests/base/apis/crds/core.rukpak.io_bundledeployments.yaml b/manifests/base/apis/crds/core.rukpak.io_bundledeployments.yaml
@@ -223,6 +223,10 @@ spec:
                     description: Image is the bundle image that backs the content
                       of this bundle.
                     properties:
+                      certificateData:
+                        description: CertificateData contains the PEM data of the
+                          certificate that is to be used for the TLS connection
+                        type: string
                       insecureSkipTLSVerify:
                         description: |-
                           InsecureSkipTLSVerify indicates that TLS certificate validation should be skipped.
@@ -468,6 +472,10 @@ spec:
                     description: Image is the bundle image that backs the content
                       of this bundle.
                     properties:
+                      certificateData:
+                        description: CertificateData contains the PEM data of the
+                          certificate that is to be used for the TLS connection
+                        type: string
                       insecureSkipTLSVerify:
                         description: |-
                           InsecureSkipTLSVerify indicates that TLS certificate validation should be skipped.

diff --git a/pkg/source/image_registry.go b/pkg/source/image_registry.go
@@ -4,6 +4,7 @@ import (
 	"archive/tar"
 	"context"
 	"crypto/tls"
+	"crypto/x509"
 	"errors"
 	"fmt"
 	"io/fs"
@@ -64,14 +65,25 @@ func (i *ImageRegistry) Unpack(ctx context.Context, bundle *rukpakv1alpha2.Bundl
 		remoteOpts = append(remoteOpts, remote.WithAuthFromKeychain(authChain))
 	}
 
+	transport := remote.DefaultTransport.(*http.Transport).Clone()
+	if transport.TLSClientConfig == nil {
+		transport.TLSClientConfig = &tls.Config{
+			InsecureSkipVerify: false,
+			MinVersion:         tls.VersionTLS12,
+		} // nolint:gosec
+	}
 	if bundle.Spec.Source.Image.InsecureSkipTLSVerify {
-		insecureTransport := remote.DefaultTransport.(*http.Transport).Clone()
-		if insecureTransport.TLSClientConfig == nil {
-			insecureTransport.TLSClientConfig = &tls.Config{} // nolint:gosec
+		transport.TLSClientConfig.InsecureSkipVerify = true // nolint:gosec
+	}
+	if bundle.Spec.Source.Image.CertificateData != "" {
+		pool, err := x509.SystemCertPool()
+		if err != nil || pool == nil {
+			pool = x509.NewCertPool()
 		}
-		insecureTransport.TLSClientConfig.InsecureSkipVerify = true // nolint:gosec
-		remoteOpts = append(remoteOpts, remote.WithTransport(insecureTransport))
+		transport.TLSClientConfig.RootCAs = pool
+		transport.TLSClientConfig.RootCAs.AppendCertsFromPEM([]byte(bundle.Spec.Source.Image.CertificateData))
 	}
+	remoteOpts = append(remoteOpts, remote.WithTransport(transport))
 
 	digest, isDigest := imgRef.(name.Digest)
 	if isDigest {
@@ -141,6 +153,7 @@ func unpackedResult(fsys fs.FS, bundle *rukpakv1alpha2.BundleDeployment, ref str
 				Ref:                   ref,
 				ImagePullSecretName:   bundle.Spec.Source.Image.ImagePullSecretName,
 				InsecureSkipTLSVerify: bundle.Spec.Source.Image.InsecureSkipTLSVerify,
+				CertificateData:       bundle.Spec.Source.Image.CertificateData,
 			},
 		},
 		State: StateUnpacked,

diff --git a/pkg/source/unpacker.go b/pkg/source/unpacker.go
@@ -2,11 +2,8 @@ package source
 
 import (
 	"context"
-	"crypto/tls"
-	"crypto/x509"
 	"fmt"
 	"io/fs"
-	"net/http"
 
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 
@@ -103,14 +100,7 @@ func (s *unpacker) Cleanup(ctx context.Context, bundle *rukpakv1alpha2.BundleDep
 // source types.
 //
 // TODO: refactor NewDefaultUnpacker due to growing parameter list
-func NewDefaultUnpacker(mgr manager.Manager, namespace, cacheDir string, rootCAs *x509.CertPool) (Unpacker, error) {
-	httpTransport := http.DefaultTransport.(*http.Transport).Clone()
-	if httpTransport.TLSClientConfig == nil {
-		httpTransport.TLSClientConfig = &tls.Config{
-			MinVersion: tls.VersionTLS12,
-		}
-	}
-	httpTransport.TLSClientConfig.RootCAs = rootCAs
+func NewDefaultUnpacker(mgr manager.Manager, namespace, cacheDir string) (Unpacker, error) {
 	return NewUnpacker(map[rukpakv1alpha2.SourceType]Unpacker{
 		rukpakv1alpha2.SourceTypeImage: &ImageRegistry{
 			BaseCachePath: cacheDir,

diff --git a/test/e2e/registry_provisioner_test.go b/test/e2e/registry_provisioner_test.go
@@ -7,8 +7,10 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	rukpakv1alpha2 "github.com/operator-framework/rukpak/api/v1alpha2"
@@ -23,6 +25,19 @@ var _ = Describe("registry provisioner bundle", func() {
 		)
 		BeforeEach(func() {
 			ctx = context.Background()
+			var secret corev1.Secret
+
+			name := types.NamespacedName{
+				Name:      "rukpak-e2e-registry",
+				Namespace: "rukpak-e2e",
+			}
+			err := c.Get(ctx, name, &secret)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(secret.Type).To(Equal(corev1.SecretTypeTLS))
+			data, ok := secret.Data["ca.crt"]
+			Expect(ok).To(BeTrue())
+			Expect(data).ToNot(BeNil())
+			certData := string(data[:])
 
 			bd = &rukpakv1alpha2.BundleDeployment{
 				ObjectMeta: metav1.ObjectMeta{
@@ -38,12 +53,13 @@ var _ = Describe("registry provisioner bundle", func() {
 						Type: rukpakv1alpha2.SourceTypeImage,
 						Image: &rukpakv1alpha2.ImageSource{
 							Ref:                   fmt.Sprintf("%v/%v", ImageRepo, "registry:valid"),
-							InsecureSkipTLSVerify: true,
+							InsecureSkipTLSVerify: false,
+							CertificateData:       certData,
 						},
 					},
 				},
 			}
-			err := c.Create(ctx, bd)
+			err = c.Create(ctx, bd)
 			Expect(err).ToNot(HaveOccurred())
 		})
 		AfterEach(func() {

diff --git a/test/tools/imageregistry/image-registry-secure.sh b/test/tools/imageregistry/image-registry-secure.sh
@@ -47,6 +47,7 @@ spec:
   isCA: true
   dnsNames:
     - ${name}-secure.${namespace}.svc
+    - ${name}-secure.${namespace}.svc.cluster.local
   privateKey:
     algorithm: ECDSA
     size: 256

diff --git a/test/tools/imageregistry/image-registry.sh b/test/tools/imageregistry/image-registry.sh
@@ -47,6 +47,7 @@ spec:
   isCA: true
   dnsNames:
     - ${name}.${namespace}.svc
+    - ${name}.${namespace}.svc.cluster.local
   privateKey:
     algorithm: ECDSA
     size: 256