Skip to content

Commit

Permalink
VA: Add metrics to measure key exchange cipher suites (#7322)
Browse files Browse the repository at this point in the history
Add a new field to the structured JSON object logged by the VA
indicating whether the HTTP-01 or TLS-ALPN-01 requests ended up
negotiating a TLS cipher suite which uses RSA key exchange. This is
useful for measuring how many servers we reach out to are RSA-only, so
we can determine the deprecation timeline for RSA key exchange (which
has been removed from go1.22).

The go TLS library always prefers ECDHE key exchange over RSA, so we
should only be negotiating RSA key exchange if the server we're reaching
out to doesn't support ECDHE at all.

Part of #7321
  • Loading branch information
aarongable authored Feb 14, 2024
1 parent b5932f0 commit b483ec2
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 25 deletions.
7 changes: 7 additions & 0 deletions core/objects.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,13 @@ type ValidationRecord struct {
// lookup for AddressUsed. During recursive A and AAAA lookups, a record may
// instead look like A:host:port or AAAA:host:port
ResolverAddrs []string `json:"resolverAddrs,omitempty"`
// UsedRSAKEX is a *temporary* addition to the validation record, so we can
// see how many servers that we reach out to during HTTP-01 and TLS-ALPN-01
// validation are only willing to negotiate RSA key exchange mechanisms. The
// field is not included in the serialized json to avoid cluttering the
// database and log lines.
// TODO(#7321): Remove this when we have collected sufficient data.
UsedRSAKEX bool `json:"-"`
}

func looksLikeKeyAuthorization(str string) error {
Expand Down
15 changes: 15 additions & 0 deletions va/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,13 @@ func (va *ValidationAuthorityImpl) processHTTPValidation(
numRedirects++
va.metrics.http01Redirects.Inc()

// If TLS was used, record the negotiated key exchange mechanism in the most
// recent validationRecord.
// TODO(#7321): Remove this when we have collected enough data.
if req.Response.TLS != nil {
records[len(records)-1].UsedRSAKEX = usedRSAKEX(req.Response.TLS.CipherSuite)
}

if req.Response.TLS != nil && req.Response.TLS.Version < tls.VersionTLS12 {
return berrors.ConnectionFailureError(
"validation attempt was redirected to an HTTPS server that doesn't " +
Expand Down Expand Up @@ -648,6 +655,14 @@ func (va *ValidationAuthorityImpl) processHTTPValidation(
return nil, records, newIPError(target, berrors.UnauthorizedError("Invalid response from %s: %q",
records[len(records)-1].URL, body))
}

// We were successful, so record the negotiated key exchange mechanism in the
// last validationRecord.
// TODO(#7321): Remove this when we have collected enough data.
if httpResponse.TLS != nil {
records[len(records)-1].UsedRSAKEX = usedRSAKEX(httpResponse.TLS.CipherSuite)
}

return body, records, nil
}

Expand Down
55 changes: 30 additions & 25 deletions va/tlsalpn.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,65 +58,62 @@ func certAltNames(cert *x509.Certificate) []string {

func (va *ValidationAuthorityImpl) tryGetChallengeCert(ctx context.Context,
identifier identifier.ACMEIdentifier, challenge core.Challenge,
tlsConfig *tls.Config) (*x509.Certificate, *tls.ConnectionState, []core.ValidationRecord, error) {
tlsConfig *tls.Config) (*x509.Certificate, *tls.ConnectionState, core.ValidationRecord, error) {

allAddrs, resolvers, err := va.getAddrs(ctx, identifier.Value)
validationRecords := []core.ValidationRecord{
{
Hostname: identifier.Value,
AddressesResolved: allAddrs,
Port: strconv.Itoa(va.tlsPort),
ResolverAddrs: resolvers,
},
validationRecord := core.ValidationRecord{
Hostname: identifier.Value,
AddressesResolved: allAddrs,
Port: strconv.Itoa(va.tlsPort),
ResolverAddrs: resolvers,
}
if err != nil {
return nil, nil, validationRecords, err
return nil, nil, validationRecord, err
}
thisRecord := &validationRecords[0]

// Split the available addresses into v4 and v6 addresses
v4, v6 := availableAddresses(allAddrs)
addresses := append(v4, v6...)

// This shouldn't happen, but be defensive about it anyway
if len(addresses) < 1 {
return nil, nil, validationRecords, berrors.MalformedError("no IP addresses found for %q", identifier.Value)
return nil, nil, validationRecord, berrors.MalformedError("no IP addresses found for %q", identifier.Value)
}

// If there is at least one IPv6 address then try it first
if len(v6) > 0 {
address := net.JoinHostPort(v6[0].String(), thisRecord.Port)
thisRecord.AddressUsed = v6[0]
address := net.JoinHostPort(v6[0].String(), validationRecord.Port)
validationRecord.AddressUsed = v6[0]

cert, cs, err := va.getChallengeCert(ctx, address, identifier, challenge, tlsConfig)

// If there is no problem, return immediately
if err == nil {
return cert, cs, validationRecords, nil
return cert, cs, validationRecord, nil
}

// Otherwise, we note that we tried an address and fall back to trying IPv4
thisRecord.AddressesTried = append(thisRecord.AddressesTried, thisRecord.AddressUsed)
validationRecord.AddressesTried = append(validationRecord.AddressesTried, validationRecord.AddressUsed)
va.metrics.ipv4FallbackCounter.Inc()
}

// If there are no IPv4 addresses and we tried an IPv6 address return
// an error - there's nothing left to try
if len(v4) == 0 && len(thisRecord.AddressesTried) > 0 {
return nil, nil, validationRecords, berrors.MalformedError("Unable to contact %q at %q, no IPv4 addresses to try as fallback",
thisRecord.Hostname, thisRecord.AddressesTried[0])
} else if len(v4) == 0 && len(thisRecord.AddressesTried) == 0 {
if len(v4) == 0 && len(validationRecord.AddressesTried) > 0 {
return nil, nil, validationRecord, berrors.MalformedError("Unable to contact %q at %q, no IPv4 addresses to try as fallback",
validationRecord.Hostname, validationRecord.AddressesTried[0])
} else if len(v4) == 0 && len(validationRecord.AddressesTried) == 0 {
// It shouldn't be possible that there are no IPv4 addresses and no previous
// attempts at an IPv6 address connection but be defensive about it anyway
return nil, nil, validationRecords, berrors.MalformedError("No IP addresses found for %q", thisRecord.Hostname)
return nil, nil, validationRecord, berrors.MalformedError("No IP addresses found for %q", validationRecord.Hostname)
}

// Otherwise if there are no IPv6 addresses, or there was an error
// talking to the first IPv6 address, try the first IPv4 address
thisRecord.AddressUsed = v4[0]
cert, cs, err := va.getChallengeCert(ctx, net.JoinHostPort(v4[0].String(), thisRecord.Port),
validationRecord.AddressUsed = v4[0]
cert, cs, err := va.getChallengeCert(ctx, net.JoinHostPort(v4[0].String(), validationRecord.Port),
identifier, challenge, tlsConfig)
return cert, cs, validationRecords, err
return cert, cs, validationRecord, err
}

func (va *ValidationAuthorityImpl) getChallengeCert(
Expand Down Expand Up @@ -213,11 +210,15 @@ func (va *ValidationAuthorityImpl) validateTLSALPN01(ctx context.Context, identi
return nil, berrors.MalformedError("Identifier type for TLS-ALPN-01 was not DNS")
}

cert, cs, validationRecords, problem := va.tryGetChallengeCert(ctx, identifier, challenge, &tls.Config{
cert, cs, tvr, problem := va.tryGetChallengeCert(ctx, identifier, challenge, &tls.Config{
MinVersion: tls.VersionTLS12,
NextProtos: []string{ACMETLS1Protocol},
ServerName: identifier.Value,
})
// Copy the single validationRecord into the slice that we have to return, and
// get a reference to it so we can modify it if we have to.
validationRecords := []core.ValidationRecord{tvr}
validationRecord := &validationRecords[0]
if problem != nil {
return validationRecords, problem
}
Expand All @@ -230,7 +231,7 @@ func (va *ValidationAuthorityImpl) validateTLSALPN01(ctx context.Context, identi
}

badCertErr := func(msg string) error {
hostPort := net.JoinHostPort(validationRecords[0].AddressUsed.String(), validationRecords[0].Port)
hostPort := net.JoinHostPort(validationRecord.AddressUsed.String(), validationRecord.Port)

return berrors.UnauthorizedError(
"Incorrect validation certificate for %s challenge. "+
Expand Down Expand Up @@ -287,6 +288,10 @@ func (va *ValidationAuthorityImpl) validateTLSALPN01(ctx context.Context, identi
hex.EncodeToString(h[:]),
))
}
// We were successful, so record the negotiated key exchange mechanism in
// the validationRecord.
// TODO(#7321): Remove this when we have collected enough data.
validationRecord.UsedRSAKEX = usedRSAKEX(cs.CipherSuite)
return validationRecords, nil
}
}
Expand Down
17 changes: 17 additions & 0 deletions va/va.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,7 @@ type verificationRequestEvent struct {
Hostname string `json:",omitempty"`
Challenge core.Challenge `json:",omitempty"`
ValidationLatency float64
UsedRSAKEX bool `json:",omitempty"`
Error string `json:",omitempty"`
}

Expand Down Expand Up @@ -756,6 +757,15 @@ func (va *ValidationAuthorityImpl) PerformValidation(ctx context.Context, req *v
validationLatency := time.Since(vStart)
logEvent.ValidationLatency = validationLatency.Round(time.Millisecond).Seconds()

// Copy the "UsedRSAKEX" value from the last validationRecord into the log
// event. Only the last record should have this bool set, because we only
// record it if/when validation is finally successful, but we use the loop
// just in case that assumption changes.
// TODO(#7321): Remove this when we have collected enough data.
for _, record := range records {
logEvent.UsedRSAKEX = record.UsedRSAKEX || logEvent.UsedRSAKEX
}

va.metrics.localValidationTime.With(prometheus.Labels{
"type": string(challenge.Type),
"result": string(challenge.Status),
Expand All @@ -774,3 +784,10 @@ func (va *ValidationAuthorityImpl) PerformValidation(ctx context.Context, req *v
prob = filterProblemDetails(prob)
return bgrpc.ValidationResultToPB(records, prob)
}

// usedRSAKEX returns true if the given cipher suite involves the use of an
// RSA key exchange mechanism.
// TODO(#7321): Remove this when we have collected enough data.
func usedRSAKEX(cs uint16) bool {
return strings.HasPrefix(tls.CipherSuiteName(cs), "TLS_RSA_")
}

0 comments on commit b483ec2

Please sign in to comment.