[openssl] master update

dev at ddvo.net dev at ddvo.net
Tue Jun 16 16:53:19 UTC 2020


The branch master has been updated
       via  e98c7350bfaf0ae1f2b72d68d4c5721de24a478f (commit)
       via  3f528d0899e8fe850c63d600ee29146bc8a9c125 (commit)
       via  c0fff24e0dc168a0dff4209d120733e6684e0767 (commit)
      from  9ac916c7529a21cd01d1b539362abf8402719e30 (commit)


- Log -----------------------------------------------------------------
commit e98c7350bfaf0ae1f2b72d68d4c5721de24a478f
Author: Dr. David von Oheimb <David.von.Oheimb at siemens.com>
Date:   Thu May 28 15:16:45 2020 +0200

    Improve BIO_socket_wait(), BIO_wait(), BIO_connect_retry(), and their docs
    
    Add/extend range check for 'fd' argument of BIO_socket_wait() and bio_wait()
    Correct nap time calculations in bio_wait(), thus correcting also BIO_wait()
    Update a type cast from 'unsigned long' to 'unsigned int'
    Extend the comments and documentation of BIO_wait()
    
    Rename BIO_connect_retry() to BIO_do_connect_retry()
    Make its 'timeout' argument < 0 lead to BIO_do_connect() tried only once
    Add optional 'nap_milliseconds' parameter determining the polling granularity
    Correct and generalize the retry case checking
    Extend the comments and documentation of BIO_do_connect_retry()
    
    Reviewed-by: Bernd Edlinger <bernd.edlinger at hotmail.de>
    Reviewed-by: Matt Caswell <matt at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/11986)

commit 3f528d0899e8fe850c63d600ee29146bc8a9c125
Author: Dr. David von Oheimb <David.von.Oheimb at siemens.com>
Date:   Thu Jun 4 10:33:28 2020 +0200

    Add OPENSSL_strdup failure check to cpt_ctrl() in bss_acpt.c
    
    Reviewed-by: Bernd Edlinger <bernd.edlinger at hotmail.de>
    Reviewed-by: Matt Caswell <matt at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/11986)

commit c0fff24e0dc168a0dff4209d120733e6684e0767
Author: Dr. David von Oheimb <David.von.Oheimb at siemens.com>
Date:   Wed Jun 3 07:49:27 2020 +0200

    Fix err checking and mem leaks of BIO_set_conn_port and BIO_set_conn_address
    
    Reviewed-by: Bernd Edlinger <bernd.edlinger at hotmail.de>
    Reviewed-by: Matt Caswell <matt at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/11986)

-----------------------------------------------------------------------

Summary of changes:
 crypto/bio/b_sock.c             |   2 +
 crypto/bio/bio_lib.c            | 116 +++++++++++++++++++++++-----------------
 crypto/bio/bss_acpt.c           |   6 ++-
 crypto/bio/bss_conn.c           |  25 ++++++---
 crypto/http/http_client.c       |   4 +-
 doc/man3/BIO_socket_wait.pod    |  43 +++++++++------
 doc/man3/OSSL_CMP_CTX_new.pod   |   2 +-
 doc/man3/OSSL_HTTP_transfer.pod |   2 +-
 include/openssl/bio.h           |   4 +-
 util/libcrypto.num              |   2 +-
 10 files changed, 125 insertions(+), 81 deletions(-)

diff --git a/crypto/bio/b_sock.c b/crypto/bio/b_sock.c
index f5d2a627cb..79f7743b2f 100644
--- a/crypto/bio/b_sock.c
+++ b/crypto/bio/b_sock.c
@@ -387,6 +387,8 @@ int BIO_socket_wait(int fd, int for_read, time_t max_time)
     struct timeval tv;
     time_t now;
 
+    if (fd < 0 || fd >= FD_SETSIZE)
+        return -1;
     if (max_time == 0)
         return 1;
 
