x509: reject certificates having duplicate extensions

According to RFC5280 a certificate must not include more than
one instance of a particular extension. We were previously printing
warnings when such extensions were found, but that is insufficient
to flag such certificates. Instead, refuse to import them.

Resolves: #887

Signed-off-by: Nikos Mavrogiannopoulos <nmav@gnutls.org>

3 files changed, 82 insertions, 25 deletions

diff --git a/lib/errors.c b/lib/errors.c
index 0ba7106cfa..842afe0e24 100644
--- a/lib/errors.c
+++ b/lib/errors.c
@@ -186,6 +186,8 @@ static const gnutls_error_entry error_entries[] = {
 		    GNUTLS_E_X509_UNSUPPORTED_CRITICAL_EXTENSION),
 	ERROR_ENTRY(N_("Unsupported extension in X.509 certificate."),
 		    GNUTLS_E_X509_UNSUPPORTED_EXTENSION),
+	ERROR_ENTRY(N_("Duplicate extension in X.509 certificate."),
+		    GNUTLS_E_X509_DUPLICATE_EXTENSION),
 	ERROR_ENTRY(N_
 		    ("Key usage violation in certificate has been detected."),
 		    GNUTLS_E_KEY_USAGE_VIOLATION),
diff --git a/lib/includes/gnutls/gnutls.h.in b/lib/includes/gnutls/gnutls.h.in
index d8464c94da..fb617f9963 100644
--- a/lib/includes/gnutls/gnutls.h.in
+++ b/lib/includes/gnutls/gnutls.h.in
@@ -3386,6 +3386,7 @@ void gnutls_fips140_set_mode(gnutls_fips_mode_t mode, unsigned flags);
 #define GNUTLS_E_MISSING_EXTENSION -427
 #define GNUTLS_E_DB_ENTRY_EXISTS -428
 #define GNUTLS_E_EARLY_DATA_REJECTED -429
+#define GNUTLS_E_X509_DUPLICATE_EXTENSION -430
 
 #define GNUTLS_E_UNIMPLEMENTED_FEATURE -1250
 
diff --git a/lib/x509/x509.c b/lib/x509/x509.c
index 62e7abe8e9..bf3e1e8f16 100644
--- a/lib/x509/x509.c
+++ b/lib/x509/x509.c
@@ -38,6 +38,8 @@
 #include <pkcs11_int.h>
 #include "urls.h"
 #include "system-keys.h"
+#include "hash.h"
+#include "hash-pjw-bare.h"
 
 static int crt_reinit(gnutls_x509_crt_t crt)
 {
@@ -404,41 +406,94 @@ static int cache_alt_names(gnutls_x509_crt_t cert)
 	return 0;
 }
 
