[gnutls-devel] [PATCH] Check for all error conditions when verifying a certificate
Armin Burgmeier
armin at arbur.net
Fri Sep 12 20:16:06 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 | 164 ++++++++++++++++++++++++----------------------------
tests/test-chains.h | 13 +++--
2 files changed, 82 insertions(+), 95 deletions(-)
diff --git a/lib/x509/verify.c b/lib/x509/verify.c
index 81b9b4d..26dfd19 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)--;
@@ -599,31 +598,26 @@ verify_crt(gnutls_x509_crt_t cert,
* authorities.
*/
if (issuer == NULL) {
- out = GNUTLS_CERT_SIGNER_NOT_FOUND | GNUTLS_CERT_INVALID;
- if (output)
- *output |= out;
+ out |= GNUTLS_CERT_SIGNER_NOT_FOUND | GNUTLS_CERT_INVALID;
gnutls_assert();
result = 0;
- goto cleanup;
}
if (_issuer != NULL)
*_issuer = issuer;
- if (nc != NULL) {
+ if (nc != NULL && issuer != 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 =
+ out |=
GNUTLS_CERT_SIGNER_CONSTRAINTS_FAILURE |
GNUTLS_CERT_INVALID;
- if (output)
- *output |= out;
gnutls_assert();
result = 0;
- goto cleanup;
+ goto nc_done;
}
/* only check name constraints in server certificates, not CAs */
@@ -659,26 +653,26 @@ verify_crt(gnutls_x509_crt_t cert,
}
}
}
+ nc_done:
+
+ if(issuer != NULL)
+ issuer_version = gnutls_x509_crt_get_version(issuer);
+ else
+ issuer_version = 0;
- 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)) {
+ } else if (issuer != NULL &&
+ !(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 =
+ out |=
GNUTLS_CERT_SIGNER_NOT_CA |
GNUTLS_CERT_INVALID;
- if (output)
- *output |= out;
result = 0;
- goto cleanup;
}
ret =
@@ -686,13 +680,10 @@ verify_crt(gnutls_x509_crt_t cert,
if (ret >= 0) {
if (!(usage & GNUTLS_KEY_KEY_CERT_SIGN)) {
gnutls_assert();
- out =
+ out |=
GNUTLS_CERT_SIGNER_CONSTRAINTS_FAILURE
| GNUTLS_CERT_INVALID;
- if (output)
- *output |= out;
result = 0;
- goto cleanup;
}
}
}
@@ -703,7 +694,7 @@ verify_crt(gnutls_x509_crt_t cert,
if (ret < 0) {
result = 0;
gnutls_assert();
- goto cleanup;
+ cert_signed_data.data = NULL;
}
ret =
@@ -712,7 +703,7 @@ verify_crt(gnutls_x509_crt_t cert,
if (ret < 0) {
result = 0;
gnutls_assert();
- goto cleanup;
+ cert_signature.data = NULL;
}
ret =
@@ -721,75 +712,70 @@ 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 (sigalg >= 0 &&
+ cert_signed_data.data != NULL &&
+ cert_signature.data != NULL) {
+ if (issuer != NULL &&
+ is_level_acceptable(cert, issuer, sigalg, flags) == 0) {
+ gnutls_assert();
+ out |=
+ GNUTLS_CERT_INSECURE_ALGORITHM |
+ GNUTLS_CERT_INVALID;
+ result = 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;
- 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.
+ */
+ 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;
+ }
- hash_algo = gnutls_sign_get_hash_algorithm(sigalg);
- me = mac_to_entry(hash_algo);
- if (me == NULL) {
- gnutls_assert();
- result = 0;
- goto cleanup;
+ hash_algo = gnutls_sign_get_hash_algorithm(sigalg);
+ me = mac_to_entry(hash_algo);
+ if (me == NULL) {
+ gnutls_assert();
+ result = 0;
+ } else if (issuer != 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;
-
/* 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 +783,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;
@@ -1414,7 +1401,6 @@ gnutls_x509_crl_verify(gnutls_x509_crl_t crl,
*verify |=
GNUTLS_CERT_SIGNER_NOT_FOUND |
GNUTLS_CERT_INVALID;
- return 0;
}
if (!(flags & GNUTLS_VERIFY_DISABLE_CA_SIGN)) {
@@ -1424,7 +1410,6 @@ gnutls_x509_crl_verify(gnutls_x509_crl_t crl,
*verify |=
GNUTLS_CERT_SIGNER_NOT_CA |
GNUTLS_CERT_INVALID;
- return 0;
}
result =
@@ -1436,7 +1421,6 @@ gnutls_x509_crl_verify(gnutls_x509_crl_t crl,
*verify |=
GNUTLS_CERT_SIGNER_CONSTRAINTS_FAILURE
| GNUTLS_CERT_INVALID;
- return 0;
}
}
}
@@ -1504,10 +1488,10 @@ gnutls_x509_crl_verify(gnutls_x509_crl_t crl,
*verify |= GNUTLS_CERT_REVOCATION_DATA_SUPERSEDED;
- cleanup:
if (verify)
*verify |= GNUTLS_CERT_INVALID;
+ cleanup:
_gnutls_free_datum(&crl_signed_data);
_gnutls_free_datum(&crl_signature);
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