fix: preferred chain support. (#1298)

acme/api/certificate.go

@@ -20,19 +20,82 @@ type CertificateService service
 // Get Returns the certificate and the issuer certificate.
 // 'bundle' is only applied if the issuer is provided by the 'up' link.
 func (c *CertificateService) Get(certURL string, bundle bool) ([]byte, []byte, error) {
-	cert, up, err := c.get(certURL)
+	cert, _, err := c.get(certURL, bundle)
 	if err != nil {
 		return nil, nil, err
 	}
 
+	return cert.Cert, cert.Issuer, nil
+}
+
+// GetAll the certificates and the alternate certificates.
+// bundle' is only applied if the issuer is provided by the 'up' link.
+func (c *CertificateService) GetAll(certURL string, bundle bool) (map[string]*acme.RawCertificate, error) {
+	cert, headers, err := c.get(certURL, bundle)
+	if err != nil {
+		return nil, err
+	}
+
+	certs := map[string]*acme.RawCertificate{certURL: cert}
+
+	// URLs of "alternate" link relation
+	// - https://tools.ietf.org/html/rfc8555#section-7.4.2
+	alts := getLinks(headers, "alternate")
+
+	for _, alt := range alts {
+		altCert, _, err := c.get(alt, bundle)
+		if err != nil {
+			return nil, err
+		}
+
+		certs[alt] = altCert
+	}
+
+	return certs, nil
+}
+
+// Revoke Revokes a certificate.
+func (c *CertificateService) Revoke(req acme.RevokeCertMessage) error {
+	_, err := c.core.post(c.core.GetDirectory().RevokeCertURL, req, nil)
+	return err
+}
+
+// get Returns the certificate and the "up" link.
+func (c *CertificateService) get(certURL string, bundle bool) (*acme.RawCertificate, http.Header, error) {
+	if len(certURL) == 0 {
+		return nil, nil, errors.New("certificate[get]: empty URL")
+	}
+
+	resp, err := c.core.postAsGet(certURL, nil)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	data, err := ioutil.ReadAll(http.MaxBytesReader(nil, resp.Body, maxBodySize))
+	if err != nil {
+		return nil, resp.Header, err
+	}
+
+	cert := c.getCertificateChain(data, resp.Header, bundle, certURL)
+
+	return cert, resp.Header, err
+}
+
+// getCertificateChain Returns the certificate and the issuer certificate.
+func (c *CertificateService) getCertificateChain(cert []byte, headers http.Header, bundle bool, certURL string) *acme.RawCertificate {
 	// Get issuerCert from bundled response from Let's Encrypt
 	// See https://community.letsencrypt.org/t/acme-v2-no-up-link-in-response/64962
 	_, issuer := pem.Decode(cert)
 	if issuer != nil {
-		return cert, issuer, nil
+		return &acme.RawCertificate{Cert: cert, Issuer: issuer}
 	}
 
-	issuer, err = c.getIssuerFromLink(up)
+	// The issuer certificate link may be supplied via an "up" link
+	// in the response headers of a new certificate.
+	// See https://tools.ietf.org/html/rfc8555#section-7.4.2
+	up := getLink(headers, "up")
+
+	issuer, err := c.getIssuerFromLink(up)
 	if err != nil {
 		// If we fail to acquire the issuer cert, return the issued certificate - do not fail.
 		log.Warnf("acme: Could not bundle issuer certificate [%s]: %v", certURL, err)
@@ -44,37 +107,7 @@ func (c *CertificateService) Get(certURL string, bundle bool) ([]byte, []byte, e
 		}
 	}
 
-	return cert, issuer, nil
-}
-
-// Revoke Revokes a certificate.
-func (c *CertificateService) Revoke(req acme.RevokeCertMessage) error {
-	_, err := c.core.post(c.core.GetDirectory().RevokeCertURL, req, nil)
-	return err
-}
-
-// get Returns the certificate and the "up" link.
-func (c *CertificateService) get(certURL string) ([]byte, string, error) {
-	if len(certURL) == 0 {
-		return nil, "", errors.New("certificate[get]: empty URL")
-	}
-
-	resp, err := c.core.postAsGet(certURL, nil)
-	if err != nil {
-		return nil, "", err
-	}
-
-	cert, err := ioutil.ReadAll(http.MaxBytesReader(nil, resp.Body, maxBodySize))
-	if err != nil {
-		return nil, "", err
-	}
-
-	// The issuer certificate link may be supplied via an "up" link
-	// in the response headers of a new certificate.
-	// See https://tools.ietf.org/html/rfc8555#section-7.4.2
-	up := getLink(resp.Header, "up")
-
-	return cert, up, err
+	return &acme.RawCertificate{Cert: cert, Issuer: issuer}
 }
 
 // getIssuerFromLink requests the issuer certificate.
@@ -85,15 +118,15 @@ func (c *CertificateService) getIssuerFromLink(up string) ([]byte, error) {
 
 	log.Infof("acme: Requesting issuer cert from %s", up)
 
-	cert, _, err := c.get(up)
+	cert, _, err := c.get(up, false)
 	if err != nil {
 		return nil, err
 	}
 
-	_, err = x509.ParseCertificate(cert)
+	_, err = x509.ParseCertificate(cert.Cert)
 	if err != nil {
 		return nil, err
 	}
 
-	return certcrypto.PEMEncode(certcrypto.DERCertificateBytes(cert)), nil
+	return certcrypto.PEMEncode(certcrypto.DERCertificateBytes(cert.Cert)), nil
 }

acme/api/order.go

@@ -25,9 +25,8 @@ func (o *OrderService) New(domains []string) (acme.ExtendedOrder, error) {
 	}
 
 	return acme.ExtendedOrder{
-		Order:               order,
-		Location:            resp.Header.Get("Location"),
-		AlternateChainLinks: getLinks(resp.Header, "alternate"),
+		Order:    order,
+		Location: resp.Header.Get("Location"),
 	}, nil
 }
 
