Skip to content
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.

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

Consider a new SVID field customization plugin #3253

Closed
azdagron opened this issue Jul 14, 2022 · 20 comments
Closed

Consider a new SVID field customization plugin #3253

azdagron opened this issue Jul 14, 2022 · 20 comments
Assignees
Labels
priority/backlog Issue is approved and in the backlog
Milestone

Comments

@azdagron
Copy link
Member

There have been some requests to allow X509-SVID or JWT-SVID field customization. One way in which this might be achieved is by creating a new plugin type that would get a chance to modify the SVID before it was minted. Other solutions could allow for targeted customization of individual fields (i.e. X509-SVID Subject), but would not easily accommodate a wide variety of fields that might need customization.

This issue tracks the discussion of whether or not a plugin of this type would be ultimately beneficial and/or to weigh alternatives.

@azdagron
Copy link
Member Author

azdagron commented Jul 14, 2022

I dug into X.509 certificate related fields. I think the following fields could have reasonable use cases for customization. I probably missed some:

  1. Subject
    Customization of the subject can be required in some instances when interfacing with legacy software that does not know how to pull the identity from the URI SAN, or when there are other requirements for visibility and auditing. See SPIRE Server should issue unique Subject DN  #3110.
  2. Key Usage
    Customization of this field is dubious. There isn't a lot of breathing room here since the X509-SVID specification is pretty restrictive, although it does allow for setting some key usage bits that we currently don't set. It's unclear to me how much benefit there is to opening this up.
  3. Extended Key Usage
    This field isn't restricted quite as much as Key Usage. To my knowledge there hasn't been any complaints about expanding the values set here. Most conversation has been about further restricting the extended key usage (in a way that is currently non-spec conformant). See Both id-kp-serverAuth and id-kp-clientAuth MUST be set spiffe#227.
  4. OCSP Server
    This field only seems useful if we provide a mechanism for SPIRE to participate in certificate transparency logging ([RFC] Certificate Transparency support #1858), but could potentially just be handled by SPIRE core if CT support was first-class and not plugin driven.
  5. SANs (other than URI)
    We currently allow DNS SANs to be influenced per-entry (via DnsNames on the registration entry). We don't allow customization of the other SANs. I don't know of any strong use cases for filling out IP and Email SANs. Since network location isn't a strong identity mechanism, I could see IP SAN even being problematic depending on how validation was performed. I do think a "localhost" IP SAN might have some use.
  6. Name Constraints
    If we also invoked this plugin when crafting the CSR for the SPIRE intermediate CA, this seems like it might be useful in some environments where name constraints are already in use. From what I understand, name constraints may not be widely implemented.
  7. CRL Distribution Points
    This MAY be useful when an upstream authority already employs a CRL. While the Workload API allows for a CRL to be distributed, SPIRE does not implement that.
  8. Arbitrary Extensions
    I think if we allowed for something like this, we'd have to scrub out any known extensions that SPIRE core wanted control over. While this would provide some incredible flexibility, I'm not sure if we'd want to open these gates.

@sebastianGit
Copy link

With regard to the SPIRE CA certificate, I currently have the requirement to set certificatePolicies (OID 2.5.29.32) which is a standard extension. A plugin mechanism for the CA CSR could help to get extensions like this in the CA certificate.

@amoore877
Copy link
Member

I agree that some additional customizations could be helpful in a variety of cases, but I am curious on more details for:

Other solutions could allow for targeted customization of individual fields (i.e. X509-SVID Subject), but would not easily accommodate a wide variety of fields that might need customization.

Specifically my first go-to might've been further tags for registration entries in the DB that could be smartly interpreted when minting X509 or JWT. For instance, today we have the CN/DNS configuration there.

Is the concern DB load, some of those fields would be common across "all" signings (so dupe data in DB), or something else?

The entries in the DB have the benefit of listing out precisely what can be expected in the ecosystem as far as issued identities, so splitting the logic between DB and a plugin IMO might add some maintainability complexities.

@azdagron
Copy link
Member Author

azdagron commented Jul 22, 2022

Thanks for adding your thoughts here, @amoore877!

We discussed this at length as we (maintainers) ourselves weren't totally settled on whether or not a 1) plugin, 2) a configurable, or additional fields on the registration entry would be more appropriate.

Here are some of the things we discussed:

Registration Entry

PROS:

  • Simple determination since the customized field can be fixed value (i.e. no templates or other wizardry)
  • Operator can easily forecast what the resulting SVID shape will be like (assuming we document things well)

CONS:

  • Maintainers have long lamented the shape of the registration entry and the presence of SVID specific fields. Adding one or more fields to allow for customization of the SVID will make this problem worse. A large majority of the fields we'd want to customize here are only applicable to X509-SVIDs.
  • Growing the registration entry is currently VERY painful. Requires a database migration, a multitude of query updates, etc.
  • Would require a 1 extra field on the entry per field on the SVID that required customization. This could easily be half a dozen extra fields.
  • Doesn't accommodate customization of the CA or other non-workload SVIDs (i.e. agent and server SVIDs)

Configurables

PROS:

  • Doesn't require us to use the registration entry :)
  • There is existing precedent (i.e. the ca_subject configurable)

CONS:

  • Harder to introduce per-entry flexibility (templating is one solution... but not a great UX)
  • Also requires a 1-1 introduction of configurable to field needing customization. This is less problematic than with the entry since we can isolate this configuration into a section clearly intended for a given SVID type.
  • Doesn't easily accommodate customization of other non-workload related SVIDs or CAs without the introduction of additional configurables. Feels like it would get out of control quickly.

Plugin

PROS:

  • Allows for richer customization (programs are more expressive than fixed values or templates)
  • Gets SPIRE core out of the business of attempting to build a one-size-fits-all solution
  • Accomodates customization of CA and other non-workload SVIDS
  • Adding support for additional fields is easy
  • Supports per-entry customization (i.e. provide the entry as input)

CONS:

  • As you pointed out, less clear what the resulting SVID will look like just by inspecting the entry.

There are probably others that I've missed. After weighing all this, we didn't feel super great about any of the solutions, but felt like the plugin covered more use cases.

Happy to hear any additional thoughts you (or others) have!

We want to land on the optimal solution :)

