| Message ID | 20230517165105.2986692-2-i.maximets@ovn.org |
|---|---|
| State | Accepted |
| Commit | 0826de990cd2c3d55b5f8ea4da74416cc3d40a22 |
| Headers | show |
| Series | ssl: Ignore unexpected EOF + ovsdb log checking in tests. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On Wed, May 17, 2023 at 06:51:04PM +0200, Ilya Maximets wrote: > OpenSSL 3.0 enabled alerts for unexpected EOF by default. It supposed > to alert the application whenever the connection terminated without > a proper close_notify. And that should allow applications to take > actions to protect themselves from potential TLS truncation attack. > This is how it looks like in the log: > > |stream_ssl|WARN|SSL_read: error:0A000126:SSL routines::unexpected eof while reading > |jsonrpc|WARN|ssl:127.0.0.1:34288: receive error: Input/output error > |reconnect|WARN|ssl:127.0.0.1:34288: connection dropped (Input/output error) > > The problem is that clients based on OVS libraries do not wait for > the proper termination if it didn't happen right away. It means that > chances to have alerts on the server side for every single disconnection > are very high. > > None of the high level protocols supported by OVS daemons can carry > state between re-connections, e.g., there are no session cookies or > anything like that. So, the TLS truncation attack is no applicable. > > Disable the alert to avoid unnecessary warnings in the log. > > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Reviewed-by: Simon Horman <simon.horman@corigine.com> Are there any plans to enhance the client-side behaviour?
On 5/25/23 15:21, Simon Horman wrote: > On Wed, May 17, 2023 at 06:51:04PM +0200, Ilya Maximets wrote: >> OpenSSL 3.0 enabled alerts for unexpected EOF by default. It supposed >> to alert the application whenever the connection terminated without >> a proper close_notify. And that should allow applications to take >> actions to protect themselves from potential TLS truncation attack. >> This is how it looks like in the log: >> >> |stream_ssl|WARN|SSL_read: error:0A000126:SSL routines::unexpected eof while reading >> |jsonrpc|WARN|ssl:127.0.0.1:34288: receive error: Input/output error >> |reconnect|WARN|ssl:127.0.0.1:34288: connection dropped (Input/output error) >> >> The problem is that clients based on OVS libraries do not wait for >> the proper termination if it didn't happen right away. It means that >> chances to have alerts on the server side for every single disconnection >> are very high. >> >> None of the high level protocols supported by OVS daemons can carry >> state between re-connections, e.g., there are no session cookies or >> anything like that. So, the TLS truncation attack is no applicable. >> >> Disable the alert to avoid unnecessary warnings in the log. >> >> Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > > Reviewed-by: Simon Horman <simon.horman@corigine.com> Thanks for review! I applied this one patch for now. Backported down to 2.17 since it's an issue with support for OpenSSL 3.0+. > Are there any plans to enhance the client-side behaviour? It's a bit unclear how to do. SSL_shutdown() may fail with either WANT_READ or WANT_WRITE. On a nonblocking socket that means that the application is expected to poll for the file descriptor to become readable or writable. That means that we either block in the stream_close(), which may take unpredictable amount of time, or we need to fail the stream_close() with EAGAIN. At this point we will have the same dilemma for a library that calls stream_close(). Currently, we're just force-closing the socket instead. Best regards, Ilya Maximets.
On Fri, May 26, 2023 at 06:57:09PM +0200, Ilya Maximets wrote: > On 5/25/23 15:21, Simon Horman wrote: > > On Wed, May 17, 2023 at 06:51:04PM +0200, Ilya Maximets wrote: > >> OpenSSL 3.0 enabled alerts for unexpected EOF by default. It supposed > >> to alert the application whenever the connection terminated without > >> a proper close_notify. And that should allow applications to take > >> actions to protect themselves from potential TLS truncation attack. > >> This is how it looks like in the log: > >> > >> |stream_ssl|WARN|SSL_read: error:0A000126:SSL routines::unexpected eof while reading > >> |jsonrpc|WARN|ssl:127.0.0.1:34288: receive error: Input/output error > >> |reconnect|WARN|ssl:127.0.0.1:34288: connection dropped (Input/output error) > >> > >> The problem is that clients based on OVS libraries do not wait for > >> the proper termination if it didn't happen right away. It means that > >> chances to have alerts on the server side for every single disconnection > >> are very high. > >> > >> None of the high level protocols supported by OVS daemons can carry > >> state between re-connections, e.g., there are no session cookies or > >> anything like that. So, the TLS truncation attack is no applicable. > >> > >> Disable the alert to avoid unnecessary warnings in the log. > >> > >> Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > > > > Reviewed-by: Simon Horman <simon.horman@corigine.com> > > Thanks for review! I applied this one patch for now. > Backported down to 2.17 since it's an issue with support for OpenSSL 3.0+. > > > Are there any plans to enhance the client-side behaviour? > > It's a bit unclear how to do. SSL_shutdown() may fail with either > WANT_READ or WANT_WRITE. On a nonblocking socket that means that > the application is expected to poll for the file descriptor to become > readable or writable. That means that we either block in the > stream_close(), which may take unpredictable amount of time, or we > need to fail the stream_close() with EAGAIN. At this point we will > have the same dilemma for a library that calls stream_close(). > Currently, we're just force-closing the socket instead. Thanks, understood.
diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c index 62da9febb..86747e58b 100644 --- a/lib/stream-ssl.c +++ b/lib/stream-ssl.c @@ -1075,7 +1075,13 @@ do_ssl_init(void) VLOG_ERR("SSL_CTX_new: %s", ERR_error_string(ERR_get_error(), NULL)); return ENOPROTOOPT; } - SSL_CTX_set_options(ctx, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3); + + long options = SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3; +#ifdef SSL_OP_IGNORE_UNEXPECTED_EOF + options |= SSL_OP_IGNORE_UNEXPECTED_EOF; +#endif + SSL_CTX_set_options(ctx, options); + #if OPENSSL_VERSION_NUMBER < 0x3000000fL SSL_CTX_set_tmp_dh_callback(ctx, tmp_dh_callback); #else
OpenSSL 3.0 enabled alerts for unexpected EOF by default. It supposed to alert the application whenever the connection terminated without a proper close_notify. And that should allow applications to take actions to protect themselves from potential TLS truncation attack. This is how it looks like in the log: |stream_ssl|WARN|SSL_read: error:0A000126:SSL routines::unexpected eof while reading |jsonrpc|WARN|ssl:127.0.0.1:34288: receive error: Input/output error |reconnect|WARN|ssl:127.0.0.1:34288: connection dropped (Input/output error) The problem is that clients based on OVS libraries do not wait for the proper termination if it didn't happen right away. It means that chances to have alerts on the server side for every single disconnection are very high. None of the high level protocols supported by OVS daemons can carry state between re-connections, e.g., there are no session cookies or anything like that. So, the TLS truncation attack is no applicable. Disable the alert to avoid unnecessary warnings in the log. Signed-off-by: Ilya Maximets <i.maximets@ovn.org> --- lib/stream-ssl.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)