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

Expand JWT claim matching capabilities #493

Open
cipherboy opened this issue Aug 28, 2024 · 26 comments · May be fixed by #869
Open

Expand JWT claim matching capabilities #493

cipherboy opened this issue Aug 28, 2024 · 26 comments · May be fixed by #869
Labels
auth/jwt Related to the JWT/OIDC auth engine feature New feature or request good first issue Good for newcomers help wanted Extra attention is needed rfc RFC design document included

Comments

@cipherboy
Copy link
Member

Is your feature request related to a problem? Please describe.

GitLab provisions complex JWT claims on ID tokens used with its Vault integration. However, the JWT role isn't expressive enough to fully authenticate these fields.

Describe the solution you'd like

In particular, the sub claim contains the following structure, which is unique to pipeline jobs:

project_path:{group}/{project}:ref_type:{type}:ref:{branch_name}

Matching on a prefix of this (bound_subject=project_path:my-group/my-project:ref_type:*:ref:*) isn't possible, to allow a pipeline running in any branch to have access to protected secrets. Similarly, for bound_claims, it isn't possible to match a glob or an either-or choice (say, bound_claims[environment]=prod-* or bound_claims[ref]=v* (with ref_type=tag)).

I'm not sure what tools we have currently, or what make the best sense to solve it:

  • Globs could work, but would be rather greedy.
  • Regexes could also work but be more error-prone.
  • Templating is a usual design approach but because we're authenticating, we won't have it yet.
  • Lists of possible matches (allowed_domains in PKI secrets enging).

Describe alternatives you've considered

Manually specifying every possible value could potentially require a large increase in the number of roles, or tight coupling to their current values which could be inflexible (e.g., binding strictly to version number from a tag, while not binding at all could result in say, requests from another branch).

Explain any additional use-cases

n/a

Additional context

n/a

@cipherboy cipherboy added feature New feature or request good first issue Good for newcomers help wanted Extra attention is needed labels Aug 28, 2024
@cipherboy cipherboy added the auth/jwt Related to the JWT/OIDC auth engine label Sep 7, 2024
@suprjinx
Copy link

Hi @cipherboy, I'm pondering this story and hoping to make a stab at it! I'm a little confused how the sub claim works in the example case, since we'd usually expect sub to match an entity_alias. In this case, I don't think there would be entity or entity_alias which corresponds, so we'd just be matching to a role via bound_claims only?

An additional question: would the policies granted by the role be a static list? Ie, any claim match gets a specific list of policies.

@cipherboy
Copy link
Member Author

cipherboy commented Dec 11, 2024

since we'd usually expect sub to match an entity_alias.

@suprjinx This is true in some deployments, but I didn't think this was a universal assumption (that everyone would be using entities to back the JWT engine -- you can use it without provisioning entities). However, even with entities backing JWTs, this is definitely still applicable to bound_claims.

One of the other ideas I had for this was using something like #753, but for matching JWTs like GCP uses. Then you could template your entity if desired. That would be an alternative to the role system entirely. Thoughts?

@suprjinx
Copy link

One of the other ideas I had for this was using something like #753, but for matching JWTs like GCP uses. Then you could template your entity if desired. That would be an alternative to the role system entirely. Thoughts?

I'm totally unfamiliar with GCP -- do you have a reference to their docs on how this works? I'm thinking that regex pattern in the bound_claim for a role would be the simplest implementation, with well-understood syntax and complete control?

@cipherboy
Copy link
Member Author

@suprjinx Hmm, I think the k8s docs are a little better (which also uses CEL): https://kubernetes.io/docs/reference/using-api/cel/, https://kubernetes.io/docs/reference/access-authn-authz/validating-admission-policy/, and https://github.com/google/cel-spec are good pointers to start with.

Essentially, it is a non-Turing-complete language with more expressive properties for validation and supporting custom environment-provided functions. While a regex is OK, it becomes hard to describe complex templating semantics with it that you could with CEL. This would require two languages (one for validation, using regex) and one for templating (which, would also partially re-do validation and could err separately from request validation erring).

