No longer emit x509UniqueIdentifier in X509-SVIDs (spiffe#4862)

* No longer emit x509UniqueIdentifier in X509-SVIDs Introduced in 1.4.2, this practice has turned out to be problematic. This change updates SPIRE Server to no long emit attribute in the X509-SVID subject. It also introduces a new built-in CredentialComposer to add the attribute back in for deployments that rely on it. The plugin only augments workload X509-SVIDs. Server and agent X509-SVIDs are not modified. Fixes: spiffe#4755 Fixes: spiffe#3110 Signed-off-by: Andrew Harding <azdagron@gmail.com>
rushi47 · Apr 11, 2024 · 4b8a692 · 4b8a692
1 parent 03b41ab
commit 4b8a692
Show file tree

Hide file tree

Showing 10 changed files with 264 additions and 55 deletions.
diff --git a/conf/server/server_full.conf b/conf/server/server_full.conf
@@ -219,7 +219,11 @@ server {
 #         enabled = [true | false]
 #     }
 plugins {
-    # DataStore "sql": An sql database storage for SQLite, PostgreSQL and MySQL
+    # CredentialComposer "uniqueid": Adds an x509UniqueIdentifier name, derived
+    # from the SPIFFE ID, to the subject of workload X509-SVIDs.
+    # CredentialComposer "uniqueid" {}
+
+    # DataStore "sql": SQL database storage for SQLite, PostgreSQL and MySQL
     # databases for the SPIRE datastore.
     DataStore "sql" {
         plugin_data {

diff --git a/doc/plugin_server_credentialcomposer_uniqueid.md b/doc/plugin_server_credentialcomposer_uniqueid.md
@@ -0,0 +1,17 @@
+# Server plugin: CredentialComposer "uniqueid"
+
+The `uniqueid` plugin adds the `x509UniqueIdentifier` attribute to the X509-SVID subject for workloads. Server and agent X509-SVIDs are not modified.
+
+The x509UniqueIdentifier is formed from a hash of the SPIFFE ID of the workload.
+
+This plugin is intended for backwards compatibility for deployments that have come to rely on this attribute (introduced in SPIRE 1.4.2 and reverted in SPIRE 1.9.0).
+
+This plugin has no configuration. To use the plugin, add it to the plugins section of the SPIRE Server configuration:
+
+```hcl
+plugins {
+    CredentialComposer "uniqueid" {}
+
+    // ... other plugins ...
+}
+```
diff --git a/doc/spire_server.md b/doc/spire_server.md
@@ -14,30 +14,31 @@ This document is a configuration reference for SPIRE Server. It includes informa
 
 ## Built-in plugins
 
-| Type              | Name                                                                 | Description                                                                                                                 |
-|-------------------|----------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------|
-| DataStore         | [sql](/doc/plugin_server_datastore_sql.md)                           | An sql database storage for SQLite, PostgreSQL and MySQL databases for the SPIRE datastore                                  |
-| KeyManager        | [aws_kms](/doc/plugin_server_keymanager_aws_kms.md)                  | A key manager which manages keys in AWS KMS                                                                                 |
-| KeyManager        | [disk](/doc/plugin_server_keymanager_disk.md)                        | A key manager which manages keys persisted on disk                                                                          |
-| KeyManager        | [memory](/doc/plugin_server_keymanager_memory.md)                    | A key manager which manages unpersisted keys in memory                                                                      |
-| NodeAttestor      | [aws_iid](/doc/plugin_server_nodeattestor_aws_iid.md)                | A node attestor which attests agent identity using an AWS Instance Identity Document                                        |
-| NodeAttestor      | [azure_msi](/doc/plugin_server_nodeattestor_azure_msi.md)            | A node attestor which attests agent identity using an Azure MSI token                                                       |
-| NodeAttestor      | [gcp_iit](/doc/plugin_server_nodeattestor_gcp_iit.md)                | A node attestor which attests agent identity using a GCP Instance Identity Token                                            |
-| NodeAttestor      | [join_token](/doc/plugin_server_nodeattestor_jointoken.md)           | A node attestor which validates agents attesting with server-generated join tokens                                          |
-| NodeAttestor      | [k8s_sat](/doc/plugin_server_nodeattestor_k8s_sat.md) (deprecated)   | A node attestor which attests agent identity using a Kubernetes Service Account token                                       |
-| NodeAttestor      | [k8s_psat](/doc/plugin_server_nodeattestor_k8s_psat.md)              | A node attestor which attests agent identity using a Kubernetes Projected Service Account token                             |
-| NodeAttestor      | [sshpop](/doc/plugin_server_nodeattestor_sshpop.md)                  | A node attestor which attests agent identity using an existing ssh certificate                                              |
-| NodeAttestor      | [tpm_devid](/doc/plugin_server_nodeattestor_tpm_devid.md)            | A node attestor which attests agent identity using a TPM that has been provisioned with a DevID certificate                 |
-| NodeAttestor      | [x509pop](/doc/plugin_server_nodeattestor_x509pop.md)                | A node attestor which attests agent identity using an existing X.509 certificate                                            |
-| Notifier          | [gcs_bundle](/doc/plugin_server_notifier_gcs_bundle.md)              | A notifier that pushes the latest trust bundle contents into an object in Google Cloud Storage.                             |
-| Notifier          | [k8sbundle](/doc/plugin_server_notifier_k8sbundle.md)                | A notifier that pushes the latest trust bundle contents into a Kubernetes ConfigMap.                                        |
-| UpstreamAuthority | [disk](/doc/plugin_server_upstreamauthority_disk.md)                 | Uses a CA loaded from disk to sign SPIRE server intermediate certificates.                                                  |
-| UpstreamAuthority | [aws_pca](/doc/plugin_server_upstreamauthority_aws_pca.md)           | Uses a Private Certificate Authority from AWS Certificate Manager to sign SPIRE server intermediate certificates.           |
-| UpstreamAuthority | [awssecret](/doc/plugin_server_upstreamauthority_awssecret.md)       | Uses a CA loaded from AWS SecretsManager to sign SPIRE server intermediate certificates.                                    |
-| UpstreamAuthority | [gcp_cas](/doc/plugin_server_upstreamauthority_gcp_cas.md)           | Uses a Private Certificate Authority from GCP Certificate Authority Service to sign SPIRE Server intermediate certificates. |
-| UpstreamAuthority | [vault](/doc/plugin_server_upstreamauthority_vault.md)               | Uses a PKI Secret Engine from HashiCorp Vault to sign SPIRE server intermediate certificates.                               |
-| UpstreamAuthority | [spire](/doc/plugin_server_upstreamauthority_spire.md)               | Uses an upstream SPIRE server in the same trust domain to obtain intermediate signing certificates for SPIRE server.        |
-| UpstreamAuthority | [cert-manager](/doc/plugin_server_upstreamauthority_cert_manager.md) | Uses a referenced cert-manager Issuer to request intermediate signing certificates.                                         |
+| Type               | Name                                                                 | Description                                                                                                                 |
+|--------------------|----------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------|
+| CredentialComposer | [uniqueid](/doc/plugin_server_credentialcomposer_uniqueid.md)        | Adds the x509UniqueIdentifier attribute to workload X509-SVIDs.                                                             |
+| DataStore          | [sql](/doc/plugin_server_datastore_sql.md)                           | An sql database storage for SQLite, PostgreSQL and MySQL databases for the SPIRE datastore                                  |
+| KeyManager         | [aws_kms](/doc/plugin_server_keymanager_aws_kms.md)                  | A key manager which manages keys in AWS KMS                                                                                 |
+| KeyManager         | [disk](/doc/plugin_server_keymanager_disk.md)                        | A key manager which manages keys persisted on disk                                                                          |
+| KeyManager         | [memory](/doc/plugin_server_keymanager_memory.md)                    | A key manager which manages unpersisted keys in memory                                                                      |
+| NodeAttestor       | [aws_iid](/doc/plugin_server_nodeattestor_aws_iid.md)                | A node attestor which attests agent identity using an AWS Instance Identity Document                                        |
+| NodeAttestor       | [azure_msi](/doc/plugin_server_nodeattestor_azure_msi.md)            | A node attestor which attests agent identity using an Azure MSI token                                                       |
+| NodeAttestor       | [gcp_iit](/doc/plugin_server_nodeattestor_gcp_iit.md)                | A node attestor which attests agent identity using a GCP Instance Identity Token                                            |
+| NodeAttestor       | [join_token](/doc/plugin_server_nodeattestor_jointoken.md)           | A node attestor which validates agents attesting with server-generated join tokens                                          |
+| NodeAttestor       | [k8s_sat](/doc/plugin_server_nodeattestor_k8s_sat.md) (deprecated)   | A node attestor which attests agent identity using a Kubernetes Service Account token                                       |
+| NodeAttestor       | [k8s_psat](/doc/plugin_server_nodeattestor_k8s_psat.md)              | A node attestor which attests agent identity using a Kubernetes Projected Service Account token                             |
+| NodeAttestor       | [sshpop](/doc/plugin_server_nodeattestor_sshpop.md)                  | A node attestor which attests agent identity using an existing ssh certificate                                              |
+| NodeAttestor       | [tpm_devid](/doc/plugin_server_nodeattestor_tpm_devid.md)            | A node attestor which attests agent identity using a TPM that has been provisioned with a DevID certificate                 |
+| NodeAttestor       | [x509pop](/doc/plugin_server_nodeattestor_x509pop.md)                | A node attestor which attests agent identity using an existing X.509 certificate                                            |
+| Notifier           | [gcs_bundle](/doc/plugin_server_notifier_gcs_bundle.md)              | A notifier that pushes the latest trust bundle contents into an object in Google Cloud Storage.                             |
+| Notifier           | [k8sbundle](/doc/plugin_server_notifier_k8sbundle.md)                | A notifier that pushes the latest trust bundle contents into a Kubernetes ConfigMap.                                        |
+| UpstreamAuthority  | [disk](/doc/plugin_server_upstreamauthority_disk.md)                 | Uses a CA loaded from disk to sign SPIRE server intermediate certificates.                                                  |
+| UpstreamAuthority  | [aws_pca](/doc/plugin_server_upstreamauthority_aws_pca.md)           | Uses a Private Certificate Authority from AWS Certificate Manager to sign SPIRE server intermediate certificates.           |
+| UpstreamAuthority  | [awssecret](/doc/plugin_server_upstreamauthority_awssecret.md)       | Uses a CA loaded from AWS SecretsManager to sign SPIRE server intermediate certificates.                                    |
+| UpstreamAuthority  | [gcp_cas](/doc/plugin_server_upstreamauthority_gcp_cas.md)           | Uses a Private Certificate Authority from GCP Certificate Authority Service to sign SPIRE Server intermediate certificates. |
+| UpstreamAuthority  | [vault](/doc/plugin_server_upstreamauthority_vault.md)               | Uses a PKI Secret Engine from HashiCorp Vault to sign SPIRE server intermediate certificates.                               |
+| UpstreamAuthority  | [spire](/doc/plugin_server_upstreamauthority_spire.md)               | Uses an upstream SPIRE server in the same trust domain to obtain intermediate signing certificates for SPIRE server.        |
+| UpstreamAuthority  | [cert-manager](/doc/plugin_server_upstreamauthority_cert_manager.md) | Uses a referenced cert-manager Issuer to request intermediate signing certificates.                                         |
 
 ## Server configuration file
 

diff --git a/pkg/server/api/svid/v1/service_test.go b/pkg/server/api/svid/v1/service_test.go
@@ -20,7 +20,6 @@ import (
 	"github.com/spiffe/spire-api-sdk/proto/spire/api/types"
 	"github.com/spiffe/spire/pkg/common/idutil"
 	"github.com/spiffe/spire/pkg/common/telemetry"
-	"github.com/spiffe/spire/pkg/common/x509svid"
 	"github.com/spiffe/spire/pkg/common/x509util"
 	"github.com/spiffe/spire/pkg/server/api"
 	"github.com/spiffe/spire/pkg/server/api/middleware"
@@ -77,7 +76,7 @@ func TestServiceMintX509SVID(t *testing.T) {
 				URIs: []*url.URL{workloadID.URL()},
 			},
 			expiredAt: expiredAt,
-			subject:   "O=SPIRE,C=US,2.5.4.45=#13203835323763353230323837636461376436323561613834373664386538336561",
+			subject:   "O=SPIRE,C=US",
 			expectLogs: func(csr []byte) []spiretest.LogEntry {
 				return []spiretest.LogEntry{
 					{
@@ -103,7 +102,7 @@ func TestServiceMintX509SVID(t *testing.T) {
 				URIs: []*url.URL{workloadID.URL()},
 			},
 			expiredAt: customExpiresAt,
-			subject:   "O=SPIRE,C=US,2.5.4.45=#13203835323763353230323837636461376436323561613834373664386538336561",
+			subject:   "O=SPIRE,C=US",
 			ttl:       10 * time.Second,
 			expectLogs: func(csr []byte) []spiretest.LogEntry {
 				return []spiretest.LogEntry{
@@ -132,7 +131,7 @@ func TestServiceMintX509SVID(t *testing.T) {
 			},
 			dns:       []string{"dns1", "dns2"},
 			expiredAt: expiredAt,
-			subject:   "CN=dns1,O=SPIRE,C=US,2.5.4.45=#13203835323763353230323837636461376436323561613834373664386538336561",
+			subject:   "CN=dns1,O=SPIRE,C=US",
 			expectLogs: func(csr []byte) []spiretest.LogEntry {
 				return []spiretest.LogEntry{
 					{
@@ -162,7 +161,7 @@ func TestServiceMintX509SVID(t *testing.T) {
 				},
 			},
 			expiredAt: expiredAt,
-			subject:   "O=ORG,C=EN+C=US,2.5.4.45=#13203835323763353230323837636461376436323561613834373664386538336561",
+			subject:   "O=ORG,C=EN+C=US",
 			expectLogs: func(csr []byte) []spiretest.LogEntry {
 				return []spiretest.LogEntry{
 					{
@@ -194,7 +193,7 @@ func TestServiceMintX509SVID(t *testing.T) {
 			},
 			dns:       []string{"dns1", "dns2"},
 			expiredAt: expiredAt,
-			subject:   "CN=dns1,O=ORG,C=EN+C=US,2.5.4.45=#13203835323763353230323837636461376436323561613834373664386538336561",
+			subject:   "CN=dns1,O=ORG,C=EN+C=US",
 			expectLogs: func(csr []byte) []spiretest.LogEntry {
 				return []spiretest.LogEntry{
 					{
@@ -1792,9 +1791,6 @@ func TestServiceBatchNewX509SVID(t *testing.T) {
 				expectedSubject := &pkix.Name{
 					Organization: []string{"SPIRE"},
 					Country:      []string{"US"},
-					Names: []pkix.AttributeTypeAndValue{
-						x509svid.UniqueIDAttribute(entrySPIFFEID),
-					},
 				}
 				if len(entry.DnsNames) > 0 {
 					expectedSubject.CommonName = entry.DnsNames[0]

diff --git a/pkg/server/ca/ca_test.go b/pkg/server/ca/ca_test.go
@@ -123,7 +123,7 @@ func (s *CATestSuite) TestSignServerX509SVID() {
 	}
 
 	// Subject is calculated by SPIRE Server and should not be pulled from the CSR.
-	s.Equal("O=SPIRE,C=US,2.5.4.45=#13203237323538663032373464643334303764623137653930626334623961646632", svid.Subject.String())
+	s.Equal("O=SPIRE,C=US", svid.Subject.String())
 }
 
 func (s *CATestSuite) TestSignServerX509SVIDUsesDefaultTTLIfTTLUnspecified() {
@@ -193,7 +193,7 @@ func (s *CATestSuite) TestSignAgentX509SVID() {
 	}
 
 	// Subject is calculated by SPIRE Server and should not be pulled from the CSR.
-	s.Equal("O=SPIRE,C=US,2.5.4.45=#13203565613834343735306363306235393262363730383830636133376238343363", svid.Subject.String())
+	s.Equal("O=SPIRE,C=US", svid.Subject.String())
 }
 
 func (s *CATestSuite) TestSignAgentX509SVIDCannotSignTrustDomainID() {
@@ -277,7 +277,7 @@ func (s *CATestSuite) TestSignWorkloadX509SVID() {
 	}
 
 	// Subject is calculated by SPIRE Server and should not be pulled from the CSR.
-	s.Equal("O=SPIRE,C=US,2.5.4.45=#13203933323965323863393434383738376466306663623363363535363035653531", svid.Subject.String())
+	s.Equal("O=SPIRE,C=US", svid.Subject.String())
 }
 
 func (s *CATestSuite) TestSignWorkloadX509SVIDCannotSignTrustDomainID() {
@@ -347,20 +347,20 @@ func (s *CATestSuite) TestSignWorkloadX509SVIDWithSubject() {
 	}{
 		{
 			name:     "empty subject",
-			expected: "O=SPIRE,C=US,2.5.4.45=#13203933323965323863393434383738376466306663623363363535363035653531",
+			expected: "O=SPIRE,C=US",
 			subject:  pkix.Name{},
 		}, {
 			name:     "no subject but DNS",
 			dns:      dns,
-			expected: "CN=dns1,O=SPIRE,C=US,2.5.4.45=#13203933323965323863393434383738376466306663623363363535363035653531",
+			expected: "CN=dns1,O=SPIRE,C=US",
 		}, {
 			name:     "subject provided",
-			expected: "CN=Common Name,O=ORG,C=EN+C=US,2.5.4.45=#13203933323965323863393434383738376466306663623363363535363035653531",
+			expected: "CN=Common Name,O=ORG,C=EN+C=US",
 			subject:  subject,
 		}, {
 			name:     "subject and dns",
 			dns:      dns,
-			expected: "CN=dns1,O=ORG,C=EN+C=US,2.5.4.45=#13203933323965323863393434383738376466306663623363363535363035653531",
+			expected: "CN=dns1,O=ORG,C=EN+C=US",
 			subject:  subject,
 		},
 	}

diff --git a/pkg/server/catalog/credentialcomposer.go b/pkg/server/catalog/credentialcomposer.go
@@ -3,6 +3,7 @@ package catalog
 import (
 	"github.com/spiffe/spire/pkg/common/catalog"
 	"github.com/spiffe/spire/pkg/server/plugin/credentialcomposer"
+	"github.com/spiffe/spire/pkg/server/plugin/credentialcomposer/uniqueid"
 )
 
 type credentialComposerRepository struct {
@@ -22,7 +23,7 @@ func (repo *credentialComposerRepository) Versions() []catalog.Version {
 }
 
 func (repo *credentialComposerRepository) BuiltIns() []catalog.BuiltIn {
-	return nil
+	return []catalog.BuiltIn{uniqueid.BuiltIn()}
 }
 
 type credentialComposerV1 struct{}

diff --git a/pkg/server/credtemplate/builder.go b/pkg/server/credtemplate/builder.go
@@ -15,7 +15,6 @@ import (
 	"github.com/go-jose/go-jose/v3/jwt"
 	"github.com/spiffe/go-spiffe/v2/spiffeid"
 	"github.com/spiffe/spire/pkg/common/idutil"
-	"github.com/spiffe/spire/pkg/common/x509svid"
 	"github.com/spiffe/spire/pkg/common/x509util"
 	"github.com/spiffe/spire/pkg/server/api"
 	"github.com/spiffe/spire/pkg/server/plugin/credentialcomposer"
@@ -396,9 +395,6 @@ func (b *Builder) buildX509SVIDTemplate(spiffeID spiffeid.ID, publicKey crypto.P
 		x509.ExtKeyUsageClientAuth,
 	}
 
-	// Append the unique ID to the subject, unless disabled
-	tmpl.Subject.ExtraNames = append(tmpl.Subject.ExtraNames, x509svid.UniqueIDAttribute(spiffeID))
-
 	return tmpl, nil
 }