Accept client certs based on SAN, not just CN (#514)

* first attempt at SAN-based VerifyNodeName

* Update docs on verify-node-name

(cherry picked from commit 77bd4e4cf6)

cmd/nfd-master/main.go

@@ -103,7 +103,7 @@ func initFlags(flagset *flag.FlagSet) *master.Args {
 	flagset.Var(&args.ResourceLabels, "resource-labels",
 		"Comma separated list of labels to be exposed as extended resources.")
 	flagset.BoolVar(&args.VerifyNodeName, "verify-node-name", false,
-		"Verify worker node name against CN from the TLS certificate. "+
+		"Verify worker node name against the worker's TLS certificate. "+
 			"Only takes effect when TLS authentication has been enabled.")
 
 	return args

deployment/node-feature-discovery/templates/master.yaml

@@ -53,7 +53,7 @@ spec:
 ## a ConfigMap named nfd-ca-cert, and, the TLS authentication credentials stored
 ## in a TLS Secret named nfd-master-cert.
 ## Additional hardening can be enabled by specifying --verify-node-name in
-## args, in which case every nfd-worker requires a individual node-specific
+## args, in which case node name will be checked against the worker's
 ## TLS certificate.
 #            - "--ca-file=/etc/kubernetes/node-feature-discovery/trust/ca.crt"
 #            - "--key-file=/etc/kubernetes/node-feature-discovery/certs/tls.key"

docs/advanced/developer-guide.md

@@ -196,7 +196,7 @@ Usage of nfd-master:
   -resource-labels value
         Comma separated list of labels to be exposed as extended resources.
   -verify-node-name
-        Verify worker node name against CN from the TLS certificate. Only takes effect when TLS authentication has been enabled.
+        Verify worker node name against the worker's TLS certificate. Only takes effect when TLS authentication has been enabled.
   -version
         Print version and exit.
 ```

docs/advanced/master-commandline-reference.md

@@ -124,14 +124,11 @@ nfd-master -key-file=/opt/nfd/master.key -cert-file=/opt/nfd/master.crt -ca-file
 The `-verify-node-name` flag controls the NodeName based authorization of
 incoming requests and only has effect when mTLS authentication has been enabled
 (with `-ca-file`, `-cert-file` and `-key-file`). If enabled, the worker node
-name of the incoming must match with the CN in its TLS certificate. Thus,
+name of the incoming must match with the CN or a SAN in its TLS certificate. Thus,
 workers are only able to label the node they are running on (or the node whose
-certificate they present), and, each worker must have an individual
-certificate.
+certificate they present).
 
-Node Name based authorization is disabled by default and thus it is possible
-for all nfd-worker pods in the cluster to use one shared certificate, making
-NFD deployment much easier.
+Node Name based authorization is disabled by default.
 
 Default: *false*
 

docs/get-started/deployment-and-usage.md

@@ -310,8 +310,8 @@ the nfd-master Service of the cluster. By default, nfd-master only check that
 the nfd-worker has been signed by the specified root certificate (-ca-file).
 Additional hardening can be enabled by specifying -verify-node-name in
 nfd-master args, in which case nfd-master verifies that the NodeName presented
-by nfd-worker matches the Common Name (CN) of its certificate. This means that
-each nfd-worker requires a individual node-specific TLS certificate.
+by nfd-worker matches the Common Name (CN) or a Subject Alternative Name (SAN)
+of its certificate.
 
 #### Automated TLS certificate management using cert-manager
 
@@ -334,9 +334,6 @@ $ kubectl apply -f nfd-cert-manager.yaml
 Finally, deploy `nfd-master.yaml` and `nfd-worker-daemonset.yaml` with the Secrets
 (`nfd-master-cert` and `nfd-worker-cert`) mounts enabled.
 
-**Note:** the automated setup to support `--verify-node-name` hardening cannot
-be configured currently.
-
 ## Worker configuration
 
 NFD-Worker supports dynamic configuration through a configuration file. The

nfd-master.yaml.template

@@ -97,7 +97,7 @@ spec:
 ## the TLS authentication credentials and a root certificate named ca.crt created.
 ## cert-manager can be used to automate the Secret creation and updates.
 ## Additional hardening can be enabled by specifying --verify-node-name in
-## args, in which case every nfd-worker requires a individual node-specific
+## args, in which case node name will be checked against the worker's
 ## TLS certificate.
 #          args:
 #            - "--ca-file=/etc/kubernetes/node-feature-discovery/certs/ca.crt"

pkg/nfd-master/nfd-master.go

@@ -18,6 +18,7 @@ package nfdmaster
 
 import (
 	"crypto/tls"
+	"crypto/x509"
 	"fmt"
 	"net"
 	"os"
@@ -347,6 +348,18 @@ func filterFeatureLabels(labels Labels, extraLabelNs map[string]struct{}, labelW
 	return outLabels, extendedResources
 }
 
+func verifyNodeName(cert *x509.Certificate, nodeName string) error {
+	if cert.Subject.CommonName == nodeName {
+		return nil
+	}
+
+	err := cert.VerifyHostname(nodeName)
+	if err != nil {
+		return fmt.Errorf("Certificate %q not valid for node %q: %v", cert.Subject.CommonName, nodeName, err)
+	}
+	return nil
+}
+
 // SetLabels implements LabelerServer
 func (m *nfdMaster) SetLabels(c context.Context, r *pb.SetLabelsRequest) (*pb.SetLabelsReply, error) {
 	if m.args.VerifyNodeName {
@@ -366,10 +379,11 @@ func (m *nfdMaster) SetLabels(c context.Context, r *pb.SetLabelsRequest) (*pb.Se
 			klog.Errorf("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 {
-			klog.Errorf("gRPC request error: authorization for %v failed: cert valid for %q, requested node name %q", client.Addr, cn, r.NodeName)
-			return &pb.SetLabelsReply{}, fmt.Errorf("request authorization failed: cert valid for %q, requested node name %q", cn, r.NodeName)
+
+		err := verifyNodeName(tlsAuth.State.VerifiedChains[0][0], r.NodeName)
+		if err != nil {
+			klog.Errorf("gRPC request error: authorization for %v failed: %v", client.Addr, err)
+			return &pb.SetLabelsReply{}, err
 		}
 	}
 	if klog.V(1).Enabled() {