From b4dcfb54b86352ed9dcb821f5db3ee7922583434 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 18 Aug 2026 11:08:46 -0400 Subject: [PATCH] acme: support standards-compliant order finalization When an order finalization response is not immediately valid, Client.CreateOrderCert depends on the ACME server returning a Location header. The header value is used as the URL for polling the updated order status. RFC 8555 does not require this header, so CreateOrderCert does not interoperate with ACME servers that omit it (e.g. Pebble v2.9.0+). This commit adds Client.CreateCertFromOrder, which accepts an *Order and uses its URI for polling instead of relying on the finalization response's Location header. It also updates existing callers and documentation to use the new method and updates the integration tests to Pebble v2.10.1 (the latest available version), which omits the finalization Location header. Fixes golang/go#77704 Change-Id: I0852f19c5d18d6debc9c9686fa2c14a2e39663c2 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/817000 LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com Reviewed-by: Cherry Mui Reviewed-by: Roland Shoemaker --- acme/acme.go | 2 +- acme/autocert/autocert.go | 2 +- acme/internal/acmeprobe/prober.go | 8 ++-- acme/pebble_test.go | 6 +-- acme/rfc8555.go | 52 +++++++++++++++++++--- acme/rfc8555_test.go | 74 +++++++++++++++++++++++++++++++ acme/types.go | 2 +- 7 files changed, 131 insertions(+), 15 deletions(-) diff --git a/acme/acme.go b/acme/acme.go index b53ea28..59ce1e2 100644 --- a/acme/acme.go +++ b/acme/acme.go @@ -221,7 +221,7 @@ func (c *Client) directoryURL() string { // CreateCert was part of the old version of ACME. It is incompatible with RFC 8555. // -// Deprecated: this was for the pre-RFC 8555 version of ACME. Callers should use CreateOrderCert. +// Deprecated: this was for the pre-RFC 8555 version of ACME. Callers should use CreateCertFromOrder. func (c *Client) CreateCert(ctx context.Context, csr []byte, exp time.Duration, bundle bool) (der [][]byte, certURL string, err error) { return nil, "", errPreRFC } diff --git a/acme/autocert/autocert.go b/acme/autocert/autocert.go index bee6de9..1fbbbe8 100644 --- a/acme/autocert/autocert.go +++ b/acme/autocert/autocert.go @@ -671,7 +671,7 @@ func (m *Manager) authorizedCert(ctx context.Context, key crypto.Signer, ck cert if err != nil { return nil, nil, err } - chain, _, err := client.CreateOrderCert(ctx, o.FinalizeURL, csr, true) + chain, _, err := client.CreateCertFromOrder(ctx, o, csr, true) if err != nil { return nil, nil, err } diff --git a/acme/internal/acmeprobe/prober.go b/acme/internal/acmeprobe/prober.go index 25dba0c..49eca2f 100644 --- a/acme/internal/acmeprobe/prober.go +++ b/acme/internal/acmeprobe/prober.go @@ -182,9 +182,9 @@ func (p *prober) runOrder(ctx context.Context, identifiers []acme.AuthzID) { log.Fatalf("WaitOrder(%q): %v", o.URI, err) } csr, certkey := newCSR(identifiers) - der, curl, err := p.client.CreateOrderCert(ctx, o.FinalizeURL, csr, true) + der, curl, err := p.client.CreateCertFromOrder(ctx, o, csr, true) if err != nil { - log.Fatalf("CreateOrderCert: %v", err) + log.Fatalf("CreateCertFromOrder: %v", err) } log.Printf("cert URL: %s", curl) if err := checkCert(der, identifiers); err != nil { @@ -247,9 +247,9 @@ func (p *prober) runPreauthz(ctx context.Context, identifiers []acme.AuthzID) { log.Fatalf("WaitOrder(%q): %v", o.URI, err) } csr, certkey := newCSR(identifiers) - der, curl, err := p.client.CreateOrderCert(ctx, o.FinalizeURL, csr, true) + der, curl, err := p.client.CreateCertFromOrder(ctx, o, csr, true) if err != nil { - log.Fatalf("CreateOrderCert: %v", err) + log.Fatalf("CreateCertFromOrder: %v", err) } log.Printf("cert URL: %s", curl) if err := checkCert(der, identifiers); err != nil { diff --git a/acme/pebble_test.go b/acme/pebble_test.go index 08dbab4..e196f16 100644 --- a/acme/pebble_test.go +++ b/acme/pebble_test.go @@ -37,7 +37,7 @@ import ( const ( // pebbleModVersion is the module version used for Pebble and Pebble's // challenge test server. It is ignored if `-pebble-local-dir` is provided. - pebbleModVersion = "v2.7.0" + pebbleModVersion = "v2.10.1" // startingPort is the first port number used for binding interface // addresses. Each call to takeNextPort() will increment a port number // starting at this value. @@ -420,7 +420,7 @@ func testIssuance(t *testing.T, env *environment, challSrv challengeServer) { } // Finalize the order by creating a certificate with our CSR. - chain, _, err := client.CreateOrderCert(ctx, order.FinalizeURL, csrDer, true) + chain, _, err := client.CreateCertFromOrder(ctx, order, csrDer, true) if err != nil { t.Fatalf("failed to finalize order %s with finalize URL %s: %v", orderURL, order.FinalizeURL, err) @@ -638,7 +638,7 @@ func startPebbleEnvironment(t *testing.T, config *environmentConfig) environment // Note: we specify -defaultIPv6 "" so that no AAAA records are served. // The LUCI CI runners have issues with IPv6 connectivity on localhost. spawnServerProcess(t, binDir, "pebble-challtestsrv", - "-dns01", fmt.Sprintf(":%d", config.dnsPort), + "-dnsserver", fmt.Sprintf(":%d", config.dnsPort), "-defaultIPv6", "", "-management", fmt.Sprintf(":%d", takeNextPort()), "-doh", "", diff --git a/acme/rfc8555.go b/acme/rfc8555.go index 1fb110e..38993f8 100644 --- a/acme/rfc8555.go +++ b/acme/rfc8555.go @@ -197,7 +197,7 @@ func (c *Client) accountKeyRollover(ctx context.Context, newKey crypto.Signer) e // identify those with StatusPending status and fulfill a challenge using Accept. // Once all authorizations are satisfied, the caller will typically want to poll // order status using WaitOrder until it's in StatusReady state. -// To finalize the order and obtain a certificate, the caller submits a CSR with CreateOrderCert. +// To finalize the order and obtain a certificate, the caller submits a CSR with CreateCertFromOrder. func (c *Client) AuthorizeOrder(ctx context.Context, id []AuthzID, opt ...OrderOption) (*Order, error) { dir, err := c.Discover(ctx) if err != nil { @@ -250,7 +250,11 @@ func (c *Client) GetOrder(ctx context.Context, url string) (*Order, error) { return nil, err } defer res.Body.Close() - return responseOrder(res) + o, err := responseOrder(res) + if err == nil && o.URI == "" { + o.URI = url + } + return o, err } // WaitOrder polls an order from the given URL until it is in one of the final states, @@ -271,6 +275,9 @@ func (c *Client) WaitOrder(ctx context.Context, url string) (*Order, error) { } o, err := responseOrder(res) res.Body.Close() + if err == nil && o.URI == "" { + o.URI = url + } switch { case err != nil: // Skip and retry. @@ -338,11 +345,43 @@ func responseOrder(res *http.Response) (*Order, error) { // certificate chain. Otherwise, only a leaf certificate is returned. // The returned URL can be used to re-fetch the certificate using FetchCert. // -// This method is only supported by CAs implementing RFC 8555. See CreateCert for pre-RFC CAs. +// This method is only supported by CAs implementing RFC 8555 that return a +// Location header for order finalization responses. Most users should prefer +// CreateCertFromOrder, an alternative that has better interoperation with RFC 8555 +// compliant CAs. See CreateCert for pre-RFC CAs. // // CreateOrderCert returns an error if the CA's response is unreasonably large. // Callers are encouraged to parse the returned value to ensure the certificate is valid and has the expected features. func (c *Client) CreateOrderCert(ctx context.Context, url string, csr []byte, bundle bool) (der [][]byte, certURL string, err error) { + return c.createOrderCert(ctx, url, "", csr, bundle) +} + +// CreateCertFromOrder submits the CSR (Certificate Signing Request) to a CA for +// the specified Order created with AuthorizeOrder. +// +// If the bundle argument is true, the returned value also contain the CA (issuer) +// certificate chain. Otherwise, only a leaf certificate is returned. +// The returned URL can be used to re-fetch the certificate using FetchCert. +// +// This method is only supported by CAs implementing RFC 8555. +// See CreateCert for pre-RFC CAs. +// +// CreateCertFromOrder returns an error if the CA's response is unreasonably large. +// Callers are encouraged to parse the returned value to ensure the certificate is valid and has the expected features. +func (c *Client) CreateCertFromOrder(ctx context.Context, order *Order, csr []byte, bundle bool) (der [][]byte, certURL string, err error) { + if order == nil { + return nil, "", errors.New("acme: nil order") + } + if order.URI == "" { + return nil, "", errors.New("acme: order URI is empty") + } + if order.FinalizeURL == "" { + return nil, "", errors.New("acme: order finalize URL is empty") + } + return c.createOrderCert(ctx, order.FinalizeURL, order.URI, csr, bundle) +} + +func (c *Client) createOrderCert(ctx context.Context, finalizeURL, orderURL string, csr []byte, bundle bool) (der [][]byte, certURL string, err error) { if _, err := c.Discover(ctx); err != nil { // required by c.accountKID return nil, "", err } @@ -353,7 +392,7 @@ func (c *Client) CreateOrderCert(ctx context.Context, url string, csr []byte, bu }{ CSR: base64.RawURLEncoding.EncodeToString(csr), } - res, err := c.post(ctx, nil, url, req, wantStatus(http.StatusOK)) + res, err := c.post(ctx, nil, finalizeURL, req, wantStatus(http.StatusOK)) if err != nil { return nil, "", err } @@ -365,7 +404,10 @@ func (c *Client) CreateOrderCert(ctx context.Context, url string, csr []byte, bu // Wait for CA to issue the cert if they haven't. if o.Status != StatusValid { - o, err = c.WaitOrder(ctx, o.URI) + if orderURL == "" { + orderURL = o.URI + } + o, err = c.WaitOrder(ctx, orderURL) } if err != nil { return nil, "", err diff --git a/acme/rfc8555_test.go b/acme/rfc8555_test.go index e9cedb5..6daa13e 100644 --- a/acme/rfc8555_test.go +++ b/acme/rfc8555_test.go @@ -979,6 +979,80 @@ func TestRFC_CreateOrderCert(t *testing.T) { } } +// TestRFC_CreateCertFromOrder tests that finalization responses work even when +// they omit a Location header. Such a header isn't required by RFC 8555 in this +// context and we should be able to complete an order without needing it. +func TestRFC_CreateCertFromOrder(t *testing.T) { + q := &x509.CertificateRequest{ + Subject: pkix.Name{CommonName: "example.org"}, + } + csr, err := x509.CreateCertificateRequest(rand.Reader, q, testKeyEC) + if err != nil { + t.Fatal(err) + } + + tmpl := &x509.Certificate{SerialNumber: big.NewInt(1)} + leaf, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &testKeyEC.PublicKey, testKeyEC) + if err != nil { + t.Fatal(err) + } + + s := newACMEServer() + s.handle("/acme/new-account", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Location", s.url("/accounts/1")) + w.Write([]byte(`{"status": "valid"}`)) + }) + s.handle("/finalize", func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, `{"status":"processing"}`) + }) + s.handle("/orders/1", func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintf(w, `{"status":"valid", "certificate":%q}`, s.url("/crt")) + }) + s.handle("/crt", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/pem-certificate-chain") + pem.Encode(w, &pem.Block{Type: "CERTIFICATE", Bytes: leaf}) + }) + s.start() + defer s.close() + + cl := &Client{Key: testKeyEC, DirectoryURL: s.url("/")} + order := &Order{URI: s.url("/orders/1"), FinalizeURL: s.url("/finalize")} + cert, curl, err := cl.CreateCertFromOrder(context.Background(), order, csr, true) + if err != nil { + t.Fatalf("CreateCertFromOrder: %v", err) + } + if _, err := x509.ParseCertificate(cert[0]); err != nil { + t.Errorf("ParseCertificate: %v", err) + } + if !reflect.DeepEqual(cert[0], leaf) { + t.Errorf("cert and leaf bytes don't match") + } + if u := s.url("/crt"); curl != u { + t.Errorf("curl = %q; want %q", curl, u) + } +} + +// TestRFC_CreateCertFromOrderInvalidOrder tests that invalid CreateCertFromOrder +// calls fail before making a request. +func TestRFC_CreateCertFromOrderInvalidOrder(t *testing.T) { + cl := new(Client) + tests := []struct { + name string + order *Order + }{ + {"nil", nil}, + {"empty URI", &Order{FinalizeURL: "https://example.com/finalize"}}, + {"empty finalize URL", &Order{URI: "https://example.com/order/1"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if _, _, err := cl.CreateCertFromOrder(context.Background(), tt.order, nil, false); err == nil { + t.Fatal("CreateCertFromOrder returned nil error") + } + }) + } +} + func TestRFC_AlreadyRevokedCert(t *testing.T) { s := newACMEServer() s.handle("/acme/revoke-cert", func(w http.ResponseWriter, r *http.Request) { diff --git a/acme/types.go b/acme/types.go index 4df7e6d..c1492fb 100644 --- a/acme/types.go +++ b/acme/types.go @@ -330,7 +330,7 @@ type Order struct { // Possible values are StatusPending, StatusReady, StatusProcessing, StatusValid and StatusInvalid. // Pending means the CA does not believe that the client has fulfilled the requirements. // Ready indicates that the client has fulfilled all the requirements and can submit a CSR - // to obtain a certificate. This is done with Client's CreateOrderCert. + // to obtain a certificate. This is done with Client's CreateCertFromOrder. // Processing means the certificate is being issued. // Valid indicates the CA has issued the certificate. It can be downloaded // from the Order's CertURL. This is done with Client's FetchCert.