Commit 11f1fd4b authored by Kurt Roeckx's avatar Kurt Roeckx
Browse files

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



Backport of beacb0f0, revert of
122580ef

Fixes: #1903

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

GH: #1966
parent c947fa49
Loading
Loading
Loading
Loading
+10 −12
Original line number Original line Diff line number Diff line
@@ -38,12 +38,13 @@ if and only if B<ret E<gt> 0>.


=item SSL_ERROR_ZERO_RETURN
=item SSL_ERROR_ZERO_RETURN


The TLS/SSL connection has been closed.  If the protocol version is SSL 3.0
The TLS/SSL connection has been closed.
or TLS 1.0, this result code is returned only if a closure
If the protocol version is SSL 3.0 or higher, this result code is returned only
alert has occurred in the protocol, i.e. if the connection has been
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>
closed cleanly.
does not necessarily indicate that the underlying transport
Note that in this case B<SSL_ERROR_ZERO_RETURN> does not necessarily
has been closed.
indicate that the underlying transport has been closed.



=item SSL_ERROR_WANT_READ, SSL_ERROR_WANT_WRITE
=item SSL_ERROR_WANT_READ, SSL_ERROR_WANT_WRITE


@@ -111,12 +112,9 @@ thread has completed.


=item SSL_ERROR_SYSCALL
=item SSL_ERROR_SYSCALL


Some I/O error occurred.  The OpenSSL error queue may contain more
Some non-recoverable I/O error occurred.
information on the error.  If the error queue is empty
The OpenSSL error queue may contain more information on the error.
(i.e. ERR_get_error() returns 0), B<ret> can be used to find out more
For socket I/O on Unix systems, consult B<errno> for details.
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).


=item SSL_ERROR_SSL
=item SSL_ERROR_SSL


+11 −18
Original line number Original line Diff line number Diff line
@@ -83,26 +83,19 @@ The following return values can occur:


=item E<gt> 0
=item E<gt> 0


The read operation was successful; the return value is the number of
The read operation was successful.
bytes actually read from the TLS/SSL connection.
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 read operation was not successful, because either the connection was closed,
the SSL_RECEIVED_SHUTDOWN flag in the ssl shutdown state is set
an error occurred or action must be taken by the calling process.
(see L<SSL_shutdown(3)>,
Call L<SSL_get_error(3)> with the return value B<ret> to find out the reason.
L<SSL_set_shutdown(3)>). It is also possible, that

the peer simply shut down the underlying transport and the shutdown is
Old documentation indicated a difference between 0 and -1, and that -1 was
incomplete. Call SSL_get_error() with the return value B<ret> to find out,
retryable.
whether an error occurred or the connection was shut down cleanly
You should instead call SSL_get_error() to find out if it's retryable.
(SSL_ERROR_ZERO_RETURN).

=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.


=back
=back


+8 −11
Original line number Original line Diff line number Diff line
@@ -79,18 +79,15 @@ The following return values can occur:
The write operation was successful, the return value is the number of
The write operation was successful, the return value is the number of
bytes actually written to the TLS/SSL connection.
bytes actually written to the TLS/SSL connection.


=item Z<>0
=item Z<><= 0


The write operation was not successful. Probably the underlying connection
The write operation was not successful, because either the connection was
was closed. Call SSL_get_error() with the return value B<ret> to find out,
closed, an error occurred or action must be taken by the calling process.
whether an error occurred or the connection was shut down cleanly
Call SSL_get_error() with the return value B<ret> to find out the reason.
(SSL_ERROR_ZERO_RETURN).


=item E<lt>0
Old documentation indicated a difference between 0 and -1, and that -1 was

retryable.
The write operation was not successful, because either an error occurred
You should instead call SSL_get_error() to find out if it's retryable.
or action must be taken by the calling process. Call SSL_get_error() with the
return value B<ret> to find out the reason.


=back
=back


+4 −10
Original line number Original line Diff line number Diff line
@@ -178,10 +178,7 @@ const char *SSL_rstate_string(const SSL *s)
}
}


/*
/*
 * Return values are as per SSL_read(), i.e.
 * Return values are as per SSL_read()
 * >0 The number of read bytes
 *  0 Failure (not retryable)
 * <0 Failure (may be retryable)
 */
 */
