[openssl-commits] [openssl] master update

Matt Caswell matt at openssl.org
Mon Mar 19 12:29:21 UTC 2018


The branch master has been updated
       via  3ec9e4ec46eb4356bc106db5e0e33148c693c8f0 (commit)
       via  d2d67a4cda39dc149cb9e11fd335f72c3b963976 (commit)
       via  78fb5374e1cc0f1f1d49055150e5415727b155a7 (commit)
       via  66d7de163491bfa5819fb80b77d321beb58384d4 (commit)
       via  f023ba2df821d186d73fefda6fa5cafcc5a3ee39 (commit)
       via  32305f88509c1d9ccb3ad676209a25fa59b95488 (commit)
      from  51cf8ba038aae10df9895b0001715938f7ad0c75 (commit)


- Log -----------------------------------------------------------------
commit 3ec9e4ec46eb4356bc106db5e0e33148c693c8f0
Author: Matt Caswell <matt at openssl.org>
Date:   Fri Mar 16 11:09:39 2018 +0000

    Add a CHANGES entry to mention the replay protection capabilities
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/5644)

commit d2d67a4cda39dc149cb9e11fd335f72c3b963976
Author: Matt Caswell <matt at openssl.org>
Date:   Fri Mar 16 11:07:58 2018 +0000

    Document the replay protection capabilities
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/5644)

commit 78fb5374e1cc0f1f1d49055150e5415727b155a7
Author: Matt Caswell <matt at openssl.org>
Date:   Fri Mar 16 09:53:38 2018 +0000

    Add a test for 0RTT replay protection
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/5644)

commit 66d7de163491bfa5819fb80b77d321beb58384d4
Author: Matt Caswell <matt at openssl.org>
Date:   Fri Mar 16 09:25:34 2018 +0000

    Add an anti-replay mechanism
    
    If the server is configured to allow early data then we check if the PSK
    session presented by the client is available in the cache or not. If it
    isn't then this may be a replay and we disallow it. If it is then we allow
    it and remove the session from the cache. Note: the anti-replay protection
    is not used for externally established PSKs.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/5644)

commit f023ba2df821d186d73fefda6fa5cafcc5a3ee39
Author: Matt Caswell <matt at openssl.org>
Date:   Thu Mar 15 21:02:15 2018 +0000

    Don't update the session cache when processing a client certificate in TLSv1.3
    
    We should only update the session cache when we issue a NewSessionTicket.
    These are issued automatically after processing a client certificate.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/5644)

commit 32305f88509c1d9ccb3ad676209a25fa59b95488
Author: Matt Caswell <matt at openssl.org>
Date:   Thu Mar 15 17:47:29 2018 +0000

    Always call the new_session_cb when issuing a NewSessionTicket in TLSv1.3
    
    Conceptually in TLSv1.3 there can be multiple sessions associated with a
    single connection. Each NewSessionTicket issued can be considered a
    separate session. We can end up issuing multiple NewSessionTickets on a
    single connection at the moment (e.g. in a post-handshake auth scenario).
    Each of those issued tickets should have the new_session_cb called, it
    should go into the session cache separately and it should have a unique
    id associated with it (so that they can be found individually in the
    cache).
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/5644)

-----------------------------------------------------------------------

Summary of changes:
 CHANGES                          |  4 +++
 doc/man3/SSL_read_early_data.pod | 57 +++++++++++++++++++++++++++-----
 ssl/ssl_sess.c                   | 14 +++++---
 ssl/statem/extensions_srvr.c     |  8 +++++
 ssl/statem/statem_srvr.c         |  8 ++---
 ssl/t1_lib.c                     |  7 ++--
 test/sslapitest.c                | 70 +++++++++++++++++++++++++++++++++++++---
 7 files changed, 145 insertions(+), 23 deletions(-)

diff --git a/CHANGES b/CHANGES
index 0e275c3..7b14ee9 100644
--- a/CHANGES
+++ b/CHANGES
@@ -9,6 +9,10 @@
 
  Changes between 1.1.0g and 1.1.1 [xx XXX xxxx]
 
+  *) TLSv1.3 replay protection for early data has been implemented. See the
+     SSL_read_early_data() man page for further details.
+     [Matt Caswell]
+
   *) Separated TLSv1.3 ciphersuite configuration out from TLSv1.2 ciphersuite
      configuration. TLSv1.3 ciphersuites are not compatible with TLSv1.2 and
      below. Similarly TLSv1.2 ciphersuites are not compatible with TLSv1.3.
