Modify recrypt command to allow recrypting file with different encryp… #232
Conversation
|
@rnelson0 Should we add tests also for this scenario? |
|
Yes, tests are definitely recommended. |
|
@rnelson0 I added more tests for recrypt command specifically for the option introduced by this PR. Initially the text is encrypted with PKCS7, then rencrypted with a different encryption plaintext and finally rencrypted back to PKCS7. We introduced the -d (change_encryption) option. |
|
@rnelson0 Looking forward to you feedback. Do you need more details to be able to review the PR? |
|
@cosmincojocar I've reached out to @TomPoulton for some verification. This looks good to me, but we (VP) have just recently accepted this gem so I want to make sure I'm not missing something obvious, and to make sure that the updated tests are properly validating this (I think we need to change the test output so it's more clear which tests are running so we know a test isn't skipped - but unrelated to this PR). |
|
@rnelson0 Any progress on this PR? |
|
I have to rely on @TomPoulton for this, sorry. |
|
Hi all, from what I can tell by glancing at the logs, it looks like there's an incompatibility between the I imagine we're going to run into these problems more are more, so I suppose we should decide whether we want to "officially" support |
|
@TomPoulton I created #238 for version support. In the meantime, can you review the code itself for possible issues, either technical or that would affect the roadmap? |
|
@TomPoulton @rnelson0 Any progress with the review? It would be grate if you could merge it if there aren't any concerns. |
ccojocar
force-pushed
the
dianac10-modify_recrypt_accept_different_input_output
branch
from
58a25d1
to
4698024
Compare
|
@rnelson0 Also this branch is rebased. |
|
@cosmincojocar This should be good to rebase again, as well! |
ccojocar
force-pushed
the
dianac10-modify_recrypt_accept_different_input_output
branch
from
4698024
to
82fbe23
Compare
|
@rnelson0 done, the tests are now green. |
|
Hooray! Thanks for your patience! |
|
Personal, rather than company opinion, but it is possible that the way this is implemented has broken GPG encryption causing the default to always be PKCS7 on recrypt, irrespective of what is listed in the various config files. It certainly appears that 3.0.0 onwards is affected by this problem. |
I rebased the PR #211 initially created by @dianac10.
@rnelson0 @nfagerlund Please could you review the changes? We plan to open source soon a plugin for Microsoft KeyVault. Without this fix, we are forced to use a forked version, which we really want to avoid.
cc @brndkfr @maximbaz-microsoft