From b3dad1c94f2fca289fdf22ded38a1f1463bab95f Mon Sep 17 00:00:00 2001

From: Rob Crittenden <rcritten@redhat.com>

Date: Wed, 15 Apr 2020 17:16:42 -0400

Subject: [PATCH 36/39] Re-order the way the SCEP signing and CA certs are

 collected

Put cacert into the ca store, the racert at the top of the

othercerts list. Then we parse certs, placing all ca certs

we find into the ca store, and all other certs we find after

the racert.

Variables are renamed to match the cm_pkcs7_parse() and

cm_pkcs7_verify_signed() calls.

A special case for IPA (dogtag) was added because dogtag

uses its CA cert to sign the PKCS7 so it is both an RA cert

and a CA cert. If a self-signed CA is detected and no other

certs are provided then the CA is treated as the RA.

https://bugzilla.redhat.com/show_bug.cgi?id=1808052

Graham Leggett did the majority of the work on this patch.

---

 src/pkcs7.c |  18 +++++++++

 src/pkcs7.h |   1 +

 src/scep.c  | 104 +++++++++++++++++++++++++++++++++++-----------------

 3 files changed, 89 insertions(+), 34 deletions(-)

diff --git a/src/pkcs7.c b/src/pkcs7.c

index 29420b9..f81174f 100644

--- a/src/pkcs7.c

+++ b/src/pkcs7.c

@@ -1189,3 +1189,21 @@ done:

 	}

 	return ret;

 }

+

+/* Return 0 if we think "issuer" could have issued "issued", which includes

+ * self-signing. */

+int

+cm_selfsigned(char *cert) 

+{

+	BIO *in;

+	X509 *c;

+

+	in = BIO_new_mem_buf(cert, -1);

+	if (in == NULL) {

+		cm_log(0, "Out of memory.\n");

+		return 1;

+	}

+	c = PEM_read_bio_X509(in, NULL, NULL, NULL);

+	BIO_free(in);

+	return(issuerissued(c, c));

+}

diff --git a/src/pkcs7.h b/src/pkcs7.h

index fae52f8..cbde1bc 100644

--- a/src/pkcs7.h

+++ b/src/pkcs7.h

@@ -62,6 +62,7 @@ int cm_pkcs7_verify_signed(unsigned char *data, size_t length,

 			   unsigned char **recipient_nonce,

 			   size_t *recipient_nonce_length,

 			   unsigned char **payload, size_t *payload_length);

+int cm_selfsigned(char *cert);

 void log_pkcs7_errors(int level, char *msg);

diff --git a/src/scep.c b/src/scep.c

index 4d00692..b80278e 100644

--- a/src/scep.c

+++ b/src/scep.c

@@ -211,12 +211,12 @@ main(int argc, const char **argv)

 	const char *mode = NULL, *content_type = NULL, *content_type2 = NULL;

 	void *ctx;

 	char *params = "", *params2 = NULL, *racert = NULL, *cacert = NULL;

-	char **othercerts = NULL, *cert1 = NULL, *cert2 = NULL, *certs = NULL;

+	char **certothers = NULL, *certleaf = NULL, *certtop = NULL, *certs = NULL;

 	char **racertp, **cacertp, *dracert = NULL, *dcacert = NULL;

 	char buf[LINE_MAX] = "";

 	const unsigned char **buffers = NULL;

 	size_t n_buffers = 0, *lengths = NULL, j;

-	const char *cacerts[3], **racerts;

+	const char *root[3], **othercerts;

 	dbus_bool_t missing_args = FALSE;

 	char *sent_tx, *tx, *msgtype, *pkistatus, *failinfo, *s, *tmp1, *tmp2;

 	unsigned char *sent_nonce, *sender_nonce, *recipient_nonce, *payload;

