Skip to content

Commit

Permalink
add acme-preferred-chain config key
Browse files Browse the repository at this point in the history
Since the deprecation of DST X3 root CA, which used to sign Let's
Encrypt root CA, a few issues raised and can be summarized as:

* If the topmost certificate of the provided chain is issued by
  `DST X3`, clients that has `DST X3` on their trusted CAs bundle, care
  about the expiration of their CAs, and a somewhat old openssl, will
  fail to trust in the Let's Encrypt chain even if they trust in the
  Let's Encrypt's `ISRG Root X1`. Clients should update openssl or
  remove `DST X3` from their trusted CAs. This is the default chain
  provided by Let's Encrypt;
* If the topmost certificate is issed by `ISRG Root X1`, which is Let's
  Encrypt's root CA, old clients will fail to trust Let's Encrypt
  certificate, mostly Android older than 7.1.1.

Let's Encrypt production API adds alternative chains that can be chosen
by the Common Name of its topmost certificate. This is the purpose of
this configuration key, so sys admins can choose the chain that will
have the lesser impact on their users.

Acme client was updated in a way that a mistyped preferred chain doesn't
fail the emission of the certificate, which avoids to being blocked by
the acme server due to the amount of new orders.

As a first implementation that's being planned to be merged to v0.13,
it wasn't added the ability to identify that a preferred chain was
changed. Such change would need to change even more code, making this
even less secure to be merged to a stable version.

Should be merged to v0.13 so users can benefit from this alternative as
soon as possible.
  • Loading branch information
