diff --git a/api/api.go b/api/api.go index 849fd24f..b91aef97 100644 --- a/api/api.go +++ b/api/api.go @@ -244,9 +244,6 @@ func (p ProvisionersResponse) MarshalJSON() ([]byte, error) { continue } - fmt.Println(scepProv.KMS) - fmt.Println(fmt.Sprintf("%#+v", scepProv)) - type old struct { challengePassword string decrypterCertificate []byte @@ -255,10 +252,9 @@ func (p ProvisionersResponse) MarshalJSON() ([]byte, error) { } o := old{scepProv.ChallengePassword, scepProv.DecrypterCertificate, scepProv.DecrypterKey, scepProv.DecrypterKeyPassword} scepProv.ChallengePassword = "*** REDACTED ***" - // TODO: remove the details in the API response - // scepProv.DecrypterCert = "" - // scepProv.DecrypterKey = "" - // scepProv.DecrtyperKeyPassword = "" + scepProv.DecrypterCertificate = []byte("*** REDACTED ***") + scepProv.DecrypterKey = "*** REDACTED ***" + scepProv.DecrypterKeyPassword = "*** REDACTED ***" defer func(o old) { //nolint:gocritic // defer in loop required to restore initial state of provisioners scepProv.ChallengePassword = o.challengePassword diff --git a/authority/authority.go b/authority/authority.go index 8b93a634..7cd0f2ac 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -262,6 +262,14 @@ func (a *Authority) ReloadAdminResources(ctx context.Context) error { a.config.AuthorityConfig.Admins = adminList a.admins = adminClxn + // update the SCEP service with the currently active SCEP + // provisioner names. + // TODO(hs): trigger SCEP authority (re)validation using + // the current set of SCEP provisioners. + if a.scepService != nil { + a.scepService.UpdateProvisioners(a.getSCEPProvisionerNames()) + } + return nil } @@ -643,7 +651,7 @@ func (a *Authority) init() error { // The SCEP functionality is provided through an instance of // scep.Service. It is initialized once when the CA is started. - // TODO: should the SCEP service support reloading? For example, + // TODO(hs): should the SCEP service support reloading? For example, // when the admin resources are reloaded, specifically the provisioners, // it can happen that the SCEP service is no longer required and can // be destroyed, or that it needs to be instantiated. It may also need @@ -664,11 +672,11 @@ func (a *Authority) init() error { return err } - // TODO: instead of creating the decrypter here, pass the + // TODO(hs): instead of creating the decrypter here, pass the // intermediate key + chain down to the SCEP service / authority, // and only instantiate it when required there. Is that possible? // Also with entering passwords? - // TODO: if moving the logic, try improving the logic for the + // TODO(hs): if moving the logic, try improving the logic for the // decrypter password too? Right now it needs to be entered multiple // times; I've observed it to be three times maximum, every time // the intermediate key is read. @@ -678,11 +686,16 @@ func (a *Authority) init() error { DecryptionKey: a.config.IntermediateKey, Password: a.password, }); err == nil { - // only pass the decrypter down when it was successfully created + // only pass the decrypter down when it was successfully created, + // meaning it's an RSA key, and `CreateDecrypter` did not fail. options.Decrypter = decrypter } } + // provide the current SCEP provisioner names, so that the provisioners + // can be validated when the CA is started. + options.SCEPProvisionerNames = a.getSCEPProvisionerNames() + a.scepService, err = scep.NewService(ctx, options) if err != nil { return err @@ -849,6 +862,15 @@ func (a *Authority) requiresSCEP() bool { return false } +func (a *Authority) getSCEPProvisionerNames() (names []string) { + for _, p := range a.config.AuthorityConfig.Provisioners { + if p.GetType() == provisioner.TypeSCEP { + names = append(names, p.GetName()) + } + } + return +} + // GetSCEP returns the configured SCEP Service. func (a *Authority) GetSCEP() *scep.Service { return a.scepService diff --git a/authority/provisioner/scep.go b/authority/provisioner/scep.go index 192c75b1..7b780d6a 100644 --- a/authority/provisioner/scep.go +++ b/authority/provisioner/scep.go @@ -208,10 +208,9 @@ func (s *SCEP) Init(config Config) (err error) { return fmt.Errorf("failed creating decrypter: %w", err) } - // Parse decrypter certificate + // parse the decrypter certificate block, rest := pem.Decode(s.DecrypterCertificate) if len(rest) > 0 { - fmt.Println(string(rest)) return errors.New("failed parsing decrypter certificate: trailing data") } if block == nil { @@ -221,9 +220,7 @@ func (s *SCEP) Init(config Config) (err error) { return fmt.Errorf("failed parsing decrypter certificate: %w", err) } - // if s.decrypterCertificate, err = pemutil.ReadCertificate(s.DecrypterCertFile); err != nil { - // return fmt.Errorf("failed reading certificate: %w", err) - // } + // validate the decrypter key decrypterPublicKey, ok := s.decrypter.Public().(*rsa.PublicKey) if !ok { return fmt.Errorf("only RSA keys are supported") diff --git a/authority/provisioner/webhook.go b/authority/provisioner/webhook.go index 3266e131..cb15547d 100644 --- a/authority/provisioner/webhook.go +++ b/authority/provisioner/webhook.go @@ -152,8 +152,6 @@ retry: return nil, err } - fmt.Println(req) - secret, err := base64.StdEncoding.DecodeString(w.Secret) if err != nil { return nil, err @@ -203,7 +201,6 @@ 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/ca/ca.go b/ca/ca.go index a3f261e7..6b9a2a13 100644 --- a/ca/ca.go +++ b/ca/ca.go @@ -256,12 +256,17 @@ func (ca *CA) Init(cfg *config.Config) (*CA, error) { return nil, errors.Wrap(err, "failed creating SCEP authority") } - // TODO: validate that the scepAuthority is fully valid? E.g. initialization - // may have configured the default decrypter, but if that's not set or if it's - // somehow not usable, all SCEP provisioners should have a valid decrypter - // configured by now. + // 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. + shouldFail := false if err := scepAuthority.Validate(); err != nil { - return nil, errors.Wrap(err, "failed validating SCEP authority") + err = errors.Wrap(err, "failed validating SCEP authority") + if shouldFail { + return nil, err + } + log.Println(err) } // According to the RFC (https://tools.ietf.org/html/rfc8894#section-7.10), diff --git a/scep/api/api.go b/scep/api/api.go index 00f693a8..98da818b 100644 --- a/scep/api/api.go +++ b/scep/api/api.go @@ -308,8 +308,6 @@ 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 @@ -317,7 +315,6 @@ 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 14e5cd1c..6f2387c1 100644 --- a/scep/authority.go +++ b/scep/authority.go @@ -67,22 +67,24 @@ func New(signAuth SignAuthority, ops AuthorityOptions) (*Authority, error) { return authority, nil } +// Validate validates if the SCEP Authority has a valid configuration. +// 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 default decrypter is set, the Authority is able - // to decrypt SCEP requests. No need to verify the provisioners. - if a.service.defaultDecrypter != nil { - return nil - } - - for _, name := range []string{"scepca"} { // TODO: correct names; provided through options + noDefaultDecrypterAvailable := a.service.defaultDecrypter == nil + for _, name := range a.service.scepProvisionerNames { p, err := a.LoadProvisionerByName(name) if err != nil { - fmt.Println("prov load fail: %w", err) + return fmt.Errorf("failed loading provisioner %q: %w", name, err) } if scepProv, ok := p.(*provisioner.SCEP); ok { - if cert, decrypter := scepProv.GetDecrypter(); cert == nil || decrypter == nil { - fmt.Println(fmt.Sprintf("SCEP provisioner %q doesn't have valid decrypter", scepProv.GetName())) - // TODO: return error + cert, decrypter := scepProv.GetDecrypter() + // TODO: return sentinel/typed error, to be able to ignore/log these cases during init? + if cert == nil && noDefaultDecrypterAvailable { + return fmt.Errorf("SCEP provisioner %q does not have a decrypter certificate", name) + } + if decrypter == nil && noDefaultDecrypterAvailable { + return fmt.Errorf("SCEP provisioner %q does not have decrypter", name) } } } @@ -153,17 +155,11 @@ func (a *Authority) DecryptPKIEnvelope(ctx context.Context, msg *PKIMessage) err return fmt.Errorf("error parsing pkcs7 content: %w", err) } - fmt.Println(fmt.Sprintf("%#+v", a.service.signerCertificate)) - 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) diff --git a/scep/options.go b/scep/options.go index 43f41fba..7ba7cfc2 100644 --- a/scep/options.go +++ b/scep/options.go @@ -20,6 +20,10 @@ type Options struct { Signer crypto.Signer `json:"-"` // Decrypter decrypts encrypted SCEP messages. Configured in the ca.json key property. Decrypter crypto.Decrypter `json:"-"` + // SCEPProvisionerNames contains the currently configured SCEP provioner names. These + // are used to be able to load the provisioners when the SCEP authority is being + // validated. + SCEPProvisionerNames []string } type comparablePublicKey interface { diff --git a/scep/service.go b/scep/service.go index f3a6d097..60d4c8b2 100644 --- a/scep/service.go +++ b/scep/service.go @@ -10,11 +10,12 @@ import ( // decrypting SCEP requests and signing certificates in response to // SCEP certificate requests. type Service struct { - roots []*x509.Certificate - intermediates []*x509.Certificate - signerCertificate *x509.Certificate - signer crypto.Signer - defaultDecrypter crypto.Decrypter + roots []*x509.Certificate + intermediates []*x509.Certificate + signerCertificate *x509.Certificate + signer crypto.Signer + defaultDecrypter crypto.Decrypter + scepProvisionerNames []string } // NewService returns a new Service type. @@ -23,10 +24,15 @@ func NewService(_ context.Context, opts Options) (*Service, error) { return nil, err } return &Service{ - roots: opts.Roots, - intermediates: opts.Intermediates, - signerCertificate: opts.SignerCert, - signer: opts.Signer, - defaultDecrypter: opts.Decrypter, + roots: opts.Roots, + intermediates: opts.Intermediates, + signerCertificate: opts.SignerCert, + signer: opts.Signer, + defaultDecrypter: opts.Decrypter, + scepProvisionerNames: opts.SCEPProvisionerNames, }, nil } + +func (s *Service) UpdateProvisioners(scepProvisionerNames []string) { + s.scepProvisionerNames = scepProvisionerNames +}