sign/digest: separate "brokenness" of signatures and hash algorithms

That is, allow digital signatures to be marked as broken irrespective
of their used hash, and restrict hash brokenness to preimage resistance.

Signed-off-by: Nikos Mavrogiannopoulos <nmav@redhat.com>

7 files changed, 57 insertions, 51 deletions

diff --git a/lib/algorithms.h b/lib/algorithms.h
index 30508e4e02..f9cb6ff1d9 100644
--- a/lib/algorithms.h
+++ b/lib/algorithms.h
@@ -153,22 +153,13 @@ inline static int _gnutls_mac_get_key_size(const mac_entry_st * e)
 #define _gnutls_digest_get_name _gnutls_mac_get_name
 #define _gnutls_hash_get_algo_len _gnutls_mac_get_algo_len
 
-/* Check generic-purpose security */
+/* Security against pre-image attacks */
 inline static int _gnutls_digest_is_secure(const mac_entry_st * e)
 {
 	if (unlikely(e == NULL))
 		return 0;
 	else
-		return (e->slevel==_SECURE || e->slevel == _INSECURE_FOR_CERTS)?1:0;
-}
-
-/* Check certificate use security */
-inline static int _gnutls_digest_is_secure_for_certs(const mac_entry_st * e)
-{
-	if (unlikely(e == NULL))
-		return 0;
-	else
-		return (e->slevel==_SECURE)?1:0;
+		return (e->preimage_insecure==0);
 }
 
 /* Functions for cipher suites. */
