Add worker (client) authentication

Implement TLS client certificate authentication. It is enabled by
specifying --ca-file, --key-file and --cert-file, on both the nfd-master
and nfd-worker side. When enabled, nfd-master verifies that the client
(worker) presents a valid certificate signed by the root certificate
(--ca-file). In addition, nfd-master does authorization based on the Common Name
(CN) of the client certificate: CN must match the node name specified in
the labeling request. This ensures (assuming that the worker
certificates are correctly deployed) that nfd-worker is only able to label
the node it is running on, i.e. prevents it from labeling other nodes.

cmd/nfd-master/main.go

@@ -17,7 +17,10 @@ limitations under the License.
 package main
 
 import (
+	"crypto/tls"
+	"crypto/x509"
 	"fmt"
+	"io/ioutil"
 	"log"
 	"net"
 	"os"
@@ -30,6 +33,7 @@ import (
 	"golang.org/x/net/context"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials"
+	"google.golang.org/grpc/peer"
 	"sigs.k8s.io/node-feature-discovery/pkg/apihelper"
 	pb "sigs.k8s.io/node-feature-discovery/pkg/labeler"
 	"sigs.k8s.io/node-feature-discovery/pkg/version"
@@ -63,6 +67,7 @@ type Annotations map[string]string
 
 // Command line arguments
 type Args struct {
+	caFile         string
 	certFile       string
 	keyFile        string
 	labelWhiteList *regexp.Regexp
@@ -101,12 +106,28 @@ func main() {
 
 	serverOpts := []grpc.ServerOption{}
 	// Use TLS if --cert-file or --key-file is defined
-	if args.certFile != "" || args.keyFile != "" {
+	if args.caFile != "" || args.certFile != "" || args.keyFile != "" {
-		creds, err := credentials.NewServerTLSFromFile(args.certFile, args.keyFile)
+		// Load cert for authenticating this server
+		cert, err := tls.LoadX509KeyPair(args.certFile, args.keyFile)
 		if err != nil {
-			log.Fatalf("Failed to generate credentials %v", err)
+			stderrLogger.Fatalf("failed to load server certificate: %v", err)
 		}
-		serverOpts = append(serverOpts, grpc.Creds(creds))
+		// Load CA cert for client cert verification
+		caCert, err := ioutil.ReadFile(args.caFile)
+		if err != nil {
+			stderrLogger.Fatalf("failed to read root certificate file: %v", err)
+		}
+		caPool := x509.NewCertPool()
+		if ok := caPool.AppendCertsFromPEM(caCert); !ok {
+			stderrLogger.Fatalf("failed to add certificate from '%s'", args.caFile)
+		}
+		// Create TLS config
+		tlsConfig := &tls.Config{
+			Certificates: []tls.Certificate{cert},
+			ClientCAs:    caPool,
+			ClientAuth:   tls.RequireAndVerifyClientCert,
+		}
+		serverOpts = append(serverOpts, grpc.Creds(credentials.NewTLS(tlsConfig)))
 	}
 	grpcServer := grpc.NewServer(serverOpts...)
 	pb.RegisterLabelerServer(grpcServer, &labelerServer{args: args, apiHelper: helper})
@@ -122,7 +143,7 @@ func argsParse(argv []string) (Args, error) {
 
   Usage:
   %s [--no-publish] [--label-whitelist=<pattern>] [--port=<port>]
-     [--cert-file=<path>] [--key-file=<path>]
+     [--ca-file=<path>] [--cert-file=<path>] [--key-file=<path>]
   %s -h | --help
   %s --version
 
@@ -131,6 +152,8 @@ func argsParse(argv []string) (Args, error) {
   --version                   Output version and exit.
   --port=<port>               Port on which to listen for connections.
                               [Default: 8080]
+  --ca-file=<path>            Root certificate for verifying connections
+                              [Default: ]
   --cert-file=<path>          Certificate used for authenticating connections
                               [Default: ]
   --key-file=<path>           Private key matching --cert-file
@@ -149,6 +172,7 @@ func argsParse(argv []string) (Args, error) {
 
 	// Parse argument values as usable types.
 	var err error
+	args.caFile = arguments["--ca-file"].(string)
 	args.certFile = arguments["--cert-file"].(string)
 	args.keyFile = arguments["--key-file"].(string)
 	args.noPublish = arguments["--no-publish"].(bool)
@@ -162,12 +186,15 @@ func argsParse(argv []string) (Args, error) {
 	}
 
 	// Check TLS related args
-	if args.certFile != "" || args.keyFile != "" {
+	if args.certFile != "" || args.keyFile != "" || args.caFile != "" {
 		if args.certFile == "" {
-			return args, fmt.Errorf("--cert-file needs to be specified alongside --key-file")
+			return args, fmt.Errorf("--cert-file needs to be specified alongside --key-file and --ca-file")
 		}
 		if args.keyFile == "" {
-			return args, fmt.Errorf("--key-file needs to be specified alongside --cert-file")
+			return args, fmt.Errorf("--key-file needs to be specified alongside --cert-file and --ca-file")
+		}
+		if args.caFile == "" {
+			return args, fmt.Errorf("--ca-file needs to be specified alongside --cert-file and --key-file")
 		}
 	}
 	return args, nil
@@ -214,7 +241,30 @@ type labelerServer struct {
 
 // Service SetLabels
 func (s *labelerServer) SetLabels(c context.Context, r *pb.SetLabelsRequest) (*pb.SetLabelsReply, error) {
+	// Client authorization.
+	// Check that the node name matches the CN from the TLS cert
+	client, ok := peer.FromContext(c)
+	if !ok {
+		stderrLogger.Printf("gRPC request error: failed to get peer (client)")
+		return &pb.SetLabelsReply{}, fmt.Errorf("failed to get peer (client)")
+	}
+	tlsAuth, ok := client.AuthInfo.(credentials.TLSInfo)
+	if !ok {
+		stderrLogger.Printf("gRPC request error: incorrect client credentials from '%v'", client.Addr)
+		return &pb.SetLabelsReply{}, fmt.Errorf("incorrect client credentials")
+	}
+	if len(tlsAuth.State.VerifiedChains) == 0 || len(tlsAuth.State.VerifiedChains[0]) == 0 {
+		stderrLogger.Printf("gRPC request error: client certificate verification for '%v' failed", client.Addr)
+		return &pb.SetLabelsReply{}, fmt.Errorf("client certificate verification failed")
+	}
+	cn := tlsAuth.State.VerifiedChains[0][0].Subject.CommonName
+	if cn != r.NodeName {
+		stderrLogger.Printf("gRPC request error: authorization for %v failed: cert valid for '%s', requested node name '%s'", client.Addr, cn, r.NodeName)
+		return &pb.SetLabelsReply{}, fmt.Errorf("request authorization failed: cert valid for '%s', requested node name '%s'", cn, r.NodeName)
+	}
+
 	stdoutLogger.Printf("REQUEST Node: %s NFD-version: %s Labels: %s", r.NodeName, r.NfdVersion, r.Labels)
+
 	if !s.args.noPublish {
 		// Advertise NFD worker version and label names as annotations
 		keys := make([]string, 0, len(r.Labels))

cmd/nfd-master/main_test.go

@@ -223,12 +223,13 @@ func TestArgsParse(t *testing.T) {
 		})
 
 		Convey("When valid args are specified", func() {
-			args, err := argsParse([]string{"--label-whitelist=.*rdt.*", "--port=1234", "--cert-file=crt", "--key-file=key"})
+			args, err := argsParse([]string{"--label-whitelist=.*rdt.*", "--port=1234", "--cert-file=crt", "--key-file=key", "--ca-file=ca"})
 			Convey("Argument parsing should succeed and args set to correct values", func() {
 				So(args.noPublish, ShouldBeFalse)
 				So(args.port, ShouldEqual, 1234)
 				So(args.certFile, ShouldEqual, "crt")
 				So(args.keyFile, ShouldEqual, "key")
+				So(args.caFile, ShouldEqual, "ca")
 				So(args.labelWhiteList.String(), ShouldResemble, ".*rdt.*")
 				So(err, ShouldBeNil)
 			})
@@ -240,12 +241,14 @@ func TestArgsParse(t *testing.T) {
 			})
 		})
 
-		Convey("When --cert-file or --key-file is specified on its own", func() {
+		Convey("When one of --cert-file, --key-file or --ca-file is missing", func() {
-			_, err := argsParse([]string{"--cert-file=crt"})
+			_, err := argsParse([]string{"--cert-file=crt", "--key-file=key"})
-			_, err2 := argsParse([]string{"--key-file=key"})
+			_, err2 := argsParse([]string{"--key-file=key", "--ca-file=ca"})
+			_, err3 := argsParse([]string{"--cert-file=crt", "--ca-file=ca"})
 			Convey("argsParse should fail", func() {
 				So(err, ShouldNotBeNil)
 				So(err2, ShouldNotBeNil)
+				So(err3, ShouldNotBeNil)
 			})
 		})
 	})

cmd/nfd-worker/main.go

@@ -17,6 +17,8 @@ limitations under the License.
 package main
 
 import (
+	"crypto/tls"
+	"crypto/x509"
 	"fmt"
 	"io/ioutil"
 	"log"
@@ -84,6 +86,8 @@ type Annotations map[string]string
 type Args struct {
 	labelWhiteList string
 	caFile         string
+	certFile       string
+	keyFile        string
 	configFile     string
 	noPublish      bool
 	options        string
@@ -120,12 +124,27 @@ func main() {
 
 	// Connect to NFD server
 	dialOpts := []grpc.DialOption{}
-	if args.caFile != "" {
+	if args.caFile != "" || args.certFile != "" || args.keyFile != "" {
-		creds, err := credentials.NewClientTLSFromFile(args.caFile, "")
+		// Load client cert for client authentication
+		cert, err := tls.LoadX509KeyPair(args.certFile, args.keyFile)
 		if err != nil {
-			stderrLogger.Fatalf("failed to create credentials %v", err)
+			stderrLogger.Fatalf("failed to load client certificate: %v", err)
 		}
-		dialOpts = append(dialOpts, grpc.WithTransportCredentials(creds))
+		// Load CA cert for server cert verification
+		caCert, err := ioutil.ReadFile(args.caFile)
+		if err != nil {
+			stderrLogger.Fatalf("failed to read root certificate file: %v", err)
+		}
+		caPool := x509.NewCertPool()
+		if ok := caPool.AppendCertsFromPEM(caCert); !ok {
+			stderrLogger.Fatalf("failed to add certificate from '%s'", args.caFile)
+		}
+		// Create TLS config
+		tlsConfig := &tls.Config{
+			Certificates: []tls.Certificate{cert},
+			RootCAs:      caPool,
+		}
+		dialOpts = append(dialOpts, grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig)))
 	} else {
 		dialOpts = append(dialOpts, grpc.WithInsecure())
 	}
@@ -171,7 +190,8 @@ func argsParse(argv []string) (Args, error) {
   Usage:
   %s [--no-publish] [--sources=<sources>] [--label-whitelist=<pattern>]
      [--oneshot | --sleep-interval=<seconds>] [--config=<path>]
-     [--options=<config>] [--ca-file=<path>] [--server=<server>]
+     [--options=<config>] [--server=<server>]
+     [--ca-file=<path>] [--cert-file=<path>] [--key-file=<path>]
   %s -h | --help
   %s --version
 
@@ -187,6 +207,10 @@ func argsParse(argv []string) (Args, error) {
                               [Default: ]
   --ca-file=<path>            Root certificate for verifying connections
                               [Default: ]
+  --cert-file=<path>          Certificate used for authenticating connections
+                              [Default: ]
+  --key-file=<path>           Private key matching --cert-file
+                              [Default: ]
   --server=<server>           NFD server address to connecto to.
                               [Default: localhost:8080]
   --sources=<sources>         Comma separated list of feature sources.
@@ -211,7 +235,9 @@ func argsParse(argv []string) (Args, error) {
 	// Parse argument values as usable types.
 	var err error
 	args.caFile = arguments["--ca-file"].(string)
+	args.certFile = arguments["--cert-file"].(string)
 	args.configFile = arguments["--config"].(string)
+	args.keyFile = arguments["--key-file"].(string)
 	args.noPublish = arguments["--no-publish"].(bool)
 	args.options = arguments["--options"].(string)
 	args.server = arguments["--server"].(string)
@@ -229,6 +255,18 @@ func argsParse(argv []string) (Args, error) {
 		args.sleepInterval = time.Second
 	}
 
+	// Check TLS related args
+	if args.certFile != "" || args.keyFile != "" || args.caFile != "" {
+		if args.certFile == "" {
+			return args, fmt.Errorf("--cert-file needs to be specified alongside --key-file and --ca-file")
+		}
+		if args.keyFile == "" {
+			return args, fmt.Errorf("--key-file needs to be specified alongside --cert-file and --ca-file")
+		}
+		if args.caFile == "" {
+			return args, fmt.Errorf("--ca-file needs to be specified alongside --cert-file and --key-file")
+		}
+	}
 	return args, nil
 }
 

cmd/nfd-worker/main_test.go

@@ -117,16 +117,29 @@ func TestArgsParse(t *testing.T) {
 		})
 
 		Convey("When valid args are specified", func() {
-			args, err := argsParse([]string{"--no-publish", "--sources=fake1,fake2,fake3", "--ca-file=ca"})
+			args, err := argsParse([]string{"--no-publish", "--sources=fake1,fake2,fake3", "--ca-file=ca", "--cert-file=crt", "--key-file=key"})
 
 			Convey("--no-publish is set and args.sources is set to appropriate values", func() {
 				So(args.noPublish, ShouldBeTrue)
 				So(args.caFile, ShouldEqual, "ca")
+				So(args.certFile, ShouldEqual, "crt")
+				So(args.keyFile, ShouldEqual, "key")
 				So(args.sources, ShouldResemble, []string{"fake1", "fake2", "fake3"})
 				So(len(args.labelWhiteList), ShouldEqual, 0)
 				So(err, ShouldBeNil)
 			})
 		})
+
+		Convey("When one of --cert-file, --key-file or --ca-file is missing", func() {
+			_, err := argsParse([]string{"--cert-file=crt", "--key-file=key"})
+			_, err2 := argsParse([]string{"--key-file=key", "--ca-file=ca"})
+			_, err3 := argsParse([]string{"--cert-file=crt", "--ca-file=ca"})
+			Convey("Argument parsing should fail", func() {
+				So(err, ShouldNotBeNil)
+				So(err2, ShouldNotBeNil)
+				So(err3, ShouldNotBeNil)
+			})
+		})
 	})
 }