From 0377fe559b2bd9be96d6c02efc8d95226c8299a4 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Fri, 26 May 2023 23:52:24 +0200 Subject: [PATCH] Add basic version of provisioner specific SCEP decrypter --- api/api.go | 22 +++++++-- authority/authority.go | 22 ++++++++- authority/provisioner/scep.go | 47 +++++++++++++++++++ authority/provisioner/webhook.go | 3 ++ scep/api/api.go | 3 ++ scep/authority.go | 79 +++++++++++++++++++++----------- scep/options.go | 46 +++++++++++-------- scep/provisioner.go | 3 ++ scep/service.go | 16 ++++--- 9 files changed, 184 insertions(+), 57 deletions(-) diff --git a/api/api.go b/api/api.go index c9820351..0474471a 100644 --- a/api/api.go +++ b/api/api.go @@ -244,11 +244,25 @@ func (p ProvisionersResponse) MarshalJSON() ([]byte, error) { continue } - old := scepProv.ChallengePassword + type old struct { + challengePassword string + decrypterCertificate string + decrypterKey string + decrypterKeyPassword string + } + o := old{scepProv.ChallengePassword, scepProv.DecrypterCert, scepProv.DecrypterKey, scepProv.DecrypterKeyPassword} scepProv.ChallengePassword = "*** REDACTED ***" - defer func(p string) { //nolint:gocritic // defer in loop required to restore initial state of provisioners - scepProv.ChallengePassword = p - }(old) + // TODO: remove the details in the API response + // scepProv.DecrypterCert = "" + // scepProv.DecrypterKey = "" + // scepProv.DecrtyperKeyPassword = "" + + defer func(o old) { //nolint:gocritic // defer in loop required to restore initial state of provisioners + scepProv.ChallengePassword = o.challengePassword + scepProv.DecrypterCert = o.decrypterCertificate + scepProv.DecrypterKey = o.decrypterKey + scepProv.DecrypterKeyPassword = o.decrypterKeyPassword + }(o) } var list = struct { diff --git a/authority/authority.go b/authority/authority.go index ae85c018..ef51b61d 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "crypto" + "crypto/rsa" "crypto/sha256" "crypto/x509" "encoding/hex" @@ -666,13 +667,30 @@ func (a *Authority) init() error { return err } + options.SignerCert = options.CertificateChain[0] + options.DecrypterCert = options.CertificateChain[0] + + // TODO: instead of creating the decrypter here, pass the + // intermediate key + chain down to the SCEP service / authority, + // and only instantiate it when required there. + // TODO: if moving the logic, try improving the logic for the + // decrypter password too? if km, ok := a.keyManager.(kmsapi.Decrypter); ok { options.Decrypter, err = km.CreateDecrypter(&kmsapi.CreateDecrypterRequest{ DecryptionKey: a.config.IntermediateKey, Password: a.password, }) - if err != nil { - return err + if err == nil { + // when creating the decrypter fails, ignore the error + // TODO(hs): decide if this is OK. It could fail at startup, but it + // could be up later. Right now decryption would always fail. + key, ok := options.Decrypter.Public().(*rsa.PublicKey) + if !ok { + return errors.New("only RSA keys are currently supported as decrypters") + } + if !key.Equal(options.DecrypterCert.PublicKey) { + return errors.New("mismatch between decryption certificate and decrypter public keys") + } } } diff --git a/authority/provisioner/scep.go b/authority/provisioner/scep.go index b0acc8fe..77c02e8f 100644 --- a/authority/provisioner/scep.go +++ b/authority/provisioner/scep.go @@ -2,13 +2,19 @@ package provisioner import ( "context" + "crypto" + "crypto/rsa" "crypto/subtle" + "crypto/x509" "fmt" "net/http" "time" "github.com/pkg/errors" + "go.step.sm/crypto/kms" + kmsapi "go.step.sm/crypto/kms/apiv1" + "go.step.sm/crypto/pemutil" "go.step.sm/linkedca" "github.com/smallstep/certificates/webhook" @@ -32,6 +38,12 @@ type SCEP struct { // MinimumPublicKeyLength is the minimum length for public keys in CSRs MinimumPublicKeyLength int `json:"minimumPublicKeyLength,omitempty"` + // TODO + KMS *kms.Options `json:"kms,omitempty"` + DecrypterCert string `json:"decrypterCert"` + DecrypterKey string `json:"decrypterKey"` + DecrypterKeyPassword string `json:"decrypterKeyPassword"` + // Numerical identifier for the ContentEncryptionAlgorithm as defined in github.com/mozilla-services/pkcs7 // at https://github.com/mozilla-services/pkcs7/blob/33d05740a3526e382af6395d3513e73d4e66d1cb/encrypt.go#L63 // Defaults to 0, being DES-CBC @@ -41,6 +53,9 @@ type SCEP struct { ctl *Controller encryptionAlgorithm int challengeValidationController *challengeValidationController + keyManager kmsapi.KeyManager + decrypter crypto.Decrypter + decrypterCertificate *x509.Certificate } // GetID returns the provisioner unique identifier. @@ -177,6 +192,34 @@ func (s *SCEP) Init(config Config) (err error) { s.GetOptions().GetWebhooks(), ) + if s.KMS != nil { + if s.keyManager, err = kms.New(context.Background(), *s.KMS); err != nil { + return fmt.Errorf("failed initializing kms: %w", err) + } + km, ok := s.keyManager.(kmsapi.Decrypter) + if !ok { + return fmt.Errorf(`%q is not a kmsapi.Decrypter`, s.KMS.Type) + } + if s.DecrypterKey != "" || s.DecrypterCert != "" { + if s.decrypter, err = km.CreateDecrypter(&kmsapi.CreateDecrypterRequest{ + DecryptionKey: s.DecrypterKey, + Password: []byte(s.DecrypterKeyPassword), + }); err != nil { + return fmt.Errorf("failed creating decrypter: %w", err) + } + if s.decrypterCertificate, err = pemutil.ReadCertificate(s.DecrypterCert); err != nil { + return fmt.Errorf("failed reading certificate: %w", err) + } + 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? s.ctl, err = NewController(s, s.Claims, config, s.Options) @@ -259,3 +302,7 @@ func (s *SCEP) selectValidationMethod() validationMethod { } return validationMethodNone } + +func (s *SCEP) GetDecrypter() (*x509.Certificate, crypto.Decrypter) { + return s.decrypterCertificate, s.decrypter +} diff --git a/authority/provisioner/webhook.go b/authority/provisioner/webhook.go index cb15547d..3266e131 100644 --- a/authority/provisioner/webhook.go +++ b/authority/provisioner/webhook.go @@ -152,6 +152,8 @@ retry: return nil, err } + fmt.Println(req) + secret, err := base64.StdEncoding.DecodeString(w.Secret) if err != nil { return nil, err @@ -201,6 +203,7 @@ retry: time.Sleep(time.Second) goto retry } + fmt.Println(fmt.Sprintf("%#+v", resp)) if resp.StatusCode >= 400 { return nil, fmt.Errorf("Webhook server responded with %d", resp.StatusCode) } diff --git a/scep/api/api.go b/scep/api/api.go index 98da818b..00f693a8 100644 --- a/scep/api/api.go +++ b/scep/api/api.go @@ -308,6 +308,8 @@ func PKIOperation(ctx context.Context, req request) (Response, error) { transactionID := string(msg.TransactionID) challengePassword := msg.CSRReqMessage.ChallengePassword + fmt.Println("challenge password: ", challengePassword) + // NOTE: we're blocking the RenewalReq if the challenge does not match, because otherwise we don't have any authentication. // The macOS SCEP client performs renewals using PKCSreq. The CertNanny SCEP client will use PKCSreq with challenge too, it seems, // even if using the renewal flow as described in the README.md. MicroMDM SCEP client also only does PKCSreq by default, unless @@ -315,6 +317,7 @@ func PKIOperation(ctx context.Context, req request) (Response, error) { // We'll have to see how it works out. if msg.MessageType == microscep.PKCSReq || msg.MessageType == microscep.RenewalReq { if err := auth.ValidateChallenge(ctx, challengePassword, transactionID); err != nil { + fmt.Println(err) if errors.Is(err, provisioner.ErrSCEPChallengeInvalid) { return createFailureResponse(ctx, csr, msg, microscep.BadRequest, err) } diff --git a/scep/authority.go b/scep/authority.go index 23c28813..af9bdf42 100644 --- a/scep/authority.go +++ b/scep/authority.go @@ -2,6 +2,7 @@ package scep import ( "context" + "crypto" "crypto/x509" "errors" "fmt" @@ -18,12 +19,10 @@ import ( // Authority is the layer that handles all SCEP interactions. type Authority struct { - prefix string - dns string - intermediateCertificate *x509.Certificate - caCerts []*x509.Certificate // TODO(hs): change to use these instead of root and intermediate - service *Service - signAuth SignAuthority + prefix string + dns string + service *Service + signAuth SignAuthority } type authorityKey struct{} @@ -74,18 +73,8 @@ func New(signAuth SignAuthority, ops AuthorityOptions) (*Authority, error) { prefix: ops.Prefix, dns: ops.DNS, signAuth: signAuth, + service: ops.Service, } - - // 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, I think. - if ops.Service != nil { - authority.caCerts = ops.Service.certificateChain - // TODO(hs): look into refactoring SCEP into using just caCerts everywhere, if it makes sense for more elaborate SCEP configuration. Keeping it like this for clarity (for now). - authority.intermediateCertificate = ops.Service.certificateChain[0] - authority.service = ops.Service - } - return authority, nil } @@ -165,30 +154,46 @@ func (a *Authority) GetCACertificates(ctx context.Context) ([]*x509.Certificate, return nil, err } - if len(a.caCerts) == 0 { + if len(a.service.certificateChain) == 0 { return nil, errors.New("no intermediate certificate available in SCEP authority") } certs := []*x509.Certificate{} - certs = append(certs, a.caCerts[0]) + if decrypterCertificate, _ := p.GetDecrypter(); decrypterCertificate != nil { + certs = append(certs, decrypterCertificate) + certs = append(certs, a.service.signerCertificate) + } else { + certs = append(certs, a.service.defaultDecrypterCertificate) + } // NOTE: we're adding the CA roots here, but they are (highly likely) different than what the RFC means. // Clients are responsible to select the right cert(s) to use, though. - if p.ShouldIncludeRootInChain() && len(a.caCerts) > 1 { - certs = append(certs, a.caCerts[1]) + if p.ShouldIncludeRootInChain() && len(a.service.certificateChain) > 1 { + certs = append(certs, a.service.certificateChain[1]) } return certs, nil } // DecryptPKIEnvelope decrypts an enveloped message -func (a *Authority) DecryptPKIEnvelope(_ context.Context, msg *PKIMessage) error { +func (a *Authority) DecryptPKIEnvelope(ctx context.Context, msg *PKIMessage) error { p7c, err := pkcs7.Parse(msg.P7.Content) if err != nil { return fmt.Errorf("error parsing pkcs7 content: %w", err) } - envelope, err := p7c.Decrypt(a.intermediateCertificate, a.service.decrypter) + fmt.Println(fmt.Sprintf("%#+v", a.service.defaultDecrypterCertificate)) + fmt.Println(fmt.Sprintf("%#+v", a.service.defaultDecrypter)) + + cert, pkey, err := a.selectDecrypter(ctx) + if err != nil { + return fmt.Errorf("failed selecting decrypter: %w", err) + } + + fmt.Println(fmt.Sprintf("%#+v", cert)) + fmt.Println(fmt.Sprintf("%#+v", pkey)) + + envelope, err := p7c.Decrypt(cert, pkey) if err != nil { return fmt.Errorf("error decrypting encrypted pkcs7 content: %w", err) } @@ -208,6 +213,9 @@ func (a *Authority) DecryptPKIEnvelope(_ context.Context, msg *PKIMessage) error if err != nil { return fmt.Errorf("parse CSR from pkiEnvelope: %w", err) } + if err := csr.CheckSignature(); err != nil { + return fmt.Errorf("invalid CSR signature; %w", err) + } // check for challengePassword cp, err := microx509util.ParseChallengePassword(msg.pkiEnvelope) if err != nil { @@ -226,6 +234,24 @@ func (a *Authority) DecryptPKIEnvelope(_ context.Context, msg *PKIMessage) error return nil } +func (a *Authority) selectDecrypter(ctx context.Context) (cert *x509.Certificate, pkey crypto.PrivateKey, err error) { + p, err := provisionerFromContext(ctx) + if err != nil { + return nil, nil, err + } + + // return provisioner specific decrypter, if available + if cert, pkey = p.GetDecrypter(); cert != nil && pkey != nil { + return + } + + // fallback to the CA wide decrypter + cert = a.service.defaultDecrypterCertificate + pkey = a.service.defaultDecrypter + + return +} + // SignCSR creates an x509.Certificate based on a CSR template and Cert Authority credentials // returns a new PKIMessage with CertRep data func (a *Authority) SignCSR(ctx context.Context, csr *x509.CertificateRequest, msg *PKIMessage) (*PKIMessage, error) { @@ -358,10 +384,11 @@ func (a *Authority) SignCSR(ctx context.Context, csr *x509.CertificateRequest, m // as the first certificate in the array signedData.AddCertificate(cert) - authCert := a.intermediateCertificate + authCert := a.service.signerCertificate + signer := a.service.signer // sign the attributes - if err := signedData.AddSigner(authCert, a.service.signer, config); err != nil { + if err := signedData.AddSigner(authCert, signer, config); err != nil { return nil, err } @@ -429,7 +456,7 @@ func (a *Authority) CreateFailureResponse(_ context.Context, _ *x509.Certificate } // sign the attributes - if err := signedData.AddSigner(a.intermediateCertificate, a.service.signer, config); err != nil { + if err := signedData.AddSigner(a.service.signerCertificate, a.service.signer, config); err != nil { return nil, err } diff --git a/scep/options.go b/scep/options.go index 201f1beb..00662ae9 100644 --- a/scep/options.go +++ b/scep/options.go @@ -2,7 +2,6 @@ package scep import ( "crypto" - "crypto/rsa" "crypto/x509" "github.com/pkg/errors" @@ -12,6 +11,8 @@ type Options struct { // CertificateChain is the issuer certificate, along with any other bundled certificates // to be returned in the chain for consumers. Configured in the ca.json crt property. CertificateChain []*x509.Certificate + SignerCert *x509.Certificate + DecrypterCert *x509.Certificate // Signer signs CSRs in SCEP. Configured in the ca.json key property. Signer crypto.Signer `json:"-"` // Decrypter decrypts encrypted SCEP messages. Configured in the ca.json key property. @@ -35,36 +36,43 @@ func (o *Options) Validate() error { // Other algorithms than RSA 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. - intermediate := o.CertificateChain[0] - if intermediate.PublicKeyAlgorithm != x509.RSA { - return errors.New("only the RSA algorithm is (currently) supported") - } + //intermediate := o.CertificateChain[0] + //intermediate := o.SignerCert + // if intermediate.PublicKeyAlgorithm != x509.RSA { + // return errors.New("only the RSA algorithm is (currently) supported") + // } // TODO: add checks for key usage? - signerPublicKey, ok := o.Signer.Public().(*rsa.PublicKey) - if !ok { - return errors.New("only RSA public keys are (currently) supported as signers") - } + //signerPublicKey, ok := o.Signer.Public().(*rsa.PublicKey) + // if !ok { + // return errors.New("only RSA public keys are (currently) supported as signers") + // } // check if the intermediate ca certificate has the same public key as the signer. // According to the RFC it seems valid to have different keys for the intermediate // and the CA signing new certificates, so this might change in the future. - if !signerPublicKey.Equal(intermediate.PublicKey) { - return errors.New("mismatch between certificate chain and signer public keys") - } + // if !signerPublicKey.Equal(intermediate.PublicKey) { + // return errors.New("mismatch between certificate chain and signer public keys") + // } - decrypterPublicKey, ok := o.Decrypter.Public().(*rsa.PublicKey) - if !ok { - return errors.New("only RSA public keys are (currently) supported as decrypters") - } + // TODO: this could be a different decrypter, based on the value + // in the provisioner. + // decrypterPublicKey, ok := o.Decrypter.Public().(*rsa.PublicKey) + // if !ok { + // return errors.New("only RSA public 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. Currently that's not supported. - if !decrypterPublicKey.Equal(intermediate.PublicKey) { - return errors.New("mismatch between certificate chain and decrypter public keys") - } + // if !decrypterPublicKey.Equal(intermediate.PublicKey) { + // return errors.New("mismatch between certificate chain and decrypter public keys") + // } + + // if !decrypterPublicKey.Equal(o.DecrypterCert.PublicKey) { + // return errors.New("mismatch between certificate chain and decrypter public keys") + // } return nil } diff --git a/scep/provisioner.go b/scep/provisioner.go index 8120057e..cb41ed47 100644 --- a/scep/provisioner.go +++ b/scep/provisioner.go @@ -2,6 +2,8 @@ package scep import ( "context" + "crypto" + "crypto/x509" "time" "github.com/smallstep/certificates/authority/provisioner" @@ -16,6 +18,7 @@ type Provisioner interface { GetOptions() *provisioner.Options GetCapabilities() []string ShouldIncludeRootInChain() bool + GetDecrypter() (*x509.Certificate, crypto.Decrypter) GetContentEncryptionAlgorithm() int ValidateChallenge(ctx context.Context, challenge, transactionID string) error } diff --git a/scep/service.go b/scep/service.go index 85f7c73f..ffb4166a 100644 --- a/scep/service.go +++ b/scep/service.go @@ -8,9 +8,11 @@ import ( // Service is a wrapper for crypto.Signer and crypto.Decrypter type Service struct { - certificateChain []*x509.Certificate - signer crypto.Signer - decrypter crypto.Decrypter + certificateChain []*x509.Certificate + signerCertificate *x509.Certificate + signer crypto.Signer + defaultDecrypterCertificate *x509.Certificate + defaultDecrypter crypto.Decrypter } // NewService returns a new Service type. @@ -21,8 +23,10 @@ func NewService(_ context.Context, opts Options) (*Service, error) { // TODO: should this become similar to the New CertificateAuthorityService as in x509CAService? return &Service{ - certificateChain: opts.CertificateChain, - signer: opts.Signer, - decrypter: opts.Decrypter, + certificateChain: opts.CertificateChain, + signerCertificate: opts.SignerCert, + signer: opts.Signer, + defaultDecrypterCertificate: opts.DecrypterCert, + defaultDecrypter: opts.Decrypter, }, nil }