Convert SSL BIO to use SSL_write_ex().
authorMatt Caswell <matt@openssl.org>
Wed, 19 Oct 2016 13:09:02 +0000 (14:09 +0100)
committerMatt Caswell <matt@openssl.org>
Fri, 4 Nov 2016 12:09:45 +0000 (12:09 +0000)
We also modify the SSL_get_error() function to handle the fact that with
SSL_write_ex() the error return is 0 not -1, and fix some bugs in the
SSL BIO reading.

Reviewed-by: Rich Salz <rsalz@openssl.org>
ssl/bio_ssl.c
ssl/ssl_lib.c

index d64451c7045a8aca294696d53d25f12438b8829e..e2769e1d6a02e64376730bd0bd53b6744dcd3163 100644 (file)
@@ -28,7 +28,7 @@ typedef struct bio_ssl_st {
     /* re-negotiate every time the total number of bytes is this size */
     int num_renegotiates;
     unsigned long renegotiate_count;
-    unsigned long byte_count;
+    size_t byte_count;
     unsigned long renegotiate_timeout;
     unsigned long last_time;
 } BIO_SSL;
@@ -112,7 +112,7 @@ static int ssl_read(BIO *b, char *buf, size_t size, size_t *readbytes)
 
     switch (SSL_get_error(ssl, ret)) {
     case SSL_ERROR_NONE:
-        if (ret <= 0)
+        if (*readbytes == 0)
             break;
         if (sb->renegotiate_count > 0) {
             sb->byte_count += *readbytes;
@@ -179,17 +179,14 @@ static int ssl_write(BIO *b, const char *buf, size_t size, size_t *written)
 
     BIO_clear_retry_flags(b);
 
-    if (size > INT_MAX)
-        size = INT_MAX;
-
-    ret = SSL_write(ssl, buf, size);
+    ret = SSL_write_ex(ssl, buf, size, written);
 
     switch (SSL_get_error(ssl, ret)) {
     case SSL_ERROR_NONE:
-        if (ret <= 0)
+        if (*written == 0)
             break;
         if (bs->renegotiate_count > 0) {
-            bs->byte_count += ret;
+            bs->byte_count += *written;
             if (bs->byte_count > bs->renegotiate_count) {
                 bs->byte_count = 0;
                 bs->num_renegotiates++;
@@ -229,11 +226,6 @@ static int ssl_write(BIO *b, const char *buf, size_t size, size_t *written)
 
     BIO_set_retry_reason(b, retry_reason);
 
-    if (ret > 0) {
-        *written = ret;
-        ret = 1;
-    }
-
     return ret;
 }
 
index 3c0cb764bfa29498e9bd3dbd3ff78c385adfae59..a869314bdae141901e1cc84d9ad5f91b0d9fa897 100644 (file)
@@ -3002,72 +3002,69 @@ int SSL_get_error(const SSL *s, int i)
             return (SSL_ERROR_SSL);
     }
 
-    if (i < 0) {
-        if (SSL_want_read(s)) {
-            bio = SSL_get_rbio(s);
-            if (BIO_should_read(bio))
-                return (SSL_ERROR_WANT_READ);
-            else if (BIO_should_write(bio))
-                /*
-                 * This one doesn't make too much sense ... We never try to write
-                 * to the rbio, and an application program where rbio and wbio
-                 * are separate couldn't even know what it should wait for.
-                 * However if we ever set s->rwstate incorrectly (so that we have
-                 * SSL_want_read(s) instead of SSL_want_write(s)) and rbio and
-                 * wbio *are* the same, this test works around that bug; so it
-                 * might be safer to keep it.
-                 */
-                return (SSL_ERROR_WANT_WRITE);
-            else if (BIO_should_io_special(bio)) {
-                reason = BIO_get_retry_reason(bio);
-                if (reason == BIO_RR_CONNECT)
-                    return (SSL_ERROR_WANT_CONNECT);
-                else if (reason == BIO_RR_ACCEPT)
-                    return (SSL_ERROR_WANT_ACCEPT);
-                else
-                    return (SSL_ERROR_SYSCALL); /* unknown */
-            }
+    if (SSL_want_read(s)) {
+        bio = SSL_get_rbio(s);
+        if (BIO_should_read(bio))
+            return (SSL_ERROR_WANT_READ);
+        else if (BIO_should_write(bio))
+            /*
+             * This one doesn't make too much sense ... We never try to write
+             * to the rbio, and an application program where rbio and wbio
+             * are separate couldn't even know what it should wait for.
+             * However if we ever set s->rwstate incorrectly (so that we have
+             * SSL_want_read(s) instead of SSL_want_write(s)) and rbio and
+             * wbio *are* the same, this test works around that bug; so it
+             * might be safer to keep it.
+             */
+            return (SSL_ERROR_WANT_WRITE);
+        else if (BIO_should_io_special(bio)) {
+            reason = BIO_get_retry_reason(bio);
+            if (reason == BIO_RR_CONNECT)
+                return (SSL_ERROR_WANT_CONNECT);
+            else if (reason == BIO_RR_ACCEPT)
+                return (SSL_ERROR_WANT_ACCEPT);
+            else
+                return (SSL_ERROR_SYSCALL); /* unknown */
         }
+    }
 
-        if (SSL_want_write(s)) {
+    if (SSL_want_write(s)) {
+        /*
+         * Access wbio directly - in order to use the buffered bio if
+         * present
+         */
+        bio = s->wbio;
+        if (BIO_should_write(bio))
+            return (SSL_ERROR_WANT_WRITE);
+        else if (BIO_should_read(bio))
             /*
-             * Access wbio directly - in order to use the buffered bio if
-             * present
+             * See above (SSL_want_read(s) with BIO_should_write(bio))
              */
-            bio = s->wbio;
-            if (BIO_should_write(bio))
-                return (SSL_ERROR_WANT_WRITE);
-            else if (BIO_should_read(bio))
-                /*
-                 * See above (SSL_want_read(s) with BIO_should_write(bio))
-                 */
-                return (SSL_ERROR_WANT_READ);
-            else if (BIO_should_io_special(bio)) {
-                reason = BIO_get_retry_reason(bio);
-                if (reason == BIO_RR_CONNECT)
-                    return (SSL_ERROR_WANT_CONNECT);
-                else if (reason == BIO_RR_ACCEPT)
-                    return (SSL_ERROR_WANT_ACCEPT);
-                else
-                    return (SSL_ERROR_SYSCALL);
-            }
-        }
-        if (SSL_want_x509_lookup(s)) {
-            return (SSL_ERROR_WANT_X509_LOOKUP);
-        }
-        if (SSL_want_async(s)) {
-            return SSL_ERROR_WANT_ASYNC;
-        }
-        if (SSL_want_async_job(s)) {
-            return SSL_ERROR_WANT_ASYNC_JOB;
+            return (SSL_ERROR_WANT_READ);
+        else if (BIO_should_io_special(bio)) {
+            reason = BIO_get_retry_reason(bio);
+            if (reason == BIO_RR_CONNECT)
+                return (SSL_ERROR_WANT_CONNECT);
+            else if (reason == BIO_RR_ACCEPT)
+                return (SSL_ERROR_WANT_ACCEPT);
+            else
+                return (SSL_ERROR_SYSCALL);
         }
     }
-
-    if (i == 0) {
-        if ((s->shutdown & SSL_RECEIVED_SHUTDOWN) &&
-            (s->s3->warn_alert == SSL_AD_CLOSE_NOTIFY))
-            return (SSL_ERROR_ZERO_RETURN);
+    if (SSL_want_x509_lookup(s)) {
+        return (SSL_ERROR_WANT_X509_LOOKUP);
     }
+    if (SSL_want_async(s)) {
+        return SSL_ERROR_WANT_ASYNC;
+    }
+    if (SSL_want_async_job(s)) {
+        return SSL_ERROR_WANT_ASYNC_JOB;
+    }
+
+    if ((s->shutdown & SSL_RECEIVED_SHUTDOWN) &&
+        (s->s3->warn_alert == SSL_AD_CLOSE_NOTIFY))
+        return (SSL_ERROR_ZERO_RETURN);
+
     return (SSL_ERROR_SYSCALL);
 }