Skip to content

Commit

Permalink
Use a hash to detect changes to Certificate resource.
Browse files Browse the repository at this point in the history
Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>
  • Loading branch information
inteon committed Feb 3, 2024
1 parent 6a9c402 commit eda4b43
Show file tree
Hide file tree
Showing 22 changed files with 1,380 additions and 360 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ require (
github.com/akamai/AkamaiOPEN-edgegrid-golang v1.2.2
github.com/aws/aws-sdk-go v1.50.5
github.com/cpu/goacmedns v0.1.1
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc
github.com/digitalocean/godo v1.108.0
github.com/go-ldap/ldap/v3 v3.4.6
github.com/go-logr/logr v1.4.1
Expand Down Expand Up @@ -66,7 +67,6 @@ require (
github.com/cespare/xxhash/v2 v2.2.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.11.2 // indirect
github.com/evanphx/json-patch v5.9.0+incompatible // indirect
github.com/evanphx/json-patch/v5 v5.9.0 // indirect
Expand Down
58 changes: 54 additions & 4 deletions internal/controller/certificates/policies/checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package policies
import (
"bytes"
"crypto/x509"
"errors"
"fmt"
"strings"
"time"
Expand All @@ -34,7 +35,10 @@ import (

cmmeta "github.com/cert-manager/cert-manager/internal/apis/meta"
internalcertificates "github.com/cert-manager/cert-manager/internal/controller/certificates"
"github.com/cert-manager/cert-manager/internal/infohash"
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"
)

Expand Down Expand Up @@ -157,6 +161,51 @@ 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.IsCertificateUpToDateWithVersionedHash(input.Certificate, certificateHash); err != nil {
if fieldChangedErr := (infohash.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.
func SecretIssuerAnnotationsMismatch(input Input) (string, string, bool) {
Expand Down Expand Up @@ -444,10 +493,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
Expand Down
6 changes: 6 additions & 0 deletions internal/controller/certificates/policies/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
51 changes: 37 additions & 14 deletions internal/controller/certificates/policies/policies.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,21 @@ func NewTriggerPolicyChain(c clock.Clock) Chain {
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
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
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
}
}

Expand All @@ -89,13 +97,21 @@ func NewReadinessPolicyChain(c clock.Clock) Chain {
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
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
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
}
}

Expand All @@ -104,6 +120,13 @@ func NewReadinessPolicyChain(c clock.Clock) Chain {
// correctness of metadata and output formats of Certificate's Secrets.
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
Expand Down
35 changes: 35 additions & 0 deletions internal/infohash/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# infohash

Am extendable hash &amp; comparator that tells you why the hash did not match.

For example if you edit the Name field in a struct, this library will tell
you that the hash did not match because the Name field was changed.

The hash is extendable, so you can add fields to the struct without breaking
the hash.

## Space Complexity

Per struct, we store a "global" hash and a "local" hash for each field.
The global hash is 64 bits long, and the local hashes are 32 bits long.
We use hamming codes to detect which "local" hash has changed. This requires
us to store log2(nr_fields+1) 32 bit parity hashes instead of nr_fields 32 bit
hashes. To store the number of fields, we use 16 bits. The total space complexity
is therefore:

```
(16 + 64 + log2(nr_fields+1) * 32) bits
= (10 + log2(nr_fields+1) * 4) bytes
= (20 + log2(nr_fields+1) * 8) hex characters
```

As a rule of thumb, this library is only useful if you have at least +-7 fields,
otherwise you can just store the hashes of each field individually.

## Safety

This library will detect any changes to the struct (with collision chance of 1/2^32, accounting for birthday attack).
However, accurately detecting which field changed is only possible if only one field changed.
If multiple fields changed, we can only detect that at least one field changed, but not which one.
This is because we use hamming codes to detect which field changed, and hamming codes can only detect
single field errors.
30 changes: 30 additions & 0 deletions internal/infohash/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package infohash

import "fmt"

// ErrInvalidHash is returned by Compare if the hash is invalid.
var ErrInvalidHash = fmt.Errorf("invalid hash")

// FieldChangedError is returned by Compare if the hashes indicate
// that the fields have changed.
type FieldChangedError struct {
// Change contains the field that changed and its new value.
// If we do know which field changed (eg. because there were
// multiple changes), Change will be nil.
Change *FieldChange
}

type FieldChange struct {
FieldName string
NewValue interface{}
}

func (e FieldChangedError) Error() string {
if e.Change == nil {
return "field changed"
}

newValue := prettyPrintConfigForHash.Sprintf("%#v", e.Change.NewValue)

return "field changed: " + e.Change.FieldName + " to " + newValue
}
47 changes: 47 additions & 0 deletions internal/infohash/hamming.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package infohash

func calculateParityCodes(values []uint32) []uint32 {
log2NumberOfFieldsPlusOne := log2OfXPlusOne(uint32(len(values)))
parityCodes := make([]uint32, log2NumberOfFieldsPlusOne)

for id, field := range values {
for i := range parityCodes {
if (id+1)&(1<<i) != 0 {
parityCodes[i] ^= field
}
}
}

return parityCodes
}

func findErrorLocation(values []uint32, parityCodes []uint32) (int, error) {
expectedCode := calculateParityCodes(values)
var errorLocation uint32

if len(parityCodes) != len(expectedCode) {
return 0, ErrInvalidHash
}

for i := range parityCodes {
if parityCodes[i] != expectedCode[i] {
errorLocation |= 1 << i
}
}

if errorLocation == 0 || errorLocation > uint32(len(values)) {
// Could not find the error location (there is probably more than one error)
return -1, nil
}

return int(errorLocation - 1), nil
}

func log2OfXPlusOne(x uint32) uint32 {
var r uint32
for x > 0 {
x >>= 1
r += 1
}
return r
}

0 comments on commit eda4b43

Please sign in to comment.