@@ -38,15 +37,12 @@ func (o *OrderService) Get(orderURL string) (acme.ExtendedOrder, error) {
 	}
 
 	var order acme.Order
-	resp, err := o.core.postAsGet(orderURL, &order)
+	_, err := o.core.postAsGet(orderURL, &order)
 	if err != nil {
 		return acme.ExtendedOrder{}, err
 	}
 
-	return acme.ExtendedOrder{
-		Order:               order,
-		AlternateChainLinks: getLinks(resp.Header, "alternate"),
-	}, nil
+	return acme.ExtendedOrder{Order: order}, nil
 }
 
 // UpdateForCSR Updates an order for a CSR.
@@ -56,7 +52,7 @@ func (o *OrderService) UpdateForCSR(orderURL string, csr []byte) (acme.ExtendedO
 	}
 
 	var order acme.Order
-	resp, err := o.core.post(orderURL, csrMsg, &order)
+	_, err := o.core.post(orderURL, csrMsg, &order)
 	if err != nil {
 		return acme.ExtendedOrder{}, err
 	}
@@ -65,8 +61,5 @@ func (o *OrderService) UpdateForCSR(orderURL string, csr []byte) (acme.ExtendedO
 		return acme.ExtendedOrder{}, order.Error
 	}
 
-	return acme.ExtendedOrder{
-		Order:               order,
-		AlternateChainLinks: getLinks(resp.Header, "alternate"),
-	}, nil
+	return acme.ExtendedOrder{Order: order}, nil
 }

acme/api/order_test.go

@@ -41,10 +41,6 @@ func TestOrderService_New(t *testing.T) {
 			return
 		}
 
-		w.Header().Add("Link", `<https://example.com/acme/cert/1>;rel="alternate"`)
-		w.Header().Add("Link", `<https://example.com/acme/cert/2>;title="foo";rel="alternate"`)
-		w.Header().Add("Link", `<https://example.com/acme/cert/3>;title="foo";rel="alternate", <https://example.com/acme/cert/4>;rel="alternate"`)
-
 		err = tester.WriteJSONResponse(w, acme.Order{
 			Status:      acme.StatusValid,
 			Identifiers: order.Identifiers,
@@ -66,12 +62,6 @@ func TestOrderService_New(t *testing.T) {
 			Status:      "valid",
 			Identifiers: []acme.Identifier{{Type: "dns", Value: "example.com"}},
 		},
-		AlternateChainLinks: []string{
-			"https://example.com/acme/cert/1",
-			"https://example.com/acme/cert/2",
-			"https://example.com/acme/cert/3",
-			"https://example.com/acme/cert/4",
-		},
 	}
 	assert.Equal(t, expected, order)
 }

acme/commons.go

