Skip to content

Add an agent_id_parent selector to x509pop in spiffe mode #7002

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kfox1111 wants to merge 6 commits into
base: main
Choose a base branch
from
+40 −17
Open

Add an agent_id_parent selector to x509pop in spiffe mode #7002

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8fb6a21
Add an agent_id_parent selector to x509pop in spiffe mode
kfox1111 May 29, 2026
d2267cf
Update docs
kfox1111 May 29, 2026
6da3970
Fix review issues
kfox1111 May 29, 2026
ac412dd
Fix review issues
kfox1111 May 29, 2026
f235994
Fix docs
kfox1111 May 29, 2026
3d5166d
Merge branch 'main' into x509pop-parent
kfox1111 May 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions doc/plugin_server_nodeattestor_x509pop.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,12 @@ A sample configuration:
## Selectors

| Selector | Example | Description |
|------------------|-------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| Common Name | `x509pop:subject:cn:example.org` | The Subject's Common Name (see X.500 Distinguished Names) |
| SHA1 Fingerprint | `x509pop:ca:fingerprint:0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33` | The SHA1 fingerprint as a hex string for each cert in the PoP chain, excluding the leaf. |
| SerialNumber | `x509pop:serialnumber:0a1b2c3d4e5f` | The leaf certificate serial number as a lowercase hexadecimal string |
| San | `x509pop:san:<key>:<value>` | The san selectors on the leaf certificate. The expected format of the uri san is `x509pop://<trust_domain>/<key>/<value>`. One selector is exposed per uri san corresponding to x509pop uri scheme. string |
|------------------|-------------------------------------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| Common Name | `x509pop:subject:cn:example.org` | The Subject's Common Name (see X.500 Distinguished Names) |
| SHA1 Fingerprint | `x509pop:ca:fingerprint:0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33` | The SHA1 fingerprint as a hex string for each cert in the PoP chain, excluding the leaf. |
| SerialNumber | `x509pop:serialnumber:0a1b2c3d4e5f` | The leaf certificate serial number as a lowercase hexadecimal string |
| San | `x509pop:san:<key>:<value>` | The san selectors on the leaf certificate. The expected format of the uri san is `x509pop://<trust_domain>/<key>/<value>`. One selector is exposed per uri san corresponding to x509pop uri scheme. string |
Comment thread
kfox1111 marked this conversation as resolved.
Outdated
| agent_id_parent | `x509pop:agent_id_parent:<id>` | When in spiffe mode, the parent id of the chosen agent id. Ex `spiffe://spire/agent/x509pop/spire-identity-exchange/1234` gets `x509pop:agent_id_parent:spiffe://spire/agent/x509pop/spire-identity-exchange` |
Comment thread
kfox1111 marked this conversation as resolved.
Outdated

## SVID Path Prefix

Expand Down
14 changes: 13 additions & 1 deletion pkg/server/plugin/nodeattestor/x509pop/x509pop.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,11 +313,23 @@
return status.Errorf(codes.Internal, "failed to make spiffe id: %v", err)
}

selectors := buildSelectorValues(leaf, chains, sanSelectors)

