Skip to content

Allow GnuPG 2 users to export keys with and without passphrase#59376

Merged
Ch3LL merged 11 commits intosaltstack:masterfrom
piterpunk:export-gpg-key-with-and-without-passphrase_Fix-Issue-58980
Mar 5, 2021
Merged

Allow GnuPG 2 users to export keys with and without passphrase#59376
Ch3LL merged 11 commits intosaltstack:masterfrom
piterpunk:export-gpg-key-with-and-without-passphrase_Fix-Issue-58980

Conversation

@piterpunk
Copy link

@piterpunk piterpunk commented Jan 29, 2021

What does this PR do?

Added the use_passphrase parameter. This allows to export keys with a passphrase. The passphrase itself comes from pillar gpg_passphrase.
Added the expect_passphrase = False to Python GnuPG's export_keys call when the use_passphrase is False (default). That allows the export of passwordless keys.

What issues does this PR fix or reference?

Fixes: #58980

Previous Behavior

An error when calling gpg.export_key, see the issue #58980
Also, there was no support to use a passphrase.

New Behavior

It's now possible to export a key with and without a passphrase.

Merge requirements satisfied?

Commits signed with GPG?

No

@piterpunk piterpunk requested a review from a team as a code owner January 29, 2021 05:51
@piterpunk piterpunk requested review from xeacott and removed request for a team January 29, 2021 05:51
@piterpunk
Copy link
Author

All unit tests for gpg execution module are skipped. Some are marked as slow and the others as destructive. I'll try to update the tests soon. But in a clean environment, the "destructive" looks scary.

@bryceml
Copy link
Contributor

bryceml commented Feb 4, 2021

If you can get this working on centos/rhel 8, that would be great, they seem to have tried to remove all methods of programmatically entering passphrases.

It would be great for this to work in conjunction with the pkgbuild.make_repo as well in centos 8.

Another issue I've seen is requiring a tty to work, if this could take care of that too, that would be pretty cool.

@piterpunk
Copy link
Author

If you can get this working on centos/rhel 8, that would be great, they seem to have tried to remove all methods of programmatically entering passphrases.
Another issue I've seen is requiring a tty to work, if this could take care of that too, that would be pretty cool.

Let's see what I can do. I still need to write the tests (which usually take a lot of time). Then I will test in other platforms, including CentOS 8.

@piterpunk
Copy link
Author

@bryceml ,

Ok, now I have the gpg module working Ok for CentOS 8. Needed to change a bit the create_key and delete_key, in addition of the changes in export_key done in the first commit.

Can you check if this covers your usecases?

Still need to update the unit tests.

@sagetherage sagetherage added the Aluminium Release Post Mg and Pre Si label Feb 10, 2021
@piterpunk piterpunk force-pushed the export-gpg-key-with-and-without-passphrase_Fix-Issue-58980 branch from ed2bc06 to cfc1ce1 Compare February 22, 2021 07:06
@piterpunk piterpunk changed the title [WIP] Allow GnuPG 2 users to export keys with and without passphrase Allow GnuPG 2 users to export keys with and without passphrase Feb 22, 2021
@piterpunk piterpunk force-pushed the export-gpg-key-with-and-without-passphrase_Fix-Issue-58980 branch from df68e19 to a2d8b08 Compare February 22, 2021 17:39
@piterpunk
Copy link
Author

Ok first the tests fails in all Debian derived platforms, then I re-write two tests to fix this but forgot to test one of them, so a new commit to fix the broked test, after that it seems all tests run Ok, except for ubuntu1804/pytest, but it did break in what I guess is an unrelated test in salt-proxy.

Did a merge with master and finally it pass all tests. Please @xeacott, can you take a look if this PR is Ok while it's all green?

@piterpunk piterpunk force-pushed the export-gpg-key-with-and-without-passphrase_Fix-Issue-58980 branch 2 times, most recently from 2cb6c38 to 78c82aa Compare February 24, 2021 13:12
@piterpunk piterpunk requested a review from Ch3LL February 24, 2021 15:11
twangboy
twangboy previously approved these changes Mar 1, 2021
@piterpunk piterpunk force-pushed the export-gpg-key-with-and-without-passphrase_Fix-Issue-58980 branch from ea74504 to 812c486 Compare March 1, 2021 22:51
@piterpunk piterpunk requested a review from dwoz March 1, 2021 22:53
dwoz
dwoz previously approved these changes Mar 3, 2021
@piterpunk piterpunk requested a review from twangboy March 3, 2021 22:01
Ch3LL
Ch3LL previously approved these changes Mar 4, 2021
Copy link
Contributor

@s0undt3ch s0undt3ch left a comment

Choose a reason for hiding this comment

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

All the tests that were decorated as destructive, if they change temp files or paths, they are not destructive, temp resources are meant to be "destroyable".

The slow decorator. why are these unit tests marked as slow?

@piterpunk piterpunk dismissed stale reviews from Ch3LL and dwoz via 0ff17f6 March 4, 2021 13:44
Copy link
Contributor

@s0undt3ch s0undt3ch left a comment

Choose a reason for hiding this comment

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

I think this way the docs will build properly

@s0undt3ch
Copy link
Contributor

And lets also remove the destructive decorator

@piterpunk
Copy link
Author

And lets also remove the destructive decorator

Ok, by night I can do the changes, test locally and then update this feature branch.

piterpunk added 10 commits March 4, 2021 18:38
- Calls gpg.export_keys with expect_passphrase = False to allow the
  export of keys without passphrase
- Added support to export keys with a passphrase.
- Behaviour controlled by parameter use_passphrase, defaulted to False
- Added parameter use_passphrase to delete_key
- Added _gen_key_input to send %no-protection command to gen_key
- Added tests for new _gen_key_input function, used by create_key
- Added tests for export_key
- Updated tests for delete_key
- Removed the _gen_key_input function
- Added code to add '%no-protection' independent of python-gnupg version
- Updated the tests to test directly the gpg.create_key
The create_key tests are entropy dependent and can wait for a long
time to have enough of it in the automated tests environment.
- Added versionadded information to use_passphrase argument in
  delete_key and export_key
- Unmarked the tests that only writes at temporary files as destructive
@piterpunk piterpunk force-pushed the export-gpg-key-with-and-without-passphrase_Fix-Issue-58980 branch from 3111633 to 3d9d3ca Compare March 4, 2021 23:16
@piterpunk
Copy link
Author

@s0undt3ch , just did the required changes. I hope it's now Ok.

@piterpunk piterpunk requested a review from s0undt3ch March 5, 2021 01:27
@s0undt3ch s0undt3ch requested a review from krionbsd March 5, 2021 11:50
@Ch3LL Ch3LL merged commit 2b81f5b into saltstack:master Mar 5, 2021
@piterpunk piterpunk deleted the export-gpg-key-with-and-without-passphrase_Fix-Issue-58980 branch March 8, 2021 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Aluminium Release Post Mg and Pre Si

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]salt.modules.gpg.export_key: exporting secret keys not possible due to missing passphrase

8 participants