From a4844fee7b3fe567a9347af8f93a5cbfade77c81 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Fri, 12 Mar 2021 16:58:52 +0100 Subject: [PATCH] Make tests green --- authority/authority.go | 107 ++++++++++++++++++++--------------------- scep/authority.go | 30 +++++++----- 2 files changed, 70 insertions(+), 67 deletions(-) diff --git a/authority/authority.go b/authority/authority.go index 975b0962..19de5210 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -7,8 +7,6 @@ import ( "crypto/x509" "encoding/hex" "log" - "os" - "strings" "sync" "time" @@ -202,53 +200,6 @@ func (a *Authority) init() error { } } - // TODO: decide if this is a good approach for providing the SCEP functionality - // It currently mirrors the logic for the x509CAServer - if a.scepService == nil { - var options casapi.Options - if a.config.AuthorityConfig.Options != nil { - options = *a.config.AuthorityConfig.Options - } - - // Read intermediate and create X509 signer and decrypter for default CAS. - if options.Is(casapi.SoftCAS) { - options.CertificateChain, err = pemutil.ReadCertificateBundle(a.config.IntermediateCert) - if err != nil { - return err - } - options.Signer, err = a.keyManager.CreateSigner(&kmsapi.CreateSignerRequest{ - SigningKey: a.config.IntermediateKey, - Password: []byte(a.config.Password), - }) - if err != nil { - return err - } - - // TODO: this is not exactly nice to do, but ensures that tests will still run while - // ECDSA keys are in the testdata. ECDSA keys are no crypto.Decrypters, resulting - // in many errors in the test suite. Needs a better solution, I think. - underTest := strings.HasSuffix(os.Args[0], ".test") - if !underTest { - if km, ok := a.keyManager.(kmsapi.Decrypter); ok { - options.Decrypter, err = km.CreateDecrypter(&kmsapi.CreateDecrypterRequest{ - DecryptionKey: a.config.IntermediateKey, - Password: []byte(a.config.Password), - }) - if err != nil { - return err - } - } - } - } - - a.scepService, err = scep.NewService(context.Background(), options) - if err != nil { - return err - } - - // TODO: mimick the x509CAService GetCertificateAuthority here too? - } - // Read root certificates and store them in the certificates map. if len(a.rootX509Certs) == 0 { a.rootX509Certs = make([]*x509.Certificate, len(a.config.Root)) @@ -399,6 +350,47 @@ func (a *Authority) init() error { } } + // TODO: decide if this is a good approach for providing the SCEP functionality + // It currently mirrors the logic for the x509CAService + if a.requiresSCEPService() && a.scepService == nil { + var options casapi.Options + if a.config.AuthorityConfig.Options != nil { + options = *a.config.AuthorityConfig.Options + } + + // Read intermediate and create X509 signer and decrypter for default CAS. + if options.Is(casapi.SoftCAS) { + options.CertificateChain, err = pemutil.ReadCertificateBundle(a.config.IntermediateCert) + if err != nil { + return err + } + options.Signer, err = a.keyManager.CreateSigner(&kmsapi.CreateSignerRequest{ + SigningKey: a.config.IntermediateKey, + Password: []byte(a.config.Password), + }) + if err != nil { + return err + } + + if km, ok := a.keyManager.(kmsapi.Decrypter); ok { + options.Decrypter, err = km.CreateDecrypter(&kmsapi.CreateDecrypterRequest{ + DecryptionKey: a.config.IntermediateKey, + Password: []byte(a.config.Password), + }) + if err != nil { + return err + } + } + } + + a.scepService, err = scep.NewService(context.Background(), options) + if err != nil { + return err + } + + // TODO: mimick the x509CAService GetCertificateAuthority here too? + } + // Store all the provisioners for _, p := range a.config.AuthorityConfig.Provisioners { if err := p.Init(config); err != nil { @@ -451,12 +443,15 @@ func (a *Authority) CloseForReload() { } } -// requiresDecrypter iterates over the configured provisioners -// and determines if the Authority requires a KMS that provides -// a crypto.Decrypter by implementing the apiv1.Decrypter -// interface. Currently only the SCEP provider requires this, -// but others may be added in the future. +// requiresDecrypter returns whether the Authority +// requires a KMS that provides a crypto.Decrypter func (a *Authority) requiresDecrypter() bool { + return a.requiresSCEPService() +} + +// requiresSCEPService iterates over the configured provisioners +// and determines if one of them is a SCEP provisioner. +func (a *Authority) requiresSCEPService() bool { for _, p := range a.config.AuthorityConfig.Provisioners { if p.GetType() == provisioner.TypeSCEP { return true @@ -470,6 +465,6 @@ func (a *Authority) requiresDecrypter() bool { // in order to make SCEP work more easily. It can be // made more correct by using the right interfaces/abstractions // after it works as expected. -func (a *Authority) GetSCEPService() scep.Service { - return *a.scepService +func (a *Authority) GetSCEPService() *scep.Service { + return a.scepService } diff --git a/scep/authority.go b/scep/authority.go index 03bd6919..2b6686fd 100644 --- a/scep/authority.go +++ b/scep/authority.go @@ -51,14 +51,14 @@ type Authority struct { intermediateCertificate *x509.Certificate - service Service + service *Service signAuth SignAuthority } // AuthorityOptions required to create a new SCEP Authority. type AuthorityOptions struct { // Service provides the SCEP functions to Authority - Service Service + Service *Service // Backdate Backdate provisioner.Duration // DB is the database used by nosql. @@ -92,15 +92,23 @@ func New(signAuth SignAuthority, ops AuthorityOptions) (*Authority, error) { } } - return &Authority{ - backdate: ops.Backdate, - db: ops.DB, - prefix: ops.Prefix, - dns: ops.DNS, - intermediateCertificate: ops.Service.certificateChain[0], - service: ops.Service, - signAuth: signAuth, - }, nil + authority := &Authority{ + backdate: ops.Backdate, + db: ops.DB, + prefix: ops.Prefix, + dns: ops.DNS, + signAuth: signAuth, + } + + // TODO: this is not really nice to do; the Service should be removed + // in its entirety to make this more interoperable with the rest of + // step-ca. + if ops.Service != nil { + authority.intermediateCertificate = ops.Service.certificateChain[0] + authority.service = ops.Service + } + + return authority, nil } var (