Fix KTLS with BIO_new_connect

When a socket connection is done using BIO_new_connect,
the ktls_enable is done too early, and fails with ENOTCONN.
Therefore the KLTS ioctl will fail later with ENOPROTOOPT.
Fix that by doing the ktls_enable after the connection
succeeded, not when the socket is created as that will
always fail.

One example where this happens is doit_localhost in
test/ssl_old_test.c, and therefore, contrary to the expectation
the -client_ktls option did never enable the client KTLS
connection, but this was not noticed, because there was no
diagnostic output, and it was only visible with strace output.

Also enhanced the ssl_old_test -client_ktls/-server_ktls
options together with -v option to print a summary line
if and how KTLS was negotiated in server and client.

While I am already there adjusted the usage info of
the -s_cert, -s_key commands, and allow -time to print the
timings of ktls connections.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18318)

diff --git a/crypto/bio/bio_sock.c b/crypto/bio/bio_sock.c
index b827c5b902b19a4ab46cd337617b43d32f9e9993..2a50ce55d1675665c31f148aa2419d740ae1cc7d 100644 (file)
--- a/crypto/bio/bio_sock.c
+++ b/crypto/bio/bio_sock.c
@@ -400,7 +400,7 @@ int BIO_socket_wait(int fd, int for_read, time_t max_time)
         return 1;
 
     now = time(NULL);
-    if (max_time <= now)
+    if (max_time < now)
         return 0;
 
     FD_ZERO(&confds);

diff --git a/crypto/bio/bio_sock2.c b/crypto/bio/bio_sock2.c
index d03cdf4646a5726f7d03b83623ab3af5f6622a17..8cd273420ae585fd09b9b4c0b18615d392ed94ad 100644 (file)
--- a/crypto/bio/bio_sock2.c
+++ b/crypto/bio/bio_sock2.c
@@ -53,17 +53,6 @@ int BIO_socket(int domain, int socktype, int protocol, int options)
         ERR_raise(ERR_LIB_BIO, BIO_R_UNABLE_TO_CREATE_SOCKET);
         return INVALID_SOCKET;
     }
-# ifndef OPENSSL_NO_KTLS
-    {
-        /*
-         * The new socket is created successfully regardless of ktls_enable.
-         * ktls_enable doesn't change any functionality of the socket, except
-         * changing the setsockopt to enable the processing of ktls_start.
-         * Thus, it is not a problem to call it for non-TLS sockets.
-         */
-        ktls_enable(sock);
-    }
-# endif
 
     return sock;
 }
@@ -192,6 +181,15 @@ int BIO_connect(int sock, const BIO_ADDR *addr, int options)
         }
         return 0;
     }
+# ifndef OPENSSL_NO_KTLS
+    /*
+     * The new socket is created successfully regardless of ktls_enable.
+     * ktls_enable doesn't change any functionality of the socket, except
+     * changing the setsockopt to enable the processing of ktls_start.
+     * Thus, it is not a problem to call it for non-TLS sockets.
+     */
+    ktls_enable(sock);
+# endif
     return 1;
 }
 

diff --git a/crypto/bio/bss_conn.c b/crypto/bio/bss_conn.c
index 87238319fbbb6201771029b2f9a1b43f70c962d9..13ac7f2d95aa96490bbbaa3afbe39e3fc6471e37 100644 (file)
--- a/crypto/bio/bss_conn.c
+++ b/crypto/bio/bss_conn.c
@@ -191,6 +191,9 @@ static int conn_state(BIO *b, BIO_CONNECT *c)
             break;
 
         case BIO_CONN_S_BLOCKED_CONNECT:
+            /* wait for socket being writable, before querying BIO_sock_error */
+            if (BIO_socket_wait(b->num, 0, time(NULL)) == 0)
+                break;
             i = BIO_sock_error(b->num);
             if (i != 0) {
                 BIO_clear_retry_flags(b);
@@ -208,8 +211,18 @@ static int conn_state(BIO *b, BIO_CONNECT *c)
                 ERR_raise(ERR_LIB_BIO, BIO_R_NBIO_CONNECT_ERROR);
                 ret = 0;
                 goto exit_loop;
-            } else
+            } else {
                 c->state = BIO_CONN_S_OK;
+# ifndef OPENSSL_NO_KTLS
+                /*
+                 * The new socket is created successfully regardless of ktls_enable.
+                 * ktls_enable doesn't change any functionality of the socket, except
+                 * changing the setsockopt to enable the processing of ktls_start.
+                 * Thus, it is not a problem to call it for non-TLS sockets.
+                 */
+                ktls_enable(b->num);
+# endif
+            }
             break;
 
         case BIO_CONN_S_CONNECT_ERROR:

