Merge pull request #6006 from vidarno/cache-private-key-hash-on-issuer-status

Cache private key hash on issuer status
This commit is contained in:
jetstack-bot 2023-05-05 08:05:07 +01:00 committed by GitHub
commit 5035dda25e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 79 additions and 14 deletions

View File

@ -1267,6 +1267,9 @@ spec:
description: ACME specific status options. This field should only be set if the Issuer is configured to use an ACME server to issue certificates.
type: object
properties:
lastPrivateKeyHash:
description: LastPrivateKeyHash is a hash of the private key associated with the latest registered ACME account, in order to track changes made to registered account associated with the Issuer
type: string
lastRegisteredEmail:
description: LastRegisteredEmail is the email associated with the latest registered ACME account, in order to track changes made to registered account associated with the Issuer
type: string

View File

@ -1267,6 +1267,9 @@ spec:
description: ACME specific status options. This field should only be set if the Issuer is configured to use an ACME server to issue certificates.
type: object
properties:
lastPrivateKeyHash:
description: LastPrivateKeyHash is a hash of the private key associated with the latest registered ACME account, in order to track changes made to registered account associated with the Issuer
type: string
lastRegisteredEmail:
description: LastRegisteredEmail is the email associated with the latest registered ACME account, in order to track changes made to registered account associated with the Issuer
type: string

View File

@ -557,4 +557,9 @@ type ACMEIssuerStatus struct {
// ACME account, in order to track changes made to registered account
// associated with the Issuer
LastRegisteredEmail string
// LastPrivateKeyHash is a hash of the private key associated with the latest
// registered ACME account, in order to track changes made to registered account
// associated with the Issuer
LastPrivateKeyHash string
}

View File

