diff --git a/authority/authority.go b/authority/authority.go index 1ba480af..875c3a14 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -62,10 +62,9 @@ type Authority struct { x509Enforcers []provisioner.CertificateEnforcer // SCEP CA - scepAuthority *scep.Authority - scepCertificate *x509.Certificate - scepSigner crypto.Signer - scepDecrypter crypto.Decrypter + scepOptions *scep.Options + validateSCEP bool + scepAuthority *scep.Authority // SSH CA sshHostPassword []byte @@ -126,6 +125,7 @@ func New(cfg *config.Config, opts ...Option) (*Authority, error) { var a = &Authority{ config: cfg, certificates: new(sync.Map), + validateSCEP: true, } // Apply options. @@ -674,15 +674,12 @@ func (a *Authority) init() error { // update that. switch { case a.requiresSCEP() && a.GetSCEP() == nil: - var options scep.Options - options.Roots = a.rootX509Certs - options.Intermediates = a.intermediateX509Certs - options.SignerCert = options.Intermediates[0] - if a.scepSigner != nil { - options.Signer = a.scepSigner - options.Decrypter = a.scepDecrypter - options.DecrypterCert = a.scepCertificate - } else { + if a.scepOptions == nil { + options := &scep.Options{ + Roots: a.rootX509Certs, + Intermediates: a.intermediateX509Certs, + SignerCert: a.intermediateX509Certs[0], + } if options.Signer, err = a.keyManager.CreateSigner(&kmsapi.CreateSignerRequest{ SigningKey: a.config.IntermediateKey, Password: a.password, @@ -709,21 +706,25 @@ func (a *Authority) init() error { options.DecrypterCert = options.Intermediates[0] } } - } - // provide the current SCEP provisioner names, so that the provisioners - // can be validated when the CA is started. - options.SCEPProvisionerNames = a.getSCEPProvisionerNames() + // provide the current SCEP provisioner names, so that the provisioners + // can be validated when the CA is started. + options.SCEPProvisionerNames = a.getSCEPProvisionerNames() + + a.scepOptions = options + } // create a new SCEP authority - scepAuthority, err := scep.New(a, options) + scepAuthority, err := scep.New(a, *a.scepOptions) if err != nil { return err } - // validate the SCEP authority - if err := scepAuthority.Validate(); err != nil { - a.initLogf("failed validating SCEP authority: %v", err) + if a.validateSCEP { + // validate the SCEP authority + if err := scepAuthority.Validate(); err != nil { + a.initLogf("failed validating SCEP authority: %v", err) + } } // set the SCEP authority diff --git a/authority/options.go b/authority/options.go index 79cd6206..f053b99c 100644 --- a/authority/options.go +++ b/authority/options.go @@ -18,6 +18,7 @@ import ( "github.com/smallstep/certificates/cas" casapi "github.com/smallstep/certificates/cas/apiv1" "github.com/smallstep/certificates/db" + "github.com/smallstep/certificates/scep" ) // Option sets options to the Authority. @@ -205,11 +206,19 @@ func WithX509SignerFunc(fn func() ([]*x509.Certificate, crypto.Signer, error)) O } } -func WithSCEPOptions(crt *x509.Certificate, s crypto.Signer, d crypto.Decrypter) Option { +// func WithSCEPOptions(crt *x509.Certificate, s crypto.Signer, d crypto.Decrypter) Option { +// return func(a *Authority) error { +// a.scepCertificate = crt +// a.scepSigner = s +// a.scepDecrypter = d +// return nil +// } +// } + +func WithFullSCEPOptions(options *scep.Options) Option { return func(a *Authority) error { - a.scepCertificate = crt - a.scepSigner = s - a.scepDecrypter = d + a.scepOptions = options + a.validateSCEP = false return nil } } diff --git a/scep/authority.go b/scep/authority.go index 8fa8a66f..292c7004 100644 --- a/scep/authority.go +++ b/scep/authority.go @@ -83,10 +83,7 @@ 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 { - // TODO(hs): don't return early - return nil //nolint:revive // validation temporarily disabled - - if a == nil { //nolint:govet // validation temporarily disabled + if a == nil { return nil } diff --git a/scep/options.go b/scep/options.go index 50c82338..8bc30a61 100644 --- a/scep/options.go +++ b/scep/options.go @@ -2,6 +2,7 @@ package scep import ( "crypto" + "crypto/rsa" "crypto/x509" "errors" ) @@ -56,28 +57,27 @@ func (o *Options) Validate() error { return nil } - // TODO(hs): reenable this validation - // // If a decrypter is available, check that it's backed by an RSA key. According to the - // // RFC: https://tools.ietf.org/html/rfc8894#section-3.1, SCEP can be used with something - // // different than RSA, but requires the encryption to be performed using the challenge - // // password in that case. An older version of specification states that only RSA is - // // supported: https://tools.ietf.org/html/draft-nourse-scep-23#section-2.1.1. Other - // // algorithms do not seem to be supported in certnanny/sscep, but it might work - // // in micromdm/scep. Currently only RSA is allowed, but it might be an option - // // to try other algorithms in the future. - // decrypterPublicKey, ok := o.Decrypter.Public().(*rsa.PublicKey) - // if !ok { - // return errors.New("only RSA keys are (currently) supported as decrypters") - // } + // If a decrypter is available, check that it's backed by an RSA key. According to the + // RFC: https://tools.ietf.org/html/rfc8894#section-3.1, SCEP can be used with something + // different than RSA, but requires the encryption to be performed using the challenge + // password in that case. An older version of specification states that only RSA is + // supported: https://tools.ietf.org/html/draft-nourse-scep-23#section-2.1.1. Other + // algorithms do not seem to be supported in certnanny/sscep, but it might work + // in micromdm/scep. Currently only RSA is allowed, but it might be an option + // to try other algorithms in the future. + decrypterPublicKey, ok := o.Decrypter.Public().(*rsa.PublicKey) + if !ok { + return errors.New("only RSA keys are (currently) supported as decrypters") + } - // // check if intermediate public key is the same as the decrypter public key. - // // In certnanny/sscep it's mentioned that the signing key can be different - // // from the decrypting (and encrypting) key. These options are only used and - // // validated when the intermediate CA is also used as the decrypter, though, - // // so they should match. - // if !decrypterPublicKey.Equal(o.SignerCert.PublicKey) { - // return errors.New("mismatch between certificate chain and decrypter public keys") - // } + // check if intermediate public key is the same as the decrypter public key. + // In certnanny/sscep it's mentioned that the signing key can be different + // from the decrypting (and encrypting) key. These options are only used and + // validated when the intermediate CA is also used as the decrypter, though, + // so they should match. + if !decrypterPublicKey.Equal(o.SignerCert.PublicKey) { + return errors.New("mismatch between certificate chain and decrypter public keys") + } return nil }