diff --git a/authority/authority.go b/authority/authority.go index 4318246b..e6044e4d 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -262,13 +262,22 @@ func (a *Authority) ReloadAdminResources(ctx context.Context) error { a.config.AuthorityConfig.Admins = adminList a.admins = adminClxn - // update the SCEP Authority with the currently active SCEP - // provisioner names and revalidate the configuration. - if a.scepAuthority != nil { + switch { + case a.requiresSCEP() && a.GetSCEP() == nil: + // TODO(hs): try to initialize SCEP here too? It's a bit + // problematic if this method is called as part of an update + // via Admin API and a password needs to be provided. + case a.requiresSCEP() && a.GetSCEP() != nil: + // update the SCEP Authority with the currently active SCEP + // provisioner names and revalidate the configuration. a.scepAuthority.UpdateProvisioners(a.getSCEPProvisionerNames()) if err := a.scepAuthority.Validate(); err != nil { log.Printf("failed validating SCEP authority: %v\n", err) } + case !a.requiresSCEP() && a.GetSCEP() != nil: + // TODO(hs): don't remove the authority if we can't also + // reload it. + //a.scepAuthority = nil } return nil @@ -651,14 +660,17 @@ func (a *Authority) init() error { } // The SCEP functionality is provided through an instance of - // scep.Authority. It is initialized once when the CA is started. - // TODO(hs): should the SCEP Authority support reloading? For example, - // when the admin resources are reloaded, specifically the provisioners, - // it can happen that the SCEP Authority is no longer required and can - // be destroyed, or that it needs to be instantiated. It may also need - // to be revalidated, because not all SCEP provisioner may have a - // valid decrypter available. - if a.requiresSCEP() && a.GetSCEP() == nil { + // scep.Authority. It is initialized when the CA is started and + // if it doesn't exist yet. It gets refreshed if it already + // exists. If the SCEP authority is no longer required on reload, + // it gets removed. + // TODO(hs): reloading through SIGHUP doesn't hit these cases. This + // is because an entirely new authority.Authority is created, including + // a new scep.Authority. Look into this to see if we want this to + // keep working like that, or want to reuse a single instance and + // update that. + switch { + case a.requiresSCEP() && a.GetSCEP() == nil: var options scep.Options options.Roots = a.rootX509Certs options.Intermediates, err = pemutil.ReadCertificateBundle(a.config.IntermediateCert) @@ -698,15 +710,28 @@ func (a *Authority) init() error { options.SCEPProvisionerNames = a.getSCEPProvisionerNames() // create a new SCEP authority - a.scepAuthority, err = scep.New(a, options) + scepAuthority, err := scep.New(a, options) if err != nil { return err } // validate the SCEP authority - if err := a.scepAuthority.Validate(); err != nil { + if err := scepAuthority.Validate(); err != nil { a.initLogf("failed validating SCEP authority: %v", err) } + + // set the SCEP authority + a.scepAuthority = scepAuthority + case !a.requiresSCEP() && a.GetSCEP() != nil: + // clear the SCEP authority if it's no longer required + a.scepAuthority = nil + case a.requiresSCEP() && a.GetSCEP() != nil: + // update the SCEP Authority with the currently active SCEP + // provisioner names and revalidate the configuration. + a.scepAuthority.UpdateProvisioners(a.getSCEPProvisionerNames()) + if err := a.scepAuthority.Validate(); err != nil { + log.Printf("failed validating SCEP authority: %v\n", err) + } } // Load X509 constraints engine. @@ -869,12 +894,15 @@ func (a *Authority) requiresSCEP() bool { return false } +// getSCEPProvisionerNames returns the names of the SCEP provisioners +// that are currently available in the CA. func (a *Authority) getSCEPProvisionerNames() (names []string) { for _, p := range a.config.AuthorityConfig.Provisioners { if p.GetType() == provisioner.TypeSCEP { names = append(names, p.GetName()) } } + return } diff --git a/ca/ca.go b/ca/ca.go index c13496a6..911cb2d7 100644 --- a/ca/ca.go +++ b/ca/ca.go @@ -250,19 +250,9 @@ func (ca *CA) Init(cfg *config.Config) (*CA, error) { var scepAuthority *scep.Authority if ca.shouldServeSCEPEndpoints() { - // validate the SCEP authority configuration. Currently this - // will not result in a failure to start if one or more SCEP - // provisioners are not correctly configured. Only a log will - // be emitted. + // get the SCEP authority configuration. Validation is + // performed within the authority instantiation process. scepAuthority = auth.GetSCEP() - if err := scepAuthority.Validate(); err != nil { - err = errors.Wrap(err, "failed validating SCEP authority") - shouldFail := false - if shouldFail { - return nil, err - } - log.Println(err) - } // According to the RFC (https://tools.ietf.org/html/rfc8894#section-7.10), // SCEP operations are performed using HTTP, so that's why the API is mounted diff --git a/scep/authority.go b/scep/authority.go index b0a5420a..60141191 100644 --- a/scep/authority.go +++ b/scep/authority.go @@ -6,6 +6,7 @@ import ( "crypto/x509" "errors" "fmt" + "sync" microx509util "github.com/micromdm/scep/v2/cryptoutil/x509util" microscep "github.com/micromdm/scep/v2/scep" @@ -25,6 +26,8 @@ type Authority struct { signer crypto.Signer defaultDecrypter crypto.Decrypter scepProvisionerNames []string + + mu sync.RWMutex } type authorityKey struct{} @@ -77,6 +80,13 @@ func New(signAuth SignAuthority, opts Options) (*Authority, error) { // The validation includes a check if a decrypter is available, either // an authority wide decrypter, or a provisioner specific decrypter. func (a *Authority) Validate() error { + if a == nil { + return nil + } + + a.mu.RLock() + defer a.mu.RUnlock() + noDefaultDecrypterAvailable := a.defaultDecrypter == nil for _, name := range a.scepProvisionerNames { p, err := a.LoadProvisionerByName(name) @@ -102,6 +112,13 @@ func (a *Authority) Validate() error { // current SCEP provisioners configured. This allows the Authority to be // validated with the latest data. func (a *Authority) UpdateProvisioners(scepProvisionerNames []string) { + if a == nil { + return + } + + a.mu.Lock() + defer a.mu.Unlock() + a.scepProvisionerNames = scepProvisionerNames }