Skip to content
/ spire Public
forked from spiffe/spire

Commit

Permalink
No longer emit x509UniqueIdentifier in X509-SVIDs
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
azdagron committed Feb 2, 2024
1 parent f86bc23 commit 975add9
Show file tree
Hide file tree
Showing 8 changed files with 240 additions and 41 deletions.
6 changes: 3 additions & 3 deletions conf/server/server.conf
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ server {
}

plugins {
CredentialComposer "uniqueid" {}

DataStore "sql" {
plugin_data {
database_type = "sqlite3"
Expand All @@ -20,9 +22,7 @@ plugins {
}
}

KeyManager "memory" {
plugin_data = {}
}
KeyManager "memory" {}

UpstreamAuthority "disk" {
plugin_data {
Expand Down
7 changes: 7 additions & 0 deletions doc/plugin_server_credentialcomposer_uniqueid.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# 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).
49 changes: 25 additions & 24 deletions doc/spire_server.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 2 additions & 1 deletion pkg/server/catalog/credentialcomposer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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{}
Expand Down
4 changes: 0 additions & 4 deletions pkg/server/credtemplate/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand Down
9 changes: 0 additions & 9 deletions pkg/server/credtemplate/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"github.com/spiffe/go-spiffe/v2/spiffeid"
credentialcomposerv1 "github.com/spiffe/spire-plugin-sdk/proto/spire/plugin/server/credentialcomposer/v1"
"github.com/spiffe/spire/pkg/common/catalog"
"github.com/spiffe/spire/pkg/common/x509svid"
"github.com/spiffe/spire/pkg/common/x509util"
"github.com/spiffe/spire/pkg/server/credtemplate"
"github.com/spiffe/spire/pkg/server/plugin/credentialcomposer"
Expand Down Expand Up @@ -565,7 +564,6 @@ func TestBuildServerX509SVIDTemplate(t *testing.T) {
overrideExpected: func(expected *x509.Certificate) {
expected.Subject = pkix.Name{
CommonName: "OVERRIDE",
ExtraNames: []pkix.AttributeTypeAndValue{x509svid.UniqueIDAttribute(serverID)},
}
},
},
Expand Down Expand Up @@ -626,7 +624,6 @@ func TestBuildServerX509SVIDTemplate(t *testing.T) {
Subject: pkix.Name{
Country: []string{"US"},
Organization: []string{"SPIRE"},
ExtraNames: []pkix.AttributeTypeAndValue{x509svid.UniqueIDAttribute(serverID)},
},
SubjectKeyId: publicKeyID,
AuthorityKeyId: parentKeyID,
Expand Down Expand Up @@ -730,7 +727,6 @@ func TestBuildAgentX509SVIDTemplate(t *testing.T) {
overrideExpected: func(expected *x509.Certificate) {
expected.Subject = pkix.Name{
CommonName: "OVERRIDE",
ExtraNames: []pkix.AttributeTypeAndValue{x509svid.UniqueIDAttribute(agentID)},
}
},
},
Expand Down Expand Up @@ -792,7 +788,6 @@ func TestBuildAgentX509SVIDTemplate(t *testing.T) {
Subject: pkix.Name{
Country: []string{"US"},
Organization: []string{"SPIRE"},
ExtraNames: []pkix.AttributeTypeAndValue{x509svid.UniqueIDAttribute(agentID)},
},
SubjectKeyId: publicKeyID,
AuthorityKeyId: parentKeyID,
Expand Down Expand Up @@ -893,7 +888,6 @@ func TestBuildWorkloadX509SVIDTemplate(t *testing.T) {
overrideExpected: func(expected *x509.Certificate) {
expected.Subject = pkix.Name{
CommonName: "OVERRIDE",
ExtraNames: []pkix.AttributeTypeAndValue{x509svid.UniqueIDAttribute(workloadID)},
}
},
},
Expand All @@ -920,7 +914,6 @@ func TestBuildWorkloadX509SVIDTemplate(t *testing.T) {
// Subject is explicit.
expected.Subject = pkix.Name{
CommonName: "DNSNAME1",
ExtraNames: []pkix.AttributeTypeAndValue{x509svid.UniqueIDAttribute(workloadID)},
}
},
},
Expand All @@ -940,7 +933,6 @@ func TestBuildWorkloadX509SVIDTemplate(t *testing.T) {
// allowed to override.
expected.Subject = pkix.Name{
CommonName: "OVERRIDE-1",
ExtraNames: []pkix.AttributeTypeAndValue{x509svid.UniqueIDAttribute(workloadID)},
}
},
},
Expand Down Expand Up @@ -1021,7 +1013,6 @@ func TestBuildWorkloadX509SVIDTemplate(t *testing.T) {
Subject: pkix.Name{
Country: []string{"US"},
Organization: []string{"SPIRE"},
ExtraNames: []pkix.AttributeTypeAndValue{x509svid.UniqueIDAttribute(workloadID)},
},
SubjectKeyId: publicKeyID,
AuthorityKeyId: parentKeyID,
Expand Down
108 changes: 108 additions & 0 deletions pkg/server/plugin/credentialcomposer/uniqueid/plugin.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
package uniqueid

import (
"context"

"github.com/spiffe/go-spiffe/v2/spiffeid"
credentialcomposerv1 "github.com/spiffe/spire-plugin-sdk/proto/spire/plugin/server/credentialcomposer/v1"
"github.com/spiffe/spire/pkg/common/catalog"
"github.com/spiffe/spire/pkg/common/x509svid"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)

func BuiltIn() catalog.BuiltIn {
return builtIn(New())
}

func builtIn(p *Plugin) catalog.BuiltIn {
return catalog.MakeBuiltIn("uniqueid",
credentialcomposerv1.CredentialComposerPluginServer(p),
)
}

type Plugin struct {
credentialcomposerv1.UnsafeCredentialComposerServer
}

func New() *Plugin {
return &Plugin{}
}

func (p *Plugin) ComposeServerX509CA(context.Context, *credentialcomposerv1.ComposeServerX509CARequest) (*credentialcomposerv1.ComposeServerX509CAResponse, error) {
// Intentionally not implemented.
return nil, status.Error(codes.Unimplemented, "not implemented")
}

func (p *Plugin) ComposeServerX509SVID(context.Context, *credentialcomposerv1.ComposeServerX509SVIDRequest) (*credentialcomposerv1.ComposeServerX509SVIDResponse, error) {
// Intentionally not implemented.
return nil, status.Error(codes.Unimplemented, "not implemented")
}

func (p *Plugin) ComposeAgentX509SVID(context.Context, *credentialcomposerv1.ComposeAgentX509SVIDRequest) (*credentialcomposerv1.ComposeAgentX509SVIDResponse, error) {
// Intentionally not implemented.
return nil, status.Error(codes.Unimplemented, "not implemented")
}

func (p *Plugin) ComposeWorkloadX509SVID(_ context.Context, req *credentialcomposerv1.ComposeWorkloadX509SVIDRequest) (*credentialcomposerv1.ComposeWorkloadX509SVIDResponse, error) {
switch {
case req.Attributes == nil:
return nil, status.Error(codes.InvalidArgument, "request missing attributes")
case req.SpiffeId == "":
return nil, status.Error(codes.InvalidArgument, "request missing SPIFFE ID")
}

uniqueID, err := uniqueIDAttributeTypeAndValue(req.SpiffeId)
if err != nil {
return nil, err
}

// No need to clone
attributes := req.Attributes
if attributes.Subject == nil {
attributes.Subject = &credentialcomposerv1.DistinguishedName{}
}

// Add the attribute if does not already exist. Otherwise replace the old value.
found := false
for i := 0; i < len(attributes.Subject.ExtraNames); i++ {
if attributes.Subject.ExtraNames[i].Oid == uniqueID.Oid {
attributes.Subject.ExtraNames[i] = uniqueID
found = true
break
}
}
if !found {
attributes.Subject.ExtraNames = append(attributes.Subject.ExtraNames, uniqueID)
}

return &credentialcomposerv1.ComposeWorkloadX509SVIDResponse{
Attributes: attributes,
}, nil
}

func (p *Plugin) ComposeWorkloadJWTSVID(context.Context, *credentialcomposerv1.ComposeWorkloadJWTSVIDRequest) (*credentialcomposerv1.ComposeWorkloadJWTSVIDResponse, error) {
// Intentionally not implemented.
return nil, status.Error(codes.Unimplemented, "not implemented")
}

func uniqueIDAttributeTypeAndValue(id string) (*credentialcomposerv1.AttributeTypeAndValue, error) {
spiffeID, err := spiffeid.FromString(id)
if err != nil {
return nil, status.Errorf(codes.InvalidArgument, "malformed SPIFFE ID: %v", err)
}

uniqueID := x509svid.UniqueIDAttribute(spiffeID)

oid := uniqueID.Type.String()
stringValue, ok := uniqueID.Value.(string)
if !ok {
// purely defensive.
return nil, status.Errorf(codes.Internal, "unique ID value is not a string")
}

return &credentialcomposerv1.AttributeTypeAndValue{
Oid: oid,
StringValue: stringValue,
}, nil
}
Loading

0 comments on commit 975add9

Please sign in to comment.