@amoore877
Copy link
Member

Registration Entry
...
CONS
...
Would require a 1 extra field on the entry per field on the SVID that required customization. This could easily be half a dozen extra fields.

this isn't necessarily true I think, depending on how loose we might allow definitions to be. For instance, we could mimic how selector works and just have it be fairly arbitrary key-values, where the key is will have some meaning for SVIDs, like an ocsp or SAN or keyUsage key. This would also then be extensible as the business logic handles what the keys mean while the DB just needs a single migration for the one new field.

Configurables
...
CONS
...
Harder to introduce per-entry flexibility (templating is one solution... but not a great UX)

this is to me the biggest con, and the strongest pro for registration entry / plugin.

I agree with all the pros though for plugin, and they are compelling to me. Another con though would be requiring a deployment to alter behavior, unless the external plugin implementation includes its own complexities of referencing some dynamic config / DB / other signals itself.

@azdagron
Copy link
Member Author

Another con though would be requiring a deployment to alter behavior, unless the external plugin implementation includes its own complexities of referencing some dynamic config / DB / other signals itself.

I'm not sure I follow here. Can you give an example?

@amoore877
Copy link
Member

amoore877 commented Jul 22, 2022

So if some operators want to alter what the plugin itself is doing for their customizations of SVIDs, they either need a code change of the plugin's implementation or (if possible) a config change of it. This means deploying a new binary of the plugin (or the SPIRE deployment itself, depending on build packaging).

This could be mitigated if the plugin is referencing some other dynamic source for what it should be doing, but that adds complexity and IMO is Entry approach with extra steps.

Consider if operators want to alter key usage for some of the SVIDs in their ecosystem.
For a plugin approach, they would need to alter plugin code (or its imported config) to handle their new case, and then deploy the change, which may then need coordination with any other changes that could be going out. In the worst case, a rollback is needed for some reason which then halts progress on the migration.
For entry approach, this is a series of (careful) entry updates + ensure future new entries follow the desired format at creation.


additional pro for entry approach: when we want to update certain SVID behaviors, live SVIDs are updated through re-signings "immediately" due to entry revision change. Agents will get the new contents and any subscribing workloads will also be updated.

@radoslav-tomov
Copy link

Right now I would really appreciate the ability to configure custom extensions. Our PKI folks insist on setting certificatePolicies (OID 2.5.29.32) .

@sebastianGit
Copy link

An additional aspect of customization w.r.t. to the CSR for certificates of workloads and agents would be the capability to decide based on the used key material (algorithm and key length) whether a CSR is acceptable.

The requirement behind is to be able to ensure that the key material for certificates issued by SPIRE can be restricted by algorithm (e.g., RSA only, e.g. selected ECC curves only) and key length.

@azdagron
Copy link
Member Author

azdagron commented Aug 2, 2022

Hmm, that is interesting @sebastianGit. Agent CSRs are only used to convey the public key. All other fields are ignored, so as of right now agents don't get to choose the signature algorithm directly. SPIRE server uses the default signature algorithm for the key type + strength that the Go x509 package chooses.

@sebastianGit
Copy link

@azdagron : My use case is that I'm asked to make on the server-side the decision whether the agent's/workload's key material that is presented in a CSR is compliant with requirements that only selected types of key material must be supported using an explicit allow list. E.g., the server must ensure that it only issues certificates to agent's/workload's with public keys for 'RSA using 2048 bits' or 'RSA using 4048 bits' or 'ECC P-256', but NOT 'RSA using 1024 bits' or other key material.

