Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: preferred chain support. #1298

Merged
merged 1 commit into from
Nov 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 59 additions & 26 deletions acme/api/certificate.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,31 +20,38 @@ 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
}

// 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 cert.Cert, cert.Issuer, nil
}

issuer, err = c.getIssuerFromLink(up)
// 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 {
// 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)
} else if len(issuer) > 0 {
// If bundle is true, we want to return a certificate bundle.
// To do this, we append the issuer cert to the issued cert.
if bundle {
cert = append(cert, issuer...)
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 cert, issuer, nil
return certs, nil
}

// Revoke Revokes a certificate.
Expand All @@ -54,27 +61,53 @@ func (c *CertificateService) Revoke(req acme.RevokeCertMessage) error {
}

// get Returns the certificate and the "up" link.
func (c *CertificateService) get(certURL string) ([]byte, string, error) {
func (c *CertificateService) get(certURL string, bundle bool) (*acme.RawCertificate, http.Header, error) {
if len(certURL) == 0 {
return nil, "", errors.New("certificate[get]: empty URL")
return nil, nil, errors.New("certificate[get]: empty URL")
}

resp, err := c.core.postAsGet(certURL, nil)
if err != nil {
return nil, "", err
return nil, nil, err
}

cert, err := ioutil.ReadAll(http.MaxBytesReader(nil, resp.Body, maxBodySize))
data, err := ioutil.ReadAll(http.MaxBytesReader(nil, resp.Body, maxBodySize))
if err != nil {
return nil, "", err
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 &acme.RawCertificate{Cert: cert, Issuer: issuer}
}

// 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")
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)
} else if len(issuer) > 0 {
// If bundle is true, we want to return a certificate bundle.
// To do this, we append the issuer cert to the issued cert.
if bundle {
cert = append(cert, issuer...)
}
}

return cert, up, err
return &acme.RawCertificate{Cert: cert, Issuer: issuer}
}

// getIssuerFromLink requests the issuer certificate.
Expand All @@ -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
}
19 changes: 6 additions & 13 deletions acme/api/order.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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.
Expand All @@ -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
}
Expand All @@ -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
}
10 changes: 0 additions & 10 deletions acme/api/order_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
}
Expand Down
12 changes: 7 additions & 5 deletions acme/commons.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
38 changes: 17 additions & 21 deletions certificate/certificates.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,38 +307,34 @@ func (c *Certifier) checkResponse(order acme.ExtendedOrder, certRes *Resource, b
return valid, err
}

links := append([]string{order.Certificate}, order.AlternateChainLinks...)

for i, link := range links {
cert, issuer, err := c.core.Certificates.Get(link, bundle)
if err != nil {
return false, err
}
certs, err := c.core.Certificates.GetAll(order.Certificate, bundle)
if err != nil {
return false, err
}

// Set the default certificate
if i == 0 {
certRes.IssuerCertificate = issuer
certRes.Certificate = cert
certRes.CertURL = link
certRes.CertStableURL = link
}
// Set the default certificate
certRes.IssuerCertificate = certs[order.Certificate].Issuer
certRes.Certificate = certs[order.Certificate].Cert
certRes.CertURL = order.Certificate
certRes.CertStableURL = order.Certificate

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
}

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

Expand Down
17 changes: 11 additions & 6 deletions certificate/certificates_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"crypto/rand"
"crypto/rsa"
"encoding/pem"
"fmt"
"net/http"
"testing"

Expand Down Expand Up @@ -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)
Expand All @@ -314,19 +318,20 @@ 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")
require.NoError(t, err)

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")
Expand Down