int ssl3_read_n(SSL *s, int n, int max, int extend, int clearold)
int ssl3_read_n(SSL *s, int n, int max, int extend, int clearold)
{
{
@@ -312,7 +309,7 @@ int ssl3_read_n(SSL *s, int n, int max, int extend, int clearold)
            if (s->mode & SSL_MODE_RELEASE_BUFFERS && !SSL_IS_DTLS(s))
            if (s->mode & SSL_MODE_RELEASE_BUFFERS && !SSL_IS_DTLS(s))
                if (len + left == 0)
                if (len + left == 0)
                    ssl3_release_read_buffer(s);
                    ssl3_release_read_buffer(s);
            return -1;
            return i;
        }
        }
        left += i;
        left += i;
        /*
        /*
@@ -882,10 +879,7 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf,


/* if s->s3->wbuf.left != 0, we need to call this
/* if s->s3->wbuf.left != 0, we need to call this
 *
 *
 * Return values are as per SSL_read(), i.e.
 * Return values are as per SSL_write()
 * >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,
int ssl3_write_pending(SSL *s, int type, const unsigned char *buf,
                       unsigned int len)
                       unsigned int len)
@@ -936,7 +930,7 @@ int ssl3_write_pending(SSL *s, int type, const unsigned char *buf,
                 */
                 */
                SSL3_BUFFER_set_left(&wb[currbuf], 0);
                SSL3_BUFFER_set_left(&wb[currbuf], 0);
            }
            }
            return -1;
            return i;
        }
        }
        SSL3_BUFFER_add_offset(&wb[currbuf], i);
        SSL3_BUFFER_add_offset(&wb[currbuf], i);
        SSL3_BUFFER_add_left(&wb[currbuf], -i);
        SSL3_BUFFER_add_left(&wb[currbuf], -i);
+42 −15
Original line number Original line Diff line number Diff line
@@ -297,32 +297,59 @@ int main(int argc, char *argv[])
         * we hit at least one async event in both reading and writing
         * we hit at least one async event in both reading and writing
         */
         */
        for (j = 0; j < 2; j++) {
        for (j = 0; j < 2; j++) {
            int len;

            /*
            /*
             * Write some test data. It should never take more than 2 attempts
             * Write some test data. It should never take more than 2 attempts
             * (the first one might be a retryable fail). A zero return from
             * (the first one might be a retryable fail).
             * SSL_write() is a non-retryable failure, so fail immediately if
             * we get that.
             */
             */
            for (ret = -1, i = 0; ret < 0 && i < 2 * sizeof(testdata); i++)
            for (ret = -1, i = 0, len = 0; len != sizeof(testdata) && i < 2;
                ret = SSL_write(clientssl, testdata, sizeof(testdata));
                i++) {
            if (ret <= 0) {
                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);
                        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;
                goto end;
            }
            }
            /*
            /*
             * Now read the test data. It may take more attemps here because
             * Now read the test data. It may take more attemps here because
             * it could fail once for each byte read, including all overhead
             * it could fail once for each byte read, including all overhead
             * bytes from the record header/padding etc. Fail immediately if we
             * bytes from the record header/padding etc.
             * get a zero return from SSL_read().
             */
             */
            for (ret = -1, i = 0; ret < 0 && i < MAX_ATTEMPTS; i++)
            for (ret = -1, i = 0, len = 0; len != sizeof(testdata) &&
                ret = SSL_read(serverssl, buf, sizeof(buf));
                i < MAX_ATTEMPTS; i++)
            if (ret <= 0) {
            {
                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);
                        printf("Test %d failed: Failed to read app data\n", test);
                        err = -1;
                        goto end;
                        goto end;
                    }
                    }
            if (ret != sizeof(testdata)
                }
            }
            if (len != sizeof(testdata)
                    || memcmp(buf, testdata, sizeof(testdata)) != 0) {
                    || memcmp(buf, testdata, sizeof(testdata)) != 0) {
                err = -1;
                printf("Test %d failed: Unexpected app data received\n", test);
                printf("Test %d failed: Unexpected app data received\n", test);
                goto end;
                goto end;
            }
            }