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.

Sign up for GitHub

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

Jump to bottom

Implement merging of new key material when importing pubkeys #3083

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

Implement merging of new key material when importing pubkeys #3083

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4621abc
Fix error handling in rpmtsImportPubkey
mlschroe May 7, 2024
0872648
Make subkeys available right away when a pubkey is imported
mlschroe May 7, 2024
5926604
Split subkey initialization into its own function
mlschroe May 7, 2024
2292bb2
Add rpmKeyringModify to change the keys of a keyring
mlschroe May 7, 2024
8ac10de
Add a method to lookup a key from the keyring
mlschroe May 7, 2024
885a3ab
Support pubkey merging in the keyring code
mlschroe May 7, 2024
028d84b
Merge keys and update the database in rpmtsImportPubkey()
mlschroe May 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Prev Previous commit
Next Next commit
Add rpmKeyringModify to change the keys of a keyring 
This allows us to replace or delete keys from the keyring.
  • Loading branch information
@mlschroe
mlschroe committed May 8, 2024
commit 2292bb23715074743a3264b4f9e89d8e1bd21f3e
19 changes: 19 additions & 0 deletions include/rpm/rpmkeyring.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,16 @@
extern "C" {
#endif

/** \ingroup rpmkeyring
* Operation mode definitions for rpmKeyringModify
*/
typedef enum rpmKeyringModifyMode_e {
RPMKEYRING_ADD = 1,
RPMKEYRING_REPLACE = 2,
RPMKEYRING_DELETE = 3
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please explain the semantics of these different operations. My feeling is that there are only two relevant operations: ADD_OR_MERGE and DELETE. Does your ADD fail if the certificate is already present? Does REPLACE do a merge or not (looking at the code, it doesn't appear to)? If it doesn't, why not? (What's the use case?) Doesn't not merging mean that you potentially lose old binding signatures? That seems problematic to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is about if you do the merge in the keyring code or in the import code. I played with both options and then settled in doing the merge call in the import code and then replacing the old pubkey with the merged pubkey.

It's up to the merge code if it wants to keep all binding signatures. The rpmpgp_legacy merge code does not throw away any pgp package.

} rpmKeyringModifyMode;


/** \ingroup rpmkeyring
* Create a new, empty keyring
* @return new keyring handle
Expand Down Expand Up @@ -101,6 +111,15 @@ char * rpmPubkeyBase64(rpmPubkey key);
*/
pgpDigParams rpmPubkeyPgpDigParams(rpmPubkey key);

/** \ingroup rpmkeyring
* Modify the keys in the keyring
* @param key Pubkey
* @param key pubkey handle
* @param mode mode of operation
* @return 0 on success, -1 on error, 1 if key already present (add) or not found (delete)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please make this comment a bit more verbose, please. I get the idea, but the semantics aren't entirely clear to me.

*/
int rpmKeyringModify(rpmKeyring keyring, rpmPubkey key, rpmKeyringModifyMode mode);

#ifdef __cplusplus
}
#endif
Expand Down
20 changes: 18 additions & 2 deletions rpmio/rpmkeyring.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,26 @@ rpmKeyring rpmKeyringFree(rpmKeyring keyring)
return NULL;
}

int rpmKeyringAddKey(rpmKeyring keyring, rpmPubkey key)
int rpmKeyringModify(rpmKeyring keyring, rpmPubkey key, rpmKeyringModifyMode mode)
{
int rc = 1; /* assume already seen key */
if (keyring == NULL || key == NULL)
return -1;
if (mode != RPMKEYRING_ADD && mode != RPMKEYRING_DELETE && mode != RPMKEYRING_REPLACE)
return -1;

/* check if we already have this key, but always wrlock for simplicity */
pthread_rwlock_wrlock(&keyring->lock);
if (keyring->keys.find(key->keyid) == keyring->keys.end()) {
auto item = keyring->keys.find(key->keyid);
if (item != keyring->keys.end() && mode == RPMKEYRING_DELETE) {
rpmPubkeyFree(item->second);
keyring->keys.erase(item);
rc = 0;
} else if (item != keyring->keys.end() && mode == RPMKEYRING_REPLACE) {
rpmPubkeyFree(item->second);
item->second = rpmPubkeyLink(key);
rc = 0;
} else if (item == keyring->keys.end() && (mode == RPMKEYRING_ADD ||mode == RPMKEYRING_REPLACE) ) {
keyring->keys.insert({key->keyid, rpmPubkeyLink(key)});
rc = 0;
}
Expand All @@ -78,6 +89,11 @@ int rpmKeyringAddKey(rpmKeyring keyring, rpmPubkey key)
return rc;
}

int rpmKeyringAddKey(rpmKeyring keyring, rpmPubkey key)
{
return rpmKeyringModify(keyring, key, RPMKEYRING_ADD);
}

rpmKeyring rpmKeyringLink(rpmKeyring keyring)
{
if (keyring) {
Expand Down