Update mrcrypt to act as a thin translation layer on top of aws-encryption-cli

[mattsb42-aws]

As discussed in #8, this removes most of the bespoke logic in mrcrypt, instead largely acting as an argument transformation layer on top of aws-encryption-cli.

One point where this implementation does add bespoke logic is in the form of a custom cryptographic materials manager that enables this new version of mrcrypt to read files written by the earlier versions while only writing messages that are fully compliant with the AWS Encryption SDK spec (in other words, it can read messages with both compressed and uncompressed ECC points but will only write messages with compressed points).

In addition to the internal changes, there are some notable externally visible changes as well, that largely came for free by sitting on top of aws-encryption-cli, including:

• Now works on Python 2.7, 3.4, 3.5, and 3.6 rather than just 2.7
• Now works equally well on Linux, MacOS, and Windows
  • MacOS test results
  • Amazon Linux test results
  • Windows test results
• In addition to accepting input from stdin, output can be sent to stdout

Questions

There are a couple outstanding questions I have for you all:

File Permissions

As discussed in #8, the permissions of the written files currently honor either the umask (on *nix) or the parent directory permissions (on Windows). We made the explicit decision to use this pattern with aws-encryption-cli, but it would be fairly simple to add a hook to this new mrcrypt logic to change the permissions of written files to match what previous versions wrote.

Decrypted Suffix

In fleshing out the test suite to cover the surface test cases that you had in the old tests, I realized that I had forgotten that mrcrypt would remove the .encrypted suffix from decrypted files when the encrypted file had that suffix.

We did originally consider this behavior for aws-encryption-cli, but we decided that it would be less surprising to add an additional suffix (.decrypted in our case) to the decrypted filename rather than trying to remove a suffix. This was both to avoid accidentally overwriting files and to make it clear what the history of a file was.

I wanted to get this PR up since it's been collecting dust in my "to do" bin for a few weeks, but if you want to keep this suffix behavior, it would be fairly straightforward with a hook similar to the one that would be needed change the permissions.

If you want to retain either of these behaviors I can add this post-processing hook. This would involve writing the output metadata to a temporary file, collecting the names of successfully processed files, and performing any desired actions on the resulting list of files.

[codecov-io]

Codecov Report

Merging #9 into master will decrease coverage by 9.96%.
The diff coverage is 83.69%.

@@            Coverage Diff             @@
##           master       #9      +/-   ##
==========================================
- Coverage   94.17%   84.21%   -9.97%     
==========================================
  Files          16        3      -13     
  Lines         772      114     -658     
  Branches        0       14      +14     
==========================================
- Hits          727       96     -631     
+ Misses         45       14      -31     
- Partials        0        4       +4

Impacted Files	Coverage Δ
mrcrypt/main.py	0% <0%> (ø)	⬆️
mrcrypt/materials_manager.py	100% <100%> (ø)
mrcrypt/cli/parser.py	84.88% <80.59%> (+11.07%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 84eaf79...4cf3758. Read the comment docs.

[mattsb42-aws]

(FWIW, Codecov isn't picking up the coverage with the integration tests: with those it jumps to 98%)

[austinmoore-]

Thank you for this excellent work. I will be digging into these changes over the weekend.

As for your questions:

Regarding the umask (and parent directory permissions) change, for now, we can leave the PR as is here. I will add back the original functionality if needed.

Regarding the decrypt filename suffix, again this is another change that I think we can leave as is. I like that the file's history is clearer this way, and it reduces the chance of unexpected file overwrites.

[austinmoore-]

@mattsb42-aws I've looked it over, and it LGTM. This is really great!

One thing before I merge; with the decision to follow the decryption suffix logic of the aws-encryption-sdk-cli, could you please remove the @pytest.mark.xfail(strict=True) decorator, and change the test to expect the .encrypted.decrypted suffix?

Also, regarding this section, I have added commented on aws/aws-encryption-sdk-python#21 with a (nitpicky) request.

Finally, just so you know, I plan on releasing this as a major (2.0) release, since there is a potentially breaking change with the decryption suffix, and because this is simply just a big change.

[mattsb42-aws]

Updated to address requested changes and adding a note about the best route to building a post-processing hook.

I'll leave the version as is for whatever you might want to do before release.