Skip to content

Commit

Permalink
Update certificate support
Browse files Browse the repository at this point in the history
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 <todd.short@me.com>
  • Loading branch information
tmshort committed Jun 18, 2024
1 parent d2888ec commit e1d8b0e
Show file tree
Hide file tree
Showing 9 changed files with 51 additions and 20 deletions.
2 changes: 2 additions & 0 deletions api/v1alpha2/bundle_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion cmd/core/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion cmd/helm/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
23 changes: 18 additions & 5 deletions pkg/source/image_registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"archive/tar"
"context"
"crypto/tls"
"crypto/x509"
"errors"
"fmt"
"io/fs"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down
12 changes: 1 addition & 11 deletions pkg/source/unpacker.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,8 @@ package source

import (
"context"
"crypto/tls"
"crypto/x509"
"fmt"
"io/fs"
"net/http"

"sigs.k8s.io/controller-runtime/pkg/manager"

Expand Down Expand Up @@ -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,
Expand Down
20 changes: 18 additions & 2 deletions test/e2e/registry_provisioner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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{
Expand All @@ -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() {
Expand Down
1 change: 1 addition & 0 deletions test/tools/imageregistry/image-registry-secure.sh
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ spec:
isCA: true
dnsNames:
- ${name}-secure.${namespace}.svc
- ${name}-secure.${namespace}.svc.cluster.local
privateKey:
algorithm: ECDSA
size: 256
Expand Down
1 change: 1 addition & 0 deletions test/tools/imageregistry/image-registry.sh
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ spec:
isCA: true
dnsNames:
- ${name}.${namespace}.svc
- ${name}.${namespace}.svc.cluster.local
privateKey:
algorithm: ECDSA
size: 256
Expand Down

0 comments on commit e1d8b0e

Please sign in to comment.