From 6030f8bc2eda65617987d13ae7db9b92f0ba0113 Mon Sep 17 00:00:00 2001 From: max furman Date: Mon, 28 Feb 2022 10:48:01 -0800 Subject: [PATCH 1/3] Validate provisioner configuration before storing in DB --- authority/provisioners.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/authority/provisioners.go b/authority/provisioners.go index 3b14657c..7dbc84f7 100644 --- a/authority/provisioners.go +++ b/authority/provisioners.go @@ -133,9 +133,18 @@ func (a *Authority) StoreProvisioner(ctx context.Context, prov *linkedca.Provisi "provisioner with token ID %s already exists", certProv.GetIDForToken()) } + provisionerConfig, err := a.generateProvisionerConfig(ctx) + if err != nil { + return admin.WrapErrorISE(err, "error generating provisioner config") + } + + if err := certProv.Init(*provisionerConfig); err != nil { + return admin.WrapError(admin.ErrorBadRequestType, err, "error validating configuration for provisioner %s", prov.Name) + } + // Store to database -- this will set the ID. if err := a.adminDB.CreateProvisioner(ctx, prov); err != nil { - return admin.WrapErrorISE(err, "error creating admin") + return admin.WrapErrorISE(err, "error creating provisioner") } // We need a new conversion that has the newly set ID. @@ -145,11 +154,6 @@ func (a *Authority) StoreProvisioner(ctx context.Context, prov *linkedca.Provisi "error converting to certificates provisioner from linkedca provisioner") } - provisionerConfig, err := a.generateProvisionerConfig(ctx) - if err != nil { - return admin.WrapErrorISE(err, "error generating provisioner config") - } - if err := certProv.Init(*provisionerConfig); err != nil { return admin.WrapErrorISE(err, "error initializing provisioner %s", prov.Name) } From a79d4af19bf3205312f7ae8e220a8c7158296572 Mon Sep 17 00:00:00 2001 From: max furman Date: Mon, 28 Feb 2022 11:04:40 -0800 Subject: [PATCH 2/3] change return value of generateProvisionerConfig to value - always used as value (rather than pointer) --- authority/authority.go | 2 +- authority/provisioners.go | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/authority/authority.go b/authority/authority.go index b10c3c33..f396c588 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -175,7 +175,7 @@ func (a *Authority) reloadAdminResources(ctx context.Context) error { // Create provisioner collection. provClxn := provisioner.NewCollection(provisionerConfig.Audiences) for _, p := range provList { - if err := p.Init(*provisionerConfig); err != nil { + if err := p.Init(provisionerConfig); err != nil { return err } if err := provClxn.Store(p); err != nil { diff --git a/authority/provisioners.go b/authority/provisioners.go index 7dbc84f7..f8e7f4ef 100644 --- a/authority/provisioners.go +++ b/authority/provisioners.go @@ -87,20 +87,20 @@ func (a *Authority) LoadProvisionerByName(name string) (provisioner.Interface, e return p, nil } -func (a *Authority) generateProvisionerConfig(ctx context.Context) (*provisioner.Config, error) { +func (a *Authority) generateProvisionerConfig(ctx context.Context) (provisioner.Config, error) { // Merge global and configuration claims claimer, err := provisioner.NewClaimer(a.config.AuthorityConfig.Claims, config.GlobalProvisionerClaims) if err != nil { - return nil, err + return provisioner.Config{}, err } // TODO: should we also be combining the ssh federated roots here? // If we rotate ssh roots keys, sshpop provisioner will lose ability to // validate old SSH certificates, unless they are added as federated certs. sshKeys, err := a.GetSSHRoots(ctx) if err != nil { - return nil, err + return provisioner.Config{}, err } - return &provisioner.Config{ + return provisioner.Config{ Claims: claimer.Claims(), Audiences: a.config.GetAudiences(), DB: a.db, @@ -138,7 +138,7 @@ func (a *Authority) StoreProvisioner(ctx context.Context, prov *linkedca.Provisi return admin.WrapErrorISE(err, "error generating provisioner config") } - if err := certProv.Init(*provisionerConfig); err != nil { + if err := certProv.Init(provisionerConfig); err != nil { return admin.WrapError(admin.ErrorBadRequestType, err, "error validating configuration for provisioner %s", prov.Name) } @@ -154,7 +154,7 @@ func (a *Authority) StoreProvisioner(ctx context.Context, prov *linkedca.Provisi "error converting to certificates provisioner from linkedca provisioner") } - if err := certProv.Init(*provisionerConfig); err != nil { + if err := certProv.Init(provisionerConfig); err != nil { return admin.WrapErrorISE(err, "error initializing provisioner %s", prov.Name) } @@ -183,7 +183,7 @@ func (a *Authority) UpdateProvisioner(ctx context.Context, nu *linkedca.Provisio return admin.WrapErrorISE(err, "error generating provisioner config") } - if err := certProv.Init(*provisionerConfig); err != nil { + if err := certProv.Init(provisionerConfig); err != nil { return admin.WrapErrorISE(err, "error initializing provisioner %s", nu.Name) } From 51210dfef956c4fae9a927cf58acba9d5acd41ba Mon Sep 17 00:00:00 2001 From: max furman Date: Mon, 28 Feb 2022 11:05:59 -0800 Subject: [PATCH 3/3] changelog update --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 101985f6..6ab02287 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Deprecated ### Removed ### Fixed +- During provisioner add - validate provisioner configuration before storing to DB. ### Security ## [0.18.1] - 2022-02-03