@@ -871,27 +871,27 @@ main(int argc, const char **argv)

 			n_buffers++;

 		}

 		if (cm_pkcs7_parsev(CM_PKCS7_LEAF_PREFER_ENCRYPT, ctx,

-				    racertp, cacertp, &othercerts,

+				    racertp, cacertp, &certothers,

 				    NULL, NULL,

 				    n_buffers, buffers, lengths) == 0) {

 			if (racert != NULL) {

 				printf("%s", racert);

 				if (cacert != NULL) {

 					printf("%s", cacert);

-					if (othercerts != NULL) {

+					if (certothers != NULL) {

 						for (c = 0;

-						     othercerts[c] != NULL;

+						     certothers[c] != NULL;

 						     c++) {

 							printf("%s",

-							       othercerts[c]);

+							       certothers[c]);

 						}

 					}

 					if ((dracert != NULL) &&

-					    (cert_among(dracert, racert, cacert, othercerts) != 0)) {

+					    (cert_among(dracert, racert, cacert, certothers) != 0)) {

 						printf("%s", dracert);

 					}

 					if ((dcacert != NULL) &&

-					    (cert_among(dcacert, racert, cacert, othercerts) != 0)) {

+					    (cert_among(dcacert, racert, cacert, certothers) != 0)) {

 						printf("%s", dcacert);

 					}

 				}

@@ -907,47 +907,83 @@ main(int argc, const char **argv)

 	case op_pkcsreq:

 		if ((content_type2 != NULL) && (strcasecmp(content_type2,

 			       "application/x-pki-message") == 0)) {

-			memset(&cacerts, 0, sizeof(cacerts));

-			cacerts[0] = cacert ? cacert : racert;

-			cacerts[1] = cacert ? racert : NULL;

-			cacerts[2] = NULL;

-			racerts = NULL;

+			/*

+			 * At this point, we have:

+			 * - zero or more ra certs; and

+			 * - zero or more ca certificates; and

+			 * - zero or more other certificates; that

+			 * need to be reordered so that the leaf

+			 * certificates go first, the ca certificates

+			 * are separated into a seperate certificate

+			 * store, and the other certificates go after

+			 * the leaf certificates.

+			 *

+			 * To do this we put cacert into the ca store,

+			 * the racert at the top of the othercerts list.

+			 * Then we parse certs, placing all ca certs

+			 * we find into the ca store, and all other

+			 * certs we find after the racert.

+			 *

+			 * As a limitation of cm_pkcs7_parse(), we

+			 * can only isolate one ca certificate in the

+			 * list of other certificates.

+			 */

+			/* handle the other certs */

 			if ((certs != NULL) &&

 			    (cm_pkcs7_parse(0, ctx,

-					    &cert1, &cert2, &othercerts,

+					    &certleaf, &certtop, &certothers,

 					    NULL, NULL,

 					    (const unsigned char *) certs,

 					    strlen(certs), NULL) == 0)) {

-				for (c = 0;

-				     (othercerts != NULL) &&

-				     (othercerts[c] != NULL);

-				     c++) {

-					continue;

+				/* Special case for IPA which uses dogtag which signs SCEP

+				 * certs using the CA cert and the typical way to get

+				 * verification to work is to use -I /etc/ipa/ca.crt.

+				 * Because cm_pkcs7_parse explicitly doesn't allow

+				 * certleaf to equal certtop we end up with no CAs so verification

+				 * fails.

+				 * 

+				 * So if cacert and certleaf are both NULL and certtop is

+				 * self-signed then assume the IPA case and set certtop equal

+				 * to certleaf.

+				 */

+				if ((cacert == NULL) && (certtop == NULL) && (certleaf != NULL)) {

+					if (cm_selfsigned(certleaf) == 0) {

+						certtop = certleaf;

+					}

 				}

-				racerts = talloc_array_ptrtype(ctx, racerts, c + 5);

+				memset(&root, 0, sizeof(root));

+				root[0] = cacert ? cacert : certtop ? certtop : NULL;

+				root[1] = cacert ? certtop : NULL;

+				root[2] = NULL;

 				for (c = 0;

-				     (othercerts != NULL) &&

-				     (othercerts[c] != NULL);

+				     (certothers != NULL) &&

+				     (certothers[c] != NULL);

 				     c++) {

-					racerts[c] = othercerts[c];

-				}

-				if (cacert != NULL) {

-					racerts[c++] = cacert;

+					continue;

 				}

-				if (cert1 != NULL) {

-					racerts[c++] = cert1;

+				othercerts = talloc_array_ptrtype(ctx, othercerts, c + 3);

+				c = 0;

+				if (racert != NULL) {

+					othercerts[c++] = racert;

 				}

-				if (cert2 != NULL) {

-					racerts[c++] = cert2;

+				if (certleaf != NULL) {

+					othercerts[c++] = certleaf;

 				}

-				if (racert != NULL) {

-					racerts[c++] = racert;

+				while (certothers != NULL && *certothers != NULL) {

+					othercerts[c++] = *certothers++;

 				}

-				racerts[c++] = NULL;

+				othercerts[c++] = NULL;

+			}

+			else {

+				root[0] = cacert;

+				root[1] = NULL;

+				othercerts = talloc_array_ptrtype(ctx, othercerts, 2);

+				othercerts[0] = racert ? racert : NULL;

+				othercerts[1] = NULL;

 			}

 			ERR_clear_error();

 			i = cm_pkcs7_verify_signed((unsigned char *) results2, results_length2,

-						   cacerts, racerts,

+						   root, othercerts,

 						   NID_pkcs7_data, ctx, NULL,

 						   &tx, &msgtype, &pkistatus, &failinfo,

 						   &sender_nonce, &sender_nonce_length,

-- 

2.21.1