From 2e560caf68304319595917f4fa6154108c276a3c Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Mon, 2 Oct 2023 15:58:31 +0200 Subject: [PATCH] Add some basic tests for `GenerateConfig` So far the `GenerateConfig` method wasn't tested. This commit adds a couple of basic tests for this method. It's not fully covered yet, nor are all properties being checked, but it provides a starting point for refactoring the CA (configuration) initialization process. --- pki/pki.go | 10 +- pki/pki_test.go | 313 ++++++++++++++++++ ...ith-ssh-and-duplicate-provisioner-name.yml | 2 +- 3 files changed, 322 insertions(+), 3 deletions(-) create mode 100644 pki/pki_test.go diff --git a/pki/pki.go b/pki/pki.go index bb299dd9..234bae5a 100644 --- a/pki/pki.go +++ b/pki/pki.go @@ -319,7 +319,10 @@ type PKI struct { func New(o apiv1.Options, opts ...Option) (*PKI, error) { // TODO(hs): invoking `New` with a context active will use values from // that CA context while generating the context. Thay may or may not - // be fully expected and/or what we want. Check that. + // be fully expected and/or what we want. This specific behavior was + // changed after not relying on the `init` inside of `step`, resulting in + // the default context being active if `step.Init` isn't called explicitly. + // It can still result in surprising results, though. currentCtx := step.Contexts().GetCurrent() caService, err := cas.New(context.Background(), o) if err != nil { @@ -330,7 +333,7 @@ func New(o apiv1.Options, opts ...Option) (*PKI, error) { if o.IsCreator { creator, ok := caService.(apiv1.CertificateAuthorityCreator) if !ok { - return nil, errors.Errorf("cas type '%s' does not implements CertificateAuthorityCreator", o.Type) + return nil, errors.Errorf("cas type %q does not implement CertificateAuthorityCreator", o.Type) } caCreator = creator } @@ -923,10 +926,13 @@ func (p *PKI) GenerateConfig(opt ...ConfigOption) (*authconfig.Config, error) { if err != nil { return nil, err } + defer _db.Shutdown() // free DB resources; unlock BadgerDB file + adminDB, err := admindb.New(_db.(nosql.DB), admin.DefaultAuthorityID) if err != nil { return nil, err } + // Add all the provisioners to the db. var adminID string for i, p := range provisioners { diff --git a/pki/pki_test.go b/pki/pki_test.go new file mode 100644 index 00000000..7d5bc8c5 --- /dev/null +++ b/pki/pki_test.go @@ -0,0 +1,313 @@ +package pki + +import ( + "context" + "path/filepath" + "testing" + + "github.com/smallstep/certificates/authority/admin" + admindb "github.com/smallstep/certificates/authority/admin/db/nosql" + authconfig "github.com/smallstep/certificates/authority/config" + "github.com/smallstep/certificates/authority/provisioner" + "github.com/smallstep/certificates/cas/apiv1" + "github.com/smallstep/certificates/db" + "github.com/smallstep/nosql" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.step.sm/cli-utils/step" +) + +func withDBDataSource(t *testing.T, dataSource string) func(c *authconfig.Config) error { + return func(c *authconfig.Config) error { + if c == nil || c.DB == nil { + require.Fail(t, "withDBDataSource prerequisites not met") + } + c.DB.DataSource = dataSource + return nil + } +} + +func TestPKI_GenerateConfig(t *testing.T) { + var preparePKI = func(t *testing.T, opts ...Option) *PKI { + o := apiv1.Options{ + Type: "softcas", + IsCreator: true, + } + + // TODO(hs): invoking `New` doesn't perform all operations that are executed + // when `ca init` is executed. Ideally this logic should be handled in one + // place and probably inside of the PKI initialization. For testing purposes + // the missing operations are faked by `setKeyPair`. + p, err := New(o, opts...) + require.NoError(t, err) + + // setKeyPair sets a predefined JWK and a default JWK provisioner. This is one + // of the things performed in the `ca init` code that's not part of `New`, but + // performed after that in p.GenerateKeyPairs`. We're currently using the same + // JWK for every test to keep test variance small: we're not testing JWK generation + // here after all. It's a bit dangerous to redefine the function here, but it's + // the simplest way to make this fully testable without refactoring the init now. + // The password for the predefined encrypted key is \x01\x03\x03\x07. + setKeyPair(t, p) + + return p + } + type args struct { + opt []ConfigOption + } + type test struct { + pki *PKI + args args + want *authconfig.Config + wantErr bool + } + var tests = map[string]func(t *testing.T) test{ + "ok/simple": func(t *testing.T) test { + pki := preparePKI(t) + pki.options.deploymentType = StandaloneDeployment + pki.options.provisioner = "default-prov" + return test{ + pki: pki, + args: args{ + []ConfigOption{}, + }, + want: &authconfig.Config{ + Address: "127.0.0.1:9000", + InsecureAddress: "", + DNSNames: []string{"127.0.0.1"}, + AuthorityConfig: &authconfig.AuthConfig{ + DeploymentType: "", // TODO(hs): (why is) this is not set to standalone? + EnableAdmin: false, + Provisioners: provisioner.List{ + &provisioner.JWK{ + Type: "JWK", + Name: "default-prov", + }, + }, + }, + DB: &db.Config{ + Type: "badgerv2", + DataSource: filepath.Join(step.Path(), "db"), + }, + }, + wantErr: false, + } + }, + "ok/with-acme": func(t *testing.T) test { + pki := preparePKI(t) + pki.options.deploymentType = StandaloneDeployment + pki.options.provisioner = "default-prov" + pki.options.enableACME = true + return test{ + pki: pki, + args: args{ + []ConfigOption{}, + }, + want: &authconfig.Config{ + Address: "127.0.0.1:9000", + InsecureAddress: "", + DNSNames: []string{"127.0.0.1"}, + AuthorityConfig: &authconfig.AuthConfig{ + DeploymentType: "", // TODO(hs): (why is) this is not set to standalone? + EnableAdmin: false, + Provisioners: provisioner.List{ + &provisioner.JWK{ + Type: "JWK", + Name: "default-prov", + }, + &provisioner.ACME{ + Type: "ACME", + Name: "acme", + }, + }, + }, + DB: &db.Config{ + Type: "badgerv2", + DataSource: filepath.Join(step.Path(), "db"), + }, + }, + wantErr: false, + } + }, + "ok/with-acme-and-double-provisioner-name": func(t *testing.T) test { + pki := preparePKI(t) + pki.options.deploymentType = StandaloneDeployment + pki.options.provisioner = "acme" + pki.options.enableACME = true + return test{ + pki: pki, + args: args{ + []ConfigOption{}, + }, + want: &authconfig.Config{ + Address: "127.0.0.1:9000", + InsecureAddress: "", + DNSNames: []string{"127.0.0.1"}, + AuthorityConfig: &authconfig.AuthConfig{ + DeploymentType: "", // TODO(hs): (why is) this is not set to standalone? + EnableAdmin: false, + Provisioners: provisioner.List{ + &provisioner.JWK{ + Type: "JWK", + Name: "acme", + }, + &provisioner.ACME{ + Type: "ACME", + Name: "acme-1", + }, + }, + }, + DB: &db.Config{ + Type: "badgerv2", + DataSource: filepath.Join(step.Path(), "db"), + }, + }, + wantErr: false, + } + }, + "ok/with-ssh": func(t *testing.T) test { + pki := preparePKI(t) + pki.options.deploymentType = StandaloneDeployment + pki.options.provisioner = "default-prov" + pki.options.enableSSH = true + return test{ + pki: pki, + args: args{ + []ConfigOption{}, + }, + want: &authconfig.Config{ + Address: "127.0.0.1:9000", + InsecureAddress: "", + DNSNames: []string{"127.0.0.1"}, + AuthorityConfig: &authconfig.AuthConfig{ + DeploymentType: "", // TODO(hs): (why is) this is not set to standalone? + EnableAdmin: false, + Provisioners: provisioner.List{ + &provisioner.JWK{ + Type: "JWK", + Name: "default-prov", + }, + &provisioner.SSHPOP{ + Type: "SSHPOP", + Name: "sshpop", + }, + }, + }, + DB: &db.Config{ + Type: "badgerv2", + DataSource: filepath.Join(step.Path(), "db"), + }, + }, + wantErr: false, + } + }, + "ok/with-ssh-and-double-provisioner-name": func(t *testing.T) test { + pki := preparePKI(t) + pki.options.deploymentType = StandaloneDeployment + pki.options.provisioner = "sshpop" + pki.options.enableSSH = true + return test{ + pki: pki, + args: args{ + []ConfigOption{}, + }, + want: &authconfig.Config{ + Address: "127.0.0.1:9000", + InsecureAddress: "", + DNSNames: []string{"127.0.0.1"}, + AuthorityConfig: &authconfig.AuthConfig{ + DeploymentType: "", // TODO(hs): (why is) this is not set to standalone? + EnableAdmin: false, + Provisioners: provisioner.List{ + &provisioner.JWK{ + Type: "JWK", + Name: "sshpop", + }, + &provisioner.SSHPOP{ + Type: "SSHPOP", + Name: "sshpop-1", + }, + }, + }, + DB: &db.Config{ + Type: "badgerv2", + DataSource: filepath.Join(step.Path(), "db"), + }, + }, + wantErr: false, + } + }, + "ok/with-admin": func(t *testing.T) test { + pki := preparePKI(t) + pki.options.deploymentType = StandaloneDeployment + pki.options.provisioner = "default-prov" + pki.options.enableAdmin = true + tempDir := t.TempDir() + return test{ + pki: pki, + args: args{ + []ConfigOption{withDBDataSource(t, filepath.Join(tempDir, "db"))}, + }, + want: &authconfig.Config{ + Address: "127.0.0.1:9000", + InsecureAddress: "", + DNSNames: []string{"127.0.0.1"}, + AuthorityConfig: &authconfig.AuthConfig{ + DeploymentType: "", // TODO(hs): (why is) this is not set to standalone? + EnableAdmin: true, + Provisioners: provisioner.List{}, // when admin is enabled, provisioner list is expected to be empty + }, + DB: &db.Config{ + Type: "badgerv2", + DataSource: filepath.Join(tempDir, "db"), + }, + }, + wantErr: false, + } + }, + } + for name, run := range tests { + tc := run(t) + t.Run(name, func(t *testing.T) { + got, err := tc.pki.GenerateConfig(tc.args.opt...) + if tc.wantErr { + assert.NotNil(t, err) + assert.Nil(t, got) + return + } + + assert.Nil(t, err) + if assert.NotNil(t, got) { + assert.Equal(t, tc.want.Address, got.Address) + assert.Equal(t, tc.want.InsecureAddress, got.InsecureAddress) + assert.Equal(t, tc.want.DNSNames, got.DNSNames) + assert.Equal(t, tc.want.DB, got.DB) + if assert.NotNil(t, tc.want.AuthorityConfig) { + assert.Equal(t, tc.want.AuthorityConfig.DeploymentType, got.AuthorityConfig.DeploymentType) + assert.Equal(t, tc.want.AuthorityConfig.EnableAdmin, got.AuthorityConfig.EnableAdmin) + if numberOfProvisioners := len(tc.want.AuthorityConfig.Provisioners); numberOfProvisioners > 0 { + if assert.Len(t, got.AuthorityConfig.Provisioners, numberOfProvisioners) { + for i, p := range tc.want.AuthorityConfig.Provisioners { + assert.Equal(t, p.GetType(), got.AuthorityConfig.Provisioners[i].GetType()) + assert.Equal(t, p.GetName(), got.AuthorityConfig.Provisioners[i].GetName()) + } + } + } + if tc.want.AuthorityConfig.EnableAdmin { + _db, err := db.New(tc.want.DB) + require.NoError(t, err) + defer _db.Shutdown() + + adminDB, err := admindb.New(_db.(nosql.DB), admin.DefaultAuthorityID) + require.NoError(t, err) + + provs, err := adminDB.GetProvisioners(context.Background()) + require.NoError(t, err) + + assert.NotEmpty(t, provs) // currently about the best we can do in terms of checks + } + } + } + }) + } +} diff --git a/pki/testdata/helm/with-ssh-and-duplicate-provisioner-name.yml b/pki/testdata/helm/with-ssh-and-duplicate-provisioner-name.yml index d8d32dd6..a499b155 100644 --- a/pki/testdata/helm/with-ssh-and-duplicate-provisioner-name.yml +++ b/pki/testdata/helm/with-ssh-and-duplicate-provisioner-name.yml @@ -23,7 +23,7 @@ inject: authority: enableAdmin: false provisioners: - - {"type":"JWK","name":"sshpop","key":{"use":"sig","kty":"EC","kid":"zsUmysmDVoGJ71YoPHyZ-68tNihDaDaO5Mu7xX3M-_I","crv":"P-256","alg":"ES256","x":"Pqnua4CzqKz6ua41J3yeWZ1sRkGt0UlCkbHv8H2DGuY","y":"UhoZ_2ItDen9KQTcjay-ph-SBXH0mwqhHyvrrqIFDOI"},"encryptedKey":"eyJhbGciOiJQQkVTMi1IUzI1NitBMTI4S1ciLCJjdHkiOiJqd2sranNvbiIsImVuYyI6IkEyNTZHQ00iLCJwMmMiOjEwMDAwMCwicDJzIjoiZjVvdGVRS2hvOXl4MmQtSGlMZi05QSJ9.eYA6tt3fNuUpoxKWDT7P0Lbn2juxhEbTxEnwEMbjlYLLQ3sxL-dYTA.ven-FhmdjlC9itH0.a2jRTarN9vPd6F_mWnNBlOn6KbfMjCApmci2t65XbAsLzYFzhI_79Ykm5ueMYTupWLTjBJctl-g51ZHmsSB55pStbpoyyLNAsUX2E1fTmHe-Ni8bRrspwLv15FoN1Xo1g0mpR-ufWIFxOsW-QIfnMmMIIkygVuHFXmg2tFpzTNNG5aS29K3dN2nyk0WJrdIq79hZSTqVkkBU25Yu3A46sgjcM86XcIJJ2XUEih_KWEa6T1YrkixGu96pebjVqbO0R6dbDckfPF7FqNnwPHVtb1ACFpEYoOJVIbUCMaARBpWsxYhjJZlEM__XA46l8snFQDkNY3CdN0p1_gF3ckA.JLmq9nmu1h9oUi1S8ZxYjA","claims":{"enableSSHCA":true,"disableRenewal":false,"allowRenewalAfterExpiry":false},"options":{"x509":{},"ssh":{}}} + - {"type":"JWK","name":"sshpop","key":{"use":"sig","kty":"EC","kid":"zsUmysmDVoGJ71YoPHyZ-68tNihDaDaO5Mu7xX3M-_I","crv":"P-256","alg":"ES256","x":"Pqnua4CzqKz6ua41J3yeWZ1sRkGt0UlCkbHv8H2DGuY","y":"UhoZ_2ItDen9KQTcjay-ph-SBXH0mwqhHyvrrqIFDOI"},"encryptedKey":"eyJhbGciOiJQQkVTMi1IUzI1NitBMTI4S1ciLCJjdHkiOiJqd2sranNvbiIsImVuYyI6IkEyNTZHQ00iLCJwMmMiOjEwMDAwMCwicDJzIjoiZjVvdGVRS2hvOXl4MmQtSGlMZi05QSJ9.eYA6tt3fNuUpoxKWDT7P0Lbn2juxhEbTxEnwEMbjlYLLQ3sxL-dYTA.ven-FhmdjlC9itH0.a2jRTarN9vPd6F_mWnNBlOn6KbfMjCApmci2t65XbAsLzYFzhI_79Ykm5ueMYTupWLTjBJctl-g51ZHmsSB55pStbpoyyLNAsUX2E1fTmHe-Ni8bRrspwLv15FoN1Xo1g0mpR-ufWIFxOsW-QIfnMmMIIkygVuHFXmg2tFpzTNNG5aS29K3dN2nyk0WJrdIq79hZSTqVkkBU25Yu3A46sgjcM86XcIJJ2XUEih_KWEa6T1YrkixGu96pebjVqbO0R6dbDckfPF7FqNnwPHVtb1ACFpEYoOJVIbUCMaARBpWsxYhjJZlEM__XA46l8snFQDkNY3CdN0p1_gF3ckA.JLmq9nmu1h9oUi1S8ZxYjA","claims":{"enableSSHCA":true,"disableRenewal":false,"allowRenewalAfterExpiry":false,"disableSmallstepExtensions":false},"options":{"x509":{},"ssh":{}}} - {"type":"SSHPOP","name":"sshpop-1","claims":{"enableSSHCA":true}} tls: cipherSuites: