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 Jan 7, 2024
1 parent 3234974 commit b7ceda9
Show file tree
Hide file tree
Showing 16 changed files with 1,391 additions and 303 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ require (
github.com/akamai/AkamaiOPEN-edgegrid-golang v1.2.2
github.com/aws/aws-sdk-go v1.49.13
github.com/cpu/goacmedns v0.1.1
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc
github.com/digitalocean/godo v1.107.0
github.com/go-ldap/ldap/v3 v3.4.6
github.com/go-logr/logr v1.4.1
Expand Down Expand Up @@ -73,7 +74,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.0 // indirect
github.com/evanphx/json-patch v5.7.0+incompatible // indirect
github.com/evanphx/json-patch/v5 v5.7.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: the %q field has changed.", fieldChangedErr.Field), 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
31 changes: 31 additions & 0 deletions internal/infohash/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# infohash

A 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.

## 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. The total space complexity is therefore:

```
64 + log2(nr_fields+1) * 32 bits
= 8 + log2(nr_fields+1) * 4 bytes
= 16 + 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.
58 changes: 58 additions & 0 deletions internal/infohash/hamming.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
Copyright 2020 The cert-manager Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package infohash

func calculateHammingCode(fields []uint32) []uint32 {
log2NumberOfFieldsPlusOne := log2OfXPlusOne(uint32(len(fields)))
parityCodes := make([]uint32, log2NumberOfFieldsPlusOne)

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

return parityCodes
}

func findErrorLocation(fields []uint32, hammingCode []uint32) (bool, uint32) {
expectedCode := calculateHammingCode(fields)
var errorLocation uint32

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

if errorLocation == 0 || errorLocation > uint32(len(fields)) {
return false, 0
}

return true, errorLocation - 1
}

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

0 comments on commit b7ceda9

Please sign in to comment.