Make SSL_read and SSL_write return the old behaviour and document it.

Backport of beacb0f0c1ae7b0542fe053b95307f515b578eb7, revert of
fa4c374572e94f467900f5820cd1d00af2470a17

Fixes: #1903

Reviewed-by: Matt Caswell <matt@openssl.org>

GH: #1967

6 files changed, 45 insertions, 72 deletions

diff --git a/doc/ssl/SSL_get_error.pod b/doc/ssl/SSL_get_error.pod
index 48c6b15db7..2a93894096 100644
--- a/doc/ssl/SSL_get_error.pod
+++ b/doc/ssl/SSL_get_error.pod
@@ -38,12 +38,13 @@ if and only if B<ret E<gt> 0>.
 
 =item SSL_ERROR_ZERO_RETURN
 
-The TLS/SSL connection has been closed.  If the protocol version is SSL 3.0
-or TLS 1.0, this result code is returned only if a closure
-alert has occurred in the protocol, i.e. if the connection has been
-closed cleanly. Note that in this case B<SSL_ERROR_ZERO_RETURN>
-does not necessarily indicate that the underlying transport
-has been closed.
+The TLS/SSL connection has been closed.
+If the protocol version is SSL 3.0 or higher, this result code is returned only
+if a closure alert has occurred in the protocol, i.e. if the connection has been
+closed cleanly.
+Note that in this case B<SSL_ERROR_ZERO_RETURN> does not necessarily
+indicate that the underlying transport has been closed.
+
 
 =item SSL_ERROR_WANT_READ, SSL_ERROR_WANT_WRITE
 
@@ -89,12 +90,9 @@ Details depend on the application.
 
 =item SSL_ERROR_SYSCALL
 
-Some I/O error occurred.  The OpenSSL error queue may contain more
-information on the error.  If the error queue is empty
-(i.e. ERR_get_error() returns 0), B<ret> can be used to find out more
-about the error: If B<ret == 0>, an EOF was observed that violates
-the protocol.  If B<ret == -1>, the underlying B<BIO> reported an
-I/O error (for socket I/O on Unix systems, consult B<errno> for details).
+Some non-recoverable I/O error occurred.
+The OpenSSL error queue may contain more information on the error.
+For socket I/O on Unix systems, consult B<errno> for details.
 
 =item SSL_ERROR_SSL
 
diff --git a/doc/ssl/SSL_read.pod b/doc/ssl/SSL_read.pod
index 8ca0ce5058..ef983c9d3f 100644
--- a/doc/ssl/SSL_read.pod
+++ b/doc/ssl/SSL_read.pod
@@ -81,33 +81,29 @@ The following return values can occur:
 
 =over 4
 
-=item E<gt>0
+=item E<gt> 0
 
-The read operation was successful; the return value is the number of
-bytes actually read from the TLS/SSL connection.
+The read operation was successful.
+The return value is the number of bytes actually read from the TLS/SSL
+connection.
 
-=item Z<>0
+=item Z<><= 0
 
