async tls handshake work

9 files changed, 47 insertions, 21 deletions

diff --git a/vespalib/src/tests/net/crypto_socket/crypto_socket_test.cpp b/vespalib/src/tests/net/crypto_socket/crypto_socket_test.cpp
index 67077f86f1e..e943ba68237 100644
--- a/vespalib/src/tests/net/crypto_socket/crypto_socket_test.cpp
+++ b/vespalib/src/tests/net/crypto_socket/crypto_socket_test.cpp
@@ -191,6 +191,8 @@ void verify_handshake(CryptoSocket &socket) {
         case CryptoSocket::HandshakeResult::NEED_WRITE:
             ASSERT_TRUE(selector.wait_writable());
             break;
+        case CryptoSocket::HandshakeResult::NEED_WORK:
+            socket.do_handshake_work();
         }
     }
 }
diff --git a/vespalib/src/vespa/vespalib/net/crypto_engine.cpp b/vespalib/src/vespa/vespalib/net/crypto_engine.cpp
index bf875a70985..a8808741806 100644
--- a/vespalib/src/vespa/vespalib/net/crypto_engine.cpp
+++ b/vespalib/src/vespa/vespalib/net/crypto_engine.cpp
@@ -44,6 +44,7 @@ public:
     NullCryptoSocket(SocketHandle socket) : _socket(std::move(socket)) {}
     int get_fd() const override { return _socket.get(); }
     HandshakeResult handshake() override { return HandshakeResult::DONE; }
+    void do_handshake_work() override {}
     size_t min_read_buffer_size() const override { return 1; }
     ssize_t read(char *buf, size_t len) override { return _socket.read(buf, len); }
     ssize_t drain(char *, size_t) override { return 0; }
@@ -118,6 +119,7 @@ public:
         }
         return HandshakeResult::DONE;
     }