I guess, lacking another concrete example, in the above subject, we'd have project_path:{group}/{project}:ref_type:{type}:ref:{branch_name} provisioned by the GitLab ID token issuer... but perhaps only {group}/{project} makes sense to use as an entity name. Even if you had a regex to allow-list a specific role, you'd still need a templating engine (Go's text/template is used in our project) to handle the entity evaluation.

CEL would also help with repetition: you could do conditional matches (e.g., if the project is XYZ, also allow ABC branch, but only DEF branches for all other projects) and other more complex things with fewer, more succinct policies.

My 2c!

@suprjinx
Copy link

I came across this PR in vault hashicorp/vault-plugin-auth-jwt#89, where globs were preferred to regex

@cipherboy
Copy link
Member Author

@suprjinx Right, we should have that as well (https://openbao.org/api-docs/auth/jwt/ -- search for bound_claims_type which supports a "glob" option). But globs aren't quite the right format for enforcing co-dependent values (in the example above, either (environment=prod-*) OR (ref=v* AND ref_type=tag).

The latter, even if it were for a single "entity" (if we defined it as above as {group}/{project}), would require multiple roles to match on, and thus require the pipeline to conditionally choose a role based on some contextual information. This sometimes is possible but sometimes isn't, depending on how the application is written (GitLab CI/Runner don't easily support conditional auth roles, to my knowledge, though perhaps could be extended to do so). So in the worst case, you'd end up with a (one entity)->(many roles) mapping, which is tough to manage and maintain.

@suprjinx
Copy link

Sounds like CEL is the way to go here. Would you suggest we add it as a bound_claims_type, similar to glob?
image

@cipherboy
Copy link
Member Author

@suprjinx Hmmm, I think that would be fine for an incremental improvement, but I think adding CELs would be a good time to rethink the binding logic entirely. How about a new interface instead? What would that look like?

@suprjinx
Copy link

suprjinx commented Dec 11, 2024

@suprjinx Hmmm, I think that would be fine for an incremental improvement, but I think adding CELs would be a good time to rethink the binding logic entirely. How about a new interface instead? What would that look like?

We could add a new map attribute to the role, eg claims_expressions:

{
  "user_claim":  "sub",
  "bound_claims": {
    "groups": "admin"
  },
  "claims_expressions": {
    "sub": "claim_value.startsWith('svc-')",
    "groups": "'admin' in claim_value"
  },
  "policies": ....
}

@suprjinx
Copy link

I do think expanding the bound_claims_type property to include cel is the simplest path forward here. But if we want to rethink the interface -- we could follow the Sentinel Policy idea and put the CEL logic into the policy itself. So in the Gitlab example, a glob might bind the Role to the sub pattern generally, but a CEL Policy attached to the role could evaluate more nuanced access.

@cipherboy
Copy link
Member Author

But if we want to rethink the interface -- we could follow the Sentinel Policy idea and put the CEL logic into the policy itself.

I don't think this is quite sufficient. Unless we wish to issue valid-but-useless OpenBao tokens (which are reject-all, and I'd rather not issue a token in the first place), we still need a moderately-complex token issuance rejection logic. In particular, if your matching on sub is not sufficient, even if your CEL policy is sufficiently advanced to reject it based on associated metadata, you would still have the potential to accept validly-signed-but-unauthorized JWTs and issue a OpenBao token for them, which your underlying CEL policy would reject all access for, because your role isn't sufficiently expressive.

So I'd argue you need some amount of CEL-in-JWT-engine anyways. And if you have it, the choice of backend policy engine doesn't matter, it is just a language for constraining OpenBao token issuance.

I guess, articulating a bit more what I'd expect out of just the JWT engine... I think I'd do something like:

  • CRUD on auth/jwt/cel/:role-name with params:
    • auth_program --> CEL program which outputs false or a valid (subset of?) the logical.Auth { ... } object.

(you'd enforce exclusivity with cel/:role-name and roles/:name)

Authentication is then simplified to run OIDC or JWT authentication up until you do the role-match evaluation, where you'd then switch over to executing the CEL program auth_program instead of the role-based policy stuff after https://github.com/openbao/openbao/blob/main/builtin/credential/jwt/path_login.go#L141-L156 (though, modifying the claim validator to remove the other claim validation occurring there).

The resulting logical.Auth would come from the auth_program assuming it is not false-y.

Thoughts?

@suprjinx
Copy link

that sounds like a great approach -- do we need an RFC step before starting to work on it?

@cipherboy
Copy link
Member Author

@suprjinx Given we're introducing a whole new language as a dependency to this plugin, I'd say it's prudent. If you don't want to write it though, I'm happy to do the paperwork.

@cipherboy
Copy link
Member Author

@suprjinx ah one thing I hadn't considered is the role_type={oidc,jwt} parameter should probably be part of the path direct rather than inferred from the program. It determines how authentication goes so needs to be statically set.

Then @DrDaveD can give us his thoughts on this :-)