-The read operation was not successful. The reason may either be a clean
-shutdown due to a "close notify" alert sent by the peer (in which case
-the SSL_RECEIVED_SHUTDOWN flag in the ssl shutdown state is set
-(see L<SSL_shutdown(3)|SSL_shutdown(3)>,
-L<SSL_set_shutdown(3)|SSL_set_shutdown(3)>). It is also possible, that
-the peer simply shut down the underlying transport and the shutdown is
-incomplete. Call SSL_get_error() with the return value B<ret> to find out,
-whether an error occurred or the connection was shut down cleanly
-(SSL_ERROR_ZERO_RETURN).
+
+=item E<lt>0
+
+The read operation was not successful, because either the connection was closed,
+an error occurred or action must be taken by the calling process.
+Call L<SSL_get_error(3)> with the return value B<ret> to find out the reason.
 
 SSLv2 (deprecated) does not support a shutdown alert protocol, so it can
 only be detected, whether the underlying connection was closed. It cannot
 be checked, whether the closure was initiated by the peer or by something
 else.
 
-=item E<lt>0
-
-The read operation was not successful, because either an error occurred
-or action must be taken by the calling process. Call SSL_get_error() with the
-return value B<ret> to find out the reason.
+Old documentation indicated a difference between 0 and -1, and that -1 was
+retryable.
+You should instead call SSL_get_error() to find out if it's retryable.
 
 =back
 
diff --git a/doc/ssl/SSL_write.pod b/doc/ssl/SSL_write.pod
index a57617f3ee..4c1a7ee71f 100644
--- a/doc/ssl/SSL_write.pod
+++ b/doc/ssl/SSL_write.pod
@@ -74,27 +74,24 @@ The following return values can occur:
 
 =over 4
 
-=item E<gt>0
+=item E<gt> 0
 
 The write operation was successful, the return value is the number of
 bytes actually written to the TLS/SSL connection.
 
-=item Z<>0
+=item Z<><= 0
 
-The write operation was not successful. Probably the underlying connection
-was closed. Call SSL_get_error() with the return value B<ret> to find out,
-whether an error occurred or the connection was shut down cleanly
-(SSL_ERROR_ZERO_RETURN).
+The write operation was not successful, because either the connection was
+closed, an error occurred or action must be taken by the calling process.
+Call SSL_get_error() with the return value B<ret> to find out the reason.
 
 SSLv2 (deprecated) does not support a shutdown alert protocol, so it can
 only be detected, whether the underlying connection was closed. It cannot
 be checked, why the closure happened.
 
-=item E<lt>0
-
-The write operation was not successful, because either an error occurred
-or action must be taken by the calling process. Call SSL_get_error() with the
-return value B<ret> to find out the reason.
+Old documentation indicated a difference between 0 and -1, and that -1 was
+retryable.
+You should instead call SSL_get_error() to find out if it's retryable.
 
 =back
 
diff --git a/ssl/s23_pkt.c b/ssl/s23_pkt.c
index 5a63eff740..6544180efe 100644
--- a/ssl/s23_pkt.c
+++ b/ssl/s23_pkt.c
@@ -64,10 +64,7 @@
 #include <openssl/buffer.h>
 
 /*
- * Return values are as per SSL_write(), i.e.
- * >0 The number of read bytes
- *  0 Failure (not retryable)
- * <0 Failure (may be retryable)
+ * Return values are as per SSL_write()
  */
 int ssl23_write_bytes(SSL *s)
 {
@@ -83,7 +80,7 @@ int ssl23_write_bytes(SSL *s)
         if (i <= 0) {
             s->init_off = tot;
             s->init_num = num;
-            return -1;
+            return i;
         }
         s->rwstate = SSL_NOTHING;
         if (i == num)
@@ -95,11 +92,8 @@ int ssl23_write_bytes(SSL *s)
 }
 
 /* return regularly only when we have read (at least) 'n' bytes
- * 
- * Return values are as per SSL_read(), i.e.
- * >0 The number of read bytes
- *  0 Failure (not retryable)
- * <0 Failure (may be retryable)
+ *
+ * Return values are as per SSL_read()
  */
 int ssl23_read_bytes(SSL *s, int n)
 {
@@ -114,7 +108,7 @@ int ssl23_read_bytes(SSL *s, int n)
             j = BIO_read(s->rbio, (char *)&(p[s->packet_length]),
                          n - s->packet_length);
             if (j <= 0)
-                return -1;
+                return j;
             s->rwstate = SSL_NOTHING;
             s->packet_length += j;
             if (s->packet_length >= (unsigned int)n)
diff --git a/ssl/s2_pkt.c b/ssl/s2_pkt.c
index 394b433479..e44bc0335a 100644
--- a/ssl/s2_pkt.c
+++ b/ssl/s2_pkt.c
@@ -308,10 +308,7 @@ int ssl2_peek(SSL *s, void *buf, int len)
 }
 
 /*
- * Return values are as per SSL_read(), i.e.
- * >0 The number of read bytes
- *  0 Failure (not retryable)
- * <0 Failure (may be retryable)
+ * Return values are as per SSL_read()
  */
 static int read_n(SSL *s, unsigned int n, unsigned int max,
                   unsigned int extend)
@@ -380,7 +377,7 @@ static int read_n(SSL *s, unsigned int n, unsigned int max,
 # endif
         if (i <= 0) {
             s->s2->rbuf_left += newb;
-            return -1;
+            return i;
         }
         newb += i;
     }
@@ -448,10 +445,7 @@ int ssl2_write(SSL *s, const void *_buf, int len)
 }
 
 /*
- * Return values are as per SSL_write(), i.e.
- * >0 The number of read bytes
- *  0 Failure (not retryable)
- * <0 Failure (may be retryable)
+ * Return values are as per SSL_write()
  */
 static int write_pending(SSL *s, const unsigned char *buf, unsigned int len)
 {
@@ -489,7 +483,7 @@ static int write_pending(SSL *s, const unsigned char *buf, unsigned int len)
             s->rwstate = SSL_NOTHING;
             return (s->s2->wpend_ret);
         } else if (i <= 0)
-            return -1;
+            return i;
         s->s2->wpend_off += i;
         s->s2->wpend_len -= i;
     }
diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c
index cb74d467bb..6ece87d062 100644
--- a/ssl/s3_pkt.c
+++ b/ssl/s3_pkt.c
@@ -137,10 +137,7 @@ static int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
 static int ssl3_get_record(SSL *s);
 
 /*
- * Return values are as per SSL_read(), i.e.
- * >0 The number of read bytes
- *  0 Failure (not retryable)
- * <0 Failure (may be retryable)
+ * Return values are as per SSL_read()
  */
 int ssl3_read_n(SSL *s, int n, int max, int extend)
 {
@@ -269,7 +266,7 @@ int ssl3_read_n(SSL *s, int n, int max, int extend)
             if (s->mode & SSL_MODE_RELEASE_BUFFERS && !SSL_IS_DTLS(s))
                 if (len + left == 0)
                     ssl3_release_read_buffer(s);
-            return -1;
+            return (i);
         }
         left += i;
         /*
@@ -1089,11 +1086,8 @@ static int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
 }
 
 /* if s->s3->wbuf.left != 0, we need to call this
- * 
+ *
  * Return values are as per SSL_write(), i.e.
- * >0 The number of read bytes
- *  0 Failure (not retryable)
- * <0 Failure (may be retryable)
  */
 int ssl3_write_pending(SSL *s, int type, const unsigned char *buf,
                        unsigned int len)
@@ -1134,7 +1128,7 @@ int ssl3_write_pending(SSL *s, int type, const unsigned char *buf,
                  */
                 wb->left = 0;
             }
-            return -1;
+            return i;
         }
         wb->offset += i;
         wb->left -= i;