+static bool hcomparator(const void *v1, const void *v2)
+{
+	return (strcmp(v1, v2)==0);
+}
+
+static size_t hhasher(const void *entry, size_t n)
+{
+	const char *e = entry;
+	if (e == NULL || e[0] == 0)
+		return 0;
+
+	return hash_pjw_bare(e, strlen(e)) % n;
+}
+
 int _gnutls_check_cert_sanity(gnutls_x509_crt_t cert)
 {
-	int result = 0, version;
+	int ret = 0, version;
 	gnutls_datum_t exts;
+	Hash_table *htable = NULL;
 
 	if (cert->flags & GNUTLS_X509_CRT_FLAG_IGNORE_SANITY)
 		return 0;
 
 	/* enforce the rule that only version 3 certificates carry extensions */
-	result = gnutls_x509_crt_get_version(cert);
-	if (result < 0) {
-		gnutls_assert();
-		goto cleanup;
+	ret = gnutls_x509_crt_get_version(cert);
+	if (ret < 0) {
+		return gnutls_assert_val(ret);
 	}
 
-	version = result;
+	version = ret;
 
 	if (version < 3) {
 		if (!cert->modified) {
-			result = _gnutls_x509_get_raw_field2(cert->cert, &cert->der,
+			ret = _gnutls_x509_get_raw_field2(cert->cert, &cert->der,
 				"tbsCertificate.extensions", &exts);
-			if (result >= 0 && exts.size > 0) {
-				gnutls_assert();
+			if (ret >= 0 && exts.size > 0) {
 				_gnutls_debug_log("error: extensions present in certificate with version %d\n", version);
-				result = GNUTLS_E_X509_CERTIFICATE_ERROR;
-				goto cleanup;
+				return gnutls_assert_val(GNUTLS_E_X509_CERTIFICATE_ERROR);
 			}
 		} else {
 			if (cert->use_extensions) {
-				gnutls_assert();
 				_gnutls_debug_log("error: extensions set in certificate with version %d\n", version);
-				result = GNUTLS_E_X509_CERTIFICATE_ERROR;
+				return gnutls_assert_val(GNUTLS_E_X509_CERTIFICATE_ERROR);
+			}
+		}
+	} else {
+		/* Version is >= 3; ensure no duplicate extensions are
+		 * present. */
+		unsigned i;
+		char oid[MAX_OID_SIZE];
+		size_t oid_size;
+		char *o;
+
+		htable = hash_initialize(16, NULL, hhasher, hcomparator, gnutls_free);
+		if (htable == NULL)
+			return gnutls_assert_val(GNUTLS_E_MEMORY_ERROR);
+
+		for (i=0;;i++) {
+			oid_size = sizeof(oid);
+			ret = gnutls_x509_crt_get_extension_info(cert, i, oid, &oid_size, NULL);
+			if (ret < 0) {
+				if (ret == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE)
+					break;
+				gnutls_assert();
+				goto cleanup;
+			}
+			o = gnutls_strdup(oid);
+			if (o == NULL) {
+				ret = gnutls_assert_val(GNUTLS_E_MEMORY_ERROR);
+				goto cleanup;
+			}
+
+			ret = hash_insert_if_absent(htable, o, NULL);
+			if (ret == -1) {
+				gnutls_free(o);
+				ret = gnutls_assert_val(GNUTLS_E_MEMORY_ERROR);
+				goto cleanup;
+			} else if (ret == 0) {
+				/* duplicate */
+				gnutls_free(o);
+				_gnutls_debug_log("error: duplicate extension (%s) detected\n", oid);
+				ret = gnutls_assert_val(GNUTLS_E_X509_DUPLICATE_EXTENSION);
 				goto cleanup;
 			}
 		}
+
+		hash_free(htable);
+		htable = NULL;
 	}
 
 	if (version < 2) {
@@ -446,36 +501,35 @@ int _gnutls_check_cert_sanity(gnutls_x509_crt_t cert)
 		size_t id_size;
 
 		id_size = sizeof(id);
-		result = gnutls_x509_crt_get_subject_unique_id(cert, id, &id_size);
-		if (result >= 0 || result == GNUTLS_E_SHORT_MEMORY_BUFFER) {
-			gnutls_assert();
+		ret = gnutls_x509_crt_get_subject_unique_id(cert, id, &id_size);
+		if (ret >= 0 || ret == GNUTLS_E_SHORT_MEMORY_BUFFER) {
 			_gnutls_debug_log("error: subjectUniqueID present in certificate with version %d\n", version);
-			result = GNUTLS_E_X509_CERTIFICATE_ERROR;
+			ret = gnutls_assert_val(GNUTLS_E_X509_CERTIFICATE_ERROR);
 			goto cleanup;
 		}
 
 		id_size = sizeof(id);
-		result = gnutls_x509_crt_get_issuer_unique_id(cert, id, &id_size);
-		if (result >= 0 || result == GNUTLS_E_SHORT_MEMORY_BUFFER) {
-			gnutls_assert();
+		ret = gnutls_x509_crt_get_issuer_unique_id(cert, id, &id_size);
+		if (ret >= 0 || ret == GNUTLS_E_SHORT_MEMORY_BUFFER) {
 			_gnutls_debug_log("error: subjectUniqueID present in certificate with version %d\n", version);
-			result = GNUTLS_E_X509_CERTIFICATE_ERROR;
+			ret = gnutls_assert_val(GNUTLS_E_X509_CERTIFICATE_ERROR);
 			goto cleanup;
 		}
 	}
 
 	if (gnutls_x509_crt_get_expiration_time(cert) == -1 ||
 	    gnutls_x509_crt_get_activation_time(cert) == -1) {
-		gnutls_assert();
 		_gnutls_debug_log("error: invalid expiration or activation time in certificate\n");
-		result = GNUTLS_E_CERTIFICATE_TIME_ERROR;
+		ret = gnutls_assert_val(GNUTLS_E_CERTIFICATE_TIME_ERROR);
 		goto cleanup;
 	}
 
-	result = 0;
+	ret = 0;
 
  cleanup:
-	return result;
+	if (htable)
+		hash_free(htable);
+	return ret;
 }
 
 /**