Add non-TLS server and improve crypto.Decrypter interface

A server without TLS was added to serve the SCEP endpoints. According
to the RFC, SCEP has to be served via HTTP. The `sscep` client, for
example, will stop any URL that does not start with `http://` from
being used, so serving SCEP seems to be the right way to do it.

This commit adds a second server for which no TLS configuration is
configured. A distinct field in the configuration, `insecureAddress`
was added to specify the address for the insecure server.

The SCEP endpoints will also still be served via HTTPS. Some clients
may be able to work with that.

This commit also improves how the crypto.Decrypter interface is
handled for the different types of KMSes supported by step. The
apiv1.Decrypter interface was added. Currently only SoftKMS
implements this interface, providing a crypto.Decrypter required
for SCEP operations.

authority/authority.go

@@ -18,6 +18,7 @@ import (
 	casapi "github.com/smallstep/certificates/cas/apiv1"
 	"github.com/smallstep/certificates/db"
 	"github.com/smallstep/certificates/kms"
+	"github.com/smallstep/certificates/kms/apiv1"
 	kmsapi "github.com/smallstep/certificates/kms/apiv1"
 	"github.com/smallstep/certificates/kms/sshagentkms"
 	"github.com/smallstep/certificates/templates"
@@ -201,13 +202,14 @@ func (a *Authority) init() error {
 	}
 
 	// TODO: decide if this is a good approach for providing the SCEP functionality
+	// It currently mirrors the logic for the x509CAServer
 	if a.scepService == nil {
 		var options casapi.Options
 		if a.config.AuthorityConfig.Options != nil {
 			options = *a.config.AuthorityConfig.Options
 		}
 
-		// Read intermediate and create X509 signer for default CAS.
+		// Read intermediate and create X509 signer and decrypter for default CAS.
 		if options.Is(casapi.SoftCAS) {
 			options.CertificateChain, err = pemutil.ReadCertificateBundle(a.config.IntermediateCert)
 			if err != nil {
@@ -220,12 +222,15 @@ func (a *Authority) init() error {
 			if err != nil {
 				return err
 			}
-			options.Decrypter, err = a.keyManager.CreateDecrypter(&kmsapi.CreateDecrypterRequest{
+
-				DecryptionKey: a.config.IntermediateKey,
+			if km, ok := a.keyManager.(apiv1.Decrypter); ok {
-				Password:      []byte(a.config.Password),
+				options.Decrypter, err = km.CreateDecrypter(&kmsapi.CreateDecrypterRequest{
-			})
+					DecryptionKey: a.config.IntermediateKey,
-			if err != nil {
+					Password:      []byte(a.config.Password),
-				return err
+				})
+				if err != nil {
+					return err
+				}
 			}
 		}
 
@@ -368,11 +373,6 @@ func (a *Authority) init() error {
 	audiences := a.config.getAudiences()
 	a.provisioners = provisioner.NewCollection(audiences)
 	config := provisioner.Config{
-		// 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,
@@ -382,6 +382,14 @@ func (a *Authority) init() error {
 		},
 		GetIdentityFunc: a.getIdentityFunc,
 	}
+
+	// Check if a KMS with decryption capability is required and available
+	if a.requiresDecrypter() {
+		if _, ok := a.keyManager.(apiv1.Decrypter); !ok {
+			return errors.New("keymanager doesn't provide crypto.Decrypter")
+		}
+	}
+
 	// Store all the provisioners
 	for _, p := range a.config.AuthorityConfig.Provisioners {
 		if err := p.Init(config); err != nil {
@@ -434,6 +442,20 @@ func (a *Authority) CloseForReload() {
 	}
 }
 
+// requiresDecrypter iterates over the configured provisioners
+// and determines if the Authority requires a KMS that provides
+// a crypto.Decrypter by implementing the apiv1.Decrypter
+// interface. Currently only the SCEP provider requires this,
+// but others may be added in the future.
+func (a *Authority) requiresDecrypter() bool {
+	for _, p := range a.config.AuthorityConfig.Provisioners {
+		if p.GetType() == provisioner.TypeSCEP {
+			return true
+		}
+	}
+	return false
+}
+
 // GetSCEPService returns the configured SCEP Service
 // TODO: this function is intended to exist temporarily
 // in order to make SCEP work more easily. It can be

authority/config.go

@@ -53,6 +53,7 @@ type Config struct {
 	IntermediateCert string               `json:"crt"`
 	IntermediateKey  string               `json:"key"`
 	Address          string               `json:"address"`
+	InsecureAddress  string               `json:"insecureAddress"`
 	DNSNames         []string             `json:"dnsNames"`
 	KMS              *kms.Options         `json:"kms,omitempty"`
 	SSH              *SSHConfig           `json:"ssh,omitempty"`
@@ -207,6 +208,13 @@ func (c *Config) Validate() error {
 		return errors.Errorf("invalid address %s", c.Address)
 	}
 
+	// Validate insecure address if it is configured
+	if c.InsecureAddress != "" {
+		if _, _, err := net.SplitHostPort(c.InsecureAddress); err != nil {
+			return errors.Errorf("invalid address %s", c.InsecureAddress)
+		}
+	}
+
 	if c.TLS == nil {
 		c.TLS = &DefaultTLSOptions
 	} else {

ca/ca.go

@@ -65,11 +65,12 @@ func WithDatabase(db db.AuthDB) Option {
 // CA is the type used to build the complete certificate authority. It builds
 // the HTTP server, set ups the middlewares and the HTTP handlers.
 type CA struct {
-	auth    *authority.Authority
+	auth        *authority.Authority
-	config  *authority.Config
+	config      *authority.Config
-	srv     *server.Server
+	srv         *server.Server
-	opts    *options
+	insecureSrv *server.Server
-	renewer *TLSRenewer
+	opts        *options
+	renewer     *TLSRenewer
 }
 
 // New creates and initializes the CA with the given configuration and options.
@@ -107,6 +108,9 @@ func (ca *CA) Init(config *authority.Config) (*CA, error) {
 	mux := chi.NewRouter()
 	handler := http.Handler(mux)
 
+	insecureMux := chi.NewRouter()
+	insecureHandler := http.Handler(insecureMux)
+
 	// Add regular CA api endpoints in / and /1.0
 	routerHandler := api.New(auth)
 	routerHandler.Route(mux)
@@ -145,17 +149,6 @@ func (ca *CA) Init(config *authority.Config) (*CA, error) {
 		acmeRouterHandler.Route(r)
 	})
 
-	// TODO: THIS SHOULDN'T HAPPEN (or should become configurable)
-	// Current SCEP client I'm testing with doesn't seem to easily trust untrusted certs.
-	// Idea: provide a second mux/handler that runs without TLS. It probably should only
-	// have routes that are intended to be ran without TLS, like the SCEP ones. Look into
-	// option to not enable it in case no SCEP providers are configured. It might
-	// be nice to still include the SCEP routes in the secure handler too, for
-	// client that do understand HTTPS. The RFC does not seem to explicitly exclude HTTPS
-	// usage, but it mentions some caveats related to managing web PKI certificates as
-	// well as certificates via SCEP.
-	tlsConfig = nil
-
 	scepPrefix := "scep"
 	scepAuthority, err := scep.New(auth, scep.AuthorityOptions{
 		IntermediateCertificatePath: config.IntermediateCert,
@@ -173,6 +166,16 @@ func (ca *CA) Init(config *authority.Config) (*CA, error) {
 		scepRouterHandler.Route(r)
 	})
 
+	// According to the RFC (https://tools.ietf.org/html/rfc8894#section-7.10),
+	// SCEP operations are performed using HTTP, so that's why the API is mounted
+	// to the insecure mux. To my current understanding there's no strong reason
+	// to not use HTTPS also, so that's why I've kept the API endpoints in both
+	// muxes and both HTTP as well as HTTPS can be used to request certificates
+	// using SCEP.
+	insecureMux.Route("/"+scepPrefix, func(r chi.Router) {
+		scepRouterHandler.Route(r)
+	})
+
 	// helpful routine for logging all routes
 	//dumpRoutes(mux)
 
@@ -183,6 +186,7 @@ func (ca *CA) Init(config *authority.Config) (*CA, error) {
 			return nil, err
 		}
 		handler = m.Middleware(handler)
+		insecureHandler = m.Middleware(insecureHandler)
 	}
 
 	// Add logger if configured
@@ -192,16 +196,37 @@ func (ca *CA) Init(config *authority.Config) (*CA, error) {
 			return nil, err
 		}
 		handler = logger.Middleware(handler)
+		insecureHandler = logger.Middleware(insecureHandler)
 	}
 
 	ca.auth = auth
 	ca.srv = server.New(config.Address, handler, tlsConfig)
+
+	// TODO: instead opt for having a single server.Server but two http.Servers
+	// handling the HTTP vs. HTTPS handler?
+	if config.InsecureAddress != "" {
+		ca.insecureSrv = server.New(config.InsecureAddress, insecureHandler, nil)
+	}
+
 	return ca, nil
 }
 
 // Run starts the CA calling to the server ListenAndServe method.
 func (ca *CA) Run() error {
-	return ca.srv.ListenAndServe()
+
+	errors := make(chan error, 1)
+	go func() {
+		if ca.insecureSrv != nil {
+			errors <- ca.insecureSrv.ListenAndServe()
+		}
+	}()
+	go func() {
+		errors <- ca.srv.ListenAndServe()
+	}()
+
+	// wait till error occurs; ensures the servers keep listening
+	err := <-errors
+	return err
 }
 
 // Stop stops the CA calling to the server Shutdown method.
@@ -210,7 +235,17 @@ func (ca *CA) Stop() error {
 	if err := ca.auth.Shutdown(); err != nil {
 		log.Printf("error stopping ca.Authority: %+v\n", err)
 	}
-	return ca.srv.Shutdown()
+	var insecureShutdownErr error
+	if ca.insecureSrv != nil {
+		insecureShutdownErr = ca.insecureSrv.Shutdown()
+	}
+
+	secureErr := ca.srv.Shutdown()
+
+	if insecureShutdownErr != nil {
+		return insecureShutdownErr
+	}
+	return secureErr
 }
 
 // Reload reloads the configuration of the CA and calls to the server Reload
@@ -243,6 +278,13 @@ func (ca *CA) Reload() error {
 		return errors.Wrap(err, "error reloading ca")
 	}
 
+	if ca.insecureSrv != nil {
+		if err = ca.insecureSrv.Reload(newCA.insecureSrv); err != nil {
+			logContinue("Reload failed because insecure server could not be replaced.")
+			return errors.Wrap(err, "error reloading insecure server")
+		}
+	}
+
 	if err = ca.srv.Reload(newCA.srv); err != nil {
 		logContinue("Reload failed because server could not be replaced.")
 		return errors.Wrap(err, "error reloading server")

kms/apiv1/options.go

@@ -13,10 +13,13 @@ type KeyManager interface {
 	GetPublicKey(req *GetPublicKeyRequest) (crypto.PublicKey, error)
 	CreateKey(req *CreateKeyRequest) (*CreateKeyResponse, error)
 	CreateSigner(req *CreateSignerRequest) (crypto.Signer, error)
-	CreateDecrypter(req *CreateDecrypterRequest) (crypto.Decrypter, error) // TODO: split into separate interface?
 	Close() error
 }
 
+type Decrypter interface {
+	CreateDecrypter(req *CreateDecrypterRequest) (crypto.Decrypter, error)
+}
+
 // CertificateManager is the interface implemented by the KMS that can load and
 // store x509.Certificates.
 type CertificateManager interface {

kms/awskms/awskms.go

@@ -3,7 +3,6 @@ package awskms
 import (
 	"context"
 	"crypto"
-	"fmt"
 	"net/url"
 	"strings"
 	"time"
@@ -222,11 +221,6 @@ func (k *KMS) CreateSigner(req *apiv1.CreateSignerRequest) (crypto.Signer, error
 	return NewSigner(k.service, req.SigningKey)
 }
 
-// CreateDecrypter creates a new crypto.decrypter backed by AWS KMS
-func (k *KMS) CreateDecrypter(req *apiv1.CreateDecrypterRequest) (crypto.Decrypter, error) {
-	return nil, fmt.Errorf("not implemented yet")
-}
-
 // Close closes the connection of the KMS client.
 func (k *KMS) Close() error {
 	return nil

kms/cloudkms/cloudkms.go

@@ -3,7 +3,6 @@ package cloudkms
 import (
 	"context"
 	"crypto"
-	"fmt"
 	"log"
 	"strings"
 	"time"
@@ -285,11 +284,6 @@ func (k *CloudKMS) GetPublicKey(req *apiv1.GetPublicKeyRequest) (crypto.PublicKe
 	return pk, nil
 }
 
-// CreateDecrypter creates a new crypto.Decrypter backed by Google Cloud KMS
-func (k *CloudKMS) CreateDecrypter(req *apiv1.CreateDecrypterRequest) (crypto.Decrypter, error) {
-	return nil, fmt.Errorf("not implemented yet")
-}
-
 // getPublicKeyWithRetries retries the request if the error is
 // FailedPrecondition, caused because the key is in the PENDING_GENERATION
 // status.

kms/pkcs11/pkcs11.go

@@ -352,8 +352,3 @@ func findCertificate(ctx P11, rawuri string) (*x509.Certificate, error) {
 	}
 	return cert, nil
 }
-
-// CreateDecrypter creates a new crypto.Decrypter backed by PKCS11
-func (k *PKCS11) CreateDecrypter(req *apiv1.CreateDecrypterRequest) (crypto.Decrypter, error) {
-	return nil, fmt.Errorf("not implemented yet")
-}

kms/sshagentkms/sshagentkms.go

@@ -7,7 +7,6 @@ import (
 	"crypto/ed25519"
 	"crypto/rsa"
 	"crypto/x509"
-	"fmt"
 	"io"
 	"net"
 	"os"
@@ -205,8 +204,3 @@ func (k *SSHAgentKMS) GetPublicKey(req *apiv1.GetPublicKeyRequest) (crypto.Publi
 		return nil, errors.Errorf("unsupported public key type %T", v)
 	}
 }
-
-// CreateDecrypter creates a crypto.Decrypter backed by ssh-agent
-func (k *SSHAgentKMS) CreateDecrypter(req *apiv1.CreateDecrypterRequest) (crypto.Decrypter, error) {
-	return nil, fmt.Errorf("not implemented yet")
-}

kms/yubikey/yubikey.go

@@ -7,7 +7,6 @@ import (
 	"crypto"
 	"crypto/x509"
 	"encoding/hex"
-	"fmt"
 	"net/url"
 	"strings"
 
@@ -190,11 +189,6 @@ func (k *YubiKey) CreateSigner(req *apiv1.CreateSignerRequest) (crypto.Signer, e
 	return signer, nil
 }
 
-// CreateDecrypter creates a new crypto.Decrypter backed by a YubiKey
-func (k *YubiKey) CreateDecrypter(req *apiv1.CreateDecrypterRequest) (crypto.Decrypter, error) {
-	return nil, fmt.Errorf("not implemented yet")
-}
-
 // Close releases the connection to the YubiKey.
 func (k *YubiKey) Close() error {
 	return errors.Wrap(k.yk.Close(), "error closing yubikey")