diff --git a/doc/man3/SSL_read_early_data.pod b/doc/man3/SSL_read_early_data.pod
index a420e73..1b14a73 100644
--- a/doc/man3/SSL_read_early_data.pod
+++ b/doc/man3/SSL_read_early_data.pod
@@ -41,9 +41,9 @@ can be used to send data from the server to the client when the client has not
 yet completed the authentication stage of the handshake.
 
 Early data has weaker security properties than other data sent over an SSL/TLS
-connection. In particular the data does not have forward secrecy and there are
-no guarantees that the same early data was not replayed across multiple
-connections. For this reason extreme care should be exercised when using early
+connection. In particular the data does not have forward secrecy. There are also
+additional considerations around replay attacks (see L<REPLAY PROTECTION>
+below). For these reasons extreme care should be exercised when using early
 data. For specific details, consult the TLS 1.3 specification.
 
 When a server receives early data it may opt to immediately respond by sending
@@ -171,12 +171,16 @@ connection attempt. By default the server does not accept early data; a
 server may indicate support for early data by calling
 SSL_CTX_set_max_early_data() or
 SSL_set_max_early_data() to set it for the whole SSL_CTX or an individual SSL
-object respectively. Similarly the SSL_CTX_get_max_early_data() and
+object respectively. The B<max_early_data> parameter specifies the maximum
+amount of early data in bytes that is permitted to be sent on a single
+connection. Similarly the SSL_CTX_get_max_early_data() and
 SSL_get_max_early_data() functions can be used to obtain the current maximum
-early data settings for the SSL_CTX and SSL objects respectively.
-Generally a server application will either use both of SSL_read_early_data()
-and SSL_CTX_set_max_early_data() (or SSL_set_max_early_data()), or neither
-of them, since there is no practical benefit from using only one of them.
+early data settings for the SSL_CTX and SSL objects respectively. Generally a
+server application will either use both of SSL_read_early_data() and
+SSL_CTX_set_max_early_data() (or SSL_set_max_early_data()), or neither of them,
+since there is no practical benefit from using only one of them. If the maximum
+early data setting for a server is non-zero then replay protection is
+automatically enabled (see L<REPLAY PROTECTION> below).
 
 In the event that the current maximum early data setting for the server is
 different to that originally specified in a session that a client is resuming
@@ -209,6 +213,43 @@ Nagle's algorithm. If an application opts to disable Nagle's algorithm
 consideration should be given to turning it back on again after the handshake is
 complete if appropriate.
 
+=head1 REPLAY PROTECTION
+
+When early data is in use the TLS protocol provides no security guarantees that
+the same early data was not replayed across multiple connections. As a
+mitigation for this issue OpenSSL automatically enables replay protection if the
+server is configured with a non-zero max early data value. With replay
+protection enabled sessions are forced to be single use only. If a client
+attempts to reuse a session ticket more than once, then the second and
+subsequent attempts will fall back to a full handshake (and any early data that
+was submitted will be ignored). Note that single use tickets are enforced even
+if a client does not send any early data.
+
+The replay protection mechanism relies on the internal OpenSSL server session
+cache (see L<SSL_CTX_set_session_cache_mode(3)>). By default sessions will be
+added to the cache whenever a session ticket is issued. When a client attempts
+to resume the session OpenSSL will check for its presence in the internal cache.
+If it exists then the resumption is allowed and the session is removed from the
+cache. If it does not exist then the resumption is not allowed and a full
+handshake will occur.
+
+Note that some applications may maintain an external cache of sessions (see
+L<SSL_CTX_sess_set_new_cb(3)> and similar functions). It is the application's
+responsibility to ensure that any sessions in the external cache are also
+populated in the internal cache and that once removed from the internal cache
+they are similarly removed from the external cache. Failing to do this could
+result in an application becoming vulnerable to replay attacks. Note that
+OpenSSL will lock the internal cache while a session is removed but that lock is
+not held when the remove session callback (see L<SSL_CTX_sess_set_remove_cb(3)>)
+is called. This could result in a small amount of time where the session has
+been removed from the internal cache but is still available in the external
+cache. Applications should be designed with this in mind in order to minimise
+the possibility of replay attacks.
+
+The OpenSSL replay protection does not apply to external Pre Shared Keys (PSKs)
+(e.g. see SSL_CTX_set_psk_find_session_callback(3)). Therefore extreme caution
+should be applied when combining external PSKs with early data.
+
 =head1 RETURN VALUES
 
 SSL_write_early_data() returns 1 for success or 0 for failure. In the event of a
diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
index 1873237..6513bf8 100644
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -417,7 +417,13 @@ int ssl_get_new_session(SSL *s, int session)
     s->session = NULL;
 
     if (session) {
-        if (!ssl_generate_session_id(s, ss)) {
+        if (SSL_IS_TLS13(s)) {
+            /*
+             * We generate the session id while constructing the
+             * NewSessionTicket in TLSv1.3.
+             */
+            ss->session_id_length = 0;
+        } else if (!ssl_generate_session_id(s, ss)) {
             /* SSLfatal() already called */
             SSL_SESSION_free(ss);
             return 0;
@@ -755,10 +761,10 @@ static int remove_session_lock(SSL_CTX *ctx, SSL_SESSION *c, int lck)
     if ((c != NULL) && (c->session_id_length != 0)) {
         if (lck)
             CRYPTO_THREAD_write_lock(ctx->lock);
-        if ((r = lh_SSL_SESSION_retrieve(ctx->sessions, c)) == c) {
+        if ((r = lh_SSL_SESSION_retrieve(ctx->sessions, c)) != NULL) {
             ret = 1;
-            r = lh_SSL_SESSION_delete(ctx->sessions, c);
-            SSL_SESSION_list_remove(ctx, c);
+            r = lh_SSL_SESSION_delete(ctx->sessions, r);
+            SSL_SESSION_list_remove(ctx, r);
         }
         c->not_resumable = 1;
 
diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index 7c9a3f7..ee4cad1 100644
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -1134,6 +1134,14 @@ int tls_parse_ctos_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
             if (ret == SSL_TICKET_NO_DECRYPT)
                 continue;
 
+            /* Check for replay */
+            if (s->max_early_data > 0
+                    && !SSL_CTX_remove_session(s->session_ctx, sess)) {
+                SSL_SESSION_free(sess);
+                sess = NULL;
+                continue;
+            }
+
             ticket_age = (uint32_t)ticket_agel;
             now = (uint32_t)time(NULL);
             agesec = now - (uint32_t)sess->time;
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 50be825..c198aa7 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -3608,9 +3608,6 @@ MSG_PROCESS_RETURN tls_process_client_certificate(SSL *s, PACKET *pkt)
     sk_X509_pop_free(s->session->peer_chain, X509_free);
     s->session->peer_chain = sk;
 
-    if (new_sess != NULL)
-        ssl_update_cache(s, SSL_SESS_CACHE_SERVER);
-
     /*
      * Freeze the handshake buffer. For <TLS1.3 we do this after the CKE
      * message
@@ -3691,6 +3688,10 @@ int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt)
     } age_add_u;
 
     if (SSL_IS_TLS13(s)) {
+        if (!ssl_generate_session_id(s, s->session)) {
+            /* SSLfatal() already called */
+            goto err;
+        }
         if (ssl_randbytes(s, age_add_u.age_add_c, sizeof(age_add_u)) <= 0) {
             SSLfatal(s, SSL_AD_INTERNAL_ERROR,
                      SSL_F_TLS_CONSTRUCT_NEW_SESSION_TICKET,
@@ -3776,7 +3777,6 @@ int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt)
                  SSL_F_TLS_CONSTRUCT_NEW_SESSION_TICKET, ERR_R_INTERNAL_ERROR);
         goto err;
     }
-    sess->session_id_length = 0; /* ID is irrelevant for the ticket */
 
     slen = i2d_SSL_SESSION(sess, NULL);
     if (slen == 0 || slen > slen_full) {
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 596fdd4..796e9d4 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1409,7 +1409,7 @@ SSL_TICKET_RETURN tls_decrypt_ticket(SSL *s, const unsigned char *etick,
     OPENSSL_free(sdec);
     if (sess) {
         /* Some additional consistency checks */
-        if (slen != 0 || sess->session_id_length != 0) {
+        if (slen != 0) {
             SSL_SESSION_free(sess);
             return SSL_TICKET_NO_DECRYPT;
         }
@@ -1419,9 +1419,10 @@ SSL_TICKET_RETURN tls_decrypt_ticket(SSL *s, const unsigned char *etick,
          * structure. If it is empty set length to zero as required by
          * standard.
          */
-        if (sesslen)
+        if (sesslen) {
             memcpy(sess->session_id, sess_id, sesslen);
-        sess->session_id_length = sesslen;
+            sess->session_id_length = sesslen;
+        }
         *psess = sess;
         if (renew_ticket)
             return SSL_TICKET_SUCCESS_RENEW;
diff --git a/test/sslapitest.c b/test/sslapitest.c
index 8e91151..64f10cc 100644
--- a/test/sslapitest.c
+++ b/test/sslapitest.c
@@ -1021,15 +1021,20 @@ static int execute_test_session(int maxprot, int use_int_cache,
         goto end;
 
     if (use_ext_cache) {
-        if (!TEST_int_eq(new_called, 0)
-                || !TEST_int_eq(remove_called, 0))
+        if (!TEST_int_eq(remove_called, 0))
             goto end;
 
         if (maxprot == TLS1_3_VERSION) {
-            if (!TEST_int_eq(get_called, 0))
+            /*
+             * Every time we issue a NewSessionTicket we are creating a new
+             * session for next time in TLSv1.3
+             */
+            if (!TEST_int_eq(new_called, 1)
+                    || !TEST_int_eq(get_called, 0))
                 goto end;
         } else {
-            if (!TEST_int_eq(get_called, 1))
+            if (!TEST_int_eq(new_called, 0)
+                    || !TEST_int_eq(get_called, 1))
                 goto end;
         }
     }
@@ -1853,6 +1858,58 @@ static int test_early_data_read_write(int idx)
     return testresult;
 }
 
+static int test_early_data_replay(int idx)
+{
+    SSL_CTX *cctx = NULL, *sctx = NULL;
+    SSL *clientssl = NULL, *serverssl = NULL;
+    int testresult = 0;
+    SSL_SESSION *sess = NULL;
+
+    if (!TEST_true(setupearly_data_test(&cctx, &sctx, &clientssl,
+                                        &serverssl, &sess, idx)))
+        goto end;
+
+    /*
+     * The server is configured to accept early data. Create a connection to
+     * "use up" the ticket
+     */
+    if (!TEST_true(create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE))
+            || !TEST_true(SSL_session_reused(clientssl)))
+        goto end;
+
+    SSL_shutdown(clientssl);
+    SSL_shutdown(serverssl);
+    SSL_free(serverssl);
+    SSL_free(clientssl);
+    serverssl = clientssl = NULL;
+
+    if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl,
+                                      &clientssl, NULL, NULL))
+            || !TEST_true(SSL_set_session(clientssl, sess))
+            || !TEST_true(create_ssl_connection(serverssl, clientssl,
+                          SSL_ERROR_NONE))
+               /*
+                * This time we should not have resumed the session because we
+                * already used it once.
+                */
+            || !TEST_false(SSL_session_reused(clientssl)))
+        goto end;
+
+    testresult = 1;
+
+ end:
+    if (sess != clientpsk)
+        SSL_SESSION_free(sess);
+    SSL_SESSION_free(clientpsk);
+    SSL_SESSION_free(serverpsk);
+    clientpsk = serverpsk = NULL;
+    SSL_free(serverssl);
+    SSL_free(clientssl);
+    SSL_CTX_free(sctx);
+    SSL_CTX_free(cctx);
+    return testresult;
+}
+
 /*
  * Helper function to test that a server attempting to read early data can
  * handle a connection from a client where the early data should be skipped.
@@ -3683,6 +3740,11 @@ int setup_tests(void)
 #endif
 #ifndef OPENSSL_NO_TLS1_3
     ADD_ALL_TESTS(test_early_data_read_write, 3);
+    /*
+     * We don't do replay tests for external PSK. Replay protection isn't used
+     * in that scenario.
+     */
+    ADD_ALL_TESTS(test_early_data_replay, 2);
     ADD_ALL_TESTS(test_early_data_skip, 3);
     ADD_ALL_TESTS(test_early_data_skip_hrr, 3);
     ADD_ALL_TESTS(test_early_data_not_sent, 3);


More information about the openssl-commits mailing list