Be better behaved with WANT_READ, etc. in proxy_tls_recv()

Split out the functionality so that it can potentially
be called from proxy_tls_send(), too

src/include/radiusd.h

@@ -422,6 +422,7 @@ typedef struct listen_socket_t {
 	VALUE_PAIR	*certs;
 	pthread_mutex_t mutex;
 	uint8_t		*data;
+	size_t		partial;
 #endif
 
 	RADCLIENT_LIST	*clients;

src/main/tls_listen.c

@@ -480,85 +480,149 @@ int dual_tls_send(rad_listen_t *listener, REQUEST *request)
 
 
 #ifdef WITH_PROXY
-int proxy_tls_recv(rad_listen_t *listener)
+/*
+ *	Read from the SSL socket.  Safe with either blocking or
+ *	non-blocking IO.  This level of complexity is probably not
+ *	necessary, as each packet gets put into one SSL application
+ *	record.  When SSL has a full record, we should be able to read
+ *	the entire packet via one SSL_read().
+ *
+ *	When SSL has a partial record, SSL_read() will return
+ *	WANT_READ or WANT_WRITE, and zero application data.
+ *
+ *	Called with the mutex held.
+ */
+static int proxy_tls_read(rad_listen_t *listener)
 {
 	int rcode;
 	size_t length;
-	listen_socket_t *sock = listener->data;
-	char buffer[256];
-	RADIUS_PACKET *packet;
 	uint8_t *data;
+	listen_socket_t *sock = listener->data;
 
 	/*
 	 *	Get the maximum size of data to receive.
 	 */
 	if (!sock->data) sock->data = talloc_array(sock, uint8_t,
 						   sock->ssn->offset);
-	data = sock->data;
 
-	DEBUG3("Proxy SSL socket has data to read");
-	PTHREAD_MUTEX_LOCK(&sock->mutex);
-redo:
-	rcode = SSL_read(sock->ssn->ssl, data, 4);
-	if (rcode <= 0) {
-		int err = SSL_get_error(sock->ssn->ssl, rcode);
-		switch (err) {
-		case SSL_ERROR_WANT_READ:
-		case SSL_ERROR_WANT_WRITE:
-			goto redo;
-
-		case SSL_ERROR_ZERO_RETURN:
-			/* remote end sent close_notify, send one back */
-			SSL_shutdown(sock->ssn->ssl);
-
-		case SSL_ERROR_SYSCALL:
-		do_close:
-			PTHREAD_MUTEX_UNLOCK(&sock->mutex);
-			tls_socket_close(listener);
-			return 0;
+	data = sock->data;
 
-		default:
-			while ((err = ERR_get_error())) {
-				DEBUG("proxy recv says %s",
-				      ERR_error_string(err, NULL));
+	if (sock->partial < 4) {
+		rcode = SSL_read(sock->ssn->ssl, data + sock->partial,
+				 4 - sock->partial);
+		if (rcode <= 0) {
+			int err = SSL_get_error(sock->ssn->ssl, rcode);
+			switch (err) {
+			case SSL_ERROR_WANT_READ:
+			case SSL_ERROR_WANT_WRITE:
+				return 0; /* do some more work later */
+
+			case SSL_ERROR_ZERO_RETURN:
+				/* remote end sent close_notify, send one back */
+				SSL_shutdown(sock->ssn->ssl);
+
+			case SSL_ERROR_SYSCALL:
+			do_close:
+				return -1;
+
+			default:
+				while ((err = ERR_get_error())) {
+					DEBUG("proxy recv says %s",
+					      ERR_error_string(err, NULL));
+				}
+
+				goto do_close;
 			}
+		}
+
+		sock->partial = rcode;
+	} /* try reading the packet header */
 
+	if (sock->partial < 4) return 0; /* read more data */
+
+	length = (data[2] << 8) | data[3];
+
+	/*
+	 *	Do these checks only once, when we read the header.
+	 */
+	if (sock->partial == 4) {
+		DEBUG3("Proxy received header saying we have a packet of %u bytes",
+		       (unsigned int) length);
+
+		/*
+		 *	FIXME: allocate a RADIUS_PACKET, and set
+		 *	"data" to be as large as necessary.
+		 */
+		if (length > sock->ssn->offset) {
+			INFO("Received packet will be too large! Set \"fragment_size = %u\"",
+			     (data[2] << 8) | data[3]);
 			goto do_close;
 		}
 	}
 
-	length = (data[2] << 8) | data[3];
-	DEBUG3("Proxy received header saying we have a packet of %u bytes",
-	       (unsigned int) length);
+	/*
+	 *	Try to read some more.
+	 */
+	if (sock->partial < length) {
+		rcode = SSL_read(sock->ssn->ssl, data + sock->partial,
+				 length - sock->partial);
+		if (rcode <= 0) {
+			switch (SSL_get_error(sock->ssn->ssl, rcode)) {
+			case SSL_ERROR_WANT_READ:
+			case SSL_ERROR_WANT_WRITE:
+				return 0;
+
+			case SSL_ERROR_ZERO_RETURN:
+				/* remote end sent close_notify, send one back */
+				SSL_shutdown(sock->ssn->ssl);
+				goto do_close;
+			default:
+				goto do_close;
+			}
+		}
 
-	if (length > sock->ssn->offset) {
-		INFO("Received packet will be too large! Set \"fragment_size=%u\"",
-		       (data[2] << 8) | data[3]);
-		goto do_close;
+		sock->partial += rcode;
 	}
 
-	rcode = SSL_read(sock->ssn->ssl, data + 4, length);
-	if (rcode <= 0) {
-		switch (SSL_get_error(sock->ssn->ssl, rcode)) {
-		case SSL_ERROR_WANT_READ:
-		case SSL_ERROR_WANT_WRITE:
-			/*
-			 *  This should never happen as SSL_read blocks
-			 *  until the data is available.
-			 */
-			rad_assert(0);
-			break;
-
-		case SSL_ERROR_ZERO_RETURN:
-			/* remote end sent close_notify, send one back */
-			SSL_shutdown(sock->ssn->ssl);
-			goto do_close;
-		default:
-			goto do_close;
-		}
+	/*
+	 *	If we're not done, say so.
+	 *
+	 *	Otherwise, reset the partially read data flag, and say
+	 *	we have a packet.
+	 */
+	if (sock->partial < length) {
+		return 0;
 	}
+
+	sock->partial = 0;
+
+	return 1;
+}
+
+
+int proxy_tls_recv(rad_listen_t *listener)
+{
+	int rcode;
+	listen_socket_t *sock = listener->data;
+	char buffer[256];
+	RADIUS_PACKET *packet;
+	uint8_t *data;
+
+	DEBUG3("Proxy SSL socket has data to read");
+	PTHREAD_MUTEX_LOCK(&sock->mutex);
+	rcode = proxy_tls_read(listener);
 	PTHREAD_MUTEX_UNLOCK(&sock->mutex);
 
+	if (rcode < 0) {
+		PTHREAD_MUTEX_UNLOCK(&sock->mutex);
+		tls_socket_close(listener);
+		return 0;
+	}
+
+	if (rcode == 0) return 0; /* no data to read */
+
+	data = sock->data;
+
 	packet = rad_alloc(sock, 0);
 	packet->sockfd = listener->fd;
 	packet->src_ipaddr = sock->other_ipaddr;
@@ -567,7 +631,7 @@ int proxy_tls_recv(rad_listen_t *listener)
 	packet->dst_port = sock->my_port;
 	packet->code = data[0];
 	packet->id = data[1];
-	packet->data_len = length;
+	packet->data_len = (data[2] << 8) | data[3];
 	packet->data = talloc_array(packet, uint8_t, packet->data_len);
 	memcpy(packet->data, data, packet->data_len);
 	memcpy(packet->vector, packet->data + 4, 16);
@@ -607,6 +671,7 @@ int proxy_tls_recv(rad_listen_t *listener)
 	return 1;
 }
 
+
 int proxy_tls_send(rad_listen_t *listener, REQUEST *request)
 {
 	int rcode;