Fix auto_encrypt IP/DNS SANs

The initial auto encrypt CSR wasn’t containing the user supplied IP and DNS SANs. This fixes that. Also We were configuring a default :: IP SAN. This should be ::1 instead and was fixed.

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
 	}

agent/cache-types/connect_ca_leaf.go

@@ -532,7 +532,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,7 +19,53 @@ const (
 	retryJitterWindow = 30 * time.Second
 )
 
-func (c *Client) RequestAutoEncryptCerts(ctx context.Context, servers []string, port int, token string) (*structs.SignedResponse, string, error) {
+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
+	// dummyTrustDomain the first time. Subsequent CSRs will have the
+	// correct TrustDomain.
+	id := &connect.SpiffeIDAgent{
+		Host:       dummyTrustDomain,
+		Datacenter: c.config.Datacenter,
+		Agent:      c.config.NodeName,
+	}
+
+	conf, err := c.config.CAConfig.GetCommonConfig()
+	if err != nil {
+		return "", "", err
+	}
+
+	if conf.PrivateKeyType == "" {
+		conf.PrivateKeyType = connect.DefaultPrivateKeyType
+	}
+	if conf.PrivateKeyBits == 0 {
+		conf.PrivateKeyBits = connect.DefaultPrivateKeyBits
+	}
+
+	// Create a new private key
+	pk, pkPEM, err := connect.GeneratePrivateKeyWithConfig(conf.PrivateKeyType, conf.PrivateKeyBits)
+	if err != nil {
+		return "", "", err
+	}
+
+	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
 	}
@@ -36,44 +82,7 @@ func (c *Client) RequestAutoEncryptCerts(ctx context.Context, servers []string,
 		return errFn(fmt.Errorf("No servers to request AutoEncrypt.Sign"))
 	}
 
-	// We don't provide the correct host here, because we don't know any
+	pkPEM, csr, err := c.autoEncryptCSR(extraDNSSANs, extraIPSANs)
-	// better at this point. Apart from the domain, we would need the
-	// ClusterID, which we don't have. This is why we go with
-	// dummyTrustDomain the first time. Subsequent CSRs will have the
-	// correct TrustDomain.
-	id := &connect.SpiffeIDAgent{
-		Host:       dummyTrustDomain,
-		Datacenter: c.config.Datacenter,
-		Agent:      c.config.NodeName,
-	}
-
-	conf, err := c.config.CAConfig.GetCommonConfig()
-	if err != nil {
-		return errFn(err)
-	}
-
-	if conf.PrivateKeyType == "" {
-		conf.PrivateKeyType = connect.DefaultPrivateKeyType
-	}
-	if conf.PrivateKeyBits == 0 {
-		conf.PrivateKeyBits = connect.DefaultPrivateKeyBits
-	}
-
-	// Create a new private key
-	pk, pkPEM, err := connect.GeneratePrivateKeyWithConfig(conf.PrivateKeyType, conf.PrivateKeyBits)
-	if err != nil {
-		return errFn(err)
-	}
-
-	dnsNames := []string{"localhost"}
-	ipAddresses := []net.IP{net.ParseIP("127.0.0.1"), net.ParseIP("::")}
-
-	// 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 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)
+			}
+		})
+	}
+}