Worked around a problem with select/poll/epoll and gnutls

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.

5 files changed, 47 insertions, 1 deletions

diff --git a/source4/ldap_server/ldap_extended.c b/source4/ldap_server/ldap_extended.c
index 4479eab5609..66ab4eea322 100644
--- a/source4/ldap_server/ldap_extended.c
+++ b/source4/ldap_server/ldap_extended.c
@@ -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,
diff --git a/source4/ldap_server/ldap_server.c b/source4/ldap_server/ldap_server.c
index 61ff387152a..da44c02aa85 100644
--- a/source4/ldap_server/ldap_server.c
+++ b/source4/ldap_server/ldap_server.c
@@ -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);
diff --git a/source4/lib/stream/packet.c b/source4/lib/stream/packet.c
index f614e9490a1..f5e2b843cd0 100644
--- a/source4/lib/stream/packet.c
+++ b/source4/lib/stream/packet.c
@@ -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)) {
diff --git a/source4/lib/stream/packet.h b/source4/lib/stream/packet.h
index 3c2fb0a683c..85f0f26265c 100644
--- a/source4/lib/stream/packet.h
+++ b/source4/lib/stream/packet.h
@@ -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, 
diff --git a/source4/libcli/ldap/ldap_client.c b/source4/libcli/ldap/ldap_client.c
index e30d5032fb5..3e54d7fff02 100644
--- a/source4/libcli/ldap/ldap_client.c
+++ b/source4/libcli/ldap/ldap_client.c
@@ -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);
 }