Add Certificate Hash

cmd/webhook/go.mod

@@ -23,7 +23,6 @@ replace github.com/cert-manager/cert-manager => ../../
 require (
 	github.com/cert-manager/cert-manager v0.0.0-00010101000000-000000000000
 	github.com/spf13/cobra v1.8.0
-	k8s.io/apimachinery v0.30.1
 	k8s.io/component-base v0.30.1
 	sigs.k8s.io/controller-runtime v0.18.2
 )
@@ -101,6 +100,7 @@ require (
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 	k8s.io/api v0.30.1 // indirect
 	k8s.io/apiextensions-apiserver v0.30.1 // indirect
+	k8s.io/apimachinery v0.30.1 // indirect
 	k8s.io/apiserver v0.30.1 // indirect
 	k8s.io/client-go v0.30.1 // indirect
 	k8s.io/klog/v2 v2.120.1 // indirect

go.mod

@@ -31,6 +31,7 @@ require (
 	github.com/aws/aws-sdk-go-v2/service/sts v1.28.9
 	github.com/aws/smithy-go v1.20.2
 	github.com/cpu/goacmedns v0.1.1
+	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc
 	github.com/digitalocean/godo v1.116.0
 	github.com/go-ldap/ldap/v3 v3.4.8
 	github.com/go-logr/logr v1.4.1
@@ -90,7 +91,6 @@ require (
 	github.com/cespare/xxhash/v2 v2.3.0 // indirect
 	github.com/coreos/go-semver v0.3.1 // indirect
 	github.com/coreos/go-systemd/v22 v22.5.0 // indirect
-	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
 	github.com/emicklei/go-restful/v3 v3.12.0 // indirect
 	github.com/evanphx/json-patch v5.9.0+incompatible // indirect
 	github.com/evanphx/json-patch/v5 v5.9.0 // indirect

internal/controller/certificates/policies/checks.go

@@ -19,6 +19,7 @@ package policies
 import (
 	"bytes"
 	"crypto/x509"
+	"errors"
 	"fmt"
 	"strings"
 	"time"
@@ -34,7 +35,9 @@ import (
 
 	cmmeta "github.com/cert-manager/cert-manager/internal/apis/meta"
 	internalcertificates "github.com/cert-manager/cert-manager/internal/controller/certificates"
+	apiutil "github.com/cert-manager/cert-manager/pkg/api/util"
 	cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
+	cmmetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1"
 	"github.com/cert-manager/cert-manager/pkg/util/pki"
 )
 
@@ -157,8 +160,55 @@ func SecretKeystoreFormatMismatch(input Input) (string, string, bool) {
 	return "", "", false
 }
 
+func SecretCertificateHashAnnotationMissingAndStable(input Input) (string, string, bool) {
+	if input.Secret.Annotations[cmapi.CertificateHashAnnotationKey] == "" {
+		readyCondition := apiutil.GetCertificateCondition(input.Certificate, cmapi.CertificateConditionReady)
+		issuingCondition := apiutil.GetCertificateCondition(input.Certificate, cmapi.CertificateConditionIssuing)
+
+		// If the certificate's ready condition is not up-to-date, we do not change the
+		// certificate hash annotation. Also, if the certificate is already being issued,
+		// we do not need to update the certificate hash annotation (as it will be updated once
+		// the certificate is issued).
+
+		if readyCondition != nil && readyCondition.ObservedGeneration != input.Certificate.Generation {
+			return "", "", false
+		}
+
+		if issuingCondition != nil && issuingCondition.Status == cmmetav1.ConditionTrue {
+			return "", "", false
+		}
+
+		return CertificateHashMissing, "Updating Secret, because it is missing a certificate hash annotation", true
+	}
+
+	return "", "", false
+}
+
+func SecretCertificateHashAnnotationMismatch(
+	fallbackChain Chain,
+) Func {
+	return func(input Input) (reason, message string, failed bool) {
+		certificateHash := input.Secret.Annotations[cmapi.CertificateHashAnnotationKey]
+		if certificateHash == "" {
+			return fallbackChain.Evaluate(input)
+		}
+
+		if err := pki.IsCertificateUpToDateWithInfoHash(input.Certificate, certificateHash); err != nil {
+			if fieldChangedErr := (pki.FieldChangedError{}); errors.As(err, &fieldChangedErr) {
+				return CertificateHashMismatch, fmt.Sprintf("The Secret is not up-to-date for the Certificate: %v", err), true
+			}
+
+			return CertificateHashMismatch, fmt.Sprintf("Failed to check if certificate is up to date: %v", err), true
+		}
+
+		return "", "", false
+	}
+}
+
 // SecretIssuerAnnotationsMismatch - When the issuer annotations are defined,
 // it must match the issuer ref.
+// Deprecated: This policy is only used for backwards compatibility with existing
+// Secrets without hash annotation.
 func SecretIssuerAnnotationsMismatch(input Input) (string, string, bool) {
 	name, ok1 := input.Secret.Annotations[cmapi.IssuerNameAnnotationKey]
 	kind, ok2 := input.Secret.Annotations[cmapi.IssuerKindAnnotationKey]
@@ -174,6 +224,8 @@ func SecretIssuerAnnotationsMismatch(input Input) (string, string, bool) {
 
 // SecretCertificateNameAnnotationsMismatch - When the CertificateName annotation is defined,
 // it must match the name of the Certificate.
+// Deprecated: This policy is only used for backwards compatibility with existing
+// Secrets without hash annotation.
 func SecretCertificateNameAnnotationsMismatch(input Input) (string, string, bool) {
 	name, ok := input.Secret.Annotations[cmapi.CertificateNameKey]
 	if (ok) && // only check if an annotation is present
@@ -187,6 +239,8 @@ func SecretCertificateNameAnnotationsMismatch(input Input) (string, string, bool
 // 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.
+// Deprecated: This policy is only used for backwards compatibility with existing
+// Secrets without hash annotation.
 func SecretPublicKeyDiffersFromCurrentCertificateRequest(input Input) (string, string, bool) {
 	if input.CurrentRevisionRequest == nil {
 		return "", "", false
@@ -212,6 +266,8 @@ func SecretPublicKeyDiffersFromCurrentCertificateRequest(input Input) (string, s
 	return "", "", false
 }
 
+// Deprecated: This policy is only used for backwards compatibility with existing
+// Secrets without hash annotation.
 func CurrentCertificateRequestMismatchesSpec(input Input) (string, string, bool) {
 	if input.CurrentRevisionRequest == nil {
 		// Fallback to comparing the Certificate spec with the issued certificate.
@@ -444,10 +500,11 @@ func SecretManagedLabelsAndAnnotationsManagedFieldsMismatch(fieldManager string)
 
 		// Ignore the CertificateName and IssuerRef annotations as these cannot be set by the postIssuance controller.
 		managedAnnotations.Delete(
-			cmapi.CertificateNameKey,       // SecretCertificateNameAnnotationMismatch checks the value
-			cmapi.IssuerNameAnnotationKey,  // SecretIssuerAnnotationsMismatch checks the value
-			cmapi.IssuerKindAnnotationKey,  // SecretIssuerAnnotationsMismatch checks the value
-			cmapi.IssuerGroupAnnotationKey, // SecretIssuerAnnotationsMismatch checks the value
+			cmapi.CertificateNameKey,           // SecretCertificateNameAnnotationMismatch checks the value
+			cmapi.IssuerNameAnnotationKey,      // SecretIssuerAnnotationsMismatch checks the value
+			cmapi.IssuerKindAnnotationKey,      // SecretIssuerAnnotationsMismatch checks the value
+			cmapi.IssuerGroupAnnotationKey,     // SecretIssuerAnnotationsMismatch checks the value
+			cmapi.CertificateHashAnnotationKey, // SecretCertificateHashAnnotationMismatch checks the value
 		)
 
 		// Remove the non cert-manager labels from the managed labels so we can compare

internal/controller/certificates/policies/constants.go

@@ -36,6 +36,12 @@ const (
 	// SecretMismatch is a policy violation reason for a scenario where Secret's
 	// private key does not match spec.
 	SecretMismatch string = "SecretMismatch"
+	// CertificateHashMissing is a policy violation reason for a scenario where
+	// Certificate's hash does not exist on the Secret.
+	CertificateHashMissing string = "CertificateHashMissing"
+	// CertificateHashMismatch is a policy violation reason for a scenario where
+	// Certificate's hash does not match the hash of the certificate on the Secret.
+	CertificateHashMismatch string = "CertificateHashMismatch"
 	// IncorrectIssuer is a policy violation reason for a scenario where
 	// Certificate has been issued by incorrect Issuer.
 	IncorrectIssuer string = "IncorrectIssuer"

internal/controller/certificates/policies/policies.go

@@ -65,45 +65,79 @@ func (c Chain) Evaluate(input Input) (string, string, bool) {
 
 // NewTriggerPolicyChain includes trigger policy checks, which if return true,
 // should cause a Certificate to be marked for issuance.
+//
+// If this chain returns true, a new certificate will be issued.
+// This chain should include all checks that are in the readiness chain, as well
+// as additional checks that proactively trigger re-issuance before the
+// certificate is marked as not ready.
 func NewTriggerPolicyChain(c clock.Clock) Chain {
 	return Chain{
-		SecretDoesNotExist,     // Make sure the Secret exists
-		SecretIsMissingData,    // Make sure the Secret has the required keys set
-		SecretPublicKeysDiffer, // Make sure the PrivateKey and PublicKey match in the Secret
-
-		SecretIssuerAnnotationsMismatch,          // Make sure the Secret's IssuerRef annotations match the Certificate spec
-		SecretCertificateNameAnnotationsMismatch, // Make sure the Secret's CertificateName annotation matches the Certificate's name
-
-		SecretPrivateKeyMismatchesSpec,                      // Make sure the PrivateKey Type and Size match the Certificate spec
-		SecretPublicKeyDiffersFromCurrentCertificateRequest, // Make sure the Secret's PublicKey matches the current CertificateRequest
-		CurrentCertificateRequestMismatchesSpec,             // Make sure the current CertificateRequest matches the Certificate spec
-		CurrentCertificateNearingExpiry(c),                  // Make sure the Certificate in the Secret is not nearing expiry
+		SecretDoesNotExist,             // Make sure the Secret exists
+		SecretIsMissingData,            // Make sure the Secret has the required keys set
+		SecretPublicKeysDiffer,         // Make sure the PrivateKey and PublicKey match in the Secret
+		SecretPrivateKeyMismatchesSpec, // Make sure the PrivateKey Type and Size match the Certificate spec
+
+		SecretCertificateHashAnnotationMismatch(
+			// BACKWARDS COMPATIBILITY: The following checks are only performed in case no
+			// CertificateRequest hash annotation is present on the Secret. This is to
+			// ensure that existing users of cert-manager do not have all their existing
+			// Certificates marked as not ready.
+			Chain{
+				SecretIssuerAnnotationsMismatch,                     // Make sure the Secret's IssuerRef annotations match the Certificate spec
+				SecretCertificateNameAnnotationsMismatch,            // Make sure the Secret's CertificateName annotation matches the Certificate's name
+				SecretPublicKeyDiffersFromCurrentCertificateRequest, // Make sure the Secret's PublicKey matches the current CertificateRequest
+				CurrentCertificateRequestMismatchesSpec,             // Make sure the current CertificateRequest matches the Certificate spec
+			},
+		),
+
+		CurrentCertificateNearingExpiry(c), // Make sure the Certificate in the Secret is not nearing expiry
 	}
 }
 
 // NewReadinessPolicyChain includes readiness policy checks, which if return
 // true, would cause a Certificate to be marked as not ready.
+//
+// If this chain returns true, the status of the Certificate resource will show
+// NotReady.
 func NewReadinessPolicyChain(c clock.Clock) Chain {
 	return Chain{
-		SecretDoesNotExist,     // Make sure the Secret exists
-		SecretIsMissingData,    // Make sure the Secret has the required keys set
-		SecretPublicKeysDiffer, // Make sure the PrivateKey and PublicKey match in the Secret
-
-		SecretIssuerAnnotationsMismatch,          // Make sure the Secret's IssuerRef annotations match the Certificate spec
-		SecretCertificateNameAnnotationsMismatch, // Make sure the Secret's CertificateName annotation matches the Certificate's name
-
-		SecretPrivateKeyMismatchesSpec,                      // Make sure the PrivateKey Type and Size match the Certificate spec
-		SecretPublicKeyDiffersFromCurrentCertificateRequest, // Make sure the Secret's PublicKey matches the current CertificateRequest
-		CurrentCertificateRequestMismatchesSpec,             // Make sure the current CertificateRequest matches the Certificate spec
-		CurrentCertificateHasExpired(c),                     // Make sure the Certificate in the Secret has not expired
+		SecretDoesNotExist,             // Make sure the Secret exists
+		SecretIsMissingData,            // Make sure the Secret has the required keys set
+		SecretPublicKeysDiffer,         // Make sure the PrivateKey and PublicKey match in the Secret
+		SecretPrivateKeyMismatchesSpec, // Make sure the PrivateKey Type and Size match the Certificate spec
+
+		SecretCertificateHashAnnotationMismatch(
+			// BACKWARDS COMPATIBILITY: The following checks are only performed in case no
+			// CertificateRequest hash annotation is present on the Secret. This is to
+			// ensure that existing users of cert-manager do not have all their existing
+			// Certificates marked as not ready.
+			Chain{
+				SecretIssuerAnnotationsMismatch,                     // Make sure the Secret's IssuerRef annotations match the Certificate spec
+				SecretCertificateNameAnnotationsMismatch,            // Make sure the Secret's CertificateName annotation matches the Certificate's name
+				SecretPublicKeyDiffersFromCurrentCertificateRequest, // Make sure the Secret's PublicKey matches the current CertificateRequest
+				CurrentCertificateRequestMismatchesSpec,             // Make sure the current CertificateRequest matches the Certificate spec
+			},
+		),
+
+		CurrentCertificateHasExpired(c), // Make sure the Certificate in the Secret has not expired
 	}
 }
 
 // NewSecretPostIssuancePolicyChain includes policy checks that are to be
 // performed _after_ issuance has been successful, testing for the presence and
 // correctness of metadata and output formats of Certificate's Secrets.
+//
+// If this chain returns true, the Secret will be updated without having to
+// re-issue the certificate.
 func NewSecretPostIssuancePolicyChain(ownerRefEnabled bool, fieldManager string) Chain {
 	return Chain{
+		// Make sure the Secret has the Certificate hash annotation, if not, we will
+		// calculate a hash from the Certificate and add it to the Secret based on the
+		// Certificate's ready condition.
+		// We do this to ensure that all Secrets have the Certificate hash annotation and
+		// we can remove the fallback chain in a future release.
+		SecretCertificateHashAnnotationMissingAndStable,
+
 		SecretBaseLabelsMismatch,                                             // Make sure the managed labels have the correct values
 		SecretCertificateDetailsAnnotationsMismatch,                          // Make sure the managed certificate details annotations have the correct values
 		SecretManagedLabelsAndAnnotationsManagedFieldsMismatch(fieldManager), // Make sure the only the expected managed labels and annotations exist

pkg/apis/certmanager/v1/types.go

@@ -89,6 +89,10 @@ const (
 	// Annotation key for the 'group' of the Issuer resource.
 	IssuerGroupAnnotationKey = "cert-manager.io/issuer-group"
 
+	// Annotation key for the hash of the Certificate that created the certificate
+	// block encoded in this Secret resource.
+	CertificateHashAnnotationKey = "cert-manager.io/certificate-hash"
+
 	// Annotation key for the name of the certificate that a resource is related to.
 	CertificateNameKey = "cert-manager.io/certificate-name"
 

pkg/controller/certificates/issuing/internal/secret.go

@@ -60,6 +60,7 @@ type SecretsManager struct {
 // SecretData is a structure wrapping private key, Certificate and CA data
 type SecretData struct {
 	PrivateKey, Certificate, CA         []byte
+	CertificateHash                     string
 	CertificateName                     string
 	IssuerName, IssuerKind, IssuerGroup string
 }
@@ -200,6 +201,10 @@ func (s *SecretsManager) setValues(crt *cmapi.Certificate, secret *corev1.Secret
 		secret.Annotations[cmapi.IssuerGroupAnnotationKey] = data.IssuerGroup
 	}
 
+	if data.CertificateHash != "" {
+		secret.Annotations[cmapi.CertificateHashAnnotationKey] = data.CertificateHash
+	}
+
 	secret.Labels[cmapi.PartOfCertManagerControllerLabelKey] = "true"
 
 	return nil

pkg/controller/certificates/issuing/issuing_controller.go

@@ -385,6 +385,11 @@ func (c *controller) issueCertificate(ctx context.Context, nextRevision int, crt
 		crt.Spec.PrivateKey = &cmapi.CertificatePrivateKey{}
 	}
 
+	certificateHash, err := utilpki.CertificateInfoHash(crt)
+	if err != nil {
+		return err
+	}
+
 	pkData, err := utilpki.EncodePrivateKey(pk, crt.Spec.PrivateKey.Encoding)
 	if err != nil {
 		return err
@@ -393,6 +398,7 @@ func (c *controller) issueCertificate(ctx context.Context, nextRevision int, crt
 		PrivateKey:      pkData,
 		Certificate:     req.Status.Certificate,
 		CA:              req.Status.CA,
+		CertificateHash: certificateHash,
 		CertificateName: crt.Name,
 		IssuerName:      req.Spec.IssuerRef.Name,
 		IssuerKind:      req.Spec.IssuerRef.Kind,

pkg/controller/certificates/issuing/issuing_controller_test.go

@@ -36,6 +36,7 @@ import (
 	cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1"
 	"github.com/cert-manager/cert-manager/pkg/controller/certificates/issuing/internal"
 	testpkg "github.com/cert-manager/cert-manager/pkg/controller/test"
+	utilpki "github.com/cert-manager/cert-manager/pkg/util/pki"
 	testcrypto "github.com/cert-manager/cert-manager/test/unit/crypto"
 	"github.com/cert-manager/cert-manager/test/unit/gen"
 )
@@ -85,6 +86,8 @@ func TestIssuingController(t *testing.T) {
 			LastTransitionTime: &metaFixedClockStart,
 		}),
 	)
+	issuingCertHash, err := utilpki.CertificateInfoHash(issuingCert)
+	require.NoError(t, err)
 
 	tests := map[string]testT{
 		"if certificate is not in Issuing state, then do nothing": {
@@ -514,6 +517,7 @@ func TestIssuingController(t *testing.T) {
 				IssuerName:      "ca-issuer",
 				IssuerKind:      "Issuer",
 				IssuerGroup:     "foo.io",
+				CertificateHash: issuingCertHash,
 			},
 			expectedErr: false,
 		},
@@ -572,6 +576,7 @@ func TestIssuingController(t *testing.T) {
 				IssuerName:      "ca-issuer",
 				IssuerKind:      "Issuer",
 				IssuerGroup:     "foo.io",
+				CertificateHash: issuingCertHash,
 			},
 			expectedErr: false,
 		},
@@ -629,6 +634,7 @@ func TestIssuingController(t *testing.T) {
 				IssuerName:      "ca-issuer",
 				IssuerKind:      "Issuer",
 				IssuerGroup:     "foo.io",
+				CertificateHash: issuingCertHash,
 			},
 			expectedErr: false,
 		},
@@ -687,6 +693,7 @@ func TestIssuingController(t *testing.T) {
 				IssuerName:      "ca-issuer",
 				IssuerKind:      "Issuer",
 				IssuerGroup:     "foo.io",
+				CertificateHash: issuingCertHash,
 			},
 			expectedErr: false,
 		},
@@ -971,6 +978,7 @@ func TestIssuingController(t *testing.T) {
 				IssuerName:      "ca-issuer",
 				IssuerKind:      "Issuer",
 				IssuerGroup:     "foo.io",
+				CertificateHash: issuingCertHash,
 			},
 			expectedErr: false,
 		},