Split tls_handshake() out from tls_accept/tls_connect. By doing this the

tls_accept/tls_connect functions can be guaranteed to succeed or fail and
will no longer return TLS_READ_AGAIN/TLS_WRITE_AGAIN. This also resolves
the semantics of tls_accept_*.

The tls_handshake() function now does I/O and can return
TLS_READ_AGAIN/TLS_WRITE_AGAIN. Calls to tls_read() and tls_write() will
trigger the handshake if it has not already completed, meaning that in many
cases existing code will continue to work.

Discussed over many coffees at l2k15.

ok beck@ bluhm@

5 files changed, 120 insertions, 59 deletions

diff --git a/lib/libtls/tls.c b/lib/libtls/tls.c
index 6b9834565c9..fe5bc964e29 100644
--- a/lib/libtls/tls.c
+++ b/lib/libtls/tls.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls.c,v 1.19 2015/09/09 19:49:07 jsing Exp $ */
+/* $OpenBSD: tls.c,v 1.20 2015/09/10 10:14:20 jsing Exp $ */
 /*
  * Copyright (c) 2014 Joel Sing <jsing@openbsd.org>
  *
@@ -315,6 +315,9 @@ tls_reset(struct tls *ctx)
 	ctx->socket = -1;
 	ctx->state = 0;
 
+	free(ctx->servername);
+	ctx->servername = NULL;
+
 	free(ctx->errmsg);
 	ctx->errmsg = NULL;
 	ctx->errnum = 0;
@@ -367,6 +370,20 @@ tls_ssl_error(struct tls *ctx, SSL *ssl_conn, int ssl_ret, const char *prefix)
 }
 
 int
+tls_handshake(struct tls *ctx)
+{
+	int rv = -1;
+
+	if ((ctx->flags & TLS_CLIENT) != 0)
+		rv = tls_handshake_client(ctx);
+	else if ((ctx->flags & TLS_SERVER_CONN) != 0)
+		rv = tls_handshake_server(ctx);
+
+	errno = 0;
+	return (rv);
+}
+
+int
 tls_read(struct tls *ctx, void *buf, size_t buflen, size_t *outlen)
 {
 	int ssl_ret;
@@ -374,13 +391,17 @@ tls_read(struct tls *ctx, void *buf, size_t buflen, size_t *outlen)
 
 	*outlen = 0;
 
+	if ((ctx->state & TLS_HANDSHAKE_COMPLETE) == 0) {
+		if ((rv = tls_handshake(ctx)) != 0)
+			goto out;
+	}
+
 	if (buflen > INT_MAX) {
 		tls_set_errorx(ctx, "buflen too long");
 		goto out;
 	}
 
-	ssl_ret = SSL_read(ctx->ssl_conn, buf, buflen);
-	if (ssl_ret > 0) {
+	if ((ssl_ret = SSL_read(ctx->ssl_conn, buf, buflen)) > 0) {
 		*outlen = (size_t)ssl_ret;
 		rv = 0;
 		goto out;
@@ -400,13 +421,17 @@ tls_write(struct tls *ctx, const void *buf, size_t buflen, size_t *outlen)
 
 	*outlen = 0;
 
+	if ((ctx->state & TLS_HANDSHAKE_COMPLETE) == 0) {
+		if ((rv = tls_handshake(ctx)) != 0)
+			goto out;
+	}
+
 	if (buflen > INT_MAX) {
 		tls_set_errorx(ctx, "buflen too long");
 		goto out;
 	}
 
-	ssl_ret = SSL_write(ctx->ssl_conn, buf, buflen);
-	if (ssl_ret > 0) {
+	if ((ssl_ret = SSL_write(ctx->ssl_conn, buf, buflen)) > 0) {
 		*outlen = (size_t)ssl_ret;
 		rv = 0;
 		goto out;
diff --git a/lib/libtls/tls.h b/lib/libtls/tls.h
index 579a97798e8..8548fe1d831 100644
--- a/lib/libtls/tls.h
+++ b/lib/libtls/tls.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls.h,v 1.15 2015/09/10 09:10:42 jsing Exp $ */
+/* $OpenBSD: tls.h,v 1.16 2015/09/10 10:14:20 jsing Exp $ */
 /*
  * Copyright (c) 2014 Joel Sing <jsing@openbsd.org>
  *
@@ -94,6 +94,7 @@ int tls_connect_fds(struct tls *_ctx, int _fd_read, int _fd_write,
 int tls_connect_servername(struct tls *_ctx, const char *_host,
     const char *_port, const char *_servername);
 int tls_connect_socket(struct tls *_ctx, int _s, const char *_servername);
+int tls_handshake(struct tls *_ctx);
 int tls_read(struct tls *_ctx, void *_buf, size_t _buflen, size_t *_outlen);
 int tls_write(struct tls *_ctx, const void *_buf, size_t _buflen,
     size_t *_outlen);
diff --git a/lib/libtls/tls_client.c b/lib/libtls/tls_client.c
index 81e47da0e1d..fb7f3a6f755 100644
--- a/lib/libtls/tls_client.c
+++ b/lib/libtls/tls_client.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls_client.c,v 1.25 2015/09/09 19:49:07 jsing Exp $ */
+/* $OpenBSD: tls_client.c,v 1.26 2015/09/10 10:14:20 jsing Exp $ */
 /*
  * Copyright (c) 2014 Joel Sing <jsing@openbsd.org>
  *
@@ -166,20 +166,23 @@ tls_connect_fds(struct tls *ctx, int fd_read, int fd_write,
     const char *servername)
 {
 	union { struct in_addr ip4; struct in6_addr ip6; } addrbuf;
-	X509 *cert = NULL;
-	int ret, err;
+	int rv = -1;
 
 	if ((ctx->flags & TLS_CLIENT) == 0) {
 		tls_set_errorx(ctx, "not a client context");
 		goto err;
 	}
 
-	if (ctx->state & TLS_STATE_CONNECTING)
-		goto connecting;
-
 	if (fd_read < 0 || fd_write < 0) {
 		tls_set_errorx(ctx, "invalid file descriptors");
-		return (-1);
+		goto err;
+	}
+
+	if (servername != NULL) {
+		if ((ctx->servername = strdup(servername)) == NULL) {
+			tls_set_errorx(ctx, "out of memory");
+			goto err;
+		}
 	}
 
 	if ((ctx->ssl_ctx = SSL_CTX_new(SSLv23_client_method())) == NULL) {
@@ -230,16 +233,28 @@ tls_connect_fds(struct tls *ctx, int fd_read, int fd_write,
 		}
 	}
 
- connecting:
-	if ((ret = SSL_connect(ctx->ssl_conn)) != 1) {
-		err = tls_ssl_error(ctx, ctx->ssl_conn, ret, "connect");
-		if (err == TLS_READ_AGAIN || err == TLS_WRITE_AGAIN) {
-			ctx->state |= TLS_STATE_CONNECTING;
-			return (err);
-		}
+	rv = 0;
+
+ err:
+	return (rv);
+}
+
+int
+tls_handshake_client(struct tls *ctx)
+{
+	X509 *cert = NULL;
+	int ssl_ret;
+	int rv = -1;
+
+	if ((ctx->flags & TLS_CLIENT) == 0) {
+		tls_set_errorx(ctx, "not a client context");
+		goto err;
+	}
+
+	if ((ssl_ret = SSL_connect(ctx->ssl_conn)) != 1) {
+		rv = tls_ssl_error(ctx, ctx->ssl_conn, ssl_ret, "handshake");
 		goto err;
 	}
-	ctx->state &= ~TLS_STATE_CONNECTING;
 
 	if (ctx->config->verify_name) {
 		cert = SSL_get_peer_certificate(ctx->ssl_conn);
@@ -247,19 +262,20 @@ tls_connect_fds(struct tls *ctx, int fd_read, int fd_write,
 			tls_set_errorx(ctx, "no server certificate");
 			goto err;
 		}
-		if ((ret = tls_check_servername(ctx, cert, servername)) != 0) {
-			if (ret != -2)
+		if ((rv = tls_check_servername(ctx, cert,
+		    ctx->servername)) != 0) {
+			if (rv != -2)
 				tls_set_errorx(ctx, "name `%s' not present in"
-				    " server certificate", servername);
+				    " server certificate", ctx->servername);
 			goto err;
 		}
-		X509_free(cert);
 	}
 
-	return (0);
+	ctx->state |= TLS_HANDSHAKE_COMPLETE;
+	rv = 0;
 
  err:
 	X509_free(cert);
 
-	return (-1);
+	return (rv);
 }
diff --git a/lib/libtls/tls_internal.h b/lib/libtls/tls_internal.h
index 78ae542cb6d..a5399d5594b 100644
--- a/lib/libtls/tls_internal.h
+++ b/lib/libtls/tls_internal.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls_internal.h,v 1.17 2015/09/10 09:10:42 jsing Exp $ */
+/* $OpenBSD: tls_internal.h,v 1.18 2015/09/10 10:14:20 jsing Exp $ */
 /*
  * Copyright (c) 2014 Jeremie Courreges-Anglas <jca@openbsd.org>
  * Copyright (c) 2014 Joel Sing <jsing@openbsd.org>
@@ -52,7 +52,7 @@ struct tls_config {
 #define TLS_SERVER		(1 << 1)
 #define TLS_SERVER_CONN		(1 << 2)
 
-#define TLS_STATE_CONNECTING	(1 << 0)
+#define TLS_HANDSHAKE_COMPLETE	(1 << 0)
 
 struct tls {
 	struct tls_config *config;
@@ -62,6 +62,7 @@ struct tls {
 	char *errmsg;
 	int errnum;
 
+	char *servername;
 	int socket;
 
 	SSL *ssl_conn;
@@ -76,6 +77,8 @@ int tls_configure_keypair(struct tls *ctx, int);
 int tls_configure_server(struct tls *ctx);
 int tls_configure_ssl(struct tls *ctx);
 int tls_configure_ssl_verify(struct tls *ctx, int verify);
+int tls_handshake_client(struct tls *ctx);
+int tls_handshake_server(struct tls *ctx);
 int tls_host_port(const char *hostport, char **host, char **port);
 int tls_set_error(struct tls *ctx, const char *fmt, ...)
     __attribute__((__format__ (printf, 2, 3)))
diff --git a/lib/libtls/tls_server.c b/lib/libtls/tls_server.c
index a3cee095962..3dfd29ac19f 100644
--- a/lib/libtls/tls_server.c
+++ b/lib/libtls/tls_server.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls_server.c,v 1.14 2015/09/10 09:10:42 jsing Exp $ */
+/* $OpenBSD: tls_server.c,v 1.15 2015/09/10 10:14:20 jsing Exp $ */
 /*
  * Copyright (c) 2014 Joel Sing <jsing@openbsd.org>
  *
@@ -110,54 +110,70 @@ tls_configure_server(struct tls *ctx)
 }
 
 int
+tls_accept_socket(struct tls *ctx, struct tls **cctx, int socket)
+{
+	return (tls_accept_fds(ctx, cctx, socket, socket));
+}
+
+int
 tls_accept_fds(struct tls *ctx, struct tls **cctx, int fd_read, int fd_write)
 {
-	struct tls *conn_ctx = *cctx;
-	int ret, err;
+	struct tls *conn_ctx = NULL;
 	
 	if ((ctx->flags & TLS_SERVER) == 0) {
 		tls_set_errorx(ctx, "not a server context");
 		goto err;
 	}
 
-	if (conn_ctx == NULL) {
-		if ((conn_ctx = tls_server_conn(ctx)) == NULL) {
-			tls_set_errorx(ctx, "connection context failure");
-			goto err;
-		}
-		*cctx = conn_ctx;
-
-		if ((conn_ctx->ssl_conn = SSL_new(ctx->ssl_ctx)) == NULL) {
-			tls_set_errorx(ctx, "ssl failure");
-			goto err;
-		}
-		if (SSL_set_app_data(conn_ctx->ssl_conn, conn_ctx) != 1) {
-			tls_set_errorx(ctx, "ssl application data failure");
-			goto err;
-		}
-		if (SSL_set_rfd(conn_ctx->ssl_conn, fd_read) != 1 ||
-		    SSL_set_wfd(conn_ctx->ssl_conn, fd_write) != 1) {
-			tls_set_errorx(ctx, "ssl file descriptor failure");
-			goto err;
-		}
+	if ((conn_ctx = tls_server_conn(ctx)) == NULL) {
+		tls_set_errorx(ctx, "connection context failure");
+		goto err;
 	}
 
-	if ((ret = SSL_accept(conn_ctx->ssl_conn)) != 1) {
-		err = tls_ssl_error(ctx, conn_ctx->ssl_conn, ret, "accept");
-		if (err == TLS_READ_AGAIN || err == TLS_WRITE_AGAIN) {
-			return (err);
-		}
+	if ((conn_ctx->ssl_conn = SSL_new(ctx->ssl_ctx)) == NULL) {
+		tls_set_errorx(ctx, "ssl failure");
+		goto err;
+	}
+	if (SSL_set_app_data(conn_ctx->ssl_conn, conn_ctx) != 1) {
+		tls_set_errorx(ctx, "ssl application data failure");
+		goto err;
+	}
+	if (SSL_set_rfd(conn_ctx->ssl_conn, fd_read) != 1 ||
+	    SSL_set_wfd(conn_ctx->ssl_conn, fd_write) != 1) {
+		tls_set_errorx(ctx, "ssl file descriptor failure");
 		goto err;
 	}
 
+	*cctx = conn_ctx;
+
 	return (0);
 
  err:
+	tls_free(conn_ctx);
+
+	*cctx = NULL;
+
 	return (-1);
 }
 
 int
-tls_accept_socket(struct tls *ctx, struct tls **cctx, int socket)
+tls_handshake_server(struct tls *ctx)
 {
-	return (tls_accept_fds(ctx, cctx, socket, socket));
+	int ssl_ret;
+	int rv = -1;
+
+	if ((ctx->flags & TLS_SERVER_CONN) == 0) {
+		tls_set_errorx(ctx, "not a server connection context");
+		goto err;
+	}
+
+	if ((ssl_ret = SSL_accept(ctx->ssl_conn)) != 1) {
+		rv = tls_ssl_error(ctx, ctx->ssl_conn, ssl_ret, "handshake");
+		goto err;
+	}
+
+	ctx->state |= TLS_HANDSHAKE_COMPLETE;
+
+ err:
+	return (rv);
 }