Add awareness of server mode to TLS configurator

Preivously the TLS configurator would default to presenting auto TLS
certificates as client certificates.

Server agents should not have this behavior and should instead present
the manually configured certs. The autoTLS certs for servers are
exclusively used for peering and should not be used as the default for
outbound communication.

agent/config/builder.go

@@ -2596,6 +2596,7 @@ func (b *builder) buildTLSConfig(rt RuntimeConfig, t TLS) (tlsutil.Config, error
 	mapCommon("grpc", t.GRPC, &c.GRPC)
 	c.GRPC.UseAutoCert = boolValWithDefault(t.GRPC.UseAutoCert, false)
 
+	c.ServerMode = rt.ServerMode
 	c.ServerName = rt.ServerName
 	c.NodeName = rt.NodeName
 	c.Domain = rt.DNSDomain

agent/config/runtime_oss_test.go

@@ -66,6 +66,7 @@ func TestLoad_IntegrationWithFlags_OSS(t *testing.T) {
 			expected: func(rt *RuntimeConfig) {
 				rt.DataDir = dataDir
 				rt.ServerMode = true
+				rt.TLS.ServerMode = true
 				rt.LeaveOnTerm = false
 				rt.SkipLeaveOnInt = true
 				rt.RPCConfig.EnableStreaming = true

agent/config/runtime_test.go

@@ -177,6 +177,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 		expected: func(rt *RuntimeConfig) {
 			rt.Bootstrap = true
 			rt.ServerMode = true
+			rt.TLS.ServerMode = true
 			rt.LeaveOnTerm = false
 			rt.SkipLeaveOnInt = true
 			rt.DataDir = dataDir
@@ -194,6 +195,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 		expected: func(rt *RuntimeConfig) {
 			rt.BootstrapExpect = 3
 			rt.ServerMode = true
+			rt.TLS.ServerMode = true
 			rt.LeaveOnTerm = false
 			rt.SkipLeaveOnInt = true
 			rt.DataDir = dataDir
@@ -208,6 +210,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			`-data-dir=` + dataDir,
 		},
 		expected: func(rt *RuntimeConfig) {
+			rt.TLS.ServerMode = false
 			rt.ClientAddrs = []*net.IPAddr{ipAddr("1.2.3.4")}
 			rt.DNSAddrs = []net.Addr{tcpAddr("1.2.3.4:8600"), udpAddr("1.2.3.4:8600")}
 			rt.HTTPAddrs = []net.Addr{tcpAddr("1.2.3.4:8500")}
@@ -319,6 +322,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.SerfBindAddrLAN = tcpAddr("127.0.0.1:8301")
 			rt.SerfBindAddrWAN = tcpAddr("127.0.0.1:8302")
 			rt.ServerMode = true
+			rt.TLS.ServerMode = true
 			rt.SkipLeaveOnInt = true
 			rt.TaggedAddresses = map[string]string{
 				"lan":      "127.0.0.1",
@@ -659,6 +663,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.DataDir = dataDir
 			// server things
 			rt.ServerMode = true
+			rt.TLS.ServerMode = true
 			rt.LeaveOnTerm = false
 			rt.SkipLeaveOnInt = true
 			rt.RPCConfig.EnableStreaming = true
@@ -841,6 +846,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 		},
 		expected: func(rt *RuntimeConfig) {
 			rt.ServerMode = true
+			rt.TLS.ServerMode = true
 			rt.LeaveOnTerm = false
 			rt.SkipLeaveOnInt = true
 			rt.DataDir = dataDir
@@ -1881,6 +1887,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.BootstrapExpect = 0
 			rt.LeaveOnTerm = false
 			rt.ServerMode = true
+			rt.TLS.ServerMode = true
 			rt.SkipLeaveOnInt = true
 			rt.DataDir = dataDir
 			rt.RPCConfig.EnableStreaming = true
@@ -1898,6 +1905,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.BootstrapExpect = 2
 			rt.LeaveOnTerm = false
 			rt.ServerMode = true
+			rt.TLS.ServerMode = true
 			rt.SkipLeaveOnInt = true
 			rt.DataDir = dataDir
 			rt.RPCConfig.EnableStreaming = true
@@ -1918,6 +1926,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.BootstrapExpect = 4
 			rt.LeaveOnTerm = false
 			rt.ServerMode = true
+			rt.TLS.ServerMode = true
 			rt.SkipLeaveOnInt = true
 			rt.DataDir = dataDir
 			rt.RPCConfig.EnableStreaming = true
@@ -1937,6 +1946,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 		expected: func(rt *RuntimeConfig) {
 			rt.LeaveOnTerm = true
 			rt.ServerMode = false
+			rt.TLS.ServerMode = false
 			rt.SkipLeaveOnInt = false
 			rt.DataDir = dataDir
 		},
@@ -3056,6 +3066,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 
 			// server things
 			rt.ServerMode = true
+			rt.TLS.ServerMode = true
 			rt.LeaveOnTerm = false
 			rt.SkipLeaveOnInt = true
 			rt.RPCConfig.EnableStreaming = true
@@ -3087,6 +3098,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 
 			// server things
 			rt.ServerMode = true
+			rt.TLS.ServerMode = true
 			rt.LeaveOnTerm = false
 			rt.SkipLeaveOnInt = true
 			rt.RPCConfig.EnableStreaming = true
@@ -3115,6 +3127,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 
 			// server things
 			rt.ServerMode = true
+			rt.TLS.ServerMode = true
 			rt.LeaveOnTerm = false
 			rt.SkipLeaveOnInt = true
 			rt.RPCConfig.EnableStreaming = true
@@ -3140,6 +3153,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.ConnectEnabled = true
 			// server things
 			rt.ServerMode = true
+			rt.TLS.ServerMode = true
 			rt.LeaveOnTerm = false
 			rt.SkipLeaveOnInt = true
 			rt.RPCConfig.EnableStreaming = true
@@ -3162,6 +3176,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			// rpc.enable_streaming make no sense in not-server mode
 			rt.RPCConfig.EnableStreaming = true
 			rt.ServerMode = false
+			rt.TLS.ServerMode = false
 		},
 	})
 	run(t, testCase{
@@ -3185,6 +3200,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.UseStreamingBackend = true
 			// server things
 			rt.ServerMode = true
+			rt.TLS.ServerMode = true
 			rt.LeaveOnTerm = false
 			rt.SkipLeaveOnInt = true
 		},
@@ -3602,6 +3618,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.ConnectMeshGatewayWANFederationEnabled = true
 			// server things
 			rt.ServerMode = true
+			rt.TLS.ServerMode = true
 			rt.LeaveOnTerm = false
 			rt.SkipLeaveOnInt = true
 			rt.RPCConfig.EnableStreaming = true
@@ -4509,7 +4526,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 		},
 	})
 
-	///////////////////////////////////
+	// /////////////////////////////////
 	// Defaults sanity checks
 
 	run(t, testCase{
@@ -4532,7 +4549,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 		},
 	})
 
