Allow auto import only for specific key #421
Conversation
Implemented a new command option "--gpg-auto-import-key-id" which allows users to only auto-import a GPG key with the provided key ID.
src/Config.cc
Outdated
| @@ -570,6 +571,18 @@ std::vector<ZyppFlags::CommandGroup> Config::cliOptions() | |||
| // translators: --gpg-auto-import-keys | |||
| _("Automatically trust and import new repository signing keys.") | |||
| }, | |||
| { "gpg-auto-import-key-id", 0, ZyppFlags::RequiredArgument, std::move( ZyppFlags::StringType( &gpg_auto_import_key_id, "", "KEY_ID"). | |||
There was a problem hiding this comment.
It's IMO debatable whether a new option is introduced or gpg-auto-import-keys is amended to take an optional argument. In any case a key or ,-separated list of keys should be accepted.
There was a problem hiding this comment.
Great, I will make sure to accept comma separated keys as well.
I also like your suggestion of amending gpg-auto-import-keys rather than adding a new parameter. Do you want to open up that conversation to other stakeholders, or can we move forward with the single parameter implementation?
There was a problem hiding this comment.
We can move on. Staying with gpg-auto-import-key and amending it to
- take an optional argument (comma separated list of IDs)
- allow multiple occurrences of
gpg-auto-import-key
should be feasible for the argparser .
This enables the user to use --gpg-auto-import-key=A,B or --gpg-auto-import-key=A --gpg-auto-import-key=B as they like it.
It also allows to solve the issue of --gpg-auto-import-key ... --gpg-auto-import-key=A in a natural way. No errors about conflicting options are needed. The options build up a set of key IDs. An empty set accepts all keys as it currently does. Otherwise only the keys in the set are auto-accepted. Keys not in the set are treated as usual.
(@bzeller JFYI)
There was a problem hiding this comment.
In order to make gpg-auto-import-keys optional, while still accepting comma separated lists of IDs as you suggest in your comment here, I had to make a small adjustment to the GenericContainerType definition in the argparser. If there was a better way to go about achieving this, let me know and I would be happy to make adjustments if necessary.
The change to GenericContainerType is in its own commit.
src/callbacks/keyring.h
Outdated
| if (_gopts.gpg_auto_import_keys) | ||
| // if --gpg-auto-import-keys, --no-gpg-check, or --gpg-auto-import-key-id matches key, print info and don't ask. | ||
| if (_gopts.gpg_auto_import_keys | ||
| || (!_gopts.gpg_auto_import_key_id.empty() && _gopts.gpg_auto_import_key_id == key_r.id())) |
There was a problem hiding this comment.
_gopts.gpg_auto_import_key_id == key_r.id() IMO is to strict.
The argument should be ID or FPR (recommended 16byte/40byte). As convenience a short-ID (8byte insecure) could be accepted. This would be key_r.providesKey( _gopts.gpg_auto_import_key_id ).
At least the manpage should state that the short-ID is considered to be insecure. Optionally the argparser could print a note/warning if a ! PublicKeyData::isSafeKeyId( _gopts.gpg_auto_import_key_id ) is passed.
dwymark
changed the title
This helps to keep the code style consistent with the rest of the code base.
Implemented a new command option "--gpg-auto-import-key-id" which allows users to only auto-import a GPG key with the provided key ID. This was done in response to a feature request in issue 401.