-
Notifications
You must be signed in to change notification settings - Fork 177
Allow specifying the data length for CCM mode #359
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
Conversation
It should be a separate method rather than an overload of #update. I have no idea why OpenSSL did not use the EVP_CIPHER_CTX_ctrl() interface to notify the total length, but we don't need to follow that. Perhaps #ccm_message_len=, as CCM is the only mode that requires this? I don't know if "message" is the appropriate word here. This needs a test case. RFC 3610 includes some test vectors for AES-CCM. |
I reverted the changes that overloaded the I also added unit tests using Test Vector 1 from Section 8 of RFC 3610. It looks like the unit tests are failing on a number of systems where the |
The test case can be defined conditionally. The AES-OCB test case does this. Please rebase commits and it's good to merge 👍 |
16448fc
to
6e110c3
Compare
Changes made and commits squashed. Thanks for the review. |
2f85c6a
to
db06bd3
Compare
I fixed an instance where the AES-128-CCM cipher was not being identified as an AEAD cipher by checking |
b34c3d5
to
542cf13
Compare
I'm sorry for the long silence. I'm suspect the failure with 1.1.0l is related to OpenSSL commit openssl/openssl@67c81ec, which went into OpenSSL 1.1.1 but was not backported to 1.1.0. Reordering the keyword arguments to new_decryptor so that ccm_data_len will come after tag_len (to match the example in https://wiki.openssl.org/index.php/EVP_Authenticated_Encryption_and_Decryption#Authenticated_Encryption_using_CCM_mode) might fix it. |
I tried another test configuration to replicate the ordering from the wiki you referenced as closely as I could with no luck. I'll have to look into this a little more. |
This appears to be another OpenSSL bug, which was fixed by openssl/openssl@197421b / OpenSSL 1.1.1. The 'final' call is a fundamental part of the EVP_Cipher* API and my view now is that OpenSSL 1.1.0 didn't really support CCM mode and nobody tried to use it. I think the test code can just be skipped for OpenSSL < 1.1.1. |
Allow specifying just length to #update CCM mode ciphers need to specify the total plaintext or ciphertext length to EVP_CipherUpdate. Update the link to the tests file Define Cipher#ccm_data_len= for CCM mode ciphers Add a unit test for CCM mode Also check CCM is authenticated when testing
74c6500
to
bb38169
Compare
Alright all unit tests are passing and it's been rebased again into one commit. Thanks alot for your help in working through the test issues. |
Looks good to me. Thank you. |
As it is currently written the
Cipher
object can not be used in CCM mode with additional authenticate data. This is because per the OpenSSL documentation, when a Cipher is used in CCM mode the ciphertext, plaintext length must be specified toEVP_CipherUpdate
as theinl
parameter while bothin
andout
are set toNULL
(see the "CCM Mode" section of the OpenSSL docs here: https://www.openssl.org/docs/man1.1.1/man3/EVP_CipherUpdate.html). This PR changes the implementation of#update
to allow the data parameter to be specified as an integer which is assumed to be the length of the ciphertext / plaintext for the update operation effectively fixing the limitation and allowing users to utilize the CCM mode.Before the patch there would be an exception when trying to set
#auth_data
stating that additional data could not be set. After the patch, users can set the length of the message to encrypt using#update
and fully utilize this cipher mode. Note that this behavior is only necessary for CCM mode. GCM on the other hand works as is, and does not require specifying the length of the plaintext / ciphertext.Example Usage
Example functioning CCM mode implementation (requires changes proposed by this PR):
Output:
If for whatever reason this implementation isn't ideal, the gist of an alternative is that the length of the plaintext / ciphertext needs to be able to be specified in a call similar to
EVP_CipherUpdate(ctx, NULL, &len, NULL, text_len)
before the AAD is set. An example implementation in C is available on the OpenSSL wiki here: https://wiki.openssl.org/index.php/EVP_Authenticated_Encryption_and_Decryption#Authenticated_Encryption_using_CCM_mode.