diff --git a/test/ssl_old_test.c b/test/ssl_old_test.c
index 6b3427a138d48f8dcbe6bcbdaca03716117d1a78..829f683a91b9de3c4a1b7d1972a721f68c746468 100644 (file)
--- a/test/ssl_old_test.c
+++ b/test/ssl_old_test.c
@@ -666,9 +666,9 @@ static void sv_usage(void)
 #endif
     fprintf(stderr, " -CApath arg   - PEM format directory of CA's\n");
     fprintf(stderr, " -CAfile arg   - PEM format file of CA's\n");
-    fprintf(stderr, " -cert arg     - Server certificate file\n");
+    fprintf(stderr, " -s_cert arg   - Server certificate file\n");
     fprintf(stderr,
-            " -key arg      - Server key file (default: same as -cert)\n");
+            " -s_key arg    - Server key file (default: same as -cert)\n");
     fprintf(stderr, " -c_cert arg   - Client certificate file\n");
     fprintf(stderr,
             " -c_key arg    - Client key file (default: same as -c_cert)\n");
@@ -1296,7 +1296,7 @@ int main(int argc, char *argv[])
     }
 
     if (print_time) {
-        if (bio_type != BIO_PAIR) {
+        if (bio_type == BIO_MEM) {
             fprintf(stderr, "Using BIO pair (-bio_pair)\n");
             bio_type = BIO_PAIR;
         }
@@ -2024,7 +2024,7 @@ int doit_localhost(SSL *s_ssl, SSL *c_ssl, int family, long count,
                 r = BIO_write(c_ssl_bio, cbuf, i);
                 if (r < 0) {
                     if (!BIO_should_retry(c_ssl_bio)) {
-                        fprintf(stderr, "ERROR in CLIENT\n");
+                        fprintf(stderr, "ERROR in CLIENT (write)\n");
                         err_in_client = 1;
                         goto err;
                     }
@@ -2050,7 +2050,7 @@ int doit_localhost(SSL *s_ssl, SSL *c_ssl, int family, long count,
                 r = BIO_read(c_ssl_bio, cbuf, sizeof(cbuf));
                 if (r < 0) {
                     if (!BIO_should_retry(c_ssl_bio)) {
-                        fprintf(stderr, "ERROR in CLIENT\n");
+                        fprintf(stderr, "ERROR in CLIENT (read)\n");
                         err_in_client = 1;
                         goto err;
                     }
@@ -2103,7 +2103,7 @@ int doit_localhost(SSL *s_ssl, SSL *c_ssl, int family, long count,
                 r = BIO_write(s_ssl_bio, sbuf, i);
                 if (r < 0) {
                     if (!BIO_should_retry(s_ssl_bio)) {
-                        fprintf(stderr, "ERROR in SERVER\n");
+                        fprintf(stderr, "ERROR in SERVER (write)\n");
                         err_in_server = 1;
                         goto err;
                     }
@@ -2124,7 +2124,7 @@ int doit_localhost(SSL *s_ssl, SSL *c_ssl, int family, long count,
                 r = BIO_read(s_ssl_bio, sbuf, sizeof(sbuf));
                 if (r < 0) {
                     if (!BIO_should_retry(s_ssl_bio)) {
-                        fprintf(stderr, "ERROR in SERVER\n");
+                        fprintf(stderr, "ERROR in SERVER (read)\n");
                         err_in_server = 1;
                         goto err;
                     }
@@ -2144,8 +2144,25 @@ int doit_localhost(SSL *s_ssl, SSL *c_ssl, int family, long count,
     }
     while (cw_num > 0 || cr_num > 0 || sw_num > 0 || sr_num > 0);
 
-    if (verbose)
+    if (verbose) {
         print_details(c_ssl, "DONE via TCP connect: ");
+
+        if (BIO_get_ktls_send(SSL_get_wbio(s_ssl))
+                && BIO_get_ktls_recv(SSL_get_rbio(s_ssl)))
+            BIO_printf(bio_stdout, "Server using Kernel TLS in both directions\n");
+        else if (BIO_get_ktls_send(SSL_get_wbio(s_ssl)))
+            BIO_printf(bio_stdout, "Server using Kernel TLS for sending\n");
+        else if (BIO_get_ktls_recv(SSL_get_rbio(s_ssl)))
+            BIO_printf(bio_stdout, "Server using Kernel TLS for receiving\n");
+
+        if (BIO_get_ktls_send(SSL_get_wbio(c_ssl))
+                && BIO_get_ktls_recv(SSL_get_rbio(c_ssl)))
+            BIO_printf(bio_stdout, "Client using Kernel TLS in both directions\n");
+        else if (BIO_get_ktls_send(SSL_get_wbio(c_ssl)))
+            BIO_printf(bio_stdout, "Client using Kernel TLS for sending\n");
+        else if (BIO_get_ktls_recv(SSL_get_rbio(c_ssl)))
+            BIO_printf(bio_stdout, "Client using Kernel TLS for receiving\n");
+    }
 # ifndef OPENSSL_NO_NEXTPROTONEG
     if (verify_npn(c_ssl, s_ssl) < 0)
         goto end;