Skip to content

Commit 2fd24b1

Browse files
hc-github-team-secure-vault-coreschultz-is
hc-github-team-secure-vault-core
and
schultz-is
authored
backport of commit 8cc7be2 (#21293)
Co-authored-by: Matt Schultz <975680+schultz-is@users.noreply.github.com>
1 parent 9e85fef commit 2fd24b1

File tree

8 files changed

+442
-23
lines changed

8 files changed

+442
-23
lines changed

‎builtin/logical/pki/acme_challenges.go

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"bytes"
55
"context"
66
"crypto/sha256"
7+
"crypto/subtle"
78
"crypto/tls"
89
"crypto/x509"
910
"encoding/asn1"
@@ -55,7 +56,7 @@ func ValidateKeyAuthorization(keyAuthz string, token string, thumbprint string)
5556
// challenge matches our expectation, returning (true, nil) if so, or
5657
// (false, err) if not.
5758
//
58-
// This is for use with DNS challenges, which require
59+
// This is for use with DNS challenges, which require base64 encoding.
5960
func ValidateSHA256KeyAuthorization(keyAuthz string, token string, thumbprint string) (bool, error) {
6061
authzContents := token + "." + thumbprint
6162
checksum := sha256.Sum256([]byte(authzContents))
@@ -68,6 +69,22 @@ func ValidateSHA256KeyAuthorization(keyAuthz string, token string, thumbprint st
6869
return true, nil
6970
}
7071

72+
// ValidateRawSHA256KeyAuthorization validates that the given keyAuthz from a
73+
// challenge matches our expectation, returning (true, nil) if so, or
74+
// (false, err) if not.
75+
//
76+
// This is for use with TLS challenges, which require the raw hash output.
77+
func ValidateRawSHA256KeyAuthorization(keyAuthz []byte, token string, thumbprint string) (bool, error) {
78+
authzContents := token + "." + thumbprint
79+
expectedAuthz := sha256.Sum256([]byte(authzContents))
80+
81+
if len(keyAuthz) != len(expectedAuthz) || subtle.ConstantTimeCompare(expectedAuthz[:], keyAuthz) != 1 {
82+
return false, fmt.Errorf("sha256 key authorization was invalid")
83+
}
84+
85+
return true, nil
86+
}
87+
7188
func buildResolver(config *acmeConfigEntry) (*net.Resolver, error) {
7289
if len(config.DNSResolver) == 0 {
7390
return net.DefaultResolver, nil
@@ -286,9 +303,13 @@ func ValidateTLSALPN01Challenge(domain string, token string, thumbprint string,
286303
// Verify that this is a self-signed certificate that isn't signed
287304
// by another certificate (i.e., with the same key material but
288305
// different issuer).
289-
if err := cert.CheckSignatureFrom(cert); err != nil {
290-
return fmt.Errorf("server under test returned a non-self-signed certificate: %w", err)
306+
// NOTE: Do not use cert.CheckSignatureFrom(cert) as we need to bypass the
307+
// checks for the parent certificate having the IsCA basic constraint set.
308+
err := cert.CheckSignature(cert.SignatureAlgorithm, cert.RawTBSCertificate, cert.Signature)
309+
if err != nil {
310+
return fmt.Errorf("server under test returned a non-self-signed certificate: %v", err)
291311
}
312+
292313
if !bytes.Equal(cert.RawSubject, cert.RawIssuer) {
293314
return fmt.Errorf("server under test returned a non-self-signed certificate: invalid subject (%v) <-> issuer (%v) match", cert.Subject.String(), cert.Issuer.String())
294315
}
@@ -339,8 +360,16 @@ func ValidateTLSALPN01Challenge(domain string, token string, thumbprint string,
339360
return fmt.Errorf("server under test returned a certificate with an acmeIdentifier extension marked non-Critical")
340361
}
341362

342-
keyAuthz := string(ext.Value)
343-
ok, err := ValidateSHA256KeyAuthorization(keyAuthz, token, thumbprint)
363+
var keyAuthz []byte
364+
remainder, err := asn1.Unmarshal(ext.Value, &keyAuthz)
365+
if err != nil {
366+
return fmt.Errorf("server under test returned a certificate with invalid acmeIdentifier extension value: %w", err)
367+
}
368+
if len(remainder) > 0 {
369+
return fmt.Errorf("server under test returned a certificate with invalid acmeIdentifier extension value with additional trailing data")
370+
}
371+
372+
ok, err := ValidateRawSHA256KeyAuthorization(keyAuthz, token, thumbprint)
344373
if !ok || err != nil {
345374
return fmt.Errorf("server under test returned a certificate with an invalid key authorization (%w)", err)
346375
}

‎builtin/logical/pki/acme_challenges_test.go

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"crypto/tls"
1111
"crypto/x509"
1212
"crypto/x509/pkix"
13+
"encoding/asn1"
1314
"encoding/base64"
1415
"fmt"
1516
"math/big"
@@ -308,7 +309,8 @@ func TestAcmeValidateTLSALPN01Challenge(t *testing.T) {
308309
t.Logf("using keyAuthorizationTestCase [tc=%d] as alpnTestCase [tc=%d]...", index, len(alpnTestCases))
309310
// Properly encode the authorization.
310311
checksum := sha256.Sum256([]byte(tc.keyAuthz))
311-
authz := base64.RawURLEncoding.EncodeToString(checksum[:])
312+
authz, err := asn1.Marshal(checksum[:])
313+
require.NoError(t, err, "failed asn.1 marshalling authz")
312314

313315
// Build a self-signed certificate.
314316
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
@@ -329,11 +331,11 @@ func TestAcmeValidateTLSALPN01Challenge(t *testing.T) {
329331
{
330332
Id: OIDACMEIdentifier,
331333
Critical: true,
332-
Value: []byte(authz),
334+
Value: authz,
333335
},
334336
},
335337
BasicConstraintsValid: true,
336-
IsCA: true,
338+
IsCA: false,
337339
}
338340
certBytes, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, key.Public(), key)
339341
require.NoError(t, err, "failed to create certificate")
@@ -378,7 +380,8 @@ func TestAcmeValidateTLSALPN01Challenge(t *testing.T) {
378380

379381
// Compute our authorization.
380382
checksum := sha256.Sum256([]byte("valid.valid"))
381-
authz := base64.RawURLEncoding.EncodeToString(checksum[:])
383+
authz, err := asn1.Marshal(checksum[:])
384+
require.NoError(t, err, "failed to marshal authz with asn.1 ")
382385

383386
// Build a leaf certificate which _could_ pass validation
384387
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
@@ -399,11 +402,11 @@ func TestAcmeValidateTLSALPN01Challenge(t *testing.T) {
399402
{
400403
Id: OIDACMEIdentifier,
401404
Critical: true,
402-
Value: []byte(authz),
405+
Value: authz,
403406
},
404407
},
405408
BasicConstraintsValid: true,
406-
IsCA: true,
409+
IsCA: false,
407410
}
408411
certBytes, err := x509.CreateCertificate(rand.Reader, tmpl, rootCert, key.Public(), rootKey)
409412
require.NoError(t, err, "failed to create leaf certificate")
@@ -426,7 +429,8 @@ func TestAcmeValidateTLSALPN01Challenge(t *testing.T) {
426429
// Test case: cert without DNSSan
427430
// Compute our authorization.
428431
checksum := sha256.Sum256([]byte("valid.valid"))
429-
authz := base64.RawURLEncoding.EncodeToString(checksum[:])
432+
authz, err := asn1.Marshal(checksum[:])
433+
require.NoError(t, err, "failed to marshal authz with asn.1 ")
430434

431435
// Build a leaf certificate without a DNSSan
432436
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
@@ -447,11 +451,11 @@ func TestAcmeValidateTLSALPN01Challenge(t *testing.T) {
447451
{
448452
Id: OIDACMEIdentifier,
449453
Critical: true,
450-
Value: []byte(authz),
454+
Value: authz,
451455
},
452456
},
453457
BasicConstraintsValid: true,
454-
IsCA: true,
458+
IsCA: false,
455459
}
456460
certBytes, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, key.Public(), key)
457461
require.NoError(t, err, "failed to create leaf certificate")
@@ -474,7 +478,8 @@ func TestAcmeValidateTLSALPN01Challenge(t *testing.T) {
474478
// Test case: cert without matching DNSSan
475479
// Compute our authorization.
476480
checksum := sha256.Sum256([]byte("valid.valid"))
477-
authz := base64.RawURLEncoding.EncodeToString(checksum[:])
481+
authz, err := asn1.Marshal(checksum[:])
482+
require.NoError(t, err, "failed to marshal authz with asn.1 ")
478483

479484
// Build a leaf certificate which fails validation due to bad DNSName
480485
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
@@ -495,11 +500,11 @@ func TestAcmeValidateTLSALPN01Challenge(t *testing.T) {
495500
{
496501
Id: OIDACMEIdentifier,
497502
Critical: true,
498-
Value: []byte(authz),
503+
Value: authz,
499504
},
500505
},
501506
BasicConstraintsValid: true,
502-
IsCA: true,
507+
IsCA: false,
503508
}
504509
certBytes, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, key.Public(), key)
505510
require.NoError(t, err, "failed to create leaf certificate")
@@ -522,7 +527,8 @@ func TestAcmeValidateTLSALPN01Challenge(t *testing.T) {
522527
// Test case: cert with additional SAN
523528
// Compute our authorization.
524529
checksum := sha256.Sum256([]byte("valid.valid"))
525-
authz := base64.RawURLEncoding.EncodeToString(checksum[:])
530+
authz, err := asn1.Marshal(checksum[:])
531+
require.NoError(t, err, "failed to marshal authz with asn.1 ")
526532

527533
// Build a leaf certificate which has an invalid additional SAN
528534
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
@@ -544,11 +550,11 @@ func TestAcmeValidateTLSALPN01Challenge(t *testing.T) {
544550
{
545551
Id: OIDACMEIdentifier,
546552
Critical: true,
547-
Value: []byte(authz),
553+
Value: authz,
548554
},
549555
},
550556
BasicConstraintsValid: true,
551-
IsCA: true,
557+
IsCA: false,
552558
}
553559
certBytes, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, key.Public(), key)
554560
require.NoError(t, err, "failed to create leaf certificate")
@@ -571,7 +577,8 @@ func TestAcmeValidateTLSALPN01Challenge(t *testing.T) {
571577
// Test case: cert without CN
572578
// Compute our authorization.
573579
checksum := sha256.Sum256([]byte("valid.valid"))
574-
authz := base64.RawURLEncoding.EncodeToString(checksum[:])
580+
authz, err := asn1.Marshal(checksum[:])
581+
require.NoError(t, err, "failed to marshal authz with asn.1 ")
575582

576583
// Build a leaf certificate which should pass validation
577584
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
@@ -588,11 +595,11 @@ func TestAcmeValidateTLSALPN01Challenge(t *testing.T) {
588595
{
589596
Id: OIDACMEIdentifier,
590597
Critical: true,
591-
Value: []byte(authz),
598+
Value: authz,
592599
},
593600
},
594601
BasicConstraintsValid: true,
595-
IsCA: true,
602+
IsCA: false,
596603
}
597604
certBytes, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, key.Public(), key)
598605
require.NoError(t, err, "failed to create leaf certificate")

0 commit comments

Comments
 (0)