diff --git a/.gitignore b/.gitignore index 8c717ec1e5..e1b5e2f13b 100644 --- a/.gitignore +++ b/.gitignore @@ -88,8 +88,9 @@ bin **/go.work **/go.work.sum -# ignore .pem key files +# ignore .pem key files except those in tests/fixtures **/*.pem +!tests/fixtures/certificates/*.pem # rendered manifest **/manifest.yaml diff --git a/internal/controller/istiogatewaysecret/setup.go b/internal/controller/istiogatewaysecret/setup.go index 0c5de88ca3..6da436a567 100644 --- a/internal/controller/istiogatewaysecret/setup.go +++ b/internal/controller/istiogatewaysecret/setup.go @@ -20,6 +20,7 @@ import ( gatewaysecretclient "github.com/kyma-project/lifecycle-manager/internal/gatewaysecret/client" "github.com/kyma-project/lifecycle-manager/internal/gatewaysecret/legacy" "github.com/kyma-project/lifecycle-manager/internal/pkg/flags" + "github.com/kyma-project/lifecycle-manager/internal/service/watcher/certificate" "github.com/kyma-project/lifecycle-manager/pkg/queue" ) @@ -53,8 +54,11 @@ func SetupReconciler(mgr ctrl.Manager, if flagVar.UseLegacyStrategyForIstioGatewaySecret { handler = legacy.NewGatewaySecretHandler(clnt, parseLastModifiedFunc) } else { - handler = cabundle.NewGatewaySecretHandler(clnt, parseLastModifiedFunc, - flagVar.IstioGatewayServerCertSwitchGracePeriod) + handler = cabundle.NewGatewaySecretHandler(clnt, + parseLastModifiedFunc, + flagVar.IstioGatewayServerCertSwitchGracePeriod, + certificate.NewBundler(), + ) } var getSecretFunc GetterFunc = func(ctx context.Context, name types.NamespacedName) (*apicorev1.Secret, error) { diff --git a/internal/gatewaysecret/cabundle/handler.go b/internal/gatewaysecret/cabundle/handler.go index a1c1d85ef8..041689b27a 100644 --- a/internal/gatewaysecret/cabundle/handler.go +++ b/internal/gatewaysecret/cabundle/handler.go @@ -3,7 +3,7 @@ package cabundle import ( "context" "errors" - "slices" + "fmt" "time" apicorev1 "k8s.io/api/core/v1" @@ -20,19 +20,28 @@ const ( caBundleTempCertKey = "temp.ca.crt" ) +type Bundler interface { + Bundle(bundle *[]byte, cert []byte) (bool, error) + DropExpiredCerts(bundle *[]byte) (bool, error) +} + type Handler struct { client gatewaysecret.Client parseTimeFromAnnotationFunc gatewaysecret.TimeParserFunc serverCertSwitchGracePeriod time.Duration + bundler Bundler } -func NewGatewaySecretHandler(client gatewaysecret.Client, timeParserFunc gatewaysecret.TimeParserFunc, +func NewGatewaySecretHandler(client gatewaysecret.Client, + timeParserFunc gatewaysecret.TimeParserFunc, serverCertSwitchGracePeriod time.Duration, + bundler Bundler, ) *Handler { return &Handler{ client: client, parseTimeFromAnnotationFunc: timeParserFunc, serverCertSwitchGracePeriod: serverCertSwitchGracePeriod, + bundler: bundler, } } @@ -55,10 +64,20 @@ func (h *Handler) ManageGatewaySecret(ctx context.Context, rootSecret *apicorev1 // this is for the case when we switch existing secret from legacy to new rotation mechanism bootstrapLegacyGatewaySecret(gwSecret, rootSecret) - if h.requiresBundling(gwSecret, notBefore) { - bundleCACrt(gwSecret, rootSecret) + bundled, err := h.bundleCACerts(gwSecret, rootSecret) + if err != nil { + return err + } + + if bundled { setLastModifiedToNow(gwSecret) } + + err = h.dropExpiredCertsFromBundle(gwSecret) + if err != nil { + return err + } + if h.requiresCertSwitching(notBefore) { switchCertificate(gwSecret, rootSecret) } @@ -90,17 +109,6 @@ func (h *Handler) createGatewaySecretFromRootSecret(ctx context.Context, return h.client.CreateGatewaySecret(ctx, newSecret) } -func (h *Handler) requiresBundling(gwSecret *apicorev1.Secret, notBefore time.Time) bool { - // If the last modified time of the gateway secret is after the notBefore time of the CA certificate, - // then we don't need to update the gateway secret - if lastModified, err := h.parseTimeFromAnnotationFunc(gwSecret, shared.LastModifiedAtAnnotation); err == nil { - if lastModified.After(notBefore) { - return false - } - } - return true -} - func (h *Handler) requiresCertSwitching(caCertNotBefore time.Time) bool { // If the grace period after CA rotation has expired, then we need to switch the certificate and private key return time.Now().After(caCertNotBefore.Add(h.serverCertSwitchGracePeriod)) @@ -121,12 +129,35 @@ func setLastModifiedToNow(secret *apicorev1.Secret) { secret.Annotations[shared.LastModifiedAtAnnotation] = apimetav1.Now().Format(time.RFC3339) } -func bundleCACrt(gatewaySecret *apicorev1.Secret, rootSecret *apicorev1.Secret) { - gatewaySecret.Data[gatewaysecret.CACrt] = slices.Clone(rootSecret.Data[gatewaysecret.CACrt]) - gatewaySecret.Data[gatewaysecret.CACrt] = append(gatewaySecret.Data[gatewaysecret.CACrt], - gatewaySecret.Data[caBundleTempCertKey]...) +func (h *Handler) bundleCACerts(gatewaySecret *apicorev1.Secret, rootSecret *apicorev1.Secret) (bool, error) { + caBundle := gatewaySecret.Data[gatewaysecret.CACrt] + cert := rootSecret.Data[gatewaysecret.TLSCrt] // tls.crt and ca.crt are the same in the root secret + + bundled, err := h.bundler.Bundle(&caBundle, cert) + if err != nil { + return false, fmt.Errorf("failed to bundle root secret's tls.crt into gateway secret's ca.crt: %w", err) + } + + if !bundled { + return false, nil + } + + gatewaySecret.Data[gatewaysecret.CACrt] = caBundle + return true, nil +} + +func (h *Handler) dropExpiredCertsFromBundle(gatewaySecret *apicorev1.Secret) error { + caBundle := gatewaySecret.Data[gatewaysecret.CACrt] + certsDropped, err := h.bundler.DropExpiredCerts(&caBundle) + if err != nil { + return fmt.Errorf("failed to drop expired certs from gateway secret's ca.crt: %w", err) + } + + if certsDropped { + gatewaySecret.Data[gatewaysecret.CACrt] = caBundle + } - gatewaySecret.Data[caBundleTempCertKey] = rootSecret.Data[gatewaysecret.CACrt] + return nil } func switchCertificate(gatewaySecret *apicorev1.Secret, rootSecret *apicorev1.Secret) { diff --git a/internal/gatewaysecret/cabundle/handler_test.go b/internal/gatewaysecret/cabundle/handler_test.go index 48081935ec..baf3ccad3f 100644 --- a/internal/gatewaysecret/cabundle/handler_test.go +++ b/internal/gatewaysecret/cabundle/handler_test.go @@ -1,10 +1,12 @@ package cabundle_test import ( + "crypto/x509" "errors" "testing" "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" apicorev1 "k8s.io/api/core/v1" @@ -14,6 +16,8 @@ import ( "github.com/kyma-project/lifecycle-manager/internal/gatewaysecret" "github.com/kyma-project/lifecycle-manager/internal/gatewaysecret/cabundle" "github.com/kyma-project/lifecycle-manager/internal/gatewaysecret/testutils" + "github.com/kyma-project/lifecycle-manager/internal/service/watcher/certificate" + "github.com/kyma-project/lifecycle-manager/tests/fixtures/certificates" ) const gatewayServerCertSwitchGracePeriod = 90 * time.Minute @@ -33,7 +37,11 @@ func TestManageGatewaySecret_WhenGetWatcherServingCertValidityReturnsError_Retur someError := errors.New("some-error") mockClient.On("GetWatcherServingCertValidity", mock.Anything).Return(time.Time{}, time.Time{}, someError) - handler := cabundle.NewGatewaySecretHandler(mockClient, nil, gatewayServerCertSwitchGracePeriod) + handler := cabundle.NewGatewaySecretHandler(mockClient, + nil, + gatewayServerCertSwitchGracePeriod, + certificate.NewBundler(), + ) // ACT err := handler.ManageGatewaySecret(t.Context(), &apicorev1.Secret{}) @@ -49,7 +57,11 @@ func TestManageGatewaySecret_WhenGetWatcherServingCertValidityReturnsInvalidNotB mockClient := &testutils.ClientMock{} mockClient.On("GetWatcherServingCertValidity", mock.Anything).Return(time.Time{}, time.Now(), nil) - handler := cabundle.NewGatewaySecretHandler(mockClient, nil, gatewayServerCertSwitchGracePeriod) + handler := cabundle.NewGatewaySecretHandler(mockClient, + nil, + gatewayServerCertSwitchGracePeriod, + certificate.NewBundler(), + ) // ACT err := handler.ManageGatewaySecret(t.Context(), &apicorev1.Secret{}) @@ -67,7 +79,11 @@ func TestManageGatewaySecret_WhenGetGatewaySecretReturnsError_ReturnsError(t *te someError := errors.New("some-error") mockClient.On("GetGatewaySecret", mock.Anything).Return(nil, someError) - handler := cabundle.NewGatewaySecretHandler(mockClient, nil, gatewayServerCertSwitchGracePeriod) + handler := cabundle.NewGatewaySecretHandler(mockClient, + nil, + gatewayServerCertSwitchGracePeriod, + certificate.NewBundler(), + ) // ACT err := handler.ManageGatewaySecret(t.Context(), &apicorev1.Secret{}) @@ -92,7 +108,11 @@ func TestManageGatewaySecret_WhenGetGatewaySecretReturnsNotFoundError_CreatesGat "ca.crt": []byte("value3"), }, } - handler := cabundle.NewGatewaySecretHandler(mockClient, nil, gatewayServerCertSwitchGracePeriod) + handler := cabundle.NewGatewaySecretHandler(mockClient, + nil, + gatewayServerCertSwitchGracePeriod, + certificate.NewBundler(), + ) // ACT err := handler.ManageGatewaySecret(t.Context(), rootSecret) @@ -124,7 +144,11 @@ func TestManageGatewaySecret_WhenGetGatewaySecretReturnsNotFoundErrorAndCreation expectedError := errors.New("some-error") mockClient.On("CreateGatewaySecret", mock.Anything, mock.Anything).Return(expectedError) - handler := cabundle.NewGatewaySecretHandler(mockClient, nil, gatewayServerCertSwitchGracePeriod) + handler := cabundle.NewGatewaySecretHandler(mockClient, + nil, + gatewayServerCertSwitchGracePeriod, + certificate.NewBundler(), + ) // ACT err := handler.ManageGatewaySecret(t.Context(), &apicorev1.Secret{}) @@ -142,19 +166,23 @@ func TestManageGatewaySecret_WhenLegacySecret_BootstrapsLegacyGatewaySecret(t *t mockClient.On("GetWatcherServingCertValidity", mock.Anything).Return(notBefore, notAfter, nil) mockClient.On("GetGatewaySecret", mock.Anything).Return(&apicorev1.Secret{ Data: map[string][]byte{ - "tls.crt": []byte("value1"), + "tls.crt": certificates.Cert1, "tls.key": []byte("value2"), - "ca.crt": []byte("value3"), + "ca.crt": certificates.Cert1, }, }, nil) mockClient.On("UpdateGatewaySecret", mock.Anything, mock.Anything).Return(nil) timeParserFunction := getTimeParserFunction(false) - handler := cabundle.NewGatewaySecretHandler(mockClient, timeParserFunction, gatewayServerCertSwitchGracePeriod) + handler := cabundle.NewGatewaySecretHandler(mockClient, + timeParserFunction, + gatewayServerCertSwitchGracePeriod, + certificate.NewBundler(), + ) rootSecret := &apicorev1.Secret{ Data: map[string][]byte{ - "tls.crt": []byte("value1"), + "tls.crt": certificates.Cert2, "tls.key": []byte("value2"), - "ca.crt": []byte("value3"), + "ca.crt": certificates.Cert2, }, } @@ -164,32 +192,32 @@ func TestManageGatewaySecret_WhenLegacySecret_BootstrapsLegacyGatewaySecret(t *t // ASSERT require.NoError(t, err) mockClient.AssertNumberOfCalls(t, "UpdateGatewaySecret", 1) - mockClient.AssertCalled(t, "UpdateGatewaySecret", mock.Anything, mock.MatchedBy( - func(secret *apicorev1.Secret) bool { - return string(secret.Data["temp.ca.crt"]) == "value3" - })) } -func TestManageGatewaySecret_WhenRequiresBundling_BundlesGatewaySecretWithRootSecretCA(t *testing.T) { +func TestManageGatewaySecret_BundlesAndDropsCerts(t *testing.T) { // ARRANGE mockClient := &testutils.ClientMock{} mockClient.On("GetWatcherServingCertValidity", mock.Anything).Return(notBefore, notAfter, nil) mockClient.On("GetGatewaySecret", mock.Anything).Return(&apicorev1.Secret{ Data: map[string][]byte{ - "tls.crt": []byte("value1"), + "tls.crt": certificates.Cert1, "tls.key": []byte("value2"), - "ca.crt": []byte("value3"), + "ca.crt": append(certificates.Cert1, certificates.CertExpired...), "temp.ca.crt": []byte("value3"), }, }, nil) mockClient.On("UpdateGatewaySecret", mock.Anything, mock.Anything).Return(nil) timeParserFunction := getTimeParserFunction(true) - handler := cabundle.NewGatewaySecretHandler(mockClient, timeParserFunction, gatewayServerCertSwitchGracePeriod) + handler := cabundle.NewGatewaySecretHandler(mockClient, + timeParserFunction, + gatewayServerCertSwitchGracePeriod, + certificate.NewBundler(), + ) rootSecret := &apicorev1.Secret{ Data: map[string][]byte{ - "tls.crt": []byte("new-value1"), + "tls.crt": certificates.Cert2, "tls.key": []byte("new-value2"), - "ca.crt": []byte("new-value3"), + "ca.crt": certificates.Cert2, }, } @@ -201,10 +229,9 @@ func TestManageGatewaySecret_WhenRequiresBundling_BundlesGatewaySecretWithRootSe mockClient.AssertNumberOfCalls(t, "UpdateGatewaySecret", 1) mockClient.AssertCalled(t, "UpdateGatewaySecret", mock.Anything, mock.MatchedBy( func(secret *apicorev1.Secret) bool { - return string(secret.Data["tls.crt"]) == "value1" && + return string(secret.Data["tls.crt"]) == string(certificates.Cert1) && string(secret.Data["tls.key"]) == "value2" && - string(secret.Data["ca.crt"]) == "new-value3value3" && - string(secret.Data["temp.ca.crt"]) == "new-value3" + string(secret.Data["ca.crt"]) == string(append(certificates.Cert2, certificates.Cert1...)) })) } @@ -214,21 +241,25 @@ func TestManageGatewaySecret_WhenUpdateSecretFails_ReturnsError(t *testing.T) { mockClient.On("GetWatcherServingCertValidity", mock.Anything).Return(notBefore, notAfter, nil) mockClient.On("GetGatewaySecret", mock.Anything).Return(&apicorev1.Secret{ Data: map[string][]byte{ - "tls.crt": []byte("value1"), + "tls.crt": certificates.Cert1, "tls.key": []byte("value2"), - "ca.crt": []byte("value3"), + "ca.crt": certificates.Cert1, "temp.ca.crt": []byte("value3"), }, }, nil) expectedError := errors.New("some-error") mockClient.On("UpdateGatewaySecret", mock.Anything, mock.Anything).Return(expectedError) timeParserFunction := getTimeParserFunction(true) - handler := cabundle.NewGatewaySecretHandler(mockClient, timeParserFunction, gatewayServerCertSwitchGracePeriod) + handler := cabundle.NewGatewaySecretHandler(mockClient, + timeParserFunction, + gatewayServerCertSwitchGracePeriod, + certificate.NewBundler(), + ) rootSecret := &apicorev1.Secret{ Data: map[string][]byte{ - "tls.crt": []byte("new-value1"), + "tls.crt": certificates.Cert2, "tls.key": []byte("new-value2"), - "ca.crt": []byte("new-value3"), + "ca.crt": certificates.Cert2, }, } @@ -248,21 +279,25 @@ func TestManageGatewaySecret_WhenRequiresCertSwitching_SwitchesTLSCertAndKeyWith mockClient.On("GetWatcherServingCertValidity", mock.Anything).Return(notBefore, notAfter, nil) mockClient.On("GetGatewaySecret", mock.Anything).Return(&apicorev1.Secret{ Data: map[string][]byte{ - "tls.crt": []byte("value1"), + "tls.crt": certificates.Cert1, "tls.key": []byte("value2"), - "ca.crt": []byte("new-value3value3"), + "ca.crt": certificates.Cert2, "temp.ca.crt": []byte("new-value3"), }, }, nil) mockClient.On("UpdateGatewaySecret", mock.Anything, mock.Anything).Return(nil) timeParserFunction := getTimeParserFunction(false) expiredServerCertSwitchGracePeriod := 30 * time.Minute - handler := cabundle.NewGatewaySecretHandler(mockClient, timeParserFunction, expiredServerCertSwitchGracePeriod) + handler := cabundle.NewGatewaySecretHandler(mockClient, + timeParserFunction, + expiredServerCertSwitchGracePeriod, + certificate.NewBundler(), + ) rootSecret := &apicorev1.Secret{ Data: map[string][]byte{ - "tls.crt": []byte("new-value1"), + "tls.crt": certificates.Cert2, "tls.key": []byte("new-value2"), - "ca.crt": []byte("new-value3"), + "ca.crt": certificates.Cert2, }, } @@ -274,13 +309,86 @@ func TestManageGatewaySecret_WhenRequiresCertSwitching_SwitchesTLSCertAndKeyWith mockClient.AssertNumberOfCalls(t, "UpdateGatewaySecret", 1) mockClient.AssertCalled(t, "UpdateGatewaySecret", mock.Anything, mock.MatchedBy( func(secret *apicorev1.Secret) bool { - return string(secret.Data["tls.crt"]) == "new-value1" && + return string(secret.Data["tls.crt"]) == string(certificates.Cert2) && string(secret.Data["tls.key"]) == "new-value2" && - string(secret.Data["ca.crt"]) == "new-value3value3" && - string(secret.Data["temp.ca.crt"]) == "new-value3" + string(secret.Data["ca.crt"]) == string(certificates.Cert2) })) } +func TestManageGatewaySecret_WhenBundlingFails_ReturnsError(t *testing.T) { + // ARRANGE + mockClient := &testutils.ClientMock{} + mockClient.On("GetWatcherServingCertValidity", mock.Anything).Return(notBefore, notAfter, nil) + mockClient.On("GetGatewaySecret", mock.Anything).Return(&apicorev1.Secret{ + Data: map[string][]byte{ + "tls.crt": certificates.Cert1, + "tls.key": []byte("value2"), + "ca.crt": []byte("invalid bundle"), + "temp.ca.crt": []byte("value3"), + }, + }, nil) + mockClient.On("UpdateGatewaySecret", mock.Anything, mock.Anything).Return(assert.AnError) + handler := cabundle.NewGatewaySecretHandler(mockClient, + getTimeParserFunction(false), + gatewayServerCertSwitchGracePeriod, + certificate.NewBundler(), + ) + rootSecret := &apicorev1.Secret{ + Data: map[string][]byte{ + "tls.crt": certificates.Cert2, + "tls.key": []byte("new-value2"), + "ca.crt": certificates.Cert2, + }, + } + + // ACT + err := handler.ManageGatewaySecret(t.Context(), rootSecret) + + // ASSERT + require.ErrorIs(t, err, certificate.ErrInvalidPEM) + mockClient.AssertNumberOfCalls(t, "UpdateGatewaySecret", 0) +} + +func TestManageGatewaySecret_WhenDroppingExpiredCertsFails_ReturnsError(t *testing.T) { + // ARRANGE + mockClient := &testutils.ClientMock{} + mockClient.On("GetWatcherServingCertValidity", mock.Anything).Return(notBefore, notAfter, nil) + mockClient.On("GetGatewaySecret", mock.Anything).Return(&apicorev1.Secret{ + Data: map[string][]byte{ + "tls.crt": certificates.Cert1, + "tls.key": []byte("value2"), + "ca.crt": certificates.Cert1, + "temp.ca.crt": []byte("value3"), + }, + }, nil) + mockClient.On("UpdateGatewaySecret", mock.Anything, mock.Anything).Return(assert.AnError) + handler := cabundle.NewGatewaySecretHandler(mockClient, + getTimeParserFunction(false), + gatewayServerCertSwitchGracePeriod, + certificate.NewBundler(certificate.WithParseX509Function( + func(_ []byte) (*x509.Certificate, error) { + return &x509.Certificate{ + NotAfter: time.Time{}, + }, nil + }, + )), + ) + rootSecret := &apicorev1.Secret{ + Data: map[string][]byte{ + "tls.crt": certificates.Cert2, + "tls.key": []byte("new-value2"), + "ca.crt": certificates.Cert2, + }, + } + + // ACT + err := handler.ManageGatewaySecret(t.Context(), rootSecret) + + // ASSERT + require.ErrorIs(t, err, certificate.ErrX509NotAfterIsZero) + mockClient.AssertNumberOfCalls(t, "UpdateGatewaySecret", 0) +} + func getTimeParserFunction(bundlingRequired bool) gatewaysecret.TimeParserFunc { var lastModifiedAt time.Time diff --git a/internal/service/watcher/certificate/bundler.go b/internal/service/watcher/certificate/bundler.go new file mode 100644 index 0000000000..c50fb0b7e7 --- /dev/null +++ b/internal/service/watcher/certificate/bundler.go @@ -0,0 +1,175 @@ +package certificate + +import ( + "crypto/x509" + "encoding/pem" + "errors" + "fmt" + "time" +) + +const ( + certificateBlockType = "CERTIFICATE" +) + +var ( + ErrInvalidPEM = errors.New("invalid PEM") + ErrFailedToParseX509 = errors.New("failed to parse x509 certificate") + ErrX509NotAfterIsZero = errors.New("x509 certificate NotAfter is zero") +) + +type Bundler struct { + parseX509Func func([]byte) (*x509.Certificate, error) +} + +func NewBundler(opts ...func(*Bundler) *Bundler) *Bundler { + bundler := &Bundler{ + parseX509Func: x509.ParseCertificate, + } + + for _, opt := range opts { + bundler = opt(bundler) + } + + return bundler +} + +// NOTE: LOW LEVEL PRIMITIVE! +// Use only if intended to exchange the default x509.ParseCertificate function. +func WithParseX509Function(f func([]byte) (*x509.Certificate, error)) func(*Bundler) *Bundler { + return func(b *Bundler) *Bundler { + b.parseX509Func = f + return b + } +} + +// Bundle adds the given cert to the bundle. +// It returns true if the cert was added, false if it was already present. +func (b Bundler) Bundle(bundle *[]byte, newCert []byte) (bool, error) { + newCertPEM, _ := pem.Decode(newCert) + if newCertPEM == nil { + return false, fmt.Errorf("invalid newCert: %w", ErrInvalidPEM) + } + + certsPEM, err := decodeCertsToPEM(*bundle) + if err != nil { + return false, fmt.Errorf("invalid bundle: %w", err) + } + + if containsCert(certsPEM, newCertPEM) { + return false, nil + } + + certsPEM = prependCert(certsPEM, newCertPEM) + + *bundle = encodePEMToBytes(certsPEM) + return true, nil +} + +// DropExpiredCerts removes expired certs from the bundle. +// It returns true if any certs were removed, false otherwise. +func (b Bundler) DropExpiredCerts(bundle *[]byte) (bool, error) { + certsPEM, err := decodeCertsToPEM(*bundle) + if err != nil { + return false, err + } + + certsX509, err := b.decodePEMToX509(certsPEM) + if err != nil { + return false, err + } + + unexpiredCertsX509, err := dropExpiredCerts(certsX509) + if err != nil || len(unexpiredCertsX509) == len(certsX509) { + return false, err + } + + *bundle = encodePEMToBytes(encodeX509ToPEM(unexpiredCertsX509)) + + return true, nil +} + +func decodeCertsToPEM(bundle []byte) ([]*pem.Block, error) { + var certs []*pem.Block + rest := bundle + for len(rest) > 0 { + var cert *pem.Block + cert, rest = pem.Decode(rest) + if cert != nil { + certs = append(certs, cert) + } else { + return nil, ErrInvalidPEM + } + } + return certs, nil +} + +func (b Bundler) decodePEMToX509(certsPEM []*pem.Block) ([]*x509.Certificate, error) { + certsX509 := make([]*x509.Certificate, len(certsPEM)) + for i, certBlock := range certsPEM { + cert, err := b.parseX509Func(certBlock.Bytes) + if err != nil { + return nil, errors.Join(ErrFailedToParseX509, err) + } + certsX509[i] = cert + } + return certsX509, nil +} + +func encodeX509ToPEM(certsX509 []*x509.Certificate) []*pem.Block { + certsPEM := make([]*pem.Block, len(certsX509)) + for i, cert := range certsX509 { + certBlock := &pem.Block{ + Type: certificateBlockType, + Bytes: cert.Raw, + } + certsPEM[i] = certBlock + } + return certsPEM +} + +func containsCert(certs []*pem.Block, newCert *pem.Block) bool { + for _, cert := range certs { + if string(cert.Bytes) == string(newCert.Bytes) { + return true + } + } + return false +} + +func prependCert(certs []*pem.Block, newCert *pem.Block) []*pem.Block { + newBundle := []*pem.Block{newCert} + newBundle = append(newBundle, certs...) + return newBundle +} + +func encodePEMToBytes(certs []*pem.Block) []byte { + var bundle []byte + for _, cert := range certs { + bundle = append(bundle, pem.EncodeToMemory(cert)...) + } + return bundle +} + +func dropExpiredCerts(certs []*x509.Certificate) ([]*x509.Certificate, error) { + var validCerts []*x509.Certificate + for _, cert := range certs { + isExpired, err := expired(cert) + if err != nil { + return nil, err + } + + if !isExpired { + validCerts = append(validCerts, cert) + } + } + return validCerts, nil +} + +func expired(cert *x509.Certificate) (bool, error) { + if cert.NotAfter.IsZero() { + return false, ErrX509NotAfterIsZero + } + + return cert.NotAfter.Before(time.Now()), nil +} diff --git a/internal/service/watcher/certificate/bundler_test.go b/internal/service/watcher/certificate/bundler_test.go new file mode 100644 index 0000000000..94f11b2b7c --- /dev/null +++ b/internal/service/watcher/certificate/bundler_test.go @@ -0,0 +1,255 @@ +package certificate_test + +import ( + "crypto/x509" + "errors" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/kyma-project/lifecycle-manager/internal/service/watcher/certificate" + "github.com/kyma-project/lifecycle-manager/tests/fixtures/certificates" +) + +var ( + cert1 = certificates.Cert1 + cert2 = certificates.Cert2 + cert3 = certificates.Cert3 + certExpired = certificates.CertExpired +) + +func Test_AddCert_AddsCert(t *testing.T) { + bundle := appendCerts(cert2, cert1) + expectedBundle := appendCerts(cert3, cert2, cert1) + + bndlr := certificate.NewBundler() + + added, err := bndlr.Bundle(&bundle, cert3) + + require.NoError(t, err) + assert.True(t, added) + assert.Equal(t, expectedBundle, bundle) +} + +func Test_AddCert_AddsCertToEmptyBundle(t *testing.T) { + bundle := []byte{} + expectedBundle := cert1 + + bndlr := certificate.NewBundler() + + added, err := bndlr.Bundle(&bundle, cert1) + + require.NoError(t, err) + assert.True(t, added) + assert.Equal(t, expectedBundle, bundle) +} + +func Test_AddCert_AddsCertToNilBundle(t *testing.T) { + var bundle []byte + expectedBundle := cert1 + + bndlr := certificate.NewBundler() + + added, err := bndlr.Bundle(&bundle, cert1) + + require.NoError(t, err) + assert.True(t, added) + assert.Equal(t, expectedBundle, bundle) +} + +func Test_AddCert_NoOpOnExistingCert(t *testing.T) { + bundle := appendCerts(cert3, cert2, cert1) + expectedBundle := appendCerts(cert3, cert2, cert1) + + bndlr := certificate.NewBundler() + + added, err := bndlr.Bundle(&bundle, cert3) + + require.NoError(t, err) + assert.False(t, added) + assert.Equal(t, expectedBundle, bundle) +} + +func Test_Bundle_NoOpOnEmptyCert(t *testing.T) { + bundle := appendCerts(cert2, cert1) + expectedBundle := appendCerts(cert2, cert1) + + bndlr := certificate.NewBundler() + + added, err := bndlr.Bundle(&bundle, []byte("")) + + require.ErrorIs(t, err, certificate.ErrInvalidPEM) + require.Contains(t, err.Error(), "newCert") + assert.False(t, added) + assert.Equal(t, expectedBundle, bundle) +} + +func Test_Bundle_NoOpOnNilCert(t *testing.T) { + bundle := appendCerts(cert2, cert1) + expectedBundle := appendCerts(cert2, cert1) + + bndlr := certificate.NewBundler() + + added, err := bndlr.Bundle(&bundle, nil) + + require.ErrorIs(t, err, certificate.ErrInvalidPEM) + require.Contains(t, err.Error(), "newCert") + assert.False(t, added) + assert.Equal(t, expectedBundle, bundle) +} + +func Test_Bundle_NoOpOnInvalidCert(t *testing.T) { + bundle := appendCerts(cert2, cert1) + expectedBundle := appendCerts(cert2, cert1) + + bndlr := certificate.NewBundler() + + added, err := bndlr.Bundle(&bundle, []byte("invalid cert")) + + require.ErrorIs(t, err, certificate.ErrInvalidPEM) + require.Contains(t, err.Error(), "newCert") + assert.False(t, added) + assert.Equal(t, expectedBundle, bundle) +} + +func Test_Bundle_NoOpOnInvalidBundle(t *testing.T) { + bundle := []byte("invalid bundle") + expectedBundle := []byte("invalid bundle") + + bndlr := certificate.NewBundler() + + added, err := bndlr.Bundle(&bundle, cert1) + + require.ErrorIs(t, err, certificate.ErrInvalidPEM) + require.Contains(t, err.Error(), "bundle") + assert.False(t, added) + assert.Equal(t, expectedBundle, bundle) +} + +func Test_Bundle_NoOpOnBundleWithInvalidParts(t *testing.T) { + bundle := appendCerts(cert1, []byte("invalid string")) + expectedBundle := appendCerts(cert1, []byte("invalid string")) + + bndlr := certificate.NewBundler() + + added, err := bndlr.Bundle(&bundle, cert1) + + require.ErrorIs(t, err, certificate.ErrInvalidPEM) + require.Contains(t, err.Error(), "bundle") + assert.False(t, added) + assert.Equal(t, expectedBundle, bundle) +} + +func Test_DropExpiredCerts_DropsExpiredCerts(t *testing.T) { + bundle := appendCerts(cert2, certExpired, cert1) + expectedBundle := appendCerts(cert2, cert1) + + bndlr := certificate.NewBundler() + + dropped, err := bndlr.DropExpiredCerts(&bundle) + + require.NoError(t, err) + assert.True(t, dropped) + assert.Equal(t, expectedBundle, bundle) +} + +func Test_DropExpiredCerts_NoOpOnNoExpiredCerts(t *testing.T) { + bundle := appendCerts(cert2, cert1) + expectedBundle := appendCerts(cert2, cert1) + + bndlr := certificate.NewBundler() + + dropped, err := bndlr.DropExpiredCerts(&bundle) + + require.NoError(t, err) + assert.False(t, dropped) + assert.Equal(t, expectedBundle, bundle) +} + +func Test_DropExpiredCerts_NoOpOnEmptyBundle(t *testing.T) { + bundle := []byte{} + expectedBundle := []byte{} + + bndlr := certificate.NewBundler() + + dropped, err := bndlr.DropExpiredCerts(&bundle) + + require.NoError(t, err) + assert.False(t, dropped) + assert.Equal(t, expectedBundle, bundle) +} + +func Test_DropExpiredCerts_NoOpOnNilBundle(t *testing.T) { + var bundle []byte + + bndlr := certificate.NewBundler() + + dropped, err := bndlr.DropExpiredCerts(&bundle) + + require.NoError(t, err) + assert.False(t, dropped) + assert.Nil(t, bundle) +} + +func Test_DropExpiredCerts_NoOpOnInvalidBundle(t *testing.T) { + bundle := appendCerts(cert2, []byte("invalid string")) + expectedBundle := appendCerts(cert2, []byte("invalid string")) + + bndlr := certificate.NewBundler() + + dropped, err := bndlr.DropExpiredCerts(&bundle) + + require.ErrorIs(t, err, certificate.ErrInvalidPEM) + assert.False(t, dropped) + assert.Equal(t, expectedBundle, bundle) +} + +func Test_DropExpiredCerts_NoOpOnErrorParsingX509(t *testing.T) { + bundle := appendCerts(cert2, cert1) + expectedBundle := appendCerts(cert2, cert1) + + bndlr := certificate.NewBundler( + certificate.WithParseX509Function( + func(_ []byte) (*x509.Certificate, error) { + return nil, errors.New("error parsing x509") + }, + ), + ) + + dropped, err := bndlr.DropExpiredCerts(&bundle) + + require.ErrorIs(t, err, certificate.ErrFailedToParseX509) + assert.False(t, dropped) + assert.Equal(t, expectedBundle, bundle) +} + +func Test_DropExpiredCerts_NoOpOnEmptyNotBefore(t *testing.T) { + bundle := appendCerts(cert2, cert1) + expectedBundle := appendCerts(cert2, cert1) + + bndlr := certificate.NewBundler( + certificate.WithParseX509Function( + func(_ []byte) (*x509.Certificate, error) { + return &x509.Certificate{ + NotAfter: time.Time{}, + }, nil + }, + ), + ) + + dropped, err := bndlr.DropExpiredCerts(&bundle) + + require.ErrorIs(t, err, certificate.ErrX509NotAfterIsZero) + assert.False(t, dropped) + assert.Equal(t, expectedBundle, bundle) +} + +func appendCerts(certs ...[]byte) []byte { + var appended []byte + for _, cert := range certs { + appended = append(appended, cert...) + } + return appended +} diff --git a/tests/fixtures/certificates/certificate-expired.pem b/tests/fixtures/certificates/certificate-expired.pem new file mode 100644 index 0000000000..1013f97cac --- /dev/null +++ b/tests/fixtures/certificates/certificate-expired.pem @@ -0,0 +1,22 @@ +-----BEGIN CERTIFICATE----- +MIIDnzCCAoegAwIBAgIUNuDw8CFDFJ1UYiCQ9eqojITp6VIwDQYJKoZIhvcNAQEL +BQAwXzELMAkGA1UEBhMCREUxCzAJBgNVBAgMAkJZMQ8wDQYDVQQHDAZNdW5pY2gx +DzANBgNVBAoMBlNBUCBTRTENMAsGA1UECwwES3ltYTESMBAGA1UEAwwJamVsbHlm +aXNoMB4XDTI2MDIwNDEwMjQ0MVoXDTI2MDIwNTEwMjQ0MVowXzELMAkGA1UEBhMC +REUxCzAJBgNVBAgMAkJZMQ8wDQYDVQQHDAZNdW5pY2gxDzANBgNVBAoMBlNBUCBT +RTENMAsGA1UECwwES3ltYTESMBAGA1UEAwwJamVsbHlmaXNoMIIBIjANBgkqhkiG +9w0BAQEFAAOCAQ8AMIIBCgKCAQEAr9n2M+lQVw76OE78u+drs0u1Wa3pht8xzDt3 +BdjE7foNa6MXaWeUAA1OkhgZ/TsyuWIGt3XeK+ZeXZQY2lamBaNYJAHnnPECVF0L +6D92sxnapOAGj0zcrrWcsV8+vqmChNeIZAjZxf+oyYOgLCTPPjNQOHZrxPmUHbdw +GbXLb5FKplGzJgivwEtV5RQI7a8ccdvZUd6YpRrLqCriKJy59jW1E++FrlbgP7Ik +q+HiwEDFmf+9XIgHRWoiMd/mA+8MEY8XEvAj1HC9Tq39ub+5RA0SovudS3ETUopJ +oq8gSFZb5fcGf4VtR0m/5f2hn0PAAafz3e2CPYPfVJfP9muPxwIDAQABo1MwUTAd +BgNVHQ4EFgQUp9vCF0Ib77Vlur6M6k+rDWQnU68wHwYDVR0jBBgwFoAUp9vCF0Ib +77Vlur6M6k+rDWQnU68wDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOC +AQEArMPp714aiLFett6uQHeXvPIwcM6S9/EIS5UUrxBHQHB4pfPNqnt/KovVqmAR +Dg0SXLNW6HHLxDsGSbu3Wb1t+fHwKWvvBT/e2bNNj3YfYfBb2NZON0CgVH0g0oBX +lr70h5qIp8auxbMkQeLP5qI30QfJCJYVKXQVLMLa5XCiUAVSHuJPA2ddBFv5g9vY +NoAy4JwrBSK4f03+USTA0goeGWn4PyxGblvcAZYirAoaIsVbzg2nL4ZuHtlpzPIc +4rX7EfoOerb42tJ2gjluhN+MbT3YHIpdfy6/fFaH6YXFM7HwSP0LalqmTq/kwETf +0uIafAtLQZsgSZrVFidt2Om/aA== +-----END CERTIFICATE----- diff --git a/tests/fixtures/certificates/certificate-valid-1.pem b/tests/fixtures/certificates/certificate-valid-1.pem new file mode 100644 index 0000000000..4fe5679bad --- /dev/null +++ b/tests/fixtures/certificates/certificate-valid-1.pem @@ -0,0 +1,22 @@ +-----BEGIN CERTIFICATE----- +MIIDnzCCAoegAwIBAgIUAc9uOH5wl3Nbl3iZXUyQf/esTOQwDQYJKoZIhvcNAQEL +BQAwXzELMAkGA1UEBhMCREUxCzAJBgNVBAgMAkJZMQ8wDQYDVQQHDAZNdW5pY2gx +DzANBgNVBAoMBlNBUCBTRTENMAsGA1UECwwES3ltYTESMBAGA1UEAwwJamVsbHlm +aXNoMB4XDTI2MDIwNDEwMTcwN1oXDTM2MDIwMjEwMTcwN1owXzELMAkGA1UEBhMC +REUxCzAJBgNVBAgMAkJZMQ8wDQYDVQQHDAZNdW5pY2gxDzANBgNVBAoMBlNBUCBT +RTENMAsGA1UECwwES3ltYTESMBAGA1UEAwwJamVsbHlmaXNoMIIBIjANBgkqhkiG +9w0BAQEFAAOCAQ8AMIIBCgKCAQEAr9n2M+lQVw76OE78u+drs0u1Wa3pht8xzDt3 +BdjE7foNa6MXaWeUAA1OkhgZ/TsyuWIGt3XeK+ZeXZQY2lamBaNYJAHnnPECVF0L +6D92sxnapOAGj0zcrrWcsV8+vqmChNeIZAjZxf+oyYOgLCTPPjNQOHZrxPmUHbdw +GbXLb5FKplGzJgivwEtV5RQI7a8ccdvZUd6YpRrLqCriKJy59jW1E++FrlbgP7Ik +q+HiwEDFmf+9XIgHRWoiMd/mA+8MEY8XEvAj1HC9Tq39ub+5RA0SovudS3ETUopJ +oq8gSFZb5fcGf4VtR0m/5f2hn0PAAafz3e2CPYPfVJfP9muPxwIDAQABo1MwUTAd +BgNVHQ4EFgQUp9vCF0Ib77Vlur6M6k+rDWQnU68wHwYDVR0jBBgwFoAUp9vCF0Ib +77Vlur6M6k+rDWQnU68wDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOC +AQEAQSFA3yQqhiMYOwBWbEHfW+Y7llWmece/Rmkg+mHwWs1sFAAKokGOPQgji7kN +b3D/Zc4/XJja4uKVp7jfbyNyQuZ44m/mpSa7G+v6h7eNa4IkxFj8AmKinyzMl8ID +dvs9kagyu8v+zUqGJeBhMtnOZR/MaFP8vnPrukMdZt8cVsc4bX3zFwXws0RNMjHa +scb1wMoAT+YkAxq2ukc0mUSMjFCv/aVR1UOPmQyHuZMo9Tzs3y5ZkrbFlf5+OniN +EXNqHRSTQh6kGHq6I63zAoSCPi0NOMSl5+zWXQpFAGMEwnH0kJkV7U61PFOSZ2FF +t92Dv8IXyshLc8o2FG34vq74Pw== +-----END CERTIFICATE----- diff --git a/tests/fixtures/certificates/certificate-valid-2.pem b/tests/fixtures/certificates/certificate-valid-2.pem new file mode 100644 index 0000000000..0a53456c2b --- /dev/null +++ b/tests/fixtures/certificates/certificate-valid-2.pem @@ -0,0 +1,22 @@ +-----BEGIN CERTIFICATE----- +MIIDnzCCAoegAwIBAgIUI0HLiDa0X1aDjx7gVHq0pTunQEQwDQYJKoZIhvcNAQEL +BQAwXzELMAkGA1UEBhMCREUxCzAJBgNVBAgMAkJZMQ8wDQYDVQQHDAZNdW5pY2gx +DzANBgNVBAoMBlNBUCBTRTENMAsGA1UECwwES3ltYTESMBAGA1UEAwwJamVsbHlm +aXNoMB4XDTI2MDIwNDEwMTgwMFoXDTM2MDIwMjEwMTgwMFowXzELMAkGA1UEBhMC +REUxCzAJBgNVBAgMAkJZMQ8wDQYDVQQHDAZNdW5pY2gxDzANBgNVBAoMBlNBUCBT +RTENMAsGA1UECwwES3ltYTESMBAGA1UEAwwJamVsbHlmaXNoMIIBIjANBgkqhkiG +9w0BAQEFAAOCAQ8AMIIBCgKCAQEAr9n2M+lQVw76OE78u+drs0u1Wa3pht8xzDt3 +BdjE7foNa6MXaWeUAA1OkhgZ/TsyuWIGt3XeK+ZeXZQY2lamBaNYJAHnnPECVF0L +6D92sxnapOAGj0zcrrWcsV8+vqmChNeIZAjZxf+oyYOgLCTPPjNQOHZrxPmUHbdw +GbXLb5FKplGzJgivwEtV5RQI7a8ccdvZUd6YpRrLqCriKJy59jW1E++FrlbgP7Ik +q+HiwEDFmf+9XIgHRWoiMd/mA+8MEY8XEvAj1HC9Tq39ub+5RA0SovudS3ETUopJ +oq8gSFZb5fcGf4VtR0m/5f2hn0PAAafz3e2CPYPfVJfP9muPxwIDAQABo1MwUTAd +BgNVHQ4EFgQUp9vCF0Ib77Vlur6M6k+rDWQnU68wHwYDVR0jBBgwFoAUp9vCF0Ib +77Vlur6M6k+rDWQnU68wDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOC +AQEAmKPxASNR5e/HB2gn/ZSMoqMjeGRMU2P/XlieYB5SbzrZR5sAGn3myEY0SyVH +xfKlM4DPumMbamWjVHuFHBaBLbfzcJKMkxlGLgeL6ZETIK1N4PSFwGZf8WjIeQwT +Qaqf9uqkJBbZ2hRu7B4GjVy/eAdf87nzomtzhhbgUU/veyidJr0dsnhKdwolxPFR +frcW8SE+aoIxv5UMLvEC/9oLD+DCxvDstWUV8QCq8sqIipdlxFf5XFELBzeldI2j +kIe9aiKvPGKcgKcoD0uRJrrx8wydP69fUFhO0S00X2mgzdz4Z9qCXl0ty1Xktws4 +DtLryDWROjIt9ueG8rnib+wSRA== +-----END CERTIFICATE----- diff --git a/tests/fixtures/certificates/certificate-valid-3.pem b/tests/fixtures/certificates/certificate-valid-3.pem new file mode 100644 index 0000000000..cf32f8c24b --- /dev/null +++ b/tests/fixtures/certificates/certificate-valid-3.pem @@ -0,0 +1,21 @@ +-----BEGIN CERTIFICATE----- +MIIDfTCCAmWgAwIBAgIUa+97T7FZ+dPmugP0vY0063baul8wDQYJKoZIhvcNAQEL +BQAwTjELMAkGA1UEBhMCREUxCzAJBgNVBAgMAkJZMQ8wDQYDVQQHDAZNdW5pY2gx +DTALBgNVBAoMBEt5bWExEjAQBgNVBAsMCWplbGx5ZmlzaDAeFw0yNjAyMDQxMjAw +MTlaFw0zNjAyMDIxMjAwMTlaME4xCzAJBgNVBAYTAkRFMQswCQYDVQQIDAJCWTEP +MA0GA1UEBwwGTXVuaWNoMQ0wCwYDVQQKDARLeW1hMRIwEAYDVQQLDAlqZWxseWZp +c2gwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDaafClR88vH1hc07HR +VpC3bHBfBoXbifSb9Rmi/cGKUVDbDDY3hlPjmjKbxGCVOFCGNSBwDLJ60oNc/WqO +tqozWWYo8EY1HipANuOOKUbyx+9RRNJ0dvSblcVwTvqoB6fsOXX9XhYfQiWSb8hw +6cUykn0ozK22Q7mJCXgkx4Mii7JcIq3Kh0iWkiiTY1DJTPgI+mAE4tU1Ab3x0x7Q +ddMyPdrVrnFsr0nrJLwcaxlsSMSpfSSfLwTOgr7oLhXQq7eK2UQJPB3krXVDdCMQ +ygh5j+43hitG93Ku5SjiGSU9uoYSnwFacWutytk3pXMuKLaaTZntn1jedmiLUjwH +8Hx9AgMBAAGjUzBRMB0GA1UdDgQWBBSCyf29Oy5fcRuJPspZ6KCzLS1KBzAfBgNV +HSMEGDAWgBSCyf29Oy5fcRuJPspZ6KCzLS1KBzAPBgNVHRMBAf8EBTADAQH/MA0G +CSqGSIb3DQEBCwUAA4IBAQAX6l6nir+kaxUwQnjIKR1oj9EDoky7ZB4vMkykC1v2 +PYINjpRxizglULYVBw7T/QRXcs9mh5eIwdpsedPsl6bp5vaBDQaObUEnzNKg2BRo +W029AUDrph4N7eF5IkcyE4nGqhKtkGJsk6bBgwWPYNY8Ldp/QArCyhCo682Z005e +Hk5diJqnvecuJpuc4ib2PeO+zfF7UWSKH1ByS/qotcJV4AFenF5qVrgsiA6PCPdW +4ec5tOTXdoMvLJDVLhCEk14p4rJZT/DpjWu6LFok+z35OhMWKTcxYqxuhrvJ/x1n +gCijWuiRjwWQjdVehu579RLtyF9n+Hfwradwgj0GIrKj +-----END CERTIFICATE----- diff --git a/tests/fixtures/certificates/certificates.go b/tests/fixtures/certificates/certificates.go new file mode 100644 index 0000000000..4ca99d5b7c --- /dev/null +++ b/tests/fixtures/certificates/certificates.go @@ -0,0 +1,17 @@ +package certificates + +import ( + _ "embed" +) + +//go:embed certificate-valid-1.pem +var Cert1 []byte // valid until Feb 2 10:17:07 2036 GMT + +//go:embed certificate-valid-2.pem +var Cert2 []byte // valid until Feb 2 10:18:00 2036 GMT + +//go:embed certificate-valid-3.pem +var Cert3 []byte // valid until Feb 2 12:00:19 2036 GMT + +//go:embed certificate-expired.pem +var CertExpired []byte // expired on Feb 5 10:24:41 2026 GMT