From 09ef8f82dc508ee38016667fcfe36bcc19ec055c Mon Sep 17 00:00:00 2001 From: CentOS Sources Date: Tue, 28 Apr 2020 04:56:43 -0400 Subject: [PATCH] import certmonger-0.79.7-6.el8 --- ...timize-closing-open-file-descriptors.patch | 134 +++++++++ ...UT-when-calling-the-CA-fetch_roots-f.patch | 33 +++ ...UT-when-calling-the-CA-fetch_roots-f.patch | 35 +++ ...lag-flag-from-context-init-look-for-.patch | 259 ++++++++++++++++++ ...include-the-security-module-DB-in-ex.patch | 233 ++++++++++++++++ ...to-pull-the-entire-CA-chain-from-IPA.patch | 50 ++++ SOURCES/0032-Fix-use-after-free-issue.patch | 34 +++ SPECS/certmonger.spec | 32 ++- 8 files changed, 807 insertions(+), 3 deletions(-) create mode 100644 SOURCES/0026-Optimize-closing-open-file-descriptors.patch create mode 100644 SOURCES/0027-Don-t-close-STDOUT-when-calling-the-CA-fetch_roots-f.patch create mode 100644 SOURCES/0028-Don-t-close-STDOUT-when-calling-the-CA-fetch_roots-f.patch create mode 100644 SOURCES/0029-Remove-NOMODDB-flag-flag-from-context-init-look-for-.patch create mode 100644 SOURCES/0030-Update-tests-to-include-the-security-module-DB-in-ex.patch create mode 100644 SOURCES/0031-Try-to-pull-the-entire-CA-chain-from-IPA.patch create mode 100644 SOURCES/0032-Fix-use-after-free-issue.patch diff --git a/SOURCES/0026-Optimize-closing-open-file-descriptors.patch b/SOURCES/0026-Optimize-closing-open-file-descriptors.patch new file mode 100644 index 0000000..c94fa0c --- /dev/null +++ b/SOURCES/0026-Optimize-closing-open-file-descriptors.patch @@ -0,0 +1,134 @@ +From 9bbb628620d4e586941344e1bdbbc166a885c0a9 Mon Sep 17 00:00:00 2001 +From: Rob Crittenden +Date: Thu, 5 Sep 2019 12:45:52 -0400 +Subject: [PATCH] Optimize closing open file descriptors + +When forking, the code would close all unused file descriptors up +to maximum number of files. In the default case this is 1024. In +the container case this is 1048576. Huge delays in startup were +seen due to this. + +Even in a default 1024 ulimit case this drastically reduces the +number of file descriptors to mark FD_CLOEXEC but in the container +default case this saves another order of magnitude of work. + +This patch takes inspiration from systemd[1] and walks /proc/self/fd +if it is available to determine the list of open descriptors. It +falls back to the "close all fds we don't care about up to limit" +method. + +https://bugzilla.redhat.com/show_bug.cgi?id=1656519 + +[1] https://github.com/systemd/systemd/blob/5238e9575906297608ff802a27e2ff9effa3b338/src/basic/fd-util.c#L217 +--- + src/subproc.c | 71 ++++++++++++++++++++++++++++++++++++++++++++------- + 1 file changed, 62 insertions(+), 9 deletions(-) + +diff --git a/src/subproc.c b/src/subproc.c +index e49e3762..8df836ae 100644 +--- a/src/subproc.c ++++ b/src/subproc.c +@@ -19,6 +19,7 @@ + + #include + #include ++#include + #include + #include + #include +@@ -436,6 +437,25 @@ cm_subproc_parse_args(void *parent, const char *cmdline, const char **error) + return argv; + } + ++/* Based heavily on systemd version */ ++static ++int safe_atoi(const char *s, int *ret_i) { ++ char *x = NULL; ++ long l; ++ ++ errno = 0; ++ l = strtol(s, &x, 0); ++ if (errno > 0) ++ return -1; ++ if (!x || x == s || *x != 0) ++ return -1; ++ if ((long) (int) l != l) ++ return -1; ++ ++ *ret_i = (int) l; ++ return 0; ++} ++ + /* Redirect stdio to /dev/null, and mark everything else as close-on-exec, + * except for perhaps one to three of them that are passed in by number. */ + void +@@ -443,6 +463,9 @@ cm_subproc_mark_most_cloexec(int fd, int fd2, int fd3) + { + int i; + long l; ++ DIR *dir = NULL; ++ struct dirent *de; ++ + if ((fd != STDIN_FILENO) && + (fd2 != STDIN_FILENO) && + (fd3 != STDIN_FILENO)) { +@@ -482,17 +505,47 @@ cm_subproc_mark_most_cloexec(int fd, int fd2, int fd3) + close(STDERR_FILENO); + } + } +- for (i = getdtablesize() - 1; i >= 3; i--) { +- if ((i == fd) || +- (i == fd2) || +- (i == fd3)) { +- continue; ++ dir = opendir("/proc/self/fd"); ++ if (!dir) { ++ /* /proc isn't available, fall back to old way */ ++ for (i = getdtablesize() - 1; i >= 3; i--) { ++ if ((i == fd) || ++ (i == fd2) || ++ (i == fd3)) { ++ continue; ++ } ++ l = fcntl(i, F_GETFD); ++ if (l != -1) { ++ if (fcntl(i, F_SETFD, l | FD_CLOEXEC) != 0) { ++ cm_log(0, "Potentially leaking FD %d.\n", i); ++ } ++ } + } +- l = fcntl(i, F_GETFD); +- if (l != -1) { +- if (fcntl(i, F_SETFD, l | FD_CLOEXEC) != 0) { +- cm_log(0, "Potentially leaking FD %d.\n", i); ++ } else { ++ while ((de = readdir(dir)) != NULL) { ++ int i = -1; ++ ++ if (safe_atoi(de->d_name, &i) < 0) { ++ continue; ++ } ++ ++ if ((i == fd) || ++ (i == fd2) || ++ (i == fd3)) { ++ continue; ++ } ++ ++ if (i == dirfd(dir)) { ++ continue; ++ } ++ ++ l = fcntl(i, F_GETFD); ++ if (l != -1) { ++ if (fcntl(i, F_SETFD, l | FD_CLOEXEC) != 0) { ++ cm_log(0, "Potentially leaking FD %d.\n", i); ++ } + } + } ++ closedir(dir); + } + } +-- +2.21.0 + diff --git a/SOURCES/0027-Don-t-close-STDOUT-when-calling-the-CA-fetch_roots-f.patch b/SOURCES/0027-Don-t-close-STDOUT-when-calling-the-CA-fetch_roots-f.patch new file mode 100644 index 0000000..e1e6a23 --- /dev/null +++ b/SOURCES/0027-Don-t-close-STDOUT-when-calling-the-CA-fetch_roots-f.patch @@ -0,0 +1,33 @@ +From b7bcb1b3b953c2052e2d89cb2b3e9d9ccd1b3864 Mon Sep 17 00:00:00 2001 +From: Rob Crittenden +Date: Thu, 10 Oct 2019 16:28:18 -0400 +Subject: [PATCH] Don't close STDOUT when calling the CA fetch_roots function + +cm_subproc_mark_most_cloexec() now closes all open file +descriptors except for up to three requested for stdin, stdout +and stderr. Before the optimization those three were always +left open. + +This was causing errors in the IPA helper ipa-server-guard +because it tries to display the contents of stderr which was +always being closed, causing ipa-server-guard to blow up. +--- + src/cadata.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/cadata.c b/src/cadata.c +index eb87eb76..3e916c96 100644 +--- a/src/cadata.c ++++ b/src/cadata.c +@@ -109,7 +109,7 @@ fetch(int fd, struct cm_store_ca *ca, struct cm_store_entry *entry, void *data) + } + return -1; + } +- cm_subproc_mark_most_cloexec(STDOUT_FILENO, -1, -1); ++ cm_subproc_mark_most_cloexec(STDOUT_FILENO, STDERR_FILENO, -1); + cm_log(1, "Running enrollment/cadata helper \"%s\".\n", argv[0]); + execvp(argv[0], argv); + u = errno; +-- +2.21.0 + diff --git a/SOURCES/0028-Don-t-close-STDOUT-when-calling-the-CA-fetch_roots-f.patch b/SOURCES/0028-Don-t-close-STDOUT-when-calling-the-CA-fetch_roots-f.patch new file mode 100644 index 0000000..a0c100f --- /dev/null +++ b/SOURCES/0028-Don-t-close-STDOUT-when-calling-the-CA-fetch_roots-f.patch @@ -0,0 +1,35 @@ +From 205775f73f7eef7b207acccac6b853562adf604b Mon Sep 17 00:00:00 2001 +From: Rob Crittenden +Date: Fri, 25 Oct 2019 20:25:36 +0000 +Subject: [PATCH] Don't close STDERR when submitting request + +cm_subproc_mark_most_cloexec() now closes all open file +descriptors except for up to three requested for stdin, stdout +and stderr. Before the optimization those three were always +left open. + +This was causing errors in the IPA helper ipa-server-guard +because it tries to display the contents of stderr which was +always being closed, causing ipa-server-guard to blow up. +--- + src/submit-e.c | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/src/submit-e.c b/src/submit-e.c +index d6158d7a..69b4f8e2 100644 +--- a/src/submit-e.c ++++ b/src/submit-e.c +@@ -941,8 +941,8 @@ cm_submit_e_helper_main(int fd, struct cm_store_ca *ca, + } + return -1; + } +- cm_log(2, "Redirecting stdin and stderr to /dev/null, leaving stdout open for child \"%s\".\n", argv[0]); +- cm_subproc_mark_most_cloexec(STDOUT_FILENO, -1, -1); ++ cm_log(2, "Redirecting stdin to /dev/null, leaving stdout and stderr open for child \"%s\".\n", argv[0]); ++ cm_subproc_mark_most_cloexec(STDOUT_FILENO, STDERR_FILENO, -1); + cm_log(1, "Running enrollment helper \"%s\".\n", argv[0]); + execvp(argv[0], argv); + u = errno; +-- +2.21.0 + diff --git a/SOURCES/0029-Remove-NOMODDB-flag-flag-from-context-init-look-for-.patch b/SOURCES/0029-Remove-NOMODDB-flag-flag-from-context-init-look-for-.patch new file mode 100644 index 0000000..f3f77a2 --- /dev/null +++ b/SOURCES/0029-Remove-NOMODDB-flag-flag-from-context-init-look-for-.patch @@ -0,0 +1,259 @@ +From 34c120f0259750ff2228def2955de9ad985340e6 Mon Sep 17 00:00:00 2001 +From: Rob Crittenden +Date: Mon, 26 Aug 2019 22:01:35 +0000 +Subject: [PATCH] Remove NOMODDB flag flag from context init, look for full + tokens + +The NSS databases were almost universally initialized with the +NOMODDB flag. I'm not sure if something changed in NSS but the +PKCS#11 modules were not being initialized. Adding this back after +permission checks are done results in tokens working again. + +When looking for certs and keys try the full token:nickname string +as well as just nickname when comparing values. + +https://pagure.io/certmonger/issue/125 +--- + src/casave.c | 3 +-- + src/certread-n.c | 33 ++++++++++++++++----------------- + src/certsave-n.c | 5 +++++ + src/dogtag.c | 3 +-- + src/keygen-n.c | 5 +++++ + src/keyiread-n.c | 11 ++++++++++- + src/scepgen-n.c | 5 +++++ + src/submit-n.c | 5 +++++ + src/toklist.c | 2 +- + 9 files changed, 49 insertions(+), 23 deletions(-) + +diff --git a/src/casave.c b/src/casave.c +index bde63f99..1cf5a406 100644 +--- a/src/casave.c ++++ b/src/casave.c +@@ -111,8 +111,7 @@ cm_casave_main_n(int fd, struct cm_store_ca *ca, struct cm_store_entry *e, + break; + default: + flags = NSS_INIT_READONLY | +- NSS_INIT_NOROOTINIT | +- NSS_INIT_NOMODDB; ++ NSS_INIT_NOROOTINIT; + /* Sigh. Not a lot of detail. Check + * if we succeed in read-only mode, + * which we'll interpret as lack of +diff --git a/src/certread-n.c b/src/certread-n.c +index d535030b..bb61b61b 100644 +--- a/src/certread-n.c ++++ b/src/certread-n.c +@@ -157,27 +157,22 @@ cm_certread_n_main(int fd, struct cm_store_ca *ca, struct cm_store_entry *entry, + cm_log(1, "Unable to open NSS database.\n"); + _exit(status); + } ++ /* Re-open the database with modules enabled */ ++ NSS_ShutdownContext(ctx); ++ ctx = NSS_InitContext(entry->cm_cert_storage_location, ++ NULL, NULL, NULL, NULL, ++ (readwrite ? 0 : NSS_INIT_READONLY) | ++ NSS_INIT_NOROOTINIT); + es = util_n_fips_hook(); + if (es != NULL) { + cm_log(1, "Error putting NSS into FIPS mode: %s\n", es); + _exit(CM_SUB_STATUS_ERROR_INITIALIZING); + } +- /* Allocate a memory pool. */ +- arena = PORT_NewArena(sizeof(double)); +- if (arena == NULL) { +- cm_log(1, "Error opening database '%s'.\n", +- entry->cm_cert_storage_location); +- if (NSS_ShutdownContext(ctx) != SECSuccess) { +- cm_log(1, "Error shutting down NSS.\n"); +- } +- _exit(ENOMEM); +- } + /* Find the tokens that we might use for cert storage. */ + mech = CKM_RSA_X_509; + slotlist = PK11_GetAllTokens(mech, PR_FALSE, PR_FALSE, NULL); + if (slotlist == NULL) { + cm_log(1, "Error getting list of tokens.\n"); +- PORT_FreeArena(arena, PR_TRUE); + if (NSS_ShutdownContext(ctx) != SECSuccess) { + cm_log(1, "Error shutting down NSS.\n"); + } +@@ -249,6 +244,7 @@ cm_certread_n_main(int fd, struct cm_store_ca *ca, struct cm_store_entry *entry, + } + /* If we need to log in in order to read certificates, do so. */ + if (PK11_NeedLogin(sle->slot)) { ++ cm_log(3, "Need login to token %s\n", PK11_GetTokenName(sle->slot)); + if (cm_pin_read_for_cert(entry, &pin) != 0) { + cm_log(1, "Error reading PIN for cert db, " + "skipping.\n"); +@@ -272,13 +268,19 @@ cm_certread_n_main(int fd, struct cm_store_ca *ca, struct cm_store_entry *entry, + /* Walk the list of certificates in the slot, looking for one + * which matches the specified nickname. */ + certs = PK11_ListCertsInSlot(sle->slot); ++ cm_log(3, "Looking for %s\n", entry->cm_cert_nickname); + if (certs != NULL) { + for (node = CERT_LIST_HEAD(certs); + !CERT_LIST_EMPTY(certs) && + !CERT_LIST_END(node, certs); + node = CERT_LIST_NEXT(node)) { +- if (strcmp(node->cert->nickname, +- entry->cm_cert_nickname) == 0) { ++ cm_log(3, "certread-n: Slot nickname %s\n", ++ node->cert->nickname); ++ es = talloc_asprintf(entry, "%s:%s", ++ entry->cm_cert_token, entry->cm_cert_nickname); ++ if ((strcmp(node->cert->nickname, ++ entry->cm_cert_nickname) == 0) || ++ (strcmp(node->cert->nickname, es) == 0)) { + cm_log(3, "Located the certificate " + "\"%s\".\n", + entry->cm_cert_nickname); +@@ -321,7 +323,6 @@ next_slot: + if (cert == NULL) { + cm_log(1, "Error locating certificate.\n"); + PK11_FreeSlotList(slotlist); +- PORT_FreeArena(arena, PR_TRUE); + if (NSS_ShutdownContext(ctx) != SECSuccess) { + cm_log(1, "Error shutting down NSS.\n"); + } +@@ -332,7 +333,6 @@ next_slot: + fclose(fp); + CERT_DestroyCertificate(cert); + PK11_FreeSlotList(slotlist); +- PORT_FreeArena(arena, PR_TRUE); + if (NSS_ShutdownContext(ctx) != SECSuccess) { + cm_log(1, "Error shutting down NSS.\n"); + } +@@ -358,8 +358,7 @@ cm_certread_n_parse(struct cm_store_entry *entry, + NULL, NULL, NULL, NULL, + NSS_INIT_NOCERTDB | + NSS_INIT_READONLY | +- NSS_INIT_NOROOTINIT | +- NSS_INIT_NOMODDB); ++ NSS_INIT_NOROOTINIT); + if (ctx == NULL) { + cm_log(1, "Unable to initialize NSS.\n"); + _exit(1); +diff --git a/src/certsave-n.c b/src/certsave-n.c +index 972a1dfa..eda03b34 100644 +--- a/src/certsave-n.c ++++ b/src/certsave-n.c +@@ -186,6 +186,11 @@ cm_certsave_n_main(int fd, struct cm_store_ca *ca, struct cm_store_entry *entry, + } else { + /* We don't try to force FIPS mode here, as it seems to get in + * the way of saving the certificate. */ ++ NSS_ShutdownContext(ctx); ++ ctx = NSS_InitContext(entry->cm_cert_storage_location, ++ NULL, NULL, NULL, NULL, ++ (readwrite ? 0 : NSS_INIT_READONLY) | ++ NSS_INIT_NOROOTINIT); + + /* Allocate a memory pool. */ + arena = PORT_NewArena(sizeof(double)); +diff --git a/src/dogtag.c b/src/dogtag.c +index 55607f3d..c43664ef 100644 +--- a/src/dogtag.c ++++ b/src/dogtag.c +@@ -306,8 +306,7 @@ main(int argc, const char **argv) + NULL, NULL, NULL, NULL, + NSS_INIT_NOCERTDB | + NSS_INIT_READONLY | +- NSS_INIT_NOROOTINIT | +- NSS_INIT_NOMODDB); ++ NSS_INIT_NOROOTINIT); + if (nctx == NULL) { + cm_log(1, "Unable to initialize NSS.\n"); + _exit(1); +diff --git a/src/keygen-n.c b/src/keygen-n.c +index 061bd2af..e921d7ec 100644 +--- a/src/keygen-n.c ++++ b/src/keygen-n.c +@@ -226,6 +226,11 @@ cm_keygen_n_main(int fd, struct cm_store_ca *ca, struct cm_store_entry *entry, + break; + } + } ++ NSS_ShutdownContext(ctx); ++ ctx = NSS_InitContext(entry->cm_key_storage_location, ++ NULL, NULL, NULL, NULL, ++ (readwrite ? 0 : NSS_INIT_READONLY) | ++ NSS_INIT_NOROOTINIT); + reason = util_n_fips_hook(); + if (reason != NULL) { + cm_log(1, "Error putting NSS into FIPS mode: %s\n", reason); +diff --git a/src/keyiread-n.c b/src/keyiread-n.c +index 91b1be41..dc1c6092 100644 +--- a/src/keyiread-n.c ++++ b/src/keyiread-n.c +@@ -115,6 +115,11 @@ cm_keyiread_n_get_keys(struct cm_store_entry *entry, int readwrite) + break; + } + } ++ NSS_ShutdownContext(ctx); ++ ctx = NSS_InitContext(entry->cm_key_storage_location, ++ NULL, NULL, NULL, NULL, ++ (readwrite ? 0 : NSS_INIT_READONLY) | ++ NSS_INIT_NOROOTINIT); + reason = util_n_fips_hook(); + if (reason != NULL) { + cm_log(1, "Error putting NSS into FIPS mode: %s\n", reason); +@@ -340,8 +345,12 @@ cm_keyiread_n_get_keys(struct cm_store_entry *entry, int readwrite) + cnode = CERT_LIST_NEXT(cnode)) { + nickname = entry->cm_key_nickname; + cert = cnode->cert; ++ es = talloc_asprintf(entry, "%s:%s", ++ entry->cm_cert_token, ++ entry->cm_cert_nickname); + if ((nickname != NULL) && +- (strcmp(cert->nickname, nickname) == 0)) { ++ ((strcmp(cert->nickname, nickname) == 0) || ++ (strcmp(cert->nickname, es) == 0))) { + cm_log(3, "Located a certificate with " + "the key's nickname (\"%s\").\n", + nickname); +diff --git a/src/scepgen-n.c b/src/scepgen-n.c +index d6735aa7..8c67b122 100644 +--- a/src/scepgen-n.c ++++ b/src/scepgen-n.c +@@ -183,6 +183,11 @@ cm_scepgen_n_main(int fd, struct cm_store_ca *ca, struct cm_store_entry *entry, + break; + } + } ++ NSS_ShutdownContext(ctx); ++ ctx = NSS_InitContext(entry->cm_key_storage_location, ++ NULL, NULL, NULL, NULL, ++ NSS_INIT_READONLY | ++ NSS_INIT_NOROOTINIT); + reason = util_n_fips_hook(); + if (reason != NULL) { + cm_log(1, "Error putting NSS into FIPS mode: %s\n", reason); +diff --git a/src/submit-n.c b/src/submit-n.c +index b07ea23a..f27b9c7f 100644 +--- a/src/submit-n.c ++++ b/src/submit-n.c +@@ -317,6 +317,11 @@ cm_submit_n_decrypt_envelope(const unsigned char *envelope, + } + goto done; + } ++ NSS_ShutdownContext(ctx); ++ ctx = NSS_InitContext(args->entry->cm_key_storage_location, ++ NULL, NULL, NULL, NULL, ++ NSS_INIT_READONLY | ++ NSS_INIT_NOROOTINIT); + reason = util_n_fips_hook(); + if (reason != NULL) { + cm_log(1, "Error putting NSS into FIPS mode: %s\n", reason); +diff --git a/src/toklist.c b/src/toklist.c +index a4328218..ac166722 100644 +--- a/src/toklist.c ++++ b/src/toklist.c +@@ -79,7 +79,7 @@ main(int argc, const char **argv) + + /* Open the database. */ + ctx = NSS_InitContext(dbdir, NULL, NULL, NULL, NULL, +- NSS_INIT_NOROOTINIT | NSS_INIT_NOMODDB); ++ NSS_INIT_NOROOTINIT); + if (ctx == NULL) { + printf("Unable to open NSS database '%s'.\n", dbdir); + _exit(CM_SUB_STATUS_ERROR_INITIALIZING); +-- +2.21.0 + diff --git a/SOURCES/0030-Update-tests-to-include-the-security-module-DB-in-ex.patch b/SOURCES/0030-Update-tests-to-include-the-security-module-DB-in-ex.patch new file mode 100644 index 0000000..9cbdfe9 --- /dev/null +++ b/SOURCES/0030-Update-tests-to-include-the-security-module-DB-in-ex.patch @@ -0,0 +1,233 @@ +From 59df833ca5fb80c596df621a24dc461a550dba71 Mon Sep 17 00:00:00 2001 +From: Rob Crittenden +Date: Tue, 27 Aug 2019 18:01:02 +0000 +Subject: [PATCH] Update tests to include the security module DB in expected + output + +certmonger was previously always initializing the databases with +the flag NSS_INIT_NOMODDB but in at elast NSS 3.44 this doesn't +seem to initialize external modules (tested with SoftHSM2). + +https://pagure.io/certmonger/issue/125 +--- + tests/034-perms-dbm/expected.out | 16 ++++++++++++++++ + tests/034-perms-sql/expected.out | 16 ++++++++++++++++ + tests/034-perms/expected.out | 16 ++++++++++++++++ + 3 files changed, 48 insertions(+) + +diff --git a/tests/034-perms-dbm/expected.out b/tests/034-perms-dbm/expected.out +index c062d409..7bf23a37 100644 +--- a/tests/034-perms-dbm/expected.out ++++ b/tests/034-perms-dbm/expected.out +@@ -45,50 +45,66 @@ $owner:$group|0620|ee.key + [dbm:keygen] + $owner:$group|0600|cert8.db + $owner:$group|0620|key3.db ++$owner:$group|0600|secmod.db + [dbm:reset] + $owner:$group|0755|cert8.db + $owner:$group|0755|key3.db ++$owner:$group|0755|secmod.db + [dbm:csrgen] + $owner:$group|0755|cert8.db + $owner:$group|0620|key3.db ++$owner:$group|0755|secmod.db + [dbm:reset] + $owner:$group|0755|cert8.db + $owner:$group|0755|key3.db ++$owner:$group|0755|secmod.db + [dbm:submit] + $owner:$group|0755|cert8.db + $owner:$group|0755|key3.db ++$owner:$group|0755|secmod.db + [dbm:reset] + $owner:$group|0755|cert8.db + $owner:$group|0755|key3.db ++$owner:$group|0755|secmod.db + [dbm:save] + $owner:$group|0662|cert8.db + $owner:$group|0620|key3.db ++$owner:$group|0662|secmod.db + [rekey:dbm:start] + [rekey:dbm:keygen] + $owner:$group|0600|cert8.db + $owner:$group|0620|key3.db ++$owner:$group|0600|secmod.db + [rekey:dbm:reset] + $owner:$group|0755|cert8.db + $owner:$group|0755|key3.db ++$owner:$group|0755|secmod.db + [rekey:dbm:keygen] + $owner:$group|0755|cert8.db + $owner:$group|0620|key3.db ++$owner:$group|0755|secmod.db + [rekey:dbm:reset] + $owner:$group|0755|cert8.db + $owner:$group|0755|key3.db ++$owner:$group|0755|secmod.db + [rekey:dbm:csrgen] + $owner:$group|0755|cert8.db + $owner:$group|0620|key3.db ++$owner:$group|0755|secmod.db + [rekey:dbm:reset] + $owner:$group|0755|cert8.db + $owner:$group|0755|key3.db ++$owner:$group|0755|secmod.db + [rekey:dbm:submit] + $owner:$group|0755|cert8.db + $owner:$group|0755|key3.db ++$owner:$group|0755|secmod.db + [rekey:dbm:reset] + $owner:$group|0755|cert8.db + $owner:$group|0755|key3.db ++$owner:$group|0755|secmod.db + [rekey:dbm:save] + $owner:$group|0662|cert8.db + $owner:$group|0620|key3.db ++$owner:$group|0662|secmod.db + OK +diff --git a/tests/034-perms-sql/expected.out b/tests/034-perms-sql/expected.out +index 2808e02c..c5914e02 100644 +--- a/tests/034-perms-sql/expected.out ++++ b/tests/034-perms-sql/expected.out +@@ -45,50 +45,66 @@ $owner:$group|0620|ee.key + [sql:keygen] + $owner:$group|0600|cert9.db + $owner:$group|0620|key4.db ++$owner:$group|0600|pkcs11.txt + [sql:reset] + $owner:$group|0755|cert9.db + $owner:$group|0755|key4.db ++$owner:$group|0755|pkcs11.txt + [sql:csrgen] + $owner:$group|0755|cert9.db + $owner:$group|0620|key4.db ++$owner:$group|0755|pkcs11.txt + [sql:reset] + $owner:$group|0755|cert9.db + $owner:$group|0755|key4.db ++$owner:$group|0755|pkcs11.txt + [sql:submit] + $owner:$group|0755|cert9.db + $owner:$group|0755|key4.db ++$owner:$group|0755|pkcs11.txt + [sql:reset] + $owner:$group|0755|cert9.db + $owner:$group|0755|key4.db ++$owner:$group|0755|pkcs11.txt + [sql:save] + $owner:$group|0662|cert9.db + $owner:$group|0620|key4.db ++$owner:$group|0662|pkcs11.txt + [rekey:sql:start] + [rekey:sql:keygen] + $owner:$group|0600|cert9.db + $owner:$group|0620|key4.db ++$owner:$group|0600|pkcs11.txt + [rekey:sql:reset] + $owner:$group|0755|cert9.db + $owner:$group|0755|key4.db ++$owner:$group|0755|pkcs11.txt + [rekey:sql:keygen] + $owner:$group|0755|cert9.db + $owner:$group|0620|key4.db ++$owner:$group|0755|pkcs11.txt + [rekey:sql:reset] + $owner:$group|0755|cert9.db + $owner:$group|0755|key4.db ++$owner:$group|0755|pkcs11.txt + [rekey:sql:csrgen] + $owner:$group|0755|cert9.db + $owner:$group|0620|key4.db ++$owner:$group|0755|pkcs11.txt + [rekey:sql:reset] + $owner:$group|0755|cert9.db + $owner:$group|0755|key4.db ++$owner:$group|0755|pkcs11.txt + [rekey:sql:submit] + $owner:$group|0755|cert9.db + $owner:$group|0755|key4.db ++$owner:$group|0755|pkcs11.txt + [rekey:sql:reset] + $owner:$group|0755|cert9.db + $owner:$group|0755|key4.db ++$owner:$group|0755|pkcs11.txt + [rekey:sql:save] + $owner:$group|0662|cert9.db + $owner:$group|0620|key4.db ++$owner:$group|0662|pkcs11.txt + OK +diff --git a/tests/034-perms/expected.out b/tests/034-perms/expected.out +index c062d409..7bf23a37 100644 +--- a/tests/034-perms/expected.out ++++ b/tests/034-perms/expected.out +@@ -45,50 +45,66 @@ $owner:$group|0620|ee.key + [dbm:keygen] + $owner:$group|0600|cert8.db + $owner:$group|0620|key3.db ++$owner:$group|0600|secmod.db + [dbm:reset] + $owner:$group|0755|cert8.db + $owner:$group|0755|key3.db ++$owner:$group|0755|secmod.db + [dbm:csrgen] + $owner:$group|0755|cert8.db + $owner:$group|0620|key3.db ++$owner:$group|0755|secmod.db + [dbm:reset] + $owner:$group|0755|cert8.db + $owner:$group|0755|key3.db ++$owner:$group|0755|secmod.db + [dbm:submit] + $owner:$group|0755|cert8.db + $owner:$group|0755|key3.db ++$owner:$group|0755|secmod.db + [dbm:reset] + $owner:$group|0755|cert8.db + $owner:$group|0755|key3.db ++$owner:$group|0755|secmod.db + [dbm:save] + $owner:$group|0662|cert8.db + $owner:$group|0620|key3.db ++$owner:$group|0662|secmod.db + [rekey:dbm:start] + [rekey:dbm:keygen] + $owner:$group|0600|cert8.db + $owner:$group|0620|key3.db ++$owner:$group|0600|secmod.db + [rekey:dbm:reset] + $owner:$group|0755|cert8.db + $owner:$group|0755|key3.db ++$owner:$group|0755|secmod.db + [rekey:dbm:keygen] + $owner:$group|0755|cert8.db + $owner:$group|0620|key3.db ++$owner:$group|0755|secmod.db + [rekey:dbm:reset] + $owner:$group|0755|cert8.db + $owner:$group|0755|key3.db ++$owner:$group|0755|secmod.db + [rekey:dbm:csrgen] + $owner:$group|0755|cert8.db + $owner:$group|0620|key3.db ++$owner:$group|0755|secmod.db + [rekey:dbm:reset] + $owner:$group|0755|cert8.db + $owner:$group|0755|key3.db ++$owner:$group|0755|secmod.db + [rekey:dbm:submit] + $owner:$group|0755|cert8.db + $owner:$group|0755|key3.db ++$owner:$group|0755|secmod.db + [rekey:dbm:reset] + $owner:$group|0755|cert8.db + $owner:$group|0755|key3.db ++$owner:$group|0755|secmod.db + [rekey:dbm:save] + $owner:$group|0662|cert8.db + $owner:$group|0620|key3.db ++$owner:$group|0662|secmod.db + OK +-- +2.21.0 + diff --git a/SOURCES/0031-Try-to-pull-the-entire-CA-chain-from-IPA.patch b/SOURCES/0031-Try-to-pull-the-entire-CA-chain-from-IPA.patch new file mode 100644 index 0000000..5c9978d --- /dev/null +++ b/SOURCES/0031-Try-to-pull-the-entire-CA-chain-from-IPA.patch @@ -0,0 +1,50 @@ +From 64702b25951ce996532afea7d627612d6bba7451 Mon Sep 17 00:00:00 2001 +From: Rob Crittenden +Date: Thu, 10 Oct 2019 18:24:32 +0000 +Subject: [PATCH] Try to pull the entire CA chain from IPA + +IPA originally stored a single cert in cn=cacert which is +what certmonger has always retrieved in fetch_roots. It was +replaced to store cn=certificates as separate entries in order +to more easily support chains and to include additional +metadata about certificates. + +Try to pull the chain from that location first and fall back +to cn=cacert if no entries are found. + +https://bugzilla.redhat.com/show_bug.cgi?id=1710632 +--- + src/ipa.c | 10 +++++++++- + 1 file changed, 9 insertions(+), 1 deletion(-) + +diff --git a/src/ipa.c b/src/ipa.c +index acd1a4e2..40a4b52c 100644 +--- a/src/ipa.c ++++ b/src/ipa.c +@@ -508,7 +508,8 @@ fetch_roots(const char *server, int ldap_uri_cmd, const char *ldap_uri, + LDAP *ld = NULL; + LDAPMessage *lresult = NULL, *lmsg = NULL; + char *lattrs[2] = {"caCertificate;binary", NULL}; +- const char *relativedn = "cn=cacert,cn=ipa,cn=etc"; ++ const char *relativedn = "cn=certificates,cn=ipa,cn=etc"; ++ const char *relativecompatdn = "cn=cacert,cn=ipa,cn=etc"; + char ldn[LINE_MAX], lfilter[LINE_MAX], uri[LINE_MAX] = "", *kerr = NULL; + struct berval **lbvalues, *lbv; + unsigned char *bv_val; +@@ -543,6 +544,13 @@ fetch_roots(const char *server, int ldap_uri_cmd, const char *ldap_uri, + rc = ldap_search_ext_s(ld, ldn, LDAP_SCOPE_SUBTREE, + lfilter, lattrs, 0, NULL, NULL, NULL, + LDAP_NO_LIMIT, &lresult); ++ if (rc == LDAP_SUCCESS && ldap_count_entries(ld, lresult) == 0) { ++ /* Fall back to the old location */ ++ snprintf(ldn, sizeof(ldn), "%s,%s", relativecompatdn, basedn); ++ rc = ldap_search_ext_s(ld, ldn, LDAP_SCOPE_SUBTREE, ++ lfilter, lattrs, 0, NULL, NULL, NULL, ++ LDAP_NO_LIMIT, &lresult); ++ } + if (rc != LDAP_SUCCESS) { + fprintf(stderr, "Error searching '%s': %s.\n", + ldn, ldap_err2string(rc)); +-- +2.21.0 + diff --git a/SOURCES/0032-Fix-use-after-free-issue.patch b/SOURCES/0032-Fix-use-after-free-issue.patch new file mode 100644 index 0000000..06b0940 --- /dev/null +++ b/SOURCES/0032-Fix-use-after-free-issue.patch @@ -0,0 +1,34 @@ +From c6f2737747cbb70adfdd1a77412b669838f9c419 Mon Sep 17 00:00:00 2001 +From: Rob Crittenden +Date: Mon, 2 Dec 2019 15:08:54 -0500 +Subject: [PATCH] Fix use-after-free issue + +The basedn value was freed after the first search but a second +one could be initiated. +--- + src/ipa.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/ipa.c b/src/ipa.c +index 40a4b52c..41ca9081 100644 +--- a/src/ipa.c ++++ b/src/ipa.c +@@ -540,7 +540,6 @@ fetch_roots(const char *server, int ldap_uri_cmd, const char *ldap_uri, + /* Now look up the root certificates for the domain. */ + snprintf(lfilter, sizeof(lfilter), "(%s=*)", lattrs[0]); + snprintf(ldn, sizeof(ldn), "%s,%s", relativedn, basedn); +- free(basedn); + rc = ldap_search_ext_s(ld, ldn, LDAP_SCOPE_SUBTREE, + lfilter, lattrs, 0, NULL, NULL, NULL, + LDAP_NO_LIMIT, &lresult); +@@ -551,6 +550,7 @@ fetch_roots(const char *server, int ldap_uri_cmd, const char *ldap_uri, + lfilter, lattrs, 0, NULL, NULL, NULL, + LDAP_NO_LIMIT, &lresult); + } ++ free(basedn); + if (rc != LDAP_SUCCESS) { + fprintf(stderr, "Error searching '%s': %s.\n", + ldn, ldap_err2string(rc)); +-- +2.21.0 + diff --git a/SPECS/certmonger.spec b/SPECS/certmonger.spec index 39d1896..f71691d 100644 --- a/SPECS/certmonger.spec +++ b/SPECS/certmonger.spec @@ -9,7 +9,7 @@ Name: certmonger Version: 0.79.7 -Release: 3%{?dist} +Release: 6%{?dist} Summary: Certificate status monitor and PKI enrollment client Group: System Environment/Daemons @@ -80,8 +80,8 @@ Requires(post): /sbin/chkconfig, /sbin/service Requires(preun): /sbin/chkconfig, /sbin/service, dbus, sed %endif -Patch1: 0001-NSS-crypto-policy-sets-minimum-RSA-and-DSA-key-size-.patch -Patch2: 0002-Convert-tests-to-use-python3.patch +Patch1: 0001-NSS-crypto-policy-sets-minimum-RSA-and-DSA-key-size-.patch +Patch2: 0002-Convert-tests-to-use-python3.patch Patch18: 0018-clang-more-Dead-assignment.patch Patch19: 0019-clang-more-Memory-leaks.patch Patch20: 0020-clang-Avoid-buffer-overflow.patch @@ -90,6 +90,14 @@ Patch22: 0022-Uninitialized-variable.patch Patch23: 0023-merge-into-clang-more-Memory-leaks.patch Patch24: 0024-Add-missing-return-type-declaration.patch Patch25: 0025-Discards-const-qualifier.patch +Patch26: 0026-Optimize-closing-open-file-descriptors.patch +Patch27: 0027-Don-t-close-STDOUT-when-calling-the-CA-fetch_roots-f.patch +Patch28: 0028-Don-t-close-STDOUT-when-calling-the-CA-fetch_roots-f.patch +Patch29: 0029-Remove-NOMODDB-flag-flag-from-context-init-look-for-.patch +Patch30: 0030-Update-tests-to-include-the-security-module-DB-in-ex.patch +Patch31: 0031-Try-to-pull-the-entire-CA-chain-from-IPA.patch +Patch32: 0032-Fix-use-after-free-issue.patch + %description Certmonger is a service which is primarily concerned with getting your @@ -107,6 +115,13 @@ system enrolled with a certificate authority (CA) and keeping it enrolled. %patch23 -p1 %patch24 -p1 %patch25 -p1 +%patch26 -p1 +%patch27 -p1 +%patch28 -p1 +%patch29 -p1 +%patch30 -p1 +%patch31 -p1 +%patch32 -p1 %build autoreconf -i -f @@ -233,6 +248,17 @@ exit 0 %endif %changelog +* Mon Dec 16 2019 Rob Crittenden - 0.79.7-6 +- Rebuild + +* Mon Dec 2 2019 Rob Crittenden - 0.79.7-5 +- Fix use-after-free issue when retrieving CA chain (#1710632) + +* Mon Dec 2 2019 Rob Crittenden - 0.79.7-4 +- Optimize closing of file descriptors on fork (#1763745) +- Remove NOMODDB flag flag from context init, look for full tokens (#1746543) +- Retrieve full IPA CA chain (#1710632) + * Tue May 14 2019 Rob Crittenden - 0.79.7-3 - Rebuild for new annobin (#1708095)