The branch OpenSSL_1_1_0-stable has been updated via 3f99bfed678b09110fda82bc6896fd45eb0b376c (commit) via 0f6c9d73cb1e1027c67d993a669719e351c25cfc (commit) from a95a0219a887611ad8e246e33c086255df771072 (commit)
- Log ----------------------------------------------------------------- commit 3f99bfed678b09110fda82bc6896fd45eb0b376c Author: Matt Caswell <m...@openssl.org> Date: Wed Nov 2 10:44:15 2016 +0000 Add a read_ahead test This test checks that read_ahead works correctly when dealing with large records. Reviewed-by: Richard Levitte <levi...@openssl.org> (cherry picked from commit 7856332e8c14fd1da1811a9d0afde243dd0f4669) commit 0f6c9d73cb1e1027c67d993a669719e351c25cfc Author: Matt Caswell <m...@openssl.org> Date: Wed Nov 2 10:34:12 2016 +0000 Fix read_ahead The function ssl3_read_n() takes a parameter |clearold| which, if set, causes any old data in the read buffer to be forgotten, and any unread data to be moved to the start of the buffer. This is supposed to happen when we first read the record header. However, the data move was only taking place if there was not already sufficient data in the buffer to satisfy the request. If read_ahead is set then the record header could be in the buffer already from when we read the preceding record. So with read_ahead we can get into a situation where even though |clearold| is set, the data does not get moved to the start of the read buffer when we read the record header. This means there is insufficient room in the read buffer to consume the rest of the record body, resulting in an internal error. This commit moves the |clearold| processing to earlier in ssl3_read_n() to ensure that it always takes place. Reviewed-by: Richard Levitte <levi...@openssl.org> (cherry picked from commit a7faa6da317887e14e8e28254a83555983ed6ca7) ----------------------------------------------------------------------- Summary of changes: ssl/record/rec_layer_s3.c | 24 ++++++++++++------------ test/sslapitest.c | 26 +++++++++++++++++++++++--- 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c index 9c8c23c..4535f89 100644 --- a/ssl/record/rec_layer_s3.c +++ b/ssl/record/rec_layer_s3.c @@ -241,6 +241,18 @@ int ssl3_read_n(SSL *s, int n, int max, int extend, int clearold) /* ... now we can act as if 'extend' was set */ } + len = s->rlayer.packet_length; + pkt = rb->buf + align; + /* + * Move any available bytes to front of buffer: 'len' bytes already + * pointed to by 'packet', 'left' extra ones at the end + */ + if (s->rlayer.packet != pkt && clearold == 1) { + memmove(pkt, s->rlayer.packet, len + left); + s->rlayer.packet = pkt; + rb->offset = len + align; + } + /* * For DTLS/UDP reads should not span multiple packets because the read * operation returns the whole packet at once (as long as it fits into @@ -263,18 +275,6 @@ int ssl3_read_n(SSL *s, int n, int max, int extend, int clearold) /* else we need to read more data */ - len = s->rlayer.packet_length; - pkt = rb->buf + align; - /* - * Move any available bytes to front of buffer: 'len' bytes already - * pointed to by 'packet', 'left' extra ones at the end - */ - if (s->rlayer.packet != pkt && clearold == 1) { /* len > 0 */ - memmove(pkt, s->rlayer.packet, len + left); - s->rlayer.packet = pkt; - rb->offset = len + align; - } - if (n > (int)(rb->len - rb->offset)) { /* does not happen */ SSLerr(SSL_F_SSL3_READ_N, ERR_R_INTERNAL_ERROR); return -1; diff --git a/test/sslapitest.c b/test/sslapitest.c index 495bf26..90326d9 100644 --- a/test/sslapitest.c +++ b/test/sslapitest.c @@ -33,7 +33,7 @@ static X509 *ocspcert = NULL; #define NUM_EXTRA_CERTS 40 static int execute_test_large_message(const SSL_METHOD *smeth, - const SSL_METHOD *cmeth) + const SSL_METHOD *cmeth, int read_ahead) { SSL_CTX *cctx = NULL, *sctx = NULL; SSL *clientssl = NULL, *serverssl = NULL; @@ -61,6 +61,14 @@ static int execute_test_large_message(const SSL_METHOD *smeth, goto end; } + if(read_ahead) { + /* + * Test that read_ahead works correctly when dealing with large + * records + */ + SSL_CTX_set_read_ahead(cctx, 1); + } + /* * We assume the supplied certificate is big enough so that if we add * NUM_EXTRA_CERTS it will make the overall message large enough. The @@ -107,14 +115,25 @@ static int execute_test_large_message(const SSL_METHOD *smeth, static int test_large_message_tls(void) { - return execute_test_large_message(TLS_server_method(), TLS_client_method()); + return execute_test_large_message(TLS_server_method(), TLS_client_method(), + 0); +} + +static int test_large_message_tls_read_ahead(void) +{ + return execute_test_large_message(TLS_server_method(), TLS_client_method(), + 1); } #ifndef OPENSSL_NO_DTLS static int test_large_message_dtls(void) { + /* + * read_ahead is not relevant to DTLS because DTLS always acts as if + * read_ahead is set. + */ return execute_test_large_message(DTLS_server_method(), - DTLS_client_method()); + DTLS_client_method(), 0); } #endif @@ -867,6 +886,7 @@ int main(int argc, char *argv[]) CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ON); ADD_TEST(test_large_message_tls); + ADD_TEST(test_large_message_tls_read_ahead); #ifndef OPENSSL_NO_DTLS ADD_TEST(test_large_message_dtls); #endif _____ openssl-commits mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-commits