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

[WIP]Add support for specifying allowed OIDC JWT signing algorithms (#2753) #2851

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

andoks
Copy link

@andoks andoks commented Nov 15, 2024

Description

Add (legacy for now) configuration flag "oidc-enabled-signing-alg" (cfg: oidc_enabled_signing_algs) that allows setting what signing algorithms are specified by provider in JWT header ("alg" header claim).

In particular useful when skip_oidc_discovery = true, as verifier defaults to only accept "RS256" in alg field in such circumstances.

Motivation and Context

For improved security by using newer hash algorithms.

Implements change suggested in #2753

How Has This Been Tested?

Tested locally with rauthy as OIDC Provider

Checklist:

  • My change requires a change to the documentation or CHANGELOG. ??
  • I have updated the documentation/CHANGELOG accordingly.
  • I have created a feature (non-master) branch for my PR.
  • I have written tests for my code changes.
  • add support in yaml (modern) config
  • add more test(s)?

Copy link
Member

@tuunit tuunit left a comment

Choose a reason for hiding this comment

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

What happens if no signing algo is set? We should keep backwards compatibility in mind and use our current RS256 as the default value.

Furthermore, please run make generate on your changes and push the resulting change to the alpha docs.

@tuunit
Copy link
Member

tuunit commented Nov 15, 2024

Furthermore instead of a simple string list I would like to restrict the input to an enum of algos which the oidc library supports.

@andoks
Copy link
Author

andoks commented Nov 15, 2024

What happens if no signing algo is set? We should keep backwards compatibility in mind and use our current RS256 as the default value.

Empty list results in the old behaviour of RS256 according to github.com/coreos/go-oidc/[email protected]/oidc/verify.go:317

Furthermore, please run make generate on your changes and push the resulting change to the alpha docs.

Done

Furthermore instead of a simple string list I would like to restrict the input to an enum of algos which the oidc library supports.

I suspect the valid list of signatures to be the ones listed at github.com/go-jose/go-jose/[email protected]/shared.go:105. Instead of an enum, we could also just allow strings, and rely on the go-jose to return a reasonable error when provided with an invalid signature algorithm? This would make it easier to maintain in the future at least when the library adds support for additional signing algorithms. If however I must make an enum out of it, do you have a link to an example of how the project prefers them, since golang doesn't support enums natively?

// Signature algorithms
const (
	EdDSA = SignatureAlgorithm("EdDSA")
	HS256 = SignatureAlgorithm("HS256") // HMAC using SHA-256
	HS384 = SignatureAlgorithm("HS384") // HMAC using SHA-384
	HS512 = SignatureAlgorithm("HS512") // HMAC using SHA-512
	RS256 = SignatureAlgorithm("RS256") // RSASSA-PKCS-v1.5 using SHA-256
	RS384 = SignatureAlgorithm("RS384") // RSASSA-PKCS-v1.5 using SHA-384
	RS512 = SignatureAlgorithm("RS512") // RSASSA-PKCS-v1.5 using SHA-512
	ES256 = SignatureAlgorithm("ES256") // ECDSA using P-256 and SHA-256
	ES384 = SignatureAlgorithm("ES384") // ECDSA using P-384 and SHA-384
	ES512 = SignatureAlgorithm("ES512") // ECDSA using P-521 and SHA-512
	PS256 = SignatureAlgorithm("PS256") // RSASSA-PSS using SHA256 and MGF1-SHA256
	PS384 = SignatureAlgorithm("PS384") // RSASSA-PSS using SHA384 and MGF1-SHA384
	PS512 = SignatureAlgorithm("PS512") // RSASSA-PSS using SHA512 and MGF1-SHA512
)

…auth2-proxy#2753)

TODO:
- [ ] update docs
- [ ] add support in yaml (modern) config
- [ ] add more test(s)?

Add (legacy for now) configuration flag "oidc-enabled-signing-alg" (cfg:
oidc_enabled_signing_algs) that allows setting what signing algorithms
are specified by provider in JWT header ("alg" header claim).

In particular useful when skip_oidc_discovery = true, as verifier
defaults to only accept "RS256" in alg field in such circumstances.
@tuunit tuunit force-pushed the topic/andoks/oidc-enabled-signing-algs-support branch from 81021df to 887f482 Compare January 9, 2025 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants