diff --git a/agent/agent_endpoint_test.go b/agent/agent_endpoint_test.go index ac4807edd..6e52157df 100644 --- a/agent/agent_endpoint_test.go +++ b/agent/agent_endpoint_test.go @@ -1443,8 +1443,8 @@ func TestAgent_Self(t *testing.T) { } ports = { grpc = -1 - } - `, + grpc_tls = -1 + }`, expectXDS: false, grpcTLS: false, }, @@ -1453,7 +1453,9 @@ func TestAgent_Self(t *testing.T) { node_meta { somekey = "somevalue" } - `, + ports = { + grpc_tls = -1 + }`, expectXDS: true, grpcTLS: false, }, @@ -1461,8 +1463,7 @@ func TestAgent_Self(t *testing.T) { hcl: ` node_meta { somekey = "somevalue" - } - `, + }`, expectXDS: true, grpcTLS: true, }, diff --git a/agent/connect/testing_ca.go b/agent/connect/testing_ca.go index 343de77bc..06d965ce6 100644 --- a/agent/connect/testing_ca.go +++ b/agent/connect/testing_ca.go @@ -183,8 +183,7 @@ func TestCAWithKeyType(t testing.T, xc *structs.CARoot, keyType string, keyBits return testCA(t, xc, keyType, keyBits, 0) } -func testLeafWithID(t testing.T, spiffeId CertURI, root *structs.CARoot, keyType string, keyBits int, expiration time.Duration) (string, string, error) { - +func testLeafWithID(t testing.T, spiffeId CertURI, dnsSAN string, root *structs.CARoot, keyType string, keyBits int, expiration time.Duration) (string, string, error) { if expiration == 0 { // this is 10 years expiration = 10 * 365 * 24 * time.Hour @@ -238,6 +237,7 @@ func testLeafWithID(t testing.T, spiffeId CertURI, root *structs.CARoot, keyType NotBefore: time.Now(), AuthorityKeyId: testKeyID(t, caSigner.Public()), SubjectKeyId: testKeyID(t, pkSigner.Public()), + DNSNames: []string{dnsSAN}, } // Create the certificate, PEM encode it and return that value. @@ -263,7 +263,7 @@ func TestAgentLeaf(t testing.T, node string, datacenter string, root *structs.CA Agent: node, } - return testLeafWithID(t, spiffeId, root, DefaultPrivateKeyType, DefaultPrivateKeyBits, expiration) + return testLeafWithID(t, spiffeId, "", root, DefaultPrivateKeyType, DefaultPrivateKeyBits, expiration) } func testLeaf(t testing.T, service string, namespace string, root *structs.CARoot, keyType string, keyBits int) (string, string, error) { @@ -275,7 +275,7 @@ func testLeaf(t testing.T, service string, namespace string, root *structs.CARoo Service: service, } - return testLeafWithID(t, spiffeId, root, keyType, keyBits, 0) + return testLeafWithID(t, spiffeId, "", root, keyType, keyBits, 0) } // TestLeaf returns a valid leaf certificate and it's private key for the named @@ -305,7 +305,23 @@ func TestMeshGatewayLeaf(t testing.T, partition string, root *structs.CARoot) (s Datacenter: "dc1", } - certPEM, keyPEM, err := testLeafWithID(t, spiffeId, root, DefaultPrivateKeyType, DefaultPrivateKeyBits, 0) + certPEM, keyPEM, err := testLeafWithID(t, spiffeId, "", root, DefaultPrivateKeyType, DefaultPrivateKeyBits, 0) + if err != nil { + t.Fatalf(err.Error()) + } + return certPEM, keyPEM +} + +func TestServerLeaf(t testing.T, dc string, root *structs.CARoot) (string, string) { + t.Helper() + + spiffeID := &SpiffeIDServer{ + Datacenter: dc, + Host: fmt.Sprintf("%s.consul", TestClusterID), + } + san := PeeringServerSAN(dc, TestTrustDomain) + + certPEM, keyPEM, err := testLeafWithID(t, spiffeID, san, root, DefaultPrivateKeyType, DefaultPrivateKeyBits, 0) if err != nil { t.Fatalf(err.Error()) } diff --git a/agent/consul/leader_peering_test.go b/agent/consul/leader_peering_test.go index 7df1a4b06..df7332300 100644 --- a/agent/consul/leader_peering_test.go +++ b/agent/consul/leader_peering_test.go @@ -22,6 +22,7 @@ import ( "google.golang.org/protobuf/proto" "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/consul/state" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/api" @@ -33,29 +34,53 @@ import ( "github.com/hashicorp/consul/types" ) +type tlsMode byte + +const ( + tlsModeNone tlsMode = iota + tlsModeManual + tlsModeAuto +) + func TestLeader_PeeringSync_Lifecycle_ClientDeletion(t *testing.T) { t.Run("without-tls", func(t *testing.T) { - testLeader_PeeringSync_Lifecycle_ClientDeletion(t, false) + testLeader_PeeringSync_Lifecycle_ClientDeletion(t, tlsModeNone) }) - t.Run("with-tls", func(t *testing.T) { - testLeader_PeeringSync_Lifecycle_ClientDeletion(t, true) + t.Run("manual-tls", func(t *testing.T) { + testLeader_PeeringSync_Lifecycle_ClientDeletion(t, tlsModeManual) + }) + t.Run("auto-tls", func(t *testing.T) { + testLeader_PeeringSync_Lifecycle_ClientDeletion(t, tlsModeAuto) }) } -func testLeader_PeeringSync_Lifecycle_ClientDeletion(t *testing.T, enableTLS bool) { +func testLeader_PeeringSync_Lifecycle_ClientDeletion(t *testing.T, mode tlsMode) { if testing.Short() { t.Skip("too slow for testing.Short") } + ca := connect.TestCA(t, nil) _, acceptor := testServerWithConfig(t, func(c *Config) { c.NodeName = "acceptor" c.Datacenter = "dc1" c.TLSConfig.Domain = "consul" - if enableTLS { + if mode == tlsModeManual { + c.ConnectEnabled = false c.TLSConfig.GRPC.CAFile = "../../test/hostname/CertAuth.crt" c.TLSConfig.GRPC.CertFile = "../../test/hostname/Bob.crt" c.TLSConfig.GRPC.KeyFile = "../../test/hostname/Bob.key" } + if mode == tlsModeAuto { + c.CAConfig = &structs.CAConfiguration{ + ClusterID: connect.TestClusterID, + Provider: structs.ConsulCAProvider, + Config: map[string]interface{}{ + "PrivateKey": ca.SigningKey, + "RootCert": ca.RootCert, + }, + } + + } }) testrpc.WaitForLeader(t, acceptor.RPC, "dc1") @@ -94,11 +119,6 @@ func testLeader_PeeringSync_Lifecycle_ClientDeletion(t *testing.T, enableTLS boo c.NodeName = "dialer" c.Datacenter = "dc2" c.PrimaryDatacenter = "dc2" - if enableTLS { - c.TLSConfig.GRPC.CAFile = "../../test/hostname/CertAuth.crt" - c.TLSConfig.GRPC.CertFile = "../../test/hostname/Betty.crt" - c.TLSConfig.GRPC.KeyFile = "../../test/hostname/Betty.key" - } }) testrpc.WaitForLeader(t, dialer.RPC, "dc2") @@ -345,27 +365,43 @@ func TestLeader_PeeringSync_Lifecycle_UnexportWhileDown(t *testing.T) { func TestLeader_PeeringSync_Lifecycle_ServerDeletion(t *testing.T) { t.Run("without-tls", func(t *testing.T) { - testLeader_PeeringSync_Lifecycle_AcceptorDeletion(t, false) + testLeader_PeeringSync_Lifecycle_AcceptorDeletion(t, tlsModeNone) }) - t.Run("with-tls", func(t *testing.T) { - testLeader_PeeringSync_Lifecycle_AcceptorDeletion(t, true) + t.Run("manual-tls", func(t *testing.T) { + testLeader_PeeringSync_Lifecycle_AcceptorDeletion(t, tlsModeManual) + }) + t.Run("auto-tls", func(t *testing.T) { + testLeader_PeeringSync_Lifecycle_AcceptorDeletion(t, tlsModeAuto) }) } -func testLeader_PeeringSync_Lifecycle_AcceptorDeletion(t *testing.T, enableTLS bool) { +func testLeader_PeeringSync_Lifecycle_AcceptorDeletion(t *testing.T, mode tlsMode) { if testing.Short() { t.Skip("too slow for testing.Short") } + ca := connect.TestCA(t, nil) _, acceptor := testServerWithConfig(t, func(c *Config) { c.NodeName = "acceptor" c.Datacenter = "dc1" c.TLSConfig.Domain = "consul" - if enableTLS { + if mode == tlsModeManual { + c.ConnectEnabled = false c.TLSConfig.GRPC.CAFile = "../../test/hostname/CertAuth.crt" c.TLSConfig.GRPC.CertFile = "../../test/hostname/Bob.crt" c.TLSConfig.GRPC.KeyFile = "../../test/hostname/Bob.key" } + if mode == tlsModeAuto { + c.CAConfig = &structs.CAConfiguration{ + ClusterID: connect.TestClusterID, + Provider: structs.ConsulCAProvider, + Config: map[string]interface{}{ + "PrivateKey": ca.SigningKey, + "RootCert": ca.RootCert, + }, + } + + } }) testrpc.WaitForLeader(t, acceptor.RPC, "dc1") @@ -399,11 +435,6 @@ func testLeader_PeeringSync_Lifecycle_AcceptorDeletion(t *testing.T, enableTLS b c.NodeName = "dialer" c.Datacenter = "dc2" c.PrimaryDatacenter = "dc2" - if enableTLS { - c.TLSConfig.GRPC.CAFile = "../../test/hostname/CertAuth.crt" - c.TLSConfig.GRPC.CertFile = "../../test/hostname/Betty.crt" - c.TLSConfig.GRPC.KeyFile = "../../test/hostname/Betty.key" - } }) testrpc.WaitForLeader(t, dialer.RPC, "dc2") @@ -496,6 +527,7 @@ func testLeader_PeeringSync_failsForTLSError(t *testing.T, tokenMutateFn func(to c.Datacenter = "dc1" c.TLSConfig.Domain = "consul" + c.ConnectEnabled = false c.TLSConfig.GRPC.CAFile = "../../test/hostname/CertAuth.crt" c.TLSConfig.GRPC.CertFile = "../../test/hostname/Bob.crt" c.TLSConfig.GRPC.KeyFile = "../../test/hostname/Bob.key" diff --git a/agent/consul/peering_backend.go b/agent/consul/peering_backend.go index 8ed8a6079..d0f7e3165 100644 --- a/agent/consul/peering_backend.go +++ b/agent/consul/peering_backend.go @@ -9,12 +9,14 @@ import ( "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/acl/resolver" + "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/consul/state" "github.com/hashicorp/consul/agent/consul/stream" "github.com/hashicorp/consul/agent/grpc-external/services/peerstream" "github.com/hashicorp/consul/agent/rpc/peering" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/ipaddr" + "github.com/hashicorp/consul/lib" "github.com/hashicorp/consul/proto/pbpeering" ) @@ -53,10 +55,39 @@ func (b *PeeringBackend) GetLeaderAddress() string { return b.leaderAddr } -// GetAgentCACertificates gets the server's raw CA data from its TLS Configurator. -func (b *PeeringBackend) GetAgentCACertificates() ([]string, error) { - // TODO(peering): handle empty CA pems - return b.srv.tlsConfigurator.GRPCManualCAPems(), nil +// GetTLSMaterials returns the TLS materials for the dialer to dial the acceptor using TLS. +// It returns the server name to validate, and the CA certificate to validate with. +func (b *PeeringBackend) GetTLSMaterials() (string, []string, error) { + // Do not send TLS materials to the dialer if we to not have TLS configured for gRPC. + if b.srv.config.GRPCTLSPort <= 0 && !b.srv.tlsConfigurator.GRPCServerUseTLS() { + return "", nil, nil + } + + // If the Connect CA is not in use we rely on the manually configured certs. + // Otherwise we rely on the internally managed server certificate. + if !b.srv.config.ConnectEnabled { + serverName := b.srv.tlsConfigurator.ServerSNI(b.srv.config.Datacenter, "") + caPems := b.srv.tlsConfigurator.GRPCManualCAPems() + + return serverName, caPems, nil + } + + roots, err := b.srv.getCARoots(nil, b.srv.fsm.State()) + if err != nil { + return "", nil, fmt.Errorf("failed to fetch roots: %w", err) + } + if len(roots.Roots) == 0 { + return "", nil, fmt.Errorf("CA has not finished initializing") + } + + serverName := connect.PeeringServerSAN(b.srv.config.Datacenter, roots.TrustDomain) + + var caPems []string + for _, r := range roots.Roots { + caPems = append(caPems, lib.EnsureTrailingNewline(r.RootCert)) + } + + return serverName, caPems, nil } // GetServerAddresses looks up server or mesh gateway addresses from the state store. @@ -117,12 +148,6 @@ func serverAddresses(state *state.Store) ([]string, error) { return addrs, nil } -// GetServerName returns the SNI to be returned in the peering token data which -// will be used by peers when establishing peering connections over TLS. -func (b *PeeringBackend) GetServerName() string { - return b.srv.tlsConfigurator.ServerSNI(b.srv.config.Datacenter, "") -} - // EncodeToken encodes a peering token as a bas64-encoded representation of JSON (for now). func (b *PeeringBackend) EncodeToken(tok *structs.PeeringToken) ([]byte, error) { jsonToken, err := json.Marshal(tok) diff --git a/agent/consul/server_test.go b/agent/consul/server_test.go index 287dca871..b9f8e7db9 100644 --- a/agent/consul/server_test.go +++ b/agent/consul/server_test.go @@ -232,7 +232,7 @@ func testServerWithConfig(t *testing.T, configOpts ...func(*Config)) (string, *S } // Apply config to copied fields because many tests only set the old - //values. + // values. config.ACLResolverSettings.ACLsEnabled = config.ACLsEnabled config.ACLResolverSettings.NodeName = config.NodeName config.ACLResolverSettings.Datacenter = config.Datacenter @@ -247,15 +247,32 @@ func testServerWithConfig(t *testing.T, configOpts ...func(*Config)) (string, *S }) t.Cleanup(func() { srv.Shutdown() }) - if srv.config.GRPCPort > 0 { + for _, grpcPort := range []int{srv.config.GRPCPort, srv.config.GRPCTLSPort} { + if grpcPort == 0 { + continue + } + // Normally the gRPC server listener is created at the agent level and // passed down into the Server creation. - externalGRPCAddr := fmt.Sprintf("127.0.0.1:%d", srv.config.GRPCPort) - ln, err := net.Listen("tcp", externalGRPCAddr) + ln, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", grpcPort)) require.NoError(t, err) - // Wrap the listener with TLS - if deps.TLSConfigurator.GRPCServerUseTLS() { + if grpcPort == srv.config.GRPCTLSPort || deps.TLSConfigurator.GRPCServerUseTLS() { + // Set the internally managed server certificate. The cert manager is hooked to the Agent, so we need to bypass that here. + if srv.config.PeeringEnabled && srv.config.ConnectEnabled { + key, _ := srv.config.CAConfig.Config["PrivateKey"].(string) + cert, _ := srv.config.CAConfig.Config["RootCert"].(string) + if key != "" && cert != "" { + ca := &structs.CARoot{ + SigningKey: key, + RootCert: cert, + } + require.NoError(t, deps.TLSConfigurator.UpdateAutoTLSCert(connect.TestServerLeaf(t, srv.config.Datacenter, ca))) + deps.TLSConfigurator.UpdateAutoTLSPeeringServerName(connect.PeeringServerSAN("dc1", connect.TestTrustDomain)) + } + } + + // Wrap the listener with TLS. ln = tls.NewListener(ln, deps.TLSConfigurator.IncomingGRPCConfig()) } diff --git a/agent/peering_endpoint_test.go b/agent/peering_endpoint_test.go index 0f5692bad..c7006a925 100644 --- a/agent/peering_endpoint_test.go +++ b/agent/peering_endpoint_test.go @@ -22,25 +22,6 @@ import ( "github.com/hashicorp/consul/testrpc" ) -var validCA = ` ------BEGIN CERTIFICATE----- -MIICmDCCAj6gAwIBAgIBBzAKBggqhkjOPQQDAjAWMRQwEgYDVQQDEwtDb25zdWwg -Q0EgNzAeFw0xODA1MjExNjMzMjhaFw0yODA1MTgxNjMzMjhaMBYxFDASBgNVBAMT -C0NvbnN1bCBDQSA3MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAER0qlxjnRcMEr -iSGlH7G7dYU7lzBEmLUSMZkyBbClmyV8+e8WANemjn+PLnCr40If9cmpr7RnC9Qk -GTaLnLiF16OCAXswggF3MA4GA1UdDwEB/wQEAwIBhjAPBgNVHRMBAf8EBTADAQH/ -MGgGA1UdDgRhBF8xZjo5MTpjYTo0MTo4ZjphYzo2NzpiZjo1OTpjMjpmYTo0ZTo3 -NTo1YzpkODpmMDo1NTpkZTpiZTo3NTpiODozMzozMTpkNToyNDpiMDowNDpiMzpl -ODo5Nzo1Yjo3ZTBqBgNVHSMEYzBhgF8xZjo5MTpjYTo0MTo4ZjphYzo2NzpiZjo1 -OTpjMjpmYTo0ZTo3NTo1YzpkODpmMDo1NTpkZTpiZTo3NTpiODozMzozMTpkNToy -NDpiMDowNDpiMzplODo5Nzo1Yjo3ZTA/BgNVHREEODA2hjRzcGlmZmU6Ly8xMjRk -ZjVhMC05ODIwLTc2YzMtOWFhOS02ZjYyMTY0YmExYzIuY29uc3VsMD0GA1UdHgEB -/wQzMDGgLzAtgisxMjRkZjVhMC05ODIwLTc2YzMtOWFhOS02ZjYyMTY0YmExYzIu -Y29uc3VsMAoGCCqGSM49BAMCA0gAMEUCIQDzkkI7R+0U12a+zq2EQhP/n2mHmta+ -fs2hBxWIELGwTAIgLdO7RRw+z9nnxCIA6kNl//mIQb+PGItespiHZKAz74Q= ------END CERTIFICATE----- -` - func TestHTTP_Peering_GenerateToken(t *testing.T) { if testing.Short() { t.Skip("too slow for testing.Short") @@ -50,6 +31,7 @@ func TestHTTP_Peering_GenerateToken(t *testing.T) { a := NewTestAgent(t, "") testrpc.WaitForTestAgent(t, a.RPC, "dc1") + testrpc.WaitForActiveCARoot(t, a.RPC, "dc1", nil) t.Run("No Body", func(t *testing.T) { req, err := http.NewRequest("POST", "/v1/peering/token", nil) @@ -107,9 +89,9 @@ func TestHTTP_Peering_GenerateToken(t *testing.T) { var token structs.PeeringToken require.NoError(t, json.Unmarshal(tokenJSON, &token)) - require.Nil(t, token.CA) - require.Equal(t, []string{fmt.Sprintf("127.0.0.1:%d", a.config.GRPCPort)}, token.ServerAddresses) - require.Equal(t, "server.dc1.consul", token.ServerName) + require.NotNil(t, token.CA) + require.Equal(t, []string{fmt.Sprintf("127.0.0.1:%d", a.config.GRPCTLSPort)}, token.ServerAddresses) + require.Equal(t, "server.dc1.peering.11111111-2222-3333-4444-555555555555.consul", token.ServerName) // The PeerID in the token is randomly generated so we don't assert on its value. require.NotEmpty(t, token.PeerID) @@ -140,9 +122,9 @@ func TestHTTP_Peering_GenerateToken(t *testing.T) { var token structs.PeeringToken require.NoError(t, json.Unmarshal(tokenJSON, &token)) - require.Nil(t, token.CA) + require.NotNil(t, token.CA) require.Equal(t, []string{externalAddress}, token.ServerAddresses) - require.Equal(t, "server.dc1.consul", token.ServerName) + require.Equal(t, "server.dc1.peering.11111111-2222-3333-4444-555555555555.consul", token.ServerName) // The PeerID in the token is randomly generated so we don't assert on its value. require.NotEmpty(t, token.PeerID) @@ -159,6 +141,7 @@ func TestHTTP_Peering_GenerateToken_EdgeCases(t *testing.T) { a := NewTestAgent(t, "") testrpc.WaitForTestAgent(t, a.RPC, "dc1") + testrpc.WaitForActiveCARoot(t, a.RPC, "dc1", nil) body := &pbpeering.GenerateTokenRequest{ PeerName: "peering-a", @@ -219,10 +202,9 @@ func TestHTTP_Peering_Establish(t *testing.T) { t.Skip("too slow for testing.Short") } - t.Parallel() a := NewTestAgent(t, "") - testrpc.WaitForTestAgent(t, a.RPC, "dc1") + testrpc.WaitForActiveCARoot(t, a.RPC, "dc1", nil) t.Run("No Body", func(t *testing.T) { req, err := http.NewRequest("POST", "/v1/peering/establish", nil) @@ -291,14 +273,17 @@ func TestHTTP_Peering_Establish(t *testing.T) { }) require.NoError(t, err) - req, err = http.NewRequest("POST", "/v1/peering/establish", bytes.NewReader(b)) - require.NoError(t, err) - resp = httptest.NewRecorder() - a2.srv.h.ServeHTTP(resp, req) - require.Equal(t, http.StatusOK, resp.Code, "expected 200, got %d: %v", resp.Code, resp.Body.String()) + retry.Run(t, func(r *retry.R) { + req, err = http.NewRequest("POST", "/v1/peering/establish", bytes.NewReader(b)) + require.NoError(r, err) - // success response does not currently return a value so {} is correct - require.Equal(t, "{}", resp.Body.String()) + resp = httptest.NewRecorder() + a2.srv.h.ServeHTTP(resp, req) + require.Equal(r, http.StatusOK, resp.Code, "expected 200, got %d: %v", resp.Code, resp.Body.String()) + + // success response does not currently return a value so {} is correct + require.Equal(r, "{}", resp.Body.String()) + }) }) } @@ -416,6 +401,7 @@ func TestHTTP_Peering_Delete(t *testing.T) { a := NewTestAgent(t, "") testrpc.WaitForTestAgent(t, a.RPC, "dc1") + testrpc.WaitForActiveCARoot(t, a.RPC, "dc1", nil) bodyBytes, err := json.Marshal(&pbpeering.GenerateTokenRequest{ PeerName: "foo", diff --git a/agent/rpc/peering/service.go b/agent/rpc/peering/service.go index d9ddbdc6c..cb72fe84c 100644 --- a/agent/rpc/peering/service.go +++ b/agent/rpc/peering/service.go @@ -114,17 +114,14 @@ type Backend interface { // partition and namespace from the token. ResolveTokenAndDefaultMeta(token string, entMeta *acl.EnterpriseMeta, authzCtx *acl.AuthorizerContext) (resolver.Result, error) - // GetAgentCACertificates returns the CA certificate to be returned in the peering token data - GetAgentCACertificates() ([]string, error) + // GetTLSMaterials returns the TLS materials for the dialer to dial the acceptor using TLS. + // It returns the server name to validate, and the CA certificate to validate with. + GetTLSMaterials() (string, []string, error) // GetServerAddresses returns the addresses used for establishing a peering connection. // These may be server addresses or mesh gateway addresses if peering through mesh gateways. GetServerAddresses() ([]string, error) - // GetServerName returns the SNI to be returned in the peering token data which - // will be used by peers when establishing peering connections over TLS. - GetServerName() string - // EncodeToken packages a peering token into a slice of bytes. EncodeToken(tok *structs.PeeringToken) ([]byte, error) @@ -291,7 +288,7 @@ func (s *Server) GenerateToken( break } - ca, err := s.Backend.GetAgentCACertificates() + serverName, caPEMs, err := s.Backend.GetTLSMaterials() if err != nil { return nil, err } @@ -310,9 +307,9 @@ func (s *Server) GenerateToken( tok := structs.PeeringToken{ // Store the UUID so that we can do a global search when handling inbound streams. PeerID: peering.ID, - CA: ca, + CA: caPEMs, ServerAddresses: serverAddrs, - ServerName: s.Backend.GetServerName(), + ServerName: serverName, EstablishmentSecret: secretID, } @@ -487,8 +484,13 @@ func (s *Server) validatePeeringLocality(token *structs.PeeringToken, partition // If the token has the same server name as this cluster, but we can't find the peering // in our store, it indicates a naming conflict. - if s.Backend.GetServerName() == token.ServerName && peering == nil { - return fmt.Errorf("conflict - peering token's server name matches the current cluster's server name, %q, but there is no record in the database", s.Backend.GetServerName()) + serverName, _, err := s.Backend.GetTLSMaterials() + if err != nil { + return fmt.Errorf("failed to fetch TLS materials: %w", err) + } + + if serverName != "" && token.ServerName != "" && serverName == token.ServerName && peering == nil { + return fmt.Errorf("conflict - peering token's server name matches the current cluster's server name, %q, but there is no record in the database", serverName) } if peering != nil && acl.EqualPartitions(peering.GetPartition(), partition) { diff --git a/agent/rpc/peering/service_test.go b/agent/rpc/peering/service_test.go index 3d04981f4..e20472a35 100644 --- a/agent/rpc/peering/service_test.go +++ b/agent/rpc/peering/service_test.go @@ -2,6 +2,7 @@ package peering_test import ( "context" + "crypto/tls" "encoding/base64" "encoding/json" "fmt" @@ -19,6 +20,7 @@ import ( grpcstatus "google.golang.org/grpc/status" "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/consul" "github.com/hashicorp/consul/agent/consul/state" "github.com/hashicorp/consul/agent/consul/stream" @@ -62,6 +64,7 @@ func generateTooManyMetaKeys() map[string]string { func TestPeeringService_GenerateToken(t *testing.T) { dir := testutil.TempDir(t, "consul") + signer, _, _ := tlsutil.GeneratePrivateKey() ca, _, _ := tlsutil.GenerateCA(tlsutil.CAOpts{Signer: signer}) cafile := path.Join(dir, "cacert.pem") @@ -97,10 +100,14 @@ func TestPeeringService_GenerateToken(t *testing.T) { token := &structs.PeeringToken{} require.NoError(t, json.Unmarshal(tokenJSON, token)) - require.Equal(t, "server.dc1.consul", token.ServerName) + require.Equal(t, "server.dc1.peering.11111111-2222-3333-4444-555555555555.consul", token.ServerName) require.Len(t, token.ServerAddresses, 1) require.Equal(t, s.PublicGRPCAddr, token.ServerAddresses[0]) - require.Equal(t, []string{ca}, token.CA) + + // The roots utilized should be the ConnectCA roots and not the ones manually configured. + _, roots, err := s.Server.FSM().State().CARoots(nil) + require.NoError(t, err) + require.Equal(t, []string{roots.Active().RootCert}, token.CA) require.NotEmpty(t, token.EstablishmentSecret) secret = token.EstablishmentSecret @@ -165,6 +172,7 @@ func TestPeeringService_GenerateToken(t *testing.T) { func TestPeeringService_GenerateTokenExternalAddress(t *testing.T) { dir := testutil.TempDir(t, "consul") + signer, _, _ := tlsutil.GeneratePrivateKey() ca, _, _ := tlsutil.GenerateCA(tlsutil.CAOpts{Signer: signer}) cafile := path.Join(dir, "cacert.pem") @@ -191,10 +199,14 @@ func TestPeeringService_GenerateTokenExternalAddress(t *testing.T) { token := &structs.PeeringToken{} require.NoError(t, json.Unmarshal(tokenJSON, token)) - require.Equal(t, "server.dc1.consul", token.ServerName) + require.Equal(t, "server.dc1.peering.11111111-2222-3333-4444-555555555555.consul", token.ServerName) require.Len(t, token.ServerAddresses, 1) require.Equal(t, externalAddress, token.ServerAddresses[0]) - require.Equal(t, []string{ca}, token.CA) + + // The roots utilized should be the ConnectCA roots and not the ones manually configured. + _, roots, err := s.Server.FSM().State().CARoots(nil) + require.NoError(t, err) + require.Equal(t, []string{roots.Active().RootCert}, token.CA) } func TestPeeringService_GenerateToken_ACLEnforcement(t *testing.T) { @@ -385,9 +397,13 @@ func TestPeeringService_Establish_serverNameConflict(t *testing.T) { // Manufacture token to have the same server name but a PeerID not in the store. id, err := uuid.GenerateUUID() require.NoError(t, err, "could not generate uuid") + + serverName, _, err := s.Server.GetPeeringBackend().GetTLSMaterials() + require.NoError(t, err) + peeringToken := structs.PeeringToken{ ServerAddresses: []string{"1.2.3.4:8502"}, - ServerName: s.Server.GetPeeringBackend().GetServerName(), + ServerName: serverName, EstablishmentSecret: "foo", PeerID: id, } @@ -409,12 +425,15 @@ func TestPeeringService_Establish_serverNameConflict(t *testing.T) { func TestPeeringService_Establish(t *testing.T) { // TODO(peering): see note on newTestServer, refactor to not use this - s1 := newTestServer(t, nil) + s1 := newTestServer(t, func(conf *consul.Config) { + conf.NodeName = "s1" + }) client1 := pbpeering.NewPeeringServiceClient(s1.ClientConn(t)) s2 := newTestServer(t, func(conf *consul.Config) { + conf.NodeName = "s2" conf.Datacenter = "dc2" - conf.GRPCPort = 5301 + conf.PrimaryDatacenter = "dc2" }) client2 := pbpeering.NewPeeringServiceClient(s2.ClientConn(t)) @@ -430,8 +449,10 @@ func TestPeeringService_Establish(t *testing.T) { var peerID string testutil.RunStep(t, "peering can be established from token", func(t *testing.T) { - _, err = client2.Establish(ctx, &pbpeering.EstablishRequest{PeerName: "my-peer-s1", PeeringToken: tokenResp.PeeringToken}) - require.NoError(t, err) + retry.Run(t, func(r *retry.R) { + _, err = client2.Establish(ctx, &pbpeering.EstablishRequest{PeerName: "my-peer-s1", PeeringToken: tokenResp.PeeringToken}) + require.NoError(r, err) + }) ctx, cancel = context.WithTimeout(context.Background(), 10*time.Second) t.Cleanup(cancel) @@ -1095,9 +1116,7 @@ func TestPeeringService_TrustBundleListByService(t *testing.T) { } func TestPeeringService_validatePeer(t *testing.T) { - s1 := newTestServer(t, func(c *consul.Config) { - c.SerfLANConfig.MemberlistConfig.AdvertiseAddr = "127.0.0.1" - }) + s1 := newTestServer(t, nil) client1 := pbpeering.NewPeeringServiceClient(s1.ClientConn(t)) ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) @@ -1111,8 +1130,8 @@ func TestPeeringService_validatePeer(t *testing.T) { }) s2 := newTestServer(t, func(conf *consul.Config) { - conf.GRPCPort = 5301 conf.Datacenter = "dc2" + conf.PrimaryDatacenter = "dc2" }) client2 := pbpeering.NewPeeringServiceClient(s2.ClientConn(t)) @@ -1362,7 +1381,18 @@ func newTestServer(t *testing.T, cb func(conf *consul.Config)) testingServer { conf.PrimaryDatacenter = "dc1" conf.ConnectEnabled = true - conf.GRPCPort = ports[3] + ca := connect.TestCA(t, nil) + conf.CAConfig = &structs.CAConfiguration{ + ClusterID: connect.TestClusterID, + Provider: structs.ConsulCAProvider, + Config: map[string]interface{}{ + "PrivateKey": ca.SigningKey, + "RootCert": ca.RootCert, + "LeafCertTTL": "72h", + "IntermediateCertTTL": "288h", + }, + } + conf.GRPCTLSPort = ports[3] nodeID, err := uuid.GenerateUUID() if err != nil { @@ -1381,27 +1411,34 @@ func newTestServer(t *testing.T, cb func(conf *consul.Config)) testingServer { conf.ACLResolverSettings.Datacenter = conf.Datacenter conf.ACLResolverSettings.EnterpriseMeta = *conf.AgentEnterpriseMeta() - externalGRPCServer := gogrpc.NewServer() - deps := newDefaultDeps(t, conf) + externalGRPCServer := external.NewServer(deps.Logger) + server, err := consul.NewServer(conf, deps, externalGRPCServer) require.NoError(t, err) t.Cleanup(func() { require.NoError(t, server.Shutdown()) }) + require.NoError(t, deps.TLSConfigurator.UpdateAutoTLSCert(connect.TestServerLeaf(t, conf.Datacenter, ca))) + deps.TLSConfigurator.UpdateAutoTLSPeeringServerName(connect.PeeringServerSAN(conf.Datacenter, connect.TestTrustDomain)) + // Normally the gRPC server listener is created at the agent level and // passed down into the Server creation. - grpcAddr := fmt.Sprintf("127.0.0.1:%d", conf.GRPCPort) + grpcAddr := fmt.Sprintf("127.0.0.1:%d", conf.GRPCTLSPort) ln, err := net.Listen("tcp", grpcAddr) require.NoError(t, err) + + ln = tls.NewListener(ln, deps.TLSConfigurator.IncomingGRPCConfig()) + go func() { _ = externalGRPCServer.Serve(ln) }() t.Cleanup(externalGRPCServer.Stop) testrpc.WaitForLeader(t, server.RPC, conf.Datacenter) + testrpc.WaitForActiveCARoot(t, server.RPC, conf.Datacenter, nil) return testingServer{ Server: server, diff --git a/agent/rpc/peering/validate.go b/agent/rpc/peering/validate.go index 340e4c5ad..af415102b 100644 --- a/agent/rpc/peering/validate.go +++ b/agent/rpc/peering/validate.go @@ -38,9 +38,7 @@ func validatePeeringToken(tok *structs.PeeringToken) error { } } - // TODO(peering): validate name matches SNI? - // TODO(peering): validate name well formed? - if tok.ServerName == "" { + if len(tok.CA) > 0 && tok.ServerName == "" { return errPeeringTokenEmptyServerName } diff --git a/agent/testagent.go b/agent/testagent.go index 2fb4438dd..2d34ba198 100644 --- a/agent/testagent.go +++ b/agent/testagent.go @@ -190,7 +190,7 @@ func (a *TestAgent) Start(t *testing.T) error { Name: name, }) - portsConfig := randomPortsSource(t, a.UseHTTPS, a.UseGRPCTLS) + portsConfig := randomPortsSource(t, a.UseHTTPS) // Create NodeID outside the closure, so that it does not change testHCLConfig := TestConfigHCL(NodeID()) @@ -412,7 +412,7 @@ func (a *TestAgent) consulConfig() *consul.Config { // chance of port conflicts for concurrently executed test binaries. // Instead of relying on one set of ports to be sufficient we retry // starting the agent with different ports on port conflict. -func randomPortsSource(t *testing.T, useHTTPS bool, useGRPCTLS bool) string { +func randomPortsSource(t *testing.T, useHTTPS bool) string { ports := freeport.GetN(t, 8) var http, https int @@ -424,15 +424,6 @@ func randomPortsSource(t *testing.T, useHTTPS bool, useGRPCTLS bool) string { https = -1 } - var grpc, grpcTLS int - if useGRPCTLS { - grpc = -1 - grpcTLS = ports[7] - } else { - grpc = ports[6] - grpcTLS = -1 - } - return ` ports = { dns = ` + strconv.Itoa(ports[0]) + ` @@ -441,8 +432,8 @@ func randomPortsSource(t *testing.T, useHTTPS bool, useGRPCTLS bool) string { serf_lan = ` + strconv.Itoa(ports[3]) + ` serf_wan = ` + strconv.Itoa(ports[4]) + ` server = ` + strconv.Itoa(ports[5]) + ` - grpc = ` + strconv.Itoa(grpc) + ` - grpc_tls = ` + strconv.Itoa(grpcTLS) + ` + grpc = ` + strconv.Itoa(ports[6]) + ` + grpc_tls = ` + strconv.Itoa(ports[7]) + ` } ` } diff --git a/api/api_test.go b/api/api_test.go index 3574240d1..e1fabbb13 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -96,6 +96,10 @@ func makeClientWithConfig( if server.Config.Bootstrap { server.WaitForLeader(t) } + connectEnabled := server.Config.Connect["enabled"] + if enabled, ok := connectEnabled.(bool); ok && server.Config.Server && enabled { + server.WaitForActiveCARoot(t) + } conf.Address = server.HTTPAddr diff --git a/api/peering_test.go b/api/peering_test.go index 9b7974ea4..eb58f22e4 100644 --- a/api/peering_test.go +++ b/api/peering_test.go @@ -44,7 +44,6 @@ func TestAPI_Peering_ACLDeny(t *testing.T) { serverConfig.ACL.Tokens.InitialManagement = "root" serverConfig.ACL.Enabled = true serverConfig.ACL.DefaultPolicy = "deny" - serverConfig.Ports.GRPC = 5300 }) defer s1.Stop() @@ -52,7 +51,6 @@ func TestAPI_Peering_ACLDeny(t *testing.T) { serverConfig.ACL.Tokens.InitialManagement = "root" serverConfig.ACL.Enabled = true serverConfig.ACL.DefaultPolicy = "deny" - serverConfig.Ports.GRPC = 5301 serverConfig.Datacenter = "dc2" }) defer s2.Stop() @@ -265,7 +263,7 @@ func TestAPI_Peering_GenerateToken_ExternalAddresses(t *testing.T) { func TestAPI_Peering_GenerateToken_Read_Establish_Delete(t *testing.T) { t.Parallel() - c, s := makeClient(t) // this is "dc1" + c, s := makeClientWithConfig(t, nil, nil) // this is "dc1" defer s.Stop() s.WaitForSerfCheck(t) diff --git a/command/peering/establish/establish_test.go b/command/peering/establish/establish_test.go index 4d1d83e36..f24ac2a26 100644 --- a/command/peering/establish/establish_test.go +++ b/command/peering/establish/establish_test.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/consul/agent" "github.com/hashicorp/consul/api" + "github.com/hashicorp/consul/sdk/testutil/retry" "github.com/hashicorp/consul/testrpc" ) @@ -84,10 +85,12 @@ func TestEstablishCommand(t *testing.T) { fmt.Sprintf("-peering-token=%s", res.PeeringToken), } - code := cmd.Run(args) - require.Equal(t, 0, code) - output := ui.OutputWriter.String() - require.Contains(t, output, "Success") + retry.Run(t, func(r *retry.R) { + code := cmd.Run(args) + require.Equal(r, 0, code) + output := ui.OutputWriter.String() + require.Contains(r, output, "Success") + }) }) t.Run("establish connection with options", func(t *testing.T) { @@ -107,12 +110,14 @@ func TestEstablishCommand(t *testing.T) { "-meta=region=us-west-1", } - code := cmd.Run(args) - require.Equal(t, 0, code) - output := ui.OutputWriter.String() - require.Contains(t, output, "Success") + retry.Run(t, func(r *retry.R) { + code := cmd.Run(args) + require.Equal(r, 0, code) + output := ui.OutputWriter.String() + require.Contains(r, output, "Success") + }) - //Meta + // Meta peering, _, err := dialingClient.Peerings().Read(context.Background(), "bar", &api.QueryOptions{}) require.NoError(t, err) diff --git a/command/peering/generate/generate_test.go b/command/peering/generate/generate_test.go index c74597610..b23d664d8 100644 --- a/command/peering/generate/generate_test.go +++ b/command/peering/generate/generate_test.go @@ -78,7 +78,7 @@ func TestGenerateCommand(t *testing.T) { require.Equal(t, 0, code) token, err := base64.StdEncoding.DecodeString(ui.OutputWriter.String()) require.NoError(t, err, "error decoding token") - require.Contains(t, string(token), "\"ServerName\":\"server.dc1.consul\"") + require.Contains(t, string(token), "\"ServerName\":\"server.dc1.peering.11111111-2222-3333-4444-555555555555.consul\"") }) t.Run("generate token with options", func(t *testing.T) { @@ -97,13 +97,13 @@ func TestGenerateCommand(t *testing.T) { require.Equal(t, 0, code) token, err := base64.StdEncoding.DecodeString(ui.OutputWriter.String()) require.NoError(t, err, "error decoding token") - require.Contains(t, string(token), "\"ServerName\":\"server.dc1.consul\"") + require.Contains(t, string(token), "\"ServerName\":\"server.dc1.peering.11111111-2222-3333-4444-555555555555.consul\"") - //ServerExternalAddresses + // ServerExternalAddresses require.Contains(t, string(token), "1.2.3.4") require.Contains(t, string(token), "5.6.7.8") - //Meta + // Meta peering, _, err := client.Peerings().Read(context.Background(), "bar", &api.QueryOptions{}) require.NoError(t, err) @@ -136,6 +136,6 @@ func TestGenerateCommand(t *testing.T) { token, err := base64.StdEncoding.DecodeString(outputRes.PeeringToken) require.NoError(t, err, "error decoding token") - require.Contains(t, string(token), "\"ServerName\":\"server.dc1.consul\"") + require.Contains(t, string(token), "\"ServerName\":\"server.dc1.peering.11111111-2222-3333-4444-555555555555.consul\"") }) } diff --git a/sdk/testutil/server.go b/sdk/testutil/server.go index a657919ad..928b84299 100644 --- a/sdk/testutil/server.go +++ b/sdk/testutil/server.go @@ -52,6 +52,7 @@ type TestPortConfig struct { SerfWan int `json:"serf_wan,omitempty"` Server int `json:"server,omitempty"` GRPC int `json:"grpc,omitempty"` + GRPCTLS int `json:"grpc_tls,omitempty"` ProxyMinPort int `json:"proxy_min_port,omitempty"` ProxyMaxPort int `json:"proxy_max_port,omitempty"` } @@ -156,7 +157,7 @@ func defaultServerConfig(t TestingTB) *TestServerConfig { panic(err) } - ports := freeport.GetN(t, 7) + ports := freeport.GetN(t, 8) logBuffer := NewLogBuffer(t) @@ -180,6 +181,7 @@ func defaultServerConfig(t TestingTB) *TestServerConfig { SerfWan: ports[4], Server: ports[5], GRPC: ports[6], + GRPCTLS: ports[7], }, ReadyTimeout: 10 * time.Second, StopTimeout: 10 * time.Second, @@ -224,11 +226,12 @@ type TestServer struct { cmd *exec.Cmd Config *TestServerConfig - HTTPAddr string - HTTPSAddr string - LANAddr string - WANAddr string - GRPCAddr string + HTTPAddr string + HTTPSAddr string + LANAddr string + WANAddr string + GRPCAddr string + GRPCTLSAddr string HTTPClient *http.Client @@ -302,11 +305,12 @@ func NewTestServerConfigT(t TestingTB, cb ServerConfigCallback) (*TestServer, er Config: cfg, cmd: cmd, - HTTPAddr: httpAddr, - HTTPSAddr: fmt.Sprintf("127.0.0.1:%d", cfg.Ports.HTTPS), - LANAddr: fmt.Sprintf("127.0.0.1:%d", cfg.Ports.SerfLan), - WANAddr: fmt.Sprintf("127.0.0.1:%d", cfg.Ports.SerfWan), - GRPCAddr: fmt.Sprintf("127.0.0.1:%d", cfg.Ports.GRPC), + HTTPAddr: httpAddr, + HTTPSAddr: fmt.Sprintf("127.0.0.1:%d", cfg.Ports.HTTPS), + LANAddr: fmt.Sprintf("127.0.0.1:%d", cfg.Ports.SerfLan), + WANAddr: fmt.Sprintf("127.0.0.1:%d", cfg.Ports.SerfWan), + GRPCAddr: fmt.Sprintf("127.0.0.1:%d", cfg.Ports.GRPC), + GRPCTLSAddr: fmt.Sprintf("127.0.0.1:%d", cfg.Ports.GRPCTLS), HTTPClient: client,