Skip to content

Commit

Permalink
Make keystore read protected by transaction lock too
Browse files Browse the repository at this point in the history
Take read-only transaction lock at the rpmts side, pass it down the
rpmKeystoreLoad() hatch.

Somehow this feels like an overkill but then our keystores haven't
been designed with concurrent access in mind *at all*, so play safe
and just take a read lock while loading they keystore. It also balances
the API: everything just takes a txn item now.

Related: #3342
  • Loading branch information
pmatilai authored and ffesti committed Oct 28, 2024
1 parent c680d5e commit 79b26f1
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 9 deletions.
15 changes: 8 additions & 7 deletions lib/keystore.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ using std::string;

static int makePubkeyHeader(rpmts ts, rpmPubkey key, Header * hdrp);

static int rpmtsLoadKeyringFromFiles(rpmts ts, rpmKeyring keyring)
static int rpmtsLoadKeyringFromFiles(rpmtxn txn, rpmKeyring keyring)
{
ARGV_t files = NULL;
/* XXX TODO: deal with chroot path issues */
char *pkpath = rpmGetPath(ts->rootDir, "%{_keyringpath}/*.key", NULL);
char *pkpath = rpmGetPath(rpmtxnRootDir(txn), "%{_keyringpath}/*.key", NULL);
int nkeys = 0;

rpmlog(RPMLOG_DEBUG, "loading keyring from pubkeys in %s\n", pkpath);
Expand Down Expand Up @@ -133,14 +133,14 @@ static rpmRC rpmtsImportFSKey(rpmtxn txn, rpmPubkey key, rpmFlags flags, int rep
return rc;
}

static int rpmtsLoadKeyringFromDB(rpmts ts, rpmKeyring keyring)
static int rpmtsLoadKeyringFromDB(rpmtxn txn, rpmKeyring keyring)
{
Header h;
rpmdbMatchIterator mi;
int nkeys = 0;

rpmlog(RPMLOG_DEBUG, "loading keyring from rpmdb\n");
mi = rpmtsInitIterator(ts, RPMDBI_NAME, "gpg-pubkey", 0);
mi = rpmtsInitIterator(rpmtxnTs(txn), RPMDBI_NAME, "gpg-pubkey", 0);
while ((h = rpmdbNextIterator(mi)) != NULL) {
struct rpmtd_s pubkeys;
const char *key;
Expand Down Expand Up @@ -405,13 +405,14 @@ rpmRC rpmKeystoreDeletePubkey(rpmtxn txn, rpmPubkey key)
return rc;
}

int rpmKeystoreLoad(rpmts ts, rpmKeyring keyring)
int rpmKeystoreLoad(rpmtxn txn, rpmKeyring keyring)
{
int nkeys = 0;
rpmts ts = rpmtxnTs(txn);
if (ts->keyringtype == KEYRING_FS) {
nkeys = rpmtsLoadKeyringFromFiles(ts, keyring);
nkeys = rpmtsLoadKeyringFromFiles(txn, keyring);
} else {
nkeys = rpmtsLoadKeyringFromDB(ts, keyring);
nkeys = rpmtsLoadKeyringFromDB(txn, keyring);
}
return nkeys;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/keystore.hh
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ enum {
};

RPM_GNUC_INTERNAL
int rpmKeystoreLoad(rpmts ts, rpmKeyring keyring);
int rpmKeystoreLoad(rpmtxn txn, rpmKeyring keyring);

RPM_GNUC_INTERNAL
rpmRC rpmKeystoreImportPubkey(rpmtxn txn, rpmPubkey key, int replace = 0);
Expand Down
6 changes: 5 additions & 1 deletion lib/rpmts.cc
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,11 @@ static void loadKeyring(rpmts ts)
if (!ts->keyringtype)
ts->keyringtype = getKeyringType();
ts->keyring = rpmKeyringNew();
rpmKeystoreLoad(ts, ts->keyring);
rpmtxn txn = rpmtxnBegin(ts, RPMTXN_READ);
if (txn) {
rpmKeystoreLoad(txn, ts->keyring);
rpmtxnEnd(txn);
}
}
}

Expand Down

0 comments on commit 79b26f1

Please sign in to comment.