[openssl-commits] [openssl] master update
Matt Caswell
matt at openssl.org
Thu Mar 16 15:49:13 UTC 2017
The branch master has been updated
via 6828358c6565af0e31ac1a9ff9c54c94a04bec75 (commit)
via 6bc6ca623b1785653ae2e0332957f0355f496509 (commit)
via d702ad121c18b43f61832318a9e61b8d42aaa06c (commit)
from 635b7d3f2a3a4c1caaf772dc9a6c1cdcb958f6fe (commit)
- Log -----------------------------------------------------------------
commit 6828358c6565af0e31ac1a9ff9c54c94a04bec75
Author: Matt Caswell <matt at openssl.org>
Date: Thu Mar 16 15:09:59 2017 +0000
Handle TLSv1.3 being disabled in clienthello test
Reviewed-by: Rich Salz <rsalz at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2968)
commit 6bc6ca623b1785653ae2e0332957f0355f496509
Author: Matt Caswell <matt at openssl.org>
Date: Thu Mar 16 12:11:23 2017 +0000
Add tests for the padding extension
Check that the padding extension pads correctly for various scenarios.
Reviewed-by: Rich Salz <rsalz at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2968)
commit d702ad121c18b43f61832318a9e61b8d42aaa06c
Author: Matt Caswell <matt at openssl.org>
Date: Thu Mar 16 10:18:39 2017 +0000
Fix the Padding extension
In OpenSSL 1.1.0 the padding extension MUST be last because it calculates
the length of everything that has been written into the ClientHello to
determine whether it needs to be padded or not. With TLSv1.3 that isn't
possible because the specification requires that the PSK extension is last.
Therefore we need to fix the padding extension to take account of any PSK
extension that will be later added.
Reviewed-by: Rich Salz <rsalz at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2968)
-----------------------------------------------------------------------
Summary of changes:
ssl/statem/extensions.c | 1 -
ssl/statem/extensions_clnt.c | 47 ++++++-
test/build.info | 2 +-
test/clienthellotest.c | 255 +++++++++++++++++++++++++------------
test/recipes/70-test_clienthello.t | 5 +-
test/session.pem | 30 +++++
6 files changed, 249 insertions(+), 91 deletions(-)
create mode 100644 test/session.pem
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index f11f5e0..d62c5af 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -289,7 +289,6 @@ static const EXTENSION_DEFINITION ext_defs[] = {
},
{
/* Must be immediately before pre_shared_key */
- /* TODO(TLS1.3): Fix me */
TLSEXT_TYPE_padding,
EXT_CLIENT_HELLO,
NULL,
diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index 84bfb3c..400de3f 100644
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -691,6 +691,20 @@ int tls_construct_ctos_early_data(SSL *s, WPACKET *pkt, unsigned int context,
#define F5_WORKAROUND_MIN_MSG_LEN 0xff
#define F5_WORKAROUND_MAX_MSG_LEN 0x200
+/*
+ * PSK pre binder overhead =
+ * 2 bytes for TLSEXT_TYPE_psk
+ * 2 bytes for extension length
+ * 2 bytes for identities list length
+ * 2 bytes for identity length
+ * 4 bytes for obfuscated_ticket_age
+ * 2 bytes for binder list length
+ * 1 byte for binder length
+ * The above excludes the number of bytes for the identity itself and the
+ * subsequent binder bytes
+ */
+#define PSK_PRE_BINDER_OVERHEAD (2 + 2 + 2 + 2 + 4 + 2 + 1)
+
int tls_construct_ctos_padding(SSL *s, WPACKET *pkt, unsigned int context,
X509 *x, size_t chainidx, int *al)
{
@@ -701,16 +715,35 @@ int tls_construct_ctos_padding(SSL *s, WPACKET *pkt, unsigned int context,
return 1;
/*
- * Add padding to workaround bugs in F5 terminators. See
- * https://tools.ietf.org/html/draft-agl-tls-padding-03 NB: because this
- * code calculates the length of all existing extensions it MUST always
- * appear last.
+ * Add padding to workaround bugs in F5 terminators. See RFC7685.
+ * This code calculates the length of all extensions added so far but
+ * excludes the PSK extension (because that MUST be written last). Therefore
+ * this extension MUST always appear second to last.
*/
if (!WPACKET_get_total_written(pkt, &hlen)) {
SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_PADDING, ERR_R_INTERNAL_ERROR);
return 0;
}
+ /*
+ * If we're going to send a PSK then that will be written out after this
+ * extension, so we need to calculate how long it is going to be.
+ */
+ if (s->session->ssl_version == TLS1_3_VERSION
+ && s->session->ext.ticklen != 0
+ && s->session->cipher != NULL) {
+ const EVP_MD *md = ssl_md(s->session->cipher->algorithm2);
+
+ if (md != NULL) {
+ /*
+ * Add the fixed PSK overhead, the identity length and the binder
+ * length.
+ */
+ hlen += PSK_PRE_BINDER_OVERHEAD + s->session->ext.ticklen
+ + EVP_MD_size(md);
+ }
+ }
+
if (hlen > F5_WORKAROUND_MIN_MSG_LEN && hlen < F5_WORKAROUND_MAX_MSG_LEN) {
/* Calculate the amond of padding we need to add */
hlen = F5_WORKAROUND_MAX_MSG_LEN - hlen;
@@ -751,6 +784,12 @@ int tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context, X509 *x,
s->session->ext.tick_identity = TLSEXT_PSK_BAD_IDENTITY;
/*
+ * Note: At this stage of the code we only support adding a single
+ * resumption PSK. If we add support for multiple PSKs then the length
+ * calculations in the padding extension will need to be adjusted.
+ */
+
+ /*
* If this is an incompatible or new session then we have nothing to resume
* so don't add this extension.
*/
diff --git a/test/build.info b/test/build.info
index 35ce601..104d3a5 100644
--- a/test/build.info
+++ b/test/build.info
@@ -182,7 +182,7 @@ IF[{- !$disabled{tests} -}]
INCLUDE[verify_extra_test]=../include
DEPEND[verify_extra_test]=../libcrypto
- SOURCE[clienthellotest]=clienthellotest.c
+ SOURCE[clienthellotest]=clienthellotest.c testutil.c test_main_custom.c
INCLUDE[clienthellotest]=../include
DEPEND[clienthellotest]=../libcrypto ../libssl
diff --git a/test/clienthellotest.c b/test/clienthellotest.c
index 718b582..e72ecc2 100644
--- a/test/clienthellotest.c
+++ b/test/clienthellotest.c
@@ -15,144 +15,233 @@
#include <openssl/evp.h>
#include <openssl/ssl.h>
#include <openssl/err.h>
+#include <time.h>
#include "../ssl/packet_locl.h"
-#define CLIENT_VERSION_LEN 2
+#include "testutil.h"
+#include "test_main_custom.h"
+#define CLIENT_VERSION_LEN 2
-#define TOTAL_NUM_TESTS 1
+#define TOTAL_NUM_TESTS 4
/*
* Test that explicitly setting ticket data results in it appearing in the
* ClientHello for a negotiated SSL/TLS version
*/
#define TEST_SET_SESSION_TICK_DATA_VER_NEG 0
+/* Enable padding and make sure ClientHello is long enough to require it */
+#define TEST_ADD_PADDING 1
+/* Enable padding and make sure ClientHello is short enough to not need it */
+#define TEST_PADDING_NOT_NEEDED 2
+/*
+ * Enable padding and add a PSK to the ClientHello (this will also ensure the
+ * ClientHello is long enough to need padding)
+ */
+#define TEST_ADD_PADDING_AND_PSK 3
+
+#define F5_WORKAROUND_MIN_MSG_LEN 0xff
+#define F5_WORKAROUND_MAX_MSG_LEN 0x200
+
+static const char *sessionfile = NULL;
+/* Dummy ALPN protocols used to pad out the size of the ClientHello */
+static const char alpn_prots[] =
+ "0123456789012345678901234567890123456789012345678901234567890123456789"
+ "0123456789012345678901234567890123456789012345678901234567890123456789";
-int main(int argc, char *argv[])
+static int test_client_hello(int currtest)
{
SSL_CTX *ctx;
SSL *con = NULL;
BIO *rbio;
BIO *wbio;
- BIO *err;
long len;
unsigned char *data;
PACKET pkt, pkt2, pkt3;
char *dummytick = "Hello World!";
unsigned int type;
int testresult = 0;
- int currtest = 0;
+ size_t msglen;
+ BIO *sessbio = NULL;
+ SSL_SESSION *sess = NULL;
- err = BIO_new_fp(stderr, BIO_NOCLOSE | BIO_FP_TEXT);
-
- CRYPTO_set_mem_debug(1);
- CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ON);
+#ifdef OPENSSL_NO_TLS1_3
+ if (currtest == TEST_ADD_PADDING_AND_PSK)
+ return 1;
+#endif
/*
* For each test set up an SSL_CTX and SSL and see what ClientHello gets
* produced when we try to connect
*/
- for (; currtest < TOTAL_NUM_TESTS; currtest++) {
- testresult = 0;
- ctx = SSL_CTX_new(TLS_method());
+ ctx = SSL_CTX_new(TLS_method());
+ if (ctx == NULL)
+ goto end;
+ switch(currtest) {
+ case TEST_SET_SESSION_TICK_DATA_VER_NEG:
/* Testing for session tickets <= TLS1.2; not relevant for 1.3 */
- if (ctx == NULL || !SSL_CTX_set_max_proto_version(ctx, TLS1_2_VERSION))
+ if (!SSL_CTX_set_max_proto_version(ctx, TLS1_2_VERSION))
goto end;
-
- con = SSL_new(ctx);
- if (con == NULL)
+ break;
+
+ case TEST_ADD_PADDING_AND_PSK:
+ case TEST_ADD_PADDING:
+ case TEST_PADDING_NOT_NEEDED:
+ SSL_CTX_set_options(ctx, SSL_OP_TLSEXT_PADDING);
+ /*
+ * Add lots of ciphersuites so that the ClientHello is at least
+ * F5_WORKAROUND_MIN_MSG_LEN bytes long - meaning padding will be
+ * needed. Also add some dummy ALPN protocols in case we still don't
+ * have enough.
+ * In the padding not needed case we assume the test will pass, but then
+ * set testresult to 0 if we see the padding extension.
+ */
+ if (currtest == TEST_ADD_PADDING
+ && (!SSL_CTX_set_cipher_list(ctx, "ALL")
+ || SSL_CTX_set_alpn_protos(ctx,
+ (unsigned char *)alpn_prots,
+ sizeof(alpn_prots) - 1)))
goto end;
+ else if (currtest == TEST_PADDING_NOT_NEEDED)
+ testresult = 1;
+ break;
- rbio = BIO_new(BIO_s_mem());
- wbio = BIO_new(BIO_s_mem());
- if (rbio == NULL || wbio == NULL) {
- BIO_free(rbio);
- BIO_free(wbio);
- goto end;
- }
+ default:
+ goto end;
+ }
- SSL_set_bio(con, rbio, wbio);
- SSL_set_connect_state(con);
+ con = SSL_new(ctx);
+ if (con == NULL)
+ goto end;
- if (currtest == TEST_SET_SESSION_TICK_DATA_VER_NEG) {
- if (!SSL_set_session_ticket_ext(con, dummytick, strlen(dummytick)))
- goto end;
+ if (currtest == TEST_ADD_PADDING_AND_PSK) {
+ sessbio = BIO_new_file(sessionfile, "r");
+ if (sessbio == NULL) {
+ printf("Unable to open session.pem\n");
+ goto end;
}
-
- if (SSL_connect(con) > 0) {
- /* This shouldn't succeed because we don't have a server! */
+ sess = PEM_read_bio_SSL_SESSION(sessbio, NULL, NULL, NULL);
+ if (sess == NULL) {
+ printf("Unable to load SSL_SESSION\n");
goto end;
}
-
- len = BIO_get_mem_data(wbio, (char **)&data);
- if (!PACKET_buf_init(&pkt, data, len))
+ /*
+ * We reset the creation time so that we don't discard the session as
+ * too old.
+ */
+ if (!SSL_SESSION_set_time(sess, time(NULL))) {
+ printf("Unable to set creation time on SSL_SESSION\n");
goto end;
-
- /* Skip the record header */
- if (!PACKET_forward(&pkt, SSL3_RT_HEADER_LENGTH))
+ }
+ if (!SSL_set_session(con, sess)) {
+ printf("Unable to set the session on the connection\n");
goto end;
+ }
+ }
- /* Skip the handshake message header */
- if (!PACKET_forward(&pkt, SSL3_HM_HEADER_LENGTH))
- goto end;
+ rbio = BIO_new(BIO_s_mem());
+ wbio = BIO_new(BIO_s_mem());
+ if (rbio == NULL || wbio == NULL) {
+ BIO_free(rbio);
+ BIO_free(wbio);
+ goto end;
+ }
- /* Skip client version and random */
- if (!PACKET_forward(&pkt, CLIENT_VERSION_LEN + SSL3_RANDOM_SIZE))
- goto end;
+ SSL_set_bio(con, rbio, wbio);
+ SSL_set_connect_state(con);
- /* Skip session id */
- if (!PACKET_get_length_prefixed_1(&pkt, &pkt2))
+ if (currtest == TEST_SET_SESSION_TICK_DATA_VER_NEG) {
+ if (!SSL_set_session_ticket_ext(con, dummytick, strlen(dummytick)))
goto end;
+ }
- /* Skip ciphers */
- if (!PACKET_get_length_prefixed_2(&pkt, &pkt2))
- goto end;
+ if (SSL_connect(con) > 0) {
+ /* This shouldn't succeed because we don't have a server! */
+ goto end;
+ }
- /* Skip compression */
- if (!PACKET_get_length_prefixed_1(&pkt, &pkt2))
- goto end;
+ len = BIO_get_mem_data(wbio, (char **)&data);
+ if (!PACKET_buf_init(&pkt, data, len))
+ goto end;
+
+ /* Skip the record header */
+ if (!PACKET_forward(&pkt, SSL3_RT_HEADER_LENGTH))
+ goto end;
+
+ msglen = PACKET_remaining(&pkt);
+
+ /* Skip the handshake message header */
+ if (!PACKET_forward(&pkt, SSL3_HM_HEADER_LENGTH))
+ goto end;
+
+ /* Skip client version and random */
+ if (!PACKET_forward(&pkt, CLIENT_VERSION_LEN + SSL3_RANDOM_SIZE))
+ goto end;
+
+ /* Skip session id */
+ if (!PACKET_get_length_prefixed_1(&pkt, &pkt2))
+ goto end;
+
+ /* Skip ciphers */
+ if (!PACKET_get_length_prefixed_2(&pkt, &pkt2))
+ goto end;
+
+ /* Skip compression */
+ if (!PACKET_get_length_prefixed_1(&pkt, &pkt2))
+ goto end;
- /* Extensions len */
- if (!PACKET_as_length_prefixed_2(&pkt, &pkt2))
+ /* Extensions len */
+ if (!PACKET_as_length_prefixed_2(&pkt, &pkt2))
+ goto end;
+
+ /* Loop through all extensions */
+ while (PACKET_remaining(&pkt2)) {
+
+ if (!PACKET_get_net_2(&pkt2, &type) ||
+ !PACKET_get_length_prefixed_2(&pkt2, &pkt3))
goto end;
- /* Loop through all extensions */
- while (PACKET_remaining(&pkt2)) {
-
- if (!PACKET_get_net_2(&pkt2, &type) ||
- !PACKET_get_length_prefixed_2(&pkt2, &pkt3))
- goto end;
-
- if (type == TLSEXT_TYPE_session_ticket) {
- if (currtest == TEST_SET_SESSION_TICK_DATA_VER_NEG) {
- if (PACKET_equal(&pkt3, dummytick, strlen(dummytick))) {
- /* Ticket data is as we expected */
- testresult = 1;
- } else {
- printf("Received session ticket is not as expected\n");
- }
- break;
+ if (type == TLSEXT_TYPE_session_ticket) {
+ if (currtest == TEST_SET_SESSION_TICK_DATA_VER_NEG) {
+ if (PACKET_equal(&pkt3, dummytick, strlen(dummytick))) {
+ /* Ticket data is as we expected */
+ testresult = 1;
+ } else {
+ printf("Received session ticket is not as expected\n");
}
+ break;
}
-
}
-
- end:
- SSL_free(con);
- SSL_CTX_free(ctx);
- if (!testresult) {
- printf("ClientHello test: FAILED (Test %d)\n", currtest);
- break;
+ if (type == TLSEXT_TYPE_padding) {
+ if (currtest == TEST_ADD_PADDING
+ || currtest == TEST_ADD_PADDING_AND_PSK)
+ testresult = (msglen == F5_WORKAROUND_MAX_MSG_LEN);
+ else
+ testresult = 0;
}
}
-#ifndef OPENSSL_NO_CRYPTO_MDEBUG
- if (CRYPTO_mem_leaks(err) <= 0)
- testresult = 0;
-#endif
- BIO_free(err);
+end:
+ SSL_free(con);
+ SSL_CTX_free(ctx);
+ SSL_SESSION_free(sess);
+ BIO_free(sessbio);
+ if (!testresult)
+ printf("ClientHello test: FAILED (Test %d)\n", currtest);
+
+ return testresult;
+}
+
+int test_main(int argc, char *argv[])
+{
+ if (argc != 2)
+ return EXIT_FAILURE;
+
+ sessionfile = argv[1];
+
+ ADD_ALL_TESTS(test_client_hello, TOTAL_NUM_TESTS);
- return testresult?0:1;
+ return run_tests(argv[0]);
}
diff --git a/test/recipes/70-test_clienthello.t b/test/recipes/70-test_clienthello.t
index ef0868f..ccb3a5a 100644
--- a/test/recipes/70-test_clienthello.t
+++ b/test/recipes/70-test_clienthello.t
@@ -7,7 +7,7 @@
# https://www.openssl.org/source/license.html
-use OpenSSL::Test;
+use OpenSSL::Test qw/:DEFAULT srctop_file/;
use OpenSSL::Test::Utils;
setup("test_clienthello");
@@ -17,4 +17,5 @@ plan skip_all => "No TLS/SSL protocols are supported by this OpenSSL build"
plan tests => 1;
-ok(run(test(["clienthellotest"])), "running clienthellotest");
+ok(run(test(["clienthellotest", srctop_file("test", "session.pem")])),
+ "running clienthellotest");
diff --git a/test/session.pem b/test/session.pem
new file mode 100644
index 0000000..fa23277
--- /dev/null
+++ b/test/session.pem
@@ -0,0 +1,30 @@
+-----BEGIN SSL SESSION PARAMETERS-----
+MIIFMAIBAQICAwQEAhMCBCAuhyL8Neo+jOicuNiWOzIDX/HXQRGGkgru3aX+p7+6
+CgQwXZWvZnbuON/qITvDWC7KoECPjyThlAd3fRe7ZxD/6C+vqf+SpSUMcxS7P24t
+RyXYoQYCBFjKfImiBAICHCCjggPrMIID5zCCAs+gAwIBAgIJALnu1NlVpZ6zMA0G
+CSqGSIb3DQEBBQUAMHAxCzAJBgNVBAYTAlVLMRYwFAYDVQQKDA1PcGVuU1NMIEdy
+b3VwMSIwIAYDVQQLDBlGT1IgVEVTVElORyBQVVJQT1NFUyBPTkxZMSUwIwYDVQQD
+DBxPcGVuU1NMIFRlc3QgSW50ZXJtZWRpYXRlIENBMB4XDTExMTIwODE0MDE0OFoX
+DTIxMTAxNjE0MDE0OFowZDELMAkGA1UEBhMCVUsxFjAUBgNVBAoMDU9wZW5TU0wg
+R3JvdXAxIjAgBgNVBAsMGUZPUiBURVNUSU5HIFBVUlBPU0VTIE9OTFkxGTAXBgNV
+BAMMEFRlc3QgU2VydmVyIENlcnQwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEK
+AoIBAQDzhPOSNtyyRspmeuUpxfNJKCLTuf7g3uQ4zu4iHOmRO5TQci+HhVlLZrHF
+9XqFXcIP0y4pWDbMSGuiorUmzmfiR7bfSdI/+qIQt8KXRH6HNG1t8ou0VSvWId5T
+S5Dq/er5ODUr9OaaDva7EquHIcMvvPQGuI+OEAcnleVCy9HVEIySrO4P3CNIicnG
+kwwiAud05yUAq/gPXBC1hTtmlPD7TVcGVSEiJdvzqqlgv02qedGrkki6GY4S7GjZ
+xrrf7Foc2EP+51LJzwLQx3/JfrCU41NEWAsu/Sl0tQabXESN+zJ1pDqoZ3uHMgpQ
+jeGiE0olr+YcsSW/tJmiU9OiAr8RAgMBAAGjgY8wgYwwDAYDVR0TAQH/BAIwADAO
+BgNVHQ8BAf8EBAMCBeAwLAYJYIZIAYb4QgENBB8WHU9wZW5TU0wgR2VuZXJhdGVk
+IENlcnRpZmljYXRlMB0GA1UdDgQWBBSCvM8AABPR9zklmifnr9LvIBturDAfBgNV
+HSMEGDAWgBQ2w2yI55X+sL3szj49hqshgYfa2jANBgkqhkiG9w0BAQUFAAOCAQEA
+qb1NV0B0/pbpK9Z4/bNjzPQLTRLKWnSNm/Jh5v0GEUOE/Beg7GNjNrmeNmqxAlpq
+Wz9qoeoFZax+QBpIZYjROU3TS3fpyLsrnlr0CDQ5R7kCCDGa8dkXxemmpZZLbUCp
+W2Uoy8sAA4JjN9OtsZY7dvUXFgJ7vVNTRnI01ghknbtD+2SxSQd3CWF6QhcRMAzZ
+J1z1cbbwGDDzfvGFPzJ+Sq+zEPdsxoVLLSetCiBc+40ZcDS5dV98h9XD7JMTQfxz
+A7mNGv73JoZJA6nFgj+ADSlJsY/tJBv+z1iQRueoh9Qeee+ZbRifPouCB8FDx+Al
+tvHTANdAq0t/K3o+pplMVKQCBAClAwIBFakEAgIcIKqBswSBsKXqWrhXS9CdUYkn
+yj8+BRslsixGMMFyWSHsivOMmAf3dX5z/iDaY8cqytsRkNRKzlSPjblplzcGo9pz
+sUazmp39cWRsWrKJs2izBxqVRcp4rpzzDCSTZK3UiY2uhKgGmC2WPwIMyxuEya00
+rmMgKGee7AQPG8qQGQgDEd/6Vh1ZPbpsh+XQW42ZgMhc4iDsRETH/DTlRkm527lH
+IA1ez17Zk5vMIa65o82opA4KCVRqrgcCBQDXFjTErwQCAkAA
+-----END SSL SESSION PARAMETERS-----
More information about the openssl-commits
mailing list