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 <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>

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
 }

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
 	}

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 {

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", "",

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

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

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.