diff --git a/CHANGELOG.md b/CHANGELOG.md index 101985f6e..6ab022878 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 diff --git a/authority/authority.go b/authority/authority.go index b10c3c330..f396c5883 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 3b14657c8..f8e7f4efc 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, @@ -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,12 +154,7 @@ 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 { + if err := certProv.Init(provisionerConfig); err != nil { return admin.WrapErrorISE(err, "error initializing provisioner %s", prov.Name) } @@ -179,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) }