Skip to content

Conversation

cpanato
Copy link
Member

@cpanato cpanato commented Feb 27, 2025

Summary

not sure if that is 100% correct

trying to upgrade go-jose

Signed-off-by: cpanato <ctadeu@gmail.com>
Signed-off-by: cpanato <ctadeu@gmail.com>
// GetIDToken extracts an OIDCIDToken from the raw token *without verification*
func (stg *StaticTokenGetter) GetIDToken(_ *oidc.Provider, _ oauth2.Config) (*OIDCIDToken, error) {
unsafeTok, err := jose.ParseSigned(stg.RawToken)
unsafeTok, err := jose.ParseSigned(stg.RawToken, allowedSignatureAlgorithms)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this identical behavior between v3 and v4, or was there a different set of allowed algs?

Copy link
Contributor

Choose a reason for hiding this comment

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

This list should be sufficient and secure. Did a little reading on this change, it was in response to https://i.blackhat.com/BH-US-23/Presentations/US-23-Tervoort-Three-New-Attacks-Against-JSON-Web-Tokens.pdf.

Copy link
Member Author

Choose a reason for hiding this comment

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

seems in the past that was not checked

this is for the new V4

// ParseSigned parses a signed message in JWS Compact or JWS JSON Serialization. Validation fails if
// the JWS is signed with an algorithm that isn't in the provided list of signature algorithms.
// Applications should decide for themselves which signature algorithms are acceptable. If you're
// not sure which signature algorithms your application might receive, consult the documentation of
// the program which provides them or the protocol that you are implementing. You can also try
// getting an example JWS and decoding it with a tool like https://jwt.io to see what its "alg"
// header parameter indicates. The signature on the JWS does not get validated during parsing. Call
// Verify() after parsing to validate the signature and obtain the payload.

// GetIDToken extracts an OIDCIDToken from the raw token *without verification*
func (stg *StaticTokenGetter) GetIDToken(_ *oidc.Provider, _ oauth2.Config) (*OIDCIDToken, error) {
unsafeTok, err := jose.ParseSigned(stg.RawToken)
unsafeTok, err := jose.ParseSigned(stg.RawToken, allowedSignatureAlgorithms)
Copy link
Contributor

Choose a reason for hiding this comment

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

This list should be sufficient and secure. Did a little reading on this change, it was in response to https://i.blackhat.com/BH-US-23/Presentations/US-23-Tervoort-Three-New-Attacks-Against-JSON-Web-Tokens.pdf.

@cpanato cpanato marked this pull request as ready for review March 3, 2025 13:31
@cpanato cpanato requested review from a team as code owners March 3, 2025 13:31
@cpanato cpanato merged commit 97659d8 into sigstore:main Mar 3, 2025
16 checks passed
@cpanato cpanato deleted the upgrade-go-jose branch March 3, 2025 14:28
@cpanato cpanato mentioned this pull request Mar 3, 2025
@mtrmac
Copy link
Contributor

mtrmac commented Mar 3, 2025

Thank you!

@haydentherapper
Copy link
Contributor

We'll cut a new release soon, after #2001 merges

@cpanato
Copy link
Member Author

cpanato commented Mar 3, 2025

We'll cut a new release soon, after #2001 merges

@haydentherapper lets try to get the current dependencies update as well, there are a few PRs opened

@haydentherapper
Copy link
Contributor

Ack, I can help with that too. Bumping to 1.23 will hopefully help unblock the updates

@cpanato
Copy link
Member Author

cpanato commented Mar 3, 2025

Ack, I can help with that too. Bumping to 1.23 will hopefully help unblock the updates

on it, doing one by one to make sure all is working :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants