Merge pull request #1803 from smallstep/herman/fix-scep-vault-ra

Fix CA startup with Vault RA configuration

Author: hslatman

authority/authority.go

@@ -447,6 +447,7 @@ func (a *Authority) init() error {
 				return err
 			}
 			a.rootX509Certs = append(a.rootX509Certs, resp.RootCertificate)
+			a.intermediateX509Certs = append(a.intermediateX509Certs, resp.IntermediateCertificates...)
 		}
 	}
 
@@ -695,32 +696,42 @@ func (a *Authority) init() error {
 			options := &scep.Options{
 				Roots:         a.rootX509Certs,
 				Intermediates: a.intermediateX509Certs,
-				SignerCert:    a.intermediateX509Certs[0],
 			}
-			if options.Signer, err = a.keyManager.CreateSigner(&kmsapi.CreateSignerRequest{
-				SigningKey: a.config.IntermediateKey,
-				Password:   a.password,
-			}); err != nil {
-				return err
+
+			// intermediate certificates can be empty in RA mode
+			if len(a.intermediateX509Certs) > 0 {
+				options.SignerCert = a.intermediateX509Certs[0]
 			}
-			// TODO(hs): instead of creating the decrypter here, pass the
-			// intermediate key + chain down to the SCEP authority,
-			// and only instantiate it when required there. Is that possible?
-			// Also with entering passwords?
-			// 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.
-			_, isRSA := options.Signer.Public().(*rsa.PublicKey)
-			if km, ok := a.keyManager.(kmsapi.Decrypter); ok && isRSA {
-				if decrypter, err := km.CreateDecrypter(&kmsapi.CreateDecrypterRequest{
-					DecryptionKey: a.config.IntermediateKey,
-					Password:      a.password,
-				}); err == nil {
-					// only pass the decrypter down when it was successfully created,
-					// meaning it's an RSA key, and `CreateDecrypter` did not fail.
-					options.Decrypter = decrypter
-					options.DecrypterCert = options.Intermediates[0]
+
+			// attempt to create the (default) SCEP signer if the intermediate
+			// key is configured.
+			if a.config.IntermediateKey != "" {
+				if options.Signer, err = a.keyManager.CreateSigner(&kmsapi.CreateSignerRequest{
+					SigningKey: a.config.IntermediateKey,
+					Password:   a.password,
+				}); err != nil {
+					return err
+				}
+
+				// TODO(hs): instead of creating the decrypter here, pass the
+				// intermediate key + chain down to the SCEP authority,
+				// and only instantiate it when required there. Is that possible?
+				// Also with entering passwords?
+				// 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.
+				_, isRSAKey := options.Signer.Public().(*rsa.PublicKey)
+				if km, ok := a.keyManager.(kmsapi.Decrypter); ok && isRSAKey {
+					if decrypter, err := km.CreateDecrypter(&kmsapi.CreateDecrypterRequest{
+						DecryptionKey: a.config.IntermediateKey,
+						Password:      a.password,
+					}); err == nil {
+						// only pass the decrypter down when it was successfully created,
+						// meaning it's an RSA key, and `CreateDecrypter` did not fail.
+						options.Decrypter = decrypter
+						options.DecrypterCert = options.Intermediates[0]
+					}
 				}
 			}
 

cas/apiv1/requests.go

@@ -116,7 +116,8 @@ type GetCertificateAuthorityRequest struct {
 // GetCertificateAuthorityResponse is the response that contains
 // the root certificate.
 type GetCertificateAuthorityResponse struct {
-	RootCertificate *x509.Certificate
+	RootCertificate          *x509.Certificate
+	IntermediateCertificates []*x509.Certificate
 }
 
 // CreateKeyRequest is the request used to generate a new key using a KMS.

cas/vaultcas/vaultcas.go

@@ -165,7 +165,8 @@ func (v *VaultCAS) GetCertificateAuthority(*apiv1.GetCertificateAuthorityRequest
 	}
 
 	return &apiv1.GetCertificateAuthorityResponse{
-		RootCertificate: cert.root,
+		RootCertificate:          cert.root,
+		IntermediateCertificates: cert.intermediates,
 	}, nil
 }
 

scep/options.go

@@ -37,19 +37,21 @@ func (o *Options) Validate() error {
 	switch {
 	case len(o.Intermediates) == 0:
 		return errors.New("no intermediate certificate available for SCEP authority")
-	case o.Signer == nil:
-		return errors.New("no signer available for SCEP authority")
 	case o.SignerCert == nil:
 		return errors.New("no signer certificate available for SCEP authority")
 	}
 
-	// check if the signer (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.
-	signerPublicKey := o.Signer.Public().(comparablePublicKey)
-	if !signerPublicKey.Equal(o.SignerCert.PublicKey) {
-		return errors.New("mismatch between signer certificate and public key")
+	// the signer is optional, but if it's set, its public key must match the signer
+	// certificate public key.
+	if o.Signer != nil {
+		// check if the signer (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.
+		signerPublicKey := o.Signer.Public().(comparablePublicKey)
+		if !signerPublicKey.Equal(o.SignerCert.PublicKey) {
+			return errors.New("mismatch between signer certificate and public key")
+		}
 	}
 
 	// decrypter can be nil in case a signing only key is used; validation complete.

test/integration/scep/common_test.go

@@ -0,0 +1,273 @@
+package sceptest
+
+import (
+	"context"
+	"crypto/rand"
+	"crypto/rsa"
+	"crypto/tls"
+	"crypto/x509"
+	"encoding/base64"
+	"errors"
+	"fmt"
+	"io"
+	"math/big"
+	"net"
+	"net/http"
+	"net/url"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/smallstep/pkcs7"
+	"github.com/smallstep/scep"
+	"go.step.sm/crypto/minica"
+	"go.step.sm/crypto/x509util"
+
+	"github.com/smallstep/certificates/ca"
+	"github.com/smallstep/certificates/cas/apiv1"
+)
+
+func newCAClient(t *testing.T, caURL, rootFilepath string) *ca.Client {
+	caClient, err := ca.NewClient(
+		caURL,
+		ca.WithRootFile(rootFilepath),
+	)
+	require.NoError(t, err)
+	return caClient
+}
+
+func requireHealthyCA(t *testing.T, caClient *ca.Client) {
+	ctx := context.Background()
+	healthResponse, err := caClient.HealthWithContext(ctx)
+	require.NoError(t, err)
+	if assert.NotNil(t, healthResponse) {
+		require.Equal(t, "ok", healthResponse.Status)
+	}
+}
+
+// reservePort "reserves" a TCP port by opening a listener on a random
+// port and immediately closing it. The port can then be assumed to be
+// available for running a server on.
+func reservePort(t *testing.T) (host, port string) {
+	t.Helper()
+	l, err := net.Listen("tcp", ":0")
+	require.NoError(t, err)
+
+	address := l.Addr().String()
+	err = l.Close()
+	require.NoError(t, err)
+
+	host, port, err = net.SplitHostPort(address)
+	require.NoError(t, err)
+
+	return
+}
+
+type client struct {
+	caURL      string
+	caCert     *x509.Certificate
+	httpClient *http.Client
+}
+
+func createSCEPClient(t *testing.T, caURL string, root *x509.Certificate) *client {
+	t.Helper()
+	trustedRoots := x509.NewCertPool()
+	trustedRoots.AddCert(root)
+	transport := http.DefaultTransport.(*http.Transport).Clone()
+	transport.TLSClientConfig = &tls.Config{
+		RootCAs: trustedRoots,
+	}
+	httpClient := &http.Client{
+		Transport: transport,
+	}
+	return &client{
+		caURL:      caURL,
+		httpClient: httpClient,
+	}
+}
+
+func (c *client) getCACert(t *testing.T) error {
+	// return early if CA certificate already available
+	if c.caCert != nil {
+		return nil
+	}
+
+	resp, err := c.httpClient.Get(fmt.Sprintf("%s?operation=GetCACert&message=test", c.caURL))
+	if err != nil {
+		return fmt.Errorf("failed get request: %w", err)
+	}
+	defer resp.Body.Close()
+
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return fmt.Errorf("failed reading response body: %w", err)
+	}
+
+	t.Log(string(body))
+
+	// SCEP CA/RA certificate selection. If there's only a single certificate, it will
+	// be used as the CA certificate at all times. If there's multiple, the first certificate
+	// is assumed to be the certificate of the recipient to encrypt messages to.
+	switch ct := resp.Header.Get("Content-Type"); ct {
+	case "application/x-x509-ca-cert":
+		cert, err := x509.ParseCertificate(body)
+		if err != nil {
+			return fmt.Errorf("failed parsing response body: %w", err)
+		}
+		if _, ok := cert.PublicKey.(*rsa.PublicKey); !ok {
+			return fmt.Errorf("certificate has unexpected public key type %T", cert.PublicKey)
+		}
+		c.caCert = cert
+	case "application/x-x509-ca-ra-cert":
+		certs, err := scep.CACerts(body)
+		if err != nil {
+			return fmt.Errorf("failed parsing response body: %w", err)
+		}
+		cert := certs[0]
+		if _, ok := cert.PublicKey.(*rsa.PublicKey); !ok {
+			return fmt.Errorf("certificate has unexpected public key type %T", cert.PublicKey)
+		}
+		c.caCert = cert
+	default:
+		return fmt.Errorf("unexpected content-type value %q", ct)
+	}
+
+	return nil
+}
+
+func (c *client) requestCertificate(t *testing.T, commonName string, sans []string) (*x509.Certificate, error) {
+	if err := c.getCACert(t); err != nil {
+		return nil, fmt.Errorf("failed getting CA certificate: %w", err)
+	}
+
+	signer, err := rsa.GenerateKey(rand.Reader, 2048)
+	if err != nil {
+		return nil, fmt.Errorf("failed creating SCEP private key: %w", err)
+	}
+
+	csr, err := x509util.CreateCertificateRequest(commonName, sans, signer)
+	if err != nil {
+		return nil, fmt.Errorf("failed creating CSR: %w", err)
+	}
+
+	tmpl := &x509.Certificate{
+		Subject:        csr.Subject,
+		PublicKey:      signer.Public(),
+		SerialNumber:   big.NewInt(1),
+		NotBefore:      time.Now().Add(-1 * time.Hour),
+		NotAfter:       time.Now().Add(1 * time.Hour),
+		DNSNames:       csr.DNSNames,
+		IPAddresses:    csr.IPAddresses,
+		EmailAddresses: csr.EmailAddresses,
+		URIs:           csr.URIs,
+	}
+
+	selfSigned, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, signer.Public(), signer)
+	if err != nil {
+		return nil, fmt.Errorf("failed creating self signed certificate: %w", err)
+	}
+	selfSignedCertificate, err := x509.ParseCertificate(selfSigned)
+	if err != nil {
+		return nil, fmt.Errorf("failed parsing self signed certificate: %w", err)
+	}
+
+	msgTmpl := &scep.PKIMessage{
+		TransactionID: "test-1",
+		MessageType:   scep.PKCSReq,
+		SenderNonce:   []byte("test-nonce-1"),
+		Recipients:    []*x509.Certificate{c.caCert},
+		SignerCert:    selfSignedCertificate,
+		SignerKey:     signer,
+	}
+
+	msg, err := scep.NewCSRRequest(csr, msgTmpl)
+	if err != nil {
+		return nil, fmt.Errorf("failed creating SCEP PKCSReq message: %w", err)
+	}
+
+	t.Log(string(msg.Raw))
+
+	u, err := url.Parse(c.caURL)
+	if err != nil {
+		return nil, fmt.Errorf("failed parsing CA URL: %w", err)
+	}
+
+	opURL := u.ResolveReference(&url.URL{RawQuery: fmt.Sprintf("operation=PKIOperation&message=%s", url.QueryEscape(base64.StdEncoding.EncodeToString(msg.Raw)))})
+	resp, err := c.httpClient.Get(opURL.String())
+	if err != nil {
+		return nil, fmt.Errorf("failed get request: %w", err)
+	}
+	defer resp.Body.Close()
+
+	if ct := resp.Header.Get("Content-Type"); ct != "application/x-pki-message" {
+		return nil, fmt.Errorf("received unexpected content type %q", ct)
+	}
+
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return nil, fmt.Errorf("failed reading response body: %w", err)
+	}
+
+	t.Log(string(body))
+
+	signedData, err := pkcs7.Parse(body)
+	if err != nil {
+		return nil, fmt.Errorf("failed parsing response body: %w", err)
+	}
+
+	// TODO: verify the signature?
+
+	p7, err := pkcs7.Parse(signedData.Content)
+	if err != nil {
+		return nil, fmt.Errorf("failed decrypting inner p7: %w", err)
+	}
+
+	content, err := p7.Decrypt(selfSignedCertificate, signer)
+	if err != nil {
+		return nil, fmt.Errorf("failed decrypting response: %w", err)
+	}
+
+	p7, err = pkcs7.Parse(content)
+	if err != nil {
+		return nil, fmt.Errorf("failed parsing p7 content: %w", err)
+	}
+
+	cert := p7.Certificates[0]
+
+	return cert, nil
+}
+
+type testCAS struct {
+	ca *minica.CA
+}
+
+func (c *testCAS) CreateCertificate(req *apiv1.CreateCertificateRequest) (*apiv1.CreateCertificateResponse, error) {
+	cert, err := c.ca.SignCSR(req.CSR)
+	if err != nil {
+		return nil, fmt.Errorf("failed signing CSR: %w", err)
+	}
+
+	return &apiv1.CreateCertificateResponse{
+		Certificate:      cert,
+		CertificateChain: []*x509.Certificate{cert, c.ca.Intermediate},
+	}, nil
+}
+func (c *testCAS) RenewCertificate(req *apiv1.RenewCertificateRequest) (*apiv1.RenewCertificateResponse, error) {
+	return nil, errors.New("not implemented")
+}
+
+func (c *testCAS) RevokeCertificate(req *apiv1.RevokeCertificateRequest) (*apiv1.RevokeCertificateResponse, error) {
+	return nil, errors.New("not implemented")
+}
+
+func (c *testCAS) GetCertificateAuthority(req *apiv1.GetCertificateAuthorityRequest) (*apiv1.GetCertificateAuthorityResponse, error) {
+	return &apiv1.GetCertificateAuthorityResponse{
+		RootCertificate:          c.ca.Root,
+		IntermediateCertificates: []*x509.Certificate{c.ca.Intermediate},
+	}, nil
+}
+
+var _ apiv1.CertificateAuthorityService = (*testCAS)(nil)
+var _ apiv1.CertificateAuthorityGetter = (*testCAS)(nil)