From 6dd8d035d18b3d858f419e0ed735162d25a290d1 Mon Sep 17 00:00:00 2001 From: Samantha Date: Fri, 8 Mar 2024 10:22:09 -0500 Subject: [PATCH] feat: implement 'replaces' field in newOrder and draft-ietf-acme-ari-03 CertID changes (#2114) --- .golangci.yml | 2 +- README.md | 2 +- acme/api/order.go | 8 +++ acme/commons.go | 10 ++- certificate/certificates.go | 18 +++-- certificate/renewal.go | 59 +++++++--------- certificate/renewal_test.go | 134 +++--------------------------------- cmd/cmd_renew.go | 28 ++++---- 8 files changed, 79 insertions(+), 182 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 9571af97..831b7606 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -226,6 +226,6 @@ issues: - path: providers/dns/hosttech/internal/client_test.go text: 'Duplicate words \(0\) found' - path: cmd/cmd_renew.go - text: 'cyclomatic complexity 15 of func `renewForDomains` is high' + text: 'cyclomatic complexity \d+ of func `renewForDomains` is high' - path: providers/dns/cpanel/cpanel.go text: 'cyclomatic complexity 13 of func `\(\*DNSProvider\)\.CleanUp` is high' diff --git a/README.md b/README.md index ff62dd69..6f545e35 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ Let's Encrypt client and ACME library written in Go. - ACME v2 [RFC 8555](https://www.rfc-editor.org/rfc/rfc8555.html) - Support [RFC 8737](https://www.rfc-editor.org/rfc/rfc8737.html): TLS Application‑Layer Protocol Negotiation (ALPN) Challenge Extension - Support [RFC 8738](https://www.rfc-editor.org/rfc/rfc8738.html): certificates for IP addresses - - Support [draft-ietf-acme-ari-02](https://datatracker.ietf.org/doc/draft-ietf-acme-ari/): Renewal Information (ARI) Extension + - Support [draft-ietf-acme-ari-03](https://datatracker.ietf.org/doc/draft-ietf-acme-ari/): Renewal Information (ARI) Extension - Register with CA - Obtain certificates, both from scratch or with an existing CSR - Renew certificates diff --git a/acme/api/order.go b/acme/api/order.go index fe1be94f..5179d061 100644 --- a/acme/api/order.go +++ b/acme/api/order.go @@ -13,6 +13,10 @@ import ( type OrderOptions struct { NotBefore time.Time NotAfter time.Time + // A string uniquely identifying a previously-issued certificate which this + // order is intended to replace. + // - https://datatracker.ietf.org/doc/html/draft-ietf-acme-ari-03#section-5 + ReplacesCertID string } type OrderService service @@ -45,6 +49,10 @@ func (o *OrderService) NewWithOptions(domains []string, opts *OrderOptions) (acm if !opts.NotBefore.IsZero() { orderReq.NotBefore = opts.NotBefore.Format(time.RFC3339) } + + if o.core.GetDirectory().RenewalInfo != "" { + orderReq.Replaces = opts.ReplacesCertID + } } var order acme.Order diff --git a/acme/commons.go b/acme/commons.go index 70b2783d..39aa35ac 100644 --- a/acme/commons.go +++ b/acme/commons.go @@ -181,6 +181,12 @@ type Order struct { // certificate (optional, string): // A URL for the certificate that has been issued in response to this order Certificate string `json:"certificate,omitempty"` + + // replaces (optional, string): + // replaces (string, optional): A string uniquely identifying a + // previously-issued certificate which this order is intended to replace. + // - https://datatracker.ietf.org/doc/html/draft-ietf-acme-ari-03#section-5 + Replaces string `json:"replaces,omitempty"` } // Authorization the ACME authorization object. @@ -329,11 +335,11 @@ type RenewalInfoResponse struct { } // RenewalInfoUpdateRequest is the JWS payload for POST requests made to the renewalInfo endpoint. -// - (4.2. RenewalInfo Objects) https://datatracker.ietf.org/doc/html/draft-ietf-acme-ari-02#section-4.2 +// - (4.2. RenewalInfo Objects) https://datatracker.ietf.org/doc/html/draft-ietf-acme-ari-03#section-4.2 type RenewalInfoUpdateRequest struct { // CertID is a composite string in the format: base64url(AKI) || '.' || base64url(Serial), where AKI is the // certificate's authority key identifier and Serial is the certificate's serial number. For details, see: - // https://datatracker.ietf.org/doc/html/draft-ietf-acme-ari-02#section-4.1 + // https://datatracker.ietf.org/doc/html/draft-ietf-acme-ari-03#section-4.1 CertID string `json:"certID"` // Replaced is required and indicates whether or not the client considers the certificate to have been replaced. // A certificate is considered replaced when its revocation would not disrupt any ongoing services, diff --git a/certificate/certificates.go b/certificate/certificates.go index d6a7438c..7e69d1f4 100644 --- a/certificate/certificates.go +++ b/certificate/certificates.go @@ -63,6 +63,10 @@ type ObtainRequest struct { Bundle bool PreferredChain string AlwaysDeactivateAuthorizations bool + // A string uniquely identifying a previously-issued certificate which this + // order is intended to replace. + // - https://datatracker.ietf.org/doc/html/draft-ietf-acme-ari-03#section-5 + ReplacesCertID string } // ObtainForCSRRequest The request to obtain a certificate matching the CSR passed into it. @@ -79,6 +83,10 @@ type ObtainForCSRRequest struct { Bundle bool PreferredChain string AlwaysDeactivateAuthorizations bool + // A string uniquely identifying a previously-issued certificate which this + // order is intended to replace. + // - https://datatracker.ietf.org/doc/html/draft-ietf-acme-ari-03#section-5 + ReplacesCertID string } type resolver interface { @@ -124,8 +132,9 @@ func (c *Certifier) Obtain(request ObtainRequest) (*Resource, error) { } orderOpts := &api.OrderOptions{ - NotBefore: request.NotBefore, - NotAfter: request.NotAfter, + NotBefore: request.NotBefore, + NotAfter: request.NotAfter, + ReplacesCertID: request.ReplacesCertID, } order, err := c.core.Orders.NewWithOptions(domains, orderOpts) @@ -189,8 +198,9 @@ func (c *Certifier) ObtainForCSR(request ObtainForCSRRequest) (*Resource, error) } orderOpts := &api.OrderOptions{ - NotBefore: request.NotBefore, - NotAfter: request.NotAfter, + NotBefore: request.NotBefore, + NotAfter: request.NotAfter, + ReplacesCertID: request.ReplacesCertID, } order, err := c.core.Orders.NewWithOptions(domains, orderOpts) diff --git a/certificate/renewal.go b/certificate/renewal.go index 314cd3ea..4f420301 100644 --- a/certificate/renewal.go +++ b/certificate/renewal.go @@ -2,12 +2,12 @@ package certificate import ( "crypto/x509" + "encoding/asn1" "encoding/base64" "encoding/json" "errors" "fmt" "math/rand" - "strings" "time" "github.com/go-acme/lego/v4/acme" @@ -65,7 +65,7 @@ func (r *RenewalInfoResponse) ShouldRenewAt(now time.Time, willingToSleep time.D // // https://datatracker.ietf.org/doc/draft-ietf-acme-ari func (c *Certifier) GetRenewalInfo(req RenewalInfoRequest) (*RenewalInfoResponse, error) { - certID, err := makeARICertID(req.Cert) + certID, err := MakeARICertID(req.Cert) if err != nil { return nil, fmt.Errorf("error making certID: %w", err) } @@ -84,39 +84,32 @@ func (c *Certifier) GetRenewalInfo(req RenewalInfoRequest) (*RenewalInfoResponse return &info, nil } -// UpdateRenewalInfo sends an update to the ACME server's renewal info endpoint to indicate that the client has successfully replaced a certificate. -// A certificate is considered replaced when its revocation would not disrupt any ongoing services, -// for instance because it has been renewed and the new certificate is in use, or because it is no longer in use. -// -// Note: this endpoint is part of a draft specification, not all ACME servers will implement it. -// This method will return api.ErrNoARI if the server does not advertise a renewal info endpoint. -// -// https://datatracker.ietf.org/doc/draft-ietf-acme-ari -func (c *Certifier) UpdateRenewalInfo(req RenewalInfoRequest) error { - certID, err := makeARICertID(req.Cert) - if err != nil { - return fmt.Errorf("error making certID: %w", err) - } - - _, err = c.core.Certificates.UpdateRenewalInfo(acme.RenewalInfoUpdateRequest{ - CertID: certID, - Replaced: true, - }) - if err != nil { - return err - } - - return nil -} - -// makeARICertID constructs a certificate identifier as described in draft-ietf-acme-ari-02, section 4.1. -func makeARICertID(leaf *x509.Certificate) (string, error) { +// MakeARICertID constructs a certificate identifier as described in draft-ietf-acme-ari-03, section 4.1. +func MakeARICertID(leaf *x509.Certificate) (string, error) { if leaf == nil { return "", errors.New("leaf certificate is nil") } - return fmt.Sprintf("%s.%s", - strings.TrimRight(base64.URLEncoding.EncodeToString(leaf.AuthorityKeyId), "="), - strings.TrimRight(base64.URLEncoding.EncodeToString(leaf.SerialNumber.Bytes()), "="), - ), nil + // Marshal the Serial Number into DER. + der, err := asn1.Marshal(leaf.SerialNumber) + if err != nil { + return "", err + } + + // Check if the DER encoded bytes are sufficient (at least 3 bytes: tag, + // length, and value). + if len(der) < 3 { + return "", errors.New("invalid DER encoding of serial number") + } + + // Extract only the integer bytes from the DER encoded Serial Number + // Skipping the first 2 bytes (tag and length). + serial := base64.RawURLEncoding.EncodeToString(der[2:]) + + // Convert the Authority Key Identifier to base64url encoding without + // padding. + aki := base64.RawURLEncoding.EncodeToString(leaf.AuthorityKeyId) + + // Construct the final identifier by concatenating AKI and Serial Number. + return fmt.Sprintf("%s.%s", aki, serial), nil } diff --git a/certificate/renewal_test.go b/certificate/renewal_test.go index 23815924..ef6fb399 100644 --- a/certificate/renewal_test.go +++ b/certificate/renewal_test.go @@ -3,8 +3,6 @@ package certificate import ( "crypto/rand" "crypto/rsa" - "encoding/json" - "io" "net/http" "testing" "time" @@ -13,40 +11,28 @@ import ( "github.com/go-acme/lego/v4/acme/api" "github.com/go-acme/lego/v4/certcrypto" "github.com/go-acme/lego/v4/platform/tester" - "github.com/go-jose/go-jose/v4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) const ( ariLeafPEM = `-----BEGIN CERTIFICATE----- -MIIDMDCCAhigAwIBAgIIPqNFaGVEHxwwDQYJKoZIhvcNAQELBQAwIDEeMBwGA1UE -AxMVbWluaWNhIHJvb3QgY2EgM2ExMzU2MB4XDTIyMDMxNzE3NTEwOVoXDTI0MDQx -NjE3NTEwOVowFjEUMBIGA1UEAxMLZXhhbXBsZS5jb20wggEiMA0GCSqGSIb3DQEB -AQUAA4IBDwAwggEKAoIBAQCgm9K/c+il2Pf0f8qhgxn9SKqXq88cOm9ov9AVRbPA -OWAAewqX2yUAwI4LZBGEgzGzTATkiXfoJ3cN3k39cH6tBbb3iSPuEn7OZpIk9D+e -3Q9/hX+N/jlWkaTB/FNA+7aE5IVWhmdczYilXa10V9r+RcvACJt0gsipBZVJ4jfJ -HnWJJGRZzzxqG/xkQmpXxZO7nOPFc8SxYKWdfcgp+rjR2ogYhSz7BfKoVakGPbpX -vZOuT9z4kkHra/WjwlkQhtHoTXdAxH3qC2UjMzO57Tx+otj0CxAv9O7CTJXISywB -vEVcmTSZkHS3eZtvvIwPx7I30ITRkYk/tLl1MbyB3SiZAgMBAAGjeDB2MA4GA1Ud -DwEB/wQEAwIFoDAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYBBQUHAwIwDAYDVR0T -AQH/BAIwADAfBgNVHSMEGDAWgBQ4zzDRUaXHVKqlSTWkULGU4zGZpTAWBgNVHREE -DzANggtleGFtcGxlLmNvbTANBgkqhkiG9w0BAQsFAAOCAQEAx0aYvmCk7JYGNEXe -+hrOfKawkHYzWvA92cI/Oi6h+oSdHZ2UKzwFNf37cVKZ37FCrrv5pFP/xhhHvrNV -EnOx4IaF7OrnaTu5miZiUWuvRQP7ZGmGNFYbLTEF6/dj+WqyYdVaWzxRqHFu1ptC -TXysJCeyiGnR+KOOjOOQ9ZlO5JUK3OE4hagPLfaIpDDy6RXQt3ss0iNLuB1+IOtp -1URpvffLZQ8xPsEgOZyPWOcabTwJrtqBwily+lwPFn2mChUx846LwQfxtsXU/lJg -HX2RteNJx7YYNeX3Uf960mgo5an6vE8QNAsIoNHYrGyEmXDhTRe9mCHyiW2S7fZq -o9q12g== +MIIBQzCB66ADAgECAgUAh2VDITAKBggqhkjOPQQDAjAVMRMwEQYDVQQDEwpFeGFt +cGxlIENBMCIYDzAwMDEwMTAxMDAwMDAwWhgPMDAwMTAxMDEwMDAwMDBaMBYxFDAS +BgNVBAMTC2V4YW1wbGUuY29tMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEeBZu +7cbpAYNXZLbbh8rNIzuOoqOOtmxA1v7cRm//AwyMwWxyHz4zfwmBhcSrf47NUAFf +qzLQ2PPQxdTXREYEnKMjMCEwHwYDVR0jBBgwFoAUaYhba4dGQEHhs3uEe6CuLN4B +yNQwCgYIKoZIzj0EAwIDRwAwRAIge09+S5TZAlw5tgtiVvuERV6cT4mfutXIlwTb ++FYN/8oCIClDsqBklhB9KAelFiYt9+6FDj3z4KGVelYM5MdsO3pK -----END CERTIFICATE-----` - ariLeafCertID = "OM8w0VGlx1SqpUk1pFCxlOMxmaU.PqNFaGVEHxw" + ariLeafCertID = "aYhba4dGQEHhs3uEe6CuLN4ByNQ.AIdlQyE" ) func Test_makeCertID(t *testing.T) { leaf, err := certcrypto.ParsePEMCertificate([]byte(ariLeafPEM)) require.NoError(t, err) - actual, err := makeARICertID(leaf) + actual, err := MakeARICertID(leaf) require.NoError(t, err) assert.Equal(t, ariLeafCertID, actual) } @@ -145,85 +131,6 @@ func TestCertifier_GetRenewalInfo_errors(t *testing.T) { } } -func TestCertifier_UpdateRenewalInfo(t *testing.T) { - leaf, err := certcrypto.ParsePEMCertificate([]byte(ariLeafPEM)) - require.NoError(t, err) - - key, err := rsa.GenerateKey(rand.Reader, 2048) - require.NoError(t, err, "Could not generate test key") - - // Test with a fake API. - mux, apiURL := tester.SetupFakeAPI(t) - mux.HandleFunc("/renewalInfo", func(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodPost { - http.Error(w, http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed) - return - } - - body, rsbErr := readSignedBody(r, key) - if rsbErr != nil { - http.Error(w, rsbErr.Error(), http.StatusBadRequest) - return - } - - var req acme.RenewalInfoUpdateRequest - err = json.Unmarshal(body, &req) - assert.NoError(t, err) - assert.True(t, req.Replaced) - assert.Equal(t, ariLeafCertID, req.CertID) - - w.WriteHeader(http.StatusOK) - }) - - core, err := api.New(http.DefaultClient, "lego-test", apiURL+"/dir", "", key) - require.NoError(t, err) - - certifier := NewCertifier(core, &resolverMock{}, CertifierOptions{KeyType: certcrypto.RSA2048}) - - err = certifier.UpdateRenewalInfo(RenewalInfoRequest{leaf}) - require.NoError(t, err) -} - -func TestCertifier_UpdateRenewalInfo_errors(t *testing.T) { - leaf, err := certcrypto.ParsePEMCertificate([]byte(ariLeafPEM)) - require.NoError(t, err) - - key, err := rsa.GenerateKey(rand.Reader, 2048) - require.NoError(t, err, "Could not generate test key") - - testCases := []struct { - desc string - request RenewalInfoRequest - }{ - { - desc: "API error", - request: RenewalInfoRequest{leaf}, - }, - } - - for _, test := range testCases { - test := test - t.Run(test.desc, func(t *testing.T) { - t.Parallel() - - mux, apiURL := tester.SetupFakeAPI(t) - - // Always returns an error. - mux.HandleFunc("/renewalInfo", func(w http.ResponseWriter, r *http.Request) { - http.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest) - }) - - core, err := api.New(http.DefaultClient, "lego-test", apiURL+"/dir", "", key) - require.NoError(t, err) - - certifier := NewCertifier(core, &resolverMock{}, CertifierOptions{KeyType: certcrypto.RSA2048}) - - err = certifier.UpdateRenewalInfo(test.request) - require.Error(t, err) - }) - } -} - func TestRenewalInfoResponse_ShouldRenew(t *testing.T) { now := time.Now().UTC() @@ -289,26 +196,3 @@ func TestRenewalInfoResponse_ShouldRenew(t *testing.T) { assert.Nil(t, rt) }) } - -func readSignedBody(r *http.Request, privateKey *rsa.PrivateKey) ([]byte, error) { - reqBody, err := io.ReadAll(r.Body) - if err != nil { - return nil, err - } - - sigAlgs := []jose.SignatureAlgorithm{jose.RS256} - jws, err := jose.ParseSigned(string(reqBody), sigAlgs) - if err != nil { - return nil, err - } - - body, err := jws.Verify(&jose.JSONWebKey{ - Key: privateKey.Public(), - Algorithm: "RSA", - }) - if err != nil { - return nil, err - } - - return body, nil -} diff --git a/cmd/cmd_renew.go b/cmd/cmd_renew.go index 0940f580..6c0c7853 100644 --- a/cmd/cmd_renew.go +++ b/cmd/cmd_renew.go @@ -187,6 +187,11 @@ func renewForDomains(ctx *cli.Context, client *lego.Client, certsStorage *Certif time.Sleep(sleepTime) } + replacesCertID, err := certificate.MakeARICertID(cert) + if err != nil { + log.Fatalf("Error while construction the ARI CertID for domain %s\n\t%v", domain, err) + } + request := certificate.ObtainRequest{ Domains: merge(certDomains, domains), PrivateKey: privateKey, @@ -196,6 +201,7 @@ func renewForDomains(ctx *cli.Context, client *lego.Client, certsStorage *Certif Bundle: bundle, PreferredChain: ctx.String("preferred-chain"), AlwaysDeactivateAuthorizations: ctx.Bool("always-deactivate-authorizations"), + ReplacesCertID: replacesCertID, } certRes, err := client.Certificate.Obtain(request) @@ -205,14 +211,6 @@ func renewForDomains(ctx *cli.Context, client *lego.Client, certsStorage *Certif certsStorage.SaveResource(certRes) - if ariRenewalTime != nil { - // Post to the renewalInfo endpoint to indicate that we have renewed and replaced the certificate. - err := client.Certificate.UpdateRenewalInfo(certificate.RenewalInfoRequest{Cert: certificates[0]}) - if err != nil { - log.Warnf("[%s] Failed to update renewal info: %v", domain, err) - } - } - meta[renewEnvCertDomain] = domain meta[renewEnvCertPath] = certsStorage.GetFileName(domain, ".crt") meta[renewEnvCertKeyPath] = certsStorage.GetFileName(domain, ".key") @@ -264,6 +262,11 @@ func renewForCSR(ctx *cli.Context, client *lego.Client, certsStorage *Certificat timeLeft := cert.NotAfter.Sub(time.Now().UTC()) log.Infof("[%s] acme: Trying renewal with %d hours remaining", domain, int(timeLeft.Hours())) + replacesCertID, err := certificate.MakeARICertID(cert) + if err != nil { + log.Fatalf("Error while construction the ARI CertID for domain %s\n\t%v", domain, err) + } + request := certificate.ObtainForCSRRequest{ CSR: csr, NotBefore: getTime(ctx, "not-before"), @@ -271,6 +274,7 @@ func renewForCSR(ctx *cli.Context, client *lego.Client, certsStorage *Certificat Bundle: bundle, PreferredChain: ctx.String("preferred-chain"), AlwaysDeactivateAuthorizations: ctx.Bool("always-deactivate-authorizations"), + ReplacesCertID: replacesCertID, } certRes, err := client.Certificate.ObtainForCSR(request) @@ -280,14 +284,6 @@ func renewForCSR(ctx *cli.Context, client *lego.Client, certsStorage *Certificat certsStorage.SaveResource(certRes) - if ariRenewalTime != nil { - // Post to the renewalInfo endpoint to indicate that we have renewed and replaced the certificate. - err := client.Certificate.UpdateRenewalInfo(certificate.RenewalInfoRequest{Cert: certificates[0]}) - if err != nil { - log.Warnf("[%s] Failed to update renewal info: %v", domain, err) - } - } - meta[renewEnvCertDomain] = domain meta[renewEnvCertPath] = certsStorage.GetFileName(domain, ".crt") meta[renewEnvCertKeyPath] = certsStorage.GetFileName(domain, ".key")