[gnutls-devel] BUG: Cannot connect with non-blocking OS to OCSP stapling-enabled (CERTIFICATE STATUS) server

[Nils Maier]

Affected: Likely all GnuTLS versions supporting OCSP stapling. Tested
with 3.1.18 and 3.2.8.

STR:
- Program client using non-blocking sockets. Or if you're lazy, use
aria2, where we discovered this.
http://aria2.sourceforge.net/
https://github.com/tatsuhiro-t/aria2/issues/179
Or wget master, which is affected as well, or something like that.

- Try to connect to a TLS server that uses stapling. E.g. https://tn123.org/

- The connection will fail (more often than not), right after the full
CERTIFICATE STATUS packet is received, with:
"An unexpected TLS handshake packet was received."

- It fails in STATE8 of the handshake (kx) not STATE6 (CERTIFICATE STATUS).

Reasons:
https://gitorious.org/gnutls/gnutls/source/1b5fce89c569e4abf9784e6a2944b3ccf9dd61f8:lib/ext/status_request.c#L581

_gnutls_recv_server_certificate_status() does not handle incomplete
packets (EGAIN) correctly.
It sets |priv->expect_cstatus = 0;| before actually checking if there
was already a full packet received. If there wasn't (and this is likely
with non-blocking I/O), the method will later exit, to be called again
when additional data is received.
However, in that subsequent call the function will bail without further
processing as |priv->expect_cstatus| was already set in the first call,
leaving the unprocessed CERTIFICATE STATUS packet in the buffers.
After bailing, the STATE in _gnutls_recv_server_certificate_status()
transitions from STATE6 -> STATE7 -> STATE8. In STATE8,
_gnutls_recv_server_kx_message() is called, with the CERTIFICATE STATUS
still in the buffer, and therefore fails as it rightfully didn't expect
that packet, aborting the connection attempt entirely.

Fix:
A preliminary test indicates the attached patch (against 3_1_x, because
I'm to lazy to build a nettle-2.7 right now) fixes the problem, by
moving |priv->expect_cstatus = 0;| after the |_gnutls_recv_handshake|
call. I'm not a GnuTLS expert, so I have no idea what it might break in
turn.

Cheers
Nils

PS: Please CC me if you need something, as I'm not watching the lists.
-------------- next part --------------
>From d6d4a177e9e705c4bbac0eaa689fa80025f52f71 Mon Sep 17 00:00:00 2001
From: Nils Maier <maierman at web.de>
Date: Mon, 6 Jan 2014 15:15:58 +0100
Subject: [PATCH] Fix CERTIFICATE STATUS processing when using non-blocking I/O

_gnutls_recv_server_certificate_status() must wait for the first full
packet before setting priv->expect_cstatus = 0, or else CERTIFCATE
STATUS packets won't be processed in subsequent calls at all, leaving
them in the buffer and therefore causing later connection aborts.
---
 lib/ext/status_request.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/ext/status_request.c b/lib/ext/status_request.c
index ac512c2..267ba5d 100644
--- a/lib/ext/status_request.c
+++ b/lib/ext/status_request.c
@@ -574,28 +574,28 @@ _gnutls_recv_server_certificate_status (gnutls_session_t session)
     _gnutls_ext_get_session_data (session, GNUTLS_EXTENSION_STATUS_REQUEST,
                                   &epriv);
   if (ret < 0)
     return 0;
 
   priv = epriv.ptr;
 
   if (!priv->expect_cstatus)
     return 0;
 
-  priv->expect_cstatus = 0;
-
   ret = _gnutls_recv_handshake (session, 
                                 GNUTLS_HANDSHAKE_CERTIFICATE_STATUS,
                                 0, &buf);
   if (ret < 0)
     return gnutls_assert_val_fatal(ret);
 
+  priv->expect_cstatus = 0;
+
   data = buf.data;
   data_size = buf.length;
 
   /* minimum message is type (1) + response (3) + data */
   if (data_size == 0)
     return 0;
   else if (data_size < 4)
     return gnutls_assert_val (GNUTLS_E_UNEXPECTED_PACKET_LENGTH);
 
   if (data[0] != 0x01)
-- 
1.8.5.2