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

Fixed Symmetric Key Encryption/Decryption #12

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

wendtr
Copy link

@wendtr wendtr commented Mar 3, 2016

So I noticed that symmetric key encryption was not working, even though the functions existed. This fix that I put in isn't necessarily the greatest, but it allows you to use the AES encryption/decryption functions for CEK. I know this might not necessarily be the best way to do so, but it is currently a quick fix for anyone who wants to use symmetric crypto with your library.
In addition, I created a simple test in the test suite.

sregister is the other contributor to this, although it may not reflect so in the commits.

Ridge Wendt added 2 commits March 2, 2016 23:35
@yuriikonovaliuk
Copy link
Collaborator

@wendtr This is not right way to do it. A(128|192|256)CBC algorithms are not allowed for key encryption (refer to http://tools.ietf.org/html/rfc7518#section-4.1). AES encryption can be used for key wrapping but there are dedicated AES algorithms (A(128|192|256)KW. Refer to http://tools.ietf.org/html/rfc3394#section-2.2) which are not currently implemented in the library.

@wendtr
Copy link
Author

wendtr commented Mar 9, 2016

Right after sending that pull request we realized we implemented everything poorly, and that it did not satisfy any of the algorithms supported by JWE. What we were looking to do was use Direct Symmetric Encryption (RFC 7518, Section 4.1, 4.5). I believe our current implementation of Direct Encryption implements this. The code is not implemented in the best way (uses an If-Else statement), but it could be cleaned up possibly.

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.

3 participants