Add option for DNS SAN based on SA name

deploy/charts/csi-driver-spiffe/README.md

@@ -94,6 +94,13 @@ Verbosity of cert-manager-csi-driver logging.
 > ```
 
 Duration requested for requested certificates.
+#### **app.includeDnsSan** ~ `string`
+> Default value:
+> ```yaml
+> "false"
+> ```
+
+Include service account name as DNS SAN
 #### **app.runtimeIssuanceConfigMap** ~ `string`
 > Default value:
 > ```yaml

deploy/charts/csi-driver-spiffe/templates/daemonset.yaml

@@ -70,6 +70,7 @@ spec:
             - --csi-driver-name={{ .Values.app.name }}
 
             - --certificate-request-duration={{ .Values.app.certificateRequestDuration }}
+            - --include-dns-san={{ .Values.app.includeDnsSan }}
             - --issuer-name={{ .Values.app.issuer.name }}
             - --issuer-kind={{ .Values.app.issuer.kind }}
             - --issuer-group={{ .Values.app.issuer.group }}

deploy/charts/csi-driver-spiffe/templates/deployment.yaml

@@ -34,6 +34,7 @@ spec:
           - --csi-driver-name={{ .Values.app.name }}
 
           - --certificate-request-duration={{ .Values.app.certificateRequestDuration }}
+          - --include-dns-san={{ .Values.app.includeDnsSan }}
           - --trust-domain={{ .Values.app.trustDomain }}
 
           - --leader-election-namespace=$(POD_NAMESPACE)

deploy/charts/csi-driver-spiffe/values.schema.json

@@ -59,6 +59,9 @@
         "extraCertificateRequestAnnotations": {
           "$ref": "#/$defs/helm-values.app.extraCertificateRequestAnnotations"
         },
+        "includeDnsSan": {
+          "$ref": "#/$defs/helm-values.app.includeDnsSan"
+        },
         "issuer": {
           "$ref": "#/$defs/helm-values.app.issuer"
         },
@@ -413,6 +416,11 @@
     "helm-values.app.extraCertificateRequestAnnotations": {
       "description": "List of annotations to add to certificate requests\n\nFor example:\nextraCertificateRequestAnnotations: app=csi-driver-spiffe,foo=bar"
     },