-	///////////////////////////////////
+	// /////////////////////////////////
 	// Auto Config related tests
 	run(t, testCase{
 		desc: "auto config and auto encrypt error",
@@ -5023,6 +5040,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.DataDir = dataDir
 			rt.LeaveOnTerm = false
 			rt.ServerMode = true
+			rt.TLS.ServerMode = true
 			rt.SkipLeaveOnInt = true
 			rt.TLS.InternalRPC.CertFile = "foo"
 			rt.RPCConfig.EnableStreaming = true
@@ -6441,6 +6459,7 @@ func TestLoad_FullConfig(t *testing.T) {
 			},
 			NodeName:                "otlLxGaI",
 			ServerName:              "Oerr9n1G",
+			ServerMode:              true,
 			Domain:                  "7W1xXSqd",
 			EnableAgentTLSForChecks: true,
 		},

agent/config/testdata/TestRuntimeConfig_Sanitize.golden

@@ -374,10 +374,10 @@
             "CipherSuites": [],
             "KeyFile": "hidden",
             "TLSMinVersion": "",
+            "UseAutoCert": false,
             "VerifyIncoming": false,
             "VerifyOutgoing": false,
-            "VerifyServerHostname": false,
-            "UseAutoCert": false
+            "VerifyServerHostname": false
         },
         "HTTPS": {
             "CAFile": "",
@@ -386,10 +386,10 @@
             "CipherSuites": [],
             "KeyFile": "hidden",
             "TLSMinVersion": "",
+            "UseAutoCert": false,
             "VerifyIncoming": false,
             "VerifyOutgoing": false,
-            "VerifyServerHostname": false,
-            "UseAutoCert": false
+            "VerifyServerHostname": false
         },
         "InternalRPC": {
             "CAFile": "",
@@ -398,12 +398,13 @@
             "CipherSuites": [],
             "KeyFile": "hidden",
             "TLSMinVersion": "",
+            "UseAutoCert": false,
             "VerifyIncoming": false,
             "VerifyOutgoing": false,
-            "VerifyServerHostname": false,
-            "UseAutoCert": false
+            "VerifyServerHostname": false
         },
         "NodeName": "",
+        "ServerMode": false,
         "ServerName": ""
     },
     "TaggedAddresses": {},
@@ -471,4 +472,4 @@
     "VersionMetadata": "",
     "VersionPrerelease": "",
     "Watches": []
