[openssl-commits] [openssl] OpenSSL_1_1_0-stable update
Matt Caswell
matt at openssl.org
Wed Nov 2 16:54:47 UTC 2016
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 <matt at 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 <levitte at openssl.org>
(cherry picked from commit 7856332e8c14fd1da1811a9d0afde243dd0f4669)
commit 0f6c9d73cb1e1027c67d993a669719e351c25cfc
Author: Matt Caswell <matt at 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 <levitte at 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
More information about the openssl-commits
mailing list