The branch master has been updated
via beacb0f0c1ae7b0542fe053b95307f515b578eb7 (commit)
from 52fe14e6628781f78ebe5468200e6c895c9bb47c (commit)
- Log -----------------------------------------------------------------
commit beacb0f0c1ae7b0542fe053b95307f515b578eb7
Author: Kurt Roeckx <k...@roeckx.be>
Date: Tue Nov 15 18:58:52 2016 +0100
Make SSL_read and SSL_write return the old behaviour and document it.
This reverts commit 4880672a9b41a09a0984b55e219f02a2de7ab75e.
Fixes: #1903
Reviewed-by: Matt Caswell <m...@openssl.org>
GH: #1931
-----------------------------------------------------------------------
Summary of changes:
doc/man3/SSL_get_error.pod | 21 ++++++++---------
doc/man3/SSL_read.pod | 42 +++++++++++++++-------------------
doc/man3/SSL_write.pod | 19 +++++++---------
include/openssl/ssl.h | 4 ++--
ssl/record/rec_layer_s3.c | 14 ++++--------
test/asynciotest.c | 57 ++++++++++++++++++++++++++++++++++------------
6 files changed, 83 insertions(+), 74 deletions(-)
diff --git a/doc/man3/SSL_get_error.pod b/doc/man3/SSL_get_error.pod
index a1f8bc0..db8f85c 100644
--- a/doc/man3/SSL_get_error.pod
+++ b/doc/man3/SSL_get_error.pod
@@ -38,12 +38,12 @@ 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
@@ -112,12 +112,9 @@ thread has completed.
=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/man3/SSL_read.pod b/doc/man3/SSL_read.pod
index e2490d4..215d4c5 100644
--- a/doc/man3/SSL_read.pod
+++ b/doc/man3/SSL_read.pod
@@ -9,17 +9,17 @@ SSL_read_ex, SSL_read, SSL_peek_ex, SSL_peek
#include <openssl/ssl.h>
- int SSL_read_ex(SSL *ssl, void *buf, size_t num, size_t *read);
+ int SSL_read_ex(SSL *ssl, void *buf, size_t num, size_t *readbytes);
int SSL_read(SSL *ssl, void *buf, int num);
- int SSL_peek_ex(SSL *ssl, void *buf, size_t num, size_t *read);
+ int SSL_peek_ex(SSL *ssl, void *buf, size_t num, size_t *readbytes);
int SSL_peek(SSL *ssl, void *buf, int num);
=head1 DESCRIPTION
SSL_read_ex() and SSL_read() try to read B<num> bytes from the specified B<ssl>
into the buffer B<buf>. On success SSL_read_ex() will store the number of bytes
-actually read in B<*read>.
+actually read in B<*readbytes>.
SSL_peek_ex() and SSL_peek() are identical to SSL_read_ex() and SSL_read()
respectively except no bytes are actually removed from the underlying BIO
during
@@ -90,38 +90,32 @@ with the same arguments.
SSL_read_ex() and SSL_peek_ex() will return 1 for success or 0 for failure.
Success means that 1 or more application data bytes have been read from the SSL
-connection. Failure means that no bytes could be read from the SSL connection.
+connection.
+Failure means that no bytes could be read from the SSL connection.
Failures can be retryable (e.g. we are waiting for more bytes to
-be delivered by the network) or non-retryable (e.g. a fatal network error). In
-the event of a failure call L<SSL_get_error(3)> to find out the reason which
+be delivered by the network) or non-retryable (e.g. a fatal network error).
+In the event of a failure call L<SSL_get_error(3)> to find out the reason which
indicates whether the call is retryable or not.
For SSL_read() and SSL_peek() 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)>,
-L<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).
+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.
-=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/man3/SSL_write.pod b/doc/man3/SSL_write.pod
index 8ed9192..2a75110 100644
--- a/doc/man3/SSL_write.pod
+++ b/doc/man3/SSL_write.pod
@@ -86,23 +86,20 @@ For SSL_write() 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.
-=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/include/openssl/ssl.h b/include/openssl/ssl.h
index f428af7..1f9aaf8 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -1580,9 +1580,9 @@ __owur int SSL_get_changed_async_fds(SSL *s,
OSSL_ASYNC_FD *addfd,
__owur int SSL_accept(SSL *ssl);
__owur int SSL_connect(SSL *ssl);
__owur int SSL_read(SSL *ssl, void *buf, int num);
-__owur int SSL_read_ex(SSL *ssl, void *buf, size_t num, size_t *read);
+__owur int SSL_read_ex(SSL *ssl, void *buf, size_t num, size_t *readbytes);
__owur int SSL_peek(SSL *ssl, void *buf, int num);
-__owur int SSL_peek_ex(SSL *ssl, void *buf, size_t num, size_t *read);
+__owur int SSL_peek_ex(SSL *ssl, void *buf, size_t num, size_t *readbytes);
__owur int SSL_write(SSL *ssl, const void *buf, int num);
__owur int SSL_write_ex(SSL *s, const void *buf, size_t num, size_t *written);
long SSL_ctrl(SSL *ssl, int cmd, long larg, void *parg);
diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index 779a29f..317ee30 100644
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -178,10 +178,7 @@ const char *SSL_rstate_string(const SSL *s)
}
/*
- * Return values are as per SSL_read(), i.e.
- * 1 Success
- * 0 Failure (not retryable)
- * <0 Failure (may be retryable)
+ * Return values are as per SSL_read()
*/
int ssl3_read_n(SSL *s, size_t n, size_t max, int extend, int clearold,
size_t *readbytes)
@@ -319,7 +316,7 @@ int ssl3_read_n(SSL *s, size_t n, size_t max, int extend,
int clearold,
if (s->mode & SSL_MODE_RELEASE_BUFFERS && !SSL_IS_DTLS(s))
if (len + left == 0)
ssl3_release_read_buffer(s);
- return -1;
+ return ret;
}
left += bioread;
/*
@@ -897,10 +894,7 @@ 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_read(), i.e.
- * 1 Success
- * 0 Failure (not retryable)
- * <0 Failure (may be retryable)
+ * Return values are as per SSL_write()
*/
int ssl3_write_pending(SSL *s, int type, const unsigned char *buf, size_t len,
size_t *written)
@@ -955,7 +949,7 @@ int ssl3_write_pending(SSL *s, int type, const unsigned
char *buf, size_t len,
*/
SSL3_BUFFER_set_left(&wb[currbuf], 0);
}
- return -1;
+ return (i);
}
SSL3_BUFFER_add_offset(&wb[currbuf], tmpwrit);
SSL3_BUFFER_sub_left(&wb[currbuf], tmpwrit);
diff --git a/test/asynciotest.c b/test/asynciotest.c
index 23d0907..a4f43f8 100644
--- a/test/asynciotest.c
+++ b/test/asynciotest.c
@@ -297,32 +297,59 @@ int main(int argc, char *argv[])
* we hit at least one async event in both reading and writing
*/
for (j = 0; j < 2; j++) {
+ int len;
+
/*
* Write some test data. It should never take more than 2 attempts
- * (the first one might be a retryable fail). A zero return from
- * SSL_write() is a non-retryable failure, so fail immediately if
- * we get that.
+ * (the first one might be a retryable fail).
*/
- for (ret = -1, i = 0; ret < 0 && i < 2 * sizeof(testdata); i++)
- ret = SSL_write(clientssl, testdata, sizeof(testdata));
- if (ret <= 0) {
- printf("Test %d failed: Failed to write app data\n", test);
+ for (ret = -1, i = 0, len = 0; len != sizeof(testdata) && i < 2;
+ i++) {
+ ret = SSL_write(clientssl, testdata + len,
+ sizeof(testdata) - len);
+ if (ret > 0) {
+ len += ret;
+ } else {
+ int ssl_error = SSL_get_error(clientssl, ret);
+
+ if (ssl_error == SSL_ERROR_SYSCALL ||
+ ssl_error == SSL_ERROR_SSL) {
+ printf("Test %d failed: Failed to write app data\n",
test);
+ err = -1;
+ goto end;
+ }
+ }
+ }
+ if (len != sizeof(testdata)) {
+ err = -1;
+ printf("Test %d failed: Failed to write all app data\n", test);
goto end;
}
/*
* Now read the test data. It may take more attemps here because
* it could fail once for each byte read, including all overhead
- * bytes from the record header/padding etc. Fail immediately if we
- * get a zero return from SSL_read().
+ * bytes from the record header/padding etc.
*/
- for (ret = -1, i = 0; ret < 0 && i < MAX_ATTEMPTS; i++)
- ret = SSL_read(serverssl, buf, sizeof(buf));
- if (ret <= 0) {
- printf("Test %d failed: Failed to read app data\n", test);
- goto end;
+ for (ret = -1, i = 0, len = 0; len != sizeof(testdata) &&
+ i < MAX_ATTEMPTS; i++)
+ {
+ ret = SSL_read(serverssl, buf + len, sizeof(buf) - len);
+ if (ret > 0) {
+ len += ret;
+ } else {
+ int ssl_error = SSL_get_error(serverssl, ret);
+
+ if (ssl_error == SSL_ERROR_SYSCALL ||
+ ssl_error == SSL_ERROR_SSL) {
+ printf("Test %d failed: Failed to read app data\n",
test);
+ err = -1;
+ goto end;
+ }
+ }
}
- if (ret != sizeof(testdata)
+ if (len != sizeof(testdata)
|| memcmp(buf, testdata, sizeof(testdata)) != 0) {
+ err = -1;
printf("Test %d failed: Unexpected app data received\n", test);
goto end;
}
_____
openssl-commits mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-commits