@@ -308,6 +299,12 @@ enum encipher_type _gnutls_kx_encipher_type(gnutls_kx_algorithm_t
 
 /* Functions for sign algorithms. */
 
+typedef enum hash_security_level_t {
+	_SECURE,
+	_INSECURE_FOR_CERTS,
+	_INSECURE
+} hash_security_level_t;
+
 struct gnutls_sign_entry_st {
 	const char *name;
 	const char *oid;
@@ -317,6 +314,7 @@ struct gnutls_sign_entry_st {
 	/* See RFC 5246 HashAlgorithm and SignatureAlgorithm
 	   for values to use in aid struct. */
 	const sign_algorithm_st aid;
+	hash_security_level_t slevel;	/* contains values of hash_security_level_t */
 };
 typedef struct gnutls_sign_entry_st gnutls_sign_entry_st;
 
@@ -324,6 +322,8 @@ const gnutls_sign_entry_st *_gnutls_sign_to_entry(gnutls_sign_algorithm_t sign);
 const gnutls_sign_entry_st *_gnutls_pk_to_sign_entry(gnutls_pk_algorithm_t, gnutls_digest_algorithm_t);
 const gnutls_sign_entry_st *_gnutls_oid_to_sign_entry(const char *oid);
 
+bool _gnutls_sign_is_secure2(const gnutls_sign_entry_st *se, unsigned int flags);
+
 gnutls_pk_algorithm_t _gnutls_x509_sign_to_pk(gnutls_sign_algorithm_t
 					      sign);
 const char *_gnutls_x509_sign_to_oid(gnutls_pk_algorithm_t,
diff --git a/lib/algorithms/mac.c b/lib/algorithms/mac.c
index 8c338c9477..776e44c2be 100644
--- a/lib/algorithms/mac.c
+++ b/lib/algorithms/mac.c
@@ -32,12 +32,6 @@
 #define MAC_OID_SHA384 "1.2.840.113549.2.10"
 #define MAC_OID_SHA512 "1.2.840.113549.2.11"
 
-#ifdef ALLOW_SHA1
-# define SHA1_SECURE_VAL _SECURE
-#else
-# define SHA1_SECURE_VAL _INSECURE_FOR_CERTS
-#endif
-
 static const mac_entry_st hash_algorithms[] = {
 	{.name = "SHA1",
 	 .oid = HASH_OID_SHA1,
@@ -45,13 +39,12 @@ static const mac_entry_st hash_algorithms[] = {
 	 .id = GNUTLS_MAC_SHA1,
 	 .output_size = 20,
 	 .key_size = 20,
-	 .slevel = SHA1_SECURE_VAL,
 	 .block_size = 64},
 	{.name = "MD5+SHA1",
 	 .id = GNUTLS_MAC_MD5_SHA1,
 	 .output_size = 36,
 	 .key_size = 36,
-	 .slevel = _INSECURE,
+	 .preimage_insecure = 1,
 	 .block_size = 64},
 	{.name = "SHA256",
 	 .oid = HASH_OID_SHA256,
@@ -123,18 +116,17 @@ static const mac_entry_st hash_algorithms[] = {
 	 .id = GNUTLS_MAC_MD5,
 	 .output_size = 16,
 	 .key_size = 16,
-	 .slevel = _INSECURE,
+	 .preimage_insecure = 1,
 	 .block_size = 64},
 	{.name = "MD2",
 	 .oid = HASH_OID_MD2,
-	 .slevel = _INSECURE,
+	 .preimage_insecure = 1,
 	 .id = GNUTLS_MAC_MD2},
 	{.name = "RIPEMD160",
 	 .oid = HASH_OID_RMD160,
 	 .id = GNUTLS_MAC_RMD160,
 	 .output_size = 20,
 	 .key_size = 20,
-	 .slevel = _INSECURE_FOR_CERTS,
 	 .block_size = 64},
 	{.name = "MAC-NULL",
 	 .id = GNUTLS_MAC_NULL},
diff --git a/lib/algorithms/sign.c b/lib/algorithms/sign.c
index 265dca9708..2809acf7ec 100644
--- a/lib/algorithms/sign.c
+++ b/lib/algorithms/sign.c
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2011-2012 Free Software Foundation, Inc.
+ * Copyright (C) 2017 Red Hat, Inc.
  *
  * Author: Nikos Mavrogiannopoulos
  *
@@ -29,6 +30,12 @@
 /* signature algorithms;
  */
 
+#ifdef ALLOW_SHA1
+# define SHA1_SECURE_VAL _SECURE
+#else
+# define SHA1_SECURE_VAL _INSECURE_FOR_CERTS
+#endif
+
 /* Signature algorithms may be listed twice with a different PK algorithm,
  * e.g., RSA-PSS-SHA256 can be generated by GNUTLS_PK_RSA or GNUTLS_PK_RSA_PSS.
  */
@@ -38,11 +45,13 @@ static const gnutls_sign_entry_st sign_algorithms[] = {
 	 .id = GNUTLS_SIGN_RSA_SHA1,
 	 .pk = GNUTLS_PK_RSA,
 	 .hash = GNUTLS_DIG_SHA1,
+	 .slevel = SHA1_SECURE_VAL,
 	 .aid = {{2, 1}}},
 	{.name = "RSA-SHA1",
 	 .oid = ISO_SIG_RSA_SHA1_OID, 
 	 .id = GNUTLS_SIGN_RSA_SHA1,
 	 .pk = GNUTLS_PK_RSA,
+	 .slevel = SHA1_SECURE_VAL,
 	 .hash = GNUTLS_DIG_SHA1,
 	 .aid = {{2, 1}}},
 	{.name = "RSA-SHA224",
@@ -74,11 +83,13 @@ static const gnutls_sign_entry_st sign_algorithms[] = {
 	 .id = GNUTLS_SIGN_RSA_RMD160,
 	 .pk = GNUTLS_PK_RSA,
 	 .hash = GNUTLS_DIG_RMD160,
+	 .slevel = _INSECURE_FOR_CERTS,
 	 .aid = TLS_SIGN_AID_UNKNOWN},
 	{.name = "DSA-SHA1",
 	 .oid = SIG_DSA_SHA1_OID, 
 	 .id = GNUTLS_SIGN_DSA_SHA1,
 	 .pk = GNUTLS_PK_DSA,
+	 .slevel = SHA1_SECURE_VAL,
 	 .hash = GNUTLS_DIG_SHA1,
 	 .aid = {{2, 2}}},
 	{.name = "DSA-SHA1",
@@ -86,6 +97,7 @@ static const gnutls_sign_entry_st sign_algorithms[] = {
 	 .id = GNUTLS_SIGN_DSA_SHA1,
 	 .pk = GNUTLS_PK_DSA,
 	 .hash = GNUTLS_DIG_SHA1,
+	 .slevel = SHA1_SECURE_VAL,
 	 .aid = {{2, 2}}},
 	{.name = "DSA-SHA224",
 	 .oid = SIG_DSA_SHA224_OID, 
@@ -104,23 +116,27 @@ static const gnutls_sign_entry_st sign_algorithms[] = {
 	 .id = GNUTLS_SIGN_RSA_MD5,
 	 .pk = GNUTLS_PK_RSA,
 	 .hash = GNUTLS_DIG_MD5,
+	 .slevel = _INSECURE,
 	 .aid = {{1, 1}}},
 	{.name = "RSA-MD5",
 	 .oid = "1.3.14.3.2.25", 
 	 .id = GNUTLS_SIGN_RSA_MD5,
 	 .pk = GNUTLS_PK_RSA,
 	 .hash = GNUTLS_DIG_MD5,
+	 .slevel = _INSECURE,
 	 .aid = {{1, 1}}},
 	{.name = "RSA-MD2",
 	 .oid = SIG_RSA_MD2_OID, 
 	 .id = GNUTLS_SIGN_RSA_MD2,
 	 .pk = GNUTLS_PK_RSA,
 	 .hash = GNUTLS_DIG_MD2,
+	 .slevel = _INSECURE,
 	 .aid = TLS_SIGN_AID_UNKNOWN},
 	{.name = "ECDSA-SHA1",
 	 .oid = "1.2.840.10045.4.1", 
 	 .id = GNUTLS_SIGN_ECDSA_SHA1,
 	 .pk = GNUTLS_PK_EC,
+	 .slevel = SHA1_SECURE_VAL,
 	 .hash = GNUTLS_DIG_SHA1,
 	 .aid = {{2, 3}}},
 	{.name = "ECDSA-SHA224",
@@ -331,6 +347,14 @@ unsigned gnutls_sign_is_secure(gnutls_sign_algorithm_t algorithm)
 	return gnutls_sign_is_secure2(algorithm, 0);
 }
 
+bool _gnutls_sign_is_secure2(const gnutls_sign_entry_st *se, unsigned int flags)
+{
+	if (flags & GNUTLS_SIGN_FLAG_SECURE_FOR_CERTS)
+		return (se->slevel==_SECURE)?1:0;
+	else
+		return (se->slevel==_SECURE || se->slevel == _INSECURE_FOR_CERTS)?1:0;
+}
+
 /**
  * gnutls_sign_is_secure2:
  * @algorithm: is a sign algorithm
@@ -340,20 +364,13 @@ unsigned gnutls_sign_is_secure(gnutls_sign_algorithm_t algorithm)
  **/
 unsigned gnutls_sign_is_secure2(gnutls_sign_algorithm_t algorithm, unsigned int flags)
 {
-	gnutls_sign_algorithm_t sign = algorithm;
-	gnutls_digest_algorithm_t dig = GNUTLS_DIG_UNKNOWN;
+	const gnutls_sign_entry_st *se;
 
-	/* avoid prefix */
-	GNUTLS_SIGN_ALG_LOOP(dig = p->hash);
+	se = _gnutls_sign_to_entry(algorithm);
+	if (se == NULL)
+		return 0;
 
-	if (dig != GNUTLS_DIG_UNKNOWN) {
-		if (flags & GNUTLS_SIGN_FLAG_SECURE_FOR_CERTS)
-			return _gnutls_digest_is_secure_for_certs(hash_to_entry(dig));
-		else
-			return _gnutls_digest_is_secure(hash_to_entry(dig));
-	}
-
-	return 0;
+	return _gnutls_sign_is_secure2(se, flags);
 }
 
 /**
diff --git a/lib/gnutls_int.h b/lib/gnutls_int.h
index eb5fab1506..0b0fcd27fe 100644
--- a/lib/gnutls_int.h
+++ b/lib/gnutls_int.h
@@ -476,12 +476,6 @@ typedef struct gnutls_cipher_suite_entry_st {
 } gnutls_cipher_suite_entry_st;
 
 
-typedef enum hash_security_level_t {
-	_SECURE,
-	_INSECURE_FOR_CERTS,
-	_INSECURE
-} hash_security_level_t;
-
 typedef struct gnutls_group_entry_st {
 	const char *name;
 	gnutls_group_t id;
@@ -504,8 +498,8 @@ typedef struct mac_entry_st {
 	unsigned key_size;
 	unsigned nonce_size;
 	unsigned placeholder;	/* if set, then not a real MAC */
-	hash_security_level_t slevel;	/* contains values of hash_security_level_t */
 	unsigned block_size;	/* internal block size for HMAC */
+	unsigned preimage_insecure; /* if this algorithm should not be trusted for pre-image attacks */
 } mac_entry_st;
 
 typedef struct {
diff --git a/lib/pubkey.c b/lib/pubkey.c
index 0fb9b92f4f..e461195d4e 100644
--- a/lib/pubkey.c
+++ b/lib/pubkey.c
@@ -1961,7 +1961,7 @@ pubkey_verify_hashed_data(const gnutls_sign_entry_st *se,
 
 	}
 
-	if (gnutls_sign_is_secure(se->id) == 0 && _gnutls_is_broken_sig_allowed(se->id, flags) == 0) {
+	if (_gnutls_sign_is_secure2(se, 0) == 0 && _gnutls_is_broken_sig_allowed(se, flags) == 0) {
 		return gnutls_assert_val(GNUTLS_E_INSUFFICIENT_SECURITY);
 	}
 
@@ -2023,7 +2023,7 @@ pubkey_verify_data(const gnutls_sign_entry_st *se,
 
 	}
 
-	if (gnutls_sign_is_secure(se->id) == 0 && _gnutls_is_broken_sig_allowed(se->id, flags) == 0) {
+	if (_gnutls_sign_is_secure2(se,0) == 0 && _gnutls_is_broken_sig_allowed(se, flags) == 0) {
 		return gnutls_assert_val(GNUTLS_E_INSUFFICIENT_SECURITY);
 	}
 
diff --git a/lib/x509/verify.c b/lib/x509/verify.c
index d8aef3f368..d50d655b67 100644
--- a/lib/x509/verify.c
+++ b/lib/x509/verify.c
@@ -387,7 +387,7 @@ static unsigned int check_time_status(gnutls_x509_crt_t crt, time_t now)
 	return 0;
 }
 
-unsigned _gnutls_is_broken_sig_allowed(gnutls_sign_algorithm_t sig, unsigned int flags)
+unsigned _gnutls_is_broken_sig_allowed(const gnutls_sign_entry_st *se, unsigned int flags)
 {
 	gnutls_digest_algorithm_t hash;
 
@@ -396,14 +396,14 @@ unsigned _gnutls_is_broken_sig_allowed(gnutls_sign_algorithm_t sig, unsigned int
 		return 1;
 
 	/* the first two are for backwards compatibility */
-	if ((sig == GNUTLS_SIGN_RSA_MD2)
+	if ((se->id == GNUTLS_SIGN_RSA_MD2)
 	    && (flags & GNUTLS_VERIFY_ALLOW_SIGN_RSA_MD2))
 		return 1;
-	if ((sig == GNUTLS_SIGN_RSA_MD5)
+	if ((se->id == GNUTLS_SIGN_RSA_MD5)
 	    && (flags & GNUTLS_VERIFY_ALLOW_SIGN_RSA_MD5))
 		return 1;
 
-	hash = gnutls_sign_get_hash_algorithm(sig);
+	hash = se->hash;
 	if (hash == GNUTLS_DIG_SHA1 && (flags & GNUTLS_VERIFY_ALLOW_SIGN_WITH_SHA1))
 		return 1;
 
@@ -613,6 +613,7 @@ verify_crt(gnutls_x509_crt_t cert,
 	unsigned result = 1;
 	unsigned int out = 0, usage;
 	int sigalg, ret;
+	const gnutls_sign_entry_st *se;
 
 	if (output)
 		*output = 0;
@@ -651,6 +652,8 @@ verify_crt(gnutls_x509_crt_t cert,
 	}
 	sigalg = ret;
 
+	se = _gnutls_sign_to_entry(sigalg);
+
 	/* issuer is not in trusted certificate
 	 * authorities.
 	 */
@@ -776,7 +779,7 @@ verify_crt(gnutls_x509_crt_t cert,
 		}
 	}
 
-	if (sigalg >= 0) {
+	if (sigalg >= 0 && se) {
 		if (is_level_acceptable(cert, issuer, sigalg, flags) == 0) {
 			MARK_INVALID(GNUTLS_CERT_INSECURE_ALGORITHM);
 		}
@@ -785,8 +788,8 @@ verify_crt(gnutls_x509_crt_t cert,
 		 * used are secure. If the certificate is self signed it doesn't
 		 * really matter.
 		 */
-		if (gnutls_sign_is_secure2(sigalg, GNUTLS_SIGN_FLAG_SECURE_FOR_CERTS) == 0 &&
-		    _gnutls_is_broken_sig_allowed(sigalg, flags) == 0 &&
+		if (_gnutls_sign_is_secure2(se, GNUTLS_SIGN_FLAG_SECURE_FOR_CERTS) == 0 &&
+		    _gnutls_is_broken_sig_allowed(se, flags) == 0 &&
 		    is_issuer(cert, cert) == 0) {
 			MARK_INVALID(GNUTLS_CERT_INSECURE_ALGORITHM);
 		}
diff --git a/lib/x509/x509_int.h b/lib/x509/x509_int.h
index 40c635dc4e..e886ac1acd 100644
--- a/lib/x509/x509_int.h
+++ b/lib/x509/x509_int.h
@@ -526,6 +526,6 @@ struct gnutls_x509_tlsfeatures_st {
 	unsigned int size;
 };
 
-unsigned _gnutls_is_broken_sig_allowed(gnutls_sign_algorithm_t sig, unsigned int flags);
+unsigned _gnutls_is_broken_sig_allowed(const gnutls_sign_entry_st *se, unsigned int flags);
 
 #endif