-}
+}

tlsutil/config.go

@@ -110,6 +110,10 @@ type ProtocolConfig struct {
 
 // Config configures the Configurator.
 type Config struct {
+	// ServerMode indicates whether the configurator is attached to a server
+	// or client agent.
+	ServerMode bool
+
 	// InternalRPC is used to configure the internal multiplexed RPC protocol.
 	InternalRPC ProtocolConfig
 
@@ -597,9 +601,12 @@ func (c *Configurator) commonTLSConfig(state protocolConfig, cfg ProtocolConfig,
 	// to a server requesting a certificate. Return the autoEncrypt certificate
 	// if possible, otherwise default to the manually provisioned one.
 	tlsConfig.GetClientCertificate = func(*tls.CertificateRequestInfo) (*tls.Certificate, error) {
-		cert := c.autoTLS.cert
-		if cert == nil {
-			cert = state.cert
+		cert := state.cert
+
+		// In the general case we only prefer to dial out with the autoTLS cert if we are a client.
+		// The server's autoTLS cert is exclusively for peering control plane traffic.
+		if !c.base.ServerMode && c.autoTLS.cert != nil {
+			cert = c.autoTLS.cert
 		}
 
 		if cert == nil {

tlsutil/config_test.go

@@ -467,6 +467,98 @@ func TestConfigurator_ALPNRPCConfig(t *testing.T) {
 	})
 }
 
+func TestConfigurator_OutgoingRPC_ServerMode(t *testing.T) {
+	type testCase struct {
+		clientConfig Config
+		expectName   string
+	}
+
+	run := func(t *testing.T, tc testCase) {
+		serverCfg := makeConfigurator(t, Config{
+			InternalRPC: ProtocolConfig{
+				CAFile:         "../test/hostname/CertAuth.crt",
+				CertFile:       "../test/hostname/Alice.crt",
+				KeyFile:        "../test/hostname/Alice.key",
+				VerifyIncoming: true,
+			},
+			ServerMode: true,
+		})
+
+		serverConn, errc, certc := startTLSServer(serverCfg.IncomingRPCConfig())
+		if serverConn == nil {
+			t.Fatalf("startTLSServer err: %v", <-errc)
+		}
+
+		clientCfg := makeConfigurator(t, tc.clientConfig)
+
+		bettyCert := loadFile(t, "../test/hostname/Betty.crt")
+		bettyKey := loadFile(t, "../test/hostname/Betty.key")
+		require.NoError(t, clientCfg.UpdateAutoTLSCert(bettyCert, bettyKey))
+
+		wrap := clientCfg.OutgoingRPCWrapper()
+		require.NotNil(t, wrap)
+
+		tlsClient, err := wrap("dc1", serverConn)
+		require.NoError(t, err)
+		defer tlsClient.Close()
+
+		err = tlsClient.(*tls.Conn).Handshake()
+		require.NoError(t, err)
+
+		err = <-errc
+		require.NoError(t, err)
+
+		clientCerts := <-certc
+		require.NotEmpty(t, clientCerts)
+
+		require.Equal(t, tc.expectName, clientCerts[0].Subject.CommonName)
+
+		// Check the server side of the handshake succeeded.
+		require.NoError(t, <-errc)
+	}
+
+	tt := map[string]testCase{
+		"server with manual cert": {
+			clientConfig: Config{
+				InternalRPC: ProtocolConfig{
+					VerifyOutgoing: true,
+					CAFile:         "../test/hostname/CertAuth.crt",
+					CertFile:       "../test/hostname/Bob.crt",
+					KeyFile:        "../test/hostname/Bob.key",
+				},
+				ServerMode: true,
+			},
+			// Even though an AutoTLS cert is configured, the server will prefer the manually configured cert.
+			expectName: "Bob",
+		},
+		"client with manual cert": {
+			clientConfig: Config{
+				InternalRPC: ProtocolConfig{
+					VerifyOutgoing: true,
+					CAFile:         "../test/hostname/CertAuth.crt",
+					CertFile:       "../test/hostname/Bob.crt",
+					KeyFile:        "../test/hostname/Bob.key",
+				},
+				ServerMode: false,
+			},
+			expectName: "Betty",
+		},
+		"client with auto-TLS": {
+			clientConfig: Config{
+				ServerMode: false,
+				AutoTLS:    true,
+			},
+			expectName: "Betty",
+		},
+	}
+
+	for name, tc := range tt {
+		t.Run(name, func(t *testing.T) {
+			run(t, tc)
+		})
+	}
+}
+
 func TestConfigurator_OutgoingInternalRPCWrapper(t *testing.T) {
 	// if this test is failing because of expired certificates
 	// use the procedure in test/CA-GENERATION.md