cert selection: prioritize RSA-PSS certs over RSA

RSA and RSA-PSS can both be used for RSA-PSS operations, and
as such without prioritizing RSA-PSS certificates it is unknown
which certificate will be used for an RSA-PSS operation. The
reason we want to have only RSA-PSS keys used for RSA-PSS operations
is to cover the use case where a server uses a legacy RSA certificate
for clients that don't support RSA-PSS and an RSA-PSS certificate
for the rest, thus separating the keys used for these client
groups. That separation ensures that any issue on PKCS#1 1.5
(legacy RSA), would not affect sessions which use RSA-PSS.

Resolves #243

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

4 files changed, 48 insertions, 9 deletions

diff --git a/lib/auth/cert.c b/lib/auth/cert.c
index 5fd2286266..d6f6109b29 100644
--- a/lib/auth/cert.c
+++ b/lib/auth/cert.c
@@ -1488,7 +1488,7 @@ int select_sign_algorithm(gnutls_session_t session,
 int
 _gnutls_server_select_cert(gnutls_session_t session, const gnutls_cipher_suite_entry_st *cs)
 {
-	unsigned i;
+	unsigned i, j;
 	int idx, ret;
 	gnutls_certificate_credentials_t cred;
 	char server_name[MAX_CN];
@@ -1552,7 +1552,9 @@ _gnutls_server_select_cert(gnutls_session_t session, const gnutls_cipher_suite_e
 	 */
 
 	if (server_name[0] != 0) {
-		for (i = 0; i < cred->ncerts; i++) {
+		for (j = 0; j < cred->ncerts; j++) {
+			i = cred->sorted_cert_idx[j];
+
 			if (cred->certs[i].names != NULL
 			    && _gnutls_str_array_match(cred->certs[i].names,
 						       server_name) != 0) {
@@ -1586,7 +1588,9 @@ _gnutls_server_select_cert(gnutls_session_t session, const gnutls_cipher_suite_e
 	}
 
 	/* no name match */
-	for (i = 0; i < cred->ncerts; i++) {
+	for (j = 0; j < cred->ncerts; j++) {
+		i = cred->sorted_cert_idx[j];
+
 		_gnutls_handshake_log
 		    ("HSK[%p]: checking compat of %s with certificate[%d] (%s/%s)\n",
 		     session, cs->name, i,
diff --git a/lib/auth/cert.h b/lib/auth/cert.h
index be6945dd21..9383c2e2cb 100644
--- a/lib/auth/cert.h
+++ b/lib/auth/cert.h
@@ -59,6 +59,13 @@ typedef struct gnutls_certificate_credentials_st {
 	certs_st *certs;
 	unsigned ncerts;	/* the number of certs */
 
+	/* contains sorted index values for certs. Sorted in a way
+	 * that RSA-PSS keys always take precedence over plain RSA keys
+	 * to ensure that we use only RSA-PSS keys if present for RSA-PSS
+	 * operations. We keep indexes to certs structures above.
+	 */
+	unsigned int *sorted_cert_idx;
+
 	/* X509 specific stuff */
 	gnutls_x509_trust_list_t tlist;
 	unsigned flags; /* gnutls_certificate_flags */
diff --git a/lib/cert.c b/lib/cert.c
index 4defec87e2..b966dc212b 100644
--- a/lib/cert.c
+++ b/lib/cert.c
@@ -66,7 +66,9 @@ void gnutls_certificate_free_keys(gnutls_certificate_credentials_t sc)
 	}
 
 	gnutls_free(sc->certs);
+	gnutls_free(sc->sorted_cert_idx);
 	sc->certs = NULL;
+	sc->sorted_cert_idx = NULL;
 
 	sc->ncerts = 0;
 }
diff --git a/lib/x509.c b/lib/x509.c
index d1171dc7c7..29435647b0 100644
--- a/lib/x509.c
+++ b/lib/x509.c
@@ -876,7 +876,7 @@ read_cert_url(gnutls_certificate_credentials_t res, gnutls_privkey_t key, const
 			gnutls_assert();
 			goto cleanup;
 		}
-		
+
 		ret = gnutls_x509_crt_import(crt, &t, GNUTLS_X509_FMT_DER);
 		if (ret < 0) {
 			gnutls_assert();
@@ -1085,13 +1085,17 @@ certificate_credential_append_crt_list(gnutls_certificate_credentials_t res,
 				       gnutls_str_array_t names,
 				       gnutls_pcert_st * crt, int nr)
 {
+	res->sorted_cert_idx = gnutls_realloc_fast(res->sorted_cert_idx,
+						(1 + res->ncerts) *
+						sizeof(unsigned int));
+	if (res->sorted_cert_idx == NULL)
+		return gnutls_assert_val(GNUTLS_E_MEMORY_ERROR);
+
 	res->certs = gnutls_realloc_fast(res->certs,
 					 (1 + res->ncerts) *
 					 sizeof(certs_st));
-	if (res->certs == NULL) {
-		gnutls_assert();
-		return GNUTLS_E_MEMORY_ERROR;
-	}
+	if (res->certs == NULL)
+		return gnutls_assert_val(GNUTLS_E_MEMORY_ERROR);
 
 	memset(&res->certs[res->ncerts], 0, sizeof(res->certs[0]));
 
@@ -1100,6 +1104,29 @@ certificate_credential_append_crt_list(gnutls_certificate_credentials_t res,
 	res->certs[res->ncerts].names = names;
 	res->certs[res->ncerts].pkey = key;
 
+	/* move RSA-PSS certificates before any RSA key.
+	 * Note that we cannot assume that any previous pointers
+	 * to sorted list are ok, due to the realloc in res->certs. */
+	if (crt->pubkey->params.algo == GNUTLS_PK_RSA_PSS) {
+		unsigned i,ridx;
+		unsigned tmp;
+
+		for (i=0;i<res->ncerts;i++) {
+			ridx = res->sorted_cert_idx[i];
+
+			if (res->certs[ridx].cert_list->pubkey->params.algo == GNUTLS_PK_RSA) {
+				tmp = ridx;
+				res->sorted_cert_idx[i] = res->ncerts;
+				res->sorted_cert_idx[res->ncerts] = tmp;
+				goto finish;
+			}
+		}
+	}
+
+	/* otherwise append it normally on the end */
+	res->sorted_cert_idx[res->ncerts] = res->ncerts;
+
+ finish:
 	return 0;
 
 }
@@ -1583,7 +1610,6 @@ gnutls_certificate_set_x509_key_file2(gnutls_certificate_credentials_t res,
 	 */
 	if ((ret = read_key_file(res, keyfile, type, pass, flags, &rkey)) < 0)
 		return ret;
-	
 
 	if ((ret = read_cert_file(res, rkey, certfile, type)) < 0) {
 		gnutls_privkey_deinit(rkey);