Skip to content

Commit

Permalink
Add TLS-ALPN-01 Challenge Type to ACME (#20943)
Browse files Browse the repository at this point in the history
* Add ACME TLS-ALPN-01 Challenge validator to PKI

This adds support for verifying the last missing challenge type,
TLS-ALPN-01 challenges, using Go's TLS library. We wish to add this as
many servers (such as Caddy) support transparently renewing certificates
via this protocol, without influencing the contents of sites served.

Signed-off-by: Alexander Scheel <[email protected]>

* Enable suggesting, validating tls-alpn-01 in PKI

Notably, while RFC 8737 is somewhat vague about what identifier types
can be validated with this protocol, it does restrict SANs to be only
DNSSans; from this, we can infer that it is not applicable for IP
typed identifiers. Additionally, since this must resolve to a specific
domain name, we cannot provision it for wildcard identifiers either.

Signed-off-by: Alexander Scheel <[email protected]>

* Fix test expectations to allow ALPN challenges

Signed-off-by: Alexander Scheel <[email protected]>

* Add tls-alpn-01 as a supported challenge to docs

Signed-off-by: Alexander Scheel <[email protected]>

* Add test for tls-alpn-01 challenge verifier

This hacks the challenge engine to allow non-standard (non-443) ports,
letting us use a local server listener with custom implementation.

In addition to the standard test cases, we run:

 - A test with a longer chain (bad),
 - A test without a DNSSan (bad),
 - A test with a bad DNSSan (bad),
 - A test with some other SANs (bad),
 - A test without a CN (good),
 - A test without any leaf (bad), and
 - A test without the extension (bad).

Signed-off-by: Alexander Scheel <[email protected]>

* Add changelog entry

Signed-off-by: Alexander Scheel <[email protected]>

* Update builtin/logical/pki/acme_challenges.go

Co-authored-by: Alexander Scheel <[email protected]>

---------

Signed-off-by: Alexander Scheel <[email protected]>
Co-authored-by: Kit Haines <[email protected]>
  • Loading branch information
cipherboy and kitography authored Jun 7, 2023
1 parent aca58d8 commit f079b7b
Show file tree
Hide file tree
Showing 7 changed files with 799 additions and 14 deletions.
16 changes: 16 additions & 0 deletions builtin/logical/pki/acme_challenge_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,22 @@ func (ace *ACMEChallengeEngine) _verifyChallenge(sc *storageContext, id string,
err = fmt.Errorf("%w: error validating dns-01 challenge %v: %v; %v", ErrIncorrectResponse, id, err, ChallengeAttemptFailedMsg)
return ace._verifyChallengeRetry(sc, cv, authzPath, authz, challenge, err, id)
}
case ACMEALPNChallenge:
if authz.Identifier.Type != ACMEDNSIdentifier {
err = fmt.Errorf("unsupported identifier type for authorization %v/%v in challenge %v: %v", cv.Account, cv.Authorization, id, authz.Identifier.Type)
return ace._verifyChallengeCleanup(sc, err, id)
}

if authz.Wildcard {
err = fmt.Errorf("unable to validate wildcard authorization %v/%v in challenge %v via tls-alpn-01 challenge", cv.Account, cv.Authorization, id)
return ace._verifyChallengeCleanup(sc, err, id)
}

valid, err = ValidateTLSALPN01Challenge(authz.Identifier.Value, cv.Token, cv.Thumbprint, config)
if err != nil {
err = fmt.Errorf("%w: error validating tls-alpn-01 challenge %v: %s", ErrIncorrectResponse, id, err.Error())
return ace._verifyChallengeRetry(sc, cv, authzPath, authz, challenge, err, id)
}
default:
err = fmt.Errorf("unsupported ACME challenge type %v for challenge %v", cv.ChallengeType, id)
return ace._verifyChallengeCleanup(sc, err, id)
Expand Down
295 changes: 287 additions & 8 deletions builtin/logical/pki/acme_challenges.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
package pki

import (
"bytes"
"context"
"crypto/sha256"
"crypto/tls"
"crypto/x509"
"encoding/asn1"
"encoding/base64"
"fmt"
"io"
Expand All @@ -12,7 +16,21 @@ import (
"time"
)

const DNSChallengePrefix = "_acme-challenge."
const (
DNSChallengePrefix = "_acme-challenge."
ALPNProtocol = "acme-tls/1"
)

// While this should be a constant, there's no way to do a low-level test of
// ValidateTLSALPN01Challenge without spinning up a complicated Docker
// instance to build a custom responder. Because we already have a local
// toolchain, it is far easier to drive this through Go tests with a custom
// (high) port, rather than requiring permission to bind to port 443 (root-run
// tests are even worse).
var ALPNPort = "443"

// OID of the acmeIdentifier X.509 Certificate Extension.
var OIDACMEIdentifier = asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 31}

// ValidateKeyAuthorization validates that the given keyAuthz from a challenge
// matches our expectation, returning (true, nil) if so, or (false, err) if
Expand Down Expand Up @@ -67,15 +85,28 @@ func buildResolver(config *acmeConfigEntry) (*net.Resolver, error) {
}, nil
}