if config.mode == "spiffe" {
agentIDStr := spiffeid.String()

if lastSlash := strings.LastIndex(agentIDStr, "/"); lastSlash != -1 {
parentID := agentIDStr[:lastSlash]
parentID = strings.TrimSuffix(parentID, "/")
selectors = append(selectors, "agent_id_parent:" + parentID)

Check failure on line 324 in pkg/server/plugin/nodeattestor/x509pop/x509pop.go

View workflow job for this annotation

GitHub Actions / lint (linux) 

File is not properly formatted (gofmt)

Check failure on line 324 in pkg/server/plugin/nodeattestor/x509pop/x509pop.go

View workflow job for this annotation

GitHub Actions / lint (windows) 

File is not properly formatted (gofmt)
}
Comment on lines +319 to +325

@sorindumitru sorindumitru Jun 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very single-use selector, for the case that you mentioned. We can publish the SPIFFE ID as the selector. For your particular case you could make the agent SPIFFE ID template be derived from the SHA1 fingerprint, to get unique SPIFFE IDs, and have the full SPIFFE ID be spiffe://trust domain/x509pop/identity-exchange, which you can then use as a node alias. You might have to make sure that no 2 agents get deployed to the same node so that they do not use the same exact SVID to attest themselves.

@kfox1111 kfox1111 Jun 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please give an example of what the selector for the node alias would actually be in this configuration? I dont understand how it is possible with the approach your suggesting.

the docs mention selectors:

Common Name	x509pop:subject:cn:example.org	The Subject's Common Name (see X.500 Distinguished Names)
SHA1 Fingerprint	x509pop:ca:fingerprint:0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33	The SHA1 fingerprint as a hex string for each cert in the PoP chain, excluding the leaf.
SerialNumber	x509pop:serialnumber:0a1b2c3d4e5f	The leaf certificate serial number as a lowercase hexadecimal string
San	x509pop:san:<key>:<value>	The san selectors on the leaf certificate. The expected format of the uri san is x509pop://<trust_domain>/<key>/<value>. One selector is exposed per uri san corresponding to x509pop uri scheme. string

the san selector wont work as a spiffe svid will never have a url with x509pop.

the serial number/sha1 fingerprint selectors are not useful as you will never know what it is to alias with. common name wont be defined with spiffe certs.
the experimental spiffe_id selector will always be the full spiffeid that is genreated from the template. if it has the fingerprint in it, its not useable as a generic point to alias over either.

So, there is a lack of a selector that fits the use case I think.

It is ok if it is narrowly targeted a selector. it only is needed for x509 pop with spiffe mode. There are lots of selectors that only are useful for certain use cases.

@sorindumitru sorindumitru Jun 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SPIFFE ID of the SVID the agent presents to spire-server to attest would be:

  • spiffe://trust domain/spire-identity-exchange (or something like that this). The same one for all agents.
  • The SPIFFE of the SVID resulting from the attestation with the above SVID would be spiffe://trust domain/spire/agent/x509pop/<cert fingerprint>, instead of the usual format for spiffe mode. You'd need to make sure that the agents get unique "spire-identity-exchange" SVIDs so that they get unique fingerprints.

We'd still need to add the id as a selector so you can use in the node aliase, but that feels like a reasonable thing to do.

It is ok if it is narrowly targeted a selector. it only is needed for x509 pop with spiffe mode.

It's not that it's only needed for spiffe mode, it's only needed for this particular deployment. Most deployments wouldn't care about the "parent id".

@kfox1111 kfox1111 Jun 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. I see.

I think that has two drawbacks.

  1. I would be worried about an agent who's identifier can change over time. When reattesting, it will get a new cert which will have a new fingerprint.
  2. You'd never know the fingerprint if you still did want to hang off svds off of the individual agents up front. Forcing it to be the fingerprint id would make it harder to predict. I was planning on using the {{ pod.Name }} in it for stable identifiers in case of a statefulset in k8s.

I agree though, either way, we need a new selector. We're just debating the name of it I guess. So we could pick a name potentially independent of the rest.

This does raise a discussion I wanted to have at some point about dynamic entries. This idea does kind of match an idea I've been rolling around in my head. It would be useful to simplify my own entry management, as well as potentially helping dynamic entries for spire-identity-exchange too.

In most of my usage, I've run across needing to tack on the node name over and over to identifiers to guarantee uniqueness of identity.

example.com/spire/agent/node123
example.com/sshd/node123
example.com/fluent-bit/node123
example.com/node-agent/node123
example.com/kubelet/node123

Times the number of nodes. It gets a bit painful after a while.

But, what if we could set some labels on the node:
example.com/spire/agent/node123 (label nodeName=node123)

we could put generic workload entries on it:
{example.com/node123, example.com/node234}-> example.com/k8s-node-group -> {example.com/sshd, example.com/fluent-bit, example.com/node-agent, example.com/kubelet}

Each one would use those spiffeid's for lookups and resolving workloads. but, during issuence of the svid, it would have an extra template that would resolve before issuing the final spiffeid. so it could say, append "/{{ nodeLabels.nodeName }}"

@sorindumitru sorindumitru Jun 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear to me if the solution above is ok for you, which would mean that we'd have just an agent_spiffe_id selector, based on the full SPIFFE ID of the presented SVID.

We can continue the other discussion independently of this.

@kfox1111 kfox1111 Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If your asking if the proposed solution of just making the underlying spiffe id be the common group part, and then making them unique by adding the SHA1 fingerprint automatically to it for this special case, then no, I dont think that is a good way to solve it. It makes it too hard to:

  • hang off individual spiffeids off the individual agents when needed
  • It can make an agent get a completely different agent id when it gets a new svid from the underlying system and its sha changes. I dont think spire-agent will handle this right, along with the registration entries now pointing at the wrong agent when individual ones are registered.

@sorindumitru sorindumitru Jun 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, your proposed solution here also has a random, changing, identifier in the agent identity, the pod UUID. So that's something you have to deal with anyway. I also don't think it's that big of a deal since you're planning on using the node alias.

I'm not sure it's worth adding this selector which is very specific to your deployment and is likely hard to reason about in a large deployment. The full spiffe id would be ok to have.

@kfox1111 kfox1111 Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You keep falling back to very specific cases, designing a feature that is so specific, when the next case comes out, we're back to square one. IMO we should design features that are useful for more then just one use case so we're not constantly adding features.

The way I'm using it in this particular case, yes a pod uid is somewhat unknowable up front. But, it will never change during the life of the pod. So adding it later is stable. Reboot the node unexpectedly, the pod uid stays the same. The spire-agent gets reattested, it still gets the same spiffeid while the fingerprint changes. If you really want to put a stable identity on it so you can hang off per agent entries, you use the {.Metadata.Name}} from the pod and a statefulset to give it a persistent name. Persistent identity. done. I'm not 100% sure I will never need to do that. So, why preoptimize the ability to do that completely away? Besides, we still havent addressed how your proposal will work when the fingerprint changes. That is deal breaking on its own IMO. So, I'm still at the point of, that isnt a good solution.

I"m not asking for a ton of code here. We're talking for like 12 lines of code and a unit test. This really shoudlnt be burdensome on the spire team. That saves 57 lines of code https://github.com/spiffe/spire-identity-exchange/blob/main/cmd/spire-credentialcomposer-identity-exchange/main.go
and a whole separate plugin and infrastructure, and is much easier for the end user to deal with. This is a fair optimization I think. Over all the spire projects, spire is maintains less code.

}
Comment thread
kfox1111 marked this conversation as resolved.

return stream.Send(&nodeattestorv1.AttestResponse{
Response: &nodeattestorv1.AttestResponse_AgentAttributes{
AgentAttributes: &nodeattestorv1.AgentAttributes{
SpiffeId: spiffeid.String(),
SelectorValues: buildSelectorValues(leaf, chains, sanSelectors),
SelectorValues: selectors,
CanReattest: true,
},
},
Expand Down
8 changes: 8 additions & 0 deletions pkg/server/plugin/nodeattestor/x509pop/x509pop_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,14 @@ func (s *Suite) TestAttestSuccess() {
{Type: "x509pop", Value: "san:environment:production"},
{Type: "x509pop", Value: "san:key:path/to/value"},
}

if tt.desc == "success with spiffe exchange" {
expectedSelectors = append(expectedSelectors, &common.Selector{
Type: "x509pop",
Value: "agent_id_parent:spiffe://example.org/spire/agent/x509pop",
})
}
Comment thread
kfox1111 marked this conversation as resolved.
Outdated

spirecommonutil.SortSelectors(expectedSelectors)
spirecommonutil.SortSelectors(result.Selectors)

Expand Down
Loading