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/src/lib/libtls/tls.c b/src/lib/libtls/tls.c
index 6b9834565c..fe5bc964e2 100644
--- a/src/lib/libtls/tls.c
+++ b/src/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/src/lib/libtls/tls.h b/src/lib/libtls/tls.h
index 579a97798e..8548fe1d83 100644
--- a/src/lib/libtls/tls.h
+++ b/src/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/src/lib/libtls/tls_client.c b/src/lib/libtls/tls_client.c
index 81e47da0e1..fb7f3a6f75 100644
--- a/src/lib/libtls/tls_client.c
+++ b/src/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/src/lib/libtls/tls_internal.h b/src/lib/libtls/tls_internal.h
index 78ae542cb6..a5399d5594 100644
--- a/src/lib/libtls/tls_internal.h
+++ b/src/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/src/lib/libtls/tls_server.c b/src/lib/libtls/tls_server.c
index a3cee09596..3dfd29ac19 100644
--- a/src/lib/libtls/tls_server.c
+++ b/src/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);
 }