@ -1286,6 +1286,7 @@ func Convert_acme_ACMEIssuerDNS01ProviderWebhook_To_v1_ACMEIssuerDNS01ProviderWe
func autoConvert_v1_ACMEIssuerStatus_To_acme_ACMEIssuerStatus(in *v1.ACMEIssuerStatus, out *acme.ACMEIssuerStatus, s conversion.Scope) error {
out.URI = in.URI
out.LastRegisteredEmail = in.LastRegisteredEmail
out.LastPrivateKeyHash = in.LastPrivateKeyHash
return nil
}
@ -1297,6 +1298,7 @@ func Convert_v1_ACMEIssuerStatus_To_acme_ACMEIssuerStatus(in *v1.ACMEIssuerStatu
func autoConvert_acme_ACMEIssuerStatus_To_v1_ACMEIssuerStatus(in *acme.ACMEIssuerStatus, out *v1.ACMEIssuerStatus, s conversion.Scope) error {
out.URI = in.URI
out.LastRegisteredEmail = in.LastRegisteredEmail
out.LastPrivateKeyHash = in.LastPrivateKeyHash
return nil
}

View File

@ -629,4 +629,9 @@ type ACMEIssuerStatus struct {
// associated with the Issuer
// +optional
LastRegisteredEmail string `json:"lastRegisteredEmail,omitempty"`
// LastPrivateKeyHash is a hash of the private key associated with the latest
// registered ACME account, in order to track changes made to registered account
// associated with the Issuer
LastPrivateKeyHash string `json:"lastPrivateKeyHash,omitempty"`
}

View File

@ -1285,6 +1285,7 @@ func Convert_acme_ACMEIssuerDNS01ProviderWebhook_To_v1alpha2_ACMEIssuerDNS01Prov
func autoConvert_v1alpha2_ACMEIssuerStatus_To_acme_ACMEIssuerStatus(in *ACMEIssuerStatus, out *acme.ACMEIssuerStatus, s conversion.Scope) error {
out.URI = in.URI
out.LastRegisteredEmail = in.LastRegisteredEmail
out.LastPrivateKeyHash = in.LastPrivateKeyHash
return nil
}
@ -1296,6 +1297,7 @@ func Convert_v1alpha2_ACMEIssuerStatus_To_acme_ACMEIssuerStatus(in *ACMEIssuerSt
func autoConvert_acme_ACMEIssuerStatus_To_v1alpha2_ACMEIssuerStatus(in *acme.ACMEIssuerStatus, out *ACMEIssuerStatus, s conversion.Scope) error {
out.URI = in.URI
out.LastRegisteredEmail = in.LastRegisteredEmail
out.LastPrivateKeyHash = in.LastPrivateKeyHash
return nil
}

View File

@ -629,4 +629,9 @@ type ACMEIssuerStatus struct {
// associated with the Issuer
// +optional
LastRegisteredEmail string `json:"lastRegisteredEmail,omitempty"`
// LastPrivateKeyHash is a hash of the private key associated with the latest
// registered ACME account, in order to track changes made to registered account
// associated with the Issuer
LastPrivateKeyHash string `json:"lastPrivateKeyHash,omitempty"`
}

View File

@ -1285,6 +1285,7 @@ func Convert_acme_ACMEIssuerDNS01ProviderWebhook_To_v1alpha3_ACMEIssuerDNS01Prov
func autoConvert_v1alpha3_ACMEIssuerStatus_To_acme_ACMEIssuerStatus(in *ACMEIssuerStatus, out *acme.ACMEIssuerStatus, s conversion.Scope) error {
out.URI = in.URI
out.LastRegisteredEmail = in.LastRegisteredEmail
out.LastPrivateKeyHash = in.LastPrivateKeyHash
return nil
}
@ -1296,6 +1297,7 @@ func Convert_v1alpha3_ACMEIssuerStatus_To_acme_ACMEIssuerStatus(in *ACMEIssuerSt
func autoConvert_acme_ACMEIssuerStatus_To_v1alpha3_ACMEIssuerStatus(in *acme.ACMEIssuerStatus, out *ACMEIssuerStatus, s conversion.Scope) error {
out.URI = in.URI
out.LastRegisteredEmail = in.LastRegisteredEmail
out.LastPrivateKeyHash = in.LastPrivateKeyHash
return nil
}

View File

@ -628,4 +628,9 @@ type ACMEIssuerStatus struct {
// associated with the Issuer
// +optional
LastRegisteredEmail string `json:"lastRegisteredEmail,omitempty"`
// LastPrivateKeyHash is a hash of the private key associated with the latest
// registered ACME account, in order to track changes made to registered account
// associated with the Issuer
LastPrivateKeyHash string `json:"lastPrivateKeyHash,omitempty"`
}

View File

@ -1285,6 +1285,7 @@ func Convert_acme_ACMEIssuerDNS01ProviderWebhook_To_v1beta1_ACMEIssuerDNS01Provi
func autoConvert_v1beta1_ACMEIssuerStatus_To_acme_ACMEIssuerStatus(in *ACMEIssuerStatus, out *acme.ACMEIssuerStatus, s conversion.Scope) error {
out.URI = in.URI
out.LastRegisteredEmail = in.LastRegisteredEmail
out.LastPrivateKeyHash = in.LastPrivateKeyHash
return nil
}
@ -1296,6 +1297,7 @@ func Convert_v1beta1_ACMEIssuerStatus_To_acme_ACMEIssuerStatus(in *ACMEIssuerSta
func autoConvert_acme_ACMEIssuerStatus_To_v1beta1_ACMEIssuerStatus(in *acme.ACMEIssuerStatus, out *ACMEIssuerStatus, s conversion.Scope) error {
out.URI = in.URI
out.LastRegisteredEmail = in.LastRegisteredEmail
out.LastPrivateKeyHash = in.LastPrivateKeyHash
return nil
}

View File

@ -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

View File

@ -17,6 +17,9 @@ limitations under the License.
package accounts
import (
"crypto/sha256"
"crypto/x509"
"encoding/base64"
"net/http"
"testing"
@ -157,6 +160,10 @@ func TestRegistry_AddClient_UpdatesClientPKChecksum(t *testing.T) {
t.Fatal(err)
}
pkBytes := x509.MarshalPKCS1PrivateKey(pk)
pkChecksum := sha256.Sum256(pkBytes)
pkChecksumString := base64.StdEncoding.EncodeToString(pkChecksum[:])
// Register a new client
r.AddClient(http.DefaultClient, "abc", cmacme.ACMEIssuer{}, pk, "cert-manager-test")
l := r.ListClients()
@ -164,12 +171,12 @@ func TestRegistry_AddClient_UpdatesClientPKChecksum(t *testing.T) {
t.Errorf("expected ListClients to have 1 item but it has %d", len(l))
}
isCached := r.IsKeyCheckSumCached("abc", pk)
isCached := r.IsKeyCheckSumCached(pkChecksumString, pk)
if isCached == false {
t.Fatal("checksum failed for same key")
}
isCached = r.IsKeyCheckSumCached("abc", pk2)
isCached = r.IsKeyCheckSumCached(pkChecksumString, pk2)
if isCached == true {
t.Fatal("checksum reported same for different keys")
}

View File

@ -34,7 +34,7 @@ type FakeRegistry struct {
RemoveClientFunc func(uid string)
GetClientFunc func(uid string) (acmecl.Interface, error)
ListClientsFunc func() map[string]acmecl.Interface
IsKeyCheckSumCachedFunc func(uid string, privateKey *rsa.PrivateKey) bool
IsKeyCheckSumCachedFunc func(lastPrivateKeyHash string, privateKey *rsa.PrivateKey) bool
}
func (f *FakeRegistry) AddClient(client *http.Client, uid string, config cmacme.ACMEIssuer, privateKey *rsa.PrivateKey, userAgent string) {
@ -53,6 +53,6 @@ func (f *FakeRegistry) ListClients() map[string]acmecl.Interface {
return f.ListClientsFunc()
}
func (f *FakeRegistry) IsKeyCheckSumCached(uid string, privateKey *rsa.PrivateKey) bool {
return f.IsKeyCheckSumCachedFunc(uid, privateKey)
func (f *FakeRegistry) IsKeyCheckSumCached(lastPrivateKeyHash string, privateKey *rsa.PrivateKey) bool {
return f.IsKeyCheckSumCachedFunc(lastPrivateKeyHash, privateKey)
}

View File

@ -641,4 +641,10 @@ type ACMEIssuerStatus struct {
// associated with the Issuer
// +optional
LastRegisteredEmail string `json:"lastRegisteredEmail,omitempty"`
// LastPrivateKeyHash is a hash of the private key associated with the latest
// registered ACME account, in order to track changes made to registered account
// associated with the Issuer
// +optional
LastPrivateKeyHash string `json:"lastPrivateKeyHash,omitempty"`
}

View File

@ -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)

View File

@ -253,6 +253,7 @@ func TestAcme_Setup(t *testing.T) {
gen.SetIssuerACMEAccountURL(acmev2Prod),
gen.SetIssuerACMEEmail(someEmail),
gen.SetIssuerACMELastRegisteredEmail(someEmail),
gen.SetIssuerACMELastPrivateKeyHash(someString),
gen.AddIssuerCondition(
*gen.IssuerConditionFrom(readyTrueCondition,
gen.SetIssuerConditionStatus(cmmeta.ConditionTrue)))),
@ -538,7 +539,7 @@ func TestAcme_Setup(t *testing.T) {
AddClientFunc: func(string, cmacme.ACMEIssuer, *rsa.PrivateKey, string) {
addClientWasCalled = true
},
IsKeyCheckSumCachedFunc: func(uid string, privateKey *rsa.PrivateKey) bool {
IsKeyCheckSumCachedFunc: func(lastPrivateKeyHash string, privateKey *rsa.PrivateKey) bool {
return true
},
}

View File

@ -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