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

Pass rpmPubkey instance to rpmtxnDeletePubkey #3374

Merged
merged 4 commits into from
Oct 15, 2024
Merged

Pass rpmPubkey instance to rpmtxnDeletePubkey #3374

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7928d99
rpmkeys: Match error message of --list to --delete
ffesti Oct 14, 2024
2da4d23
Make rpmkeys --list accept short key IDs
ffesti Oct 14, 2024
52c675f
Improve matchingKeys
ffesti Oct 14, 2024
662c231
Pass rpmPubkey instance to rpmtxnDeletePubkey
ffesti Oct 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions include/rpm/rpmts.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <rpm/rpmsw.h>
#include <rpm/rpmfi.h>
#include <rpm/rpmcallback.h>
#include <rpm/rpmkeyring.h>

#ifdef __cplusplus
extern "C" {
Expand Down Expand Up @@ -353,13 +354,12 @@ rpmRC rpmtxnImportPubkey(rpmtxn txn, const unsigned char * pkt, size_t pktlen);
/** \ingroup rpmts
* Delete public key from transaction keystore.
* @param txn transaction handle
* @param keyid key fingerprint or keyid (in hex)
* @param key public key
* @return RPMRC_OK on success
* RPMRC_NOTFOUND if key not found
* RPMRC_NOKEY on invalid keyid
* RPMRC_FAIL on other failure
*/
rpmRC rpmtxnDeletePubkey(rpmtxn txn, const char *keyid);
rpmRC rpmtxnDeletePubkey(rpmtxn txn, rpmPubkey key);
Copy link
Member

Choose a reason for hiding this comment

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

I'm okay with adding an alternative API for this that takes a pubkey, and I'm okay with using this name for it even, but don't remove the version that takes a fingerprint. I explained why in the PR that added it.

Copy link
Member

@pmatilai pmatilai Oct 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, scratch that.

Going forward, it is a requirement for rpmkeys (and the library part) to be able to delete keys without being able to construct a pubkey out of it. I've seen enough crypto related failures in the last few years that this is just non-negotiable. But, with what we have now, we have to load it into the keyring first anyhow. So okay for changing the argument to rpmPubkey, we'll add another interface around the lower-level keystore later, once we have that and an iterator for the contents.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, this here is the minimal approach. It keeps the old function as rpmtxnDeletePubkeyByID. We can smush this into a different shape once we now what to do in the backend.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, but see #3375 (comment) - I drop my case on the fingerprint/keyid API - been insisting on it for what is a wrong reason.


/** \ingroup rpmts
* Retrieve handle for keyring used for this transaction set
Expand Down
16 changes: 5 additions & 11 deletions lib/rpmts.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -774,17 +774,10 @@ rpmRC rpmtxnImportPubkey(rpmtxn txn, const unsigned char * pkt, size_t pktlen)
return rc;
}

rpmRC rpmtxnDeletePubkey(rpmtxn txn, const char *keyid)
rpmRC rpmtxnDeletePubkey(rpmtxn txn, rpmPubkey key)
{
rpmRC rc = RPMRC_FAIL;
size_t klen = strlen(keyid);

/* Allow short keyid while we're transitioning */
if (klen != 40 && klen != 16 && klen != 8)
return RPMRC_NOKEY;

if (!rpmIsValidHex(keyid, klen))
return RPMRC_NOKEY;
char * keyid = rpmPubkeyKeyIDAsHex(key);

if (txn) {
/* force keyring load */
Expand All @@ -797,12 +790,13 @@ rpmRC rpmtxnDeletePubkey(rpmtxn txn, const char *keyid)
rc = RPMRC_OK;
if (!(rpmtsFlags(txn->ts) & RPMTRANS_FLAG_TEST)) {
if (txn->ts->keyringtype == KEYRING_FS)
rc = rpmtsDeleteFSKey(txn, keyid);
rc = rpmtsDeleteFSKey(txn, keyid+8);
else
rc = rpmtsDeleteDBKey(txn, keyid);
rc = rpmtsDeleteDBKey(txn, keyid+8);
}
rpmKeyringFree(keyring);
}
free(keyid);
return rc;
}

Expand Down
6 changes: 3 additions & 3 deletions tests/rpmdb.at
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,9 @@ RPMTEST_CHECK([
runroot rpmkeys --list XXX
],
[1],
[Key XXX not found
],
[])
[],
[error: invalid key id: XXX
])

