diff --git a/internal/controller/certificates/policies/checks.go b/internal/controller/certificates/policies/checks.go index 9e6f1224a97..4341c773c54 100644 --- a/internal/controller/certificates/policies/checks.go +++ b/internal/controller/certificates/policies/checks.go @@ -166,6 +166,35 @@ func SecretIssuerAnnotationsNotUpToDate(input Input) (string, string, bool) { return "", "", false } +// SecretPublicKeyDiffersFromCurrentCertificateRequest checks that the current CertificateRequest +// contains a CSR that is signed by the key stored in the Secret. A failure is often caused by the +// Secret being changed outside of the control of cert-manager, causing the current CertificateRequest +// to no longer match what is stored in the Secret. +func SecretPublicKeyDiffersFromCurrentCertificateRequest(input Input) (string, string, bool) { + if input.CurrentRevisionRequest == nil { + return "", "", false + } + pk, err := pki.DecodePrivateKeyBytes(input.Secret.Data[corev1.TLSPrivateKeyKey]) + if err != nil { + return InvalidKeyPair, fmt.Sprintf("Issuing certificate as Secret contains invalid private key data: %v", err), true + } + + csr, err := pki.DecodeX509CertificateRequestBytes(input.CurrentRevisionRequest.Spec.Request) + if err != nil { + return InvalidCertificateRequest, fmt.Sprintf("Failed to decode current CertificateRequest: %v", err), true + } + + equal, err := pki.PublicKeysEqual(csr.PublicKey, pk.Public()) + if err != nil { + return InvalidCertificateRequest, fmt.Sprintf("CertificateRequest's public key is invalid: %v", err), true + } + if !equal { + return SecretMismatch, "Secret contains a private key that does not match the current CertificateRequest", true + } + + return "", "", false +} + func CurrentCertificateRequestNotValidForSpec(input Input) (string, string, bool) { if input.CurrentRevisionRequest == nil { // Fallback to comparing the Certificate spec with the issued certificate. diff --git a/internal/controller/certificates/policies/checks_test.go b/internal/controller/certificates/policies/checks_test.go index 381952cb21c..09aa477e95b 100644 --- a/internal/controller/certificates/policies/checks_test.go +++ b/internal/controller/certificates/policies/checks_test.go @@ -213,6 +213,31 @@ func Test_NewTriggerPolicyChain(t *testing.T) { message: "Issuing certificate as Secret was previously issued by IssuerKind.new.example.com/testissuer", reissue: true, }, + "trigger if the Secret contains a different private key than was used to sign the CSR": { + certificate: &cmapi.Certificate{Spec: cmapi.CertificateSpec{SecretName: "something"}}, + secret: &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "something"}, + Data: map[string][]byte{ + corev1.TLSPrivateKeyKey: staticFixedPrivateKey, + corev1.TLSCertKey: testcrypto.MustCreateCert( + t, staticFixedPrivateKey, + &cmapi.Certificate{Spec: cmapi.CertificateSpec{CommonName: "example.com"}}, + ), + }, + }, + request: &cmapi.CertificateRequest{Spec: cmapi.CertificateRequestSpec{ + IssuerRef: cmmeta.ObjectReference{ + Name: "testissuer", + Kind: "IssuerKind", + Group: "group.example.com", + }, + Request: testcrypto.MustGenerateCSRImpl(t, testcrypto.MustCreatePEMPrivateKey(t), &cmapi.Certificate{Spec: cmapi.CertificateSpec{ + CommonName: "example.com", + }}), + }}, + reason: SecretMismatch, + message: "Secret contains a private key that does not match the current CertificateRequest", + reissue: true, + }, // we only have a basic test here for this as unit tests for the // `pki.RequestMatchesSpec` function cover all other cases. "trigger issuance when CertificateRequest does not match certificate spec": { diff --git a/internal/controller/certificates/policies/constants.go b/internal/controller/certificates/policies/constants.go index 6afde6e0e74..32d556fdd58 100644 --- a/internal/controller/certificates/policies/constants.go +++ b/internal/controller/certificates/policies/constants.go @@ -29,6 +29,10 @@ const ( // InvalidCertificate is a policy violation whereby the signed certificate in // the Input Secret could not be parsed or decoded. InvalidCertificate string = "InvalidCertificate" + // InvalidCertificateRequest is a policy violation whereby the CSR in + // the Input CertificateRequest could not be parsed or decoded. + InvalidCertificateRequest string = "InvalidCertificateRequest" + // SecretMismatch is a policy violation reason for a scenario where Secret's // private key does not match spec. SecretMismatch string = "SecretMismatch" diff --git a/internal/controller/certificates/policies/policies.go b/internal/controller/certificates/policies/policies.go index ff8f27cc56b..37dd60caa41 100644 --- a/internal/controller/certificates/policies/policies.go +++ b/internal/controller/certificates/policies/policies.go @@ -72,6 +72,7 @@ func NewTriggerPolicyChain(c clock.Clock) Chain { SecretPublicKeysDiffer, SecretPrivateKeyMatchesSpec, SecretIssuerAnnotationsNotUpToDate, + SecretPublicKeyDiffersFromCurrentCertificateRequest, CurrentCertificateRequestNotValidForSpec, CurrentCertificateNearingExpiry(c), } @@ -84,6 +85,7 @@ func NewReadinessPolicyChain(c clock.Clock) Chain { SecretDoesNotExist, SecretIsMissingData, SecretPublicKeysDiffer, + SecretPublicKeyDiffersFromCurrentCertificateRequest, CurrentCertificateRequestNotValidForSpec, CurrentCertificateHasExpired(c), } diff --git a/pkg/controller/certificates/readiness/readiness_controller_test.go b/pkg/controller/certificates/readiness/readiness_controller_test.go index b4da05de563..1b44908ca0f 100644 --- a/pkg/controller/certificates/readiness/readiness_controller_test.go +++ b/pkg/controller/certificates/readiness/readiness_controller_test.go @@ -467,7 +467,11 @@ func TestNewReadinessPolicyChain(t *testing.T) { Name: "testissuer", Kind: "IssuerKind", Group: "group.example.com", - })), + }), + gen.SetCertificateRequestCSR(testcrypto.MustGenerateCSRImpl(t, privKey, + gen.Certificate("something", + gen.SetCertificateCommonName("new.example.com")))), + ), reason: policies.Expired, message: "Certificate expired on Sun, 31 Dec 0000 23:00:00 UTC", violationFound: true,