Clean up the SCEP authority and provisioner

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

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

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")

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)
 	}

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),

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)
 			}

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)

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 {

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
+}