letsencrypt
/
boulder
mirror of https://github.com/letsencrypt/boulder.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Improve how we disable challenge types (#7677) 

When creating an authorization, populate it with all challenges
appropriate for that identifier, regardless of whether those challenge
types are currently "enabled" in the config. This ensures that
authorizations created during a incident for which we can temporarily
disabled a single challenge type can still be validated via that
challenge type after the incident is over.

Also, when finalizing an order, check that the challenge type used to
validation each authorization is not currently disabled. This ensures
that, if we temporarily disable a single challenge due to an incident,
we don't issue any more certificates using authorizations which were
fulfilled using that disabled challenge.

Note that standard rolling deployment of this change is not safe if any
challenges are disabled at the same time, due to the possibility of an
updated RA not filtering a challenge when writing it to the database,
and then a non-updated RA not filtering it when reading from the
database. But if all challenges are enabled then this change is safe for
normal deploy.

Fixes https://github.com/letsencrypt/boulder/issues/5913
This commit is contained in:
Aaron Gable 2024-08-29 15:38:50 -07:00 committed by GitHub
parent ea62f9a802
commit d58d09615a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 221 additions and 88 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
core/interfaces.go
View File

@ -10,5 +10,5 @@ type PolicyAuthority interface {
WillingToIssue([]string) error
ChallengeTypesFor(identifier.ACMEIdentifier) ([]AcmeChallenge, error)
ChallengeTypeEnabled(AcmeChallenge) bool
CheckAuthz(*Authorization) error
CheckAuthzChallenges(*Authorization) error
}

4
core/objects.go
View File

