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

Use correct iv size (#281) #300

Closed
wants to merge 1 commit into from
Closed

Conversation

jkamp-aws
Copy link

Use a 12 byte IV for GCM based algorithms and 16 byte IV for CBC based ones. This makes a JWE compatible with other libraries based on OpenSSL which can only use a 12 byte IV for GCM.

Use a 12 byte IV for GCM based algorithms and 16 byte IV for CBC based ones. This makes a JWE compatible with other libraries based on OpenSSL which can only use a 12 byte IV for GCM.
@floxay
Copy link

floxay commented Aug 28, 2023

Would be nice if this could be merged as it fixes #281, @mpdavis?

Copy link

@mikhaililin21027 mikhaililin21027 left a comment

Choose a reason for hiding this comment

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

Very nice

@mikhaililin21027
Copy link

Please fix it, this problem is very annoying

@TBoshoven
Copy link

Confirming this issue is preventing validation of generated JWEs for non-CBC algorithms.

@panva panva mentioned this pull request May 28, 2024
@twwildey
Copy link
Collaborator

Folks - is there a concern for resolving this issue?

I've opened the following issue for jose here: panva/jose#678

However, the primary maintainer of that package (who also opened this issue) has identified that python-jose is not following the RFC spec for JWE. It seems like this package should follow the specification. If there is no concern for following the JWE spec to adhere to the IV length requirements for AES GCM modes, can we merge this?

There are only two block cipher modes supported in backends/cryptography_backend.py today: https://github.com/twwildey/python-jose/blob/master/jose/backends/cryptography_backend.py#L424

As such, this change should be safe to merge. Can we please illustrate our concerns/problems so that this can be fixed promptly?

(This replicates my comment from #281 here)

@jkamp-aws jkamp-aws marked this pull request as draft May 29, 2024 08:19
@jkamp-aws jkamp-aws marked this pull request as ready for review May 29, 2024 08:19
@twwildey
Copy link
Collaborator

@panva @mikhaililin21027 Any advice on how we can fix these linting issues that are blocking a merge?

@panva
Copy link

panva commented May 29, 2024

Any advice on how we can fix these linting issues that are blocking a merge?

I'm not a maintainer here.

@twwildey
Copy link
Collaborator

I've created another PR with an equivalent change: #355

@twwildey
Copy link
Collaborator

I have merged changes from the other PR. As such, I am closing this PR in favor of #355.

@twwildey twwildey closed this May 30, 2024
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.

None yet

6 participants