Worked around a problem with select/poll/epoll and gnutls
authorAndrew Tridgell <tridge@samba.org>
Wed, 18 Feb 2009 06:37:45 +0000 (17:37 +1100)
committerAndrew Tridgell <tridge@samba.org>
Wed, 18 Feb 2009 06:37:45 +0000 (17:37 +1100)
Our packet layer relies on the event system reliably telling us when a
packet is available. When we are using a socket layer like TLS then
things get a bit trickier, as there may be bytes in the encryption
buffer which could be read even if there are no bytes at the socket
level. The GNUTLS library is supposed to prevent this happening by
always leaving some data at the socket level when there is data to be
processed in its buffers, but it seems that this is not always
reliable.

To work around this I have added a new packet option
packet_set_unreliable_select() which tells the packet layer to not
assume that the socket layer has a reliable select, and to instead
keep trying to read from the socket until it gets back no data. This
option is set for the ldap client and server when TLS is negotiated.

This seems to fix the problems with the ldaps tests.

source4/ldap_server/ldap_extended.c
source4/ldap_server/ldap_server.c
source4/lib/stream/packet.c
source4/lib/stream/packet.h
source4/libcli/ldap/ldap_client.c

index 4479eab5609ca8c8b22e71b100702bbcb1139691..66ab4eea322b5ca9791695ba761825ed33cd95b7 100644 (file)
@@ -38,6 +38,7 @@ static void ldapsrv_start_tls(void *private_data)
        ctx->conn->sockets.tls = ctx->tls_socket;
        ctx->conn->connection->socket = ctx->tls_socket;
        packet_set_socket(ctx->conn->packet, ctx->conn->connection->socket);
+       packet_set_unreliable_select(ctx->conn->packet);
 }
 
 static NTSTATUS ldapsrv_StartTLS(struct ldapsrv_call *call,
index 61ff387152aa8e681c9ff71e6d69f4f6dccea18f..da44c02aa85075d5c3eb88ae85d91ab269ce7520 100644 (file)
@@ -386,6 +386,10 @@ static void ldapsrv_accept(struct stream_connection *c)
        packet_set_event_context(conn->packet, c->event.ctx);
        packet_set_fde(conn->packet, c->event.fde);
        packet_set_serialise(conn->packet);
+
+       if (conn->sockets.tls) {
+               packet_set_unreliable_select(conn->packet);
+       }
        
        /* Ensure we don't get packets until the database is ready below */
        packet_recv_disable(conn->packet);
index f614e9490a18d6c0ca8fcf39d5e51375fef614de..f5e2b843cd04e8bb658b3b397157f3635aa50e06 100644 (file)
@@ -47,6 +47,8 @@ struct packet_context {
        bool busy;
        bool destructor_called;
 
+       bool unreliable_select;
+
        struct send_element {
                struct send_element *next, *prev;
                DATA_BLOB blob;
@@ -176,6 +178,21 @@ _PUBLIC_ void packet_set_nofree(struct packet_context *pc)
        pc->nofree = true;
 }
 
+/*
+  tell the packet system that select/poll/epoll on the underlying
+  socket may not be a reliable way to determine if data is available
+  for receive. This happens with underlying socket systems such as the
+  one implemented on top of GNUTLS, where there may be data in
+  encryption/compression buffers that could be received by
+  socket_recv(), while there is no data waiting at the real socket
+  level as seen by select/poll/epoll. The GNUTLS library is supposed
+  to cope with this by always leaving some data sitting in the socket
+  buffer, but it does not seem to be reliable.
+ */
+_PUBLIC_ void packet_set_unreliable_select(struct packet_context *pc)
+{
+       pc->unreliable_select = true;
+}
 
 /*
   tell the caller we have an error
@@ -230,6 +247,7 @@ _PUBLIC_ void packet_recv(struct packet_context *pc)
        NTSTATUS status;
        size_t nread = 0;
        DATA_BLOB blob;
+       bool recv_retry = false;
 
        if (pc->processing) {
                EVENT_FD_NOT_READABLE(pc->fde);
@@ -269,6 +287,8 @@ _PUBLIC_ void packet_recv(struct packet_context *pc)
                return;
        }
 
+again:
+
        if (npending + pc->num_read < npending) {
                packet_error(pc, NT_STATUS_INVALID_PARAMETER);
                return;
@@ -308,17 +328,33 @@ _PUBLIC_ void packet_recv(struct packet_context *pc)
                packet_error(pc, status);
                return;
        }
+       if (recv_retry && NT_STATUS_EQUAL(status, STATUS_MORE_ENTRIES)) {
+               nread = 0;
+               status = NT_STATUS_OK;
+       }
        if (!NT_STATUS_IS_OK(status)) {
                return;
        }
 
-       if (nread == 0) {
+       if (nread == 0 && !recv_retry) {
                packet_eof(pc);
                return;
        }
 
        pc->num_read += nread;
 
+       if (pc->unreliable_select && nread != 0) {
+               recv_retry = true;
+               status = socket_pending(pc->sock, &npending);
+               if (!NT_STATUS_IS_OK(status)) {
+                       packet_error(pc, status);
+                       return;
+               }
+               if (npending != 0) {
+                       goto again;
+               }
+       }
+
 next_partial:
        if (pc->partial.length != pc->num_read) {
                if (!data_blob_realloc(pc, &pc->partial, pc->num_read)) {
index 3c2fb0a683cdf41b9513e67f1f9fac5537d7efcf..85f0f26265c9e1c45620468639a95ac5b939ab68 100644 (file)
@@ -48,6 +48,7 @@ void packet_set_nofree(struct packet_context *pc);
 void packet_recv(struct packet_context *pc);
 void packet_recv_disable(struct packet_context *pc);
 void packet_recv_enable(struct packet_context *pc);
+void packet_set_unreliable_select(struct packet_context *pc);
 NTSTATUS packet_send(struct packet_context *pc, DATA_BLOB blob);
 NTSTATUS packet_send_callback(struct packet_context *pc, DATA_BLOB blob,
                              packet_send_callback_fn_t send_callback, 
index e30d5032fb5ebf665cc7e5dbc265a633e2ed305c..3e54d7fff0212d727459187d75e499d8e6b90cbd 100644 (file)
@@ -438,6 +438,10 @@ static void ldap_connect_got_sock(struct composite_context *ctx,
        packet_set_fde(conn->packet, conn->event.fde);
 /*     packet_set_serialise(conn->packet); */
 
+       if (conn->ldaps) {
+               packet_set_unreliable_select(conn->packet);
+       }
+
        composite_done(ctx);
 }