@ -337,14 +337,14 @@ func (authz *Authorization) FindChallengeByStringID(id string) int {
// challenge is valid.
func (authz *Authorization) SolvedBy() (AcmeChallenge, error) {
if len(authz.Challenges) == 0 {
return "", fmt.Errorf("Authorization has no challenges")
return "", fmt.Errorf("authorization has no challenges")
}
for _, chal := range authz.Challenges {
if chal.Status == StatusValid {
return chal.Type, nil
}
}
return "", fmt.Errorf("Authorization not solved by any challenge")
return "", fmt.Errorf("authorization not solved by any challenge")
}
// JSONBuffer fields get encoded and decoded JOSE-style, in base64url encoding

4
core/objects_test.go
View File

@ -100,7 +100,7 @@ func TestAuthorizationSolvedBy(t *testing.T) {
{
Name: "No challenges",
Authz: Authorization{},
ExpectedError: "Authorization has no challenges",
ExpectedError: "authorization has no challenges",
},
// An authz with all non-valid challenges should return nil
{
@ -108,7 +108,7 @@ func TestAuthorizationSolvedBy(t *testing.T) {
Authz: Authorization{
Challenges: []Challenge{HTTPChallenge01(""), DNSChallenge01("")},
},
ExpectedError: "Authorization not solved by any challenge",
ExpectedError: "authorization not solved by any challenge",
},
// An authz with one valid HTTP01 challenge amongst other challenges should
// return the HTTP01 challenge

2
csr/csr_test.go
View File

@ -39,7 +39,7 @@ func (pa *mockPA) ChallengeTypeEnabled(t core.AcmeChallenge) bool {
return true
}
func (pa *mockPA) CheckAuthz(a *core.Authorization) error {
func (pa *mockPA) CheckAuthzChallenges(a *core.Authorization) error {
return nil
}

60
policy/pa.go
View File

@ -517,38 +517,31 @@ func (pa *AuthorityImpl) checkHostLists(domain string) error {
}
// ChallengeTypesFor determines which challenge types are acceptable for the
// given identifier.
func (pa *AuthorityImpl) ChallengeTypesFor(identifier identifier.ACMEIdentifier) ([]core.AcmeChallenge, error) {
// If the identifier is for a DNS wildcard name we only
// provide a DNS-01 challenge as a matter of CA policy.
if strings.HasPrefix(identifier.Value, "*.") {
// We must have the DNS-01 challenge type enabled to create challenges for
// a wildcard identifier per LE policy.
if !pa.ChallengeTypeEnabled(core.ChallengeTypeDNS01) {
return nil, fmt.Errorf(
"Challenges requested for wildcard identifier but DNS-01 " +
"challenge type is not enabled")
}
// Only provide a DNS-01-Wildcard challenge
// given identifier. This determination is made purely based on the identifier,
// and not based on which challenge types are enabled, so that challenge type
// filtering can happen dynamically at request rather than being set in stone
// at creation time.
func (pa *AuthorityImpl) ChallengeTypesFor(ident identifier.ACMEIdentifier) ([]core.AcmeChallenge, error) {
// If the identifier is for a DNS wildcard name we only provide a DNS-01
// challenge, to comply with the BRs Sections 3.2.2.4.19 and 3.2.2.4.20
// stating that ACME HTTP-01 and TLS-ALPN-01 are not suitable for validating
// Wildcard Domains.
if ident.Type == identifier.DNS && strings.HasPrefix(ident.Value, "*.") {
return []core.AcmeChallenge{core.ChallengeTypeDNS01}, nil
}
// Otherwise we collect up challenges based on what is enabled.
var challenges []core.AcmeChallenge
if pa.ChallengeTypeEnabled(core.ChallengeTypeHTTP01) {
challenges = append(challenges, core.ChallengeTypeHTTP01)
// Return all challenge types we support for non-wildcard DNS identifiers.
if ident.Type == identifier.DNS {
return []core.AcmeChallenge{
core.ChallengeTypeHTTP01,
core.ChallengeTypeDNS01,
core.ChallengeTypeTLSALPN01,
}, nil
}
if pa.ChallengeTypeEnabled(core.ChallengeTypeTLSALPN01) {
challenges = append(challenges, core.ChallengeTypeTLSALPN01)
}
if pa.ChallengeTypeEnabled(core.ChallengeTypeDNS01) {
challenges = append(challenges, core.ChallengeTypeDNS01)
}
return challenges, nil
// Otherwise return an error because we don't support any challenges for this
// identifier type.
return nil, fmt.Errorf("unrecognized identifier type %q", ident.Type)
}
// ChallengeTypeEnabled returns whether the specified challenge type is enabled
@ -558,21 +551,26 @@ func (pa *AuthorityImpl) ChallengeTypeEnabled(t core.AcmeChallenge) bool {
return pa.enabledChallenges[t]
}
// CheckAuthz determines that an authorization was fulfilled by a challenge
// that was appropriate for the kind of identifier in the authorization.
func (pa *AuthorityImpl) CheckAuthz(authz *core.Authorization) error {
// CheckAuthzChallenges determines that an authorization was fulfilled by a
// challenge that is currently enabled and was appropriate for the kind of
// identifier in the authorization.
func (pa *AuthorityImpl) CheckAuthzChallenges(authz *core.Authorization) error {
chall, err := authz.SolvedBy()
if err != nil {
return err
}
if !pa.ChallengeTypeEnabled(chall) {
return errors.New("authorization fulfilled by disabled challenge type")
}
challTypes, err := pa.ChallengeTypesFor(authz.Identifier)
if err != nil {
return err
}
if !slices.Contains(challTypes, chall) {
return errors.New("authorization fulfilled by invalid challenge")
return errors.New("authorization fulfilled by inapplicable challenge type")
}
return nil

174
policy/pa_test.go
View File

@ -12,16 +12,16 @@ import (
"github.com/letsencrypt/boulder/features"
"github.com/letsencrypt/boulder/identifier"
blog "github.com/letsencrypt/boulder/log"
"github.com/letsencrypt/boulder/must"
"github.com/letsencrypt/boulder/test"
)
var enabledChallenges = map[core.AcmeChallenge]bool{
core.ChallengeTypeHTTP01: true,
core.ChallengeTypeDNS01: true,
}
func paImpl(t *testing.T) *AuthorityImpl {
enabledChallenges := map[core.AcmeChallenge]bool{
core.ChallengeTypeHTTP01: true,
core.ChallengeTypeDNS01: true,
core.ChallengeTypeTLSALPN01: true,
}
pa, err := New(enabledChallenges, blog.NewMock())
if err != nil {
t.Fatalf("Couldn't create policy implementation: %s", err)
@ -388,52 +388,52 @@ func TestWillingToIssue_SubErrors(t *testing.T) {
}
func TestChallengeTypesFor(t *testing.T) {
t.Parallel()
pa := paImpl(t)
challenges, err := pa.ChallengeTypesFor(identifier.ACMEIdentifier{})
test.AssertNotError(t, err, "ChallengesFor failed")
test.Assert(t, len(challenges) == len(enabledChallenges), "Wrong number of challenges returned")
seenChalls := make(map[core.AcmeChallenge]bool)
for _, challenge := range challenges {
test.Assert(t, !seenChalls[challenge], "should not already have seen this type")
seenChalls[challenge] = true
test.Assert(t, enabledChallenges[challenge], "Unsupported challenge returned")
}
test.AssertEquals(t, len(seenChalls), len(enabledChallenges))
}
func TestChallengeTypesForWildcard(t *testing.T) {
// wildcardIdent is an identifier for a wildcard domain name
wildcardIdent := identifier.ACMEIdentifier{
Type: identifier.DNS,
Value: "*.zombo.com",
testCases := []struct {
name string
ident identifier.ACMEIdentifier
wantChalls []core.AcmeChallenge
wantErr string
}{
{
name: "dns",
ident: identifier.DNSIdentifier("example.com"),
wantChalls: []core.AcmeChallenge{
core.ChallengeTypeHTTP01, core.ChallengeTypeDNS01, core.ChallengeTypeTLSALPN01,
},
},
{
name: "wildcard",
ident: identifier.DNSIdentifier("*.example.com"),
wantChalls: []core.AcmeChallenge{
core.ChallengeTypeDNS01,
},
},
{
name: "other",
ident: identifier.ACMEIdentifier{Type: "ip", Value: "1.2.3.4"},
wantErr: "unrecognized identifier type",
},
}
// First try to get a challenge for the wildcard ident without the
// DNS-01 challenge type enabled. This should produce an error
var enabledChallenges = map[core.AcmeChallenge]bool{
core.ChallengeTypeHTTP01: true,
core.ChallengeTypeDNS01: false,
}
pa := must.Do(New(enabledChallenges, blog.NewMock()))
_, err := pa.ChallengeTypesFor(wildcardIdent)
test.AssertError(t, err, "ChallengesFor did not error for a wildcard ident "+
"when DNS-01 was disabled")
test.AssertEquals(t, err.Error(), "Challenges requested for wildcard "+
"identifier but DNS-01 challenge type is not enabled")
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
challs, err := pa.ChallengeTypesFor(tc.ident)
// Try again with DNS-01 enabled. It should not error and
// should return only one DNS-01 type challenge
enabledChallenges[core.ChallengeTypeDNS01] = true
pa = must.Do(New(enabledChallenges, blog.NewMock()))
challenges, err := pa.ChallengeTypesFor(wildcardIdent)
test.AssertNotError(t, err, "ChallengesFor errored for a wildcard ident "+
"unexpectedly")
test.AssertEquals(t, len(challenges), 1)
test.AssertEquals(t, challenges[0], core.ChallengeTypeDNS01)
if len(tc.wantChalls) != 0 {
test.AssertNotError(t, err, "should have succeeded")
test.AssertDeepEquals(t, challs, tc.wantChalls)
}
if tc.wantErr != "" {
test.AssertError(t, err, "should have errored")
test.AssertContains(t, err.Error(), tc.wantErr)
}
})
}
}
// TestMalformedExactBlocklist tests that loading a YAML policy file with an
@ -483,3 +483,83 @@ func TestValidEmailError(t *testing.T) {
err = ValidEmail("example@-foobar.com")
test.AssertEquals(t, err.Error(), "contact email \"example@-foobar.com\" has invalid domain : Domain name contains an invalid character")
}
func TestCheckAuthzChallenges(t *testing.T) {
t.Parallel()
testCases := []struct {
name string
authz core.Authorization
enabled map[core.AcmeChallenge]bool
wantErr string
}{
{
name: "unrecognized identifier",
authz: core.Authorization{
Identifier: identifier.ACMEIdentifier{Type: "oops", Value: "example.com"},
Challenges: []core.Challenge{{Type: core.ChallengeTypeDNS01, Status: core.StatusValid}},
},
wantErr: "unrecognized identifier type",
},
{
name: "no challenges",
authz: core.Authorization{
Identifier: identifier.ACMEIdentifier{Type: identifier.DNS, Value: "example.com"},
Challenges: []core.Challenge{},
},
wantErr: "has no challenges",
},
{
name: "no valid challenges",
authz: core.Authorization{
Identifier: identifier.ACMEIdentifier{Type: identifier.DNS, Value: "example.com"},
Challenges: []core.Challenge{{Type: core.ChallengeTypeDNS01, Status: core.StatusPending}},
},
wantErr: "not solved by any challenge",
},
{
name: "solved by disabled challenge",
authz: core.Authorization{
Identifier: identifier.ACMEIdentifier{Type: identifier.DNS, Value: "example.com"},
Challenges: []core.Challenge{{Type: core.ChallengeTypeDNS01, Status: core.StatusValid}},
},
enabled: map[core.AcmeChallenge]bool{core.ChallengeTypeHTTP01: true},
wantErr: "disabled challenge type",
},
{
name: "solved by wrong kind of challenge",
authz: core.Authorization{
Identifier: identifier.ACMEIdentifier{Type: identifier.DNS, Value: "*.example.com"},
Challenges: []core.Challenge{{Type: core.ChallengeTypeHTTP01, Status: core.StatusValid}},
},
wantErr: "inapplicable challenge type",
},
{
name: "valid authz",
authz: core.Authorization{
Identifier: identifier.ACMEIdentifier{Type: identifier.DNS, Value: "example.com"},
Challenges: []core.Challenge{{Type: core.ChallengeTypeTLSALPN01, Status: core.StatusValid}},
},
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
pa := paImpl(t)
if tc.enabled != nil {
pa.enabledChallenges = tc.enabled
}
err := pa.CheckAuthzChallenges(&tc.authz)
if tc.wantErr == "" {
test.AssertNotError(t, err, "should have succeeded")
} else {
test.AssertError(t, err, "should have errored")
test.AssertContains(t, err.Error(), tc.wantErr)
}
})
}
}

2
ra/ra.go
View File

@ -826,7 +826,7 @@ func (ra *RegistrationAuthorityImpl) checkOrderAuthorizations(
expired = append(expired, ident.Value)
continue
}
err = ra.PA.CheckAuthz(authz)
err = ra.PA.CheckAuthzChallenges(authz)
if err != nil {
invalid = append(invalid, ident.Value)
continue

61
ra/ra_test.go
View File

@ -2516,7 +2516,7 @@ func TestNewOrderWildcard(t *testing.T) {
test.AssertEquals(t, authz.Challenges[0].Type, core.ChallengeTypeDNS01)
case "example.com":
// If the authz is for example.com, we expect it has normal challenges
test.AssertEquals(t, len(authz.Challenges), 2)
test.AssertEquals(t, len(authz.Challenges), 3)
default:
t.Fatalf("Received an authorization for a name not requested: %q", name)
}
@ -2556,7 +2556,7 @@ func TestNewOrderWildcard(t *testing.T) {
case "zombo.com":
// We expect that the base domain identifier auth has the normal number of
// challenges
test.AssertEquals(t, len(authz.Challenges), 2)
test.AssertEquals(t, len(authz.Challenges), 3)
case "*.zombo.com":
// We expect that the wildcard identifier auth has only a pending
// DNS-01 type challenge
@ -2590,7 +2590,7 @@ func TestNewOrderWildcard(t *testing.T) {
// We expect the authz is for the identifier the correct domain
test.AssertEquals(t, authz.Identifier.Value, "everything.is.possible.zombo.com")
// We expect the authz has the normal # of challenges
test.AssertEquals(t, len(authz.Challenges), 2)
test.AssertEquals(t, len(authz.Challenges), 3)
// Now submit an order request for a wildcard of the domain we just created an
// order for. We should **NOT** reuse the authorization from the previous
@ -3153,6 +3153,61 @@ func TestFinalizeOrderWildcard(t *testing.T) {
"wildcard order")
}
func TestFinalizeOrderDisabledChallenge(t *testing.T) {
_, sa, ra, fc, cleanUp := initAuthorities(t)
defer cleanUp()
// Create a random domain
var bytes [3]byte
_, err := rand.Read(bytes[:])
test.AssertNotError(t, err, "creating test domain name")
domain := fmt.Sprintf("%x.example.com", bytes[:])
// Create a finalized authorization for that domain
authzID := createFinalizedAuthorization(
t, sa, domain, fc.Now().Add(24*time.Hour), core.ChallengeTypeHTTP01, fc.Now().Add(-1*time.Hour))
// Create an order that reuses that authorization
order, err := ra.NewOrder(context.Background(), &rapb.NewOrderRequest{
RegistrationID: Registration.Id,
DnsNames: []string{domain},
})
test.AssertNotError(t, err, "creating test order")
test.AssertEquals(t, order.V2Authorizations[0], authzID)
// Create a CSR for this order
testKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
test.AssertNotError(t, err, "generating test key")
csr, err := x509.CreateCertificateRequest(rand.Reader, &x509.CertificateRequest{
PublicKey: testKey.PublicKey,
DNSNames: []string{domain},
}, testKey)
test.AssertNotError(t, err, "Error creating policy forbid CSR")
// Replace the Policy Authority with one which has this challenge type disabled
pa, err := policy.New(map[core.AcmeChallenge]bool{
core.ChallengeTypeDNS01: true,
core.ChallengeTypeTLSALPN01: true,
}, ra.log)
test.AssertNotError(t, err, "creating test PA")
err = pa.LoadHostnamePolicyFile("../test/hostname-policy.yaml")
test.AssertNotError(t, err, "loading test hostname policy")
ra.PA = pa
// Now finalizing this order should fail
_, err = ra.FinalizeOrder(context.Background(), &rapb.FinalizeOrderRequest{
Order: order,
Csr: csr,
})
test.AssertError(t, err, "finalization should fail")
// Unfortunately we can't test for the PA's "which is now disabled" error
// message directly, because the RA discards it and collects all invalid names
// into a single more generic error message. But it does at least distinguish
// between missing, expired, and invalid, so we can test for "invalid".
test.AssertContains(t, err.Error(), "authorizations for these identifiers not valid")
}
func TestIssueCertificateAuditLog(t *testing.T) {
_, sa, ra, _, cleanUp := initAuthorities(t)
defer cleanUp()