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

initialize JWT GEP #2213

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

brianehlert
Copy link
Contributor

@brianehlert brianehlert commented Jul 21, 2023

What type of PR is this?
/kind gep

What this PR does / why we need it:
This is the introduce the JWT Policy concept into Gateway API

Which issue(s) this PR fixes:
Fixes 2198

Does this PR introduce a user-facing change?:

NONE

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/gep PRs related to Gateway Enhancement Proposal(GEP) cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jul 21, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: brianehlert
Once this PR has been reviewed and has the lgtm label, please assign robscott for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 21, 2023
@k8s-ci-robot
Copy link
Contributor

Hi @brianehlert. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@robscott
Copy link
Member

Thanks @brianehlert!

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 24, 2023
## Properties of a JWT Claims (authorization) Policy

- token - the token that needs to be evaluated (this is the result of the validation Policy or an OIDC workflow)
- claimsMatch - an array of claim properties (exists, not exists) or match value that must match to allow the request to proceed.
Copy link
Contributor

Choose a reason for hiding this comment

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

In addition to allowing the request to proceed, Istio provides access to the evaluated token in route rules, so that users can also do things like weighted routing based on the claims. See https://istio.io/latest/docs/tasks/security/authentication/jwt-route/

Copy link
Contributor Author

@brianehlert brianehlert Aug 4, 2023

Choose a reason for hiding this comment

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

I have historically considered this a claims match modifier (Policy) on a path/route.
First the path/route must match, then the claim is tested for allow (or the request is blocked)
I am assuming the same implementation.
This is where I see the authorization Policy serving the function of defining the allow claim(s) match.

Copy link
Contributor Author

@brianehlert brianehlert Aug 7, 2023

Choose a reason for hiding this comment

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

I do like the pattern with:

outputClaimToHeaders:
- header: "x-jwt-claim-foo"
  claim: "foo"

What I am thinking about is if there is a way to have the independent of the jwksUri, so that it stands alone as a transformation action pattern that could be used for both OIDC and JWT. Since both can require the same claim-to-header, but both don't require the jwksUri.
Or the pattern is simply repeated when the OIDC Policy comes...

geps/gep-2198.md Outdated Show resolved Hide resolved
geps/gep-2198.md Outdated Show resolved Hide resolved
geps/gep-2198.md Outdated Show resolved Hide resolved
geps/gep-2198.md Show resolved Hide resolved
@youngnick
Copy link
Contributor

Once #2689 merges, this will need a rebase and update - the GEP files have moved.

@shaneutt
Copy link
Member

@brianehlert this can be rebased now as per the GEP move we did a couple months back. Let us know if you run into any trouble.

## How is a JWT used

In practice a JWT is presented to an authorization layer to make a decision of whether or not to grant access to a specific hostname or application path (backend service) or also possibly used by the backend service to decide if specific secured data can be presented.
This bring forth a set of common workflows which in turn present the use cases. In this example we will walk through the life of an encrypted JWT (though not all JWTs are encrypted)
Copy link

Choose a reason for hiding this comment

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

Not sure why it's focused on encrypted JWTs - they are quite rare. Maybe focus on the common case, and have a separate doc or section for the less common ?


- the token content is tested for the existence of some claim property or value
- claim values may be extracted and saved as header value(s) to the request
- the entire token or a sub-set of claims may be forwarded directly to backend services for evaluation
Copy link

Choose a reason for hiding this comment

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

Important use: a different token signed by the gateway is forwarded to backends ( to prevent backends from accessing the original JWT which may have escalation risks). That's sometimes called 'mint'

- [https://istio.io/latest/docs/tasks/security/authentication/jwt-route/](https://istio.io/latest/docs/tasks/security/authentication/jwt-route/)
- [https://istio.io/latest/docs/tasks/security/authentication/claim-to-header/](https://istio.io/latest/docs/tasks/security/authentication/claim-to-header/)

## Properties of a JWT Validation Policy
Copy link

Choose a reason for hiding this comment

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

I think it would be ideal to decouple JWT validation - as in 'is signature correct' ( same as mTLS handshake checks ) and the authorization parts ( which claims, paths, etc need to be matched to authorize the user ).

For the later - it is best to have authorization decisions independent of the authn, and not mix it here.


Some very high level proposed properties to open the conversation based on comparing the implementations and reviewing the RFC.

- name / realm (required) - the name / realm of the JWT, this is commonly a string representing a specific application. It is seen used in response and log messages.
Copy link

Choose a reason for hiding this comment

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

Not sure why name/realm would be required. Log messages are important but not that important.

Issuer field is probably more useful as a key.

For jwksURI - it should be mentioned 'if missing, implementation should fetch using OIDC'. And perhaps have a way to specify the public keys directly.

Not sure if keyCache is that important ( implementation detail ), and 'token' is a very bad name ( it usually means a secret ). If you mean the 'header' or 'parameter' - better be explicit.

## Properties of a JWT Claims (authorization) Policy

- token - the token that needs to be evaluated (this is the result of the validation Policy or an OIDC workflow)
- claimsMatch - an array of claim properties (exists, not exists) or match value that must match to allow the request to proceed.
Copy link

Choose a reason for hiding this comment

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

I think claim match is already in the realm of authorization.

It doesn't scale and doesn't work well in practice to have 'name=user1' ... or name=user100' in policies that specify which issuers to trust.

And there are many other authn methods ( access tokens with Oauth, client certs, user:password, apikeys ) that result in 'claims' associated with a principal. We shouldn't have a duplicated or divergent authorization depending on how a user may choose or be able to authenticate.

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/gep PRs related to Gateway Enhancement Proposal(GEP) lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dynamic header matching in HTTPRoute
9 participants