GNUTLS handshake errors and memory leaks (ECDHE related?)

[Nikos Mavrogiannopoulos]

On Tue, Jan 10, 2012 at 11:44 AM, Yaroslav <yarosla at gmail.com> wrote:
> Hi!
> I have added SSL support to my web server (nxweb) using GNUTLS. The server
> has asynchronous architecture (based on epoll), where 4 threads run almost
> independently accepting and servicing client http connections. Therefore I
> am using GNUTLS in non-blocking mode.
> Platform: Ubuntu 11.10, x64, 4-core CPU.
> Linked with latest GNUTLS 3.0.11.
> Most of the time everything works fine. Benchmarks show significant speed
> advantage over nginx (which is build on OpenSSL). But sometimes there are
> handshake errors and also memory leaks, which might be bound to those
> errors.

Hello Yaroslav,
 Reply inline.

> I am attaching full valgrind log. Here are some excerpts:
> % valgrind --leak-check=full --show-reachable=yes
> dist/Debug/GNU-Linux-x86/nxweb
> ...
> 2012-01-10 13:47:25 [28704:0x71b9700]: gnutls handshake error -110 fatal=1

This error is GNUTLS_E_PREMATURE_TERMINATION and is returned if the
client does not properly terminate the session (using a closure alert
message) and instead terminates the tcp connection. GnuTLS reports it
as an error, but you could ignore it, if premature termination attacks
are not an issue.

> 2012-01-10 13:47:25 [28704:0x71b9700]: gnutls handshake error -10 fatal=1
This is GNUTLS_E_INVALID_SESSION. It might be returned when trying to
send or received from a session that is already terminated or
invalidated for a reason (if you could set a log level of 2 and set
the log function I could provide more details on the reason). It might
also be a good idea to use gnutls_strerror() to convert the numeric
errors to strings.

> 2012-01-10 13:46:51 [28704:0x71b9700]: gnutls handshake error -24 fatal=1

This is GNUTLS_E_DECRYPTION_FAILED. This is pretty strange to happen.
Could you provide more information with a log level of 2?

> ==28704== 47,952 bytes in 999 blocks are definitely lost in loss record 11
> of 11
> ==28704==    at 0x4C28F9F: malloc (vg_replace_malloc.c:236)
> ==28704==    by 0x5F28908: __gmp_default_allocate (in
> /usr/lib/libgmp.so.10.0.1)
> ==28704==    by 0x5F3CF39: __gmpz_mul (in /usr/lib/libgmp.so.10.0.1)
> ==28704==    by 0x50ED444: ecc_projective_check_point
> (ecc_projective_check_point.c:60)

It seems there is an issue in ecc_project_check_point. Could you try
the attached patch?

> There are also other leaks related to gnutls_dh_params_generate2 and some

Those should have been fixed in the repository.

> error messages related to gnutls_certificate_set_x509_key_file but those are
> of less importance to me as they happen only once on server startup.

I'll check about gnutls_certificate_set_x509_key_file() later.

> The problems might be related to ECDHE algorithm. There were no handshake
> errors when 'PERFORMANCE:%SERVER_PRECEDENCE' priority string was used.

With which string did you see the errors? Do you know which
ciphersuite was negotiated? (gnutls_cipher_suite_get_name)

> And there seemed to be no such leaks when there were no handshake erros.

btw. from the logs I see that the version of libtasn1 you have doesn't
compile well with
the gcc you have. You can use the included libtasn1 to avoid the
valgrind warnings.

I would be interested on your results in performance. Do you use a CPU
with AES-NI or padlock for the tests?

regards,
Nikos
-------------- next part --------------
diff --git a/lib/nettle/ecc_projective_check_point.c b/lib/nettle/ecc_projective_check_point.c
index 42abeea..5649779 100644
--- a/lib/nettle/ecc_projective_check_point.c
+++ b/lib/nettle/ecc_projective_check_point.c
@@ -42,17 +42,17 @@ int ecc_projective_check_point (ecc_point * P, mpz_t b, mpz_t modulus)
     if (P == NULL || b == NULL || modulus == NULL)
 	return -1;
 
-    if ((err = mp_init_multi (&t1, &t2, &t3, NULL)) != 0)
-      {
-	  return err;
-      }
-
     if (mpz_cmp_ui (P->z, 1) != 0)
       {
 	  gnutls_assert ();
 	  return -1;
       }
 
+    if ((err = mp_init_multi (&t1, &t2, &t3, NULL)) != 0)
+      {
+	  return err;
+      }
+
     /* t1 = Z * Z */
     mpz_mul (t1, P->y, P->y);
     mpz_mod (t1, t1, modulus);	/* t1 = y^2 */
@@ -95,12 +95,16 @@ int ecc_projective_check_point (ecc_point * P, mpz_t b, mpz_t modulus)
 
     if (mpz_cmp_ui (t1, 0) != 0)
       {
-	  return -1;
+	  err = -1;
       }
     else
       {
-	  return 0;
+	  err = 0;
       }
+
+    mp_clear_multi(&t1, &t2, &t3, NULL);
+    
+    return err;
 }
 
 #endif