+    void do_handshake_work() override {}
     size_t min_read_buffer_size() const override { return 1; }
     ssize_t read(char *buf, size_t len) override {
         if (_input.obtain().size == 0) {
diff --git a/vespalib/src/vespa/vespalib/net/crypto_socket.h b/vespalib/src/vespa/vespalib/net/crypto_socket.h
index 3a95cf33add..c409e5a0e83 100644
--- a/vespalib/src/vespa/vespalib/net/crypto_socket.h
+++ b/vespalib/src/vespa/vespalib/net/crypto_socket.h
@@ -26,7 +26,7 @@ struct CryptoSocket {
      **/
     virtual int get_fd() const = 0;
 
-    enum class HandshakeResult { FAIL, DONE, NEED_READ, NEED_WRITE };
+    enum class HandshakeResult { FAIL, DONE, NEED_READ, NEED_WRITE, NEED_WORK };
 
     /**
      * Try to progress the initial connection handshake. Handshaking
@@ -35,11 +35,21 @@ struct CryptoSocket {
      * permitted. This function will be called multiple times until
      * the status is either DONE or FAIL. When NEED_READ or NEED_WRITE
      * is returned, the handshake function will be called again when
-     * the appropriate io event has triggered.
+     * the appropriate io event has triggered. When NEED_WORK is
+     * returned, the 'do_handshake_work' function will be called
+     * exactly once before this function is called again.
      **/
     virtual HandshakeResult handshake() = 0;
 
     /**
+     * This function is called to perform possibly expensive work
+     * needed by the 'handshake' function. The work is done by a
+     * separate function to enable performing it outside the critical
+     * path (transport thread).
+     **/
+    virtual void do_handshake_work() = 0;
+
+    /**
      * This function should be called after handshaking has completed
      * before calling the read function. It dictates the minimum size
      * of the application read buffer presented to the read
diff --git a/vespalib/src/vespa/vespalib/net/sync_crypto_socket.cpp b/vespalib/src/vespa/vespalib/net/sync_crypto_socket.cpp
index e9b1579fbf6..29388035bda 100644
--- a/vespalib/src/vespa/vespalib/net/sync_crypto_socket.cpp
+++ b/vespalib/src/vespa/vespalib/net/sync_crypto_socket.cpp
@@ -94,16 +94,19 @@ SyncCryptoSocket::create(CryptoEngine &engine, SocketHandle socket, bool is_serv
 {
     auto crypto_socket = engine.create_crypto_socket(std::move(socket), is_server);
     set_blocking(crypto_socket->get_fd());
-    auto hs_res = crypto_socket->handshake();
-    while ((hs_res == CryptoSocket::HandshakeResult::NEED_READ) ||
-           (hs_res == CryptoSocket::HandshakeResult::NEED_WRITE))
-    {
-        hs_res = crypto_socket->handshake();
-    }
-    if (hs_res != CryptoSocket::HandshakeResult::DONE) {
-        return std::unique_ptr<SyncCryptoSocket>(nullptr);
+    for (;;) {
+        switch (crypto_socket->handshake()) {
+        case CryptoSocket::HandshakeResult::FAIL:
+            return std::unique_ptr<SyncCryptoSocket>(nullptr);
+        case CryptoSocket::HandshakeResult::DONE:
+            return UP(new SyncCryptoSocket(std::move(crypto_socket)));
+        case CryptoSocket::HandshakeResult::NEED_READ:
+        case CryptoSocket::HandshakeResult::NEED_WRITE:
+            break;
+        case CryptoSocket::HandshakeResult::NEED_WORK:
+            crypto_socket->do_handshake_work();
+        }
     }
-    return UP(new SyncCryptoSocket(std::move(crypto_socket)));
 }
 
 } // namespace vespalib
diff --git a/vespalib/src/vespa/vespalib/net/tls/crypto_codec_adapter.cpp b/vespalib/src/vespa/vespalib/net/tls/crypto_codec_adapter.cpp
index 46c3e2d3195..660eee95132 100644
--- a/vespalib/src/vespa/vespalib/net/tls/crypto_codec_adapter.cpp
+++ b/vespalib/src/vespa/vespalib/net/tls/crypto_codec_adapter.cpp
@@ -72,16 +72,13 @@ CryptoCodecAdapter::handshake()
     for (;;) {
         auto in = _input.obtain();
         auto out = _output.reserve(_codec->min_encode_buffer_size());
-        ::vespalib::net::tls::HandshakeResult hs_res;
-        while ((hs_res = _codec->handshake(in.data, in.size, out.data, out.size)).needs_work()) {
-            _codec->do_handshake_work();
-        }
+        auto hs_res = _codec->handshake(in.data, in.size, out.data, out.size);
         _input.evict(hs_res.bytes_consumed);
         _output.commit(hs_res.bytes_produced);
         switch (hs_res.state) {
         case ::vespalib::net::tls::HandshakeResult::State::Failed: return HandshakeResult::FAIL;
         case ::vespalib::net::tls::HandshakeResult::State::Done: return hs_try_flush();
-        case ::vespalib::net::tls::HandshakeResult::State::NeedsWork: abort(); // Impossible
+        case ::vespalib::net::tls::HandshakeResult::State::NeedsWork: return HandshakeResult::NEED_WORK;
         case ::vespalib::net::tls::HandshakeResult::State::NeedsMorePeerData:
             auto flush_res = hs_try_flush();
             if (flush_res != HandshakeResult::DONE) {
@@ -96,6 +93,12 @@ CryptoCodecAdapter::handshake()
     return HandshakeResult::DONE;
 }
 
+void
+CryptoCodecAdapter::do_handshake_work()
+{
+    _codec->do_handshake_work();
+}
+
 ssize_t
 CryptoCodecAdapter::read(char *buf, size_t len)
 {
diff --git a/vespalib/src/vespa/vespalib/net/tls/crypto_codec_adapter.h b/vespalib/src/vespa/vespalib/net/tls/crypto_codec_adapter.h
index 6af04d67d41..4926c757bfb 100644
--- a/vespalib/src/vespa/vespalib/net/tls/crypto_codec_adapter.h
+++ b/vespalib/src/vespa/vespalib/net/tls/crypto_codec_adapter.h
@@ -37,6 +37,7 @@ public:
     void inject_read_data(const char *buf, size_t len) override;
     int get_fd() const override { return _socket.get(); }
     HandshakeResult handshake() override;
+    void do_handshake_work() override;
     size_t min_read_buffer_size() const override { return _codec->min_decode_buffer_size(); }
     ssize_t read(char *buf, size_t len) override;
     ssize_t drain(char *, size_t) override;
diff --git a/vespalib/src/vespa/vespalib/net/tls/maybe_tls_crypto_socket.cpp b/vespalib/src/vespa/vespalib/net/tls/maybe_tls_crypto_socket.cpp
index b6969250ae5..9af6703acbc 100644
--- a/vespalib/src/vespa/vespalib/net/tls/maybe_tls_crypto_socket.cpp
+++ b/vespalib/src/vespa/vespalib/net/tls/maybe_tls_crypto_socket.cpp
@@ -61,6 +61,7 @@ public:
         }
         return HandshakeResult::DONE;
     }
+    void do_handshake_work() override {}
     size_t min_read_buffer_size() const override { return 1; }
     ssize_t read(char *buf, size_t len) override {
         int drain_result = drain(buf, len);
diff --git a/vespalib/src/vespa/vespalib/net/tls/maybe_tls_crypto_socket.h b/vespalib/src/vespa/vespalib/net/tls/maybe_tls_crypto_socket.h
index bd4ea9d7d50..cd2d84bcc08 100644
--- a/vespalib/src/vespa/vespalib/net/tls/maybe_tls_crypto_socket.h
+++ b/vespalib/src/vespa/vespalib/net/tls/maybe_tls_crypto_socket.h
@@ -25,6 +25,7 @@ public:
     MaybeTlsCryptoSocket(SocketHandle socket, std::shared_ptr<AbstractTlsCryptoEngine> tls_engine);
     int get_fd() const override { return _socket->get_fd(); }
     HandshakeResult handshake() override { return _socket->handshake(); }
+    void do_handshake_work() override { _socket->do_handshake_work(); }
     size_t min_read_buffer_size() const override { return _socket->min_read_buffer_size(); }
     ssize_t read(char *buf, size_t len) override { return _socket->read(buf, len); }
     ssize_t drain(char *buf, size_t len) override { return _socket->drain(buf, len); }
diff --git a/vespalib/src/vespa/vespalib/portal/http_connection.cpp b/vespalib/src/vespa/vespalib/portal/http_connection.cpp
index 2bed10bfd2d..cfee71e4a7d 100644
--- a/vespalib/src/vespa/vespalib/portal/http_connection.cpp
+++ b/vespalib/src/vespa/vespalib/portal/http_connection.cpp
@@ -102,11 +102,14 @@ HttpConnection::set_state(State state, bool read, bool write)
 void
 HttpConnection::do_handshake()
 {
-    switch (_socket->handshake()) {
-    case vespalib::CryptoSocket::HandshakeResult::FAIL:       return set_state(State::NOTIFY,       false, false);
-    case vespalib::CryptoSocket::HandshakeResult::DONE:       return set_state(State::READ_REQUEST,  true, false);
-    case vespalib::CryptoSocket::HandshakeResult::NEED_READ:  return set_state(State::HANDSHAKE,     true, false);
-    case vespalib::CryptoSocket::HandshakeResult::NEED_WRITE: return set_state(State::HANDSHAKE,    false,  true);
+    for (;;) {
+        switch (_socket->handshake()) {
+        case vespalib::CryptoSocket::HandshakeResult::FAIL:       return set_state(State::NOTIFY,       false, false);
+        case vespalib::CryptoSocket::HandshakeResult::DONE:       return set_state(State::READ_REQUEST,  true, false);
+        case vespalib::CryptoSocket::HandshakeResult::NEED_READ:  return set_state(State::HANDSHAKE,     true, false);
+        case vespalib::CryptoSocket::HandshakeResult::NEED_WRITE: return set_state(State::HANDSHAKE,    false,  true);
+        case vespalib::CryptoSocket::HandshakeResult::NEED_WORK:  _socket->do_handshake_work();
+        }
     }
 }