From 92da674e9ada68ca22c91d0b119928dfecb074af Mon Sep 17 00:00:00 2001 From: vidarno <> Date: Sat, 29 Apr 2023 09:03:36 +0200 Subject: [PATCH] Update logic in function IsKeyCheckSumCached to compare private key with hash in status field of CRD instead of from Secret Signed-off-by: vidarno <> --- pkg/acme/accounts/registry.go | 15 ++++++++------- pkg/issuer/acme/setup.go | 8 +++++++- test/unit/gen/issuer.go | 10 ++++++++++ 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/pkg/acme/accounts/registry.go b/pkg/acme/accounts/registry.go index d79569a86..df7ed0658 100644 --- a/pkg/acme/accounts/registry.go +++ b/pkg/acme/accounts/registry.go @@ -20,6 +20,7 @@ import ( "crypto/rsa" "crypto/sha256" "crypto/x509" + "encoding/base64" "errors" "net/http" "sync" @@ -45,7 +46,7 @@ type Registry interface { // IsKeyCheckSumCached checks if the private key checksum is cached with registered client. // If not cached, the account is re-verified for the private key. - IsKeyCheckSumCached(uid string, privateKey *rsa.PrivateKey) bool + IsKeyCheckSumCached(lastPrivateKeyHash string, privateKey *rsa.PrivateKey) bool Getter } @@ -195,19 +196,19 @@ func (r *registry) ListClients() map[string]acmecl.Interface { // IsKeyCheckSumCached returns true when there is no difference in private key checksum. // This can be used to identify if the private key has changed for the existing // registered client. -func (r *registry) IsKeyCheckSumCached(uid string, privateKey *rsa.PrivateKey) bool { +func (r *registry) IsKeyCheckSumCached(lastPrivateKeyHash string, privateKey *rsa.PrivateKey) bool { r.lock.RLock() defer r.lock.RUnlock() - if privateKey != nil { + if privateKey != nil && lastPrivateKeyHash != "" { privateKeyBytes := x509.MarshalPKCS1PrivateKey(privateKey) checksum := sha256.Sum256(privateKeyBytes) + checksumString := base64.StdEncoding.EncodeToString(checksum[:]) - if clientMeta, ok := r.clients[uid]; ok { - if clientMeta.keyChecksum == checksum { - return true - } + if lastPrivateKeyHash == checksumString { + return true } + } // Either there is no entry found in client cache for uid diff --git a/pkg/issuer/acme/setup.go b/pkg/issuer/acme/setup.go index a3b5e65d3..89780a530 100644 --- a/pkg/issuer/acme/setup.go +++ b/pkg/issuer/acme/setup.go @@ -19,6 +19,8 @@ package acme import ( "context" "crypto/rsa" + "crypto/sha256" + "crypto/x509" "encoding/base64" "fmt" "net/url" @@ -147,7 +149,7 @@ func (a *Acme) Setup(ctx context.Context) error { return nil } - isPKChecksumSame := a.accountRegistry.IsKeyCheckSumCached(string(a.issuer.GetUID()), rsaPk) + isPKChecksumSame := a.accountRegistry.IsKeyCheckSumCached(a.issuer.GetStatus().ACMEStatus().LastPrivateKeyHash, rsaPk) // TODO: don't always clear the client cache. // In future we should intelligently manage items in the account cache @@ -314,8 +316,12 @@ func (a *Acme) Setup(ctx context.Context) error { status = cmmeta.ConditionTrue reason = successAccountRegistered msg = messageAccountRegistered + privateKeyBytes := x509.MarshalPKCS1PrivateKey(rsaPk) + checksum := sha256.Sum256(privateKeyBytes) + checksumString := base64.StdEncoding.EncodeToString(checksum[:]) a.issuer.GetStatus().ACMEStatus().URI = account.URI a.issuer.GetStatus().ACMEStatus().LastRegisteredEmail = registeredEmail + a.issuer.GetStatus().ACMEStatus().LastPrivateKeyHash = checksumString // ensure the cached client in the account registry is up to date a.accountRegistry.AddClient(httpClient, string(a.issuer.GetUID()), *a.issuer.GetSpec().ACME, rsaPk, a.userAgent) diff --git a/test/unit/gen/issuer.go b/test/unit/gen/issuer.go index e54fe2599..1ab46f34f 100644 --- a/test/unit/gen/issuer.go +++ b/test/unit/gen/issuer.go @@ -226,6 +226,16 @@ func SetIssuerACMELastRegisteredEmail(email string) IssuerModifier { } } +func SetIssuerACMELastPrivateKeyHash(privateKeyHash string) IssuerModifier { + return func(iss v1.GenericIssuer) { + status := iss.GetStatus() + if status.ACME == nil { + status.ACME = &cmacme.ACMEIssuerStatus{} + } + status.ACME.LastPrivateKeyHash = privateKeyHash + } +} + func SetIssuerCA(a v1.CAIssuer) IssuerModifier { return func(iss v1.GenericIssuer) { iss.GetSpec().CA = &a