@@ -106,13 +106,9 @@ type Account struct {
 // ExtendedOrder a extended Order.
 type ExtendedOrder struct {
 	Order
+
 	// The order URL, contains the value of the response header `Location`
 	Location string `json:"-"`
-
-	// AlternateChainLinks (optional, array of string):
-	// URLs of "alternate" link relation
-	// - https://tools.ietf.org/html/rfc8555#section-7.4.2
-	AlternateChainLinks []string `json:"-"`
 }
 
 // Order the ACME order Object.
@@ -287,3 +283,9 @@ type RevokeCertMessage struct {
 	// The problem document detail SHOULD indicate which reasonCodes are allowed.
 	Reason *uint `json:"reason,omitempty"`
 }
+
+// RawCertificate raw data of a certificate.
+type RawCertificate struct {
+	Cert   []byte
+	Issuer []byte
+}

certificate/certificates.go

@@ -307,29 +307,25 @@ func (c *Certifier) checkResponse(order acme.ExtendedOrder, certRes *Resource, b
 		return valid, err
 	}
 
-	links := append([]string{order.Certificate}, order.AlternateChainLinks...)
+	certs, err := c.core.Certificates.GetAll(order.Certificate, bundle)
+	if err != nil {
+		return false, err
+	}
 
-	for i, link := range links {
-		cert, issuer, err := c.core.Certificates.Get(link, bundle)
-		if err != nil {
-			return false, err
-		}
+	// Set the default certificate
+	certRes.IssuerCertificate = certs[order.Certificate].Issuer
+	certRes.Certificate = certs[order.Certificate].Cert
+	certRes.CertURL = order.Certificate
+	certRes.CertStableURL = order.Certificate
 
-		// Set the default certificate
-		if i == 0 {
-			certRes.IssuerCertificate = issuer
-			certRes.Certificate = cert
-			certRes.CertURL = link
-			certRes.CertStableURL = link
-		}
+	if preferredChain == "" {
+		log.Infof("[%s] Server responded with a certificate.", certRes.Domain)
 
-		if preferredChain == "" {
-			log.Infof("[%s] Server responded with a certificate.", certRes.Domain)
+		return true, nil
+	}
 
-			return true, nil
-		}
-
-		ok, err := hasPreferredChain(issuer, preferredChain)
+	for link, cert := range certs {
+		ok, err := hasPreferredChain(cert.Issuer, preferredChain)
 		if err != nil {
 			return false, err
 		}
@@ -337,8 +333,8 @@ func (c *Certifier) checkResponse(order acme.ExtendedOrder, certRes *Resource, b
 		if ok {
 			log.Infof("[%s] Server responded with a certificate for the preferred certificate chains %q.", certRes.Domain, preferredChain)
 
-			certRes.IssuerCertificate = issuer
-			certRes.Certificate = cert
+			certRes.IssuerCertificate = cert.Issuer
+			certRes.Certificate = cert.Cert
 			certRes.CertURL = link
 			certRes.CertStableURL = link
 

certificate/certificates_test.go

@@ -4,6 +4,7 @@ import (
 	"crypto/rand"
 	"crypto/rsa"
 	"encoding/pem"
+	"fmt"
 	"net/http"
 	"testing"
 
@@ -289,12 +290,15 @@ func Test_checkResponse_alternate(t *testing.T) {
 	defer tearDown()
 
 	mux.HandleFunc("/certificate", func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Add("Link", fmt.Sprintf(`<%s/certificate/1>;title="foo";rel="alternate"`, apiURL))
+
 		_, err := w.Write([]byte(certResponseMock))
 		if err != nil {
 			http.Error(w, err.Error(), http.StatusInternalServerError)
 		}
 	})
-	mux.HandleFunc("/certificate2", func(w http.ResponseWriter, _ *http.Request) {
+
+	mux.HandleFunc("/certificate/1", func(w http.ResponseWriter, _ *http.Request) {
 		_, err := w.Write([]byte(certResponseMock2))
 		if err != nil {
 			http.Error(w, err.Error(), http.StatusInternalServerError)
@@ -314,9 +318,10 @@ func Test_checkResponse_alternate(t *testing.T) {
 			Status:      acme.StatusValid,
 			Certificate: apiURL + "/certificate",
 		},
-		AlternateChainLinks: []string{apiURL + "/certificate2"},
 	}
-	certRes := &Resource{}
+	certRes := &Resource{
+		Domain: "example.com",
+	}
 	bundle := false
 
 	valid, err := certifier.checkResponse(order, certRes, bundle, "DST Root CA X3")
@@ -324,9 +329,9 @@ func Test_checkResponse_alternate(t *testing.T) {
 
 	assert.True(t, valid)
 	assert.NotNil(t, certRes)
-	assert.Equal(t, "", certRes.Domain)
-	assert.Contains(t, certRes.CertStableURL, "/certificate2")
-	assert.Contains(t, certRes.CertURL, "/certificate2")
+	assert.Equal(t, "example.com", certRes.Domain)
+	assert.Contains(t, certRes.CertStableURL, "/certificate/1")
+	assert.Contains(t, certRes.CertURL, "/certificate/1")
 	assert.Nil(t, certRes.CSR)
 	assert.Nil(t, certRes.PrivateKey)
 	assert.Equal(t, certResponseMock2, string(certRes.Certificate), "Certificate")