Refactor SCEP authority initialization and clean some code

authority/authority.go

@@ -442,13 +442,14 @@ func (a *Authority) init() error {
 	audiences := a.config.GetAudiences()
 	a.provisioners = provisioner.NewCollection(audiences)
 	config := provisioner.Config{
-		// TODO: this probably shouldn't happen like this; via SignAuth instead?
-		IntermediateCert: a.config.IntermediateCert,
-		SigningKey:       a.config.IntermediateKey,
-		CACertificates:   a.rootX509Certs,
-		Claims:           claimer.Claims(),
-		Audiences:        audiences,
-		DB:               a.db,
+		// TODO: I'm not sure if extending this configuration is a good way to integrate
+		// It's powerful, but leaks quite some seemingly internal stuff to the provisioner.
+		// IntermediateCert: a.config.IntermediateCert,
+		// SigningKey:       a.config.IntermediateKey,
+		// CACertificates:   a.rootX509Certs,
+		Claims:    claimer.Claims(),
+		Audiences: audiences,
+		DB:        a.db,
 		SSHKeys: &provisioner.SSHKeys{
 			UserKeys: sshKeys.UserKeys,
 			HostKeys: sshKeys.HostKeys,

authority/provisioner/collection.go

@@ -157,6 +157,8 @@ func (c *Collection) LoadByCertificate(cert *x509.Certificate) (Interface, bool)
 				return c.Load("gcp/" + string(provisioner.Name))
 			case TypeACME:
 				return c.Load("acme/" + string(provisioner.Name))
+			case TypeSCEP:
+				return c.Load("scep/" + string(provisioner.Name))
 			case TypeX5C:
 				return c.Load("x5c/" + string(provisioner.Name))
 			case TypeK8sSA:

authority/provisioner/provisioner.go

@@ -183,10 +183,6 @@ type SSHKeys struct {
 // Config defines the default parameters used in the initialization of
 // provisioners.
 type Config struct {
-	// TODO: these probably shouldn't be here but passed via SignAuth
-	IntermediateCert string
-	SigningKey       string
-	CACertificates   []*x509.Certificate
 	// Claims are the default claims.
 	Claims Claims
 	// Audiences are the audiences used in the default provisioner, (JWK).

authority/provisioner/scep.go

@@ -1,11 +1,7 @@
 package provisioner
 
 import (
-	"crypto/rsa"
-	"crypto/x509"
-	"encoding/pem"
-	"fmt"
-	"io/ioutil"
+	"time"
 
 	"github.com/pkg/errors"
 )
@@ -16,14 +12,11 @@ type SCEP struct {
 	*base
 	Type string `json:"type"`
 	Name string `json:"name"`
-	// ForceCN bool     `json:"forceCN,omitempty"`
-	// Claims  *Claims  `json:"claims,omitempty"`
-	// Options *Options `json:"options,omitempty"`
-	// claimer *Claimer
 
-	IntermediateCert string
-	SigningKey       string
-	CACertificates   []*x509.Certificate
+	// ForceCN bool     `json:"forceCN,omitempty"`
+	Options *Options `json:"options,omitempty"`
+	Claims  *Claims  `json:"claims,omitempty"`
+	claimer *Claimer
 }
 
 // GetID returns the provisioner unique identifier.
@@ -51,40 +44,15 @@ func (s *SCEP) GetTokenID(ott string) (string, error) {
 	return "", errors.New("scep provisioner does not implement GetTokenID")
 }
 
-// GetCACertificates returns the CA certificate chain
-// TODO: this should come from the authority instead?
-func (s *SCEP) GetCACertificates() []*x509.Certificate {
-
-	pemtxt, _ := ioutil.ReadFile(s.IntermediateCert) // TODO: move reading key to init? That's probably safer.
-	block, _ := pem.Decode([]byte(pemtxt))
-	cert, err := x509.ParseCertificate(block.Bytes)
-	if err != nil {
-		fmt.Println(err)
-	}
-
-	// TODO: return chain? I'm not sure if the client understands it correctly
-	return []*x509.Certificate{cert}
+// GetOptions returns the configured provisioner options.
+func (s *SCEP) GetOptions() *Options {
+	return s.Options
 }
 
-func (s *SCEP) GetSigningKey() *rsa.PrivateKey {
-
-	keyBytes, err := ioutil.ReadFile(s.SigningKey)
-	if err != nil {
-		return nil
-	}
-
-	block, _ := pem.Decode([]byte(keyBytes))
-	if block == nil {
-		return nil
-	}
-
-	key, err := x509.ParsePKCS1PrivateKey(block.Bytes)
-	if err != nil {
-		fmt.Println(err)
-		return nil
-	}
-
-	return key
+// DefaultTLSCertDuration returns the default TLS cert duration enforced by
+// the provisioner.
+func (s *SCEP) DefaultTLSCertDuration() time.Duration {
+	return s.claimer.DefaultTLSCertDuration()
 }
 
 // Init initializes and validates the fields of a JWK type.
@@ -97,14 +65,10 @@ func (s *SCEP) Init(config Config) (err error) {
 		return errors.New("provisioner name cannot be empty")
 	}
 
-	// // Update claims with global ones
-	// if p.claimer, err = NewClaimer(p.Claims, config.Claims); err != nil {
-	// 	return err
-	// }
-
-	s.IntermediateCert = config.IntermediateCert
-	s.SigningKey = config.SigningKey
-	s.CACertificates = config.CACertificates
+	// Update claims with global ones
+	if s.claimer, err = NewClaimer(s.Claims, config.Claims); err != nil {
+		return err
+	}
 
 	return err
 }

ca/ca.go

@@ -23,6 +23,7 @@ import (
 	"github.com/smallstep/certificates/logging"
 	"github.com/smallstep/certificates/monitoring"
 	"github.com/smallstep/certificates/scep"
+	scepAPI "github.com/smallstep/certificates/scep/api"
 	"github.com/smallstep/certificates/server"
 	"github.com/smallstep/nosql"
 )
@@ -180,8 +181,8 @@ func (ca *CA) Init(config *config.Config) (*CA, error) {
 			mgmtHandler.Route(r)
 		})
 	}
-	if ca.shouldServeSCEPEndpoints() {
 
+	if ca.shouldServeSCEPEndpoints() {
 		scepPrefix := "scep"
 		scepAuthority, err := scep.New(auth, scep.AuthorityOptions{
 			Service: auth.GetSCEPService(),

scep/api/api.go

@@ -1,4 +1,4 @@
-package scep
+package api
 
 import (
 	"context"
@@ -19,17 +19,38 @@ import (
 	"github.com/smallstep/certificates/acme"
 	"github.com/smallstep/certificates/api"
 	"github.com/smallstep/certificates/authority/provisioner"
+	"github.com/smallstep/certificates/scep"
 
 	microscep "github.com/micromdm/scep/scep"
 )
 
-// Handler is the ACME request handler.
+const (
+	opnGetCACert    = "GetCACert"
+	opnGetCACaps    = "GetCACaps"
+	opnPKIOperation = "PKIOperation"
+)
+
+// SCEPRequest is a SCEP server request.
+type SCEPRequest struct {
+	Operation string
+	Message   []byte
+}
+
+// SCEPResponse is a SCEP server response.
+type SCEPResponse struct {
+	Operation string
+	CACertNum int
+	Data      []byte
+	Err       error
+}
+
+// Handler is the SCEP request handler.
 type Handler struct {
-	Auth Interface
+	Auth scep.Interface
 }
 
-// New returns a new ACME API router.
-func NewAPI(scepAuth Interface) api.RouterHandler {
+// New returns a new SCEP API router.
+func New(scepAuth scep.Interface) api.RouterHandler {
 	return &Handler{scepAuth}
 }
 
@@ -156,7 +177,6 @@ type nextHTTP = func(http.ResponseWriter, *http.Request)
 // Responds 404 if the provisioner does not exist.
 func (h *Handler) lookupProvisioner(next nextHTTP) nextHTTP {
 	return func(w http.ResponseWriter, r *http.Request) {
-		ctx := r.Context()
 
 		// TODO: make this configurable; and we might want to look at being able to provide multiple,
 		// like the actual ACME one? The below assumes a SCEP provider (scep/) called "scep1" exists.
@@ -168,27 +188,23 @@ func (h *Handler) lookupProvisioner(next nextHTTP) nextHTTP {
 
 		scepProvisioner, ok := p.(*provisioner.SCEP)
 		if !ok {
-			api.WriteError(w, acme.AccountDoesNotExistErr(errors.New("provisioner must be of type SCEP")))
+			api.WriteError(w, errors.New("provisioner must be of type SCEP"))
 			return
 		}
 
-		ctx = context.WithValue(ctx, acme.ProvisionerContextKey, Provisioner(scepProvisioner))
+		ctx := r.Context()
+		ctx = context.WithValue(ctx, acme.ProvisionerContextKey, scep.Provisioner(scepProvisioner))
 		next(w, r.WithContext(ctx))
 	}
 }
 
 func (h *Handler) GetCACert(w http.ResponseWriter, r *http.Request, scepResponse SCEPResponse) error {
 
-	ctx := r.Context()
-
-	p, err := ProvisionerFromContext(ctx)
+	certs, err := h.Auth.GetCACertificates()
 	if err != nil {
 		return err
 	}
 
-	// TODO: get the CA Certificates from the (signing) authority instead? I think that should be doable
-	certs := p.GetCACertificates()
-
 	if len(certs) == 0 {
 		scepResponse.CACertNum = 0
 		scepResponse.Err = errors.New("missing CA Cert")
@@ -226,13 +242,16 @@ func (h *Handler) PKIOperation(w http.ResponseWriter, r *http.Request, scepReque
 		return err
 	}
 
-	ctx := r.Context()
-	p, err := ProvisionerFromContext(ctx)
+	certs, err := h.Auth.GetCACertificates()
+	if err != nil {
+		return err
+	}
+
+	// TODO: instead of getting the key to decrypt, add a decrypt function to the auth; less leaky
+	key, err := h.Auth.GetSigningKey()
 	if err != nil {
 		return err
 	}
-	certs := p.GetCACertificates()
-	key := p.GetSigningKey()
 
 	ca := certs[0]
 	if err := msg.DecryptPKIEnvelope(ca, key); err != nil {
@@ -276,10 +295,12 @@ func (h *Handler) PKIOperation(w http.ResponseWriter, r *http.Request, scepReque
 	//name := certName(cert)
 
 	// TODO: check if CN already exists, if renewal is allowed and if existing should be revoked; fail if not
-	// TODO: store the new cert for CN locally
+	// TODO: store the new cert for CN locally; should go into the DB
 
 	scepResponse.Data = certRep.Raw
 
+	api.LogCertificate(w, certRep.Certificate)
+
 	return writeSCEPResponse(w, scepResponse)
 }
 
@@ -354,14 +375,14 @@ func contentHeader(operation string, certNum int) string {
 
 // ProvisionerFromContext searches the context for a provisioner. Returns the
 // provisioner or an error.
-func ProvisionerFromContext(ctx context.Context) (Provisioner, error) {
+func ProvisionerFromContext(ctx context.Context) (scep.Provisioner, error) {
 	val := ctx.Value(acme.ProvisionerContextKey)
 	if val == nil {
-		return nil, acme.ServerInternalErr(errors.New("provisioner expected in request context"))
+		return nil, errors.New("provisioner expected in request context")
 	}
-	pval, ok := val.(Provisioner)
+	pval, ok := val.(scep.Provisioner)
 	if !ok || pval == nil {
-		return nil, acme.ServerInternalErr(errors.New("provisioner in context is not a SCEP provisioner"))
+		return nil, errors.New("provisioner in context is not a SCEP provisioner")
 	}
 	return pval, nil
 }

scep/authority.go

@@ -1,9 +1,15 @@
 package scep
 
 import (
+	"crypto/rsa"
 	"crypto/x509"
+	"encoding/pem"
+	"errors"
+	"io/ioutil"
 
 	"github.com/smallstep/certificates/authority/provisioner"
+	database "github.com/smallstep/certificates/db"
+	"go.step.sm/crypto/pemutil"
 
 	"github.com/smallstep/nosql"
 )
@@ -34,23 +40,33 @@ type Interface interface {
 	// GetLink(ctx context.Context, linkType Link, absoluteLink bool, inputs ...string) string
 	// GetLinkExplicit(linkType Link, provName string, absoluteLink bool, baseURL *url.URL, inputs ...string) string
 
-	GetCACerts() ([]*x509.Certificate, error)
+	GetCACertificates() ([]*x509.Certificate, error)
+	GetSigningKey() (*rsa.PrivateKey, error)
 }
 
 // Authority is the layer that handles all SCEP interactions.
 type Authority struct {
-	//certificates []*x509.Certificate
-	//authConfig   authority.AuthConfig
 	backdate provisioner.Duration
 	db       nosql.DB
+	prefix   string
+	dns      string
+
 	// dir      *directory
+
+	intermediateCertificate *x509.Certificate
+	intermediateKey         *rsa.PrivateKey
+
+	//signer crypto.Signer
+
 	signAuth SignAuthority
 }
 
 // AuthorityOptions required to create a new SCEP Authority.
 type AuthorityOptions struct {
-	Certificates []*x509.Certificate
-	//AuthConfig authority.AuthConfig
+	IntermediateCertificatePath string
+	IntermediateKeyPath         string
+
+	// Backdate
 	Backdate provisioner.Duration
 	// DB is the database used by nosql.
 	DB nosql.DB
@@ -69,17 +85,83 @@ type SignAuthority interface {
 	LoadProvisionerByID(string) (provisioner.Interface, error)
 }
 
+// New returns a new Authority that implements the SCEP interface.
+func New(signAuth SignAuthority, ops AuthorityOptions) (*Authority, error) {
+	if _, ok := ops.DB.(*database.SimpleDB); !ok {
+		// TODO: see ACME implementation
+	}
+
+	// TODO: the below is a bit similar as what happens in the core Authority class, which
+	// creates the full x509 service. However, those aren't accessible directly, which is
+	// why I reimplemented this (for now). There might be an alternative that I haven't
+	// found yet.
+	certificateChain, err := pemutil.ReadCertificateBundle(ops.IntermediateCertificatePath)
+	if err != nil {
+		return nil, err
+	}
+
+	intermediateKey, err := readPrivateKey(ops.IntermediateKeyPath)
+	if err != nil {
+		return nil, err
+	}
+
+	return &Authority{
+		backdate:                ops.Backdate,
+		db:                      ops.DB,
+		prefix:                  ops.Prefix,
+		dns:                     ops.DNS,
+		intermediateCertificate: certificateChain[0],
+		intermediateKey:         intermediateKey,
+		signAuth:                signAuth,
+	}, nil
+}
+
+func readPrivateKey(path string) (*rsa.PrivateKey, error) {
+
+	keyBytes, err := ioutil.ReadFile(path)
+	if err != nil {
+		return nil, err
+	}
+
+	block, _ := pem.Decode([]byte(keyBytes))
+	if block == nil {
+		return nil, nil
+	}
+
+	key, err := x509.ParsePKCS1PrivateKey(block.Bytes)
+	if err != nil {
+		return nil, err
+	}
+
+	return key, nil
+}
+
 // LoadProvisionerByID calls out to the SignAuthority interface to load a
 // provisioner by ID.
 func (a *Authority) LoadProvisionerByID(id string) (provisioner.Interface, error) {
 	return a.signAuth.LoadProvisionerByID(id)
 }
 
-func (a *Authority) GetCACerts() ([]*x509.Certificate, error) {
+// GetCACertificates returns the certificate (chain) for the CA
+func (a *Authority) GetCACertificates() ([]*x509.Certificate, error) {
+
+	if a.intermediateCertificate == nil {
+		return nil, errors.New("no intermediate certificate available in SCEP authority")
+	}
+
+	return []*x509.Certificate{a.intermediateCertificate}, nil
+}
+
+// GetSigningKey returns the RSA private key for the CA
+// TODO: we likely should provide utility functions for decrypting and
+// signing instead of providing the signing key directly
+func (a *Authority) GetSigningKey() (*rsa.PrivateKey, error) {
 
-	// TODO: implement the SCEP authority
+	if a.intermediateKey == nil {
+		return nil, errors.New("no intermediate key available in SCEP authority")
+	}
 
-	return []*x509.Certificate{}, nil
+	return a.intermediateKey, nil
 }
 
 // Interface guards

scep/provisioner.go

@@ -1,17 +1,16 @@
 package scep
 
 import (
-	"crypto/rsa"
-	"crypto/x509"
+	"time"
+
+	"github.com/smallstep/certificates/authority/provisioner"
 )
 
 // Provisioner is an interface that implements a subset of the provisioner.Interface --
 // only those methods required by the SCEP api/authority.
 type Provisioner interface {
 	// AuthorizeSign(ctx context.Context, token string) ([]provisioner.SignOption, error)
-	// GetName() string
-	// DefaultTLSCertDuration() time.Duration
-	// GetOptions() *provisioner.Options
-	GetCACertificates() []*x509.Certificate
-	GetSigningKey() *rsa.PrivateKey
+	GetName() string
+	DefaultTLSCertDuration() time.Duration
+	GetOptions() *provisioner.Options
 }

scep/scep.go

@@ -1,38 +0,0 @@
-package scep
-
-import (
-	database "github.com/smallstep/certificates/db"
-)
-
-const (
-	opnGetCACert    = "GetCACert"
-	opnGetCACaps    = "GetCACaps"
-	opnPKIOperation = "PKIOperation"
-)
-
-// New returns a new Authority that implements the SCEP interface.
-func New(signAuth SignAuthority, ops AuthorityOptions) (*Authority, error) {
-	if _, ok := ops.DB.(*database.SimpleDB); !ok {
-		// TODO: see ACME implementation
-	}
-	return &Authority{
-		//certificates: ops.Certificates,
-		backdate: ops.Backdate,
-		db:       ops.DB,
-		signAuth: signAuth,
-	}, nil
-}
-
-// SCEPRequest is a SCEP server request.
-type SCEPRequest struct {
-	Operation string
-	Message   []byte
-}
-
-// SCEPResponse is a SCEP server response.
-type SCEPResponse struct {
-	Operation string
-	CACertNum int
-	Data      []byte
-	Err       error
-}