Set TLS minimum proto version to 1.2 or greater.

[MMChrisHinshaw]

One of our security team ran some tests and found that the Zeek broker ports were TLS 1.0 and 1.1. After digging into the broker code (and falling for a red herring in the libcaf_openssl source), it was found that the minimum TLS version is not applied to the SSL context. If I am understanding the code properly, the right place to apply the limit would be in ./src/internal/connector.cc at around line 209. NOTE: The patch file below is from the broker associated with Zeek 5.2.2, not the latest.

diff --git a/src/internal/connector.cc b/src/internal/connector.cc
index 1e9ae0d0..92f40be5 100644
--- a/src/internal/connector.cc
+++ b/src/internal/connector.cc
@@ -205,6 +205,7 @@ ssl_context_from_cfg(const openssl_options_ptr& cfg) {
 #endif
   auto ctx = caf::net::openssl::make_ctx(method);
   BROKER_DEBUG(BROKER_ARG2("authentication", cfg->authentication_enabled()));
+  SSL_CTX_set_min_proto_version(ctx.get(), TLS1_2_VERSION);
   if (cfg->authentication_enabled()) {
     // Require valid certificates on both sides.
     ERR_clear_error();

It would be great if this could be a configurable item (if it is not already), but the above change would satisfy our security team requirements.

[MMChrisHinshaw]

Hmmm... The above change didn't actually stop TLS 1.0 or 1.1 connections. Below is a simplified test (note that ZeekPort is set to 7760 in zeekctl.cfg).

#!/bin/bash

for PROTO in tls1 tls1_1 tls1_2 tls1_3 ; do
    echo "Testing $PROTO"
    openssl s_client -connect 0.0.0.0:7761 -$PROTO 2>/dev/null | grep Protocol
    echo "Result $?"
done

The output shows that TLS 1.0 and 1.1 connections are valid, even with the above code change to broker.

Testing tls1
    Protocol  : TLSv1
Result 0
Testing tls1_1
    Protocol  : TLSv1.1
Result 0
Testing tls1_2
    Protocol  : TLSv1.2
Result 0
Testing tls1_3
Result 1

[Neverlord]

Hmmm... The above change didn't actually stop TLS 1.0 or 1.1 connections.

That change only affects the zeek-to-zeek (binary) connections. I'll look into the WS part.

@timwoj, @ckreibich: do you guys see a reason to make this configurable or can we simply put TLS 1.2 as the minimum (for both native and WebSocket)?

[ckreibich]

Sorry to let this sit for so long. I don't see a reason why we wouldn't just go to at least 1.2 at this point.

[Neverlord]

@MMChrisHinshaw your patch actually should affect both binary and WebSocket connections.

This is the output for me locally:

$ bash test.bash
Testing tls1
Result 1
Testing tls1_1
Result 1
Testing tls1_2
    Protocol  : TLSv1.2
Result 0
Testing tls1_3
Result 1

However, it doesn't matter whether I apply your patch or not. Looks like my OpenSSL version reject older protocols by default. What system did you test on?

[MMChrisHinshaw]

I want to say this was RHEL8, but am not 100% on that. What I'm also unsure of is whether the system minimum was set to 1.2.

In either case, it would be great to have a config option in Zeek to be able to control.

[Neverlord]

I've merged the PR, so TLS 1.2 should be enforced as minimum going forward. However, I could not verify this. The CentOS 8 docker image seems to be dead/EOL, so I couldn't use that for validation. CentOS steram 9 and other distros I've tried all set the OpenSSL minimum to 1.2 at the system level, so the patch didn't make a difference.