[PATCH] client-side TLS 1.2 support

[Daiki Ueno]

>>>>> In <address@hidden> 
>>>>>   Simon Josefsson <address@hidden> wrote:
> Daiki Ueno <address@hidden> writes:

> >> Finishing the TLS 1.2 support and adding the new cipher suites is a
> >> high-priority task and it shouldn't be too difficult since there are TLS
> >> 1.2 test servers out there to test with.
> >
> > Thanks for the hint.  I'll check which features of TLS 1.2 are not
> > implemented.  Adding HMAC-SHA256 cipher suites looks one thing to do.

> Actually TLS 1.2 is not working in GnuTLS now, the drafts changed how
> the negotiation worked after I implemented it and I never found time to
> update it to support the protocol defined by the final RFC.

I just realized it ;-)

I'm attaching a set of patches to provide minimal fix for client side
TLS 1.2 support.  I've confirmed them working against Mike's test
server:

 $ gnutls-cli --debug 10 --protocols TLS1.2 -p 443 www.mikestoolbox.net

> I don't expect finishing this would require major changes, so it would
> be a great contribution to finish the TLS 1.2 support.

While server-side support would require a bit more work, for the moment
I would like to ask for comments on my approach.  The patches mainly
follow the changes regarding SignatureAndHashAlgorithm usage.  Here is a
summary of each patch:

* 0001-Add-functions-for-TLS-signature-algorithm.patch

