[gnutls-devel] [PATCH] Check for all error conditions when verifying a certificate

Armin Burgmeier armin at arbur.net
Sat Sep 13 17:46:42 CEST 2014


This allows to check for all possible flaws with a certificate chain with a
single call to gnutls_x509_crt_list_verify and friends.

Signed-off-by: Armin Burgmeier <armin at arbur.net>
---
 lib/x509/verify.c   | 393 +++++++++++++++++++++++++---------------------------
 tests/test-chains.h |  13 +-
 2 files changed, 194 insertions(+), 212 deletions(-)

diff --git a/lib/x509/verify.c b/lib/x509/verify.c
index 0302973..6c4ef5d 100644
--- a/lib/x509/verify.c
+++ b/lib/x509/verify.c
@@ -572,7 +572,7 @@ verify_crt(gnutls_x509_crt_t cert,
 	gnutls_datum_t cert_signature = { NULL, 0 };
 	gnutls_x509_crt_t issuer = NULL;
 	int issuer_version, hash_algo;
-	bool result = 0;
+	bool result = 1;
 	const mac_entry_st * me;
 	unsigned int out = 0, usage;
 	int sigalg, ret;
@@ -581,13 +581,12 @@ verify_crt(gnutls_x509_crt_t cert,
 		*output = 0;
 
 	if (*max_path == 0) {
-		out =
+		out |=
 		    GNUTLS_CERT_SIGNER_CONSTRAINTS_FAILURE |
 		    GNUTLS_CERT_INVALID;
-		if (output)
-			*output |= out;
 		gnutls_assert();
 		result = 0;
+		/* bail immediately, to avoid inconistency  */
 		goto cleanup;
 	}
 	(*max_path)--;
@@ -595,115 +594,16 @@ verify_crt(gnutls_x509_crt_t cert,
 	if (tcas_size >= 1)
 		issuer = find_issuer(cert, trusted_cas, tcas_size);
 
-	/* issuer is not in trusted certificate
-	 * authorities.
-	 */
-	if (issuer == NULL) {
-		out = GNUTLS_CERT_SIGNER_NOT_FOUND | GNUTLS_CERT_INVALID;
-		if (output)
-			*output |= out;
-		gnutls_assert();
-		result = 0;
-		goto cleanup;
-	}
-
 	if (_issuer != NULL)
 		*_issuer = issuer;
 
-	if (nc != NULL) {
-		/* append the issuer's constraints */
-		ret = gnutls_x509_crt_get_name_constraints(issuer, nc, 
-			GNUTLS_NAME_CONSTRAINTS_FLAG_APPEND, NULL);
-		if (ret < 0 && ret != GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) {
- nc_fail:
-			out =
-			    GNUTLS_CERT_SIGNER_CONSTRAINTS_FAILURE |
-			    GNUTLS_CERT_INVALID;
-			if (output)
-				*output |= out;
-			gnutls_assert();
-			result = 0;
-			goto cleanup;
-		}
-
-		/* only check name constraints in server certificates, not CAs */
-		if (end_cert != 0) {
-			ret = gnutls_x509_name_constraints_check_crt(nc, GNUTLS_SAN_DNSNAME, cert);
-			if (ret == 0) {
-				gnutls_assert();
-				goto nc_fail;
-			}
-
-			ret = gnutls_x509_name_constraints_check_crt(nc, GNUTLS_SAN_RFC822NAME, cert);
-			if (ret == 0) {
-				gnutls_assert();
-				goto nc_fail;
-			}
-
-			ret = gnutls_x509_name_constraints_check_crt(nc, GNUTLS_SAN_DN, cert);
-			if (ret == 0) {
-				gnutls_assert();
-				goto nc_fail;
-			}
-
-			ret = gnutls_x509_name_constraints_check_crt(nc, GNUTLS_SAN_URI, cert);
-			if (ret == 0) {
-				gnutls_assert();
-				goto nc_fail;
-			}
-
-			ret = gnutls_x509_name_constraints_check_crt(nc, GNUTLS_SAN_IPADDRESS, cert);
-			if (ret == 0) {
-				gnutls_assert();
-				goto nc_fail;
-			}
-		}
-	}
-
-	issuer_version = gnutls_x509_crt_get_version(issuer);
-	if (issuer_version < 0) {
-		gnutls_assert();
-		result = 0;
-		goto cleanup;
-	}
-
-	if (!(flags & GNUTLS_VERIFY_DISABLE_CA_SIGN) &&
-	    ((flags & GNUTLS_VERIFY_DO_NOT_ALLOW_X509_V1_CA_CRT)
-	     || issuer_version != 1)) {
-		if (check_if_ca(cert, issuer, max_path, flags) != 1) {
-			gnutls_assert();
-			out =
-			    GNUTLS_CERT_SIGNER_NOT_CA |
-			    GNUTLS_CERT_INVALID;
-			if (output)
-				*output |= out;
-			result = 0;
-			goto cleanup;
-		}
-
-		ret =
-		    gnutls_x509_crt_get_key_usage(issuer, &usage, NULL);
-		if (ret >= 0) {
-			if (!(usage & GNUTLS_KEY_KEY_CERT_SIGN)) {
-				gnutls_assert();
-				out =
-				    GNUTLS_CERT_SIGNER_CONSTRAINTS_FAILURE
-				    | GNUTLS_CERT_INVALID;
-				if (output)
-					*output |= out;
-				result = 0;
-				goto cleanup;
-			}
-		}
-	}
-
 	ret =
 	    _gnutls_x509_get_signed_data(cert->cert, &cert->der, "tbsCertificate",
 					 &cert_signed_data);
 	if (ret < 0) {
 		result = 0;
 		gnutls_assert();
-		goto cleanup;
+		cert_signed_data.data = NULL;
 	}
 
 	ret =
@@ -712,7 +612,7 @@ verify_crt(gnutls_x509_crt_t cert,
 	if (ret < 0) {
 		result = 0;
 		gnutls_assert();
-		goto cleanup;
+		cert_signature.data = NULL;
 	}
 
 	ret =
@@ -721,75 +621,156 @@ verify_crt(gnutls_x509_crt_t cert,
 	if (ret < 0) {
 		result = 0;
 		gnutls_assert();
-		goto cleanup;
 	}
 	sigalg = ret;
 
-	if (is_level_acceptable(cert, issuer, sigalg, flags) == 0) {
-		gnutls_assert();
-		out =
-		    GNUTLS_CERT_INSECURE_ALGORITHM |
-		    GNUTLS_CERT_INVALID;
-		if (output)
-			*output |= out;
-		result = 0;
-		goto cleanup;
-	}
-
-	/* If the certificate is not self signed check if the algorithms
-	 * used are secure. If the certificate is self signed it doesn't
-	 * really matter.
+	/* issuer is not in trusted certificate
+	 * authorities.
 	 */
-	if (gnutls_sign_is_secure(sigalg) == 0 &&
-	    is_broken_allowed(sigalg, flags) == 0 &&
-	    is_issuer(cert, cert) == 0) {
+	if (issuer == NULL) {
+		out |= GNUTLS_CERT_SIGNER_NOT_FOUND | GNUTLS_CERT_INVALID;
 		gnutls_assert();
-		out =
-		    GNUTLS_CERT_INSECURE_ALGORITHM |
-		    GNUTLS_CERT_INVALID;
-		if (output)
-			*output |= out;
 		result = 0;
-		goto cleanup;
-	}
+	} else {
+		if (nc != NULL) {
+			/* append the issuer's constraints */
+			ret = gnutls_x509_crt_get_name_constraints(issuer, nc, 
+				GNUTLS_NAME_CONSTRAINTS_FLAG_APPEND, NULL);
+			if (ret < 0 && ret != GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) {
+ nc_fail:
+				out |=
+				    GNUTLS_CERT_SIGNER_CONSTRAINTS_FAILURE |
+				    GNUTLS_CERT_INVALID;
+				gnutls_assert();
+				result = 0;
+				goto nc_done;
+			}
 
-	hash_algo = gnutls_sign_get_hash_algorithm(sigalg);
-	me = mac_to_entry(hash_algo);
-	if (me == NULL) {
-		gnutls_assert();
-		result = 0;
-		goto cleanup;
+			/* only check name constraints in server certificates, not CAs */
+			if (end_cert != 0) {
+				ret = gnutls_x509_name_constraints_check_crt(nc, GNUTLS_SAN_DNSNAME, cert);
+				if (ret == 0) {
+					gnutls_assert();
+					goto nc_fail;
+				}
+
+				ret = gnutls_x509_name_constraints_check_crt(nc, GNUTLS_SAN_RFC822NAME, cert);
+				if (ret == 0) {
+					gnutls_assert();
+					goto nc_fail;
+				}
+
+				ret = gnutls_x509_name_constraints_check_crt(nc, GNUTLS_SAN_DN, cert);
+				if (ret == 0) {
+					gnutls_assert();
+					goto nc_fail;
+				}
+
+				ret = gnutls_x509_name_constraints_check_crt(nc, GNUTLS_SAN_URI, cert);
+				if (ret == 0) {
+					gnutls_assert();
+					goto nc_fail;
+				}
+
+				ret = gnutls_x509_name_constraints_check_crt(nc, GNUTLS_SAN_IPADDRESS, cert);
+				if (ret == 0) {
+					gnutls_assert();
+					goto nc_fail;
+				}
+			}
+		}
+ nc_done:
+
+		issuer_version = gnutls_x509_crt_get_version(issuer);
+
+		if (issuer_version < 0) {
+			gnutls_assert();
+			result = 0;
+		} else if (!(flags & GNUTLS_VERIFY_DISABLE_CA_SIGN) &&
+	                   ((flags & GNUTLS_VERIFY_DO_NOT_ALLOW_X509_V1_CA_CRT)
+	                    || issuer_version != 1)) {
+			if (check_if_ca(cert, issuer, max_path, flags) != 1) {
+				gnutls_assert();
+				out |=
+				    GNUTLS_CERT_SIGNER_NOT_CA |
+				    GNUTLS_CERT_INVALID;
+				result = 0;
+			}
+
+			ret =
+			    gnutls_x509_crt_get_key_usage(issuer, &usage, NULL);
+			if (ret >= 0) {
+				if (!(usage & GNUTLS_KEY_KEY_CERT_SIGN)) {
+					gnutls_assert();
+					out |=
+					    GNUTLS_CERT_SIGNER_CONSTRAINTS_FAILURE
+					    | GNUTLS_CERT_INVALID;
+					result = 0;
+				}
+			}
+		}
+
+		if (sigalg >= 0 &&
+		    is_level_acceptable(cert, issuer, sigalg, flags) == 0) {
+			gnutls_assert();
+			out |=
+			    GNUTLS_CERT_INSECURE_ALGORITHM |
+			    GNUTLS_CERT_INVALID;
+			result = 0;
+		}
+
+		hash_algo = gnutls_sign_get_hash_algorithm(sigalg);
+		me = mac_to_entry(hash_algo);
+		if (me == NULL) {
+			gnutls_assert();
+			result = 0;
+		} else if (cert_signed_data.data != NULL &&
+		           cert_signature.data != NULL) {
+			ret =
+			    _gnutls_x509_verify_data(me,
+						     &cert_signed_data,
+		        	                     &cert_signature,
+						     issuer);
+			if (ret == GNUTLS_E_PK_SIG_VERIFY_FAILED) {
+				gnutls_assert();
+				out |=
+				    GNUTLS_CERT_INVALID |
+				    GNUTLS_CERT_SIGNATURE_FAILURE;
+				/* error. ignore it */
+				result = 0;
+			} else if (ret < 0) {
+				result = 0;
+				gnutls_assert();
+			}
+		}
 	}
 
-	ret =
-	    _gnutls_x509_verify_data(me,
-				     &cert_signed_data, &cert_signature,
-				     issuer);
-	if (ret == GNUTLS_E_PK_SIG_VERIFY_FAILED) {
-		gnutls_assert();
-		out |= GNUTLS_CERT_INVALID | GNUTLS_CERT_SIGNATURE_FAILURE;
-		/* error. ignore it */
-		if (output)
-			*output |= out;
-		result = 0;
-	} else if (ret < 0) {
-		result = 0;
-		gnutls_assert();
-		goto cleanup;
-	} else if (ret == 1)
-		result = 1;
+	if (sigalg >= 0) {
+		/* If the certificate is not self signed check if the algorithms
+		 * used are secure. If the certificate is self signed it doesn't
+		 * really matter.
+		 */
+		if (gnutls_sign_is_secure(sigalg) == 0 &&
+		    is_broken_allowed(sigalg, flags) == 0 &&
+		    is_issuer(cert, cert) == 0) {
+			gnutls_assert();
+			out |=
+			    GNUTLS_CERT_INSECURE_ALGORITHM |
+			    GNUTLS_CERT_INVALID;
+			result = 0;
+		}
+	}
 
 	/* Check activation/expiration times
 	 */
 	if (!(flags & GNUTLS_VERIFY_DISABLE_TIME_CHECKS)) {
 		/* check the time of the issuer first */
-		if (!(flags & GNUTLS_VERIFY_DISABLE_TRUSTED_TIME_CHECKS)) {
+		if (issuer != NULL &&
+		    !(flags & GNUTLS_VERIFY_DISABLE_TRUSTED_TIME_CHECKS)) {
 			out |= check_time_status(issuer, now);
 			if (out != 0) {
 				gnutls_assert();
 				result = 0;
-				if (output)
-					*output |= out;
 			}
 		}
 
@@ -797,12 +778,13 @@ verify_crt(gnutls_x509_crt_t cert,
 		if (out != 0) {
 			gnutls_assert();
 			result = 0;
-			if (output)
-				*output |= out;
 		}
 	}
 
       cleanup:
+	if (output)
+		*output |= out;
+
 	if (func) {
 		if (result == 0) {
 			out |= GNUTLS_CERT_INVALID;
@@ -1405,42 +1387,6 @@ gnutls_x509_crl_verify(gnutls_x509_crl_t crl,
 	if (tcas_size >= 1)
 		issuer = find_crl_issuer(crl, trusted_cas, tcas_size);
 
-	/* issuer is not in trusted certificate
-	 * authorities.
-	 */
-	if (issuer == NULL) {
-		gnutls_assert();
-		if (verify)
-			*verify |=
-			    GNUTLS_CERT_SIGNER_NOT_FOUND |
-			    GNUTLS_CERT_INVALID;
-		return 0;
-	}
-
-	if (!(flags & GNUTLS_VERIFY_DISABLE_CA_SIGN)) {
-		if (gnutls_x509_crt_get_ca_status(issuer, NULL) != 1) {
-			gnutls_assert();
-			if (verify)
-				*verify |=
-				    GNUTLS_CERT_SIGNER_NOT_CA |
-				    GNUTLS_CERT_INVALID;
-			return 0;
-		}
-
-		result =
-		    gnutls_x509_crt_get_key_usage(issuer, &usage, NULL);
-		if (result >= 0) {
-			if (!(usage & GNUTLS_KEY_CRL_SIGN)) {
-				gnutls_assert();
-				if (verify)
-					*verify |=
-					    GNUTLS_CERT_SIGNER_CONSTRAINTS_FAILURE
-					    | GNUTLS_CERT_INVALID;
-				return 0;
-			}
-		}
-	}
-
 	result =
 	    _gnutls_x509_get_signed_data(crl->crl, &crl->der, "tbsCertList",
 					 &crl_signed_data);
@@ -1473,21 +1419,54 @@ gnutls_x509_crl_verify(gnutls_x509_crl_t crl,
 
 	hash_algo = gnutls_sign_get_hash_algorithm(result);
 
-	result =
-	    _gnutls_x509_verify_data(mac_to_entry(hash_algo),
-				     &crl_signed_data, &crl_signature,
-				     issuer);
-	if (result == GNUTLS_E_PK_SIG_VERIFY_FAILED) {
-		gnutls_assert();
-		/* error. ignore it */
-		if (verify)
-			*verify |= GNUTLS_CERT_SIGNATURE_FAILURE;
-		result = 0;
-	} else if (result < 0) {
+	/* issuer is not in trusted certificate
+	 * authorities.
+	 */
+	if (issuer == NULL) {
 		gnutls_assert();
 		if (verify)
-			*verify |= GNUTLS_CERT_INVALID;
-		goto cleanup;
+			*verify |=
+			    GNUTLS_CERT_SIGNER_NOT_FOUND |
+			    GNUTLS_CERT_INVALID;
+	} else {
+		if (!(flags & GNUTLS_VERIFY_DISABLE_CA_SIGN)) {
+			if (gnutls_x509_crt_get_ca_status(issuer, NULL) != 1) {
+				gnutls_assert();
+				if (verify)
+					*verify |=
+					    GNUTLS_CERT_SIGNER_NOT_CA |
+					    GNUTLS_CERT_INVALID;
+			}
+
+			result =
+			    gnutls_x509_crt_get_key_usage(issuer, &usage, NULL);
+			if (result >= 0) {
+				if (!(usage & GNUTLS_KEY_CRL_SIGN)) {
+					gnutls_assert();
+					if (verify)
+						*verify |=
+						    GNUTLS_CERT_SIGNER_CONSTRAINTS_FAILURE
+						    | GNUTLS_CERT_INVALID;
+				}
+			}
+		}
+
+		result =
+		    _gnutls_x509_verify_data(mac_to_entry(hash_algo),
+					     &crl_signed_data, &crl_signature,
+					     issuer);
+		if (result == GNUTLS_E_PK_SIG_VERIFY_FAILED) {
+			gnutls_assert();
+			/* error. ignore it */
+			if (verify)
+				*verify |= GNUTLS_CERT_SIGNATURE_FAILURE;
+			result = 0;
+		} else if (result < 0) {
+			gnutls_assert();
+			if (verify)
+				*verify |= GNUTLS_CERT_INVALID;
+			goto cleanup;
+		}
 	}
 
 	{
diff --git a/tests/test-chains.h b/tests/test-chains.h
index 28974e1..ff9086f 100644
--- a/tests/test-chains.h
+++ b/tests/test-chains.h
@@ -1366,9 +1366,11 @@ static struct
 } chains[] =
 {
   { "CVE-2014-0092", cve_2014_0092_check, &cve_2014_0092_check[1],
-    0, GNUTLS_CERT_SIGNER_NOT_CA | GNUTLS_CERT_INVALID },
+    GNUTLS_VERIFY_DISABLE_TIME_CHECKS,
+    GNUTLS_CERT_SIGNER_NOT_CA | GNUTLS_CERT_INVALID },
   { "CVE-2008-4989", cve_2008_4989_chain, &cve_2008_4989_chain[2],
-    0, GNUTLS_CERT_SIGNER_NOT_FOUND | GNUTLS_CERT_INVALID },
+    GNUTLS_VERIFY_DISABLE_TIME_CHECKS,
+    GNUTLS_CERT_SIGNER_NOT_FOUND | GNUTLS_CERT_INVALID },
   { "amazon.com ok", verisign_com_chain_g5, &verisign_com_chain_g5[4],
     GNUTLS_VERIFY_DISABLE_TIME_CHECKS | GNUTLS_PROFILE_TO_VFLAGS(GNUTLS_PROFILE_LOW),
     0 },
@@ -1404,9 +1406,10 @@ static struct
     GNUTLS_VERIFY_DISABLE_TIME_CHECKS,
     0 },
   { "rsa-md5 fail", mayfirst_chain, &mayfirst_chain[1],
-    0, GNUTLS_CERT_INSECURE_ALGORITHM | GNUTLS_CERT_INVALID },
+    GNUTLS_VERIFY_DISABLE_TIME_CHECKS,
+    GNUTLS_CERT_INSECURE_ALGORITHM | GNUTLS_CERT_INVALID },
   { "rsa-md5 not ok", mayfirst_chain, &mayfirst_chain[1],
-    GNUTLS_VERIFY_ALLOW_SIGN_RSA_MD2,
+    GNUTLS_VERIFY_DISABLE_TIME_CHECKS | GNUTLS_VERIFY_ALLOW_SIGN_RSA_MD2,
     GNUTLS_CERT_INSECURE_ALGORITHM | GNUTLS_CERT_INVALID },
   { "rsa-md5 not ok2", mayfirst_chain, &mayfirst_chain[1],
     GNUTLS_VERIFY_ALLOW_SIGN_RSA_MD5,
@@ -1449,7 +1452,7 @@ static struct
     GNUTLS_VERIFY_ALLOW_SIGN_RSA_MD5, 0 },
   { "cacertrsamd5 short-cut not ok", cacertrsamd5, &cacertrsamd5[0],
     GNUTLS_VERIFY_DO_NOT_ALLOW_SAME,
-    GNUTLS_CERT_SIGNER_NOT_FOUND | GNUTLS_CERT_INVALID },
+    GNUTLS_CERT_SIGNER_NOT_FOUND | GNUTLS_CERT_INSECURE_ALGORITHM | GNUTLS_CERT_INVALID },
   { "cacertrsamd5 short-cut ok", cacertrsamd5, &cacertrsamd5[1],
     0, 0 },
   { "ecc cert ok", ecc_cert, &ecc_cert[1], GNUTLS_PROFILE_TO_VFLAGS(GNUTLS_PROFILE_HIGH), 0 },
-- 
2.1.0





More information about the Gnutls-devel mailing list