[openssl] master update
Dr. Paul Dale
pauli at openssl.org
Wed Jul 21 04:12:19 UTC 2021
The branch master has been updated
via 0c48fda8d38ab91356c725e00ebcbbcad9ef0302 (commit)
from fd76ee47b951657cf1366fd6297bb3a85aecb169 (commit)
- Log -----------------------------------------------------------------
commit 0c48fda8d38ab91356c725e00ebcbbcad9ef0302
Author: yangyangtiantianlonglong <yangtianlong1224 at 163.com>
Date: Thu Jul 15 20:15:36 2021 +0800
Add testcases for SSL_key_update() corner case calls
Test that SSL_key_update() is not allowed if there are writes pending.
Test that there is no reset of the packet pointer in ssl3_setup_read_buffer().
Reviewed-by: Tomas Mraz <tomas at openssl.org>
Reviewed-by: Paul Dale <pauli at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16085)
-----------------------------------------------------------------------
Summary of changes:
doc/man3/SSL_get_error.pod | 4 +-
doc/man3/SSL_key_update.pod | 9 +-
test/sslapitest.c | 271 +++++++++++++++++++++++++++++++++++++++++++-
3 files changed, 274 insertions(+), 10 deletions(-)
diff --git a/doc/man3/SSL_get_error.pod b/doc/man3/SSL_get_error.pod
index c52fd04d07..eee1cbe643 100644
--- a/doc/man3/SSL_get_error.pod
+++ b/doc/man3/SSL_get_error.pod
@@ -85,7 +85,9 @@ protocol level.
It is safe to call SSL_read() or SSL_read_ex() when more data is available
even when the call that set this error was an SSL_write() or SSL_write_ex().
However, if the call was an SSL_write() or SSL_write_ex(), it should be called
-again to continue sending the application data.
+again to continue sending the application data. If you get B<SSL_ERROR_WANT_WRITE>
+from SSL_write() or SSL_write_ex() then you should not do any other operation
+that could trigger B<IO> other than to repeat the previous SSL_write() call.
For socket B<BIO>s (e.g. when SSL_set_fd() was used), select() or
poll() on the underlying socket can be used to find out when the
diff --git a/doc/man3/SSL_key_update.pod b/doc/man3/SSL_key_update.pod
index f95d89e44a..24c125a8b5 100644
--- a/doc/man3/SSL_key_update.pod
+++ b/doc/man3/SSL_key_update.pod
@@ -32,10 +32,11 @@ peer to additionally update its sending keys. It is an error if B<updatetype> is
set to B<SSL_KEY_UPDATE_NONE>.
SSL_key_update() must only be called after the initial handshake has been
-completed and TLSv1.3 has been negotiated. The key update will not take place
-until the next time an IO operation such as SSL_read_ex() or SSL_write_ex()
-takes place on the connection. Alternatively SSL_do_handshake() can be called to
-force the update to take place immediately.
+completed and TLSv1.3 has been negotiated, at the same time, the application
+needs to ensure that the writing of data has been completed. The key update
+will not take place until the next time an IO operation such as SSL_read_ex()
+or SSL_write_ex() takes place on the connection. Alternatively SSL_do_handshake()
+can be called to force the update to take place immediately.
SSL_get_key_update_type() can be used to determine whether a key update
operation has been scheduled but not yet performed. The type of the pending key
diff --git a/test/sslapitest.c b/test/sslapitest.c
index cc11eebc54..b5212d1ace 100644
--- a/test/sslapitest.c
+++ b/test/sslapitest.c
@@ -6109,12 +6109,12 @@ static int test_key_update(void)
}
/*
- * Test we can handle a KeyUpdate (update requested) message while write data
- * is pending.
+ * Test we can handle a KeyUpdate (update requested) message while
+ * write data is pending in peer.
* Test 0: Client sends KeyUpdate while Server is writing
* Test 1: Server sends KeyUpdate while Client is writing
*/
-static int test_key_update_in_write(int tst)
+static int test_key_update_peer_in_write(int tst)
{
SSL_CTX *cctx = NULL, *sctx = NULL;
SSL *clientssl = NULL, *serverssl = NULL;
@@ -6141,7 +6141,7 @@ static int test_key_update_in_write(int tst)
peerwrite = tst == 0 ? serverssl : clientssl;
if (!TEST_true(SSL_key_update(peerupdate, SSL_KEY_UPDATE_REQUESTED))
- || !TEST_true(SSL_do_handshake(peerupdate)))
+ || !TEST_int_eq(SSL_do_handshake(peerupdate), 1))
goto end;
/* Swap the writing endpoint's write BIO to force a retry */
@@ -6192,6 +6192,264 @@ static int test_key_update_in_write(int tst)
return testresult;
}
+
+/*
+ * Test we can handle a KeyUpdate (update requested) message while
+ * peer read data is pending after peer accepted keyupdate(the msg header
+ * had been read 5 bytes).
+ * Test 0: Client sends KeyUpdate while Server is reading
+ * Test 1: Server sends KeyUpdate while Client is reading
+ */
+static int test_key_update_peer_in_read(int tst)
+{
+ SSL_CTX *cctx = NULL, *sctx = NULL;
+ SSL *clientssl = NULL, *serverssl = NULL;
+ int testresult = 0;
+ char prbuf[515], lwbuf[515] = {0};
+ static char *mess = "A test message";
+ BIO *lbio = NULL, *pbio = NULL;
+ SSL *local = NULL, *peer = NULL;
+
+ if (!TEST_true(create_ssl_ctx_pair(libctx, TLS_server_method(),
+ TLS_client_method(),
+ TLS1_3_VERSION,
+ 0,
+ &sctx, &cctx, cert, privkey))
+ || !TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl,
+ NULL, NULL))
+ || !TEST_true(create_ssl_connection(serverssl, clientssl,
+ SSL_ERROR_NONE)))
+ goto end;
+
+ local = tst == 0 ? clientssl : serverssl;
+ peer = tst == 0 ? serverssl : clientssl;
+
+ if (!TEST_int_eq(BIO_new_bio_pair(&lbio, 512, &pbio, 512), 1))
+ goto end;
+
+ SSL_set_bio(local, lbio, lbio);
+ SSL_set_bio(peer, pbio, pbio);
+
+ /*
+ * we first write keyupdate msg then appdata in local
+ * write data in local will fail with SSL_ERROR_WANT_WRITE,because
+ * lwbuf app data msg size + key updata msg size > 512(the size of
+ * the bio pair buffer)
+ */
+ if (!TEST_true(SSL_key_update(local, SSL_KEY_UPDATE_REQUESTED))
+ || !TEST_int_eq(SSL_write(local, lwbuf, sizeof(lwbuf)), -1)
+ || !TEST_int_eq(SSL_get_error(local, -1), SSL_ERROR_WANT_WRITE))
+ goto end;
+
+ /*
+ * first read keyupdate msg in peer in peer
+ * then read appdata that we know will fail with SSL_ERROR_WANT_READ
+ */
+ if (!TEST_int_eq(SSL_read(peer, prbuf, sizeof(prbuf)), -1)
+ || !TEST_int_eq(SSL_get_error(peer, -1), SSL_ERROR_WANT_READ))
+ goto end;
+
+ /* Now write some data in peer - we will write the key update */
+ if (!TEST_int_eq(SSL_write(peer, mess, strlen(mess)), strlen(mess)))
+ goto end;
+
+ /*
+ * write data in local previously that we will complete
+ * read data in peer previously that we will complete
+ */
+ if (!TEST_int_eq(SSL_write(local, lwbuf, sizeof(lwbuf)), sizeof(lwbuf))
+ || !TEST_int_eq(SSL_read(peer, prbuf, sizeof(prbuf)), sizeof(prbuf)))
+ goto end;
+
+ /* check that sending and receiving appdata ok */
+ if (!TEST_int_eq(SSL_write(local, mess, strlen(mess)), strlen(mess))
+ || !TEST_int_eq(SSL_read(peer, prbuf, sizeof(prbuf)), strlen(mess)))
+ goto end;
+
+ testresult = 1;
+
+ end:
+ SSL_free(serverssl);
+ SSL_free(clientssl);
+ SSL_CTX_free(sctx);
+ SSL_CTX_free(cctx);
+
+ return testresult;
+}
+
+/*
+ * Test we can't send a KeyUpdate (update requested) message while
+ * local write data is pending.
+ * Test 0: Client sends KeyUpdate while Client is writing
+ * Test 1: Server sends KeyUpdate while Server is writing
+ */
+static int test_key_update_local_in_write(int tst)
+{
+ SSL_CTX *cctx = NULL, *sctx = NULL;
+ SSL *clientssl = NULL, *serverssl = NULL;
+ int testresult = 0;
+ char buf[20];
+ static char *mess = "A test message";
+ BIO *bretry = BIO_new(bio_s_always_retry());
+ BIO *tmp = NULL;
+ SSL *local = NULL, *peer = NULL;
+
+ if (!TEST_ptr(bretry)
+ || !TEST_true(create_ssl_ctx_pair(libctx, TLS_server_method(),
+ TLS_client_method(),
+ TLS1_3_VERSION,
+ 0,
+ &sctx, &cctx, cert, privkey))
+ || !TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl,
+ NULL, NULL))
+ || !TEST_true(create_ssl_connection(serverssl, clientssl,
+ SSL_ERROR_NONE)))
+ goto end;
+
+ local = tst == 0 ? clientssl : serverssl;
+ peer = tst == 0 ? serverssl : clientssl;
+
+ /* Swap the writing endpoint's write BIO to force a retry */
+ tmp = SSL_get_wbio(local);
+ if (!TEST_ptr(tmp) || !TEST_true(BIO_up_ref(tmp))) {
+ tmp = NULL;
+ goto end;
+ }
+ SSL_set0_wbio(local, bretry);
+ bretry = NULL;
+
+ /* write data in local will fail with SSL_ERROR_WANT_WRITE */
+ if (!TEST_int_eq(SSL_write(local, mess, strlen(mess)), -1)
+ || !TEST_int_eq(SSL_get_error(local, -1), SSL_ERROR_WANT_WRITE))
+ goto end;
+
+ /* Reinstate the original writing endpoint's write BIO */
+ SSL_set0_wbio(local, tmp);
+ tmp = NULL;
+
+ /* SSL_key_update will fail, because writing in local*/
+ if (!TEST_false(SSL_key_update(local, SSL_KEY_UPDATE_REQUESTED))
+ || !TEST_int_eq(ERR_GET_REASON(ERR_peek_error()), SSL_R_BAD_WRITE_RETRY))
+ goto end;
+
+ ERR_clear_error();
+ /* write data in local previously that we will complete */
+ if (!TEST_int_eq(SSL_write(local, mess, strlen(mess)), strlen(mess)))
+ goto end;
+
+ /* SSL_key_update will succeed because there is no pending write data */
+ if (!TEST_true(SSL_key_update(local, SSL_KEY_UPDATE_REQUESTED))
+ || !TEST_int_eq(SSL_do_handshake(local), 1))
+ goto end;
+
+ /*
+ * we write some appdata in local
+ * read data in peer - we will read the keyupdate msg
+ */
+ if (!TEST_int_eq(SSL_write(local, mess, strlen(mess)), strlen(mess))
+ || !TEST_int_eq(SSL_read(peer, buf, sizeof(buf)), strlen(mess)))
+ goto end;
+
+ /* Write more peer more data to ensure we send the keyupdate message back */
+ if (!TEST_int_eq(SSL_write(peer, mess, strlen(mess)), strlen(mess))
+ || !TEST_int_eq(SSL_read(local, buf, sizeof(buf)), strlen(mess)))
+ goto end;
+
+ testresult = 1;
+
+ end:
+ SSL_free(serverssl);
+ SSL_free(clientssl);
+ SSL_CTX_free(sctx);
+ SSL_CTX_free(cctx);
+ BIO_free(bretry);
+ BIO_free(tmp);
+
+ return testresult;
+}
+
+/*
+ * Test we can handle a KeyUpdate (update requested) message while
+ * local read data is pending(the msg header had been read 5 bytes).
+ * Test 0: Client sends KeyUpdate while Client is reading
+ * Test 1: Server sends KeyUpdate while Server is reading
+ */
+static int test_key_update_local_in_read(int tst)
+{
+ SSL_CTX *cctx = NULL, *sctx = NULL;
+ SSL *clientssl = NULL, *serverssl = NULL;
+ int testresult = 0;
+ char lrbuf[515], pwbuf[515] = {0}, prbuf[20];
+ static char *mess = "A test message";
+ BIO *lbio = NULL, *pbio = NULL;
+ SSL *local = NULL, *peer = NULL;
+
+ if (!TEST_true(create_ssl_ctx_pair(libctx, TLS_server_method(),
+ TLS_client_method(),
+ TLS1_3_VERSION,
+ 0,
+ &sctx, &cctx, cert, privkey))
+ || !TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl,
+ NULL, NULL))
+ || !TEST_true(create_ssl_connection(serverssl, clientssl,
+ SSL_ERROR_NONE)))
+ goto end;
+
+ local = tst == 0 ? clientssl : serverssl;
+ peer = tst == 0 ? serverssl : clientssl;
+
+ if (!TEST_int_eq(BIO_new_bio_pair(&lbio, 512, &pbio, 512), 1))
+ goto end;
+
+ SSL_set_bio(local, lbio, lbio);
+ SSL_set_bio(peer, pbio, pbio);
+
+ /* write app data in peer will fail with SSL_ERROR_WANT_WRITE */
+ if (!TEST_int_eq(SSL_write(peer, pwbuf, sizeof(pwbuf)), -1)
+ || !TEST_int_eq(SSL_get_error(peer, -1), SSL_ERROR_WANT_WRITE))
+ goto end;
+
+ /* read appdata in local will fail with SSL_ERROR_WANT_READ */
+ if (!TEST_int_eq(SSL_read(local, lrbuf, sizeof(lrbuf)), -1)
+ || !TEST_int_eq(SSL_get_error(local, -1), SSL_ERROR_WANT_READ))
+ goto end;
+
+ /* SSL_do_handshake will send keyupdate msg */
+ if (!TEST_true(SSL_key_update(local, SSL_KEY_UPDATE_REQUESTED))
+ || !TEST_int_eq(SSL_do_handshake(local), 1))
+ goto end;
+
+ /*
+ * write data in peer previously that we will complete
+ * read data in local previously that we will complete
+ */
+ if (!TEST_int_eq(SSL_write(peer, pwbuf, sizeof(pwbuf)), sizeof(pwbuf))
+ || !TEST_int_eq(SSL_read(local, lrbuf, sizeof(lrbuf)), sizeof(lrbuf)))
+ goto end;
+
+ /*
+ * write data in local
+ * read data in peer - we will read the key update
+ */
+ if (!TEST_int_eq(SSL_write(local, mess, strlen(mess)), strlen(mess))
+ || !TEST_int_eq(SSL_read(peer, prbuf, sizeof(prbuf)), strlen(mess)))
+ goto end;
+
+ /* Write more peer data to ensure we send the keyupdate message back */
+ if (!TEST_int_eq(SSL_write(peer, mess, strlen(mess)), strlen(mess))
+ || !TEST_int_eq(SSL_read(local, lrbuf, sizeof(lrbuf)), strlen(mess)))
+ goto end;
+
+ testresult = 1;
+
+ end:
+ SSL_free(serverssl);
+ SSL_free(clientssl);
+ SSL_CTX_free(sctx);
+ SSL_CTX_free(cctx);
+
+ return testresult;
+}
#endif /* OSSL_NO_USABLE_TLS1_3 */
static int test_ssl_clear(int idx)
@@ -9402,7 +9660,10 @@ int setup_tests(void)
#ifndef OSSL_NO_USABLE_TLS1_3
ADD_ALL_TESTS(test_export_key_mat_early, 3);
ADD_TEST(test_key_update);
- ADD_ALL_TESTS(test_key_update_in_write, 2);
+ ADD_ALL_TESTS(test_key_update_peer_in_write, 2);
+ ADD_ALL_TESTS(test_key_update_peer_in_read, 2);
+ ADD_ALL_TESTS(test_key_update_local_in_write, 2);
+ ADD_ALL_TESTS(test_key_update_local_in_read, 2);
#endif
ADD_ALL_TESTS(test_ssl_clear, 2);
ADD_ALL_TESTS(test_max_fragment_len_ext, OSSL_NELEM(max_fragment_len_test));
More information about the openssl-commits
mailing list