Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix keylist pr 163 #511

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

arozyev
Copy link
Collaborator

@arozyev arozyev commented Oct 11, 2023

This change fixes merging conflicts and add few style/code issues from PR#163

@arozyev
Copy link
Collaborator Author

arozyev commented Oct 11, 2023

Some more thoughts, suggestions and comments from @mlandres:

Revival of the keys command. That's a good idea. A few thoughts...

  • The commands should IMO not only be able to maintain the trusted keys in the rpmdb. It should also be able to tell which keys are in a given file or in the files in the standard search location (/var/cache/zypp/pubkeys, /usr/lib/rpm/gnupg/keys).

  • Keep in mind that 'a file' could contain multiple keys. We should IMO always import keys into the rpmdb. An import should accept a 'file' IFF it contains one key. Otherwise explicit 'KEYID from file' or 'ALL from file'. Or just KEYID from a file we try to find in the standard locations.

  • It should help to answer wich keys are used by which repos.

  • Maybe: Removing a trusted key from the rpmdb could place a copy in /var/cache/zypp/pubkeys, so one can re-import it in case of an accidental removal.
    If /var/cache/zypp/pubkeys becomes visible (and maintainable; i.e. remove Keys from this dir), we should make sure that we do not auto expire files in there. AFAIR we implemented something like this somewhere)

@mlandres
Copy link
Member

Regarding the command description

*keys* (*lk*) ['options'] ['part-of-keyid-or-name'] ['key-filename'] ...::
  List all trusted keys or show detailed information about those specified as arguments,
  supports also keyfiles as argument.

*addkey* (*ak*) ['options'] 'url'...::
  Imports a new key into the trusted database, the file can be specified as a URL.

*removekey* (*rk*) ['options'] 'string' ::
  Remove specified key from the trusted database. The query string can be the beginning
  or end of the key ID, or any part of the key name.

I'd suggest to be more straight in how we build and describe the CLI. It should be well defined what role an argument should play on the commandline.

A keyID (longID/FPR) is expected to uniquely denote A GPGKEY (class zypp::PublicKey).

In zypp you'd call it a string id_r for which PublicKey::providesKey(id_r) && PublicKey::isSafeKeyId(id_r) returns true. Less technically it's the at least 16 byte long trailing portion of the fingerprint of the GPGKEY's primary key or one of it's subkeys. providesKey alone would also accept the exact 8 byte trailing portion (shortID) of the primary key's FPR for legacy reasons. That's at least how the code treats keyIDs.

The 'rule' to reject files with more than one key is just because people may not be aware that a file gpg-pubkey-7fac5991-4615767f.key may in fact contain an arbitrary number of keys. We had examples where this is the case. even in out own distros. OTOH tools like SUMA may use this to ship additional keys in case the repo has packages signed by keys others than the metadata signing key. We could add a specific option for this (key-from vs keys-file).

Common to all key commands might be: They refer to a pool of GPGKEYs, optionally use just a subset of it and perform some action with them. Just a draft:

command [options] [keyPool...] [keyFilter...]

list
  keyPool:
    --key-from  FILE    requiring FILE to contain just a singe key 
                        ( maybe list but warn and return !=0 )
    --keys-file FILE	all keys in FILE
    --trusted-keys	all trusted keys (in rpmdb)
    --available-keys    all keys in /var/cache/zypp/pubkeys, ...
    [default: --trusted-keys]
  keyFilter:
    keyID		key with ID
    --match NAME        keys matching NAME
    --all-matches	all keys in keyPool
    [default: --all-matches]

add
  keyPool:
    --key-from  FILE    requiring FILE to contain just a singe key
                        ( abort if not )
    --keys-file FILE	all keys in FILE
    --available-keys    all keys in /var/cache/zypp/pubkeys, ...
    [default: --available-keys]
  keyFilter:
    keyID		key with ID
    --match NAME        keys matching NAME
    --all-matches       all keys in keyPool
    [default: This is tricky...]
    I'd default to --all-matches unless --available-keys is active (explicit or default).
    In this case a filter must be specified.
    
remove
  keyPool:
    [fidxed: --trusted-keys]
  keyFilter:
    keyID		key with ID
    --match NAME        keys matching NAME
    --all-matches	all keys in keyPool
    [default: none]

For remove we could later think about whether to allow keyPool specs (key-from, keys-file) to remove the keys the files provide. Also a [not]-used-by-repos filter could be useful. Maybe a --repo ALIAS keyPool too. We just need to think about how we handle and communicate the case of missing or not up-to-data repo metadata.

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.

4 participants