From 2a7620641f74474a7a055e20c779e6c70293c32d Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Tue, 26 Apr 2022 10:15:17 +0200 Subject: [PATCH] Fix more PR comments --- acme/account.go | 6 +- acme/account_test.go | 9 +- acme/api/account_test.go | 4 +- acme/api/eab.go | 4 +- acme/api/eab_test.go | 18 +- acme/db/nosql/eab.go | 13 +- acme/db/nosql/eab_test.go | 59 ++--- authority/admin/api/acme.go | 4 +- authority/admin/api/acme_test.go | 10 +- authority/admin/api/policy.go | 28 +- authority/admin/api/policy_test.go | 359 ++++++++++++-------------- authority/linkedca.go | 7 + authority/policy.go | 5 +- authority/policy/options.go | 20 +- authority/policy/options_test.go | 6 +- authority/policy/policy.go | 2 +- authority/policy_test.go | 20 +- authority/provisioner/options.go | 19 +- authority/provisioner/options_test.go | 32 +-- authority/tls_test.go | 2 +- ca/ca.go | 2 +- 21 files changed, 298 insertions(+), 331 deletions(-) diff --git a/acme/account.go b/acme/account.go index 18c0d646..b83defe1 100644 --- a/acme/account.go +++ b/acme/account.go @@ -91,7 +91,7 @@ func (p *Policy) IsWildcardLiteralAllowed() bool { // ShouldVerifySubjectCommonName returns true by default // for ACME account policies, as this is embedded in the // protocol. -func (p *Policy) ShouldVerifySubjectCommonName() bool { +func (p *Policy) ShouldVerifyCommonName() bool { return true } @@ -101,7 +101,7 @@ type ExternalAccountKey struct { ProvisionerID string `json:"provisionerID"` Reference string `json:"reference"` AccountID string `json:"-"` - KeyBytes []byte `json:"-"` + HmacKey []byte `json:"-"` CreatedAt time.Time `json:"createdAt"` BoundAt time.Time `json:"boundAt,omitempty"` Policy *Policy `json:"policy,omitempty"` @@ -121,6 +121,6 @@ func (eak *ExternalAccountKey) BindTo(account *Account) error { } eak.AccountID = account.ID eak.BoundAt = time.Now() - eak.KeyBytes = []byte{} // clearing the key bytes; can only be used once + eak.HmacKey = []byte{} // clearing the key bytes; can only be used once return nil } diff --git a/acme/account_test.go b/acme/account_test.go index 33524d87..edd1f5b0 100644 --- a/acme/account_test.go +++ b/acme/account_test.go @@ -7,8 +7,9 @@ import ( "time" "github.com/pkg/errors" - "github.com/smallstep/assert" "go.step.sm/crypto/jose" + + "github.com/smallstep/assert" ) func TestKeyToID(t *testing.T) { @@ -95,7 +96,7 @@ func TestExternalAccountKey_BindTo(t *testing.T) { ID: "eakID", ProvisionerID: "provID", Reference: "ref", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, }, acct: &Account{ ID: "accountID", @@ -108,7 +109,7 @@ func TestExternalAccountKey_BindTo(t *testing.T) { ID: "eakID", ProvisionerID: "provID", Reference: "ref", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, AccountID: "someAccountID", BoundAt: boundAt, }, @@ -138,7 +139,7 @@ func TestExternalAccountKey_BindTo(t *testing.T) { assert.Equals(t, ae.Subproblems, tt.err.Subproblems) } else { assert.Equals(t, eak.AccountID, acct.ID) - assert.Equals(t, eak.KeyBytes, []byte{}) + assert.Equals(t, eak.HmacKey, []byte{}) assert.NotNil(t, eak.BoundAt) } }) diff --git a/acme/api/account_test.go b/acme/api/account_test.go index e389b57f..a0161cb4 100644 --- a/acme/api/account_test.go +++ b/acme/api/account_test.go @@ -582,7 +582,7 @@ func TestHandler_NewAccount(t *testing.T) { ID: "eakID", ProvisionerID: provID, Reference: "testeak", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: time.Now(), } return test{ @@ -759,7 +759,7 @@ func TestHandler_NewAccount(t *testing.T) { ID: "eakID", ProvisionerID: provID, Reference: "testeak", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: time.Now(), }, nil }, diff --git a/acme/api/eab.go b/acme/api/eab.go index 6be906d4..84be6453 100644 --- a/acme/api/eab.go +++ b/acme/api/eab.go @@ -60,7 +60,7 @@ func (h *Handler) validateExternalAccountBinding(ctx context.Context, nar *NewAc return nil, acme.NewError(acme.ErrorUnauthorizedType, "the field 'kid' references an unknown key") } - if len(externalAccountKey.KeyBytes) == 0 { + if len(externalAccountKey.HmacKey) == 0 { return nil, acme.NewError(acme.ErrorServerInternalType, "external account binding key with id '%s' does not have secret bytes", keyID) } @@ -68,7 +68,7 @@ func (h *Handler) validateExternalAccountBinding(ctx context.Context, nar *NewAc return nil, acme.NewError(acme.ErrorUnauthorizedType, "external account binding key with id '%s' was already bound to account '%s' on %s", keyID, externalAccountKey.AccountID, externalAccountKey.BoundAt) } - payload, err := eabJWS.Verify(externalAccountKey.KeyBytes) + payload, err := eabJWS.Verify(externalAccountKey.HmacKey) if err != nil { return nil, acme.WrapErrorISE(err, "error verifying externalAccountBinding signature") } diff --git a/acme/api/eab_test.go b/acme/api/eab_test.go index 760c122c..c2725588 100644 --- a/acme/api/eab_test.go +++ b/acme/api/eab_test.go @@ -156,7 +156,7 @@ func TestHandler_validateExternalAccountBinding(t *testing.T) { ID: "eakID", ProvisionerID: provID, Reference: "testeak", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: createdAt, }, nil }, @@ -170,7 +170,7 @@ func TestHandler_validateExternalAccountBinding(t *testing.T) { ID: "eakID", ProvisionerID: provID, Reference: "testeak", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: createdAt, }, err: nil, @@ -523,7 +523,7 @@ func TestHandler_validateExternalAccountBinding(t *testing.T) { Reference: "testeak", CreatedAt: createdAt, AccountID: "some-account-id", - KeyBytes: []byte{}, + HmacKey: []byte{}, }, nil }, }, @@ -630,7 +630,7 @@ func TestHandler_validateExternalAccountBinding(t *testing.T) { Reference: "testeak", CreatedAt: createdAt, AccountID: "some-account-id", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, BoundAt: boundAt, }, nil }, @@ -686,7 +686,7 @@ func TestHandler_validateExternalAccountBinding(t *testing.T) { ID: "eakID", ProvisionerID: provID, Reference: "testeak", - KeyBytes: []byte{1, 2, 3, 4}, + HmacKey: []byte{1, 2, 3, 4}, CreatedAt: time.Now(), }, nil }, @@ -744,7 +744,7 @@ func TestHandler_validateExternalAccountBinding(t *testing.T) { ID: "eakID", ProvisionerID: provID, Reference: "testeak", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: time.Now(), }, nil }, @@ -799,7 +799,7 @@ func TestHandler_validateExternalAccountBinding(t *testing.T) { ID: "eakID", ProvisionerID: provID, Reference: "testeak", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: time.Now(), }, nil }, @@ -855,7 +855,7 @@ func TestHandler_validateExternalAccountBinding(t *testing.T) { ID: "eakID", ProvisionerID: provID, Reference: "testeak", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: time.Now(), }, nil }, @@ -898,7 +898,7 @@ func TestHandler_validateExternalAccountBinding(t *testing.T) { } else { assert.NotNil(t, tc.eak) assert.Equals(t, got.ID, tc.eak.ID) - assert.Equals(t, got.KeyBytes, tc.eak.KeyBytes) + assert.Equals(t, got.HmacKey, tc.eak.HmacKey) assert.Equals(t, got.ProvisionerID, tc.eak.ProvisionerID) assert.Equals(t, got.Reference, tc.eak.Reference) assert.Equals(t, got.CreatedAt, tc.eak.CreatedAt) diff --git a/acme/db/nosql/eab.go b/acme/db/nosql/eab.go index 5c34c20c..e87aa9bc 100644 --- a/acme/db/nosql/eab.go +++ b/acme/db/nosql/eab.go @@ -8,6 +8,7 @@ import ( "time" "github.com/pkg/errors" + "github.com/smallstep/certificates/acme" nosqlDB "github.com/smallstep/nosql" ) @@ -23,7 +24,7 @@ type dbExternalAccountKey struct { ProvisionerID string `json:"provisionerID"` Reference string `json:"reference"` AccountID string `json:"accountID,omitempty"` - KeyBytes []byte `json:"key"` + HmacKey []byte `json:"key"` CreatedAt time.Time `json:"createdAt"` BoundAt time.Time `json:"boundAt"` } @@ -72,7 +73,7 @@ func (db *DB) CreateExternalAccountKey(ctx context.Context, provisionerID, refer ID: keyID, ProvisionerID: provisionerID, Reference: reference, - KeyBytes: random, + HmacKey: random, CreatedAt: clock.Now(), } @@ -99,7 +100,7 @@ func (db *DB) CreateExternalAccountKey(ctx context.Context, provisionerID, refer ProvisionerID: dbeak.ProvisionerID, Reference: dbeak.Reference, AccountID: dbeak.AccountID, - KeyBytes: dbeak.KeyBytes, + HmacKey: dbeak.HmacKey, CreatedAt: dbeak.CreatedAt, BoundAt: dbeak.BoundAt, }, nil @@ -124,7 +125,7 @@ func (db *DB) GetExternalAccountKey(ctx context.Context, provisionerID, keyID st ProvisionerID: dbeak.ProvisionerID, Reference: dbeak.Reference, AccountID: dbeak.AccountID, - KeyBytes: dbeak.KeyBytes, + HmacKey: dbeak.HmacKey, CreatedAt: dbeak.CreatedAt, BoundAt: dbeak.BoundAt, }, nil @@ -191,7 +192,7 @@ func (db *DB) GetExternalAccountKeys(ctx context.Context, provisionerID, cursor } keys = append(keys, &acme.ExternalAccountKey{ ID: eak.ID, - KeyBytes: eak.KeyBytes, + HmacKey: eak.HmacKey, ProvisionerID: eak.ProvisionerID, Reference: eak.Reference, AccountID: eak.AccountID, @@ -256,7 +257,7 @@ func (db *DB) UpdateExternalAccountKey(ctx context.Context, provisionerID string ProvisionerID: eak.ProvisionerID, Reference: eak.Reference, AccountID: eak.AccountID, - KeyBytes: eak.KeyBytes, + HmacKey: eak.HmacKey, CreatedAt: eak.CreatedAt, BoundAt: eak.BoundAt, } diff --git a/acme/db/nosql/eab_test.go b/acme/db/nosql/eab_test.go index 568500e9..525afa72 100644 --- a/acme/db/nosql/eab_test.go +++ b/acme/db/nosql/eab_test.go @@ -8,6 +8,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/pkg/errors" + "github.com/smallstep/assert" "github.com/smallstep/certificates/acme" certdb "github.com/smallstep/certificates/db" @@ -32,7 +33,7 @@ func TestDB_getDBExternalAccountKey(t *testing.T) { ProvisionerID: provID, Reference: "ref", AccountID: "", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: now, } b, err := json.Marshal(dbeak) @@ -108,7 +109,7 @@ func TestDB_getDBExternalAccountKey(t *testing.T) { } } else if assert.Nil(t, tc.err) { assert.Equals(t, dbeak.ID, tc.dbeak.ID) - assert.Equals(t, dbeak.KeyBytes, tc.dbeak.KeyBytes) + assert.Equals(t, dbeak.HmacKey, tc.dbeak.HmacKey) assert.Equals(t, dbeak.ProvisionerID, tc.dbeak.ProvisionerID) assert.Equals(t, dbeak.Reference, tc.dbeak.Reference) assert.Equals(t, dbeak.CreatedAt, tc.dbeak.CreatedAt) @@ -136,7 +137,7 @@ func TestDB_GetExternalAccountKey(t *testing.T) { ProvisionerID: provID, Reference: "ref", AccountID: "", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: now, } b, err := json.Marshal(dbeak) @@ -154,7 +155,7 @@ func TestDB_GetExternalAccountKey(t *testing.T) { ProvisionerID: provID, Reference: "ref", AccountID: "", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: now, }, } @@ -179,7 +180,7 @@ func TestDB_GetExternalAccountKey(t *testing.T) { ProvisionerID: "aDifferentProvID", Reference: "ref", AccountID: "", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: now, } b, err := json.Marshal(dbeak) @@ -197,7 +198,7 @@ func TestDB_GetExternalAccountKey(t *testing.T) { ProvisionerID: provID, Reference: "ref", AccountID: "", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: now, }, acmeErr: acme.NewError(acme.ErrorUnauthorizedType, "provisioner does not match provisioner for which the EAB key was created"), @@ -225,7 +226,7 @@ func TestDB_GetExternalAccountKey(t *testing.T) { } } else if assert.Nil(t, tc.err) { assert.Equals(t, eak.ID, tc.eak.ID) - assert.Equals(t, eak.KeyBytes, tc.eak.KeyBytes) + assert.Equals(t, eak.HmacKey, tc.eak.HmacKey) assert.Equals(t, eak.ProvisionerID, tc.eak.ProvisionerID) assert.Equals(t, eak.Reference, tc.eak.Reference) assert.Equals(t, eak.CreatedAt, tc.eak.CreatedAt) @@ -255,7 +256,7 @@ func TestDB_GetExternalAccountKeyByReference(t *testing.T) { ProvisionerID: provID, Reference: ref, AccountID: "", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: now, } dbref := &dbExternalAccountKeyReference{ @@ -288,7 +289,7 @@ func TestDB_GetExternalAccountKeyByReference(t *testing.T) { ProvisionerID: provID, Reference: ref, AccountID: "", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: now, }, err: nil, @@ -392,7 +393,7 @@ func TestDB_GetExternalAccountKeyByReference(t *testing.T) { assert.Equals(t, eak.AccountID, tc.eak.AccountID) assert.Equals(t, eak.BoundAt, tc.eak.BoundAt) assert.Equals(t, eak.CreatedAt, tc.eak.CreatedAt) - assert.Equals(t, eak.KeyBytes, tc.eak.KeyBytes) + assert.Equals(t, eak.HmacKey, tc.eak.HmacKey) assert.Equals(t, eak.ProvisionerID, tc.eak.ProvisionerID) assert.Equals(t, eak.Reference, tc.eak.Reference) } @@ -420,7 +421,7 @@ func TestDB_GetExternalAccountKeys(t *testing.T) { ProvisionerID: provID, Reference: ref, AccountID: "", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: now, } b1, err := json.Marshal(dbeak1) @@ -430,7 +431,7 @@ func TestDB_GetExternalAccountKeys(t *testing.T) { ProvisionerID: provID, Reference: ref, AccountID: "", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: now, } b2, err := json.Marshal(dbeak2) @@ -440,7 +441,7 @@ func TestDB_GetExternalAccountKeys(t *testing.T) { ProvisionerID: "aDifferentProvID", Reference: ref, AccountID: "", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: now, } b3, err := json.Marshal(dbeak3) @@ -513,7 +514,7 @@ func TestDB_GetExternalAccountKeys(t *testing.T) { ProvisionerID: provID, Reference: ref, AccountID: "", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: now, }, { @@ -521,7 +522,7 @@ func TestDB_GetExternalAccountKeys(t *testing.T) { ProvisionerID: provID, Reference: ref, AccountID: "", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: now, }, }, @@ -598,7 +599,7 @@ func TestDB_GetExternalAccountKeys(t *testing.T) { assert.Equals(t, "", nextCursor) for i, eak := range eaks { assert.Equals(t, eak.ID, tc.eaks[i].ID) - assert.Equals(t, eak.KeyBytes, tc.eaks[i].KeyBytes) + assert.Equals(t, eak.HmacKey, tc.eaks[i].HmacKey) assert.Equals(t, eak.ProvisionerID, tc.eaks[i].ProvisionerID) assert.Equals(t, eak.Reference, tc.eaks[i].Reference) assert.Equals(t, eak.CreatedAt, tc.eaks[i].CreatedAt) @@ -627,7 +628,7 @@ func TestDB_DeleteExternalAccountKey(t *testing.T) { ProvisionerID: provID, Reference: ref, AccountID: "", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: now, } dbref := &dbExternalAccountKeyReference{ @@ -707,7 +708,7 @@ func TestDB_DeleteExternalAccountKey(t *testing.T) { ProvisionerID: "aDifferentProvID", Reference: ref, AccountID: "", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: now, } b, err := json.Marshal(dbeak) @@ -730,7 +731,7 @@ func TestDB_DeleteExternalAccountKey(t *testing.T) { ProvisionerID: provID, Reference: ref, AccountID: "", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: now, } dbref := &dbExternalAccountKeyReference{ @@ -780,7 +781,7 @@ func TestDB_DeleteExternalAccountKey(t *testing.T) { ProvisionerID: provID, Reference: ref, AccountID: "", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: now, } dbref := &dbExternalAccountKeyReference{ @@ -830,7 +831,7 @@ func TestDB_DeleteExternalAccountKey(t *testing.T) { ProvisionerID: provID, Reference: ref, AccountID: "", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: now, } dbref := &dbExternalAccountKeyReference{ @@ -953,7 +954,7 @@ func TestDB_CreateExternalAccountKey(t *testing.T) { assert.Equals(t, string(key), dbeak.ID) assert.Equals(t, eak.ProvisionerID, dbeak.ProvisionerID) assert.Equals(t, eak.Reference, dbeak.Reference) - assert.Equals(t, 32, len(dbeak.KeyBytes)) + assert.Equals(t, 32, len(dbeak.HmacKey)) assert.False(t, dbeak.CreatedAt.IsZero()) assert.Equals(t, dbeak.AccountID, eak.AccountID) assert.True(t, dbeak.BoundAt.IsZero()) @@ -1078,7 +1079,7 @@ func TestDB_UpdateExternalAccountKey(t *testing.T) { ProvisionerID: provID, Reference: ref, AccountID: "", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: now, } b, err := json.Marshal(dbeak) @@ -1096,7 +1097,7 @@ func TestDB_UpdateExternalAccountKey(t *testing.T) { ProvisionerID: provID, Reference: ref, AccountID: "", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: now, } return test{ @@ -1120,7 +1121,7 @@ func TestDB_UpdateExternalAccountKey(t *testing.T) { assert.Equals(t, dbNew.AccountID, dbeak.AccountID) assert.Equals(t, dbNew.CreatedAt, dbeak.CreatedAt) assert.Equals(t, dbNew.BoundAt, dbeak.BoundAt) - assert.Equals(t, dbNew.KeyBytes, dbeak.KeyBytes) + assert.Equals(t, dbNew.HmacKey, dbeak.HmacKey) return nu, true, nil }, }, @@ -1148,7 +1149,7 @@ func TestDB_UpdateExternalAccountKey(t *testing.T) { ProvisionerID: "aDifferentProvID", Reference: ref, AccountID: "", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: now, } b, err := json.Marshal(newDBEAK) @@ -1174,7 +1175,7 @@ func TestDB_UpdateExternalAccountKey(t *testing.T) { ProvisionerID: provID, Reference: ref, AccountID: "", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: now, } b, err := json.Marshal(newDBEAK) @@ -1200,7 +1201,7 @@ func TestDB_UpdateExternalAccountKey(t *testing.T) { ProvisionerID: provID, Reference: ref, AccountID: "", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: now, } b, err := json.Marshal(newDBEAK) @@ -1237,7 +1238,7 @@ func TestDB_UpdateExternalAccountKey(t *testing.T) { assert.Equals(t, dbeak.AccountID, tc.eak.AccountID) assert.Equals(t, dbeak.CreatedAt, tc.eak.CreatedAt) assert.Equals(t, dbeak.BoundAt, tc.eak.BoundAt) - assert.Equals(t, dbeak.KeyBytes, tc.eak.KeyBytes) + assert.Equals(t, dbeak.HmacKey, tc.eak.HmacKey) } }) } diff --git a/authority/admin/api/acme.go b/authority/admin/api/acme.go index cd4b1e17..026443fa 100644 --- a/authority/admin/api/acme.go +++ b/authority/admin/api/acme.go @@ -90,7 +90,7 @@ func eakToLinked(k *acme.ExternalAccountKey) *linkedca.EABKey { eak := &linkedca.EABKey{ Id: k.ID, - HmacKey: k.KeyBytes, + HmacKey: k.HmacKey, Provisioner: k.ProvisionerID, Reference: k.Reference, Account: k.AccountID, @@ -124,7 +124,7 @@ func linkedEAKToCertificates(k *linkedca.EABKey) *acme.ExternalAccountKey { ProvisionerID: k.Provisioner, Reference: k.Reference, AccountID: k.Account, - KeyBytes: k.HmacKey, + HmacKey: k.HmacKey, CreatedAt: k.CreatedAt.AsTime(), BoundAt: k.BoundAt.AsTime(), } diff --git a/authority/admin/api/acme_test.go b/authority/admin/api/acme_test.go index aa4aa608..5094d5f0 100644 --- a/authority/admin/api/acme_test.go +++ b/authority/admin/api/acme_test.go @@ -364,7 +364,7 @@ func Test_eakToLinked(t *testing.T) { ProvisionerID: "provID", Reference: "ref", AccountID: "accID", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: time.Date(2022, 04, 12, 9, 30, 30, 0, time.UTC).Add(-1 * time.Hour), BoundAt: time.Date(2022, 04, 12, 9, 30, 30, 0, time.UTC), Policy: nil, @@ -387,7 +387,7 @@ func Test_eakToLinked(t *testing.T) { ProvisionerID: "provID", Reference: "ref", AccountID: "accID", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: time.Date(2022, 04, 12, 9, 30, 30, 0, time.UTC).Add(-1 * time.Hour), BoundAt: time.Date(2022, 04, 12, 9, 30, 30, 0, time.UTC), Policy: &acme.Policy{ @@ -463,7 +463,7 @@ func Test_linkedEAKToCertificates(t *testing.T) { ProvisionerID: "provID", Reference: "ref", AccountID: "accID", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: time.Date(2022, 04, 12, 9, 30, 30, 0, time.UTC).Add(-1 * time.Hour), BoundAt: time.Date(2022, 04, 12, 9, 30, 30, 0, time.UTC), Policy: nil, @@ -486,7 +486,7 @@ func Test_linkedEAKToCertificates(t *testing.T) { ProvisionerID: "provID", Reference: "ref", AccountID: "accID", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: time.Date(2022, 04, 12, 9, 30, 30, 0, time.UTC).Add(-1 * time.Hour), BoundAt: time.Date(2022, 04, 12, 9, 30, 30, 0, time.UTC), Policy: &acme.Policy{}, @@ -520,7 +520,7 @@ func Test_linkedEAKToCertificates(t *testing.T) { ProvisionerID: "provID", Reference: "ref", AccountID: "accID", - KeyBytes: []byte{1, 3, 3, 7}, + HmacKey: []byte{1, 3, 3, 7}, CreatedAt: time.Date(2022, 04, 12, 9, 30, 30, 0, time.UTC).Add(-1 * time.Hour), BoundAt: time.Date(2022, 04, 12, 9, 30, 30, 0, time.UTC), Policy: &acme.Policy{ diff --git a/authority/admin/api/policy.go b/authority/admin/api/policy.go index c697b67a..70c6f01d 100644 --- a/authority/admin/api/policy.go +++ b/authority/admin/api/policy.go @@ -30,19 +30,25 @@ type policyAdminResponderInterface interface { // PolicyAdminResponder is responsible for writing ACME admin responses type PolicyAdminResponder struct { - auth adminAuthority - adminDB admin.DB - acmeDB acme.DB - deploymentType string + auth adminAuthority + adminDB admin.DB + acmeDB acme.DB + isLinkedCA bool } // NewACMEAdminResponder returns a new ACMEAdminResponder -func NewPolicyAdminResponder(auth adminAuthority, adminDB admin.DB, acmeDB acme.DB, deploymentType string) *PolicyAdminResponder { +func NewPolicyAdminResponder(auth adminAuthority, adminDB admin.DB, acmeDB acme.DB) *PolicyAdminResponder { + + var isLinkedCA bool + if a, ok := adminDB.(interface{ IsLinkedCA() bool }); ok { + isLinkedCA = a.IsLinkedCA() + } + return &PolicyAdminResponder{ - auth: auth, - adminDB: adminDB, - acmeDB: acmeDB, - deploymentType: deploymentType, + auth: auth, + adminDB: adminDB, + acmeDB: acmeDB, + isLinkedCA: isLinkedCA, } } @@ -435,8 +441,8 @@ func (par *PolicyAdminResponder) DeleteACMEAccountPolicy(w http.ResponseWriter, // blockLinkedCA blocks all API operations on linked deployments func (par *PolicyAdminResponder) blockLinkedCA() error { - // temporary blocking linked deployments based on string comparison (preventing import cycle) - if par.deploymentType == "linked" { + // temporary blocking linked deployments + if par.isLinkedCA { return admin.NewError(admin.ErrorNotImplementedType, "policy operations not yet supported in linked deployments") } return nil diff --git a/authority/admin/api/policy_test.go b/authority/admin/api/policy_test.go index ee97c2cc..fffa84f7 100644 --- a/authority/admin/api/policy_test.go +++ b/authority/admin/api/policy_test.go @@ -21,15 +21,22 @@ import ( "github.com/smallstep/certificates/authority/admin" ) +type fakeLinkedCA struct { + admin.MockDB +} + +func (f *fakeLinkedCA) IsLinkedCA() bool { + return true +} + func TestPolicyAdminResponder_GetAuthorityPolicy(t *testing.T) { type test struct { - auth adminAuthority - deploymentType string - adminDB admin.DB - ctx context.Context - err *admin.Error - policy *linkedca.Policy - statusCode int + auth adminAuthority + adminDB admin.DB + ctx context.Context + err *admin.Error + policy *linkedca.Policy + statusCode int } var tests = map[string]func(t *testing.T) test{ "fail/linkedca": func(t *testing.T) test { @@ -37,10 +44,10 @@ func TestPolicyAdminResponder_GetAuthorityPolicy(t *testing.T) { err := admin.NewError(admin.ErrorNotImplementedType, "policy operations not yet supported in linked deployments") err.Message = "policy operations not yet supported in linked deployments" return test{ - ctx: ctx, - deploymentType: "linked", - err: err, - statusCode: 501, + ctx: ctx, + adminDB: &fakeLinkedCA{}, + err: err, + statusCode: 501, } }, "fail/auth.GetAuthorityPolicy-error": func(t *testing.T) test { @@ -97,11 +104,8 @@ func TestPolicyAdminResponder_GetAuthorityPolicy(t *testing.T) { for name, prep := range tests { tc := prep(t) t.Run(name, func(t *testing.T) { - par := &PolicyAdminResponder{ - auth: tc.auth, - adminDB: tc.adminDB, - deploymentType: tc.deploymentType, - } + + par := NewPolicyAdminResponder(tc.auth, tc.adminDB, nil) req := httptest.NewRequest("GET", "/foo", nil) req = req.WithContext(tc.ctx) @@ -139,15 +143,14 @@ func TestPolicyAdminResponder_GetAuthorityPolicy(t *testing.T) { func TestPolicyAdminResponder_CreateAuthorityPolicy(t *testing.T) { type test struct { - auth adminAuthority - deploymentType string - adminDB admin.DB - body []byte - ctx context.Context - acmeDB acme.DB - err *admin.Error - policy *linkedca.Policy - statusCode int + auth adminAuthority + adminDB admin.DB + body []byte + ctx context.Context + acmeDB acme.DB + err *admin.Error + policy *linkedca.Policy + statusCode int } var tests = map[string]func(t *testing.T) test{ "fail/linkedca": func(t *testing.T) test { @@ -155,10 +158,10 @@ func TestPolicyAdminResponder_CreateAuthorityPolicy(t *testing.T) { err := admin.NewError(admin.ErrorNotImplementedType, "policy operations not yet supported in linked deployments") err.Message = "policy operations not yet supported in linked deployments" return test{ - ctx: ctx, - deploymentType: "linked", - err: err, - statusCode: 501, + ctx: ctx, + adminDB: &fakeLinkedCA{}, + err: err, + statusCode: 501, } }, "fail/auth.GetAuthorityPolicy-error": func(t *testing.T) test { @@ -343,12 +346,8 @@ func TestPolicyAdminResponder_CreateAuthorityPolicy(t *testing.T) { for name, prep := range tests { tc := prep(t) t.Run(name, func(t *testing.T) { - par := &PolicyAdminResponder{ - auth: tc.auth, - adminDB: tc.adminDB, - acmeDB: tc.acmeDB, - deploymentType: tc.deploymentType, - } + + par := NewPolicyAdminResponder(tc.auth, tc.adminDB, tc.acmeDB) req := httptest.NewRequest("POST", "/foo", io.NopCloser(bytes.NewBuffer(tc.body))) req = req.WithContext(tc.ctx) @@ -395,15 +394,14 @@ func TestPolicyAdminResponder_CreateAuthorityPolicy(t *testing.T) { func TestPolicyAdminResponder_UpdateAuthorityPolicy(t *testing.T) { type test struct { - auth adminAuthority - deploymentType string - adminDB admin.DB - body []byte - ctx context.Context - acmeDB acme.DB - err *admin.Error - policy *linkedca.Policy - statusCode int + auth adminAuthority + adminDB admin.DB + body []byte + ctx context.Context + acmeDB acme.DB + err *admin.Error + policy *linkedca.Policy + statusCode int } var tests = map[string]func(t *testing.T) test{ "fail/linkedca": func(t *testing.T) test { @@ -411,10 +409,10 @@ func TestPolicyAdminResponder_UpdateAuthorityPolicy(t *testing.T) { err := admin.NewError(admin.ErrorNotImplementedType, "policy operations not yet supported in linked deployments") err.Message = "policy operations not yet supported in linked deployments" return test{ - ctx: ctx, - deploymentType: "linked", - err: err, - statusCode: 501, + ctx: ctx, + adminDB: &fakeLinkedCA{}, + err: err, + statusCode: 501, } }, "fail/auth.GetAuthorityPolicy-error": func(t *testing.T) test { @@ -606,12 +604,8 @@ func TestPolicyAdminResponder_UpdateAuthorityPolicy(t *testing.T) { for name, prep := range tests { tc := prep(t) t.Run(name, func(t *testing.T) { - par := &PolicyAdminResponder{ - auth: tc.auth, - adminDB: tc.adminDB, - acmeDB: tc.acmeDB, - deploymentType: tc.deploymentType, - } + + par := NewPolicyAdminResponder(tc.auth, tc.adminDB, tc.acmeDB) req := httptest.NewRequest("POST", "/foo", io.NopCloser(bytes.NewBuffer(tc.body))) req = req.WithContext(tc.ctx) @@ -658,14 +652,13 @@ func TestPolicyAdminResponder_UpdateAuthorityPolicy(t *testing.T) { func TestPolicyAdminResponder_DeleteAuthorityPolicy(t *testing.T) { type test struct { - auth adminAuthority - deploymentType string - adminDB admin.DB - body []byte - ctx context.Context - acmeDB acme.DB - err *admin.Error - statusCode int + auth adminAuthority + adminDB admin.DB + body []byte + ctx context.Context + acmeDB acme.DB + err *admin.Error + statusCode int } var tests = map[string]func(t *testing.T) test{ @@ -674,10 +667,10 @@ func TestPolicyAdminResponder_DeleteAuthorityPolicy(t *testing.T) { err := admin.NewError(admin.ErrorNotImplementedType, "policy operations not yet supported in linked deployments") err.Message = "policy operations not yet supported in linked deployments" return test{ - ctx: ctx, - deploymentType: "linked", - err: err, - statusCode: 501, + ctx: ctx, + adminDB: &fakeLinkedCA{}, + err: err, + statusCode: 501, } }, "fail/auth.GetAuthorityPolicy-error": func(t *testing.T) test { @@ -762,12 +755,8 @@ func TestPolicyAdminResponder_DeleteAuthorityPolicy(t *testing.T) { for name, prep := range tests { tc := prep(t) t.Run(name, func(t *testing.T) { - par := &PolicyAdminResponder{ - auth: tc.auth, - adminDB: tc.adminDB, - acmeDB: tc.acmeDB, - deploymentType: tc.deploymentType, - } + + par := NewPolicyAdminResponder(tc.auth, tc.adminDB, tc.acmeDB) req := httptest.NewRequest("POST", "/foo", io.NopCloser(bytes.NewBuffer(tc.body))) req = req.WithContext(tc.ctx) @@ -809,14 +798,13 @@ func TestPolicyAdminResponder_DeleteAuthorityPolicy(t *testing.T) { func TestPolicyAdminResponder_GetProvisionerPolicy(t *testing.T) { type test struct { - auth adminAuthority - deploymentType string - adminDB admin.DB - ctx context.Context - acmeDB acme.DB - err *admin.Error - policy *linkedca.Policy - statusCode int + auth adminAuthority + adminDB admin.DB + ctx context.Context + acmeDB acme.DB + err *admin.Error + policy *linkedca.Policy + statusCode int } var tests = map[string]func(t *testing.T) test{ "fail/linkedca": func(t *testing.T) test { @@ -824,10 +812,10 @@ func TestPolicyAdminResponder_GetProvisionerPolicy(t *testing.T) { err := admin.NewError(admin.ErrorNotImplementedType, "policy operations not yet supported in linked deployments") err.Message = "policy operations not yet supported in linked deployments" return test{ - ctx: ctx, - deploymentType: "linked", - err: err, - statusCode: 501, + ctx: ctx, + adminDB: &fakeLinkedCA{}, + err: err, + statusCode: 501, } }, "fail/prov-no-policy": func(t *testing.T) test { @@ -863,12 +851,8 @@ func TestPolicyAdminResponder_GetProvisionerPolicy(t *testing.T) { for name, prep := range tests { tc := prep(t) t.Run(name, func(t *testing.T) { - par := &PolicyAdminResponder{ - auth: tc.auth, - adminDB: tc.adminDB, - acmeDB: tc.acmeDB, - deploymentType: tc.deploymentType, - } + + par := NewPolicyAdminResponder(tc.auth, tc.adminDB, tc.acmeDB) req := httptest.NewRequest("GET", "/foo", nil) req = req.WithContext(tc.ctx) @@ -906,13 +890,13 @@ func TestPolicyAdminResponder_GetProvisionerPolicy(t *testing.T) { func TestPolicyAdminResponder_CreateProvisionerPolicy(t *testing.T) { type test struct { - auth adminAuthority - deploymentType string - body []byte - ctx context.Context - err *admin.Error - policy *linkedca.Policy - statusCode int + auth adminAuthority + adminDB admin.DB + body []byte + ctx context.Context + err *admin.Error + policy *linkedca.Policy + statusCode int } var tests = map[string]func(t *testing.T) test{ "fail/linkedca": func(t *testing.T) test { @@ -920,10 +904,10 @@ func TestPolicyAdminResponder_CreateProvisionerPolicy(t *testing.T) { err := admin.NewError(admin.ErrorNotImplementedType, "policy operations not yet supported in linked deployments") err.Message = "policy operations not yet supported in linked deployments" return test{ - ctx: ctx, - deploymentType: "linked", - err: err, - statusCode: 501, + ctx: ctx, + adminDB: &fakeLinkedCA{}, + err: err, + statusCode: 501, } }, "fail/existing-policy": func(t *testing.T) test { @@ -1067,10 +1051,8 @@ func TestPolicyAdminResponder_CreateProvisionerPolicy(t *testing.T) { for name, prep := range tests { tc := prep(t) t.Run(name, func(t *testing.T) { - par := &PolicyAdminResponder{ - auth: tc.auth, - deploymentType: tc.deploymentType, - } + + par := NewPolicyAdminResponder(tc.auth, tc.adminDB, nil) req := httptest.NewRequest("POST", "/foo", io.NopCloser(bytes.NewBuffer(tc.body))) req = req.WithContext(tc.ctx) @@ -1117,13 +1099,13 @@ func TestPolicyAdminResponder_CreateProvisionerPolicy(t *testing.T) { func TestPolicyAdminResponder_UpdateProvisionerPolicy(t *testing.T) { type test struct { - auth adminAuthority - deploymentType string - body []byte - ctx context.Context - err *admin.Error - policy *linkedca.Policy - statusCode int + auth adminAuthority + body []byte + adminDB admin.DB + ctx context.Context + err *admin.Error + policy *linkedca.Policy + statusCode int } var tests = map[string]func(t *testing.T) test{ "fail/linkedca": func(t *testing.T) test { @@ -1131,10 +1113,10 @@ func TestPolicyAdminResponder_UpdateProvisionerPolicy(t *testing.T) { err := admin.NewError(admin.ErrorNotImplementedType, "policy operations not yet supported in linked deployments") err.Message = "policy operations not yet supported in linked deployments" return test{ - ctx: ctx, - deploymentType: "linked", - err: err, - statusCode: 501, + ctx: ctx, + adminDB: &fakeLinkedCA{}, + err: err, + statusCode: 501, } }, "fail/no-existing-policy": func(t *testing.T) test { @@ -1280,10 +1262,8 @@ func TestPolicyAdminResponder_UpdateProvisionerPolicy(t *testing.T) { for name, prep := range tests { tc := prep(t) t.Run(name, func(t *testing.T) { - par := &PolicyAdminResponder{ - auth: tc.auth, - deploymentType: tc.deploymentType, - } + + par := NewPolicyAdminResponder(tc.auth, tc.adminDB, nil) req := httptest.NewRequest("POST", "/foo", io.NopCloser(bytes.NewBuffer(tc.body))) req = req.WithContext(tc.ctx) @@ -1330,14 +1310,13 @@ func TestPolicyAdminResponder_UpdateProvisionerPolicy(t *testing.T) { func TestPolicyAdminResponder_DeleteProvisionerPolicy(t *testing.T) { type test struct { - auth adminAuthority - deploymentType string - adminDB admin.DB - body []byte - ctx context.Context - acmeDB acme.DB - err *admin.Error - statusCode int + auth adminAuthority + adminDB admin.DB + body []byte + ctx context.Context + acmeDB acme.DB + err *admin.Error + statusCode int } var tests = map[string]func(t *testing.T) test{ @@ -1346,10 +1325,10 @@ func TestPolicyAdminResponder_DeleteProvisionerPolicy(t *testing.T) { err := admin.NewError(admin.ErrorNotImplementedType, "policy operations not yet supported in linked deployments") err.Message = "policy operations not yet supported in linked deployments" return test{ - ctx: ctx, - deploymentType: "linked", - err: err, - statusCode: 501, + ctx: ctx, + adminDB: &fakeLinkedCA{}, + err: err, + statusCode: 501, } }, "fail/no-existing-policy": func(t *testing.T) test { @@ -1404,12 +1383,8 @@ func TestPolicyAdminResponder_DeleteProvisionerPolicy(t *testing.T) { for name, prep := range tests { tc := prep(t) t.Run(name, func(t *testing.T) { - par := &PolicyAdminResponder{ - auth: tc.auth, - adminDB: tc.adminDB, - acmeDB: tc.acmeDB, - deploymentType: tc.deploymentType, - } + + par := NewPolicyAdminResponder(tc.auth, tc.adminDB, tc.acmeDB) req := httptest.NewRequest("POST", "/foo", io.NopCloser(bytes.NewBuffer(tc.body))) req = req.WithContext(tc.ctx) @@ -1451,12 +1426,12 @@ func TestPolicyAdminResponder_DeleteProvisionerPolicy(t *testing.T) { func TestPolicyAdminResponder_GetACMEAccountPolicy(t *testing.T) { type test struct { - deploymentType string - ctx context.Context - acmeDB acme.DB - err *admin.Error - policy *linkedca.Policy - statusCode int + ctx context.Context + acmeDB acme.DB + adminDB admin.DB + err *admin.Error + policy *linkedca.Policy + statusCode int } var tests = map[string]func(t *testing.T) test{ "fail/linkedca": func(t *testing.T) test { @@ -1464,10 +1439,10 @@ func TestPolicyAdminResponder_GetACMEAccountPolicy(t *testing.T) { err := admin.NewError(admin.ErrorNotImplementedType, "policy operations not yet supported in linked deployments") err.Message = "policy operations not yet supported in linked deployments" return test{ - ctx: ctx, - deploymentType: "linked", - err: err, - statusCode: 501, + ctx: ctx, + adminDB: &fakeLinkedCA{}, + err: err, + statusCode: 501, } }, "fail/no-policy": func(t *testing.T) test { @@ -1514,10 +1489,8 @@ func TestPolicyAdminResponder_GetACMEAccountPolicy(t *testing.T) { for name, prep := range tests { tc := prep(t) t.Run(name, func(t *testing.T) { - par := &PolicyAdminResponder{ - acmeDB: tc.acmeDB, - deploymentType: tc.deploymentType, - } + + par := NewPolicyAdminResponder(nil, tc.adminDB, tc.acmeDB) req := httptest.NewRequest("GET", "/foo", nil) req = req.WithContext(tc.ctx) @@ -1555,13 +1528,13 @@ func TestPolicyAdminResponder_GetACMEAccountPolicy(t *testing.T) { func TestPolicyAdminResponder_CreateACMEAccountPolicy(t *testing.T) { type test struct { - deploymentType string - acmeDB acme.DB - body []byte - ctx context.Context - err *admin.Error - policy *linkedca.Policy - statusCode int + acmeDB acme.DB + adminDB admin.DB + body []byte + ctx context.Context + err *admin.Error + policy *linkedca.Policy + statusCode int } var tests = map[string]func(t *testing.T) test{ "fail/linkedca": func(t *testing.T) test { @@ -1569,10 +1542,10 @@ func TestPolicyAdminResponder_CreateACMEAccountPolicy(t *testing.T) { err := admin.NewError(admin.ErrorNotImplementedType, "policy operations not yet supported in linked deployments") err.Message = "policy operations not yet supported in linked deployments" return test{ - ctx: ctx, - deploymentType: "linked", - err: err, - statusCode: 501, + ctx: ctx, + adminDB: &fakeLinkedCA{}, + err: err, + statusCode: 501, } }, "fail/existing-policy": func(t *testing.T) test { @@ -1691,10 +1664,8 @@ func TestPolicyAdminResponder_CreateACMEAccountPolicy(t *testing.T) { for name, prep := range tests { tc := prep(t) t.Run(name, func(t *testing.T) { - par := &PolicyAdminResponder{ - acmeDB: tc.acmeDB, - deploymentType: tc.deploymentType, - } + + par := NewPolicyAdminResponder(nil, tc.adminDB, tc.acmeDB) req := httptest.NewRequest("POST", "/foo", io.NopCloser(bytes.NewBuffer(tc.body))) req = req.WithContext(tc.ctx) @@ -1741,13 +1712,13 @@ func TestPolicyAdminResponder_CreateACMEAccountPolicy(t *testing.T) { func TestPolicyAdminResponder_UpdateACMEAccountPolicy(t *testing.T) { type test struct { - deploymentType string - acmeDB acme.DB - body []byte - ctx context.Context - err *admin.Error - policy *linkedca.Policy - statusCode int + acmeDB acme.DB + adminDB admin.DB + body []byte + ctx context.Context + err *admin.Error + policy *linkedca.Policy + statusCode int } var tests = map[string]func(t *testing.T) test{ "fail/linkedca": func(t *testing.T) test { @@ -1755,10 +1726,10 @@ func TestPolicyAdminResponder_UpdateACMEAccountPolicy(t *testing.T) { err := admin.NewError(admin.ErrorNotImplementedType, "policy operations not yet supported in linked deployments") err.Message = "policy operations not yet supported in linked deployments" return test{ - ctx: ctx, - deploymentType: "linked", - err: err, - statusCode: 501, + ctx: ctx, + adminDB: &fakeLinkedCA{}, + err: err, + statusCode: 501, } }, "fail/no-existing-policy": func(t *testing.T) test { @@ -1879,10 +1850,8 @@ func TestPolicyAdminResponder_UpdateACMEAccountPolicy(t *testing.T) { for name, prep := range tests { tc := prep(t) t.Run(name, func(t *testing.T) { - par := &PolicyAdminResponder{ - acmeDB: tc.acmeDB, - deploymentType: tc.deploymentType, - } + + par := NewPolicyAdminResponder(nil, tc.adminDB, tc.acmeDB) req := httptest.NewRequest("POST", "/foo", io.NopCloser(bytes.NewBuffer(tc.body))) req = req.WithContext(tc.ctx) @@ -1929,12 +1898,12 @@ func TestPolicyAdminResponder_UpdateACMEAccountPolicy(t *testing.T) { func TestPolicyAdminResponder_DeleteACMEAccountPolicy(t *testing.T) { type test struct { - deploymentType string - body []byte - ctx context.Context - acmeDB acme.DB - err *admin.Error - statusCode int + body []byte + adminDB admin.DB + ctx context.Context + acmeDB acme.DB + err *admin.Error + statusCode int } var tests = map[string]func(t *testing.T) test{ @@ -1943,10 +1912,10 @@ func TestPolicyAdminResponder_DeleteACMEAccountPolicy(t *testing.T) { err := admin.NewError(admin.ErrorNotImplementedType, "policy operations not yet supported in linked deployments") err.Message = "policy operations not yet supported in linked deployments" return test{ - ctx: ctx, - deploymentType: "linked", - err: err, - statusCode: 501, + ctx: ctx, + adminDB: &fakeLinkedCA{}, + err: err, + statusCode: 501, } }, "fail/no-existing-policy": func(t *testing.T) test { @@ -2033,10 +2002,8 @@ func TestPolicyAdminResponder_DeleteACMEAccountPolicy(t *testing.T) { for name, prep := range tests { tc := prep(t) t.Run(name, func(t *testing.T) { - par := &PolicyAdminResponder{ - acmeDB: tc.acmeDB, - deploymentType: tc.deploymentType, - } + + par := NewPolicyAdminResponder(nil, tc.adminDB, tc.acmeDB) req := httptest.NewRequest("POST", "/foo", io.NopCloser(bytes.NewBuffer(tc.body))) req = req.WithContext(tc.ctx) diff --git a/authority/linkedca.go b/authority/linkedca.go index 2e6ed61a..0552f2d1 100644 --- a/authority/linkedca.go +++ b/authority/linkedca.go @@ -122,6 +122,13 @@ func newLinkedCAClient(token string) (*linkedCaClient, error) { }, nil } +// IsLinkedCA is a sentinel function that can be used to +// check if a linkedCaClient is the underlying type of an +// admin.DB interface. +func (c *linkedCaClient) IsLinkedCA() bool { + return true +} + func (c *linkedCaClient) Run() { c.renewer.Run() } diff --git a/authority/policy.go b/authority/policy.go index f71e37c7..08a26f16 100644 --- a/authority/policy.go +++ b/authority/policy.go @@ -15,8 +15,7 @@ import ( type policyErrorType int const ( - _ policyErrorType = iota - AdminLockOut + AdminLockOut policyErrorType = iota + 1 StoreFailure ReloadFailure ConfigurationFailure @@ -345,7 +344,7 @@ func policyToCertificates(p *linkedca.Policy) *authPolicy.Options { } opts.X509.AllowWildcardLiteral = x509.AllowWildcardLiteral - opts.X509.DisableSubjectCommonNameVerification = x509.DisableSubjectCommonNameVerification + opts.X509.DisableCommonNameVerification = x509.DisableSubjectCommonNameVerification } // fill ssh policy configuration diff --git a/authority/policy/options.go b/authority/policy/options.go index ff0eec3d..c4b7b9ce 100644 --- a/authority/policy/options.go +++ b/authority/policy/options.go @@ -31,7 +31,7 @@ type X509PolicyOptionsInterface interface { GetAllowedNameOptions() *X509NameOptions GetDeniedNameOptions() *X509NameOptions IsWildcardLiteralAllowed() bool - ShouldVerifySubjectCommonName() bool + ShouldVerifyCommonName() bool } // X509PolicyOptions is a container for x509 allowed and denied @@ -39,15 +39,19 @@ type X509PolicyOptionsInterface interface { type X509PolicyOptions struct { // AllowedNames contains the x509 allowed names AllowedNames *X509NameOptions `json:"allow,omitempty"` + // DeniedNames contains the x509 denied names DeniedNames *X509NameOptions `json:"deny,omitempty"` + // AllowWildcardLiteral indicates if literal wildcard names // such as *.example.com and @example.com are allowed. Defaults // to false. - AllowWildcardLiteral bool `json:"allow_wildcard_literal,omitempty"` - // DisableSubjectCommonNameVerification indicates if the Subject Common Name - // is verified in addition to the SANs. Defaults to false. - DisableSubjectCommonNameVerification bool `json:"disable_subject_common_name_verification,omitempty"` + AllowWildcardLiteral bool `json:"allowWildcardLiteral,omitempty"` + + // DisableCommonNameVerification indicates if the Subject Common Name + // is verified in addition to the SANs. Defaults to false, resulting in + // Common Names being verified. + DisableCommonNameVerification bool `json:"disableCommonNameVerification,omitempty"` } // X509NameOptions models the X509 name policy configuration. @@ -92,13 +96,13 @@ func (o *X509PolicyOptions) IsWildcardLiteralAllowed() bool { return o.AllowWildcardLiteral } -// ShouldVerifySubjectCommonName returns whether the authority +// ShouldVerifyCommonName returns whether the authority // should verify the Subject Common Name in addition to the SANs. -func (o *X509PolicyOptions) ShouldVerifySubjectCommonName() bool { +func (o *X509PolicyOptions) ShouldVerifyCommonName() bool { if o == nil { return false } - return !o.DisableSubjectCommonNameVerification + return !o.DisableCommonNameVerification } // SSHPolicyOptionsInterface is an interface for providers of diff --git a/authority/policy/options_test.go b/authority/policy/options_test.go index ebcd90fe..b4f456a1 100644 --- a/authority/policy/options_test.go +++ b/authority/policy/options_test.go @@ -63,21 +63,21 @@ func TestX509PolicyOptions_ShouldVerifySubjectCommonName(t *testing.T) { { name: "set-true", options: &X509PolicyOptions{ - DisableSubjectCommonNameVerification: true, + DisableCommonNameVerification: true, }, want: false, }, { name: "set-false", options: &X509PolicyOptions{ - DisableSubjectCommonNameVerification: false, + DisableCommonNameVerification: false, }, want: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := tt.options.ShouldVerifySubjectCommonName(); got != tt.want { + if got := tt.options.ShouldVerifyCommonName(); got != tt.want { t.Errorf("X509PolicyOptions.ShouldVerifySubjectCommonName() = %v, want %v", got, tt.want) } }) diff --git a/authority/policy/policy.go b/authority/policy/policy.go index 564fca24..b68bcb19 100644 --- a/authority/policy/policy.go +++ b/authority/policy/policy.go @@ -50,7 +50,7 @@ func NewX509PolicyEngine(policyOptions X509PolicyOptionsInterface) (X509Policy, return nil, nil } - if policyOptions.ShouldVerifySubjectCommonName() { + if policyOptions.ShouldVerifyCommonName() { options = append(options, policy.WithSubjectCommonNameVerification()) } diff --git a/authority/policy_test.go b/authority/policy_test.go index 075987c0..f444a7f0 100644 --- a/authority/policy_test.go +++ b/authority/policy_test.go @@ -227,8 +227,8 @@ func Test_policyToCertificates(t *testing.T) { AllowedNames: &policy.X509NameOptions{ DNSDomains: []string{"*.local"}, }, - AllowWildcardLiteral: false, - DisableSubjectCommonNameVerification: false, + AllowWildcardLiteral: false, + DisableCommonNameVerification: false, }, }, }, @@ -290,8 +290,8 @@ func Test_policyToCertificates(t *testing.T) { EmailAddresses: []string{"badhost.example.com"}, URIDomains: []string{"https://badhost.local"}, }, - AllowWildcardLiteral: true, - DisableSubjectCommonNameVerification: false, + AllowWildcardLiteral: true, + DisableCommonNameVerification: false, }, SSH: &policy.SSHPolicyOptions{ Host: &policy.SSHHostCertificateOptions{ @@ -364,8 +364,8 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) { DeniedNames: &policy.X509NameOptions{ DNSDomains: []string{"badhost.local"}, }, - AllowWildcardLiteral: true, - DisableSubjectCommonNameVerification: false, + AllowWildcardLiteral: true, + DisableCommonNameVerification: false, }) assert.NoError(t, err) @@ -648,8 +648,8 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) { DeniedNames: &policy.X509NameOptions{ DNSDomains: []string{"badhost.local"}, }, - AllowWildcardLiteral: true, - DisableSubjectCommonNameVerification: false, + AllowWildcardLiteral: true, + DisableCommonNameVerification: false, }, }, }, @@ -768,8 +768,8 @@ func TestAuthority_reloadPolicyEngines(t *testing.T) { DeniedNames: &policy.X509NameOptions{ DNSDomains: []string{"badhost.local"}, }, - AllowWildcardLiteral: true, - DisableSubjectCommonNameVerification: false, + AllowWildcardLiteral: true, + DisableCommonNameVerification: false, }, SSH: &policy.SSHPolicyOptions{ Host: &policy.SSHHostCertificateOptions{ diff --git a/authority/provisioner/options.go b/authority/provisioner/options.go index 12e371a6..406b23b4 100644 --- a/authority/provisioner/options.go +++ b/authority/provisioner/options.go @@ -69,10 +69,12 @@ type X509Options struct { // AllowWildcardLiteral indicates if literal wildcard names // such as *.example.com and @example.com are allowed. Defaults // to false. - AllowWildcardLiteral *bool `json:"-"` - // VerifySubjectCommonName indicates if the Subject Common Name - // is verified in addition to the SANs. Defaults to true. - VerifySubjectCommonName *bool `json:"-"` + AllowWildcardLiteral bool `json:"-"` + + // DisableCommonNameVerification indicates if the Subject Common Name + // is verified in addition to the SANs. Defaults to false, resulting + // in Common Names to be verified. + DisableCommonNameVerification bool `json:"-"` } // HasTemplate returns true if a template is defined in the provisioner options. @@ -102,17 +104,14 @@ func (o *X509Options) IsWildcardLiteralAllowed() bool { if o == nil { return true } - return o.AllowWildcardLiteral != nil && *o.AllowWildcardLiteral + return o.AllowWildcardLiteral } -func (o *X509Options) ShouldVerifySubjectCommonName() bool { +func (o *X509Options) ShouldVerifyCommonName() bool { if o == nil { return false } - if o.VerifySubjectCommonName == nil { - return true - } - return *o.VerifySubjectCommonName + return !o.DisableCommonNameVerification } // TemplateOptions generates a CertificateOptions with the template and data diff --git a/authority/provisioner/options_test.go b/authority/provisioner/options_test.go index 32bea92b..2edcdf3e 100644 --- a/authority/provisioner/options_test.go +++ b/authority/provisioner/options_test.go @@ -289,8 +289,6 @@ func Test_unsafeParseSigned(t *testing.T) { } func TestX509Options_IsWildcardLiteralAllowed(t *testing.T) { - trueValue := true - falseValue := false tests := []struct { name string options *X509Options @@ -301,24 +299,17 @@ func TestX509Options_IsWildcardLiteralAllowed(t *testing.T) { options: nil, want: true, }, - { - name: "nil", - options: &X509Options{ - AllowWildcardLiteral: nil, - }, - want: false, - }, { name: "set-true", options: &X509Options{ - AllowWildcardLiteral: &trueValue, + AllowWildcardLiteral: true, }, want: true, }, { name: "set-false", options: &X509Options{ - AllowWildcardLiteral: &falseValue, + AllowWildcardLiteral: false, }, want: false, }, @@ -333,8 +324,6 @@ func TestX509Options_IsWildcardLiteralAllowed(t *testing.T) { } func TestX509Options_ShouldVerifySubjectCommonName(t *testing.T) { - trueValue := true - falseValue := false tests := []struct { name string options *X509Options @@ -345,31 +334,24 @@ func TestX509Options_ShouldVerifySubjectCommonName(t *testing.T) { options: nil, want: false, }, - { - name: "nil", - options: &X509Options{ - VerifySubjectCommonName: nil, - }, - want: true, - }, { name: "set-true", options: &X509Options{ - VerifySubjectCommonName: &trueValue, + DisableCommonNameVerification: true, }, - want: true, + want: false, }, { name: "set-false", options: &X509Options{ - VerifySubjectCommonName: &falseValue, + DisableCommonNameVerification: false, }, - want: false, + want: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := tt.options.ShouldVerifySubjectCommonName(); got != tt.want { + if got := tt.options.ShouldVerifyCommonName(); got != tt.want { t.Errorf("X509PolicyOptions.ShouldVerifySubjectCommonName() = %v, want %v", got, tt.want) } }) diff --git a/authority/tls_test.go b/authority/tls_test.go index a96ce1eb..3f9946ba 100644 --- a/authority/tls_test.go +++ b/authority/tls_test.go @@ -700,7 +700,7 @@ ZYtQ9Ot36qc= AllowedNames: &policy.X509NameOptions{ DNSDomains: []string{"*.smallstep.com"}, }, - DisableSubjectCommonNameVerification: true, // allows "smallstep test" + DisableCommonNameVerification: true, // TODO(hs): allows "smallstep test"; do we want to keep it like this? } engine, err := policy.NewX509PolicyEngine(policyOptions) assert.FatalError(t, err) diff --git a/ca/ca.go b/ca/ca.go index a08dc9e9..3cb4646b 100644 --- a/ca/ca.go +++ b/ca/ca.go @@ -219,7 +219,7 @@ func (ca *CA) Init(cfg *config.Config) (*CA, error) { adminDB := auth.GetAdminDatabase() if adminDB != nil { acmeAdminResponder := adminAPI.NewACMEAdminResponder() - policyAdminResponder := adminAPI.NewPolicyAdminResponder(auth, adminDB, acmeDB, cfg.AuthorityConfig.DeploymentType) + policyAdminResponder := adminAPI.NewPolicyAdminResponder(auth, adminDB, acmeDB) adminHandler := adminAPI.NewHandler(auth, adminDB, acmeDB, acmeAdminResponder, policyAdminResponder) mux.Route("/admin", func(r chi.Router) { adminHandler.Route(r)