You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by ma...@apache.org on 2022/02/01 06:19:13 UTC

[trafficserver] branch master updated: Make TLS Early Data available with BoringSSL (#8610)

This is an automated email from the ASF dual-hosted git repository.

maskit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new 17b9c5f  Make TLS Early Data available with BoringSSL (#8610)
17b9c5f is described below

commit 17b9c5f389ccc266c967261df85262a9206041dd
Author: Masakazu Kitajo <ma...@apache.org>
AuthorDate: Tue Feb 1 15:13:37 2022 +0900

    Make TLS Early Data available with BoringSSL (#8610)
    
    * Make TLS Early Data available with BoringSSL
    
    * Update documentation
    
    * Add comments to code for BoringSSL
---
 build/crypto.m4                              |   8 +-
 doc/admin-guide/files/records.config.en.rst  |   3 +
 iocore/net/P_SSLUtils.h                      |   1 +
 iocore/net/SSLNetVConnection.cc              | 117 ++++++++++++++++++++++++---
 iocore/net/SSLUtils.cc                       |  19 +++++
 tests/gold_tests/tls/test-0rtt-s_client.py   |   2 +-
 tests/gold_tests/tls/tls_0rtt_server.test.py |   2 +-
 7 files changed, 134 insertions(+), 18 deletions(-)

diff --git a/build/crypto.m4 b/build/crypto.m4
index e9e675f..a8cc917 100644
--- a/build/crypto.m4
+++ b/build/crypto.m4
@@ -313,17 +313,15 @@ dnl
 AC_DEFUN([TS_CHECK_EARLY_DATA], [
   _set_ciphersuites_saved_LIBS=$LIBS
 
+  has_tls_early_data=0
+  early_data_check=no
   TS_ADDTO(LIBS, [$OPENSSL_LIBS])
   AC_CHECK_HEADERS(openssl/ssl.h)
   AC_CHECK_FUNCS(
-    SSL_set_max_early_data,
+    SSL_set_max_early_data SSL_read_early_data SSL_write_early_data SSL_in_early_data,
     [
       has_tls_early_data=1
       early_data_check=yes
-    ],
-    [
-      has_tls_early_data=0
-      early_data_check=no
     ]
   )
 
diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst
index 21792a3..d9fb77a 100644
--- a/doc/admin-guide/files/records.config.en.rst
+++ b/doc/admin-guide/files/records.config.en.rst
@@ -4005,6 +4005,9 @@ TLS v1.3 0-RTT Configuration
 
    Setting to ``0`` effectively disables 0-RTT.
 
+   If you use BoringSSL, setting a value grater than 0 enables early data but the value won't be used to limit the
+   maximum amount of early data.
+
 .. ts:cv:: CONFIG proxy.config.ssl.server.allow_early_data_params INT 0
 
    Set to ``1`` to allow HTTP parameters on early data requests.
diff --git a/iocore/net/P_SSLUtils.h b/iocore/net/P_SSLUtils.h
index 7c7d66c..dc8c3e8 100644
--- a/iocore/net/P_SSLUtils.h
+++ b/iocore/net/P_SSLUtils.h
@@ -226,6 +226,7 @@ private:
   virtual bool _set_alpn_callback(SSL_CTX *ctx);
   virtual bool _set_keylog_callback(SSL_CTX *ctx);
   virtual bool _enable_ktls(SSL_CTX *ctx);
+  virtual bool _enable_early_data(SSL_CTX *ctx);
 };
 
 // Create a new SSL server context fully configured (cert and keys are optional).
diff --git a/iocore/net/SSLNetVConnection.cc b/iocore/net/SSLNetVConnection.cc
index 52cfc40..63b5c33 100644
--- a/iocore/net/SSLNetVConnection.cc
+++ b/iocore/net/SSLNetVConnection.cc
@@ -184,6 +184,7 @@ SSLNetVConnection::_make_ssl_connection(SSL_CTX *ctx)
       // mechanism and causing session resumption to fail.
       SSLConfig::scoped_config params;
       if (SSL_version(ssl) >= TLS1_3_VERSION && params->server_max_early_data > 0) {
+#ifdef HAVE_SSL_SET_MAX_EARLY_DATA
         bool ret1 = false;
         bool ret2 = false;
         if ((ret1 = SSL_set_max_early_data(ssl, params->server_max_early_data)) == 1) {
@@ -202,6 +203,12 @@ SSLNetVConnection::_make_ssl_connection(SSL_CTX *ctx)
           Debug("ssl_early_data", "Must disable anti-replay if 0-rtt is enabled.");
           SSL_set_options(ssl, SSL_OP_NO_ANTI_REPLAY);
         }
+#else
+        // If SSL_set_max_early_data is unavailable, it's probably BoringSSL,
+        // and SSL_set_early_data_enabled should be available.
+        SSL_set_early_data_enabled(ssl, 1);
+        Warning("max_early_data is not used due to library limitations");
+#endif
       }
 #endif
     }
@@ -2014,15 +2021,63 @@ SSLNetVConnection::_ssl_accept()
 
 #if TS_HAS_TLS_EARLY_DATA
   if (SSLConfigParams::server_max_early_data > 0 && !this->_early_data_finish) {
-    size_t nread;
+#if HAVE_SSL_READ_EARLY_DATA
+    size_t nread = 0;
+#else
+    ssize_t nread = 0;
+#endif
 
     while (true) {
-      IOBufferBlock *block = new_IOBufferBlock();
+      bool had_error_on_reading_early_data = false;
+      bool finished_reading_early_data     = false;
+      IOBufferBlock *block                 = new_IOBufferBlock();
       block->alloc(BUFFER_SIZE_INDEX_16K);
-      ret = SSL_read_early_data(ssl, block->buf(), index_to_buffer_size(BUFFER_SIZE_INDEX_16K), &nread);
 
+#if HAVE_SSL_READ_EARLY_DATA
+      ret = SSL_read_early_data(ssl, block->buf(), index_to_buffer_size(BUFFER_SIZE_INDEX_16K), &nread);
       if (ret == SSL_READ_EARLY_DATA_ERROR) {
-        Debug("ssl_early_data", "SSL_READ_EARLY_DATA_ERROR");
+        had_error_on_reading_early_data = true;
+      } else if (ret == SSL_READ_EARLY_DATA_FINISH) {
+        finished_reading_early_data = true;
+      }
+#else
+      // If SSL_read_early_data is unavailable, it's probably BoringSSL,
+      // and SSL_in_early_data should be available.
+      ret = SSL_accept(ssl);
+      if (ret <= 0) {
+        had_error_on_reading_early_data = true;
+      } else {
+        if (SSL_in_early_data(ssl)) {
+          ret = SSL_read(ssl, block->buf(), index_to_buffer_size(BUFFER_SIZE_INDEX_16K));
+          finished_reading_early_data = !SSL_in_early_data(ssl);
+          if (ret < 0) {
+            nread = 0;
+            if (finished_reading_early_data) {
+              ret = 2; // SSL_READ_EARLY_DATA_FINISH
+            } else {
+              // Don't override ret here.
+              // Keeping the original retrurn value let ATS allow to check the value by SSL_get_error.
+              // That gives a chance to progress handshake process, or shutdown a connection if the error is serious.
+              had_error_on_reading_early_data = true;
+            }
+          } else {
+            nread = ret;
+            if (finished_reading_early_data) {
+              ret = 2; // SSL_READ_EARLY_DATA_FINISH
+            } else {
+              ret = 1; // SSL_READ_EARLY_DATA_SUCCESS
+            }
+          }
+        } else {
+          nread = 0;
+          ret = 2; // SSL_READ_EARLY_DATA_FINISH
+          finished_reading_early_data = true;
+        }
+      }
+#endif
+
+      if (had_error_on_reading_early_data) {
+        Debug("ssl_early_data", "Error on reading early data: %d", ret);
         block->free();
         break;
       } else {
@@ -2043,7 +2098,7 @@ SSLNetVConnection::_ssl_accept()
           block->free();
         }
 
-        if (ret == SSL_READ_EARLY_DATA_FINISH) {
+        if (finished_reading_early_data) {
           this->_early_data_finish = true;
           Debug("ssl_early_data", "SSL_READ_EARLY_DATA_FINISH: size = %lu", nread);
 
@@ -2141,10 +2196,17 @@ SSLNetVConnection::_ssl_write_buffer(const void *buf, int64_t nbytes, int64_t &n
   ERR_clear_error();
 
   int ret;
+  // If SSL_write_early_data is available, it's probably OpenSSL,
+  // and SSL_is_init_finished should be available.
+  // If SSL_write_early_data is unavailable, its' probably BoringSSL,
+  // and we can use SSL_write to send early data.
 #if TS_HAS_TLS_EARLY_DATA
   if (SSL_version(ssl) >= TLS1_3_VERSION) {
+#ifdef HAVE_SSL_WRITE_EARLY_DATA
     if (SSL_is_init_finished(ssl)) {
+#endif
       ret = SSL_write(ssl, buf, static_cast<int>(nbytes));
+#ifdef HAVE_SSL_WRITE_EARLY_DATA
     } else {
       size_t nwrite;
       ret = SSL_write_early_data(ssl, buf, static_cast<size_t>(nbytes), &nwrite);
@@ -2152,6 +2214,7 @@ SSLNetVConnection::_ssl_write_buffer(const void *buf, int64_t nbytes, int64_t &n
         ret = nwrite;
       }
     }
+#endif
   } else {
     ret = SSL_write(ssl, buf, static_cast<int>(nbytes));
   }
@@ -2208,15 +2271,47 @@ SSLNetVConnection::_ssl_read_buffer(void *buf, int64_t nbytes, int64_t &nread)
     }
 
     if (SSLConfigParams::server_max_early_data > 0 && !this->_early_data_finish) {
+      bool had_error_on_reading_early_data = false;
+      bool finished_reading_early_data     = false;
       Debug("ssl_early_data", "More early data to read.");
       ssl_error_t ssl_error = SSL_ERROR_NONE;
-      size_t read_bytes     = 0;
-
-      int ret = SSL_read_early_data(ssl, buf, static_cast<size_t>(nbytes), &read_bytes);
+      int ret;
+#if HAVE_SSL_READ_EARLY_DATA
+      size_t read_bytes = 0;
+#else
+      ssize_t read_bytes = 0;
+#endif
 
+#ifdef HAVE_SSL_READ_EARLY_DATA
+      ret = SSL_read_early_data(ssl, buf, static_cast<size_t>(nbytes), &read_bytes);
       if (ret == SSL_READ_EARLY_DATA_ERROR) {
-        Debug("ssl_early_data", "SSL_READ_EARLY_DATA_ERROR");
-        ssl_error = SSL_get_error(ssl, ret);
+        had_error_on_reading_early_data = true;
+        ssl_error                       = SSL_get_error(ssl, ret);
+      } else if (ret == SSL_READ_EARLY_DATA_FINISH) {
+        finished_reading_early_data = true;
+      }
+#else
+      // If SSL_read_early_data is unavailable, it's probably OpenSSL,
+      // and SSL_in_early_data should be available.
+      if (SSL_in_early_data(ssl)) {
+        ret = SSL_read(ssl, buf, nbytes);
+        finished_reading_early_data = !SSL_in_early_data(ssl);
+        if (ret < 0) {
+          if (!finished_reading_early_data) {
+            had_error_on_reading_early_data = true;
+            ssl_error = SSL_get_error(ssl, ret);
+          }
+          read_bytes = 0;
+        } else {
+          read_bytes = ret;
+        }
+      } else {
+        finished_reading_early_data = true;
+        read_bytes = 0;
+      }
+#endif
+
+      if (had_error_on_reading_early_data) {
         Debug("ssl_early_data", "Error reading early data: %s", ERR_error_string(ERR_get_error(), nullptr));
       } else {
         if ((nread = read_bytes) > 0) {
@@ -2228,7 +2323,7 @@ SSLNetVConnection::_ssl_read_buffer(void *buf, int64_t nbytes, int64_t &nread)
           }
         }
 
-        if (ret == SSL_READ_EARLY_DATA_FINISH) {
+        if (finished_reading_early_data) {
           this->_early_data_finish = true;
           Debug("ssl_early_data", "SSL_READ_EARLY_DATA_FINISH: size = %" PRId64, nread);
         } else {
diff --git a/iocore/net/SSLUtils.cc b/iocore/net/SSLUtils.cc
index 26cb731..5e045f2 100644
--- a/iocore/net/SSLUtils.cc
+++ b/iocore/net/SSLUtils.cc
@@ -695,6 +695,21 @@ SSLMultiCertConfigLoader::_enable_ktls(SSL_CTX *ctx)
   return true;
 }
 
+bool
+SSLMultiCertConfigLoader::_enable_early_data(SSL_CTX *ctx)
+{
+#if TS_HAS_TLS_EARLY_DATA
+  if (SSLConfigParams::server_max_early_data > 0) {
+#if HAVE_SSL_IN_EARLY_DATA
+    // If SSL_in_early_data is available, it's probably BoringSSL
+    // and SSL_set_early_data_enabled should be available.
+    SSL_CTX_set_early_data_enabled(ctx, 1);
+#endif
+  }
+#endif
+  return true;
+}
+
 static SSL_CTX *
 ssl_context_enable_dhe(const char *dhparams_file, SSL_CTX *ctx)
 {
@@ -1438,6 +1453,10 @@ SSLMultiCertConfigLoader::init_server_ssl_ctx(CertLoadData const &data, const SS
       goto fail;
     }
 
+    if (!this->_enable_early_data(ctx)) {
+      goto fail;
+    }
+
     if (!ssl_context_enable_dhe(_params->dhparamsFile, ctx)) {
       goto fail;
     }
diff --git a/tests/gold_tests/tls/test-0rtt-s_client.py b/tests/gold_tests/tls/test-0rtt-s_client.py
index a0a7fb6..b0033bd 100644
--- a/tests/gold_tests/tls/test-0rtt-s_client.py
+++ b/tests/gold_tests/tls/test-0rtt-s_client.py
@@ -40,7 +40,7 @@ def main():
     create_sess_proc = subprocess.Popen(s_client_cmd_1, env=os.environ.copy(
     ), stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
     try:
-        output = create_sess_proc.communicate(timeout=1)[0]
+        output = create_sess_proc.communicate(input=bytes(b'GET / HTTP/1.0\r\n\r\n'), timeout=1)[0]
     except subprocess.TimeoutExpired:
         create_sess_proc.kill()
         output = create_sess_proc.communicate()[0]
diff --git a/tests/gold_tests/tls/tls_0rtt_server.test.py b/tests/gold_tests/tls/tls_0rtt_server.test.py
index a86e979..829a68b 100644
--- a/tests/gold_tests/tls/tls_0rtt_server.test.py
+++ b/tests/gold_tests/tls/tls_0rtt_server.test.py
@@ -22,9 +22,9 @@ Test.Summary = '''
 Test ATS TLSv1.3 0-RTT support
 '''
 
+# Checking only OpenSSL version allows you to run this test with BoringSSL (and it should pass).
 Test.SkipUnless(
     Condition.HasOpenSSLVersion('1.1.1'),
-    Condition.IsOpenSSL(),
 )
 
 ts = Test.MakeATSProcess('ts', enable_tls=True)