From 5ee40a5fe860a11bd49b0b527cd4f1b87aac364e Mon Sep 17 00:00:00 2001 From: Florian Festi Date: Thu, 31 Oct 2024 10:43:01 +0100 Subject: [PATCH 1/2] Refactor s backend into shared helper functions This will be useful for the next commit. This still is a mess of C and C++ style strings that we want to clean up later by adding C++ string based path handling and may be using the filesystem C++ library. Related: #3341 --- lib/keystore.cc | 95 +++++++++++++++++++++++++++++-------------------- 1 file changed, 56 insertions(+), 39 deletions(-) diff --git a/lib/keystore.cc b/lib/keystore.cc index a3f7f1b777..9037af8112 100644 --- a/lib/keystore.cc +++ b/lib/keystore.cc @@ -24,11 +24,11 @@ using namespace rpm; static int makePubkeyHeader(rpmts ts, rpmPubkey key, Header * hdrp); -rpmRC keystore_fs::load_keys(rpmtxn txn, rpmKeyring keyring) +static rpmRC load_keys_from_glob(rpmtxn txn, rpmKeyring keyring, string glob) { ARGV_t files = NULL; /* XXX TODO: deal with chroot path issues */ - char *pkpath = rpmGetPath(rpmtxnRootDir(txn), "%{_keyringpath}/*.key", NULL); + char *pkpath = rpmGetPath(rpmtxnRootDir(txn), glob.c_str(), NULL); rpmlog(RPMLOG_DEBUG, "loading keyring from pubkeys in %s\n", pkpath); if (rpmGlob(pkpath, NULL, &files)) { @@ -55,6 +55,53 @@ rpmRC keystore_fs::load_keys(rpmtxn txn, rpmKeyring keyring) return RPMRC_OK; } +static rpmRC write_key_to_disk(rpmPubkey key, string & dir, string & filename, int replace, rpmFlags flags) +{ + rpmRC rc = RPMRC_FAIL; + char *keyval = rpmPubkeyArmorWrap(key); + string tmppath = ""; + string path = dir + "/" + filename; + + if (replace) { + tmppath = dir + "/" + filename + ".new"; + unlink(tmppath.c_str()); + } + + if (rpmMkdirs(NULL, dir.c_str())) { + rpmlog(RPMLOG_ERR, _("failed to create keyring directory %s: %s\n"), + path.c_str(), strerror(errno)); + goto exit; + } + + if (FD_t fd = Fopen(replace ? tmppath.c_str() : path.c_str(), "wx")) { + size_t keylen = strlen(keyval); + if (Fwrite(keyval, 1, keylen, fd) == keylen) + rc = RPMRC_OK; + Fclose(fd); + } + if (!rc && replace && rename(tmppath.c_str(), path.c_str()) != 0) + rc = RPMRC_FAIL; + + if (rc) { + rpmlog(RPMLOG_ERR, _("failed to import key: %s: %s\n"), + replace ? tmppath.c_str() : path.c_str(), strerror(errno)); + if (replace) + unlink(tmppath.c_str()); + } + +exit: + free(keyval); + return rc; +} + + +/*****************************************************************************/ + +rpmRC keystore_fs::load_keys(rpmtxn txn, rpmKeyring keyring) +{ + return load_keys_from_glob(txn, keyring, "%{_keyringpath}/*.key"); +} + rpmRC keystore_fs::delete_key(rpmtxn txn, const string & keyid, const string & newname) { rpmRC rc = RPMRC_NOTFOUND; @@ -82,52 +129,22 @@ rpmRC keystore_fs::delete_key(rpmtxn txn, rpmPubkey key) rpmRC keystore_fs::import_key(rpmtxn txn, rpmPubkey key, int replace, rpmFlags flags) { rpmRC rc = RPMRC_FAIL; - const char *fp = rpmPubkeyFingerprintAsHex(key); - char *keyfmt = rstrscat(NULL, "gpg-pubkey-", fp, ".key", NULL); - char *keyval = rpmPubkeyArmorWrap(key); - char *path = rpmGenPath(rpmtxnRootDir(txn), "%{_keyringpath}/", keyfmt); - char *tmppath = NULL; - - if (replace) { - rasprintf(&tmppath, "%s.new", path); - unlink(tmppath); - } + string fp = rpmPubkeyFingerprintAsHex(key); + string keyfmt = "gpg-pubkey-" + fp + ".key"; + char *dir = rpmGenPath(rpmtxnRootDir(txn), "%{_keyringpath}/", NULL); + string dirstr = dir; - if (rpmMkdirs(rpmtxnRootDir(txn), "%{_keyringpath}")) { - rpmlog(RPMLOG_ERR, _("failed to create keyring directory %s: %s\n"), - path, strerror(errno)); - goto exit; - } - - if (FD_t fd = Fopen(tmppath ? tmppath : path, "wx")) { - size_t keylen = strlen(keyval); - if (Fwrite(keyval, 1, keylen, fd) == keylen) - rc = RPMRC_OK; - Fclose(fd); - } - if (!rc && tmppath && rename(tmppath, path) != 0) - rc = RPMRC_FAIL; - - if (rc) { - rpmlog(RPMLOG_ERR, _("failed to import key: %s: %s\n"), - tmppath ? tmppath : path, strerror(errno)); - if (tmppath) - unlink(tmppath); - } + rc = write_key_to_disk(key, dirstr, keyfmt, replace, flags); if (!rc && replace) { /* find and delete the old pubkey entry */ if (delete_key(txn, fp, keyfmt) == RPMRC_NOTFOUND) { /* make sure an old, short keyid version gets removed */ - delete_key(txn, fp+32, keyfmt); + delete_key(txn, fp.substr(32), keyfmt); } } -exit: - free(path); - free(keyval); - free(keyfmt); - free(tmppath); + free(dir); return rc; } From 427e5f94e7aa35107bf3c474577fcef9f1c7b7b7 Mon Sep 17 00:00:00 2001 From: Florian Festi Date: Thu, 31 Oct 2024 10:43:01 +0100 Subject: [PATCH 2/2] Implement openpgp.cert.d based keystore This does implement the layout on the file system and the write lock of the openpgp.cert.d proposal according to https://www.ietf.org/archive/id/draft-nwjw-openpgp-cert-d-00.html but not the Trust root, Petname mapping or Trusted introducers. Resolves: #3341 --- lib/keystore.cc | 89 ++++++++++++++++++++++++++++++++++++++++++++++ lib/keystore.hh | 7 ++++ lib/rpmts.cc | 2 ++ tests/rpmsigdig.at | 77 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 175 insertions(+) diff --git a/lib/keystore.cc b/lib/keystore.cc index 9037af8112..da4e12e289 100644 --- a/lib/keystore.cc +++ b/lib/keystore.cc @@ -3,6 +3,8 @@ #include #include +#include +#include #include #include @@ -148,6 +150,93 @@ rpmRC keystore_fs::import_key(rpmtxn txn, rpmPubkey key, int replace, rpmFlags f return rc; } +/*****************************************************************************/ + +static int acquire_write_lock(rpmtxn txn) +{ + char * keyringpath = rpmGenPath(rpmtxnRootDir(txn), "%{_keyringpath}/", NULL); + char * lockpath = rpmGenPath(rpmtxnRootDir(txn), "%{_keyringpath}/", "writelock"); + int fd = -1; + + if (rpmMkdirs(NULL, keyringpath)) { + rpmlog(RPMLOG_ERR, _("failed to create keyring directory %s: %s\n"), + keyringpath, strerror(errno)); + goto exit; + } + + if ((fd = open(lockpath, O_WRONLY|O_CREAT)) == -1) { + rpmlog(RPMLOG_ERR, _("Can't create writelock for keyring at %s: %s\n"), keyringpath, strerror(errno)); + } else if (flock(fd, LOCK_EX|LOCK_NB)) { + rpmlog(RPMLOG_ERR, _("Can't acquire writelock for keyring at %s\n"), keyringpath); + close(fd); + fd = -1; + } + + exit: + free(keyringpath); + free(lockpath); + return fd; +} + +static void free_write_lock(int fd) +{ + flock(fd, LOCK_UN); +} + +rpmRC keystore_openpgp_cert_d::load_keys(rpmtxn txn, rpmKeyring keyring) +{ + return load_keys_from_glob(txn, keyring, "%{_keyringpath}/*/*"); +} + +rpmRC keystore_openpgp_cert_d::delete_key(rpmtxn txn, rpmPubkey key) +{ + rpmRC rc = RPMRC_NOTFOUND; + int lock_fd = -1; + + if ((lock_fd = acquire_write_lock(txn)) == -1) + return RPMRC_FAIL; + + string fp = rpmPubkeyFingerprintAsHex(key); + string dir = fp.substr(0, 2); + string filename = fp.substr(2); + char * filepath = rpmGetPath(rpmtxnRootDir(txn), "%{_keyringpath}/", dir.c_str(), "/", filename.c_str(), NULL); + char * dirpath = rpmGetPath(rpmtxnRootDir(txn), "%{_keyringpath}/", dir.c_str(), NULL); + + if (!access(filepath, F_OK)) + rc = unlink(filepath) ? RPMRC_FAIL : RPMRC_OK; + /* delete directory if empty */ + rmdir(dirpath); + + free(filepath); + free(dirpath); + free_write_lock(lock_fd); + return rc; +} + +rpmRC keystore_openpgp_cert_d::import_key(rpmtxn txn, rpmPubkey key, int replace, rpmFlags flags) +{ + rpmRC rc = RPMRC_NOTFOUND; + int lock_fd = -1; + + if ((lock_fd = acquire_write_lock(txn)) == -1) + return RPMRC_FAIL; + + string fp = rpmPubkeyFingerprintAsHex(key); + string dir = fp.substr(0, 2); + string filename = fp.substr(2); + char *dirpath = rpmGetPath(rpmtxnRootDir(txn), "%{_keyringpath}/", dir.c_str(), NULL); + string dirstr = dirpath; + + rc = write_key_to_disk(key, dirstr, filename, replace, flags); + + free_write_lock(lock_fd); + free(dirpath); + + return rc; +} + +/*****************************************************************************/ + rpmRC keystore_rpmdb::load_keys(rpmtxn txn, rpmKeyring keyring) { Header h; diff --git a/lib/keystore.hh b/lib/keystore.hh index 2511b5f95b..53fd53530a 100644 --- a/lib/keystore.hh +++ b/lib/keystore.hh @@ -37,6 +37,13 @@ private: rpmRC delete_key(rpmtxn txn, const std::string & keyid, unsigned int newinstance = 0); }; +class keystore_openpgp_cert_d : public keystore { +public: + virtual rpmRC load_keys(rpmtxn txn, rpmKeyring keyring); + virtual rpmRC import_key(rpmtxn txn, rpmPubkey key, int replace = 1, rpmFlags flags = 0); + virtual rpmRC delete_key(rpmtxn txn, rpmPubkey key); +}; + }; /* namespace */ #endif /* _KEYSTORE_H */ diff --git a/lib/rpmts.cc b/lib/rpmts.cc index 758b893354..e01ddf92f9 100644 --- a/lib/rpmts.cc +++ b/lib/rpmts.cc @@ -275,6 +275,8 @@ static keystore *getKeystore(rpmts ts) ts->keystore = new keystore_fs(); } else if (rstreq(krtype, "rpmdb")) { ts->keystore = new keystore_rpmdb(); + } else if (rstreq(krtype, "openpgp")) { + ts->keystore = new keystore_openpgp_cert_d(); } else { /* Fall back to using rpmdb if unknown, for now at least */ rpmlog(RPMLOG_WARNING, diff --git a/tests/rpmsigdig.at b/tests/rpmsigdig.at index ec6ad3a0b5..aa5eb95957 100644 --- a/tests/rpmsigdig.at +++ b/tests/rpmsigdig.at @@ -264,6 +264,83 @@ runroot rpmkeys -Kv /data/RPMS/hello-2.0-1.x86_64.rpm /data/RPMS/hello-1.0-1.i38 RPMTEST_CLEANUP +AT_SETUP([rpmkeys key update (openpgp)]) +AT_KEYWORDS([rpmkeys signature]) +RPMDB_INIT +# root's .rpmmacros used to keep this build prefix independent +echo "%_keyring openpgp" >> "${RPMTEST}"/root/.rpmmacros +RPMTEST_CHECK([ +runroot rpmkeys --import /data/keys/rpm.org-rsa-2048-test.pub +runroot rpmkeys -Kv /data/RPMS/hello-2.0-1.x86_64-signed-with-new-subkey.rpm +], +[1], +[/data/RPMS/hello-2.0-1.x86_64-signed-with-new-subkey.rpm: + Header OpenPGP V4 EdDSA/SHA512 signature, key ID 6323c42711450b6c: NOKEY + Header SHA256 digest: OK + Payload SHA256 digest: OK +], +[]) + +RPMTEST_CHECK([ +runroot_other touch /usr/lib/sysimage/rpm/pubkeys/writelock +runroot_other flock -x /usr/lib/sysimage/rpm/pubkeys/writelock -c "rpmkeys --import /data/keys/rpm.org-rsa-2048-add-subkey.asc" +runroot_other rm /usr/lib/sysimage/rpm/pubkeys/writelock +], +[0], +[], +[error: Can't acquire writelock for keyring at /usr/lib/sysimage/rpm/pubkeys +error: /data/keys/rpm.org-rsa-2048-add-subkey.asc: key 1 import failed. +]) + + +RPMTEST_CHECK([ +runroot rpmkeys --list | wc -l +runroot rpmkeys --import /data/keys/rpm.org-rsa-2048-add-subkey.asc +runroot rpmkeys --list | wc -l +], +[0], +[1 +1 +], +[]) + +RPMTEST_CHECK([ +runroot rpmkeys -Kv /data/RPMS/hello-2.0-1.x86_64-signed-with-new-subkey.rpm +], +[0], +[/data/RPMS/hello-2.0-1.x86_64-signed-with-new-subkey.rpm: + Header OpenPGP V4 EdDSA/SHA512 signature, key fingerprint: 771b18d3d7baa28734333c424344591e1964c5fc: OK + Header SHA256 digest: OK + Payload SHA256 digest: OK +], +[]) + +RPMTEST_CHECK([ +runroot rpmkeys --delete abcd gimmekey 1111aaaa2222bbbb +], +[1], +[], +[error: invalid key id: abcd +error: invalid key id: gimmekey +error: key not found: 1111aaaa2222bbbb +]) + +RPMTEST_CHECK([ +runroot rpmkeys --delete 1964c5fc +], +[0], +[], +[]) + +RPMTEST_CHECK([ +runroot rpmkeys --list | wc -l +], +[0], +[0 +], +[]) +RPMTEST_CLEANUP + AT_SETUP([rpmkeys -Kv 1]) AT_KEYWORDS([rpmkeys digest]) RPMDB_INIT