RPMTEST_CHECK([
runroot rpmkeys --delete 1964c5fc
Expand Down
4 changes: 2 additions & 2 deletions tests/rpmsigdig.at
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ runroot rpmkeys -Kv /data/RPMS/hello-2.0-1.x86_64-signed-with-new-subkey.rpm
RPMTEST_CHECK([
runroot rpmkeys --delete abcd gimmekey 1111aaaa2222bbbb
],
[3],
[1],
[],
[error: invalid key id: abcd
error: invalid key id: gimmekey
Expand Down Expand Up @@ -147,7 +147,7 @@ runroot rpmkeys -Kv /data/RPMS/hello-2.0-1.x86_64-signed-with-new-subkey.rpm
RPMTEST_CHECK([
runroot rpmkeys --delete abcd gimmekey 1111aaaa2222bbbb
],
[3],
[1],
[],
[error: invalid key id: abcd
error: invalid key id: gimmekey
Expand Down
58 changes: 37 additions & 21 deletions tools/rpmkeys.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,42 @@ static struct poptOption optionsTable[] = {
POPT_TABLEEND
};

static int matchingKeys(rpmKeyring keyring, ARGV_const_t args, void * userdata, int callback(rpmPubkey, void*))
static int matchingKeys(rpmts ts, ARGV_const_t args, int callback(rpmPubkey, void*), void * userdata = NULL)
{
int ec = EXIT_SUCCESS;
rpmKeyring keyring = rpmtsGetKeyring(ts, 1);
char * c;

if (args) {
for (char * const * arg = args; *arg; arg++) {
int found = false;
size_t klen = strlen(*arg);

/* Allow short keyid while we're transitioning */
if (klen != 40 && klen != 16 && klen != 8) {
rpmlog(RPMLOG_ERR, ("invalid key id: %s\n"), *arg);
ec = EXIT_FAILURE;
continue;
}

/* Check for valid hex chars */
for (c=*arg; *c; c++) {
if (strchr("0123456789abcdefABCDEF", *c) == NULL)
break;
}
if (*c) {
rpmlog(RPMLOG_ERR, ("invalid key id: %s\n"), *arg);
ec = EXIT_FAILURE;
continue;
}

auto iter = rpmKeyringInitIterator(keyring, 0);
rpmPubkey key = NULL;
while ((key = rpmKeyringIteratorNext(iter))) {
char * fp = rpmPubkeyFingerprintAsHex(key);
char * keyid = rpmPubkeyKeyIDAsHex(key);
if (!strcmp(*arg, fp) || !strcmp(*arg, keyid)) {
if (!strcmp(*arg, fp) || !strcmp(*arg, keyid) ||
!strcmp(*arg, keyid+8)) {
found = true;
}
free(fp);
Expand All @@ -67,7 +91,7 @@ static int matchingKeys(rpmKeyring keyring, ARGV_const_t args, void * userdata,
if (found) {
callback(key, userdata);
} else {
rpmlog(RPMLOG_NOTICE, "Key %s not found\n", *arg);
rpmlog(RPMLOG_ERR, ("key not found: %s\n"), *arg);
ec = EXIT_FAILURE;
}
}
Expand All @@ -85,6 +109,7 @@ static int matchingKeys(rpmKeyring keyring, ARGV_const_t args, void * userdata,
ec = EXIT_FAILURE;
}
}
rpmKeyringFree(keyring);
return ec;
}

Expand All @@ -97,13 +122,19 @@ static int printKey(rpmPubkey key, void * data)
return 0;
}

static int deleteKey(rpmPubkey key, void * data)
{
rpmtxn txn = (rpmtxn) data;
rpmtxnDeletePubkey(txn, key);
return 0;
}

int main(int argc, char *argv[])
{
int ec = EXIT_FAILURE;
poptContext optCon = NULL;
rpmts ts = NULL;
ARGV_const_t args = NULL;
rpmKeyring keyring = NULL;

optCon = rpmcliInit(argc, argv, optionsTable);

Expand All @@ -119,7 +150,6 @@ int main(int argc, char *argv[])

ts = rpmtsCreate();
rpmtsSetRootDir(ts, rpmcliRootDir);
keyring = rpmtsGetKeyring(ts, 1);

switch (mode) {
case MODE_CHECKSIG:
Expand All @@ -134,35 +164,21 @@ int main(int argc, char *argv[])
{
rpmtxn txn = rpmtxnBegin(ts, RPMTXN_WRITE);
if (txn) {
int nfail = 0;
for (char const * const *arg = args; *arg && **arg; arg++) {
rpmRC delrc = rpmtxnDeletePubkey(txn, *arg);
if (delrc) {
if (delrc == RPMRC_NOTFOUND)
rpmlog(RPMLOG_ERR, ("key not found: %s\n"), *arg);
else if (delrc == RPMRC_NOKEY)
rpmlog(RPMLOG_ERR, ("invalid key id: %s\n"), *arg);
else if (delrc == RPMRC_FAIL)
rpmlog(RPMLOG_ERR, ("failed to delete key: %s\n"), *arg);
nfail++;
}
}
ec = nfail;
ec = matchingKeys(ts, args, deleteKey, txn);
rpmtxnEnd(txn);
}
break;
}
case MODE_LISTKEY:
{
ec = matchingKeys(keyring, args, NULL, printKey);
ec = matchingKeys(ts, args, printKey);
break;
}
default:
argerror(_("only one major mode may be specified"));
}

exit:
rpmKeyringFree(keyring);
rpmtsFree(ts);
rpmcliFini(optCon);
fflush(stderr);
Expand Down
Loading