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

Fix computing Aad per the RFC by doing base64 encoding #147

Merged
merged 3 commits into from
Jun 16, 2020

Conversation

puiterwijk
Copy link
Contributor

@puiterwijk puiterwijk commented Jun 15, 2020

The Authenticated Additional Data per RFC7516, section 5.2, step 14
is the BASE64URL(UTF8(JWE Protected Header)).
Biscuit previously used the UTF8(JWE Protected Header) value as Aad.
This meant it was unable to decrypt tokens encrypted by external
parties.

Signed-off-by: Patrick Uiterwijk [email protected]

The Authenticated Additional Data per RFC7516, section 5.2, step 14
is the BASE64URL(UTF8(JWE Protected Header)).
Biscuit previously used the UTF8(JWE Protected Header) value as Aad.
This meant it was unable to decrypt tokens encrypted by external
parties.

Signed-off-by: Patrick Uiterwijk <[email protected]>
@puiterwijk
Copy link
Contributor Author

NOTE: This breaks any existing encrypted JWE's, but those are non-compliant (and won't be decryptable by external libraries anyway).

Copy link
Owner

@lawliet89 lawliet89 left a comment

Choose a reason for hiding this comment

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

Thank you for the fix.

I will be releasing a breaking version soon, so this will fit in just right.

src/jwe.rs Outdated Show resolved Hide resolved
@lawliet89 lawliet89 added breaking Fixing this will cause breaking changes bug labels Jun 16, 2020
@lawliet89 lawliet89 merged commit ec9c1e8 into lawliet89:master Jun 16, 2020
@puiterwijk puiterwijk deleted the fix_aad branch June 16, 2020 07:14
@lawliet89 lawliet89 added this to the v0.5.0 milestone Jul 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Fixing this will cause breaking changes bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants