Merge pull request #81 from gardener/fix-replace-issuer-secret

[ACME] Fix account registration on issuer secret update

build/Dockerfile

@@ -3,7 +3,7 @@
 # SPDX-License-Identifier: Apache-2.0
 
 #############      builder       #############
-FROM eu.gcr.io/gardener-project/3rd/golang:1.16.2 AS builder
+FROM eu.gcr.io/gardener-project/3rd/golang:1.16.6 AS builder
 
 WORKDIR /build
 COPY . .

go.mod

@@ -12,7 +12,6 @@ require (
 	github.com/miekg/dns v1.1.31
 	github.com/onsi/ginkgo v1.14.1
 	github.com/onsi/gomega v1.10.2
-	github.com/pkg/errors v0.9.1
 	github.com/prometheus/client_golang v1.7.1
 	golang.org/x/lint v0.0.0-20200302205851-738671d3881b
 	k8s.io/api v0.20.6

pkg/cert/legobridge/cakeypair.go

@@ -14,7 +14,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 )
 
@@ -79,7 +78,7 @@ func (c *TLSKeyPair) RawCertInfo() ([]byte, error) {
 
 	certInfo, err := json.Marshal(raw)
 	if err != nil {
-		return nil, errors.Wrap(err, "encoding certificate info failed")
+		return nil, fmt.Errorf("encoding certificate info failed: %w", err)
 	}
 	return certInfo, nil
 }

pkg/cert/legobridge/certificate.go

@@ -14,8 +14,6 @@ import (
 	"sync"
 	"time"
 
-	"github.com/pkg/errors"
-
 	"github.com/gardener/cert-management/pkg/cert/metrics"
 	"github.com/gardener/cert-management/pkg/cert/utils"
 
@@ -368,7 +366,7 @@ func DecodeCertificate(tlsCrt []byte) (*x509.Certificate, error) {
 	}
 	cert, err := x509.ParseCertificate(block.Bytes)
 	if err != nil {
-		return nil, fmt.Errorf("parsing certificate failed with %s", err.Error())
+		return nil, fmt.Errorf("parsing certificate failed: %w", err)
 	}
 	return cert, nil
 }