jcmoraisjr committed Oct 17, 2021
1 parent 7937952 commit 71295f2
Show file tree
Hide file tree
Showing 13 changed files with 297 additions and 95 deletions.
19 changes: 11 additions & 8 deletions docs/content/en/docs/configuration/keys.md
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ The table below describes all supported configuration keys.
| [`acme-emails`](#acme) | email1,email2,... | Global | |
| [`acme-endpoint`](#acme) | [`v2-staging`\|`v2`\|`endpoint`] | Global | |
| [`acme-expiring`](#acme) | number of days | Global | `30` |
| [`acme-preferred-chain`](#acme) | CN (Common Name) of the issuer | Host | |
| [`acme-shared`](#acme) | [true\|false] | Global | `false` |
| [`acme-terms-agreed`](#acme) | [true\|false] | Global | `false` |
| [`affinity`](#affinity) | affinity type | Backend | |
Expand Down Expand Up @@ -500,14 +501,15 @@ The table below describes all supported configuration keys.

## Acme

| Configuration key | Scope | Default | Since |
|---------------------|----------|---------|-------|
| `acme-emails` | `Global` | | v0.9 |
| `acme-endpoint` | `Global` | | v0.9 |
| `acme-expiring` | `Global` | `30` | v0.9 |
| `acme-shared` | `Global` | `false` | v0.9 |
| `acme-terms-agreed` | `Global` | `false` | v0.9 |
| `cert-signer` | `Host` | | v0.9 |
| Configuration key | Scope | Default | Since |
|------------------------|----------|---------|---------|
| `acme-emails` | `Global` | | v0.9 |
| `acme-endpoint` | `Global` | | v0.9 |
| `acme-expiring` | `Global` | `30` | v0.9 |
| `acme-preferred-chain` | `Host` | | v0.13.5 |
| `acme-shared` | `Global` | `false` | v0.9 |
| `acme-terms-agreed` | `Global` | `false` | v0.9 |
| `cert-signer` | `Host` | | v0.9 |

Configures dynamic options used to authorize and sign certificates against a server
which implements the acme protocol, version 2.
Expand All @@ -520,6 +522,7 @@ Supported acme configuration keys:
* `acme-emails`: mandatory, a comma-separated list of emails used to configure the client account. The account will be updated if this option is changed.
* `acme-endpoint`: mandatory, endpoint of the acme environment. `v2-staging` and `v02-staging` are alias to `https://acme-staging-v02.api.letsencrypt.org`, while `v2` and `v02` are alias to `https://acme-v02.api.letsencrypt.org`.
* `acme-expiring`: how many days before expiring a certificate should be considered old and should be updated. Defaults to `30` days.
* `acme-preferred-chain`: optional, defines the Issuer's CN (Common Name) of the topmost certificate in the chain, if the acme server offers multiple certificate chains. The default certificate chain will be used if empty or no match is found. Note that changing this option will not force a new certificate to be issued if a valid one is already in place and actual and preferred chains differ. A new certificate can be emitted by changing the secret name in the ingress resource, or removing the secret being referenced.
* `acme-shared`: defines if another certificate signer is running in the cluster. If `false`, the default value, any request to `/.well-known/acme-challenge/` is sent to the local acme server despite any ingress object configuration. Otherwise, if `true`, a configured ingress object would take precedence.
* `acme-terms-agreed`: mandatory, it should be defined as `true`, otherwise certificates won't be issued.
* `cert-signer`: defines the certificate signer that should be used to authorize and sign new certificates. The only supported value is `"acme"`. Add this config as an annotation in the ingress object that should have its certificate managed by haproxy-ingress and signed by the configured acme environment. The annotation `kubernetes.io/tls-acme: "true"` is also supported if the command-line option `--acme-track-tls-annotation` is used.
Expand Down
14 changes: 7 additions & 7 deletions pkg/acme/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ type ClientResolver interface {

// Client ...
type Client interface {
Sign(dnsnames []string) (crt, key []byte, err error)
Sign(dnsnames []string, preferredChain string) (crt, key []byte, err error)
}

type client struct {
Expand Down Expand Up @@ -128,7 +128,7 @@ func (c *client) ensureAccount() error {
return nil
}

func (c *client) Sign(dnsnames []string) (crt, key []byte, err error) {
func (c *client) Sign(dnsnames []string, preferredChain string) (crt, key []byte, err error) {
if len(dnsnames) == 0 {
return crt, key, fmt.Errorf("dnsnames is empty")
}
Expand All @@ -142,7 +142,7 @@ func (c *client) Sign(dnsnames []string) (crt, key []byte, err error) {
csrTemplate := &x509.CertificateRequest{}
csrTemplate.Subject.CommonName = dnsnames[0]
csrTemplate.DNSNames = dnsnames
return c.signRequest(order, csrTemplate)
return c.signRequest(order, csrTemplate, preferredChain)
}

func (c *client) authorize(dnsnames []string, order *acme.Order) error {
Expand Down Expand Up @@ -180,7 +180,7 @@ func (c *client) authorize(dnsnames []string, order *acme.Order) error {
return nil
}

func (c *client) signRequest(order *acme.Order, csrTemplate *x509.CertificateRequest) (crt, key []byte, err error) {
func (c *client) signRequest(order *acme.Order, csrTemplate *x509.CertificateRequest, preferredChain string) (crt, key []byte, err error) {
keys, err := rsa.GenerateKey(rand.Reader, 2048)
if err != nil {
return crt, key, err
Expand All @@ -189,8 +189,8 @@ func (c *client) signRequest(order *acme.Order, csrTemplate *x509.CertificateReq
if err != nil {
return crt, key, err
}
rawCerts, err := c.client.FinalizeOrder(c.ctx, order.FinalizeURL, csr)
if err != nil {
rawCerts, err := c.client.FinalizeOrder(c.ctx, order.FinalizeURL, csr, preferredChain)
if err != nil && rawCerts == nil {
return crt, key, err
}
key = pem.EncodeToMemory(&pem.Block{
Expand All @@ -203,5 +203,5 @@ func (c *client) signRequest(order *acme.Order, csrTemplate *x509.CertificateReq
Bytes: rawCert,
})...)
}
return crt, key, nil
return crt, key, err
}
29 changes: 26 additions & 3 deletions pkg/acme/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"encoding/base64"
"fmt"
"io/ioutil"
"os"
"testing"
"time"

Expand All @@ -36,9 +37,21 @@ const (
// Optional, nothing will be done if any value is missing.
//
// DO NOT COMMIT+PUSH THE CLIENT KEY!
//
// single line base64 encoded client's private key in DER format
// $ openssl genrsa |openssl rsa -outform der |base64 -w0 >privkey ## omit -w0 on macOS/Darwin/BSD
clientkey = ``
email = ``
domain = ``
// email that should be assigned to the account
email = ``
// domain used to execute the challenge
domain = ``
// an optional preferred chain - note that currently (oct/2021) Let's Encrypt
// staging doesn't have an alternate chain
chain = ``
// a local path where the response of the challenge should be writted
// if empty the challenge will be written to /tmp/out and the test will
// wait 20s to continue
wwwpublic = ``
)

func TestSign(t *testing.T) {
Expand All @@ -58,7 +71,7 @@ func TestSign(t *testing.T) {
}
// TODO test resulting crt
// TODO debug/fine logging in the Sign() steps
_, _, err = client.Sign([]string{domain})
_, _, err = client.Sign([]string{domain}, chain)
if err != nil {
t.Errorf("error signing certificate: %v", err)
}
Expand All @@ -78,6 +91,16 @@ func (c *clientResolver) GetKey() (crypto.Signer, error) {
}

func (c *clientResolver) SetToken(domain string, uri, token string) error {
if wwwpublic != "" {
file := wwwpublic + uri
if token == "" {
return os.Remove(file)
}
if err := os.MkdirAll(wwwpublic+"/.well-known/acme-challenge", 0755); err != nil {
return err
}
return ioutil.WriteFile(file, []byte(token), 0644)
}
if token == "" {
return nil
}
Expand Down
18 changes: 11 additions & 7 deletions pkg/acme/signer.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,13 @@ func (s *signer) Notify(item interface{}) error {
}
cert := strings.Split(item.(string), ",")
secretName := cert[0]
domains := cert[1:]
err := s.verify(secretName, domains)
preferredChain := cert[1]
domains := cert[2:]
err := s.verify(secretName, preferredChain, domains)
return err
}

func (s *signer) verify(secretName string, domains []string) (verifyErr error) {
func (s *signer) verify(secretName, preferredChain string, domains []string) (verifyErr error) {
duedate := time.Now().Add(s.expiring)
tls, errSecret := s.cache.GetTLSSecretContent(secretName)
strdomains := strings.Join(domains, ",")
Expand All @@ -139,11 +140,14 @@ func (s *signer) verify(secretName string, domains []string) (verifyErr error) {
s.verifyCount++
s.logger.Info("acme: authorizing: id=%d secret=%s domain(s)=%s endpoint=%s reason='%s'",
s.verifyCount, secretName, strdomains, s.account.Endpoint, reason)
crt, key, err := s.client.Sign(domains)
if err == nil {
crt, key, err := s.client.Sign(domains, preferredChain)
if crt != nil && key != nil {
if err != nil {
s.logger.Warn("warning from client: %v", err)
}
if errTLS := s.cache.SetTLSSecretContent(secretName, crt, key); errTLS == nil {
s.logger.Info("acme: new certificate issued: id=%d secret=%s domain(s)=%s",
s.verifyCount, secretName, strdomains)
s.logger.Info("acme: new certificate issued: id=%d secret=%s domain(s)=%s preferred-chain=%s",
s.verifyCount, secretName, strdomains, preferredChain)
} else {
s.logger.Warn("acme: error storing new certificate: id=%d secret=%s domain(s)=%s error=%v",
s.verifyCount, secretName, strdomains, errTLS)
Expand Down
38 changes: 19 additions & 19 deletions pkg/acme/signer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,58 +34,58 @@ func TestNotifyVerify(t *testing.T) {
testCases := []struct {
input string
expiresIn time.Duration
cert string
cert string
logging string
}{
// 0
{
input: "s1,d1.local",
input: "s1,,d1.local",
expiresIn: 10 * 24 * time.Hour,
cert: dumbcrt,
cert: dumbcrt,
logging: `
INFO-V(2) acme: skipping sign, certificate is updated: secret=s1 domain(s)=d1.local`,
},
// 1
{
input: "s1,d2.local",
input: "s1,,d2.local",
expiresIn: -10 * 24 * time.Hour,
cert: dumbcrt,
cert: dumbcrt,
logging: `
INFO acme: authorizing: id=1 secret=s1 domain(s)=d2.local endpoint=https://acme-v2.local reason='certificate expires in 2020-12-01 16:33:14 +0000 UTC'
INFO acme: new certificate issued: id=1 secret=s1 domain(s)=d2.local`,
INFO acme: new certificate issued: id=1 secret=s1 domain(s)=d2.local preferred-chain=`,
},
// 2
{
input: "s1,d3.local",
input: "s1,,d3.local",
expiresIn: 10 * 24 * time.Hour,
cert: dumbcrt,
cert: dumbcrt,
logging: `
INFO acme: authorizing: id=1 secret=s1 domain(s)=d3.local endpoint=https://acme-v2.local reason='added one or more domains to an existing certificate'
INFO acme: new certificate issued: id=1 secret=s1 domain(s)=d3.local`,
INFO acme: new certificate issued: id=1 secret=s1 domain(s)=d3.local preferred-chain=`,
},
// 3
{
input: "s2,d1.local",
input: "s2,,d1.local",
expiresIn: 10 * 24 * time.Hour,
cert: dumbcrt,
cert: dumbcrt,
logging: `
INFO acme: authorizing: id=1 secret=s2 domain(s)=d1.local endpoint=https://acme-v2.local reason='certificate does not exist (secret not found: s2)'
INFO acme: new certificate issued: id=1 secret=s2 domain(s)=d1.local`,
INFO acme: new certificate issued: id=1 secret=s2 domain(s)=d1.local preferred-chain=`,
},
{
input: "s1,s3.dev.local",
input: "s1,,s3.dev.local",
expiresIn: 10 * 24 * time.Hour,
cert: dumbwildcardcrt,
cert: dumbwildcardcrt,
logging: `
INFO-V(2) acme: skipping sign, certificate is updated: secret=s1 domain(s)=s3.dev.local`,
},
{
input: "s1,other.s3.dev.local",
input: "s1,,other.s3.dev.local",
expiresIn: 10 * 24 * time.Hour,
cert: dumbwildcardcrt,
cert: dumbwildcardcrt,
logging: `
INFO acme: authorizing: id=1 secret=s1 domain(s)=other.s3.dev.local endpoint=https://acme-v2.local reason='added one or more domains to an existing certificate'
INFO acme: new certificate issued: id=1 secret=s1 domain(s)=other.s3.dev.local`,
INFO acme: new certificate issued: id=1 secret=s1 domain(s)=other.s3.dev.local preferred-chain=`,
},
}
c := setup(t)
Expand Down Expand Up @@ -132,8 +132,8 @@ func (c *config) newSigner() *signer {

type clientMock struct{}

func (c *clientMock) Sign(domains []string) (crt, key []byte, err error) {
return nil, nil, nil
func (c *clientMock) Sign(domains []string, preferredChain string) (crt, key []byte, err error) {
return []byte("fake-crt"), []byte("fake-key"), nil
}

type cache struct {
Expand Down
59 changes: 49 additions & 10 deletions pkg/acme/x/acme/acme.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"crypto"
"crypto/rand"
"crypto/sha256"
"crypto/x509"
"encoding/base64"
"encoding/json"
"encoding/pem"
Expand All @@ -31,6 +32,7 @@ import (
"math/big"
"net/http"
"net/url"
"regexp"
"strconv"
"sync"
"time"
Expand Down Expand Up @@ -212,7 +214,11 @@ func (c *Client) CreateOrder(ctx context.Context, order *Order) (*Order, error)
//
// Callers are encouraged to parse the returned certificate chain to ensure it
// is valid and has the expected attributes.
func (c *Client) FinalizeOrder(ctx context.Context, finalizeURL string, csr []byte) (der [][]byte, err error) {
//
// altcn, if not empty, sbould have the Issuer's CN of the topmost certificate of
// the chain, if the acme server offers multiple certificate chains. If a match
// isn't found, an error will be returned along with the default certificate.
func (c *Client) FinalizeOrder(ctx context.Context, finalizeURL string, csr []byte, altcn string) (der [][]byte, err error) {
if _, err := c.Discover(ctx); err != nil {
return nil, err
}
Expand Down Expand Up @@ -249,8 +255,32 @@ func (c *Client) FinalizeOrder(ctx context.Context, finalizeURL string, csr []by
if o.Status != StatusValid {
return nil, fmt.Errorf("acme: unexpected order status %q", o.Status)
}
der, altURLs, err := c.getCertAndAlternates(ctx, o.CertificateURL)
if altcn == "" || err != nil || matchCN(der, altcn) {
return der, err
}
defaultDER := der
for _, altURL := range altURLs {
der, _, err = c.getCertAndAlternates(ctx, altURL)
if err != nil {
return defaultDER, err
}
if matchCN(der, altcn) {
return der, err
}
}
return defaultDER, fmt.Errorf("acme: alternate chain not found for Common Name '%s', using default chain", altcn)
}

return c.getCert(ctx, o.CertificateURL)
func matchCN(der [][]byte, cn string) bool {
if len(der) == 0 {
return false
}
crt, err := x509.ParseCertificate(der[len(der)-1])
if err != nil {
return false
}
return crt.Issuer.CommonName == cn
}

// GetOrder retrieves an order identified by url.
Expand Down Expand Up @@ -821,38 +851,47 @@ func nonceFromHeader(h http.Header) string {
return h.Get("Replay-Nonce")
}

func (c *Client) getCert(ctx context.Context, url string) ([][]byte, error) {
var linkRegex = regexp.MustCompile(`<(.*)>;rel="alternate"`)

func (c *Client) getCertAndAlternates(ctx context.Context, url string) ([][]byte, []string, error) {
res, err := c.postWithJWSAccount(ctx, url, nil)
if err != nil {
return nil, err
return nil, nil, err
}
defer res.Body.Close()
data, err := ioutil.ReadAll(io.LimitReader(res.Body, maxChainSize+1))
if err != nil {
return nil, fmt.Errorf("acme: error getting certificate: %v", err)
return nil, nil, fmt.Errorf("acme: error getting certificate: %v", err)
}
if len(data) > maxChainSize {
return nil, errors.New("acme: certificate chain is too big")
return nil, nil, errors.New("acme: certificate chain is too big")
}
var chain [][]byte
for {
var p *pem.Block
p, data = pem.Decode(data)
if p == nil {
if len(chain) == 0 {
return nil, errors.New("acme: invalid PEM certificate chain")
return nil, nil, errors.New("acme: invalid PEM certificate chain")
}
break
}
if len(chain) == maxChainLen {
return nil, errors.New("acme: certificate chain is too long")
return nil, nil, errors.New("acme: certificate chain is too long")
}
if p.Type != "CERTIFICATE" {
return nil, fmt.Errorf("acme: invalid PEM block type %q", p.Type)
return nil, nil, fmt.Errorf("acme: invalid PEM block type %q", p.Type)
}
chain = append(chain, p.Bytes)
}
return chain, nil
var alts []string
for _, alt := range res.Header["Link"] {
links := linkRegex.FindStringSubmatch(alt)
if len(links) > 0 {
alts = append(alts, links[1])
}
}
return chain, alts, nil
}

// responseError creates an error of Error type from resp.
Expand Down
Loading

0 comments on commit 71295f2

Please sign in to comment.