Merge pull request #8211 from hashicorp/bugfix/auto-encrypt-various

agent/agent.go

@@ -840,7 +840,7 @@ func (a *Agent) setupClientAutoEncrypt(ctx context.Context) (*structs.SignedResp
 	}
 	addrs = append(addrs, retryJoinAddrs(disco, retryJoinSerfVariant, "LAN", a.config.RetryJoinLAN, a.logger)...)
 
-	reply, priv, err := client.RequestAutoEncryptCerts(ctx, addrs, a.config.ServerPort, a.tokens.AgentToken())
+	reply, priv, err := client.RequestAutoEncryptCerts(ctx, addrs, a.config.ServerPort, a.tokens.AgentToken(), a.config.AutoEncryptDNSSAN, a.config.AutoEncryptIPSAN)
 	if err != nil {
 		return nil, err
 	}
@@ -877,7 +877,17 @@ func (a *Agent) setupClientAutoEncryptCache(reply *structs.SignedResponse) (*str
 	}
 
 	// prepolutate leaf cache
-	certRes := cache.FetchResult{Value: &reply.IssuedCert, Index: reply.ConnectCARoots.QueryMeta.Index}
+	certRes := cache.FetchResult{
+		Value: &reply.IssuedCert,
+		Index: reply.ConnectCARoots.QueryMeta.Index,
+	}
+
+	for _, ca := range reply.ConnectCARoots.Roots {
+		if ca.ID == reply.ConnectCARoots.ActiveRootID {
+			certRes.State = cachetype.ConnectCALeafSuccess(ca.SigningKeyID)
+			break
+		}
+	}
 	if err := a.cache.Prepopulate(cachetype.ConnectCALeafName, certRes, a.config.Datacenter, a.tokens.AgentToken(), leafReq.Key()); err != nil {
 		return nil, nil, err
 	}

agent/agent_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"crypto/tls"
+	"crypto/x509"
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
@@ -4628,7 +4629,7 @@ func TestAutoConfig_Integration(t *testing.T) {
 	})
 	require.NoError(t, err)
 
