From: pcarana <pc.moreno2099@gmail.com>
Date: Wed, 17 Jul 2019 17:04:17 +0000 (-0500)
Subject: Fix 11: validate certificates against its corresponding CRL.
X-Git-Tag: v1.0.0^2~21
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=fb30fcc5c0898fc32cbaab85c70b26a08b4a97a8;p=thirdparty%2FFORT-validator.git

Fix 11: validate certificates against its corresponding CRL.

The OpenSSL function 'X509_STORE_CTX_set0_crls' wasn't taking into consideration the CRL stack; the simple way to check revocation was doing it manually.
Pay special attention to manifests, the CRL stack that's received when its EE is validated, is grandparent's CRL; so, in this case, validate manifests on its own CRLDP.
---

diff --git a/src/asn1/signed_data.c b/src/asn1/signed_data.c
index d4a1a7ef..e85206ff 100644
--- a/src/asn1/signed_data.c
+++ b/src/asn1/signed_data.c
@@ -24,7 +24,8 @@ int
 signed_object_args_init(struct signed_object_args *args,
     struct rpki_uri *uri,
     STACK_OF(X509_CRL) *crls,
-    bool force_inherit)
+    bool force_inherit,
+    bool use_crldp)
 {
 	args->res = resources_create(force_inherit);
 	if (args->res == NULL)
@@ -32,6 +33,7 @@ signed_object_args_init(struct signed_object_args *args,
 
 	args->uri = uri;
 	args->crls = crls;
+	args->use_crldp = use_crldp;
 	memset(&args->refs, 0, sizeof(args->refs));
 	return 0;
 }
@@ -63,6 +65,7 @@ static int
 handle_sdata_certificate(ANY_t *cert_encoded, struct signed_object_args *args,
     OCTET_STRING_t *sid, ANY_t *signedData, SignatureValue_t *signature)
 {
+	STACK_OF(X509_CRL) *crls;
 	const unsigned char *tmp;
 	X509 *cert;
 	enum rpki_policy policy;
@@ -90,25 +93,44 @@ handle_sdata_certificate(ANY_t *cert_encoded, struct signed_object_args *args,
 		goto end1;
 	}
 
-	error = certificate_validate_chain(cert, args->crls);
+	crls = args->crls;
+	if (args->use_crldp) {
+		crls = sk_X509_CRL_new_null();
+		if (crls == NULL) {
+			error = pr_enomem();
+			goto end2;
+		}
+	}
+
+	error = certificate_validate_chain(cert, crls);
 	if (error)
-		goto end2;
+		goto end3;
 	error = certificate_validate_rfc6487(cert, false);
 	if (error)
-		goto end2;
+		goto end3;
 	error = certificate_validate_extensions_ee(cert, sid, &args->refs,
 	    &policy);
 	if (error)
-		goto end2;
+		goto end3;
 	error = certificate_validate_signature(cert, signedData, signature);
 	if (error)
-		goto end2;
+		goto end3;
+
+	/* Validate in CRL at CRLDP */
+	if (args->use_crldp) {
+		error = certificate_revoked_at_crldp(cert, &args->refs);
+		if (error)
+			goto end3;
+	}
 
 	resources_set_policy(args->res, policy);
 	error = certificate_get_resources(cert, args->res);
 	if (error)
-		goto end2;
+		goto end3;
 
+end3:
+	if (args->use_crldp)
+		sk_X509_CRL_free(crls);
 end2:
 	X509_free(cert);
 end1:
diff --git a/src/asn1/signed_data.h b/src/asn1/signed_data.h
index ed0e5dbd..15a0d06e 100644
--- a/src/asn1/signed_data.h
+++ b/src/asn1/signed_data.h
@@ -18,6 +18,8 @@ struct signed_object_args {
 	STACK_OF(X509_CRL) *crls;
 	/** A copy of the resources carried by the embedded certificate. */
 	struct resources *res;
+	/** Check if the certificate is revoked at CRLDP, not at crls stack */
+	bool use_crldp;
 	/**
 	 * A bunch of URLs found in the embedded certificate's extensions,
 	 * recorded for future validation.
@@ -26,7 +28,7 @@ struct signed_object_args {
 };
 
 int signed_object_args_init(struct signed_object_args *, struct rpki_uri *,
-    STACK_OF(X509_CRL) *, bool);
+    STACK_OF(X509_CRL) *, bool, bool);
 void signed_object_args_cleanup(struct signed_object_args *);
 
 int signed_data_decode(ANY_t *, struct signed_object_args *args,
diff --git a/src/object/certificate.c b/src/object/certificate.c
index 37bc077c..b64926a0 100644
--- a/src/object/certificate.c
+++ b/src/object/certificate.c
@@ -15,6 +15,7 @@
 #include "asn1/oid.h"
 #include "asn1/asn1c/IPAddrBlocks.h"
 #include "crypto/hash.h"
+#include "object/crl.h"
 #include "object/name.h"
 #include "object/manifest.h"
 #include "rsync/rsync.h"
@@ -579,6 +580,24 @@ end:
 	return error;
 }
 
+static bool
+cert_revoked(ASN1_INTEGER *serialNumber, X509_CRL *crl)
+{
+	STACK_OF(X509_REVOKED) *revoked;
+	X509_REVOKED *item;
+	int index;
+
+	revoked = X509_CRL_get_REVOKED(crl);
+	for (index = 0; index < sk_X509_REVOKED_num(revoked); index++) {
+		item = sk_X509_REVOKED_value(revoked, index);
+		if (ASN1_INTEGER_cmp(X509_REVOKED_get0_serialNumber(item),
+		    serialNumber) == 0)
+			return true;
+	}
+
+	return false;
+}
+
 int
 certificate_validate_chain(X509 *cert, STACK_OF(X509_CRL) *crls)
 {
@@ -611,7 +630,21 @@ certificate_validate_chain(X509 *cert, STACK_OF(X509_CRL) *crls)
 
 	X509_STORE_CTX_trusted_stack(ctx,
 	    certstack_get_x509s(validation_certstack(state)));
-	X509_STORE_CTX_set0_crls(ctx, crls);
+
+	/*
+	 * The function 'X509_STORE_CTX_set0_crls' could be used with a
+	 * 'X509_VERIFY_PARAM' of 'X509_V_FLAG_CRL_CHECK', but this didn't
+	 * worked as expected.
+	 * 
+	 * Instead of that, fetch the last CRL (father's) and check revoked
+	 * serials "manually".
+	 */
+	if (sk_X509_CRL_num(crls) > 0 &&
+	    cert_revoked(X509_get_serialNumber(cert),
+	    sk_X509_CRL_value(crls, sk_X509_CRL_num(crls) - 1))) {
+		pr_err("Certificate validation failed: certificate is revoked");
+		goto abort;
+	}
 
 	/*
 	 * HERE'S THE MEAT OF LIBCRYPTO'S VALIDATION.
@@ -652,6 +685,35 @@ abort:
 	return -EINVAL;
 }
 
+/*
+ * Load the CRL at CRLDP @refs and check if @cert is revoked there
+ */
+int
+certificate_revoked_at_crldp(X509 *cert, struct certificate_refs *refs)
+{
+	X509_CRL *crl;
+	struct rpki_uri *uri;
+	int error;
+
+	error = uri_create_str(&uri, refs->crldp, strlen(refs->crldp));
+	if (error)
+		return error;
+
+	error = crl_load(uri, &crl);
+	if (error)
+		goto release_uri;
+
+	/* Everything OK so far, error 0 is valid */
+	if (cert_revoked(X509_get_serialNumber(cert), crl)) {
+		error = pr_err("Certificate validation failed: certificate is revoked at CRL");
+	}
+
+	X509_CRL_free(crl);
+release_uri:
+	uri_refput(uri);
+	return error;
+}
+
 static int
 handle_ip_extension(X509_EXTENSION *ext, struct resources *resources)
 {
diff --git a/src/object/certificate.h b/src/object/certificate.h
index 5167cf80..a11859c7 100644
--- a/src/object/certificate.h
+++ b/src/object/certificate.h
@@ -45,6 +45,8 @@ int certificate_get_resources(X509 *, struct resources *);
 int certificate_validate_extensions_ee(X509 *, OCTET_STRING_t *,
     struct certificate_refs *, enum rpki_policy *);
 
+int certificate_revoked_at_crldp(X509 *cert, struct certificate_refs *refs);
+
 int certificate_traverse(struct rpp *, struct rpki_uri *);
 
 #endif /* SRC_OBJECT_CERTIFICATE_H_ */
diff --git a/src/object/ghostbusters.c b/src/object/ghostbusters.c
index 42f82ca0..c25e6e07 100644
--- a/src/object/ghostbusters.c
+++ b/src/object/ghostbusters.c
@@ -28,7 +28,7 @@ ghostbusters_traverse(struct rpki_uri *uri, struct rpp *pp)
 	if (error)
 		goto end1;
 
-	error = signed_object_args_init(&sobj_args, uri, crl, true);
+	error = signed_object_args_init(&sobj_args, uri, crl, true, false);
 	if (error)
 		goto end1;
 
diff --git a/src/object/manifest.c b/src/object/manifest.c
index 9515ce7f..58485cae 100644
--- a/src/object/manifest.c
+++ b/src/object/manifest.c
@@ -217,7 +217,7 @@ handle_manifest(struct rpki_uri *uri, STACK_OF(X509_CRL) *crls, struct rpp **pp)
 	pr_debug_add("Manifest '%s' {", uri_get_printable(uri));
 	fnstack_push_uri(uri);
 
-	error = signed_object_args_init(&sobj_args, uri, crls, false);
+	error = signed_object_args_init(&sobj_args, uri, crls, false, true);
 	if (error)
 		goto end1;
 
diff --git a/src/object/roa.c b/src/object/roa.c
index e971f5fb..8db61b67 100644
--- a/src/object/roa.c
+++ b/src/object/roa.c
@@ -254,7 +254,7 @@ roa_traverse(struct rpki_uri *uri, struct rpp *pp)
 	if (error)
 		goto revert_fnstack;
 
-	error = signed_object_args_init(&sobj_args, uri, crl, false);
+	error = signed_object_args_init(&sobj_args, uri, crl, false, false);
 	if (error)
 		goto revert_fnstack;