From 63d4acdce30032c4dcb32ed205fabf0a4e4e5a0a Mon Sep 17 00:00:00 2001 From: Michael van der Werve Date: Tue, 8 Jan 2019 12:58:38 +0100 Subject: [PATCH 1/2] should be in working order now --- src/linux_tcp/openssl.cpp | 17 ++++++++ src/linux_tcp/openssl.h | 1 + src/linux_tcp/sslconnected.h | 80 ++++++++++++++++++++++++++++-------- src/linux_tcp/sslcontext.h | 4 ++ src/linux_tcp/tcpoutbuffer.h | 3 ++ 5 files changed, 87 insertions(+), 18 deletions(-) diff --git a/src/linux_tcp/openssl.cpp b/src/linux_tcp/openssl.cpp index 18be0cd..f0c0c8d 100644 --- a/src/linux_tcp/openssl.cpp +++ b/src/linux_tcp/openssl.cpp @@ -310,6 +310,23 @@ int SSL_use_certificate_file(SSL *ssl, const char *file, int type) return func(ssl, file, type); } +/** + * Control the SSL context + * @param ctx + * @param cmd + * @param larg + * @param parg + * @return long + */ +long SSL_CTX_ctrl(SSL_CTX *ctx, int cmd, long larg, void *parg) +{ + // create a function + static Function func(handle, "SSL_CTX_ctrl"); + + // call the openssl function + return func(ctx, cmd, larg, parg); +} + /** * Clear the SSL error queue * @return void diff --git a/src/linux_tcp/openssl.h b/src/linux_tcp/openssl.h index c8d7694..4614b66 100644 --- a/src/linux_tcp/openssl.h +++ b/src/linux_tcp/openssl.h @@ -50,6 +50,7 @@ void SSL_set_connect_state(SSL *ssl); void SSL_CTX_free(SSL_CTX *ctx); void SSL_free(SSL *ssl); long SSL_ctrl(SSL *ssl, int cmd, long larg, void *parg); +long SSL_CTX_ctrl(SSL_CTX *ctx, int cmd, long larg, void *parg); void ERR_clear_error(void); /** diff --git a/src/linux_tcp/sslconnected.h b/src/linux_tcp/sslconnected.h index de36f00..8bff666 100644 --- a/src/linux_tcp/sslconnected.h +++ b/src/linux_tcp/sslconnected.h @@ -56,7 +56,8 @@ private: enum State { state_idle, state_sending, - state_receiving + state_receiving, + state_error } _state; /** @@ -101,13 +102,35 @@ private: } /** - * Method to repeat the previous call\ + * Method to repeat the previous call * @param monitor monitor to check if connection object still exists * @param state the state that we were in * @param result result of an earlier SSL_get_error call * @return TcpState* */ TcpState *repeat(const Monitor &monitor, enum State state, int error) + { + // if we are able to repeat the call, we are still in the correct state + if (repeat(state, error)) + { + // if the socket was closed in the meantime and we are not sending anything any more, we should initialize the shutdown sequence + if (_closed && _state == state_idle) return new SslShutdown(this, std::move(_ssl)); + + // otherwise, we just continue as we were, since the calls should be repeated in the future + else return this; + } + + // if the monitor is still valid, we should tear down the connection because we are unable to repeat the call + return monitor.valid() ? new TcpClosed(this) : nullptr; + } + + /** + * Method to repeat the previous call, which has then been + * @param state the state that we were in + * @param result result of an earlier SSL_get_error call + * @return bool + */ + bool repeat(enum State state, int error) { // check the error switch (error) { @@ -119,7 +142,7 @@ private: _parent->onIdle(this, _socket, readable); // allow chaining - return this; + return true; case SSL_ERROR_WANT_WRITE: // remember state @@ -128,22 +151,30 @@ private: // wait until socket becomes writable again _parent->onIdle(this, _socket, readable | writable); - // allow chaining - return this; + // we are done + return true; + // this case doesn't actually happen when repeat is called, since it will only be returned when + // the result > 0 and therefore there is no error. it is here just to be sure. case SSL_ERROR_NONE: // we're ready for the next instruction from userspace _state = state_idle; - // if already closed, proceed to next state - return proceed(); + // if we still have an outgoing buffer we want to send out data, otherwise we just read + _parent->onIdle(this, _socket, _out ? readable | writable : readable); + + // nothing is wrong, we are done + return true; default: + // we are now in an error state + _state = state_error; + // report an error to user-space _parent->onError(this, "ssl protocol error"); // ssl level error, we have to tear down the tcp connection - return monitor.valid() ? new TcpClosed(this) : nullptr; + return false; } } @@ -220,10 +251,6 @@ private: // assume default state _state = state_idle; - // we are going to check for errors after the openssl operations, so we make - // sure that the error queue is currently completely empty - OpenSSL::ERR_clear_error(); - // because the output buffer contains a lot of small buffers, we can do multiple // operations till the buffer is empty (but only if the socket is not also // readable, because then we want to read that data first instead of endless writes @@ -329,6 +356,9 @@ public: // same is true for read operations, they should also be repeated if (_state == state_receiving) return receive(monitor); + + // if we are in an error state, we close the tcp connection + if (_state == state_error) return new TcpClosed(this); // if the socket is readable, we are going to receive data if (flags & readable) return receive(monitor); @@ -352,15 +382,29 @@ public: // do nothing if already busy closing if (_closed) return; + // if we're not idle, we can just add bytes to the buffer and we're done + if (_state != state_idle) return _out.add(buffer, size); + + // clear ssl-level error + OpenSSL::ERR_clear_error(); + + // get the result + int result = OpenSSL::SSL_write(_ssl, buffer, size); + + // if the result is larger than zero, we are successful + if (result > 0) return; + + // check for error + auto error = OpenSSL::SSL_get_error(_ssl, result); + // put the data in the outgoing buffer _out.add(buffer, size); - // if we're already busy with sending or receiving, we first have to wait - // for that operation to complete before we can move on - if (_state != state_idle) return; - - // let's wait until the socket becomes writable - _parent->onIdle(this, _socket, readable | writable); + // the operation failed, we may have to repeat our call. this may detect that + // ssl is in an error state, however that is ok because it will set an internal + // state to the error state so that on the next calls to state-changing objects, + // the tcp socket will be torn down + return (void) repeat(state_sending, error); } /** diff --git a/src/linux_tcp/sslcontext.h b/src/linux_tcp/sslcontext.h index ab9dbc7..cfd628a 100644 --- a/src/linux_tcp/sslcontext.h +++ b/src/linux_tcp/sslcontext.h @@ -39,6 +39,10 @@ public: { // report error if (_ctx == nullptr) throw std::runtime_error("failed to construct ssl context"); + + // set the context to accept a moving write buffer. note that SSL_CTX_set_mode is a macro + // that expands to SSL_CTX_ctrl, so that is the real function that is used + OpenSSL::SSL_CTX_set_mode(_ctx, SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER); } /** diff --git a/src/linux_tcp/tcpoutbuffer.h b/src/linux_tcp/tcpoutbuffer.h index ec6a1d9..b3a8728 100644 --- a/src/linux_tcp/tcpoutbuffer.h +++ b/src/linux_tcp/tcpoutbuffer.h @@ -296,6 +296,9 @@ public: // just to be sure we do this check if (buffers == 0) return 0; + // make sure that the error queue is currently completely empty, so the error queue can be checked + OpenSSL::ERR_clear_error(); + // send the data auto result = OpenSSL::SSL_write(ssl, buffer[0].iov_base, buffer[0].iov_len); From a774e6c10ced8a8852c247555c22f5a30c702b40 Mon Sep 17 00:00:00 2001 From: Michael van der Werve Date: Tue, 8 Jan 2019 13:14:42 +0100 Subject: [PATCH 2/2] on ssl error, make sure that it is found out --- src/linux_tcp/sslconnected.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/linux_tcp/sslconnected.h b/src/linux_tcp/sslconnected.h index 8bff666..30f22e0 100644 --- a/src/linux_tcp/sslconnected.h +++ b/src/linux_tcp/sslconnected.h @@ -404,7 +404,10 @@ public: // ssl is in an error state, however that is ok because it will set an internal // state to the error state so that on the next calls to state-changing objects, // the tcp socket will be torn down - return (void) repeat(state_sending, error); + if (repeat(state_sending, error)) return; + + // the repeat call failed, so we are going to find out with a readable file descriptor + _parent->onIdle(this, _socket, readable); } /**