@suprjinx
Copy link

I'll try to work up an RFC draft and post here -- it seems like there's a good bit of overlap with RFC #753, so I'll follow its lead :-)

@DrDaveD
Copy link
Contributor

DrDaveD commented Dec 16, 2024

I'm on board with using CEL as a means of evaluating jwt claims, but I don't yet understand the justification for adding a new auth/jwt/cel/ path or understand how that would integrate with auth/jwt/role. Maybe we need an interactive session to explain it to me.

@cipherboy
Copy link
Member Author

cipherboy commented Dec 16, 2024

Sure, happy to @DrDaveD. The short answer is it is an either-or thing. Either you use a role (with a given role name) or you use CEL (with a given CEL role name). The purpose of the new path is that many of the options on auth/jwt/role/:name are redundant or rather, up to the CEL program author to handle. The CEL paths will have fewer parameters (I think, two) but have more logic encoded in the program itself...

We could leave it on the auth/jwt/role/:name path, but then we'd need a lot of rejection logic (if cel_program is specified, reject almost all the other parameters) and that makes it hard for discovery as well (if you're using path-help or browsing the API docs).

Let me know if you (well, both of you @suprjinx!) want to chat ahead of our Jan 9th community call or if we are happy leaving it until then.

@DrDaveD
Copy link
Contributor

DrDaveD commented Dec 16, 2024

So the concept of role is still applicable, it's just implemented way differently. Maybe it would be easier to understand if it still included the word "role" then, like auth/jwt/celrole/:name or something like that to indicate that it's a different kind of role.

I don't see any applicability of this new type of role to the oidc type because there the authentication is done entirely by the oidc token issuer not by the jwt, so this could be exclusively for jwt role_type.

Come to think of it, overall there's a lot of different options between the oidc and jwt role types. Apparently there were enough in common that it made sense to keep them combined into one path. There may yet be enough options that are applicable to jwt both with and without cel. Maybe we should figure out exactly which options are applicable before making a decision about using a new path.

@suprjinx
Copy link

in #753, the cel items are posted to path cel/policies. Would that make sense here? in this case, it seems like the cel items are "optional extensions which compute policies" -- so maybe /auth/jwt/hooks or some such?

@cipherboy
Copy link
Member Author

cipherboy commented Dec 17, 2024

@DrDaveD said:

authentication is done entirely by the oidc token issuer not by the jwt

While true that the authentication is done by the token issuer, isn't the JWT Auth method still controlling authorization, i.e., whether or not it accepts the issued OIDC ID token? If you set bound_claims to a value, can't you forbid a valid, finished OIDC auth flow from generating a Vault token?

From that standpoint, the CEL policy would also be applicable to OIDC: you'd use that (instead of the native role capabilities) to control whether you issue a Vault token and with what properties.

I see both sides of the "roles" naming -- I personally find value in not calling it a role, so there's no confusion that it doesn't augment or supplement the role -- but also see value in calling it a role for consistency / discoverability. Hmmm... :-)

@cipherboy
Copy link
Member Author

@suprjinx said:

in #753, the cel items are posted to path cel/policies. Would that make sense here? in this case, it seems like the cel items are "optional extensions which compute policies" -- so maybe /auth/jwt/hooks or some such?

Hmm, the reason why they were posted to /pki/cel/policies/:name was so that we could embed other information under /pki/cel/<here> if we needed to. /pki/config/cel will likely control some parameters related to CEL but maybe we'll seek to introduce something else under the /pki/cel namespace... My initial thoughts was perhaps something around CRL building (and extensions there) or ACME, potentially related to EAB.

I don't quite like calling them hooks. How about /auth/jwt/cel/policies for consistency with PKI then, addressing @DrDaveD's point? Perhaps calling them policies is confusing from an authentication standpoint (since it conflicts name-wise with ACL policies and potentially the future global CEL auth policies)... or /auth/jwt/cel/roles/:name and we should update the PKI engine to use the same format (and @fatima2003 can update her proposal)?

Personally I believe bikesheds should always be a nice shade of blue ;-)

@suprjinx
Copy link

There's a lot of non-intuitive naming in the Vault API, so I think it's worth some bikeshedding to get a good solution :-) I'll put /auth/jwt/cel/roles in the RFC doc.