This patch adds helper functions which convert SignatureAndHashAlgorithm
value from/to `gnutls_sign_algorithm_t'.

* 0002-Respect-TLS-signature-algorithm-in-server-KX.patch

The signature of DH params in Server Key Exchange is now a
"digitally-signed" struct.  This patch makes it read the algorithm IDs
at the beginning of the actual signature bytes.

* 0003-Use-SHA256-for-PRF-if-TLS-1.2.patch

TLS 1.2 mandates that the algorithm for the basis of PRF is SHA256, and
the same algorithm is used for the hash over handshake messages (to be
used to verify Finished message).  This patch makes it use SHA256 in
both places.

* 0004-Fix-parsing-Certificate-Request-for-TLS-1.2.patch

This patch fixes a logic to skip supported_signature_algorithms in
Certificate Request, whose type seemes to have changed from the draft.

>From 4ed3bb2ac905c13ab06c11d28f6ce7bb7e44149f Mon Sep 17 00:00:00 2001
From: Daiki Ueno <address@hidden>
Date: Mon, 31 Aug 2009 14:34:01 +0900
Subject: [PATCH 1/4] Add functions for TLS signature algorithm.

Add functions to convert TLS signature algorithm from/to constants
defined by GnuTLS.
---
 lib/gnutls_algorithms.c |   71 +++++++++++++++++++++++++++++++++++++++-------
 lib/gnutls_algorithms.h |    4 ++
 lib/gnutls_int.h        |    6 ++++
 3 files changed, 70 insertions(+), 11 deletions(-)

diff --git a/lib/gnutls_algorithms.c b/lib/gnutls_algorithms.c
index 08054c4..73179bb 100644
--- a/lib/gnutls_algorithms.c
+++ b/lib/gnutls_algorithms.c
@@ -1787,29 +1787,32 @@ struct gnutls_sign_entry
   gnutls_sign_algorithm_t id;
   gnutls_pk_algorithm_t pk;
   gnutls_mac_algorithm_t mac;
+  sign_algorithm_st aid;
 };
 typedef struct gnutls_sign_entry gnutls_sign_entry;
 
+#define TLS_SIGN_AID_UNKNOWN {255, 255}
+
 static const gnutls_sign_entry sign_algorithms[] = {
   {"RSA-SHA", SIG_RSA_SHA1_OID, GNUTLS_SIGN_RSA_SHA1, GNUTLS_PK_RSA,
-   GNUTLS_MAC_SHA1},
+   GNUTLS_MAC_SHA1, {2, 1}},
   {"RSA-SHA256", SIG_RSA_SHA256_OID, GNUTLS_SIGN_RSA_SHA256, GNUTLS_PK_RSA,
-   GNUTLS_MAC_SHA256},
+   GNUTLS_MAC_SHA256, {4, 1}},
   {"RSA-SHA384", SIG_RSA_SHA384_OID, GNUTLS_SIGN_RSA_SHA384, GNUTLS_PK_RSA,
-   GNUTLS_MAC_SHA384},
+   GNUTLS_MAC_SHA384, {5, 1}},
   {"RSA-SHA512", SIG_RSA_SHA512_OID, GNUTLS_SIGN_RSA_SHA512, GNUTLS_PK_RSA,
-   GNUTLS_MAC_SHA512},
+   GNUTLS_MAC_SHA512, {6, 1}},
   {"RSA-RMD160", SIG_RSA_RMD160_OID, GNUTLS_SIGN_RSA_RMD160, GNUTLS_PK_RSA,
-   GNUTLS_MAC_RMD160},
+   GNUTLS_MAC_RMD160, TLS_SIGN_AID_UNKNOWN},
   {"DSA-SHA", SIG_DSA_SHA1_OID, GNUTLS_SIGN_DSA_SHA1, GNUTLS_PK_DSA,
-   GNUTLS_MAC_SHA1},
+   GNUTLS_MAC_SHA1, {2, 2}},
   {"RSA-MD5", SIG_RSA_MD5_OID, GNUTLS_SIGN_RSA_MD5, GNUTLS_PK_RSA,
-   GNUTLS_MAC_MD5},
+   GNUTLS_MAC_MD5, {1, 1}},
   {"RSA-MD2", SIG_RSA_MD2_OID, GNUTLS_SIGN_RSA_MD2, GNUTLS_PK_RSA,
-   GNUTLS_MAC_MD2},
-  {"GOST R 34.10-2001", SIG_GOST_R3410_2001_OID, 0, 0, 0},
-  {"GOST R 34.10-94", SIG_GOST_R3410_94_OID, 0, 0, 0},
-  {0, 0, 0, 0, 0}
+   GNUTLS_MAC_MD2, TLS_SIGN_AID_UNKNOWN},
+  {"GOST R 34.10-2001", SIG_GOST_R3410_2001_OID, 0, 0, 0, 
TLS_SIGN_AID_UNKNOWN},
+  {"GOST R 34.10-94", SIG_GOST_R3410_94_OID, 0, 0, 0, TLS_SIGN_AID_UNKNOWN},
+  {0, 0, 0, 0, 0, TLS_SIGN_AID_UNKNOWN}
 };
 
 /* Keep the contents of this struct the same as the previous one. */
@@ -1958,6 +1961,52 @@ _gnutls_x509_sign_to_oid (gnutls_pk_algorithm_t pk,
   return ret;
 }
 
+gnutls_mac_algorithm_t
+_gnutls_sign_get_mac_algorithm (gnutls_sign_algorithm_t sign)
+{
+  gnutls_mac_algorithm_t ret = GNUTLS_MAC_UNKNOWN;
+
+  GNUTLS_SIGN_ALG_LOOP (ret = p->mac);
+
+  return ret;
+}
+
+gnutls_pk_algorithm_t
+_gnutls_sign_get_pk_algorithm (gnutls_sign_algorithm_t sign)
+{
+  gnutls_pk_algorithm_t ret = GNUTLS_PK_UNKNOWN;
+
+  GNUTLS_SIGN_ALG_LOOP (ret = p->pk);
+
+  return ret;
+}
+
+gnutls_sign_algorithm_t
+_gnutls_tls_aid_to_sign (sign_algorithm_st aid)
+{
+  gnutls_sign_algorithm_t ret = GNUTLS_SIGN_UNKNOWN;
+
+  GNUTLS_SIGN_LOOP ( if (p->aid.hash_algorithm == aid.hash_algorithm
+                        && p->aid.sign_algorithm == aid.sign_algorithm)
+                      {
+                        ret = p->id;
+                        break;
+                      } );
+
+  return ret;
+}
+
+sign_algorithm_st
+_gnutls_sign_to_tls_aid (gnutls_sign_algorithm_t sign)
+{
+  sign_algorithm_st ret = TLS_SIGN_AID_UNKNOWN;
+
+  GNUTLS_SIGN_ALG_LOOP (ret = p->aid);
+
+  return ret;
+}
+
+
 
 /* pk algorithms;
  */
diff --git a/lib/gnutls_algorithms.h b/lib/gnutls_algorithms.h
index 0e2f2b7..0a2faac 100644
--- a/lib/gnutls_algorithms.h
+++ b/lib/gnutls_algorithms.h
@@ -105,6 +105,10 @@ gnutls_sign_algorithm_t _gnutls_x509_pk_to_sign 
(gnutls_pk_algorithm_t pk,
                                                 gnutls_mac_algorithm_t mac);
 const char *_gnutls_x509_sign_to_oid (gnutls_pk_algorithm_t,
                                      gnutls_mac_algorithm_t mac);
+gnutls_sign_algorithm_t _gnutls_tls_aid_to_sign (sign_algorithm_st aid);
+sign_algorithm_st _gnutls_sign_to_tls_aid (gnutls_sign_algorithm_t sign);
+gnutls_mac_algorithm_t _gnutls_sign_get_mac_algorithm 
(gnutls_sign_algorithm_t);
+gnutls_pk_algorithm_t _gnutls_sign_get_pk_algorithm (gnutls_sign_algorithm_t);
 
 int _gnutls_mac_priority (gnutls_session_t session,
                          gnutls_mac_algorithm_t algorithm);
diff --git a/lib/gnutls_int.h b/lib/gnutls_int.h
index 9af17b0..100ad37 100644
--- a/lib/gnutls_int.h
+++ b/lib/gnutls_int.h
@@ -273,6 +273,12 @@ typedef struct
   uint8_t suite[2];
 } cipher_suite_st;
 
+typedef struct
+{
+  uint8_t hash_algorithm;
+  uint8_t sign_algorithm;
+} sign_algorithm_st;
+
 /* This structure holds parameters got from TLS extension
  * mechanism. (some extensions may hold parameters in auth_info_t
  * structures also - see SRP).
-- 
1.6.3.3

>From bea4b6f892da08f9f5f9100628b8bd97b40c5771 Mon Sep 17 00:00:00 2001
From: Daiki Ueno <address@hidden>
Date: Mon, 31 Aug 2009 14:40:38 +0900
Subject: [PATCH 2/4] Respect TLS signature algorithm in server KX.

Verify signature of DH parameters in Server Key Exchange with the
embedded signature algorithm.
---
 lib/auth_dhe.c        |   27 ++++++++++++++++++++++++---
 lib/auth_rsa_export.c |    3 ++-
 lib/auth_srp_rsa.c    |    3 ++-
 lib/gnutls_sig.c      |   23 ++++++++++++++++-------
 lib/gnutls_sig.h      |    3 ++-
 5 files changed, 46 insertions(+), 13 deletions(-)

diff --git a/lib/auth_dhe.c b/lib/auth_dhe.c
index ec595ee..ea9cf3a 100644
--- a/lib/auth_dhe.c
+++ b/lib/auth_dhe.c
@@ -180,11 +180,14 @@ proc_dhe_server_kx (gnutls_session_t session, opaque * 
data,
                    size_t _data_size)
 {
   int sigsize;
+  opaque *sigdata;
   gnutls_datum_t vparams, signature;
   int ret;
   cert_auth_info_t info = _gnutls_get_auth_info (session);
   ssize_t data_size = _data_size;
   gnutls_cert peer_cert;
+  gnutls_sign_algorithm_t sign_algo = GNUTLS_SIGN_UNKNOWN;
+  gnutls_protocol_t ver = gnutls_protocol_get_version (session);
 
   if (info == NULL || info->ncerts == 0)
     {
@@ -205,11 +208,28 @@ proc_dhe_server_kx (gnutls_session_t session, opaque * 
data,
   vparams.size = ret;
   vparams.data = data;
 
+  sigdata = &data[vparams.size];
+  if (_gnutls_version_has_selectable_sighash (ver))
+    {
+      sign_algorithm_st aid;
+
+      DECR_LEN(data_size, 1);
+      aid.hash_algorithm = *sigdata++;
+      DECR_LEN(data_size, 1);
+      aid.sign_algorithm = *sigdata++;
+      sign_algo = _gnutls_tls_aid_to_sign (aid);
+      if (sign_algo == GNUTLS_SIGN_UNKNOWN)
+       {
+         gnutls_assert ();
+         return GNUTLS_E_UNKNOWN_PK_ALGORITHM;
+       }
+    }
   DECR_LEN (data_size, 2);
-  sigsize = _gnutls_read_uint16 (&data[vparams.size]);
+  sigsize = _gnutls_read_uint16 (sigdata);
+  sigdata += 2;
 
   DECR_LEN (data_size, sigsize);
-  signature.data = &data[vparams.size + 2];
+  signature.data = sigdata;
   signature.size = sigsize;
 
   if ((ret =
@@ -221,7 +241,8 @@ proc_dhe_server_kx (gnutls_session_t session, opaque * data,
       return ret;
     }
 
-  ret = _gnutls_verify_sig_params (session, &peer_cert, &vparams, &signature);
+  ret = _gnutls_verify_sig_params (session, &peer_cert, &vparams, &signature,
+                                  sign_algo);
 
   _gnutls_gcert_deinit (&peer_cert);
   if (ret < 0)
diff --git a/lib/auth_rsa_export.c b/lib/auth_rsa_export.c
index b561063..f96fc79 100644
--- a/lib/auth_rsa_export.c
+++ b/lib/auth_rsa_export.c
@@ -310,7 +310,8 @@ proc_rsa_export_server_kx (gnutls_session_t session,
       return ret;
     }
 
-  ret = _gnutls_verify_sig_params (session, &peer_cert, &vparams, &signature);
+  ret = _gnutls_verify_sig_params (session, &peer_cert, &vparams, &signature,
+                                  GNUTLS_SIGN_UNKNOWN);
 
   _gnutls_gcert_deinit (&peer_cert);
   if (ret < 0)
diff --git a/lib/auth_srp_rsa.c b/lib/auth_srp_rsa.c
index 2f2ea96..1689ce2 100644
--- a/lib/auth_srp_rsa.c
+++ b/lib/auth_srp_rsa.c
@@ -191,7 +191,8 @@ proc_srp_cert_server_kx (gnutls_session_t session, opaque * 
data,
       return ret;
     }
 
-  ret = _gnutls_verify_sig_params (session, &peer_cert, &vparams, &signature);
+  ret = _gnutls_verify_sig_params (session, &peer_cert, &vparams, &signature,
+                                  GNUTLS_SIGN_UNKNOWN);
 
   _gnutls_gcert_deinit (&peer_cert);
   if (ret < 0)
diff --git a/lib/gnutls_sig.c b/lib/gnutls_sig.c
index bcc4412..81e8336 100644
--- a/lib/gnutls_sig.c
+++ b/lib/gnutls_sig.c
@@ -298,7 +298,8 @@ _gnutls_tls_sign (gnutls_session_t session,
 static int
 _gnutls_verify_sig (gnutls_cert * cert,
                    const gnutls_datum_t * hash_concat,
-                   gnutls_datum_t * signature, size_t sha1pos)
+                   gnutls_datum_t * signature, size_t sha1pos,
+                   gnutls_pk_algorithm_t pk_algo)
 {
   int ret;
   gnutls_datum_t vdata;
@@ -321,7 +322,9 @@ _gnutls_verify_sig (gnutls_cert * cert,
          return GNUTLS_E_KEY_USAGE_VIOLATION;
        }
 
-  switch (cert->subject_pk_algorithm)
+  if (pk_algo == GNUTLS_PK_UNKNOWN)
+    pk_algo = cert->subject_pk_algorithm;
+  switch (pk_algo)
     {
     case GNUTLS_PK_RSA:
 
@@ -340,7 +343,7 @@ _gnutls_verify_sig (gnutls_cert * cert,
     case GNUTLS_PK_DSA:
 
       vdata.data = &hash_concat->data[sha1pos];
-      vdata.size = 20;         /* sha1 */
+      vdata.size = hash_concat->size - sha1pos;
 
       /* verify signature */
       if ((ret = _gnutls_dsa_verify (&vdata, signature, cert->params,
@@ -419,7 +422,7 @@ _gnutls_verify_sig_hdata (gnutls_session_t session, 
gnutls_cert * cert,
   dconcat.data = concat;
   dconcat.size = 20 + 16;      /* md5+ sha */
 
-  ret = _gnutls_verify_sig (cert, &dconcat, signature, 16);
+  ret = _gnutls_verify_sig (cert, &dconcat, signature, 16, 
GNUTLS_SIGN_UNKNOWN);
   if (ret < 0)
     {
       gnutls_assert ();
@@ -436,7 +439,8 @@ _gnutls_verify_sig_hdata (gnutls_session_t session, 
gnutls_cert * cert,
 int
 _gnutls_verify_sig_params (gnutls_session_t session, gnutls_cert * cert,
                           const gnutls_datum_t * params,
-                          gnutls_datum_t * signature)
+                          gnutls_datum_t * signature,
+                          gnutls_sign_algorithm_t algo)
 {
   gnutls_datum_t dconcat;
   int ret;
@@ -444,6 +448,7 @@ _gnutls_verify_sig_params (gnutls_session_t session, 
gnutls_cert * cert,
   digest_hd_st td_sha;
   opaque concat[36];
   gnutls_protocol_t ver = gnutls_protocol_get_version (session);
+  gnutls_mac_algorithm_t mac_algo = GNUTLS_MAC_SHA1;
 
   if (!_gnutls_version_has_selectable_prf (ver))
     {
@@ -461,7 +466,9 @@ _gnutls_verify_sig_params (gnutls_session_t session, 
gnutls_cert * cert,
       _gnutls_hash (&td_md5, params->data, params->size);
     }
 
-  ret = _gnutls_hash_init (&td_sha, GNUTLS_MAC_SHA1);
+  if (algo != GNUTLS_SIGN_UNKNOWN)
+    mac_algo = _gnutls_sign_get_mac_algorithm (algo);
+  ret = _gnutls_hash_init (&td_sha, mac_algo);
   if (ret < 0)
     {
       gnutls_assert ();
@@ -502,7 +509,9 @@ _gnutls_verify_sig_params (gnutls_session_t session, 
gnutls_cert * cert,
 
   dconcat.data = concat;
 
-  ret = _gnutls_verify_sig (cert, &dconcat, signature, dconcat.size - 20);
+  ret = _gnutls_verify_sig (cert, &dconcat, signature,
+                           dconcat.size - _gnutls_hash_get_algo_len (mac_algo),
+                           _gnutls_sign_get_pk_algorithm (algo));
   if (ret < 0)
     {
       gnutls_assert ();
diff --git a/lib/gnutls_sig.h b/lib/gnutls_sig.h
index f16114c..81890c4 100644
--- a/lib/gnutls_sig.h
+++ b/lib/gnutls_sig.h
@@ -42,7 +42,8 @@ int _gnutls_verify_sig_hdata (gnutls_session_t session,
 int _gnutls_verify_sig_params (gnutls_session_t session,
                               gnutls_cert * cert,
                               const gnutls_datum_t * params,
-                              gnutls_datum_t * signature);
+                              gnutls_datum_t * signature,
+                              gnutls_sign_algorithm_t algo);
 
 int _gnutls_sign (gnutls_pk_algorithm_t algo,
                  bigint_t * params, int params_size,
-- 
1.6.3.3

>From 9c823ed1449f1c83e73e9e8e513a4671a37b1b00 Mon Sep 17 00:00:00 2001
From: Daiki Ueno <address@hidden>
Date: Mon, 31 Aug 2009 14:44:51 +0900
Subject: [PATCH 3/4] Use SHA256 for PRF if TLS 1.2.

Use SHA256 for the basis of PRF, and for the hash over handshake messages.
---
 lib/gnutls_handshake.c |   12 ++++++++++--
 lib/gnutls_pk.c        |    4 ++--
 lib/gnutls_state.c     |    4 ++--
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/lib/gnutls_handshake.c b/lib/gnutls_handshake.c
index 7c10fbb..83dc54e 100644
--- a/lib/gnutls_handshake.c
+++ b/lib/gnutls_handshake.c
@@ -236,7 +236,7 @@ _gnutls_finished (gnutls_session_t session, int type, void 
*ret)
   else
     {
       _gnutls_hash_deinit (&td_sha, concat);
-      len = 20;
+      len = _gnutls_hash_get_algo_len (td_sha.algorithm);
     }
 
   if (type == GNUTLS_SERVER)
@@ -2170,6 +2170,8 @@ _gnutls_abort_handshake (gnutls_session_t session, int 
ret)
 inline static int
 _gnutls_handshake_hash_init (gnutls_session_t session)
 {
+  gnutls_protocol_t ver = gnutls_protocol_get_version (session);
+  gnutls_digest_algorithm_t hash_algo = GNUTLS_MAC_SHA1;
 
   if (session->internals.handshake_mac_handle_init == 0)
     {
@@ -2183,9 +2185,15 @@ _gnutls_handshake_hash_init (gnutls_session_t session)
          return ret;
        }
 
+      /* The algorithm to compute hash over handshake messages must be
+        same as the one used as the basis for PRF.  By now we use
+        SHA256. */
+      if (_gnutls_version_has_selectable_prf (ver))
+       hash_algo = GNUTLS_MAC_SHA256;
+
       ret =
        _gnutls_hash_init (&session->internals.handshake_mac_handle_sha,
-                          GNUTLS_MAC_SHA1);
+                          hash_algo);
       if (ret < 0)
        {
          gnutls_assert ();
diff --git a/lib/gnutls_pk.c b/lib/gnutls_pk.c
index ff9fbf1..ccd98a4 100644
--- a/lib/gnutls_pk.c
+++ b/lib/gnutls_pk.c
@@ -500,8 +500,8 @@ _gnutls_dsa_verify (const gnutls_datum_t * vdata,
     pk_params.params[i] = params[i];
   pk_params.params_nr = params_len;
 
-  if (vdata->size != 20)
-    {                          /* sha-1 only */
+  if (vdata->size > 20)
+    {                          /* SHA1 or better only */
       gnutls_assert ();
       return GNUTLS_E_PK_SIG_VERIFY_FAILED;
     }
diff --git a/lib/gnutls_state.c b/lib/gnutls_state.c
index e1bfc66..fede2a0 100644
--- a/lib/gnutls_state.c
+++ b/lib/gnutls_state.c
@@ -785,7 +785,7 @@ _gnutls_P_hash (gnutls_mac_algorithm_t algorithm,
 
   digest_hd_st td2;
   int i, times, how, blocksize, A_size;
-  opaque final[20], Atmp[MAX_SEED_SIZE];
+  opaque final[MAX_HASH_SIZE], Atmp[MAX_SEED_SIZE];
   int output_bytes, result;
 
   if (seed_size > MAX_SEED_SIZE || total_bytes <= 0)
@@ -906,7 +906,7 @@ _gnutls_PRF (gnutls_session_t session,
   if (_gnutls_version_has_selectable_prf(ver))
     {
       result =
-       _gnutls_P_hash (GNUTLS_MAC_SHA1, secret, secret_size,
+       _gnutls_P_hash (GNUTLS_MAC_SHA256, secret, secret_size,
                        s_seed, s_seed_size, total_bytes, ret);
       if (result < 0)
        {
-- 
1.6.3.3

>From ce9c5df0a84121129df9b32fdb6e379a61f111b2 Mon Sep 17 00:00:00 2001
From: Daiki Ueno <address@hidden>
Date: Mon, 31 Aug 2009 14:48:12 +0900
Subject: [PATCH 4/4] Fix parsing Certificate Request for TLS 1.2.

Fix the logic to skip supported_signature_algorithms in Certificate
Request.
---
 lib/auth_cert.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/lib/auth_cert.c b/lib/auth_cert.c
index a5244c8..0262878 100644
--- a/lib/auth_cert.c
+++ b/lib/auth_cert.c
@@ -1356,10 +1356,9 @@ _gnutls_proc_cert_cert_req (gnutls_session_t session, 
opaque * data,
     {
       /* read supported hashes */
       int hash_num;
-      DECR_LEN (dsize, 1);
-
-      hash_num = p[0] & 0xFF;
-      p++;
+      DECR_LEN (dsize, 2);
+      hash_num = _gnutls_read_uint16 (p);
+      p += 2;
 
       DECR_LEN (dsize, hash_num);
       p += hash_num;
-- 
1.6.3.3

Regards,
-- 
Daiki Ueno