While I'm aware that the current SPIRE agents always create CSRs using 'ECC P-256', actually anybody can write an agent and send arbitrary CSRs to the SPIRE server using key material that might be compliant with the requirements of the PKI's policies or not. Currently, the only check implemented by the server is 'is the key material supported by the used go-libs' which is a rather liberal check.

@azdagron
Copy link
Member Author

azdagron commented Aug 3, 2022

@sebastianGit, yes that makes sense.

@amoore877 the quick reactivity of an entry based approach is pretty attractive. We've been pretty hesitant to open up the entry as a bucket for arbitrary key/values though.

We've discussed this quite a bit and I think the maintainers are leaning towards the plugin approach. That approach doesn't preclude us from doing something with the entry in the future.

I'll take a stab at a plugin interface and open a PR in the SPIRE Plugin SDK repo that I'll link here. We can figure out the right interface from there.

@rturner3
Copy link
Collaborator

Another use case I can imagine for something like this is to customize the Subject of an Agent X.509-SVID such that it can be used as a client certificate to the kubelet in the k8s WorkloadAttestor plugin. This would greatly reduce friction in managing kubelet client credentials for cases where the agent needs to talk to the kubelet but is not deployed by Kubernetes.

@Celant
Copy link

Celant commented Aug 15, 2022

Off the back of the issue mentioned above, we'd love to see a default customization plugin shipped with SPIRE too. The plugin could allow for generic modification of SVIDs based on templates defined in the config. I feel like that would cover a lot of use-cases where trivial modifications are desired (i.e. adding a single static claim to SVIDs), and anything more niche could be implemented with a custom plugin.

@azdagron
Copy link
Member Author

I've made a first pass at an interface definition here. spiffe/spire-plugin-sdk#27

@rturner3 rturner3 added priority/backlog Issue is approved and in the backlog and removed triage/in-progress Issue triage is in progress labels Aug 23, 2022
@rturner3 rturner3 added this to the 1.4.2 milestone Aug 23, 2022
@evan2645 evan2645 modified the milestones: 1.4.2, 1.4.3 Sep 6, 2022
@azdagron azdagron modified the milestones: 1.4.3, 1.5.0 Sep 22, 2022
@azdagron azdagron removed this from the 1.5.0 milestone Oct 25, 2022
@azdagron azdagron added this to the 1.5.1 milestone Oct 25, 2022
@dennisgove
Copy link
Contributor

Has there been discussion about what would happen if there are multiple configured CredentialComposer plugins? I see a couple of situations coming up.

One plugin for X509s and Another for JWTs: I think it'd be fairly common for folks to create plugins for a specific type of JWT, particularly when on-boarding to SPIRE. For example, I currently have a use-case to add specific claims to JWTs but no need to update X509s. Later on, when I do have a need to set values in an X509 I'd probably want to keep that logic in its own plugin implementation.

Pipeline of Work: I could see a use-case of folks using the plugins to add/set attributes and then having a final plugin instance that'll verify the entire set of SVID attributes before generation. Such a setup is a clear ordered list of operations.

While one certainly could achieve both situations with a single plugin, I suspect the composability of individual plugins will be attractive and utilized. Are there rules regarding plugin execution order?

@azdagron
Copy link
Member Author

azdagron commented Nov 9, 2022

I think allowing for multiple credential composer plugins makes the most sense. The order in which plugins are defined in the configuration is preserved in the plugin collections, at least for those that are designed to be a list (notifiers, credential composers) and not addressed by name (node attestors). That gives us something to anchor on as a natural order of operations.

@amoore877
Copy link
Member

amoore877 commented Dec 7, 2022

I've had a thought regarding a specific x509-visible field, and curious if it would and should be supported- a field for what Registration Entry UUID + Revision Number corresponds to the issued SVID.

This would be unlikely to be used for normal operations, but it could be helpful for:

  1. debugging triage and possibly also while investigating a security event
    With some observability in place around the information, one would be able to see exactly where a particular SVID, not just SPIFFE ID which many workloads can share, was used and learn better how the SVID came about (parent ID, selectors).

  2. migration work
    if SPIRE operators are migrating some aspect of registration entries besides the SPIFFE ID (say, the selector set), then workloads may receive multiple SVIDs at a time with the same ID (old entry and new). Being able to prove "yes all workloads are getting the SVID based off of the new entry format" would grow confidence in the operation.

@evan2645
Copy link
Member

evan2645 commented Dec 8, 2022

Hey @amoore877 - discussed this today in the SPIRE maintainer call, and consensus there was that this is an important audit feature and oversight in core. Opened #3688 to track it independently .. thanks for bringing it up.

@MarcosDY
Copy link
Collaborator

It is already solved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/backlog Issue is approved and in the backlog
Projects
None yet
Development

No branches or pull requests

9 participants