func buildDialerConfig(config *acmeConfigEntry) (*net.Dialer, error) {
resolver, err := buildResolver(config)
if err != nil {
return nil, fmt.Errorf("failed to build resolver: %w", err)
}

return &net.Dialer{
Timeout: 10 * time.Second,
KeepAlive: -1 * time.Second,
Resolver: resolver,
}, nil
}

// Validates a given ACME http-01 challenge against the specified domain,
// per RFC 8555.
//
// We attempt to be defensive here against timeouts, extra redirects, &c.
func ValidateHTTP01Challenge(domain string, token string, thumbprint string, config *acmeConfigEntry) (bool, error) {
path := "http://" + domain + "/.well-known/acme-challenge/" + token
resolver, err := buildResolver(config)
dialer, err := buildDialerConfig(config)
if err != nil {
return false, fmt.Errorf("failed to build resolver: %w", err)
return false, fmt.Errorf("failed to build dialer: %w", err)
}

transport := &http.Transport{
Expand All @@ -90,11 +121,7 @@ func ValidateHTTP01Challenge(domain string, token string, thumbprint string, con

// We'd rather timeout and re-attempt validation later than hang
// too many validators waiting for slow hosts.
DialContext: (&net.Dialer{
Timeout: 10 * time.Second,
KeepAlive: -1 * time.Second,
Resolver: resolver,
}).DialContext,
DialContext: dialer.DialContext,
ResponseHeaderTimeout: 10 * time.Second,
}

Expand Down Expand Up @@ -188,3 +215,255 @@ func ValidateDNS01Challenge(domain string, token string, thumbprint string, conf

return false, fmt.Errorf("dns-01: challenge failed against %v records", len(results))
}

func ValidateTLSALPN01Challenge(domain string, token string, thumbprint string, config *acmeConfigEntry) (bool, error) {
// This RFC is defined in RFC 8737 Automated Certificate Management
// Environment (ACME) TLS Application‑Layer Protocol Negotiation
// (ALPN) Challenge Extension.
//
// This is conceptually similar to ValidateHTTP01Challenge, but
// uses a TLS connection on port 443 with the specified ALPN
// protocol.

cfg := &tls.Config{
// Per RFC 8737 Section 3. TLS with Application-Layer Protocol
// Negotiation (TLS ALPN) Challenge, the name of the negotiated
// protocol is "acme-tls/1".
NextProtos: []string{ALPNProtocol},

// Per RFC 8737 Section 3. TLS with Application-Layer Protocol
// Negotiation (TLS ALPN) Challenge:
//
// > ... and an SNI extension containing only the domain name
// > being validated during the TLS handshake.
//
// According to the Go docs, setting this option (even though
// InsecureSkipVerify=true is also specified), allows us to
// set the SNI extension to this value.
ServerName: domain,

VerifyConnection: func(connState tls.ConnectionState) error {
// We initiated a fresh connection with no session tickets;
// even if we did have a session ticket, we do not wish to
// use it. Verify that the server has not inadvertently
// reused connections between validation attempts or something.
if connState.DidResume {
return fmt.Errorf("server under test incorrectly reported that handshake was resumed when no session cache was provided; refusing to continue")
}

// Per RFC 8737 Section 3. TLS with Application-Layer Protocol
// Negotiation (TLS ALPN) Challenge:
//
// > The ACME server verifies that during the TLS handshake the
// > application-layer protocol "acme-tls/1" was successfully
// > negotiated (and that the ALPN extension contained only the
// > value "acme-tls/1").
if connState.NegotiatedProtocol != ALPNProtocol {
return fmt.Errorf("server under test negotiated unexpected ALPN protocol %v", connState.NegotiatedProtocol)
}

// Per RFC 8737 Section 3. TLS with Application-Layer Protocol
// Negotiation (TLS ALPN) Challenge:
//
// > and that the certificate returned
//
// Because this certificate MUST be self-signed (per earlier
// statement in RFC 8737 Section 3), there is no point in sending
// more than one certificate, and so we will err early here if
// we got more than one.
if len(connState.PeerCertificates) > 1 {
return fmt.Errorf("server under test returned multiple (%v) certificates when we expected only one", len(connState.PeerCertificates))
}
cert := connState.PeerCertificates[0]

// Per RFC 8737 Section 3. TLS with Application-Layer Protocol
// Negotiation (TLS ALPN) Challenge:
//
// > The client prepares for validation by constructing a
// > self-signed certificate that MUST contain an acmeIdentifier
// > extension and a subjectAlternativeName extension [RFC5280].
//
// Verify that this is a self-signed certificate that isn't signed
// by another certificate (i.e., with the same key material but
// different issuer).
if err := cert.CheckSignatureFrom(cert); err != nil {
return fmt.Errorf("server under test returned a non-self-signed certificate: %w", err)
}
if !bytes.Equal(cert.RawSubject, cert.RawIssuer) {
return fmt.Errorf("server under test returned a non-self-signed certificate: invalid subject (%v) <-> issuer (%v) match", cert.Subject.String(), cert.Issuer.String())
}

// Per RFC 8737 Section 3. TLS with Application-Layer Protocol
// Negotiation (TLS ALPN) Challenge:
//
// > The subjectAlternativeName extension MUST contain a single
// > dNSName entry where the value is the domain name being
// > validated.
//
// TODO: this does not validate that there are not other SANs
// with unknown (to Go) OIDs.
if len(cert.DNSNames) != 1 || len(cert.EmailAddresses) > 0 || len(cert.IPAddresses) > 0 || len(cert.URIs) > 0 {
return fmt.Errorf("server under test returned a certificate with incorrect SANs")
}

// Per RFC 8737 Section 3. TLS with Application-Layer Protocol
// Negotiation (TLS ALPN) Challenge:
//
// > The comparison of dNSNames MUST be case insensitive
// > [RFC4343]. Note that as ACME doesn't support Unicode
// > identifiers, all dNSNames MUST be encoded using the rules
// > of [RFC3492].
if !strings.EqualFold(cert.DNSNames[0], domain) {
return fmt.Errorf("server under test returned a certificate with unexpected identifier: %v", cert.DNSNames[0])
}

// Per above, verify that the acmeIdentifier extension is present
// exactly once and has the correct value.
var foundACMEId bool
for _, ext := range cert.Extensions {
if !ext.Id.Equal(OIDACMEIdentifier) {
continue
}

// There must be only a single ACME extension.
if foundACMEId {
return fmt.Errorf("server under test returned a certificate with multiple acmeIdentifier extensions")
}
foundACMEId = true

// Per RFC 8737 Section 3. TLS with Application-Layer Protocol
// Negotiation (TLS ALPN) Challenge:
//
// > a critical acmeIdentifier extension
if !ext.Critical {
return fmt.Errorf("server under test returned a certificate with an acmeIdentifier extension marked non-Critical")
}

keyAuthz := string(ext.Value)
ok, err := ValidateSHA256KeyAuthorization(keyAuthz, token, thumbprint)
if !ok || err != nil {
return fmt.Errorf("server under test returned a certificate with an invalid key authorization (%w)", err)
}
}

// Per RFC 8737 Section 3. TLS with Application-Layer Protocol
// Negotiation (TLS ALPN) Challenge:
//
// > The ACME server verifies that ... the certificate returned
// > contains: ... a critical acmeIdentifier extension containing
// > the expected SHA-256 digest computed in step 1.
if !foundACMEId {
return fmt.Errorf("server under test returned a certificate without the required acmeIdentifier extension")
}

// Remove the handled critical extension and validate that we
// have no additional critical extensions left unhandled.
var index int = -1
for oidIndex, oid := range cert.UnhandledCriticalExtensions {
if oid.Equal(OIDACMEIdentifier) {
index = oidIndex
break
}
}
if index != -1 {
// Unlike the foundACMEId case, this is not a failure; if Go
// updates to "understand" this critical extension, we do not
// wish to fail.
cert.UnhandledCriticalExtensions = append(cert.UnhandledCriticalExtensions[0:index], cert.UnhandledCriticalExtensions[index+1:]...)
}
if len(cert.UnhandledCriticalExtensions) > 0 {
return fmt.Errorf("server under test returned a certificate with additional unknown critical extensions (%v)", cert.UnhandledCriticalExtensions)
}

// All good!
return nil
},

// We never want to resume a connection; do not provide session
// cache storage.
ClientSessionCache: nil,

// Do not trust any system trusted certificates; we're going to be
// manually validating the chain, so specifying a non-empty pool
// here could only cause additional, unnecessary work.
RootCAs: x509.NewCertPool(),

// Do not bother validating the client's chain; we know it should be
// self-signed. This also disables hostname verification, but we do
// this verification as part of VerifyConnection(...) ourselves.
//
// Per Go docs, this option is only safe in conjunction with
// VerifyConnection which we define above.
InsecureSkipVerify: true,

// RFC 8737 Section 4. acme-tls/1 Protocol Definition:
//
// > ACME servers that implement "acme-tls/1" MUST only negotiate
// > TLS 1.2 [RFC5246] or higher when connecting to clients for
// > validation.
MinVersion: tls.VersionTLS12,

// While RFC 8737 does not place restrictions around allowed cipher
// suites, we wish to restrict ourselves to secure defaults. Specify
// the Intermediate guideline from Mozilla's TLS config generator to
// disable obviously weak ciphers.
//
// See also: https://ssl-config.mozilla.org/#server=go&version=1.14.4&config=intermediate&guideline=5.7
CipherSuites: []uint16{
tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,
tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,
},
}

// Build a dialer using our custom DNS resolver, to ensure domains get
// resolved according to configuration.
dialer, err := buildDialerConfig(config)
if err != nil {
return false, fmt.Errorf("failed to build dialer: %w", err)
}

// Per RFC 8737 Section 3. TLS with Application-Layer Protocol
// Negotiation (TLS ALPN) Challenge:
//
// > 2. The ACME server resolves the domain name being validated and
// > chooses one of the IP addresses returned for validation (the
// > server MAY validate against multiple addresses if more than
// > one is returned).
// > 3. The ACME server initiates a TLS connection to the chosen IP
// > address. This connection MUST use TCP port 443.
address := fmt.Sprintf("%v:"+ALPNPort, domain)
conn, err := dialer.Dial("tcp", address)
if err != nil {
return false, fmt.Errorf("tls-alpn-01: failed to dial host: %w", err)
}

// Initiate the connection to the remote peer.
client := tls.Client(conn, cfg)

// We intentionally swallow this error as it isn't useful to the
// underlying protocol we perform here. Notably, per RFC 8737
// Section 4. acme-tls/1 Protocol Definition:
//
// > Once the handshake is completed, the client MUST NOT exchange
// > any further data with the server and MUST immediately close the
// > connection. ... Because of this, an ACME server MAY choose to
// > withhold authorization if either the certificate signature is
// > invalid or the handshake doesn't fully complete.
defer client.Close()

// We wish to put time bounds on the total time the handshake can
// stall for, so build a connection context here.
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()

// See note above about why we can allow Handshake to complete
// successfully.
if err := client.HandshakeContext(ctx); err != nil {
return false, fmt.Errorf("tls-alpn-01: failed to perform handshake: %w", err)
}
return true, nil
}
Loading

0 comments on commit f079b7b

Please sign in to comment.