Enforce Admin.TLS.ClientAuthRequired in orderer config (#2107)

This value is not currently used, however, mutual TLS is required
for the channel participation actions. Enforce that it is set to true
when TLS is enabled for now (panic if set to false).

FAB-18334 #done

Signed-off-by: Will Lahti <wtlahti@us.ibm.com>

integration/channelparticipation/channel_participation.go

@@ -74,18 +74,14 @@ type ChannelInfoShort struct {
 }
 
 func List(n *nwo.Network, o *nwo.Orderer) ChannelList {
-	authClient, unauthClient := nwo.OrdererOperationalClients(n, o)
+	authClient, _ := nwo.OrdererOperationalClients(n, o)
 	listChannelsURL := fmt.Sprintf("https://127.0.0.1:%d/participation/v1/channels", n.OrdererPort(o, nwo.AdminPort))
 
 	body := getBody(authClient, listChannelsURL)()
 	list := &ChannelList{}
 	err := json.Unmarshal([]byte(body), list)
 	Expect(err).NotTo(HaveOccurred())
 
-	resp, err := unauthClient.Get(listChannelsURL)
-	Expect(err).NotTo(HaveOccurred())
-	Expect(resp.StatusCode).To(Equal(http.StatusUnauthorized))
-
 	return *list
 }
 

integration/nwo/orderer_template.go

@@ -123,7 +123,7 @@ Admin:
     Certificate: {{ $w.OrdererLocalTLSDir Orderer }}/server.crt
     RootCAs:
     -  {{ $w.OrdererLocalTLSDir Orderer }}/ca.crt
-    ClientAuthRequired: {{ $w.ClientAuthRequired }}
+    ClientAuthRequired: true
     ClientRootCAs:
     -  {{ $w.OrdererLocalTLSDir Orderer }}/ca.crt
 {{- end }}

integration/raft/channel_participation_test.go

@@ -731,6 +731,16 @@ var _ = Describe("ChannelParticipation", func() {
 				Height:            7,
 			})
 		})
+
+		It("requires a client certificate to connect when TLS is enabled", func() {
+			orderer := network.Orderer("orderer1")
+			_, unauthClient := nwo.OrdererOperationalClients(network, orderer)
+			ordererAddress := fmt.Sprintf("127.0.0.1:%d", network.OrdererPort(orderer, nwo.AdminPort))
+			listChannelsURL := fmt.Sprintf("https://%s/participation/v1/channels", ordererAddress)
+
+			_, err := unauthClient.Get(listChannelsURL)
+			Expect(err).To(MatchError(fmt.Sprintf("Get \"%s\": dial tcp %s: connect: connection refused", listChannelsURL, ordererAddress)))
+		})
 	})
 
 	Describe("three node etcdraft network with a system channel", func() {

orderer/common/localconfig/config.go

@@ -486,6 +486,9 @@ func (c *TopLevel) completeInitialization(configDir string) {
 			logger.Infof("Kafka.Version unset, setting to %v", Defaults.Kafka.Version)
 			c.Kafka.Version = Defaults.Kafka.Version
 
+		case c.Admin.TLS.Enabled && !c.Admin.TLS.ClientAuthRequired:
+			logger.Panic("Admin.TLS.ClientAuthRequired must be set to true if Admin.TLS.Enabled is set to true")
+
 		default:
 			return
 		}

orderer/common/localconfig/config_test.go

@@ -181,6 +181,49 @@ func TestKafkaSASLPlain(t *testing.T) {
 	}
 }
 
+func TestAdminTLSConfig(t *testing.T) {
+	testCases := []struct {
+		name        string
+		tls         TLS
+		shouldPanic bool
+	}{
+		{
+			name: "no TLS",
+			tls: TLS{
+				Enabled:            false,
+				ClientAuthRequired: false,
+			},
+			shouldPanic: false,
+		},
+		{
+			name: "TLS enabled and ClientAuthRequired",
+			tls: TLS{
+				Enabled:            true,
+				ClientAuthRequired: true,
+			},
+			shouldPanic: false,
+		},
+		{
+			name: "TLS enabled and ClientAuthRequired set to false",
+			tls: TLS{
+				Enabled:            true,
+				ClientAuthRequired: false,
+			},
+			shouldPanic: true,
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			uconf := &TopLevel{Admin: Admin{TLS: tc.tls}}
+			if tc.shouldPanic {
+				require.PanicsWithValue(t, "Admin.TLS.ClientAuthRequired must be set to true if Admin.TLS.Enabled is set to true", func() { uconf.completeInitialization("/dummy/path") })
+			} else {
+				require.NotPanics(t, func() { uconf.completeInitialization("/dummy/path") }, "Should not panic")
+			}
+		})
+	}
+}
+
 func TestClusterDefaults(t *testing.T) {
 	cleanup := configtest.SetDevFabricConfigPath(t)
 	defer cleanup()

sampleconfig/orderer.yaml

@@ -359,7 +359,10 @@ Admin:
         # Most admin service endpoints require client authentication when TLS
         # is enabled. ClientAuthRequired requires client certificate authentication
         # at the TLS layer to access all resources.
-        ClientAuthRequired: false
+        #
+        # NOTE: When TLS is enabled, the admin endpoint requires mutual TLS. The
+        # orderer will panic on startup if this value is set to false.
+        ClientAuthRequired: true
 
         # Paths to PEM encoded ca certificates to trust for client authentication
         ClientRootCAs: []