From 1928c07d0c4faaa64a24105557a25edc05e64323 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Mon, 19 Mar 2018 21:00:01 -0700
Subject: [PATCH] agent/consul: key the public key of the CSR, verify in test

---
 agent/connect/testing_ca.go              |  3 ++-
 agent/consul/connect_ca_endpoint.go      | 15 ++++++++++++---
 agent/consul/connect_ca_endpoint_test.go | 16 ++++++++++++++--
 agent/structs/connect_ca.go              | 14 ++++++++++++++
 4 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/agent/connect/testing_ca.go b/agent/connect/testing_ca.go
index b6140bb04..b7f436834 100644
--- a/agent/connect/testing_ca.go
+++ b/agent/connect/testing_ca.go
@@ -190,7 +190,8 @@ func TestLeaf(t testing.T, service string, root *structs.CARoot) string {
 // TestCSR returns a CSR to sign the given service.
 func TestCSR(t testing.T, id SpiffeID) string {
 	template := &x509.CertificateRequest{
-		URIs: []*url.URL{id.URI()},
+		URIs:               []*url.URL{id.URI()},
+		SignatureAlgorithm: x509.ECDSAWithSHA256,
 	}
 
 	// Create the private key we'll use
diff --git a/agent/consul/connect_ca_endpoint.go b/agent/consul/connect_ca_endpoint.go
index 2e26c4e2b..9e6b8a4b1 100644
--- a/agent/consul/connect_ca_endpoint.go
+++ b/agent/consul/connect_ca_endpoint.go
@@ -71,7 +71,7 @@ func (s *ConnectCA) Roots(
 // isn't right, we're not using enough of the CSR fields, etc.
 func (s *ConnectCA) Sign(
 	args *structs.CASignRequest,
-	reply *structs.IndexedCARoots) error {
+	reply *structs.IssuedCert) error {
 	// Parse the CSR
 	csr, err := connect.ParseCSR(args.CSR)
 	if err != nil {
@@ -132,14 +132,17 @@ func (s *ConnectCA) Sign(
 		SerialNumber:          sn,
 		Subject:               pkix.Name{CommonName: serviceId.Service},
 		URIs:                  csr.URIs,
-		SignatureAlgorithm:    x509.ECDSAWithSHA256,
+		Signature:             csr.Signature,
+		SignatureAlgorithm:    csr.SignatureAlgorithm,
+		PublicKeyAlgorithm:    csr.PublicKeyAlgorithm,
+		PublicKey:             csr.PublicKey,
 		BasicConstraintsValid: true,
 		KeyUsage:              x509.KeyUsageDataEncipherment | x509.KeyUsageKeyAgreement,
 		ExtKeyUsage: []x509.ExtKeyUsage{
 			x509.ExtKeyUsageClientAuth,
 			x509.ExtKeyUsageServerAuth,
 		},
-		NotAfter:       time.Now().Add(10 * 365 * 24 * time.Hour),
+		NotAfter:       time.Now().Add(3 * 24 * time.Hour),
 		NotBefore:      time.Now(),
 		AuthorityKeyId: keyId,
 		SubjectKeyId:   keyId,
@@ -157,5 +160,11 @@ func (s *ConnectCA) Sign(
 		return fmt.Errorf("error encoding private key: %s", err)
 	}
 
+	// Set the response
+	*reply = structs.IssuedCert{
+		SerialNumber: template.SerialNumber,
+		Cert:         buf.String(),
+	}
+
 	return nil
 }
diff --git a/agent/consul/connect_ca_endpoint_test.go b/agent/consul/connect_ca_endpoint_test.go
index a08e31e04..d658c7ade 100644
--- a/agent/consul/connect_ca_endpoint_test.go
+++ b/agent/consul/connect_ca_endpoint_test.go
@@ -1,6 +1,7 @@
 package consul
 
 import (
+	"crypto/x509"
 	"os"
 	"testing"
 
@@ -30,13 +31,24 @@ func TestConnectCASign(t *testing.T) {
 
 	// Insert a CA
 	state := s1.fsm.State()
-	assert.Nil(state.CARootSet(1, connect.TestCA(t, nil)))
+	ca := connect.TestCA(t, nil)
+	assert.Nil(state.CARootSet(1, ca))
 
 	// Generate a CSR and request signing
 	args := &structs.CASignRequest{
 		Datacenter: "dc01",
 		CSR:        connect.TestCSR(t, connect.TestSpiffeIDService(t, "web")),
 	}
-	var reply interface{}
+	var reply structs.IssuedCert
 	assert.Nil(msgpackrpc.CallWithCodec(codec, "ConnectCA.Sign", args, &reply))
+
+	// Verify that the cert is signed by the CA
+	roots := x509.NewCertPool()
+	assert.True(roots.AppendCertsFromPEM([]byte(ca.RootCert)))
+	leaf, err := connect.ParseCert(reply.Cert)
+	assert.Nil(err)
+	_, err = leaf.Verify(x509.VerifyOptions{
+		Roots: roots,
+	})
+	assert.Nil(err)
 }
diff --git a/agent/structs/connect_ca.go b/agent/structs/connect_ca.go
index 045ebea90..6723d9b98 100644
--- a/agent/structs/connect_ca.go
+++ b/agent/structs/connect_ca.go
@@ -1,5 +1,9 @@
 package structs
 
+import (
+	"math/big"
+)
+
 // IndexedCARoots is the list of currently trusted CA Roots.
 type IndexedCARoots struct {
 	// ActiveRootID is the ID of a root in Roots that is the active CA root.
@@ -62,3 +66,13 @@ type CASignRequest struct {
 func (q *CASignRequest) RequestDatacenter() string {
 	return q.Datacenter
 }
+
+// IssuedCert is a certificate that has been issued by a Connect CA.
+type IssuedCert struct {
+	// SerialNumber is the unique serial number for this certificate.
+	SerialNumber *big.Int
+
+	// Cert is the PEM-encoded certificate. This should not be stored in the
+	// state store, but is present in the sign API response.
+	Cert string `json:",omitempty"`
+}