diff --git a/crypto/bio/bio_lib.c b/crypto/bio/bio_lib.c
index 1579f7c366..c3c798d4b4 100644
--- a/crypto/bio/bio_lib.c
+++ b/crypto/bio/bio_lib.c
@@ -786,41 +786,49 @@ void bio_cleanup(void)
 }
 
 /* Internal variant of the below BIO_wait() not calling BIOerr() */
-static int bio_wait(BIO *bio, time_t max_time, unsigned int milliseconds)
+static int bio_wait(BIO *bio, time_t max_time, unsigned int nap_milliseconds)
 {
 #ifndef OPENSSL_NO_SOCK
     int fd;
 #endif
+    long sec_diff;
 
-    if (max_time == 0)
+    if (max_time == 0) /* no timeout */
         return 1;
 
 #ifndef OPENSSL_NO_SOCK
-    if (BIO_get_fd(bio, &fd) > 0)
+    if (BIO_get_fd(bio, &fd) > 0 && fd < FD_SETSIZE)
         return BIO_socket_wait(fd, BIO_should_read(bio), max_time);
 #endif
-    if (milliseconds > 1000) {
-        long sec_diff = (long)(max_time - time(NULL)); /* might overflow */
+    /* fall back to polling since no sockets are available */
 
-        if (sec_diff <= 0)
-            return 0; /* timeout */
-        if ((unsigned long)sec_diff < milliseconds / 1000)
-            milliseconds = (unsigned long)sec_diff * 1000;
+    sec_diff = (long)(max_time - time(NULL)); /* might overflow */
+    if (sec_diff < 0)
+        return 0; /* clearly timeout */
+
+    /* now take a nap at most the given number of milliseconds */
+    if (sec_diff == 0) { /* we are below the 1 seconds resolution of max_time */
+        if (nap_milliseconds > 1000)
+            nap_milliseconds = 1000;
+    } else { /* for sec_diff > 0, take min(sec_diff * 1000, nap_milliseconds) */
+        if ((unsigned long)sec_diff * 1000 < nap_milliseconds)
+            nap_milliseconds = (unsigned int)sec_diff * 1000;
     }
-    ossl_sleep(milliseconds);
+    ossl_sleep(nap_milliseconds);
     return 1;
 }
 
-/*
+/*-
  * Wait on (typically socket-based) BIO at most until max_time.
- * Succeed immediately if max_time == 0. If sockets are not available succeed
- * after waiting at most given milliseconds in order to avoid a tight busy loop.
- * Call BIOerr(...) unless success.
+ * Succeed immediately if max_time == 0.
+ * If sockets are not available support polling: succeed after waiting at most
+ * the number of nap_milliseconds in order to avoid a tight busy loop.
+ * Call BIOerr(...) on timeout or error.
  * Returns -1 on error, 0 on timeout, and 1 on success.
  */
-int BIO_wait(BIO *bio, time_t max_time, unsigned int milliseconds)
+int BIO_wait(BIO *bio, time_t max_time, unsigned int nap_milliseconds)
 {
-    int rv = bio_wait(bio, max_time, milliseconds);
+    int rv = bio_wait(bio, max_time, nap_milliseconds);
 
     if (rv <= 0)
         BIOerr(0, rv == 0 ? BIO_R_TRANSFER_TIMEOUT : BIO_R_TRANSFER_ERROR);
@@ -828,13 +836,16 @@ int BIO_wait(BIO *bio, time_t max_time, unsigned int milliseconds)
 }
 
 /*
- * Connect via given BIO using BIO_do_handshake() until success/timeout/error.
- * Parameter timeout == 0 means infinite, < 0 leads to immediate timeout error.
+ * Connect via given BIO using BIO_do_connect() until success/timeout/error.
+ * Parameter timeout == 0 means no timeout, < 0 means exactly one try.
+ * For non-blocking and potentially even non-socket BIOs perform polling with
+ * the given density: between polls sleep nap_milliseconds using BIO_wait()
+ * in order to avoid a tight busy loop.
  * Returns -1 on error, 0 on timeout, and 1 on success.
  */
-int BIO_connect_retry(BIO *bio, int timeout)
+int BIO_do_connect_retry(BIO *bio, int timeout, int nap_milliseconds)
 {
-    int blocking = timeout == 0;
+    int blocking = timeout <= 0;
     time_t max_time = timeout > 0 ? time(NULL) + timeout : 0;
     int rv;
 
@@ -843,40 +854,47 @@ int BIO_connect_retry(BIO *bio, int timeout)
         return -1;
     }
 
-    if (timeout < 0) {
-        BIOerr(0, BIO_R_CONNECT_TIMEOUT);
-        return 0;
-    }
-
-    if (!blocking)
-        BIO_set_nbio(bio, 1);
-
- retry: /* it does not help here to set SSL_MODE_AUTO_RETRY */
-    rv = BIO_do_handshake(bio); /* This indirectly calls ERR_clear_error(); */
-
-    if (rv <= 0) {
-        if (get_last_sys_error() == ETIMEDOUT) {
-            /*
-             * if blocking, despite blocking BIO, BIO_do_handshake() timed out
-             * when non-blocking, BIO_do_handshake() timed out early
-             * with rv == -1 and get_last_sys_error() == 0
-             */
-            ERR_clear_error();
-            (void)BIO_reset(bio);
-            /*
-             * unless using BIO_reset(), blocking next connect() may crash and
-             * non-blocking next BIO_do_handshake() will fail
-             */
-            goto retry;
-        } else if (BIO_should_retry(bio)) {
-            /* will not actually wait if timeout == 0 (i.e., blocking BIO) */
-            rv = bio_wait(bio, max_time, 100 /* milliseconds */);
+    if (nap_milliseconds < 0)
+        nap_milliseconds = 100;
+    BIO_set_nbio(bio, !blocking);
+
+ retry:
+    rv = BIO_do_connect(bio); /* This may indirectly call ERR_clear_error(); */
+
+    if (rv <= 0) { /* could be timeout or retryable error or fatal error */
+        int err = ERR_peek_last_error();
+        int reason = ERR_GET_REASON(err);
+        int do_retry = BIO_should_retry(bio); /* may be 1 only if !blocking */
+
+        if (ERR_GET_LIB(err) == ERR_LIB_BIO) {
+            switch (reason) {
+            case ERR_R_SYS_LIB:
+                /*
+                 * likely retryable system error occurred, which may be
+                 * EAGAIN (resource temporarily unavailable) some 40 secs after
+                 * calling getaddrinfo(): Temporary failure in name resolution
+                 * or a premature ETIMEDOUT, some 30 seconds after connect()
+                 */
+            case BIO_R_CONNECT_ERROR:
+            case BIO_R_NBIO_CONNECT_ERROR:
+                /* some likely retryable connection error occurred */
+                (void)BIO_reset(bio); /* often needed to avoid retry failure */
+                do_retry = 1;
+                break;
+            default:
+                break;
+            }
+        }
+        if (timeout >= 0 && do_retry) {
+            ERR_clear_error(); /* using ERR_pop_to_mark() would be cleaner */
+            /* will not actually wait if timeout == 0 (i.e., blocking BIO): */
+            rv = bio_wait(bio, max_time, nap_milliseconds);
             if (rv > 0)
                 goto retry;
             BIOerr(0, rv == 0 ? BIO_R_CONNECT_TIMEOUT : BIO_R_CONNECT_ERROR);
         } else {
             rv = -1;
-            if (ERR_peek_error() == 0) /* missing error queue entry */
+            if (err == 0) /* missing error queue entry */
                 BIOerr(0, BIO_R_CONNECT_ERROR); /* workaround: general error */
         }
     }
diff --git a/crypto/bio/bss_acpt.c b/crypto/bio/bss_acpt.c
index 3523f68edd..7f1af71e0f 100644
--- a/crypto/bio/bss_acpt.c
+++ b/crypto/bio/bss_acpt.c
@@ -433,8 +433,10 @@ static long acpt_ctrl(BIO *b, int cmd, long num, void *ptr)
                 b->init = 1;
             } else if (num == 1) {
                 OPENSSL_free(data->param_serv);
-                data->param_serv = OPENSSL_strdup(ptr);
-                b->init = 1;
+                if ((data->param_serv = OPENSSL_strdup(ptr)) == NULL)
+                    ret = 0;
+                else
+                    b->init = 1;
             } else if (num == 2) {
                 data->bind_mode |= BIO_SOCK_NONBLOCK;
             } else if (num == 3) {
diff --git a/crypto/bio/bss_conn.c b/crypto/bio/bss_conn.c
index 31a5b58b7d..6cff2a99ac 100644
--- a/crypto/bio/bss_conn.c
+++ b/crypto/bio/bss_conn.c
@@ -438,12 +438,13 @@ static long conn_ctrl(BIO *b, int cmd, long num, void *ptr)
     case BIO_C_SET_CONNECT:
         if (ptr != NULL) {
             b->init = 1;
-            if (num == 0) {
+            if (num == 0) { /* BIO_set_conn_hostname */
                 char *hold_service = data->param_service;
                 /* We affect the hostname regardless.  However, the input
                  * string might contain a host:service spec, so we must
                  * parse it, which might or might not affect the service
                  */
+
                 OPENSSL_free(data->param_hostname);
                 data->param_hostname = NULL;
                 ret = BIO_parse_hostserv(ptr,
@@ -452,19 +453,29 @@ static long conn_ctrl(BIO *b, int cmd, long num, void *ptr)
                                          BIO_PARSE_PRIO_HOST);
                 if (hold_service != data->param_service)
                     OPENSSL_free(hold_service);
-            } else if (num == 1) {
+            } else if (num == 1) { /* BIO_set_conn_port */
                 OPENSSL_free(data->param_service);
-                data->param_service = OPENSSL_strdup(ptr);
-            } else if (num == 2) {
+                if ((data->param_service = OPENSSL_strdup(ptr)) == NULL)
+                    ret = 0;
+            } else if (num == 2) { /* BIO_set_conn_address */
                 const BIO_ADDR *addr = (const BIO_ADDR *)ptr;
+                char *host = BIO_ADDR_hostname_string(addr, 1);
+                char *service = BIO_ADDR_service_string(addr, 1);
+
+                ret = host != NULL && service != NULL;
                 if (ret) {
-                    data->param_hostname = BIO_ADDR_hostname_string(addr, 1);
-                    data->param_service = BIO_ADDR_service_string(addr, 1);
+                    OPENSSL_free(data->param_hostname);
+                    data->param_hostname = host;
+                    OPENSSL_free(data->param_service);
+                    data->param_service = service;
                     BIO_ADDRINFO_free(data->addr_first);
                     data->addr_first = NULL;
                     data->addr_iter = NULL;
+                } else {
+                    OPENSSL_free(host);
+                    OPENSSL_free(service);
                 }
-            } else if (num == 3) {
+            } else if (num == 3) { /* BIO_set_conn_ip_family */
                 data->connect_family = *(int *)ptr;
             } else {
                 ret = 0;
diff --git a/crypto/http/http_client.c b/crypto/http/http_client.c
index 64f877abed..a8dda0050a 100644
--- a/crypto/http/http_client.c
+++ b/crypto/http/http_client.c
@@ -814,7 +814,7 @@ static int update_timeout(int timeout, time_t start_time)
  *   BIO *(*OSSL_HTTP_bio_cb_t) (BIO *bio, void *arg, int conn, int detail);
  * The callback may modify the HTTP BIO provided in the bio argument,
  * whereby it may make use of any custom defined argument 'arg'.
- * During connection establishment, just after BIO_connect_retry(),
+ * During connection establishment, just after BIO_do_connect_retry(),
  * the callback function is invoked with the 'conn' argument being 1
  * 'detail' indicating whether a HTTPS (i.e., TLS) connection is requested.
  * On disconnect 'conn' is 0 and 'detail' indicates that no error occurred.
@@ -873,7 +873,7 @@ BIO *OSSL_HTTP_transfer(const char *server, const char *port, const char *path,
     /* remaining parameters are checked indirectly by the functions called */
 
     (void)ERR_set_mark(); /* prepare removing any spurious libssl errors */
-    if (rbio == NULL && BIO_connect_retry(cbio, timeout) <= 0)
+    if (rbio == NULL && BIO_do_connect_retry(cbio, timeout, -1) <= 0)
         goto end;
     /* now timeout is guaranteed to be >= 0 */
 
diff --git a/doc/man3/BIO_socket_wait.pod b/doc/man3/BIO_socket_wait.pod
index d105dfe698..b00a878c9d 100644
--- a/doc/man3/BIO_socket_wait.pod
+++ b/doc/man3/BIO_socket_wait.pod
@@ -4,8 +4,8 @@
 
 BIO_socket_wait,
 BIO_wait,
-BIO_connect_retry
-- BIO socket utility functions
+BIO_do_connect_retry
+- BIO connection utility functions
 
 =head1 SYNOPSIS
 
@@ -14,8 +14,8 @@ BIO_connect_retry
  #ifndef OPENSSL_NO_SOCK
  int BIO_socket_wait(int fd, int for_read, time_t max_time);
  #endif
- int BIO_wait(BIO *bio, time_t max_time, unsigned int milliseconds);
- int BIO_connect_retry(BIO *bio, long timeout);
+ int BIO_wait(BIO *bio, time_t max_time, unsigned int nap_milliseconds);
+ int BIO_do_connect_retry(BIO *bio, int timeout, int nap_milliseconds);
 
 =head1 DESCRIPTION
 
@@ -23,27 +23,38 @@ BIO_socket_wait() waits on the socket B<fd> for reading if B<for_read> is not 0,
 else for writing, at most until B<max_time>.
 It succeeds immediately if B<max_time> == 0 (which means no timeout given).
 
-BIO_wait() waits at most until B<max_time> on the given B<bio>,
-which is typically socket-based,
-for reading if B<bio> is supposed to read, else for writing.
+BIO_wait() waits at most until B<max_time> on the given (typically socket-based)
+B<bio>, for reading if B<bio> is supposed to read, else for writing.
+It is used by BIO_do_connect_retry() and can be used together L<BIO_read(3)>.
 It succeeds immediately if B<max_time> == 0 (which means no timeout given).
-If sockets are not available it succeeds after waiting at most given
-B<milliseconds> in order to help avoiding a tight busy loop at the caller.
-
-BIO_connect_retry() connects via the given B<bio>, retrying BIO_do_connect()
-until success or a timeout or error condition is reached.
+If sockets are not available it supports polling by succeeding after sleeping
+at most the given B<nap_milliseconds> in order to avoid a tight busy loop.
+Via B<nap_milliseconds> the caller determines the polling granularity.
+
+BIO_do_connect_retry() connects via the given B<bio>.
+It retries BIO_do_connect() as far as needed to reach a definite outcome,
+i.e., connection succeeded, timeout has been reached, or an error occurred.
+For non-blocking and potentially even non-socket BIOs it polls
+every B<nap_milliseconds> and sleeps in between using BIO_wait().
+If B<nap_milliseconds> is < 0 then a default value of 100 ms is used.
 If the B<timeout> parameter is > 0 this indicates the maximum number of seconds
-to wait until the connection is established. A value of 0 enables waiting
-indefinitely, while a value < 0 immediately leads to a timeout condition.
+to wait until the connection is established or a definite error occurred.
+A value of 0 enables waiting indefinitely (i.e, no timeout),
+while a value < 0 means that BIO_do_connect() is tried only once.
+The function may, directly or indirectly, invoke ERR_clear_error().
 
 =head1 RETURN VALUES
 
-BIO_socket_wait(), BIO_wait(), and BIO_connect_retry()
+BIO_socket_wait(), BIO_wait(), and BIO_do_connect_retry()
 return -1 on error, 0 on timeout, and 1 on success.
 
+=head1 SEE ALSO
+
+L<BIO_do_connect(3)>, L<BIO_read(3)>
+
 =head1 HISTORY
 
-BIO_socket_wait(), BIO_wait(), and BIO_connect_retry()
+BIO_socket_wait(), BIO_wait(), and BIO_do_connect_retry()
 were added in OpenSSL 3.0.
 
 =head1 COPYRIGHT
diff --git a/doc/man3/OSSL_CMP_CTX_new.pod b/doc/man3/OSSL_CMP_CTX_new.pod
index 97927fb45e..e8237b46e7 100644
--- a/doc/man3/OSSL_CMP_CTX_new.pod
+++ b/doc/man3/OSSL_CMP_CTX_new.pod
@@ -337,7 +337,7 @@ function, which has the prototype
 The callback may modify the BIO B<bio> provided by OSSL_CMP_MSG_http_perform(),
 whereby it may make use of a custom defined argument B<ctx>
 stored in the OSSL_CMP_CTX by means of OSSL_CMP_CTX_set_http_cb_arg().
-During connection establishment, just after calling BIO_connect_retry(),
+During connection establishment, just after calling BIO_do_connect_retry(),
 the function is invoked with the B<connect> argument being 1 and the B<detail>
 argument being 1 if HTTPS is requested, i.e., SSL/TLS should be enabled. On
 disconnect B<connect> is 0 and B<detail> is 1 in case no error occurred, else 0.
diff --git a/doc/man3/OSSL_HTTP_transfer.pod b/doc/man3/OSSL_HTTP_transfer.pod
index e0adb2a1d1..4459f541f3 100644
--- a/doc/man3/OSSL_HTTP_transfer.pod
+++ b/doc/man3/OSSL_HTTP_transfer.pod
@@ -154,7 +154,7 @@ B<bio_update_fn> is a BIO connect/disconnect callback function with prototype
 The callback may modify the HTTP BIO provided in the B<bio> argument,
 whereby it may make use of a custom defined argument B<arg>,
 which may for instance refer to an I<SSL_CTX> structure.
-During connection establishment, just after calling BIO_connect_retry(),
+During connection establishment, just after calling BIO_do_connect_retry(),
 the function is invoked with the B<connect> argument being 1 and the B<detail>
 argument being 1 if HTTPS is requested, i.e., SSL/TLS should be enabled.
 On disconnect B<connect> is 0 and B<detail> is 1 if no error occurred, else 0.
diff --git a/include/openssl/bio.h b/include/openssl/bio.h
index 19f9311c68..88e57e70a8 100644
--- a/include/openssl/bio.h
+++ b/include/openssl/bio.h
@@ -664,8 +664,8 @@ int BIO_sock_should_retry(int i);
 int BIO_sock_non_fatal_error(int error);
 int BIO_socket_wait(int fd, int for_read, time_t max_time);
 # endif
-int BIO_wait(BIO *bio, time_t max_time, unsigned int milliseconds);
-int BIO_connect_retry(BIO *bio, int timeout);
+int BIO_wait(BIO *bio, time_t max_time, unsigned int nap_milliseconds);
+int BIO_do_connect_retry(BIO *bio, int timeout, int nap_milliseconds);
 
 int BIO_fd_should_retry(int i);
 int BIO_fd_non_fatal_error(int error);
diff --git a/util/libcrypto.num b/util/libcrypto.num
index 317481388c..230126ff55 100644
--- a/util/libcrypto.num
+++ b/util/libcrypto.num
@@ -4926,7 +4926,7 @@ RAND_DRBG_set_callback_data             ?	3_0_0	EXIST::FUNCTION:
 RAND_DRBG_get_callback_data             ?	3_0_0	EXIST::FUNCTION:
 BIO_socket_wait                         ?	3_0_0	EXIST::FUNCTION:SOCK
 BIO_wait                                ?	3_0_0	EXIST::FUNCTION:
-BIO_connect_retry                       ?	3_0_0	EXIST::FUNCTION:
+BIO_do_connect_retry                    ?	3_0_0	EXIST::FUNCTION:
 ERR_load_HTTP_strings                   ?	3_0_0	EXIST::FUNCTION:
 OSSL_HTTP_get                           ?	3_0_0	EXIST::FUNCTION:
 OSSL_HTTP_get_asn1                      ?	3_0_0	EXIST::FUNCTION:


More information about the openssl-commits mailing list