From 59c5219a07ff29c4528cd381857325abb9834899 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 8 Sep 2022 12:34:06 -0700 Subject: [PATCH] Use a type for acme challenges --- acme/api/order.go | 2 +- acme/common.go | 2 +- authority/provisioner/acme.go | 49 ++++++++++++++++++---- authority/provisioner/acme_test.go | 67 +++++++++++++++++++++++------- authority/provisioners.go | 46 ++++++++++---------- 5 files changed, 119 insertions(+), 47 deletions(-) diff --git a/acme/api/order.go b/acme/api/order.go index 90541d79..7e49af2b 100644 --- a/acme/api/order.go +++ b/acme/api/order.go @@ -259,7 +259,7 @@ func newAuthorization(ctx context.Context, az *acme.Authorization) error { az.Challenges = make([]*acme.Challenge, 0, len(chTypes)) for _, typ := range chTypes { // Make sure the challenge is enabled - if err := prov.AuthorizeChallenge(ctx, string(typ)); err != nil { + if err := prov.AuthorizeChallenge(ctx, provisioner.ACMEChallenge(typ)); err != nil { continue } diff --git a/acme/common.go b/acme/common.go index 3a1e7a8f..cdadeebf 100644 --- a/acme/common.go +++ b/acme/common.go @@ -71,7 +71,7 @@ type Provisioner interface { AuthorizeOrderIdentifier(ctx context.Context, identifier provisioner.ACMEIdentifier) error AuthorizeSign(ctx context.Context, token string) ([]provisioner.SignOption, error) AuthorizeRevoke(ctx context.Context, token string) error - AuthorizeChallenge(ctx context.Context, challenge string) error + AuthorizeChallenge(ctx context.Context, challenge provisioner.ACMEChallenge) error GetID() string GetName() string DefaultTLSCertDuration() time.Duration diff --git a/authority/provisioner/acme.go b/authority/provisioner/acme.go index dc7533e5..c6ff508c 100644 --- a/authority/provisioner/acme.go +++ b/authority/provisioner/acme.go @@ -11,6 +11,35 @@ import ( "github.com/pkg/errors" ) +// ACMEChallenge represents the supported acme challenges. +type ACMEChallenge string + +const ( + // HTTP_01 is the http-01 ACME challenge. + HTTP_01 ACMEChallenge = "http-01" + // DNS_01 is the dns-01 ACME challenge. + DNS_01 ACMEChallenge = "dns-01" + // TLS_ALPN_01 is the tls-alpn-01 ACME challenge. + TLS_ALPN_01 ACMEChallenge = "tls-alpn-01" + // DEVICE_ATTEST_01 is the device-attest-01 ACME challenge. + DEVICE_ATTEST_01 ACMEChallenge = "device-attest-01" +) + +// String returns a normalized version of the challenge. +func (c ACMEChallenge) String() string { + return strings.ToLower(string(c)) +} + +// Validate returns an error if the acme challenge is not a valid one. +func (c ACMEChallenge) Validate() error { + switch ACMEChallenge(c.String()) { + case HTTP_01, DNS_01, TLS_ALPN_01, DEVICE_ATTEST_01: + return nil + default: + return fmt.Errorf("acme challenge %q is not supported", c) + } +} + // ACME is the acme provisioner type, an entity that can authorize the ACME // provisioning flow. type ACME struct { @@ -27,9 +56,9 @@ type ACME struct { // Challenges contains the enabled challenges for this provisioner. If this // value is not set the default http-01, dns-01 and tls-alpn-01 challenges // will be enabled, device-attest-01 will be disabled. - Challenges []string `json:"challenges,omitempty"` - Claims *Claims `json:"claims,omitempty"` - Options *Options `json:"options,omitempty"` + Challenges []ACMEChallenge `json:"challenges,omitempty"` + Claims *Claims `json:"claims,omitempty"` + Options *Options `json:"options,omitempty"` ctl *Controller } @@ -88,6 +117,12 @@ func (p *ACME) Init(config Config) (err error) { return errors.New("provisioner name cannot be empty") } + for _, c := range p.Challenges { + if err := c.Validate(); err != nil { + return err + } + } + p.ctl, err = NewController(p, p.Claims, config, p.Options) return } @@ -172,15 +207,15 @@ func (p *ACME) AuthorizeRenew(ctx context.Context, cert *x509.Certificate) error // AuthorizeChallenge checks if the given challenge is enabled. By default // http-01, dns-01 and tls-alpn-01 are enabled, to disable any of them the // Challenge provisioner property should have at least one element. -func (p *ACME) AuthorizeChallenge(ctx context.Context, challenge string) error { - enabledChallenges := []string{ - "http-01", "dns-01", "tls-alpn-01", +func (p *ACME) AuthorizeChallenge(ctx context.Context, challenge ACMEChallenge) error { + enabledChallenges := []ACMEChallenge{ + HTTP_01, DNS_01, TLS_ALPN_01, } if len(p.Challenges) > 0 { enabledChallenges = p.Challenges } for _, ch := range enabledChallenges { - if strings.EqualFold(ch, challenge) { + if strings.EqualFold(string(ch), string(challenge)) { return nil } } diff --git a/authority/provisioner/acme_test.go b/authority/provisioner/acme_test.go index 8f48e37e..d50b6ddb 100644 --- a/authority/provisioner/acme_test.go +++ b/authority/provisioner/acme_test.go @@ -13,6 +13,28 @@ import ( "github.com/smallstep/certificates/api/render" ) +func TestACMEChallenge_Validate(t *testing.T) { + tests := []struct { + name string + c ACMEChallenge + wantErr bool + }{ + {"http-01", HTTP_01, false}, + {"dns-01", DNS_01, false}, + {"tls-alpn-01", TLS_ALPN_01, false}, + {"device-attest-01", DEVICE_ATTEST_01, false}, + {"uppercase", "HTTP-01", false}, + {"fail", "http-02", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := tt.c.Validate(); (err != nil) != tt.wantErr { + t.Errorf("ACMEChallenge.Validate() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + func TestACME_Getters(t *testing.T) { p, err := generateACME() assert.FatalError(t, err) @@ -65,11 +87,26 @@ func TestACME_Init(t *testing.T) { err: errors.New("claims: MinTLSCertDuration must be greater than 0"), } }, + "fail-bad-challenge": func(t *testing.T) ProvisionerValidateTest { + return ProvisionerValidateTest{ + p: &ACME{Name: "foo", Type: "bar", Challenges: []ACMEChallenge{HTTP_01, "zar"}}, + err: errors.New("acme challenge \"zar\" is not supported"), + } + }, "ok": func(t *testing.T) ProvisionerValidateTest { return ProvisionerValidateTest{ p: &ACME{Name: "foo", Type: "bar"}, } }, + "ok with challenges": func(t *testing.T) ProvisionerValidateTest { + return ProvisionerValidateTest{ + p: &ACME{ + Name: "foo", + Type: "bar", + Challenges: []ACMEChallenge{DNS_01, DEVICE_ATTEST_01}, + }, + } + }, } config := Config{ @@ -208,11 +245,11 @@ func TestACME_AuthorizeSign(t *testing.T) { func TestACME_AuthorizeChallenge(t *testing.T) { ctx := context.Background() type fields struct { - Challenges []string + Challenges []ACMEChallenge } type args struct { ctx context.Context - challenge string + challenge ACMEChallenge } tests := []struct { name string @@ -220,19 +257,19 @@ func TestACME_AuthorizeChallenge(t *testing.T) { args args wantErr bool }{ - {"ok http-01", fields{nil}, args{ctx, "http-01"}, false}, - {"ok dns-01", fields{nil}, args{ctx, "dns-01"}, false}, - {"ok tls-alpn-01", fields{[]string{}}, args{ctx, "tls-alpn-01"}, false}, - {"fail device-attest-01", fields{[]string{}}, args{ctx, "device-attest-01"}, true}, - {"ok http-01 enabled", fields{[]string{"http-01"}}, args{ctx, "http-01"}, false}, - {"ok dns-01 enabled", fields{[]string{"http-01", "dns-01"}}, args{ctx, "dns-01"}, false}, - {"ok tls-alpn-01 enabled", fields{[]string{"http-01", "dns-01", "tls-alpn-01"}}, args{ctx, "tls-alpn-01"}, false}, - {"ok device-attest-01 enabled", fields{[]string{"device-attest-01", "dns-01"}}, args{ctx, "device-attest-01"}, false}, - {"fail http-01", fields{[]string{"dns-01"}}, args{ctx, "http-01"}, true}, - {"fail dns-01", fields{[]string{"http-01", "tls-alpn-01"}}, args{ctx, "dns-01"}, true}, - {"fail tls-alpn-01", fields{[]string{"http-01", "dns-01", "device-attest-01"}}, args{ctx, "tls-alpn-01"}, true}, - {"fail device-attest-01", fields{[]string{"http-01", "dns-01"}}, args{ctx, "device-attest-01"}, true}, - {"fail unknown", fields{[]string{"http-01", "dns-01", "tls-alpn-01", "device-attest-01"}}, args{ctx, "unknown"}, true}, + {"ok http-01", fields{nil}, args{ctx, HTTP_01}, false}, + {"ok dns-01", fields{nil}, args{ctx, DNS_01}, false}, + {"ok tls-alpn-01", fields{[]ACMEChallenge{}}, args{ctx, TLS_ALPN_01}, false}, + {"fail device-attest-01", fields{[]ACMEChallenge{}}, args{ctx, "device-attest-01"}, true}, + {"ok http-01 enabled", fields{[]ACMEChallenge{"http-01"}}, args{ctx, "HTTP-01"}, false}, + {"ok dns-01 enabled", fields{[]ACMEChallenge{"http-01", "dns-01"}}, args{ctx, DNS_01}, false}, + {"ok tls-alpn-01 enabled", fields{[]ACMEChallenge{"http-01", "dns-01", "tls-alpn-01"}}, args{ctx, TLS_ALPN_01}, false}, + {"ok device-attest-01 enabled", fields{[]ACMEChallenge{"device-attest-01", "dns-01"}}, args{ctx, DEVICE_ATTEST_01}, false}, + {"fail http-01", fields{[]ACMEChallenge{"dns-01"}}, args{ctx, "http-01"}, true}, + {"fail dns-01", fields{[]ACMEChallenge{"http-01", "tls-alpn-01"}}, args{ctx, "dns-01"}, true}, + {"fail tls-alpn-01", fields{[]ACMEChallenge{"http-01", "dns-01", "device-attest-01"}}, args{ctx, "tls-alpn-01"}, true}, + {"fail device-attest-01", fields{[]ACMEChallenge{"http-01", "dns-01"}}, args{ctx, "device-attest-01"}, true}, + {"fail unknown", fields{[]ACMEChallenge{"http-01", "dns-01", "tls-alpn-01", "device-attest-01"}}, args{ctx, "unknown"}, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/authority/provisioners.go b/authority/provisioners.go index 95969d22..02f1cd54 100644 --- a/authority/provisioners.go +++ b/authority/provisioners.go @@ -1125,39 +1125,39 @@ func parseInstanceAge(age string) (provisioner.Duration, error) { return instanceAge, nil } -func challengesToCertificates(challenges []linkedca.ACMEProvisioner_ChallengeType) []string { - ret := make([]string, len(challenges)) - for i, ch := range challenges { +// challengesToCertificates converts linkedca challenges to provisioner ones +// skipping the unknown ones. +func challengesToCertificates(challenges []linkedca.ACMEProvisioner_ChallengeType) []provisioner.ACMEChallenge { + ret := make([]provisioner.ACMEChallenge, 0, len(challenges)) + for _, ch := range challenges { switch ch { case linkedca.ACMEProvisioner_HTTP_01: - ret[i] = "http-01" + ret = append(ret, provisioner.HTTP_01) case linkedca.ACMEProvisioner_DNS_01: - ret[i] = "dns-01" + ret = append(ret, provisioner.DNS_01) case linkedca.ACMEProvisioner_TLS_ALPN_O1: - ret[i] = "tls-alpn-01" + ret = append(ret, provisioner.TLS_ALPN_01) case linkedca.ACMEProvisioner_DEVICE_ATTEST_01: - ret[i] = "device-attest-01" - default: - ret[i] = "unknown" + ret = append(ret, provisioner.DEVICE_ATTEST_01) } } return ret } -func challengesToLinkedca(challenges []string) []linkedca.ACMEProvisioner_ChallengeType { - ret := make([]linkedca.ACMEProvisioner_ChallengeType, len(challenges)) - for i, ch := range challenges { - switch ch { - case "http-01": - ret[i] = linkedca.ACMEProvisioner_HTTP_01 - case "dns-01": - ret[i] = linkedca.ACMEProvisioner_DNS_01 - case "tls-alpn-01": - ret[i] = linkedca.ACMEProvisioner_TLS_ALPN_O1 - case "device-attest-01": - ret[i] = linkedca.ACMEProvisioner_DEVICE_ATTEST_01 - default: - ret[i] = linkedca.ACMEProvisioner_UNKNOWN +// challengesToLinkedca converts provisioner challenges to linkedca ones +// skipping the unknown ones. +func challengesToLinkedca(challenges []provisioner.ACMEChallenge) []linkedca.ACMEProvisioner_ChallengeType { + ret := make([]linkedca.ACMEProvisioner_ChallengeType, 0, len(challenges)) + for _, ch := range challenges { + switch provisioner.ACMEChallenge(ch.String()) { + case provisioner.HTTP_01: + ret = append(ret, linkedca.ACMEProvisioner_HTTP_01) + case provisioner.DNS_01: + ret = append(ret, linkedca.ACMEProvisioner_DNS_01) + case provisioner.TLS_ALPN_01: + ret = append(ret, linkedca.ACMEProvisioner_TLS_ALPN_O1) + case provisioner.DEVICE_ATTEST_01: + ret = append(ret, linkedca.ACMEProvisioner_DEVICE_ATTEST_01) } } return ret