The branch OpenSSL_1_1_0-stable has been updated via dafa1c85b9bbd8ed3ff1911d00ad7f4e890bafa3 (commit) via 122580ef71e4e5f355a1a104c9bfb36feee43759 (commit) from 207a9cb3522882d1e9dc764c921425ba47a6def6 (commit)
- Log ----------------------------------------------------------------- commit dafa1c85b9bbd8ed3ff1911d00ad7f4e890bafa3 Author: Matt Caswell <m...@openssl.org> Date: Thu Oct 27 13:46:57 2016 +0100 Add a test for BIO_read() returning 0 in SSL_read() (and also for write) A BIO_read() 0 return indicates that a failure occurred that may be retryable. An SSL_read() 0 return indicates a non-retryable failure. Check that if BIO_read() returns 0, SSL_read() returns <0. Same for SSL_write(). The asyncio test filter BIO already returns 0 on a retryable failure so we build on that. Reviewed-by: Richard Levitte <levi...@openssl.org> (cherry picked from commit a34ac5b8b9c1a3281b4ee545c46177f485fb4949) commit 122580ef71e4e5f355a1a104c9bfb36feee43759 Author: Matt Caswell <m...@openssl.org> Date: Fri Oct 21 13:25:19 2016 +0100 A zero return from BIO_read()/BIO_write() could be retryable A zero return from BIO_read()/BIO_write() could mean that an IO operation is retryable. A zero return from SSL_read()/SSL_write() means that the connection has been closed down (either cleanly or not). Therefore we should not propagate a zero return value from BIO_read()/BIO_write() back up the stack to SSL_read()/SSL_write(). This could result in a retryable failure being treated as fatal. Reviewed-by: Richard Levitte <levi...@openssl.org> (cherry picked from commit 4880672a9b41a09a0984b55e219f02a2de7ab75e) ----------------------------------------------------------------------- Summary of changes: ssl/record/rec_layer_s3.c | 18 +++++++++++++++--- test/asynciotest.c | 43 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c index 0775095..9c8c23c 100644 --- a/ssl/record/rec_layer_s3.c +++ b/ssl/record/rec_layer_s3.c @@ -177,6 +177,12 @@ const char *SSL_rstate_string(const 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) + */ int ssl3_read_n(SSL *s, int n, int max, int extend, int clearold) { /* @@ -306,7 +312,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 (len + left == 0) ssl3_release_read_buffer(s); - return (i); + return -1; } left += i; /* @@ -874,7 +880,13 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf, return -1; } -/* 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. + * >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) { @@ -924,7 +936,7 @@ int ssl3_write_pending(SSL *s, int type, const unsigned char *buf, */ SSL3_BUFFER_set_left(&wb[currbuf], 0); } - return (i); + return -1; } SSL3_BUFFER_add_offset(&wb[currbuf], i); SSL3_BUFFER_add_left(&wb[currbuf], -i); diff --git a/test/asynciotest.c b/test/asynciotest.c index 720cc7c..23d0907 100644 --- a/test/asynciotest.c +++ b/test/asynciotest.c @@ -234,12 +234,17 @@ static int async_puts(BIO *bio, const char *str) return async_write(bio, str, strlen(str)); } +#define MAX_ATTEMPTS 100 + int main(int argc, char *argv[]) { SSL_CTX *serverctx = NULL, *clientctx = NULL; SSL *serverssl = NULL, *clientssl = NULL; BIO *s_to_c_fbio = NULL, *c_to_s_fbio = NULL; - int test, err = 1; + int test, err = 1, ret; + size_t i, j; + const char testdata[] = "Test data"; + char buf[sizeof(testdata)]; CRYPTO_set_mem_debug(1); CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ON); @@ -287,6 +292,42 @@ int main(int argc, char *argv[]) goto end; } + /* + * Send and receive some test data. Do the whole thing twice to ensure + * we hit at least one async event in both reading and writing + */ + for (j = 0; j < 2; j++) { + /* + * 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. + */ + 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); + 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(). + */ + 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; + } + if (ret != sizeof(testdata) + || memcmp(buf, testdata, sizeof(testdata)) != 0) { + printf("Test %d failed: Unexpected app data received\n", test); + goto end; + } + } + /* Also frees the BIOs */ SSL_free(clientssl); SSL_free(serverssl); _____ openssl-commits mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-commits