Skip to content

symcrypto.py: update to allow for AEAD#124

Merged
jakinyele merged 8 commits intoJHUISI:devfrom
locksmithone:dev
Dec 30, 2016
Merged

symcrypto.py: update to allow for AEAD#124
jakinyele merged 8 commits intoJHUISI:devfrom
locksmithone:dev

Conversation

@locksmithone
Copy link
Copy Markdown
Contributor

Added the AEAD capability to symcrypto.py, i.e., it is possible to (optionally) specify additional data to be MACed together with the ciphertext. The additional data is typically a plaintext that will not be encrypted, but it is desired that this additional data be authenticated with the ciphertext. For instance, a header can be added to the ciphertext to be authenticated. Example of usage is included in the class docstring.

The module is backwards compatible with the previous AE (Authenticated Encryption) version with no additional data. All adapter classes that utilize symcrypto without additional data should continue working with no modifications.

Added the AEAD capability to symcrypto.py, i.e., it is possible to (optionally) specify additional data to be MACed together with the ciphertext. The additional data is typically a plaintext that will not be encrypted, but it is desired that this additional data be authenticated with the ciphertext. For instance, a header can be added to the ciphertext to be authenticated. Example of usage is included in the class docstring.

The module is backwards compatible with the previous AE (Authenticated Encryption) version with no additional data. All adapter classes that utilize symcrypto without additional data should continue working with no modifications.
Copy link
Copy Markdown
Member

@jakinyele jakinyele left a comment

Choose a reason for hiding this comment

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

This is great, but a few change requests. Please clean up the paths in the doc test for AuthenticatedCryptoAbstraction and update the import on line 176 to charm.toolbox.symcrypto. Otherwise, the CI tests will fail after merge. Thanks again!

Correcting import in doc test. The original import referred to a test file, and I forgot to update paths to reflect the real configuration.
Modifying paths in exception messages within the doc tests for AuthenticatedCryptoAbstraction class. I am using relative paths to avoid representing the local structure from the computer wherein the tests are being executed. I am not sure that is the way to go for Travis CI to pass...
Two lines from doc test not passing Travis CI. Removing to see whether it passes.
Removing output from dicts in doc test, since the order of key:value pairs and contents may change from test outputs and Travis CI outputs.
@locksmithone
Copy link
Copy Markdown
Contributor Author

I have removed things from the doctest such that Travis CI stops throwing errors. But now Travis CI is complaining about unexpected EOLs in literals, whereas there are no visible EOLs in the doctest. I suspect Travis CI is wrapping lines, and thus inserting \n and that causes commands to break. I do not know how to fix that short of removing the doctest completely...

@jakinyele
Copy link
Copy Markdown
Member

jakinyele commented Dec 30, 2016

Ok, I agree that the line breaks are probably causing the CI errors now. You could just shorten like this to see if it makes a difference:

aad = b'\x10\x11\x0a\x0b'
cad = cipher.encrypt('Some network PDU.', additionalData=aad)

Superficial change but maybe that'll do the trick.

Shortening one problematic line in doctest to see whether Travis CI stops complaining. It is trial and error, will fix the next Travis CI problem when it happens.
Within the doctest, Travis CI is converting an example byte str b'\x10\x11\x0a\x0b' into the actual characters. As such, the byte \x0a is converted into a line feed (\n), which Travis CI inserts into the command and breaks everything. My solution is to substitute the \x0a for something else that Travis CI deems inoffensive. Let's try again.
Travis CI complains about a missing apostrophe in doctest. Fixed. Next: paths.
@locksmithone
Copy link
Copy Markdown
Contributor Author

Ok, it seems Travis CI is ok with my relative paths in doctest.

@jakinyele
Copy link
Copy Markdown
Member

Sorry to ask but do you mind submitting another PR for the 2.7-dev branch? Shouldn't require many changes though to work w/ 2.7. Thanks in advance!

@jakinyele jakinyele merged commit a39a194 into JHUISI:dev Dec 30, 2016
@locksmithone
Copy link
Copy Markdown
Contributor Author

Absolutely, let me work on that.

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.

2 participants