diff --git a/authority/provisioner/scep.go b/authority/provisioner/scep.go index 7bd213c6..626d26ab 100644 --- a/authority/provisioner/scep.go +++ b/authority/provisioner/scep.go @@ -3,6 +3,7 @@ package provisioner import ( "context" "crypto" + "crypto/rsa" "crypto/subtle" "crypto/x509" "encoding/pem" @@ -177,23 +178,23 @@ func (s *SCEP) Init(config Config) (err error) { if s.MinimumPublicKeyLength == 0 { s.MinimumPublicKeyLength = 2048 } - if s.MinimumPublicKeyLength%8 != 0 { return errors.Errorf("%d bits is not exactly divisible by 8", s.MinimumPublicKeyLength) } + // Set the encryption algorithm to use s.encryptionAlgorithm = s.EncryptionAlgorithmIdentifier // TODO(hs): we might want to upgrade the default security to AES-CBC? if s.encryptionAlgorithm < 0 || s.encryptionAlgorithm > 4 { return errors.New("only encryption algorithm identifiers from 0 to 4 are valid") } + // Prepare the SCEP challenge validator s.challengeValidationController = newChallengeValidationController( config.WebhookClient, s.GetOptions().GetWebhooks(), ) - skip := false // TODO(hs): remove this; currently a helper for debugging - if decryptionKey := s.DecrypterKey; decryptionKey != "" && !skip { + if decryptionKey := s.DecrypterKey; decryptionKey != "" { u, err := uri.Parse(s.DecrypterKey) if err != nil { return fmt.Errorf("failed parsing decrypter key: %w", err) @@ -226,7 +227,7 @@ func (s *SCEP) Init(config Config) (err error) { return fmt.Errorf("failed creating decrypter: %w", err) } if s.signer, err = s.keyManager.CreateSigner(&kmsapi.CreateSignerRequest{ - SigningKey: decryptionKey, // TODO(hs): support distinct signer key + SigningKey: decryptionKey, // TODO(hs): support distinct signer key in the future? Password: []byte(s.DecrypterKeyPassword), }); err != nil { return fmt.Errorf("failed creating signer: %w", err) @@ -248,23 +249,19 @@ func (s *SCEP) Init(config Config) (err error) { } // TODO(hs): alternatively, check if the KMS keyManager is a CertificateManager - // and load the certificate corresponding to the decryption key. - - // final validation for the decrypter - if s.decrypter != nil { - // // TODO(hs): enable this validation again - // if s.decrypterCertificate == nil { - // // TODO: don't hard skip at init? - // return fmt.Errorf("no decrypter certificate available for decrypter in %q", s.Name) - // } - // // validate the decrypter key - // decrypterPublicKey, ok := s.decrypter.Public().(*rsa.PublicKey) - // if !ok { - // return fmt.Errorf("only RSA keys are supported") - // } - // if !decrypterPublicKey.Equal(s.decrypterCertificate.PublicKey) { - // return errors.New("mismatch between decryption certificate and decrypter public keys") - // } + // and load the certificate corresponding to the decryption key? + + // Final validation for the decrypter. If both the decrypter and the certificate + // are available, the public keys must match. We currently allow the decrypter to + // be set without a certificate without warning the user, but + if s.decrypter != nil && s.decrypterCertificate != nil { + decrypterPublicKey, ok := s.decrypter.Public().(*rsa.PublicKey) + if !ok { + return fmt.Errorf("only RSA keys are supported") + } + if !decrypterPublicKey.Equal(s.decrypterCertificate.PublicKey) { + return errors.New("mismatch between decryption certificate and decrypter public keys") + } } // TODO: add other, SCEP specific, options? @@ -350,10 +347,19 @@ func (s *SCEP) selectValidationMethod() validationMethod { return validationMethodNone } +// GetDecrypter returns the provisioner specific decrypter, +// used to decrypt SCEP request messages sent by a SCEP client. +// The decrypter consists of a crypto.Decrypter (a private key) +// and a certificate for the public key corresponding to the +// private key. func (s *SCEP) GetDecrypter() (*x509.Certificate, crypto.Decrypter) { return s.decrypterCertificate, s.decrypter } +// GetSigner returns the provisioner specific signer, used to +// sign SCEP response messages for the client. The signer consists +// of a crypto.Signer and a certificate for the public key +// corresponding to the private key. func (s *SCEP) GetSigner() (*x509.Certificate, crypto.Signer) { return s.decrypterCertificate, s.signer } diff --git a/scep/authority.go b/scep/authority.go index 5fd7223b..c839ab99 100644 --- a/scep/authority.go +++ b/scep/authority.go @@ -486,19 +486,22 @@ func (a *Authority) ValidateChallenge(ctx context.Context, challenge, transactio func (a *Authority) selectDecrypter(ctx context.Context) (cert *x509.Certificate, pkey crypto.Decrypter, err error) { p := provisionerFromContext(ctx) - - // return provisioner specific decrypter, if available - if cert, pkey = p.GetDecrypter(); cert != nil && pkey != nil { + cert, pkey = p.GetDecrypter() + switch { + case cert != nil && pkey != nil: return + case cert == nil && pkey != nil: + return nil, nil, fmt.Errorf("provisioner %q does not have a decrypter certificate available", p.GetName()) + case cert != nil && pkey == nil: + return nil, nil, fmt.Errorf("provisioner %q does not have a decrypter available", p.GetName()) } - // fallback to the CA wide RSA decrypter, which is the - // intermediate CA. - cert = a.signerCertificate - pkey = a.defaultDecrypter - - if cert == nil || pkey == nil { - return nil, nil, fmt.Errorf("provisioner %q does not have a decrypter available", p.GetName()) + cert, pkey = a.signerCertificate, a.defaultDecrypter + switch { + case cert == nil && pkey != nil: + return nil, nil, fmt.Errorf("provisioner %q does not have a default decrypter certificate available", p.GetName()) + case cert != nil && pkey == nil: + return nil, nil, fmt.Errorf("provisioner %q does not have a default decrypter available", p.GetName()) } return @@ -506,19 +509,22 @@ func (a *Authority) selectDecrypter(ctx context.Context) (cert *x509.Certificate func (a *Authority) selectSigner(ctx context.Context) (cert *x509.Certificate, pkey crypto.PrivateKey, err error) { p := provisionerFromContext(ctx) - - // return provisioner specific signer, if available - if cert, pkey = p.GetSigner(); cert != nil && pkey != nil { + cert, pkey = p.GetSigner() + switch { + case cert != nil && pkey != nil: return + case cert == nil && pkey != nil: + return nil, nil, fmt.Errorf("provisioner %q does not have a signer certificate available", p.GetName()) + case cert != nil && pkey == nil: + return nil, nil, fmt.Errorf("provisioner %q does not have a signer available", p.GetName()) } - // fallback to the CA wide RSA signer, which is the - // intermediate CA. - cert = a.signerCertificate - pkey = a.defaultSigner - - if cert == nil || pkey == nil { - return nil, nil, fmt.Errorf("provisioner %q does not have a signer available", p.GetName()) + cert, pkey = a.signerCertificate, a.defaultSigner + switch { + case cert == nil && pkey != nil: + return nil, nil, fmt.Errorf("provisioner %q does not have a default signer certificate available", p.GetName()) + case cert != nil && pkey == nil: + return nil, nil, fmt.Errorf("provisioner %q does not have a default signer available", p.GetName()) } return