From ba42aaf86524e797cbd58a56d1cd59184ff015d7 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 8 Sep 2022 17:16:50 -0700 Subject: [PATCH 1/6] Add attestationFormat property in the ACME provisioner --- authority/provisioner/acme.go | 63 +++++++++++++++++++++++++- authority/provisioner/acme_test.go | 72 ++++++++++++++++++++++++++++-- 2 files changed, 129 insertions(+), 6 deletions(-) diff --git a/authority/provisioner/acme.go b/authority/provisioner/acme.go index 37a4fd28..f86ab07d 100644 --- a/authority/provisioner/acme.go +++ b/authority/provisioner/acme.go @@ -41,6 +41,38 @@ func (c ACMEChallenge) Validate() error { } } +// ACMEPlatform represents the format used on a device-attest-01 challenge. +type ACMEAttestationFormat string + +const ( + // APPLE is the format used to enable device-attest-01 on apple devices. + APPLE ACMEAttestationFormat = "apple" + + // STEP is the format used to enable device-attest-01 on devices that + // provide attestation certificates like the PIV interface on YubiKeys. + // + // TODO(mariano): should we rename this to something else. + STEP ACMEAttestationFormat = "step" + + // TPM is the format used to enable device-attest-01 on TPMs. + TPM ACMEAttestationFormat = "tpm" +) + +// String returns a normalized version of the attestation format. +func (f ACMEAttestationFormat) String() string { + return strings.ToLower(string(f)) +} + +// Validate returns an error if the attestation format is not a valid one. +func (f ACMEAttestationFormat) Validate() error { + switch ACMEAttestationFormat(f.String()) { + case APPLE, STEP, TPM: + return nil + default: + return fmt.Errorf("acme attestation format %q is not supported", f) + } +} + // ACME is the acme provisioner type, an entity that can authorize the ACME // provisioning flow. type ACME struct { @@ -58,8 +90,12 @@ type ACME struct { // 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 []ACMEChallenge `json:"challenges,omitempty"` - Claims *Claims `json:"claims,omitempty"` - Options *Options `json:"options,omitempty"` + // AttestationFormats contains the enabled attestation formats for this + // provisioner. If this value is not set the default apple, step and tpm + // will be used. + AttestationFormats []ACMEAttestationFormat `json:"attestationFormats,omitempty"` + Claims *Claims `json:"claims,omitempty"` + Options *Options `json:"options,omitempty"` ctl *Controller } @@ -123,6 +159,11 @@ func (p *ACME) Init(config Config) (err error) { return err } } + for _, f := range p.AttestationFormats { + if err := f.Validate(); err != nil { + return err + } + } p.ctl, err = NewController(p, p.Claims, config, p.Options) return @@ -222,3 +263,21 @@ func (p *ACME) IsChallengeEnabled(ctx context.Context, challenge ACMEChallenge) } return false } + +// IsAttestationFormatEnabled checks if the given attestation format is enabled. +// By default apple, step and tpm are enabled, to disable any of them the +// AttestationFormat provisioner property should have at least one element. +func (p *ACME) IsAttestationFormatEnabled(ctx context.Context, format ACMEAttestationFormat) bool { + enabledFormats := []ACMEAttestationFormat{ + APPLE, STEP, TPM, + } + if len(p.AttestationFormats) > 0 { + enabledFormats = p.AttestationFormats + } + for _, f := range enabledFormats { + if strings.EqualFold(string(f), string(format)) { + return true + } + } + return false +} diff --git a/authority/provisioner/acme_test.go b/authority/provisioner/acme_test.go index 641b7f38..6152a8c9 100644 --- a/authority/provisioner/acme_test.go +++ b/authority/provisioner/acme_test.go @@ -35,6 +35,27 @@ func TestACMEChallenge_Validate(t *testing.T) { } } +func TestACMEAttestationFormat_Validate(t *testing.T) { + tests := []struct { + name string + f ACMEAttestationFormat + wantErr bool + }{ + {"apple", APPLE, false}, + {"step", STEP, false}, + {"tpm", TPM, false}, + {"uppercase", "APPLE", false}, + {"fail", "FOO", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := tt.f.Validate(); (err != nil) != tt.wantErr { + t.Errorf("ACMEAttestationFormat.Validate() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + func TestACME_Getters(t *testing.T) { p, err := generateACME() assert.FatalError(t, err) @@ -93,17 +114,24 @@ func TestACME_Init(t *testing.T) { err: errors.New("acme challenge \"zar\" is not supported"), } }, + "fail-bad-attestation-format": func(t *testing.T) ProvisionerValidateTest { + return ProvisionerValidateTest{ + p: &ACME{Name: "foo", Type: "bar", AttestationFormats: []ACMEAttestationFormat{APPLE, "zar"}}, + err: errors.New("acme attestation format \"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 { + "ok attestation": func(t *testing.T) ProvisionerValidateTest { return ProvisionerValidateTest{ p: &ACME{ - Name: "foo", - Type: "bar", - Challenges: []ACMEChallenge{DNS_01, DEVICE_ATTEST_01}, + Name: "foo", + Type: "bar", + Challenges: []ACMEChallenge{DNS_01, DEVICE_ATTEST_01}, + AttestationFormats: []ACMEAttestationFormat{APPLE, STEP}, }, } }, @@ -282,3 +310,39 @@ func TestACME_IsChallengeEnabled(t *testing.T) { }) } } + +func TestACME_IsAttestationFormatEnabled(t *testing.T) { + ctx := context.Background() + type fields struct { + AttestationFormats []ACMEAttestationFormat + } + type args struct { + ctx context.Context + format ACMEAttestationFormat + } + tests := []struct { + name string + fields fields + args args + want bool + }{ + {"ok", fields{[]ACMEAttestationFormat{APPLE, STEP, TPM}}, args{ctx, TPM}, true}, + {"ok empty apple", fields{nil}, args{ctx, APPLE}, true}, + {"ok empty step", fields{nil}, args{ctx, STEP}, true}, + {"ok empty tpm", fields{[]ACMEAttestationFormat{}}, args{ctx, "tpm"}, true}, + {"ok uppercase", fields{[]ACMEAttestationFormat{APPLE, STEP, TPM}}, args{ctx, "STEP"}, true}, + {"fail apple", fields{[]ACMEAttestationFormat{STEP, TPM}}, args{ctx, APPLE}, false}, + {"fail step", fields{[]ACMEAttestationFormat{APPLE, TPM}}, args{ctx, STEP}, false}, + {"fail step", fields{[]ACMEAttestationFormat{APPLE, STEP}}, args{ctx, TPM}, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := &ACME{ + AttestationFormats: tt.fields.AttestationFormats, + } + if got := p.IsAttestationFormatEnabled(tt.args.ctx, tt.args.format); got != tt.want { + t.Errorf("ACME.IsAttestationFormatEnabled() = %v, want %v", got, tt.want) + } + }) + } +} From 53ad3a9dbe845eb1a79a28b5335081f5dfddf29c Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 8 Sep 2022 17:24:51 -0700 Subject: [PATCH 2/6] Add go workspaces files to gitignore --- .gitignore | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.gitignore b/.gitignore index 299a2c16..42e96049 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,10 @@ *.so *.dylib +# Go Workspaces +go.work +go.work.sum + # Test binary, build with `go test -c` *.test From 0f651799d0e79904fa2cd85effc64a3eaad2abd1 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 8 Sep 2022 17:38:05 -0700 Subject: [PATCH 3/6] Reject not enabled attestation formats --- acme/api/account_test.go | 4 ++++ acme/challenge.go | 7 +++++++ acme/common.go | 14 ++++++++++++-- 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/acme/api/account_test.go b/acme/api/account_test.go index 15405f24..a67e1a62 100644 --- a/acme/api/account_test.go +++ b/acme/api/account_test.go @@ -45,6 +45,10 @@ func (*fakeProvisioner) IsChallengeEnabled(ctx context.Context, challenge provis return true } +func (*fakeProvisioner) IsAttestationFormatEnabled(ctx context.Context, format provisioner.ACMEAttestationFormat) bool { + return true +} + func (*fakeProvisioner) AuthorizeRevoke(ctx context.Context, token string) error { return nil } func (*fakeProvisioner) GetID() string { return "" } func (*fakeProvisioner) GetName() string { return "" } diff --git a/acme/challenge.go b/acme/challenge.go index aec30aec..cc860f95 100644 --- a/acme/challenge.go +++ b/acme/challenge.go @@ -26,6 +26,7 @@ import ( "time" "github.com/fxamacker/cbor/v2" + "github.com/smallstep/certificates/authority/provisioner" "go.step.sm/crypto/jose" "go.step.sm/crypto/pemutil" ) @@ -341,6 +342,12 @@ func deviceAttest01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose return WrapErrorISE(err, "error unmarshalling CBOR") } + prov := MustProvisionerFromContext(ctx) + if !prov.IsAttestationFormatEnabled(ctx, provisioner.ACMEAttestationFormat(att.Format)) { + return storeError(ctx, db, ch, true, + NewError(ErrorBadAttestationStatementType, "attestation format %q is not enabled", att.Format)) + } + switch att.Format { case "apple": data, err := doAppleAttestationFormat(ctx, ch, db, &att) diff --git a/acme/common.go b/acme/common.go index 331b21ca..b7260386 100644 --- a/acme/common.go +++ b/acme/common.go @@ -72,6 +72,7 @@ type Provisioner interface { AuthorizeSign(ctx context.Context, token string) ([]provisioner.SignOption, error) AuthorizeRevoke(ctx context.Context, token string) error IsChallengeEnabled(ctx context.Context, challenge provisioner.ACMEChallenge) bool + IsAttestationFormatEnabled(ctx context.Context, format provisioner.ACMEAttestationFormat) bool GetID() string GetName() string DefaultTLSCertDuration() time.Duration @@ -110,7 +111,8 @@ type MockProvisioner struct { MauthorizeOrderIdentifier func(ctx context.Context, identifier provisioner.ACMEIdentifier) error MauthorizeSign func(ctx context.Context, ott string) ([]provisioner.SignOption, error) MauthorizeRevoke func(ctx context.Context, token string) error - MisChallengeEnabled func(Ctx context.Context, challenge provisioner.ACMEChallenge) bool + MisChallengeEnabled func(ctx context.Context, challenge provisioner.ACMEChallenge) bool + MisAttFormatEnabled func(ctx context.Context, format provisioner.ACMEAttestationFormat) bool MdefaultTLSCertDuration func() time.Duration MgetOptions func() *provisioner.Options } @@ -147,7 +149,7 @@ func (m *MockProvisioner) AuthorizeRevoke(ctx context.Context, token string) err return m.Merr } -// AuthorizeChallenge mock +// IsChallengeEnabled mock func (m *MockProvisioner) IsChallengeEnabled(ctx context.Context, challenge provisioner.ACMEChallenge) bool { if m.MisChallengeEnabled != nil { return m.MisChallengeEnabled(ctx, challenge) @@ -155,6 +157,14 @@ func (m *MockProvisioner) IsChallengeEnabled(ctx context.Context, challenge prov return m.Merr == nil } +// IsAttestationFormatEnabled mock +func (m *MockProvisioner) IsAttestationFormatEnabled(ctx context.Context, format provisioner.ACMEAttestationFormat) bool { + if m.MisAttFormatEnabled != nil { + return m.MisAttFormatEnabled(ctx, format) + } + return m.Merr == nil +} + // DefaultTLSCertDuration mock func (m *MockProvisioner) DefaultTLSCertDuration() time.Duration { if m.MdefaultTLSCertDuration != nil { From 66407139e5f41f1056e1894f743be0b6f8a0f2f5 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 8 Sep 2022 17:49:24 -0700 Subject: [PATCH 4/6] Add methods to convert attestation formats --- authority/provisioners.go | 56 ++++++++++++++++++++++++++++++++------- go.mod | 2 +- go.sum | 4 +-- 3 files changed, 49 insertions(+), 13 deletions(-) diff --git a/authority/provisioners.go b/authority/provisioners.go index 02f1cd54..1e5ddda5 100644 --- a/authority/provisioners.go +++ b/authority/provisioners.go @@ -748,14 +748,15 @@ func ProvisionerToCertificates(p *linkedca.Provisioner) (provisioner.Interface, case *linkedca.ProvisionerDetails_ACME: cfg := d.ACME return &provisioner.ACME{ - ID: p.Id, - Type: p.Type.String(), - Name: p.Name, - ForceCN: cfg.ForceCn, - RequireEAB: cfg.RequireEab, - Challenges: challengesToCertificates(cfg.Challenges), - Claims: claims, - Options: options, + ID: p.Id, + Type: p.Type.String(), + Name: p.Name, + ForceCN: cfg.ForceCn, + RequireEAB: cfg.RequireEab, + Challenges: challengesToCertificates(cfg.Challenges), + AttestationFormats: attestationFormatsToCertificates(cfg.AttestationFormats), + Claims: claims, + Options: options, }, nil case *linkedca.ProvisionerDetails_OIDC: cfg := d.OIDC @@ -1002,8 +1003,9 @@ func ProvisionerToLinkedca(p provisioner.Interface) (*linkedca.Provisioner, erro Details: &linkedca.ProvisionerDetails{ Data: &linkedca.ProvisionerDetails_ACME{ ACME: &linkedca.ACMEProvisioner{ - ForceCn: p.ForceCN, - Challenges: challengesToLinkedca(p.Challenges), + ForceCn: p.ForceCN, + Challenges: challengesToLinkedca(p.Challenges), + AttestationFormats: attestationFormatsToLinkedca(p.AttestationFormats), }, }, }, @@ -1162,3 +1164,37 @@ func challengesToLinkedca(challenges []provisioner.ACMEChallenge) []linkedca.ACM } return ret } + +// attestationFormatsToCertificates converts linkedca attestation formats to +// provisioner ones skipping the unknown ones. +func attestationFormatsToCertificates(formats []linkedca.ACMEProvisioner_AttestationFormatType) []provisioner.ACMEAttestationFormat { + ret := make([]provisioner.ACMEAttestationFormat, 0, len(formats)) + for _, f := range formats { + switch f { + case linkedca.ACMEProvisioner_APPLE: + ret = append(ret, provisioner.APPLE) + case linkedca.ACMEProvisioner_STEP: + ret = append(ret, provisioner.STEP) + case linkedca.ACMEProvisioner_TPM: + ret = append(ret, provisioner.TPM) + } + } + return ret +} + +// attestationFormatsToLinkedca converts provisioner attestation formats to +// linkedca ones skipping the unknown ones. +func attestationFormatsToLinkedca(formats []provisioner.ACMEAttestationFormat) []linkedca.ACMEProvisioner_AttestationFormatType { + ret := make([]linkedca.ACMEProvisioner_AttestationFormatType, 0, len(formats)) + for _, f := range formats { + switch provisioner.ACMEAttestationFormat(f.String()) { + case provisioner.APPLE: + ret = append(ret, linkedca.ACMEProvisioner_APPLE) + case provisioner.STEP: + ret = append(ret, linkedca.ACMEProvisioner_STEP) + case provisioner.TPM: + ret = append(ret, linkedca.ACMEProvisioner_TPM) + } + } + return ret +} diff --git a/go.mod b/go.mod index f2e828a8..a376e222 100644 --- a/go.mod +++ b/go.mod @@ -40,7 +40,7 @@ require ( go.mozilla.org/pkcs7 v0.0.0-20210826202110-33d05740a352 go.step.sm/cli-utils v0.7.4 go.step.sm/crypto v0.19.0 - go.step.sm/linkedca v0.18.1-0.20220824000236-47827c8eb300 + go.step.sm/linkedca v0.18.1-0.20220909002054-5b28651792cb golang.org/x/crypto v0.0.0-20211215153901-e495a2d5b3d3 golang.org/x/net v0.0.0-20220607020251-c690dde0001d golang.org/x/time v0.0.0-20210220033141-f8bda1e9f3ba // indirect diff --git a/go.sum b/go.sum index eeb4f32b..9cef9e1f 100644 --- a/go.sum +++ b/go.sum @@ -641,8 +641,8 @@ go.step.sm/cli-utils v0.7.4/go.mod h1:taSsY8haLmXoXM3ZkywIyRmVij/4Aj0fQbNTlJvv71 go.step.sm/crypto v0.9.0/go.mod h1:+CYG05Mek1YDqi5WK0ERc6cOpKly2i/a5aZmU1sfGj0= go.step.sm/crypto v0.19.0 h1:WxjUDeTDpuPZ1IR3v6c4jc6WdlQlS5IYYQBhfnG5uW0= go.step.sm/crypto v0.19.0/go.mod h1:qZ+pNU1nV+THwP7TPTNCRMRr9xrRURhETTAK7U5psfw= -go.step.sm/linkedca v0.18.1-0.20220824000236-47827c8eb300 h1:kDqCHUh4jqqqf+m5IXjFjlwsTXuIXpf5ciGKigqJH14= -go.step.sm/linkedca v0.18.1-0.20220824000236-47827c8eb300/go.mod h1:qSuYlIIhvPmA2+DSSS03E2IXhbXWTLW61Xh9zDQJ3VM= +go.step.sm/linkedca v0.18.1-0.20220909002054-5b28651792cb h1:YxFSzM8+nWsiAbi9tOmXRcY1LJizDTKLDa+grJp6n+8= +go.step.sm/linkedca v0.18.1-0.20220909002054-5b28651792cb/go.mod h1:qSuYlIIhvPmA2+DSSS03E2IXhbXWTLW61Xh9zDQJ3VM= go.uber.org/atomic v1.3.2/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE= go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE= go.uber.org/atomic v1.5.0/go.mod h1:sABNBOSYdrvTF6hTgEIbc7YasKWGhgEQZyfxyTvoXHQ= From 1e098aef5b0374d97d41a26b8414b7a533a18cdb Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Fri, 9 Sep 2022 10:57:32 -0700 Subject: [PATCH 5/6] Fixes ACMEAttestationFormat comment --- authority/provisioner/acme.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/authority/provisioner/acme.go b/authority/provisioner/acme.go index f86ab07d..33fa351c 100644 --- a/authority/provisioner/acme.go +++ b/authority/provisioner/acme.go @@ -41,7 +41,8 @@ func (c ACMEChallenge) Validate() error { } } -// ACMEPlatform represents the format used on a device-attest-01 challenge. +// ACMEAttestationFormat represents the format used on a device-attest-01 +// challenge. type ACMEAttestationFormat string const ( From bb0210e87526f16a2daeefe336067f321a899358 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Fri, 9 Sep 2022 14:34:32 -0700 Subject: [PATCH 6/6] Fix typo in linkedca variable --- authority/provisioners.go | 4 ++-- go.mod | 2 +- go.sum | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/authority/provisioners.go b/authority/provisioners.go index 1e5ddda5..dcf8de36 100644 --- a/authority/provisioners.go +++ b/authority/provisioners.go @@ -1137,7 +1137,7 @@ func challengesToCertificates(challenges []linkedca.ACMEProvisioner_ChallengeTyp ret = append(ret, provisioner.HTTP_01) case linkedca.ACMEProvisioner_DNS_01: ret = append(ret, provisioner.DNS_01) - case linkedca.ACMEProvisioner_TLS_ALPN_O1: + case linkedca.ACMEProvisioner_TLS_ALPN_01: ret = append(ret, provisioner.TLS_ALPN_01) case linkedca.ACMEProvisioner_DEVICE_ATTEST_01: ret = append(ret, provisioner.DEVICE_ATTEST_01) @@ -1157,7 +1157,7 @@ func challengesToLinkedca(challenges []provisioner.ACMEChallenge) []linkedca.ACM case provisioner.DNS_01: ret = append(ret, linkedca.ACMEProvisioner_DNS_01) case provisioner.TLS_ALPN_01: - ret = append(ret, linkedca.ACMEProvisioner_TLS_ALPN_O1) + ret = append(ret, linkedca.ACMEProvisioner_TLS_ALPN_01) case provisioner.DEVICE_ATTEST_01: ret = append(ret, linkedca.ACMEProvisioner_DEVICE_ATTEST_01) } diff --git a/go.mod b/go.mod index a376e222..159c5b6b 100644 --- a/go.mod +++ b/go.mod @@ -40,7 +40,7 @@ require ( go.mozilla.org/pkcs7 v0.0.0-20210826202110-33d05740a352 go.step.sm/cli-utils v0.7.4 go.step.sm/crypto v0.19.0 - go.step.sm/linkedca v0.18.1-0.20220909002054-5b28651792cb + go.step.sm/linkedca v0.18.1-0.20220909212924-c69cf68797cb golang.org/x/crypto v0.0.0-20211215153901-e495a2d5b3d3 golang.org/x/net v0.0.0-20220607020251-c690dde0001d golang.org/x/time v0.0.0-20210220033141-f8bda1e9f3ba // indirect diff --git a/go.sum b/go.sum index 9cef9e1f..eabffc39 100644 --- a/go.sum +++ b/go.sum @@ -641,8 +641,8 @@ go.step.sm/cli-utils v0.7.4/go.mod h1:taSsY8haLmXoXM3ZkywIyRmVij/4Aj0fQbNTlJvv71 go.step.sm/crypto v0.9.0/go.mod h1:+CYG05Mek1YDqi5WK0ERc6cOpKly2i/a5aZmU1sfGj0= go.step.sm/crypto v0.19.0 h1:WxjUDeTDpuPZ1IR3v6c4jc6WdlQlS5IYYQBhfnG5uW0= go.step.sm/crypto v0.19.0/go.mod h1:qZ+pNU1nV+THwP7TPTNCRMRr9xrRURhETTAK7U5psfw= -go.step.sm/linkedca v0.18.1-0.20220909002054-5b28651792cb h1:YxFSzM8+nWsiAbi9tOmXRcY1LJizDTKLDa+grJp6n+8= -go.step.sm/linkedca v0.18.1-0.20220909002054-5b28651792cb/go.mod h1:qSuYlIIhvPmA2+DSSS03E2IXhbXWTLW61Xh9zDQJ3VM= +go.step.sm/linkedca v0.18.1-0.20220909212924-c69cf68797cb h1:qCG7i7PAZcTDLqyFmOzBBl5tfyHI033U5jONS9DuN+8= +go.step.sm/linkedca v0.18.1-0.20220909212924-c69cf68797cb/go.mod h1:qSuYlIIhvPmA2+DSSS03E2IXhbXWTLW61Xh9zDQJ3VM= go.uber.org/atomic v1.3.2/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE= go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE= go.uber.org/atomic v1.5.0/go.mod h1:sABNBOSYdrvTF6hTgEIbc7YasKWGhgEQZyfxyTvoXHQ=