@@ -415,7 +413,7 @@ func RevokeCertificate(user *RegistrationUser, cert []byte) error {
 	// A client facilitates communication with the CA server.
 	client, err := lego.NewClient(config)
 	if err != nil {
-		return errors.Wrap(err, "client creation failed")
+		return fmt.Errorf("client creation failed: %w", err)
 	}
 
 	return client.Certificate.Revoke(cert)

pkg/cert/legobridge/dnscontrollerprovider.go

@@ -38,7 +38,7 @@ func newDNSControllerProvider(settings DNSControllerSettings,
 	certificateName resources.ObjectName, targetClass string, issuerKey utils.IssuerKey) (ProviderWithCount, error) {
 	itf, err := settings.Cluster.Resources().GetByExample(&dnsapi.DNSEntry{})
 	if err != nil {
-		return nil, fmt.Errorf("cannot get DNSEntry resources: %s", err.Error())
+		return nil, fmt.Errorf("cannot get DNSEntry resources: %w", err)
 	}
 	n := atomic.AddUint32(&index, 1)
 	return &dnsControllerProvider{
@@ -136,7 +136,7 @@ func (p *dnsControllerProvider) Present(domain, token, keyAuth string) error {
 		}
 		_, err := p.entryResources.Create(entry)
 		if err != nil {
-			return fmt.Errorf("creating DNSEntry %s/%s failed with %s", entry.Namespace, entry.Name, err.Error())
+			return fmt.Errorf("creating DNSEntry %s/%s failed: %w", entry.Namespace, entry.Name, err)
 		}
 		return nil
 	}
@@ -145,14 +145,14 @@ func (p *dnsControllerProvider) Present(domain, token, keyAuth string) error {
 	err := retryOnUpdateError(func() error {
 		obj, err := p.entryResources.Get_(entry)
 		if err != nil {
-			return fmt.Errorf("getting DNSEntry %s/%s failed with %s", entry.Namespace, entry.Name, err.Error())
+			return fmt.Errorf("getting DNSEntry %s/%s failed: %w", entry.Namespace, entry.Name, err)
 		}
 		entry = obj.Data().(*dnsapi.DNSEntry)
 		setSpec(entry)
 		p.logger.Infof("presenting DNSEntry %s/%s for certificate resource %s with %d values", entry.Namespace, entry.Name, p.certificateName, len(values))
 		_, err = p.entryResources.Update(entry)
 		if err != nil {
-			return &updateError{msg: fmt.Sprintf("updating DNSEntry %s/%s failed with %s", entry.Namespace, entry.Name, err.Error())}
+			return &updateError{msg: fmt.Sprintf("updating DNSEntry %s/%s failed: %s", entry.Namespace, entry.Name, err)}
 		}
 		return nil
 	})
@@ -199,7 +199,7 @@ func (p *dnsControllerProvider) CleanUp(domain, token, keyAuth string) error {
 	p.logger.Infof("cleanup DNSEntry %s/%s for request %s", entry.Namespace, entry.Name, p.certificateName)
 	err := p.entryResources.Delete(entry)
 	if err != nil {
-		return fmt.Errorf("deleting DNSEntry %s/%s failed with %s", entry.Namespace, entry.Name, err.Error())
+		return fmt.Errorf("deleting DNSEntry %s/%s failed: %w", entry.Namespace, entry.Name, err)
 	}
 	return nil
 }

pkg/cert/legobridge/pki.go

@@ -22,7 +22,6 @@ import (
 	"time"
 
 	"github.com/go-acme/lego/v4/certificate"
-	"github.com/pkg/errors"
 )
 
 var (
@@ -107,15 +106,15 @@ func generateKey(algo x509.PublicKeyAlgorithm, size int) (crypto.Signer, []byte,
 	switch algo {
 	case x509.RSA:
 		if size < RSAMinSize {
-			return nil, nil, errors.New("RSA key is too weak")
+			return nil, nil, fmt.Errorf("RSA key is too weak")
 		}
 		if size > RSAMaxSize {
-			return nil, nil, errors.New("RSA key size too large")
+			return nil, nil, fmt.Errorf("RSA key size too large")
 		}
 
 		key, err = rsa.GenerateKey(rand.Reader, size)
 		if err != nil {
-			return nil, nil, errors.Wrap(err, "unable to generate RSA private key")
+			return nil, nil, fmt.Errorf("unable to generate RSA private key: %w", err)
 		}
 	case x509.ECDSA:
 		var curve elliptic.Curve
@@ -127,20 +126,20 @@ func generateKey(algo x509.PublicKeyAlgorithm, size int) (crypto.Signer, []byte,
 		case ECCurve256:
 			curve = elliptic.P256()
 		default:
-			return nil, nil, errors.New("invalid elliptic curve")
+			return nil, nil, fmt.Errorf("invalid elliptic curve")
 		}
 
 		key, err = ecdsa.GenerateKey(curve, rand.Reader)
 		if err != nil {
-			return nil, nil, errors.Wrap(err, "unable to generate RSA private key")
+			return nil, nil, fmt.Errorf("unable to generate RSA private key: %w", err)
 		}
 	default:
-		return nil, nil, errors.New("algorithm not supported")
+		return nil, nil, fmt.Errorf("algorithm not supported")
 	}
 
 	pem, err := privateKeyToBytes(key)
 	if err != nil {
-		return nil, nil, errors.Wrap(err, "encoding private key failed")
+		return nil, nil, fmt.Errorf("encoding private key failed: %w", err)
 	}
 	return key, pem, nil
 }
@@ -185,7 +184,7 @@ func createCertReq(input ObtainInput) (*x509.CertificateRequest, error) {
 func generateCSRPEM(csr *x509.CertificateRequest, privateKey crypto.Signer) ([]byte, error) {
 	derBytes, err := x509.CreateCertificateRequest(rand.Reader, csr, privateKey)
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to create certificate request")
+		return nil, fmt.Errorf("failed to create certificate request: %w", err)
 	}
 
 	pemBytes := bytes.NewBuffer([]byte{})
@@ -220,7 +219,7 @@ func generateCertFromCSR(csrPEM []byte, duration time.Duration, isCA bool) (*x50
 
 	serialNumber, err := rand.Int(rand.Reader, serialNumberLimit)
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to generate serial number")
+		return nil, fmt.Errorf("failed to generate serial number: %w", err)
 	}
 
 	return &x509.Certificate{
@@ -244,7 +243,7 @@ func generateCertFromCSR(csrPEM []byte, duration time.Duration, isCA bool) (*x50
 func signCert(cert, issuerCert *x509.Certificate, publicKey crypto.PublicKey, signerKey crypto.PrivateKey) ([]byte, error) {
 	derBytes, err := x509.CreateCertificate(rand.Reader, cert, issuerCert, publicKey, signerKey)
 	if err != nil {
-		return nil, fmt.Errorf("error creating x509 certificate: %s", err.Error())
+		return nil, fmt.Errorf("error creating x509 certificate: %w", err)
 	}
 
 	pemBytes := bytes.NewBuffer([]byte{})
@@ -365,7 +364,7 @@ func encodeCertPEM(out io.Writer, derBytes []byte) error {
 func encodePEM(out io.Writer, b *pem.Block) error {
 	err := pem.Encode(out, b)
 	if err != nil {
-		return errors.Wrap(err, "error encoding certificate PEM")
+		return fmt.Errorf("error encoding certificate PEM: %w", err)
 	}
 	return nil
 }

pkg/cert/legobridge/reguser.go

@@ -140,7 +140,7 @@ func NewRegistrationUserFromEmailAndPrivateKey(issuerKey utils.IssuerKey,
 func (u *RegistrationUser) ToSecretData() (map[string][]byte, error) {
 	privkey, err := privateKeyToBytes(u.key)
 	if err != nil {
-		return nil, fmt.Errorf("encoding private key failed: %s", err.Error())
+		return nil, fmt.Errorf("encoding private key failed: %w", err)
 	}
 	return map[string][]byte{KeyPrivateKey: privkey}, nil
 }
@@ -149,7 +149,7 @@ func (u *RegistrationUser) ToSecretData() (map[string][]byte, error) {
 func (u *RegistrationUser) RawRegistration() ([]byte, error) {
 	reg, err := json.Marshal(u.registration)
 	if err != nil {
-		return nil, fmt.Errorf("encoding registration failed: %s", err.Error())
+		return nil, fmt.Errorf("encoding registration failed: %w", err)
 	}
 	return reg, nil
 }
@@ -169,7 +169,10 @@ func RegistrationUserFromSecretData(issuerKey utils.IssuerKey,
 	reg := &registration.Resource{}
 	err = json.Unmarshal(registrationRaw, reg)
 	if err != nil {
-		return nil, fmt.Errorf("unmarshalling registration json failed with %s", err.Error())
+		return nil, fmt.Errorf("unmarshalling registration json failed: %w", err)
+	}
+	if reg.URI == "" {
+		return nil, fmt.Errorf("unmarshalling registration with unexpected empty URI")
 	}
 	metrics.AddACMEAccountRegistration(issuerKey, reg.URI, email)
 	return &RegistrationUser{email: email, registration: reg, caDirURL: caDirURL, key: privateKey,

pkg/cert/utils/utils_certificate.go

@@ -99,7 +99,7 @@ func ExtractDomains(spec *api.CertificateSpec) ([]string, error) {
 func ExtractCommonNameAnDNSNames(csr []byte) (cn *string, san []string, err error) {
 	certificateRequest, err := extractCertificateRequest(csr)
 	if err != nil {
-		err = fmt.Errorf("parsing CSR failed with: %s", err)
+		err = fmt.Errorf("parsing CSR failed: %w", err)
 		return
 	}
 	cnvalue := certificateRequest.Subject.CommonName

pkg/controller/issuer/acme/handler.go

@@ -56,123 +56,91 @@ func (r *acmeIssuerHandler) Reconcile(logger logger.LogContext, obj resources.Ob
 		return r.failedAcme(logger, obj, api.StateError, fmt.Errorf("missing server in ACME spec"))
 	}
 
-	r.support.AddIssuerDomains(obj.ClusterKey(), issuer.Spec.ACME.Domains)
+	r.support.AddIssuerDomains(obj.ClusterKey(), acme.Domains)
 
-	r.support.RememberIssuerSecret(obj.ClusterKey(), issuer.Spec.ACME.PrivateKeySecretRef, "")
+	r.support.RememberIssuerSecret(obj.ClusterKey(), acme.PrivateKeySecretRef, "")
 
 	issuerKey := r.support.ToIssuerKey(obj.ClusterKey())
 	var secret *corev1.Secret
+	var secretHash string
 	var err error
 	if acme.PrivateKeySecretRef != nil {
 		secret, err = r.support.ReadIssuerSecret(issuerKey, acme.PrivateKeySecretRef)
 		if err != nil {
 			if acme.AutoRegistration {
 				logger.Info("spec.acme.privateKeySecretRef not existing, creating new account")
 			} else {
-				return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("loading issuer secret failed with %s", err.Error()))
+				return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("loading issuer secret failed: %w", err))
 			}
 		}
-		hash := r.support.CalcSecretHash(secret)
-		r.support.RememberIssuerSecret(obj.ClusterKey(), issuer.Spec.ACME.PrivateKeySecretRef, hash)
+		secretHash = r.support.CalcSecretHash(secret)
+		r.support.RememberIssuerSecret(obj.ClusterKey(), acme.PrivateKeySecretRef, secretHash)
 	}
-	if secret != nil && issuer.Status.ACME != nil && issuer.Status.ACME.Raw != nil {
-		eabKeyID, eabHmacKey, err := r.support.LoadEABHmacKey(issuerKey, acme)
+	if secret != nil {
+		objKey := obj.ClusterKey()
+		eabKeyID, eabHmacKey, err := r.support.LoadEABHmacKey(&objKey, issuerKey, acme)
 		if err != nil {
-			return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("loading EAB secret failed: %s", err))
+			return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("loading EAB secret failed: %w", err))
 		}
-		user, err := legobridge.RegistrationUserFromSecretData(issuerKey, acme.Email, acme.Server, issuer.Status.ACME.Raw,
+		var raw []byte
+		if core.IsSameExistingRegistration(issuer.Status.ACME, secretHash) {
+			raw = issuer.Status.ACME.Raw
+		} else {
+			user, err := legobridge.NewRegistrationUserFromEmail(issuerKey, acme.Email, acme.Server, secret.Data, eabKeyID, eabHmacKey)
+			if err != nil {
+				return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("creating registration user failed: %w", err))
+			}
+			raw, err = user.RawRegistration()
+			if err != nil {
+				return r.failedAcme(logger, obj, api.StateError, fmt.Errorf("registration marshalling failed: %w", err))
+			}
+		}
+		user, err := legobridge.RegistrationUserFromSecretData(issuerKey, acme.Email, acme.Server, raw,
 			secret.Data, eabKeyID, eabHmacKey)
 		if err != nil {
-			return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("extracting registration user from secret failed with %s", err.Error()))
+			return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("extracting registration user from secret failed: %w", err))
 		}
 		if user.GetEmail() != acme.Email {
 			return r.failedAcme(logger, obj, api.StateError, fmt.Errorf("email of registration user from secret does not match %s != %s", user.GetEmail(), acme.Email))
 		}
-		return r.support.SucceededAndTriggerCertificates(logger, obj, &acmeType, issuer.Status.ACME.Raw)
-	} else if secret != nil || acme.AutoRegistration {
-		eabKid, eabHmacKey, err := r.prepareEAB(obj, issuer)
+		wrapped, err := core.WrapRegistration(raw, secretHash)
 		if err != nil {
-			return r.failedAcmeRetry(logger, obj, api.StateError, err)
+			return r.failedAcme(logger, obj, api.StateError, fmt.Errorf("wrapped registration marshalling failed: %w", err))
 		}
-		var secretData map[string][]byte
-		if secret != nil {
-			secretData = secret.Data
-		}
-		user, err := legobridge.NewRegistrationUserFromEmail(issuerKey, acme.Email, acme.Server, secretData, eabKid, eabHmacKey)
+		return r.support.SucceededAndTriggerCertificates(logger, obj, &acmeType, wrapped)
+	} else if acme.AutoRegistration {
+		user, err := legobridge.NewRegistrationUserFromEmail(issuerKey, acme.Email, acme.Server, nil, "", "")
 		if err != nil {
-			return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("creating registration user failed with %s", err.Error()))
+			return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("creating registration user failed: %w", err))
 		}
 
-		if secret != nil {
-			err = r.support.UpdateIssuerSecret(issuerKey, user, secret)
-			if err != nil {
-				return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("updating issuer secret failed with %s", err.Error()))
-			}
-		} else {
-			secretRef, secret, err := r.support.WriteIssuerSecretFromRegistrationUser(issuerKey, issuer.UID, user, acme.PrivateKeySecretRef)
-			if err != nil {
-				return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("writing issuer secret failed with %s", err.Error()))
-			}
-			issuer.Spec.ACME.PrivateKeySecretRef = secretRef
-			hash := r.support.CalcSecretHash(secret)
-			r.support.RememberIssuerSecret(obj.ClusterKey(), issuer.Spec.ACME.PrivateKeySecretRef, hash)
+		secretRef, secret, err := r.support.WriteIssuerSecretFromRegistrationUser(issuerKey, issuer.UID, user, acme.PrivateKeySecretRef)
+		if err != nil {
+			return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("writing issuer secret failed: %w", err))
 		}
+		acme.PrivateKeySecretRef = secretRef
+		secretHash = r.support.CalcSecretHash(secret)
+		r.support.RememberIssuerSecret(obj.ClusterKey(), acme.PrivateKeySecretRef, secretHash)
 
 		regRaw, err := user.RawRegistration()
 		if err != nil {
-			return r.failedAcme(logger, obj, api.StateError, fmt.Errorf("registration marshalling failed with %s", err.Error()))
+			return r.failedAcme(logger, obj, api.StateError, fmt.Errorf("registration marshalling failed: %w", err))
 		}
 		newObj, err := r.support.GetIssuerResources(issuerKey).Update(issuer)
 		if err != nil {
-			return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("updating resource failed with %s", err.Error()))
+			return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("updating resource failed: %w", err))
 		}
 
-		return r.support.SucceededAndTriggerCertificates(logger, newObj, &acmeType, regRaw)
+		raw, err := core.WrapRegistration(regRaw, secretHash)
+		if err != nil {
+			return r.failedAcme(logger, obj, api.StateError, fmt.Errorf("wrapped registration marshalling failed: %w", err))
+		}
+		return r.support.SucceededAndTriggerCertificates(logger, newObj, &acmeType, raw)
 	} else {
 		return r.failedAcme(logger, obj, api.StateError, fmt.Errorf("neither `SecretRef` or `AutoRegistration: true` provided"))
 	}
 }
 
-func (r *acmeIssuerHandler) prepareEAB(obj resources.Object, issuer *api.Issuer) (eabKid, eabHmacKey string, err error) {
-	acme := issuer.Spec.ACME
-	eab := acme.ExternalAccountBinding
-
-	if eab == nil {
-		return
-	}
-
-	r.support.RememberIssuerEABSecret(obj.ClusterKey(), eab.KeySecretRef, "")
-
-	if eab.KeyID == "" {
-		err = fmt.Errorf("missing keyID for external account binding in ACME spec")
-		return
-	}
-
-	if eab.KeySecretRef == nil {
-		err = fmt.Errorf("missing keySecretRef for external account binding in ACME spec")
-		return
-	}
-
-	issuerKey := r.support.ToIssuerKey(obj.ClusterKey())
-	secret, err := r.support.ReadIssuerSecret(issuerKey, eab.KeySecretRef)
-	if err != nil {
-		err = fmt.Errorf("loading issuer secret for external account binding failed with %s", err.Error())
-		return
-	}
-	hash := r.support.CalcSecretHash(secret)
-	r.support.RememberIssuerEABSecret(obj.ClusterKey(), eab.KeySecretRef, hash)
-
-	hmacEncoded, ok := secret.Data[legobridge.KeyHmacKey]
-	if !ok {
-		err = fmt.Errorf("key %s not found in secret data", legobridge.KeyHmacKey)
-		return
-	}
-
-	eabKid = eab.KeyID
-	eabHmacKey = string(hmacEncoded)
-	return
-}
-
 func (r *acmeIssuerHandler) failedAcme(logger logger.LogContext, obj resources.Object, state string, err error) reconcile.Status {
 	return r.support.Failed(logger, obj, state, &acmeType, err, false)
 }