+    "helm-values.app.includeDnsSan": {
+      "default": "false",
+      "description": "Include service account name as DNS SAN",
+      "type": "string"
+    },
     "helm-values.app.issuer": {
       "additionalProperties": false,
       "properties": {

deploy/charts/csi-driver-spiffe/values.yaml

@@ -48,6 +48,8 @@ app:
   logLevel: 1 # 1-5
   # Duration requested for requested certificates.
   certificateRequestDuration: 1h
+  # Include service account name as DNS SAN
+  includeDnsSan: "false"
 
   # Name of a ConfigMap in the installation namespace to watch, providing
   # runtime configuration of an issuer to use.

internal/approver/app/app.go

@@ -83,6 +83,7 @@ func NewCommand(ctx context.Context) *cobra.Command {
 			evaluator := evaluator.New(evaluator.Options{
 				TrustDomain:                opts.CertManager.TrustDomain,
 				CertificateRequestDuration: opts.CertManager.CertificateRequestDuration,
+				IncludeDnsSan:              opts.CertManager.IncludeDnsSan,
 			})
 
 			if err := controller.AddApprover(ctx, opts.Logr, controller.Options{

internal/approver/app/options/options.go

@@ -62,6 +62,9 @@ type OptionsCertManager struct {
 	// CertificateRequestDuration is the duration the evaluator will enforce
 	// CertificateRequest request for.
 	CertificateRequestDuration time.Duration
+
+	// IncludeDnsSan is set to true to indicate that the service account name should be included as a DNS SAN
+	IncludeDnsSan string
 }
 
 func New() *Options {
@@ -79,6 +82,8 @@ func (o *Options) addCertManagerFlags(fs *pflag.FlagSet) {
 	fs.DurationVar(&o.CertManager.CertificateRequestDuration, "certificate-request-duration", time.Hour,
 		"The duration which is enforced for requests to have.")
 
+	fs.StringVar(&o.CertManager.IncludeDnsSan, "include-dns-san", "false", "include the service account name as a DNS SAN")
+
 	// allow issuer-* args to still be passed to avoid a backwards incompatible change
 	var dummyIssuerRefName, dummyIssuerRefKind, dummyIssuerRefGroup string
 

internal/approver/evaluator/evaluator.go

@@ -49,6 +49,10 @@ type Options struct {
 	// CertificateRequestDuration is the duration that users _must_ request for,
 	// else the request will be denied.
 	CertificateRequestDuration time.Duration
+
+	// IncludeDnsSan is a flag that when set to true will
+	// include and allow a DNSSan set to the service account name in the CSR
+	IncludeDnsSan string
 }
 
 // internal is the internal implementation of the evaluator that should be used
@@ -61,13 +65,18 @@ type internal struct {
 	// certificateRequestDuration is the duration that users _must_ request for,
 	// else the request will be denied.
 	certificateRequestDuration time.Duration
+
+	// IncludeDnsSan is a flag that when set to true will
+	// include and allow a DNSSan set to the service account name in the CSR
+	includeDnsSan string
 }
 
 // New constructs a new evaluator.
 func New(opts Options) Interface {
 	return &internal{
 		trustDomain:                opts.TrustDomain,
 		certificateRequestDuration: opts.CertificateRequestDuration,
+		includeDnsSan:              opts.IncludeDnsSan,
 	}
 }
 
@@ -90,10 +99,13 @@ func (i *internal) Evaluate(req *cmapi.CertificateRequest) error {
 	}
 
 	// if the csr contains any other options set, error
-	if len(csr.DNSNames) > 0 || len(csr.IPAddresses) > 0 ||
-		len(csr.Subject.CommonName) > 0 || len(csr.EmailAddresses) > 0 {
-		return fmt.Errorf("forbidden extensions, DNS=%q IPs=%q CommonName=%q Emails=%q",
-			csr.DNSNames, csr.IPAddresses, csr.Subject.CommonName, csr.EmailAddresses)
+	if len(csr.IPAddresses) > 0 || len(csr.Subject.CommonName) > 0 || len(csr.EmailAddresses) > 0 {
+		return fmt.Errorf("forbidden extensions, IPs=%q CommonName=%q Emails=%q",
+			csr.IPAddresses, csr.Subject.CommonName, csr.EmailAddresses)
+	}
+
+	if i.includeDnsSan != "true" && len(csr.DNSNames) > 0 {
+		return fmt.Errorf("forbidden extension, DNSs=%q and IncludeDnsSan is %q", csr.DNSNames, i.includeDnsSan)
 	}
 
 	if err := validateCSRExtentions(csr); err != nil {

internal/approver/evaluator/evaluator_test.go

@@ -34,8 +34,9 @@ func Test_Evaluate(t *testing.T) {
 	assert.NoError(t, err)
 
 	tests := map[string]struct {
-		req    func(t *testing.T) *cmapi.CertificateRequest
-		expErr bool
+		req           func(t *testing.T) *cmapi.CertificateRequest
+		expErr        bool
+		includeDnsSan bool
 	}{
 		"if request contains a badly encoded PEM, expect error": {
 			req: func(t *testing.T) *cmapi.CertificateRequest {
@@ -72,13 +73,40 @@ func Test_Evaluate(t *testing.T) {
 			},
 			expErr: true,
 		},
+		"if request contains DNS names and includeDnsSan is true, do not expect error": {
+			req: func(t *testing.T) *cmapi.CertificateRequest {
+				csr, err := utilpki.GenerateCSR(&cmapi.Certificate{
+					Spec: cmapi.CertificateSpec{
+						PrivateKey: &cmapi.CertificatePrivateKey{Algorithm: cmapi.ECDSAKeyAlgorithm},
+						URIs:       []string{"spiffe://foo.bar/ns/sandbox/sa/sleep"},
+						DNSNames:   []string{"sleep"},
+					},
+				})
+				assert.NoError(t, err)
+				csrDER, err := utilpki.EncodeCSR(csr, pk)
+				assert.NoError(t, err)
+				csrPEM := bytes.NewBuffer([]byte{})
+				assert.NoError(t, pem.Encode(csrPEM, &pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csrDER}))
+				return &cmapi.CertificateRequest{Spec: cmapi.CertificateRequestSpec{
+					Request:  csrPEM.Bytes(),
+					Duration: &metav1.Duration{Duration: time.Hour},
+					Username: "system:serviceaccount:sandbox:sleep",
+					Usages: []cmapi.KeyUsage{
+						cmapi.UsageServerAuth, cmapi.UsageClientAuth,
+						cmapi.UsageDigitalSignature, cmapi.UsageKeyEncipherment,
+					},
+				}}
+			},
+			expErr:        false,
+			includeDnsSan: true,
+		},
 		"if request contains DNS names, expect error": {
 			req: func(t *testing.T) *cmapi.CertificateRequest {
 				csr, err := utilpki.GenerateCSR(&cmapi.Certificate{
 					Spec: cmapi.CertificateSpec{
 						PrivateKey: &cmapi.CertificatePrivateKey{Algorithm: cmapi.ECDSAKeyAlgorithm},
 						URIs:       []string{"spiffe://foo.bar/ns/sandbox/sa/sleep"},
-						DNSNames:   []string{"example.com"},
+						DNSNames:   []string{"sleep"},
 					},
 				})
 				assert.NoError(t, err)
@@ -313,6 +341,10 @@ func Test_Evaluate(t *testing.T) {
 			i := &internal{
 				trustDomain:                "foo.bar",
 				certificateRequestDuration: time.Hour,
+				includeDnsSan:              "false",
+			}
+			if test.includeDnsSan {
+				i.includeDnsSan = "true"
 			}
 
 			err := i.Evaluate(test.req(t))

internal/approver/evaluator/extensions.go

@@ -38,6 +38,7 @@ const (
 	//      uniformResourceIdentifier       [6]     IA5String,
 	// }
 	asn1TagURI = 6
+	asn1TagDNS = 2
 )
 
 var (
@@ -162,9 +163,9 @@ func validateSubjectAltNameExtension(ext pkix.Extension) error {
 			return err
 		}
 
-		// Only URI SANs are permitted for SPIFFE certificates
-		if rawValue.Tag != asn1TagURI {
-			return fmt.Errorf("non uri san extension given: %s", rawValue.Bytes)
+		// Allow URI and DNS SANs for SPIFFE certificates
+		if !((rawValue.Tag == asn1TagURI) || (rawValue.Tag == asn1TagDNS)) {
+			return fmt.Errorf("non uri or dns san extension given: %s", rawValue.Bytes)
 		}
 	}
 

internal/approver/evaluator/extensions_test.go

@@ -101,7 +101,7 @@ func Test_validateCSRExtentions(t *testing.T) {
 			},
 			expErr: false,
 		},
-		"if multiple URI names exist, dns name, and allowed usages, should error": {
+		"if multiple URI names exist, dns name, and allowed usages, shouldn't error": {
 			uris: []string{"spiffe://foo.bar", "spiffe://bar.foo"},
 			dns:  []string{"foo.bar"},
 			usages: []cmapi.KeyUsage{
@@ -110,7 +110,7 @@ func Test_validateCSRExtentions(t *testing.T) {
 				cmapi.UsageClientAuth,
 				cmapi.UsageServerAuth,
 			},
-			expErr: true,
+			expErr: false,
 		},
 		"if multiple URI names exist, ips, and allowed usages, should error": {
 			uris: []string{"spiffe://foo.bar", "spiffe://bar.foo"},

internal/approver/evaluator/identity.go

@@ -46,5 +46,17 @@ func (i *internal) validateIdentity(csr *x509.CertificateRequest, username strin
 		return fmt.Errorf("unexpected SPIFFE ID requested, exp=%q got=%q", expSpiffeID, csr.URIs[0].String())
 	}
 
+	// We allow one DNS SAN equal to the service account name
+	if len(csr.DNSNames) > 1 {
+		return fmt.Errorf("expected exactly 0 or 1 DNS SAN present on request, got=%d", len(csr.DNSNames))
+	}
+
+	if len(csr.DNSNames) == 1 {
+		expDNSName := split[3]
+		if csr.DNSNames[0] != expDNSName {
+			return fmt.Errorf("unexpected DNS SAN requested, exp=%q got=%q", expDNSName, csr.DNSNames[0])
+		}
+	}
+
 	return nil
 }

internal/csi/app/app.go

@@ -72,6 +72,7 @@ func NewCommand(ctx context.Context) *cobra.Command {
 				CertificateRequestAnnotations: opts.CertManager.CertificateRequestAnnotations,
 				CertificateRequestDuration:    opts.CertManager.CertificateRequestDuration,
 				IssuerRef:                     &opts.CertManager.IssuerRef,
+				IncludeDnsSan:                 opts.CertManager.IncludeDnsSan,
 
 				IssuanceConfigMapName:      opts.CertManager.IssuanceConfigMapName,
 				IssuanceConfigMapNamespace: opts.CertManager.IssuanceConfigMapNamespace,

internal/csi/app/options/options.go

@@ -73,6 +73,9 @@ type OptionsCertManager struct {
 	// requested with.
 	CertificateRequestDuration time.Duration
 
+	// IncludeDnsSan is set to true to indicate that the service account name should be included as a DNS SAN
+	IncludeDnsSan string
+
 	// IssuerRef is the IssuerRef used when creating CertificateRequests.
 	IssuerRef cmmeta.ObjectReference
 }
@@ -127,6 +130,7 @@ func (o *Options) addCertManagerFlags(fs *pflag.FlagSet) {
 		"The trust domain that will be requested for on created CertificateRequests.")
 	fs.DurationVar(&o.CertManager.CertificateRequestDuration, "certificate-request-duration", time.Hour,
 		"The duration that created CertificateRequests will use.")
+	fs.StringVar(&o.CertManager.IncludeDnsSan, "include-dns-san", "false", "include the service account name as a DNS SAN")
 
 	fs.StringToStringVar(&o.CertManager.CertificateRequestAnnotations, "extra-certificate-request-annotations", map[string]string{},
 		"Comma-separated list of extra annotations to add to certificate requests e.g '--extra-certificate-request-annotations=hello=world,test=annotation'")

internal/csi/driver/driver.go

@@ -76,6 +76,10 @@ type Options struct {
 	// Defaults to 1 hour if empty.
 	CertificateRequestDuration time.Duration
 
+	// IncludeDnsSan is set to true to indicate that the service account name should be included as a DNS SAN
+	// Defaults to false
+	IncludeDnsSan string
+
 	// IssuerRef is the IssuerRef used when creating CertificateRequests.
 	IssuerRef *cmmeta.ObjectReference
 
@@ -127,6 +131,10 @@ type Driver struct {
 	// created CertificateRequests.
 	certificateRequestDuration time.Duration
 
+	// IncludeDnsSan is set to true to indicate that the service account name should be included as a DNS SAN in
+	// created certificate requests
+	includeDnsSan string
+
 	// activeIssuerRef is the issuerRef that will be set on all created CertificateRequests.
 	// Can be changed at runtime via runtime configuration (i.e. reading from a ConfigMap)
 	// Not to be confused with originalIssuerRef, which is an issuerRef optionally passed in
@@ -209,6 +217,8 @@ func New(log logr.Logger, opts Options) (*Driver, error) {
 
 		issuanceConfigMapName:      opts.IssuanceConfigMapName,
 		issuanceConfigMapNamespace: opts.IssuanceConfigMapNamespace,
+
+		includeDnsSan: opts.IncludeDnsSan,
 	}
 
 	if d.originalIssuerRef != nil {
@@ -231,6 +241,10 @@ func New(log logr.Logger, opts Options) (*Driver, error) {
 		d.certificateRequestDuration = time.Hour
 	}
 
+	if d.includeDnsSan == "" {
+		d.includeDnsSan = "false"
+	}
+
 	store, err := storage.NewFilesystem(d.log, opts.DataRoot)
 	if err != nil {
 		return nil, fmt.Errorf("failed to setup filesystem: %w", err)
@@ -509,6 +523,9 @@ func (d *Driver) generateRequest(meta metadata.Metadata) (*manager.CertificateRe
 		return nil, fmt.Errorf("internal error crafting X.509 URI, this is a bug, please report on GitHub: %w", err)
 	}
 
+	// Request dnsSAN name equivalent to service account name
+	dnsSAN := saName
+
 	crAnnotations := map[string]string{
 		annotations.SPIFFEIdentityAnnnotationKey: spiffeID,
 	}
@@ -517,10 +534,16 @@ func (d *Driver) generateRequest(meta metadata.Metadata) (*manager.CertificateRe
 		crAnnotations[key] = value
 	}
 
+	req := &x509.CertificateRequest{
+		URIs: []*url.URL{uri},
+	}
+
+	if d.includeDnsSan == "true" {
+		req.DNSNames = []string{dnsSAN}
+	}
+
 	return &manager.CertificateRequestBundle{
-		Request: &x509.CertificateRequest{
-			URIs: []*url.URL{uri},
-		},
+		Request:   req,
 		IsCA:      false,
 		Namespace: saNamespace,
 		Duration:  d.certificateRequestDuration,