@suprjinx
Copy link

suprjinx commented Dec 17, 2024

RFC: CEL Expression Support for JWT Authentication

Summary

This feature proposes the use of Common Expression Language (CEL) in Vault's jwt-based auth engines (oidc, jwt) to create a flexible framework for validating claims and dynamically assigning token policies. CEL will allow administrators to define fine-grained rules for claim validation and dynamic policy assignment, extending the more static assignment of role- and identity-based policies.


Problem Statement

Vault's jwt-based auth engines (OIDC and JWT) currently assemble token policies from roles and identities. However, administrators cannot enforce complex constraints, such as validating combinations of claims or dynamically assigning token policies based on claim values.


User-facing description

Administrators can define CEL extensions under a new endpoint, auth/{oidc|jwt}/cel/roles/:name, to be executed during authentication. CEL roles evaluate incoming claims and generate logical.Auth{} instances having appropriate token policies.

Users authenticating with JWT or OIDC tokens will have their claims validated through the CEL roles where present. If successful, they receive a token with dynamically-assigned policies.

For example:

  • A CEL role validates that the token contains specific claims like "admin" in claims.roles && matches(claims.tenant_id, [0-9]+)".
  • If validation passes, the CEL program assigns the token policies admin and team-12345 (using claims.tenant_id value).

Technical Description

CEL in Vault Auth Engines (OIDC, JWT)

CEL roles integrate with Vault's Auth engine (OIDC and JWT) to dynamically evaluate claims. Instead of using static role-to-policy assignment with bound_claim, CEL programs validate claims and return a logical.Auth instance containing dynamically assigned token policies.

Workflow:

  1. A user authenticates with a JWT or OIDC token.
  2. The incoming claims are passed to the CEL role for validation.
  3. If the CEL policy evaluates successfully:
    • A logical.Auth instance is returned.
    • Token policies are assigned dynamically based on the CEL program logic.
  4. If validation fails, false is returned and token is not generated.

API Endpoints

Use Method Path
List CEL roles LIST auth/jwt/cel/roles
Retrieve a specific CEL role GET auth/jwt/cel/roles/:name
Create a CEL role POST auth/jwt/cel/roles/:name
Update a CEL role PATCH auth/jwt/cel/roles/:name
Delete a CEL role DELETE auth/jwt/cel/roles/:name

CEL Request Format (Auth Engine to CEL)

The following parameters are sent to the CEL policy engine:

  • claims (map[string]any: required)
    All claims present in the incoming JWT or OIDC token.

CEL Reply Format (CEL role to Auth Engine)

The CEL role engine evaluates the claims and returns a logical.Auth object or error that determines the authenticated user's token and policies.

  • auth (logical.Auth: optional)
    A logical.Auth instance containing the following fields:

    • policies ([]string: required)
      List of token policies assigned to the user.
    • lease_duration (int: required)
      Duration (in seconds) for which the token is valid.
    • renewable (bool: required)
      Whether the token is renewable.
  • error (Error: optional)
    A detailed error if the program fails.

Rationale and alternatives

Dynamic policy assignment can be implemented using templating for ACLs, but the expressiveness of CEL is much greater.

Some form CEL or regex evaluation could be added to existing role instead of creating a new type, as with glob matching for bound_claims.

Alternatively, a web hook approach like Vault's CIEPS could be used to achieve similar flexibility. However, the performance implications and system brittleness make this an unattractive alternative.

Downsides

Additional complexity of a new type; not backwards compatible with Vault.

@DrDaveD
Copy link
Contributor

DrDaveD commented Dec 17, 2024

If you set bound_claims to a value, can't you forbid a valid, finished OIDC auth flow from generating a Vault token?

Oh I suppose it would be possible. My assumption was that the token issuer is trustworthy and there's no good reason to second guess it by blocking some of the things it generates.

@cipherboy cipherboy added the rfc RFC design document included label Dec 17, 2024
@suprjinx
Copy link

i think the next step here might be a POC branch which I'll work on (following @fatima2003's approach in #753), but perhaps the RFC should become it's own ticket?

@suprjinx
Copy link

suprjinx commented Dec 31, 2024

POC draft PR here #869 -- feedback most appreciated!

@suprjinx suprjinx linked a pull request Dec 31, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auth/jwt Related to the JWT/OIDC auth engine feature New feature or request good first issue Good for newcomers help wanted Extra attention is needed rfc RFC design document included
Projects
None yet
3 participants