-	client := StartTestAgent(t, TestAgent{Name: "test-client", HCL: TestACLConfigWithParams(nil) + `
+	client := StartTestAgent(t, TestAgent{Name: "test-client", HCL: `
 	   bootstrap = false
 		server = false
 		ca_file = "` + caFile + `"
@@ -4654,3 +4655,78 @@ func TestAutoConfig_Integration(t *testing.T) {
 	// spot check that we now have an ACL token
 	require.NotEmpty(t, client.tokens.AgentToken())
 }
+
+func TestAgent_AutoEncrypt(t *testing.T) {
+	// eventually this test should really live with integration tests
+	// the goal here is to have one test server and another test client
+	// spin up both agents and allow the server to authorize the auto encrypt
+	// request and then see the client get a TLS certificate
+	cfgDir := testutil.TempDir(t, "auto-encrypt")
+
+	// write some test TLS certificates out to the cfg dir
+	cert, key, cacert, err := testTLSCertificates("server.dc1.consul")
+	require.NoError(t, err)
+
+	certFile := filepath.Join(cfgDir, "cert.pem")
+	caFile := filepath.Join(cfgDir, "cacert.pem")
+	keyFile := filepath.Join(cfgDir, "key.pem")
+
+	require.NoError(t, ioutil.WriteFile(certFile, []byte(cert), 0600))
+	require.NoError(t, ioutil.WriteFile(caFile, []byte(cacert), 0600))
+	require.NoError(t, ioutil.WriteFile(keyFile, []byte(key), 0600))
+
+	hclConfig := TestACLConfigWithParams(nil) + `
+		verify_incoming = true
+		verify_outgoing = true
+		verify_server_hostname = true
+		ca_file = "` + caFile + `"
+		cert_file = "` + certFile + `"
+		key_file = "` + keyFile + `"
+		connect { enabled = true }
+		auto_encrypt { allow_tls = true }
+	`
+
+	srv := StartTestAgent(t, TestAgent{Name: "test-server", HCL: hclConfig})
+	defer srv.Shutdown()
+
+	testrpc.WaitForTestAgent(t, srv.RPC, "dc1", testrpc.WithToken(TestDefaultMasterToken))
+
+	client := StartTestAgent(t, TestAgent{Name: "test-client", HCL: TestACLConfigWithParams(nil) + `
+	   bootstrap = false
+		server = false
+		ca_file = "` + caFile + `"
+		verify_outgoing = true
+		verify_server_hostname = true
+		node_name = "test-client"
+		auto_encrypt {
+			tls = true
+		}
+		ports {
+			server = ` + strconv.Itoa(srv.Config.RPCBindAddr.Port) + `
+		}
+		retry_join = ["` + srv.Config.SerfBindAddrLAN.String() + `"]`,
+		UseTLS: true,
+	})
+
+	defer client.Shutdown()
+
+	// when this is successful we managed to get a TLS certificate and are using it for
+	// encrypted RPC connections.
+	testrpc.WaitForTestAgent(t, client.RPC, "dc1", testrpc.WithToken(TestDefaultMasterToken))
+
+	// now we need to validate that our certificate has the correct CN
+	aeCert := client.tlsConfigurator.Cert()
+	require.NotNil(t, aeCert)
+
+	id := connect.SpiffeIDAgent{
+		Host:       connect.TestClusterID + ".consul",
+		Datacenter: "dc1",
+		Agent:      "test-client",
+	}
+	expectedCN := connect.AgentCN("test-client", connect.TestClusterID)
+	x509Cert, err := x509.ParseCertificate(aeCert.Certificate[0])
+	require.NoError(t, err)
+	require.Equal(t, expectedCN, x509Cert.Subject.CommonName)
+	require.Len(t, x509Cert.URIs, 1)
+	require.Equal(t, id.URI(), x509Cert.URIs[0])
+}

agent/cache-types/connect_ca_leaf.go

@@ -121,6 +121,15 @@ type fetchState struct {
 	consecutiveRateLimitErrs int
 }
 
+func ConnectCALeafSuccess(authorityKeyID string) interface{} {
+	return fetchState{
+		authorityKeyID:           authorityKeyID,
+		forceExpireAfter:         time.Time{},
+		consecutiveRateLimitErrs: 0,
+		activeRootRotationStart:  time.Time{},
+	}
+}
+
 // fetchStart is called on each fetch that is about to block and wait for
 // changes to the leaf. It subscribes a chan to receive updates from the shared
 // root watcher and triggers root watcher if it's not already running.
@@ -532,7 +541,7 @@ func (c *ConnectCALeaf) generateNewLeaf(req *ConnectCALeafRequest,
 		}
 		commonName = connect.AgentCN(req.Agent, roots.TrustDomain)
 		dnsNames = append([]string{"localhost"}, req.DNSSAN...)
-		ipAddresses = append([]net.IP{net.ParseIP("127.0.0.1"), net.ParseIP("::")}, req.IPSAN...)
+		ipAddresses = append([]net.IP{net.ParseIP("127.0.0.1"), net.ParseIP("::1")}, req.IPSAN...)
 	} else {
 		return result, errors.New("URI must be either service or agent")
 	}

agent/consul/auto_encrypt.go

@@ -19,23 +19,7 @@ const (
 	retryJitterWindow = 30 * time.Second
 )
 
-func (c *Client) RequestAutoEncryptCerts(ctx context.Context, servers []string, port int, token string) (*structs.SignedResponse, string, error) {
-	errFn := func(err error) (*structs.SignedResponse, string, error) {
-		return nil, "", err
-	}
-
-	// Check if we know about a server already through gossip. Depending on
-	// how the agent joined, there might already be one. Also in case this
-	// gets called because the cert expired.
-	server := c.routers.FindServer()
-	if server != nil {
-		servers = []string{server.Addr.String()}
-	}
-
-	if len(servers) == 0 {
-		return errFn(fmt.Errorf("No servers to request AutoEncrypt.Sign"))
-	}
-
+func (c *Client) autoEncryptCSR(extraDNSSANs []string, extraIPSANs []net.IP) (string, string, error) {
 	// We don't provide the correct host here, because we don't know any
 	// better at this point. Apart from the domain, we would need the
 	// ClusterID, which we don't have. This is why we go with
@@ -49,7 +33,7 @@ func (c *Client) RequestAutoEncryptCerts(ctx context.Context, servers []string,
 
 	conf, err := c.config.CAConfig.GetCommonConfig()
 	if err != nil {
-		return errFn(err)
+		return "", "", err
 	}
 
 	if conf.PrivateKeyType == "" {
@@ -62,18 +46,43 @@ func (c *Client) RequestAutoEncryptCerts(ctx context.Context, servers []string,
 	// Create a new private key
 	pk, pkPEM, err := connect.GeneratePrivateKeyWithConfig(conf.PrivateKeyType, conf.PrivateKeyBits)
 	if err != nil {
-		return errFn(err)
+		return "", "", err
 	}
 
-	dnsNames := []string{"localhost"}
-	ipAddresses := []net.IP{net.ParseIP("127.0.0.1"), net.ParseIP("::")}
+	dnsNames := append([]string{"localhost"}, extraDNSSANs...)
+	ipAddresses := append([]net.IP{net.ParseIP("127.0.0.1"), net.ParseIP("::1")}, extraIPSANs...)
 
 	// Create a CSR.
 	//
 	// The Common Name includes the dummy trust domain for now but Server will
 	// override this when it is signed anyway so it's OK.
 	cn := connect.AgentCN(c.config.NodeName, dummyTrustDomain)
 	csr, err := connect.CreateCSR(id, cn, pk, dnsNames, ipAddresses)
+	if err != nil {
+		return "", "", err
+	}
+
+	return pkPEM, csr, nil
+}
+
+func (c *Client) RequestAutoEncryptCerts(ctx context.Context, servers []string, port int, token string, extraDNSSANs []string, extraIPSANs []net.IP) (*structs.SignedResponse, string, error) {
+	errFn := func(err error) (*structs.SignedResponse, string, error) {
+		return nil, "", err
+	}
+
+	// Check if we know about a server already through gossip. Depending on
+	// how the agent joined, there might already be one. Also in case this
+	// gets called because the cert expired.
+	server := c.routers.FindServer()
+	if server != nil {
+		servers = []string{server.Addr.String()}
+	}
+
+	if len(servers) == 0 {
+		return errFn(fmt.Errorf("No servers to request AutoEncrypt.Sign"))
+	}
+
+	pkPEM, csr, err := c.autoEncryptCSR(extraDNSSANs, extraIPSANs)
 	if err != nil {
 		return errFn(err)
 	}

agent/consul/auto_encrypt_test.go

@@ -2,11 +2,17 @@ package consul
 
 import (
 	"context"
+	"crypto/x509"
+	"crypto/x509/pkix"
+	"encoding/asn1"
 	"net"
+	"net/url"
 	"os"
 	"testing"
 	"time"
 
+	"github.com/hashicorp/consul/agent/connect"
+	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/sdk/testutil"
 	"github.com/hashicorp/go-hclog"
 	"github.com/stretchr/testify/require"
@@ -98,7 +104,7 @@ func TestAutoEncrypt_RequestAutoEncryptCerts(t *testing.T) {
 	doneCh := make(chan struct{})
 	var err error
 	go func() {
-		_, _, err = c1.RequestAutoEncryptCerts(ctx, servers, port, token)
+		_, _, err = c1.RequestAutoEncryptCerts(ctx, servers, port, token, nil, nil)
 		close(doneCh)
 	}()
 	select {
@@ -113,3 +119,87 @@ func TestAutoEncrypt_RequestAutoEncryptCerts(t *testing.T) {
 		// try to request certs.
 	}
 }
+
+func TestAutoEncrypt_autoEncryptCSR(t *testing.T) {
+	type testCase struct {
+		conf         *Config
+		extraDNSSANs []string
+		extraIPSANs  []net.IP
+		err          string
+
+		// to validate the csr
+		expectedSubject  pkix.Name
+		expectedSigAlg   x509.SignatureAlgorithm
+		expectedPubAlg   x509.PublicKeyAlgorithm
+		expectedDNSNames []string
+		expectedIPs      []net.IP
+		expectedURIs     []*url.URL
+	}
+
+	cases := map[string]testCase{
+		"sans": {
+			conf: &Config{
+				Datacenter: "dc1",
+				NodeName:   "test-node",
+				CAConfig:   &structs.CAConfiguration{},
+			},
+			extraDNSSANs: []string{"foo.local", "bar.local"},
+			extraIPSANs:  []net.IP{net.IPv4(198, 18, 0, 1), net.IPv4(198, 18, 0, 2)},
+			expectedSubject: pkix.Name{
+				CommonName: connect.AgentCN("test-node", dummyTrustDomain),
+				Names: []pkix.AttributeTypeAndValue{
+					{
+						// 2,5,4,3 is the CommonName type ASN1 identifier
+						Type:  asn1.ObjectIdentifier{2, 5, 4, 3},
+						Value: "testnode.agnt.dummy.tr.consul",
+					},
+				},
+			},
+			expectedSigAlg: x509.ECDSAWithSHA256,
+			expectedPubAlg: x509.ECDSA,
+			expectedDNSNames: []string{
+				"localhost",
+				"foo.local",
+				"bar.local",
+			},
+			expectedIPs: []net.IP{
+				{127, 0, 0, 1},
+				net.ParseIP("::1"),
+				{198, 18, 0, 1},
+				{198, 18, 0, 2},
+			},
+			expectedURIs: []*url.URL{
+				{
+					Scheme: "spiffe",
+					Host:   dummyTrustDomain,
+					Path:   "/agent/client/dc/dc1/id/test-node",
+				},
+			},
+		},
+	}
+
+	for name, tcase := range cases {
+		t.Run(name, func(t *testing.T) {
+			client := Client{config: tcase.conf}
+
+			_, csr, err := client.autoEncryptCSR(tcase.extraDNSSANs, tcase.extraIPSANs)
+			if tcase.err == "" {
+				require.NoError(t, err)
+
+				request, err := connect.ParseCSR(csr)
+				require.NoError(t, err)
+				require.NotNil(t, request)
+
+				require.Equal(t, tcase.expectedSubject, request.Subject)
+				require.Equal(t, tcase.expectedSigAlg, request.SignatureAlgorithm)
+				require.Equal(t, tcase.expectedPubAlg, request.PublicKeyAlgorithm)
+				require.Equal(t, tcase.expectedDNSNames, request.DNSNames)
+				require.Equal(t, tcase.expectedIPs, request.IPAddresses)
+				require.Equal(t, tcase.expectedURIs, request.URIs)
+			} else {
+				require.Error(t, err)
+				require.Empty(t, csr)
+			}
+		})
+	}
+}