Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add ACME "dns-account-01" challenge #7387

Closed
wants to merge 26 commits into from
Closed
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
d341dc5
Add DNS-ACCOUNT-01 support.
sheurich Jan 8, 2024
1c0653f
Initial tests for DNS-ACCOUNT-01.
sheurich Mar 14, 2024
72c4b09
Upgrade eggsampler/acme/v3 to v3.5.0 for DNS-ACCOUNT-01 support
sheurich Mar 15, 2024
bcec8c4
test(integration): add TestDNSAccountChallenge
sheurich Mar 15, 2024
4489f43
fix(validateDNSAccount01): reject unsupported scopes
sheurich Mar 16, 2024
14ea785
test(dns-account-01): additional unit tests
sheurich Mar 16, 2024
22d2210
test(pa): add dns-account-01 to wildcard challenges
sheurich Mar 16, 2024
0718a30
test(dns-account-01): add to core and ra unit tests
sheurich Mar 16, 2024
ece427a
feat(load-generator): add DNS-ACCOUNT-01 challenge strategy
sheurich Mar 16, 2024
e0fce1b
feat(ra): Allow DNS-ACCOUNT-01 authorizations to be reused for wildcards
sheurich Mar 16, 2024
eb7c094
rename
sheurich Mar 16, 2024
a948e01
fix(TestDNSAccountChallenge): only run test in config-next
sheurich Mar 16, 2024
9e5e388
Merge branch 'main' into add-dns-account-01
sheurich Mar 18, 2024
492ce0f
clean up challenge policy logic
sheurich Mar 18, 2024
4ea3634
fix(test): add dns-01+dns-account-01 wildcard policy test
sheurich Mar 18, 2024
b2f90f7
doc(model): explain challTypeToUint and uintToChallType
sheurich Mar 19, 2024
ac488a5
bundle authz `Scope` into core.Challenge
sheurich Mar 19, 2024
dcc624e
Merge branch 'main' into add-dns-account-01
sheurich Mar 19, 2024
4f7a8a1
Merge branch 'main' into add-dns-account-01
sheurich Mar 19, 2024
18d89dc
Merge branch 'main' into add-dns-account-01
sheurich Mar 22, 2024
a6f87c4
- Add AccountURL and Scope to core.Authorization and protobufs
sheurich Mar 20, 2024
ab01878
lint
sheurich Mar 22, 2024
528f6de
fix(akamai-purger): deep copy batch of purge entries
sheurich Mar 22, 2024
67cfe74
remove AccountURL from Authorization; pass using Challenge instead
sheurich Mar 27, 2024
d2fe3e0
fix: use jws protected header for KeyID
sheurich Mar 27, 2024
c941420
fix: use challenge pointer for setting AccountURL in WebFrontEndImpl.…
sheurich Mar 27, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 13 additions & 15 deletions bdns/mocks.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,35 +19,33 @@ type MockClient struct {

// LookupTXT is a mock
func (mock *MockClient) LookupTXT(_ context.Context, hostname string) ([]string, ResolverAddrs, error) {
if hostname == "_acme-challenge.servfail.com" {
switch hostname {
case "_acme-challenge.servfail.com":
return nil, ResolverAddrs{"MockClient"}, fmt.Errorf("SERVFAIL")
}
if hostname == "_acme-challenge.good-dns01.com" {
case "_acme-challenge.good-dns01.com",
"_djny7rmeuvxuhb2v._acme-host-challenge.good-dns01.com":
// base64(sha256("LoqXcYV8q5ONbJQxbmR7SCTNo3tiAXDfowyjxAjEuX0"
// + "." + "9jg46WB3rR_AHD-EBXdN7cBkH1WOu0tA3M9fm21mqTI"))
// expected token + test account jwk thumbprint
return []string{"LPsIwTo7o8BoG0-vjCyGQGBWSVIPxI-i_X336eUOQZo"}, ResolverAddrs{"MockClient"}, nil
}
if hostname == "_acme-challenge.wrong-dns01.com" {
case "_acme-challenge.wrong-dns01.com",
"_rpmyw7okyyoycegj._acme-host-challenge.wrong-dns01.com":
return []string{"a"}, ResolverAddrs{"MockClient"}, nil
}
if hostname == "_acme-challenge.wrong-many-dns01.com" {
case "_acme-challenge.wrong-many-dns01.com":
return []string{"a", "b", "c", "d", "e"}, ResolverAddrs{"MockClient"}, nil
}
if hostname == "_acme-challenge.long-dns01.com" {
case "_acme-challenge.long-dns01.com":
return []string{"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}, ResolverAddrs{"MockClient"}, nil
}
if hostname == "_acme-challenge.no-authority-dns01.com" {
case "_acme-challenge.no-authority-dns01.com":
// base64(sha256("LoqXcYV8q5ONbJQxbmR7SCTNo3tiAXDfowyjxAjEuX0"
// + "." + "9jg46WB3rR_AHD-EBXdN7cBkH1WOu0tA3M9fm21mqTI"))
// expected token + test account jwk thumbprint
return []string{"LPsIwTo7o8BoG0-vjCyGQGBWSVIPxI-i_X336eUOQZo"}, ResolverAddrs{"MockClient"}, nil
}
// empty-txts.com always returns zero TXT records
if hostname == "_acme-challenge.empty-txts.com" {
case "_acme-challenge.empty-txts.com",
"_6g727n6x5dk6qex5._acme-host-challenge.empty-txts.com":
return []string{}, ResolverAddrs{"MockClient"}, nil
default:
return []string{"hostname"}, ResolverAddrs{"MockClient"}, nil
}
return []string{"hostname"}, ResolverAddrs{"MockClient"}, nil
}

// makeTimeoutError returns a a net.OpError for which Timeout() returns true.
Expand Down
2 changes: 1 addition & 1 deletion cmd/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ type SMTPConfig struct {
// it should offer.
type PAConfig struct {
DBConfig `validate:"-"`
Challenges map[core.AcmeChallenge]bool `validate:"omitempty,dive,keys,oneof=http-01 dns-01 tls-alpn-01,endkeys"`
Challenges map[core.AcmeChallenge]bool `validate:"omitempty,dive,keys,oneof=http-01 dns-01 dns-account-01 tls-alpn-01,endkeys"`
}

// CheckChallenges checks whether the list of challenges in the PA config
Expand Down
7 changes: 7 additions & 0 deletions core/challenges.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ func DNSChallenge01(token string) Challenge {
return newChallenge(ChallengeTypeDNS01, token)
}

// DNSAccountChallenge01 constructs a dns-account-01 challenge.
func DNSAccountChallenge01(token string) Challenge {
return newChallenge(ChallengeTypeDNSAccount01, token)
}

// TLSALPNChallenge01 constructs a tls-alpn-01 challenge.
func TLSALPNChallenge01(token string) Challenge {
return newChallenge(ChallengeTypeTLSALPN01, token)
Expand All @@ -33,6 +38,8 @@ func NewChallenge(kind AcmeChallenge, token string) (Challenge, error) {
return HTTPChallenge01(token), nil
case ChallengeTypeDNS01:
return DNSChallenge01(token), nil
case ChallengeTypeDNSAccount01:
return DNSAccountChallenge01(token), nil
case ChallengeTypeTLSALPN01:
return TLSALPNChallenge01(token), nil
default:
Expand Down
1 change: 1 addition & 0 deletions core/core_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ func TestChallenges(t *testing.T) {

test.Assert(t, ChallengeTypeHTTP01.IsValid(), "Refused valid challenge")
test.Assert(t, ChallengeTypeDNS01.IsValid(), "Refused valid challenge")
test.Assert(t, ChallengeTypeDNSAccount01.IsValid(), "Refused valid challenge")
test.Assert(t, ChallengeTypeTLSALPN01.IsValid(), "Refused valid challenge")
test.Assert(t, !AcmeChallenge("nonsense-71").IsValid(), "Accepted invalid challenge")
}
Expand Down
22 changes: 16 additions & 6 deletions core/objects.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,21 +53,31 @@ type AcmeChallenge string

// These types are the available challenges
const (
ChallengeTypeHTTP01 = AcmeChallenge("http-01")
ChallengeTypeDNS01 = AcmeChallenge("dns-01")
ChallengeTypeTLSALPN01 = AcmeChallenge("tls-alpn-01")
ChallengeTypeHTTP01 = AcmeChallenge("http-01")
ChallengeTypeDNS01 = AcmeChallenge("dns-01")
ChallengeTypeDNSAccount01 = AcmeChallenge("dns-account-01")
ChallengeTypeTLSALPN01 = AcmeChallenge("tls-alpn-01")
)

// IsValid tests whether the challenge is a known challenge
func (c AcmeChallenge) IsValid() bool {
switch c {
case ChallengeTypeHTTP01, ChallengeTypeDNS01, ChallengeTypeTLSALPN01:
case ChallengeTypeHTTP01, ChallengeTypeDNS01, ChallengeTypeDNSAccount01, ChallengeTypeTLSALPN01:
return true
default:
return false
}
}

// AuthorizationScope defines the scope of an authorization.
// This is used to determine challenge validation behavior.
type AuthorizationScope string

const (
AuthorizationScopeHost = AuthorizationScope("host")
AuthorizationScopeWildcard = AuthorizationScope("wildcard")
)

// OCSPStatus defines the state of OCSP for a domain
type OCSPStatus string

Expand Down Expand Up @@ -192,7 +202,7 @@ type Challenge struct {
// For the V2 API the "URI" field is deprecated in favour of URL.
URL string `json:"url,omitempty"`

// Used by http-01, tls-sni-01, tls-alpn-01 and dns-01 challenges
// Used by http-01, tls-sni-01, tls-alpn-01, dns-01 and dns-account-01 challenges
Token string `json:"token,omitempty"`

// The expected KeyAuthorization for validation of the challenge. Populated by
Expand Down Expand Up @@ -256,7 +266,7 @@ func (ch Challenge) RecordsSane() bool {
ch.ValidationRecord[0].AddressUsed == nil || len(ch.ValidationRecord[0].AddressesResolved) == 0 {
return false
}
case ChallengeTypeDNS01:
case ChallengeTypeDNS01, ChallengeTypeDNSAccount01:
if len(ch.ValidationRecord) > 1 {
return false
}
Expand Down
2 changes: 1 addition & 1 deletion core/objects_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func TestChallengeSanityCheck(t *testing.T) {
}`), &accountKey)
test.AssertNotError(t, err, "Error unmarshaling JWK")

types := []AcmeChallenge{ChallengeTypeHTTP01, ChallengeTypeDNS01, ChallengeTypeTLSALPN01}
types := []AcmeChallenge{ChallengeTypeHTTP01, ChallengeTypeDNS01, ChallengeTypeDNSAccount01, ChallengeTypeTLSALPN01}
for _, challengeType := range types {
chall := Challenge{
Type: challengeType,
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ require (
github.com/aws/aws-sdk-go-v2/config v1.26.3
github.com/aws/aws-sdk-go-v2/service/s3 v1.50.2
github.com/aws/smithy-go v1.20.0
github.com/eggsampler/acme/v3 v3.4.0
github.com/eggsampler/acme/v3 v3.5.0
github.com/go-logr/stdr v1.2.2
github.com/go-sql-driver/mysql v1.5.0
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZm
github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f h1:lO4WD4F/rVNCu3HqELle0jiPLLBs70cWOduZpkS1E78=
github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f/go.mod h1:cuUVRXasLTGF7a8hSLbxyZXjz+1KgoB3wDUb6vlszIc=
github.com/dgryski/go-sip13 v0.0.0-20181026042036-e10d5fee7954/go.mod h1:vAd38F8PWV+bWy6jNmig1y/TA+kYO4g3RSRF0IAv0no=
github.com/eggsampler/acme/v3 v3.4.0 h1:LHWnB3wShVshK1+umL6ObCjnc0MM+D7TE8JINjk8zGY=
github.com/eggsampler/acme/v3 v3.4.0/go.mod h1:/qh0rKC/Dh7Jj+p4So7DbWmFNzC4dpcpK53r226Fhuo=
github.com/eggsampler/acme/v3 v3.5.0 h1:tM8IXhS95HLm2LGxRDI3yzQrs7iZ9mKep1JjQhTIsUo=
github.com/eggsampler/acme/v3 v3.5.0/go.mod h1:/qh0rKC/Dh7Jj+p4So7DbWmFNzC4dpcpK53r226Fhuo=
github.com/envoyproxy/protoc-gen-validate v1.0.2 h1:QkIBuU5k+x7/QXPvPPnWXWlCdaBFApVqftFV6k087DA=
github.com/envoyproxy/protoc-gen-validate v1.0.2/go.mod h1:GpiZQP3dDbg4JouG/NNS7QWXpgx6x8QiMKdmN72jogE=
github.com/fatih/color v1.9.0/go.mod h1:eQcE1qtQxscV5RaZvpXrrb8Drkc3/DdQ+uUYCNjL+zU=
Expand Down
28 changes: 20 additions & 8 deletions policy/pa.go
Original file line number Diff line number Diff line change
Expand Up @@ -517,17 +517,25 @@ func (pa *AuthorityImpl) challengeTypesFor(identifier identifier.ACMEIdentifier)
var challenges []core.AcmeChallenge

// If the identifier is for a DNS wildcard name we only
// provide a DNS-01 challenge as a matter of CA policy.
// provide a DNS-based 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.
// We must have a DNS-based challenge type enabled to create challenges for
// a wildcard identifier per CA policy.
if !pa.ChallengeTypeEnabled(core.ChallengeTypeDNS01) {
return nil, fmt.Errorf(
"Challenges requested for wildcard identifier but DNS-01 " +
"challenge type is not enabled")
if !pa.ChallengeTypeEnabled(core.ChallengeTypeDNSAccount01) {
Comment on lines 532 to +533
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like we can clean up this logic a little by turning it around:

  • append dns-01 if enabled
  • append dns-account-01 if enabled
  • return error if list is empty

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

return nil, fmt.Errorf(
"Challenges requested for wildcard identifier but a DNS-01 " +
"or DNS-ACCOUNT-01 challenge type is not enabled")
}
}
// Only provide DNS-based challenges based on what is enabled.
if pa.ChallengeTypeEnabled(core.ChallengeTypeDNS01) {
challenges = append(challenges, core.ChallengeTypeDNS01)
}

if pa.ChallengeTypeEnabled(core.ChallengeTypeDNSAccount01) {
challenges = append(challenges, core.ChallengeTypeDNSAccount01)
}
// Only provide a DNS-01-Wildcard challenge
challenges = []core.AcmeChallenge{core.ChallengeTypeDNS01}
} else {
// Otherwise we collect up challenges based on what is enabled.
if pa.ChallengeTypeEnabled(core.ChallengeTypeHTTP01) {
Expand All @@ -541,6 +549,10 @@ func (pa *AuthorityImpl) challengeTypesFor(identifier identifier.ACMEIdentifier)
if pa.ChallengeTypeEnabled(core.ChallengeTypeDNS01) {
challenges = append(challenges, core.ChallengeTypeDNS01)
}

if pa.ChallengeTypeEnabled(core.ChallengeTypeDNSAccount01) {
challenges = append(challenges, core.ChallengeTypeDNSAccount01)
}
}

return challenges, nil
Expand Down
31 changes: 23 additions & 8 deletions policy/pa_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -394,28 +394,43 @@ func TestChallengesForWildcard(t *testing.T) {
Value: "*.zombo.com",
}

// First try to get a challenge for the wildcard ident without the
// DNS-01 challenge type enabled. This should produce an error
// Try to get a challenge for the wildcard ident without
// DNS challenges enabled. This should error.
var enabledChallenges = map[core.AcmeChallenge]bool{
core.ChallengeTypeHTTP01: true,
core.ChallengeTypeDNS01: false,
}
pa := must.Do(New(enabledChallenges, blog.NewMock()))
_, err := pa.ChallengesFor(wildcardIdent)
test.AssertError(t, err, "ChallengesFor did not error for a wildcard ident "+
"when DNS-01 was disabled")
"when DNS challenge types were disabled")
test.AssertEquals(t, err.Error(), "Challenges requested for wildcard "+
"identifier but DNS-01 challenge type is not enabled")
"identifier but a DNS-01 or DNS-ACCOUNT-01 challenge type is not enabled")

// Try again with DNS-01 enabled. It should not error and
// Enable DNS-01 and HTTP-01. It should not error and
// should return only one DNS-01 type challenge
enabledChallenges[core.ChallengeTypeDNS01] = true
enabledChallenges = map[core.AcmeChallenge]bool{
core.ChallengeTypeHTTP01: true,
core.ChallengeTypeDNS01: true,
}
pa = must.Do(New(enabledChallenges, blog.NewMock()))
challenges, err := pa.ChallengesFor(wildcardIdent)
test.AssertNotError(t, err, "ChallengesFor errored for a wildcard ident "+
"unexpectedly")
"unexpectedly with DNS-01 enabled")
test.AssertEquals(t, len(challenges), 1)
test.AssertEquals(t, challenges[0].Type, core.ChallengeTypeDNS01)

// Enable DNS-ACCOUNT-01 and HTTP-01. It should not error and
// should return only one DNS-ACCOUNT-01 type challenge
Comment on lines +422 to +423
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like we should also have a test case where both dns-01 and dns-account-01 are enabled?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

enabledChallenges = map[core.AcmeChallenge]bool{
core.ChallengeTypeHTTP01: true,
core.ChallengeTypeDNSAccount01: true,
}
pa = must.Do(New(enabledChallenges, blog.NewMock()))
challenges, err = pa.ChallengesFor(wildcardIdent)
test.AssertNotError(t, err, "ChallengesFor errored for a wildcard ident "+
"unexpectedly with DNS-ACCOUNT-01 enabled")
test.AssertEquals(t, len(challenges), 1)
test.AssertEquals(t, challenges[0].Type, core.ChallengeTypeDNSAccount01)
}

// TestMalformedExactBlocklist tests that loading a YAML policy file with an
Expand Down
4 changes: 3 additions & 1 deletion ra/ra.go
Original file line number Diff line number Diff line change
Expand Up @@ -2535,7 +2535,9 @@ func (ra *RegistrationAuthorityImpl) NewOrder(ctx context.Context, req *rapb.New
// that doesn't meet this criteria from SA.GetAuthorizations but we verify
// again to be safe.
if strings.HasPrefix(name, "*.") &&
len(authz.Challenges) == 1 && core.AcmeChallenge(authz.Challenges[0].Type) == core.ChallengeTypeDNS01 {
len(authz.Challenges) == 1 &&
(core.AcmeChallenge(authz.Challenges[0].Type) == core.ChallengeTypeDNS01 ||
core.AcmeChallenge(authz.Challenges[0].Type) == core.ChallengeTypeDNSAccount01) {
authzID, err := strconv.ParseInt(authz.Id, 10, 64)
if err != nil {
return nil, err
Expand Down
5 changes: 5 additions & 0 deletions ra/ra_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ func createPendingAuthorization(t *testing.T, sa sapb.StorageAuthorityClient, do
Type: core.ChallengeTypeDNS01,
Status: core.StatusPending,
},
{
Token: core.NewToken(),
Type: core.ChallengeTypeDNSAccount01,
Status: core.StatusPending,
},
{
Token: core.NewToken(),
Type: core.ChallengeTypeTLSALPN01,
Expand Down
8 changes: 5 additions & 3 deletions sa/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -441,15 +441,17 @@ func modelToOrder(om *orderModel) (*corepb.Order, error) {
}

var challTypeToUint = map[string]uint8{
"http-01": 0,
"dns-01": 1,
"tls-alpn-01": 2,
"http-01": 0,
"dns-01": 1,
"tls-alpn-01": 2,
"dns-account-01": 3,
aarongable marked this conversation as resolved.
Show resolved Hide resolved
}

var uintToChallType = map[uint8]string{
0: "http-01",
1: "dns-01",
2: "tls-alpn-01",
3: "dns-account-01",
}

var identifierTypeToUint = map[string]uint8{
Expand Down
1 change: 1 addition & 0 deletions test/config-next/ca.json
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@
"challenges": {
"http-01": true,
"dns-01": true,
"dns-account-01": true,
"tls-alpn-01": true
}
},
Expand Down
1 change: 1 addition & 0 deletions test/config-next/ra.json
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@
"challenges": {
"http-01": true,
"dns-01": true,
"dns-account-01": true,
"tls-alpn-01": true
}
},
Expand Down
Loading
Loading