Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate certificate-less TLS for broker #3852

Open
simeonmiteff opened this issue Jul 26, 2024 · 9 comments
Open

Deprecate certificate-less TLS for broker #3852

simeonmiteff opened this issue Jul 26, 2024 · 9 comments

Comments

@simeonmiteff
Copy link

This is one of those "no harm in asking" situations...

I find the weird certificate-less/encryption-only/no-verification TLS mode for broker problematic to support in https://github.com/corelight/go-zeek-broker-ws because it ties me to use gorilla for websockets (which is near-abandonware) so that I can pass it a connection negotiated via openssl using Go openssl bindings that are also abandonware.

The reason why I need openssl to support it is because the Go native TLS stack doesn't (and won't ever) support the combination of cyphers needed for this weird mode. It may be because there are security issues with that combination (if it helps, I could look into that).

Given there are two other modes (no TLS, and normal TLS), what are the chances we could deprecate the weird TLS mode in a future zeek version?

Do we know if the community really values having encryption-only/no-verification TLS between nodes?

@simeonmiteff
Copy link
Author

I went looking for the Go team's "we won't support this" statement for what zeek uses here (TLS_ECDH_anon_WITH_AES_256_CBC_SHA) and I couldn't find it. I imagine the reason was any combination of:

  1. The anonymous ECDH is vulnerable to man in the middle attacks
  2. AEDHC doesn't have perfect forward secrecy
  3. SHA-1 is insecure
  4. CBC is also vulnerable in some cases

@rsmmr
Copy link
Member

rsmmr commented Jul 30, 2024

Admittedly this is a pretty unusual mode, I think many people just don't even know about it (that certainly applied to me originally). That said, can you elaborate why having Zeek support this mode is a problem for your bindings? I understand that you can't support it easily but how is Zeek offering that support (which is essentially opt-in to use) making a difference there? I'd expect your bindings to just require certificates one way or the other. Am I missing something?

@simeonmiteff
Copy link
Author

simeonmiteff commented Jul 31, 2024

@rsmmr AFAIK it is opt-out, not opt-in (this is the default mode for zeek clusters) and my desire is for the Go library is to support the default. Another frustration is that if you get it wrong (i.e., try to connect with TLS disabled in the client when it is enabled in zeek) the connection just hangs without errors.

Other than deprecating the AECDH mode (which I know is a big ask), another idea might be to have separate defaults for broker native protocol connections and the websocket API and to drop AECDH support for websockets. In this proposal the default for websockets would be one of the other two options: no TLS, or authenticated TLS (I don't mind either way as long as it is documented).

I would suggest that dropping AECDH mode for the websocket API aligns with my understanding of the intent of this API: to provide a convenient and modern way to exchange events with zeek from external processes that is language and TLS-stack agnostic.

@rsmmr
Copy link
Member

rsmmr commented Jul 31, 2024

Maybe "opt-in" was the wrong word. You're right that the default is cert-less, but only until certificates get configured on the Zeek side; once they are, cert-less gets disabled. And my point was that if we dropped cert-less, one would now always need to configure certs anyways, meaning that nothing changes from that perspective no matter whether Zeek offers the feature or not. In other words, if the Go bindings don't support cert-less, Zeek-side configuration remains the same either way.

However, I do see the issue of just hanging without errors; that certainly isn't great for a client not supporting cert-less. I don't know if there might be a way to fix on our end so that we could abort the connection in a way that the client can understand what's going on. If so, that would be my preferred approach.

I'll leave it to others to decide if want to change defaults or deprecate it altogether, I just wanted to understand the context where you're running into problems with the current approach.

@ckreibich
Copy link
Member

ckreibich commented Aug 27, 2024

I have also bumped into the inconvenience of using the current cert-less mode, both as a developer of a client (where I have to go out of my way to tell OpenSSL to use the needed cipher) and as a user (where I noticed that pointing common WebSocket clients at Zeek via wss:// URLs doesn't work, again because OpenSSL won't be using the needed cipher by default).

That aside, two other reasons have me reluctant about maintaining cert-less much longer: one, TLS 1.3 doesn't support anonymous key exchanges, so we'll need to use certs there anyway. Two, these days we often argue that the vast majority of users gets by with single-machine clusters (or conceptually similar contained environments). I don't think the unauthenticated encryption adds much there, since an attacker with access to such environments and its traffic likely already has the ability to inflict damage on Zeek beyond MITM. For users with multi-machine clusters I am curious — I am not sure to which degree folks use signed certs. I'll make sure to ask around in the upcoming testing subgroup calls, but it does not strike me as unreasonable to mandate the use of certs in those settings going forward.

One aspect that makes this unique is that in most settings "default Broker" vs "WebSocket Broker" address pretty different use cases (the former being used for the cluster itself, the latter so far exclusively for integrating third-party services). It would be handy to be able to reason about their crypto postures separately.

Things like NATS and Redis are also unencrypted by default, with TLS an opt-in.

All in all I'm pretty supportive of toggling the default to operating unencrypted, with the option of switching to cert-supported TLS. Thoughts?

@JustinAzoff
Copy link
Contributor

For people using things like zeekctl, zeekctl could generate a cluster CA, generate and sign certs for each cluster process, and install them in the right places on each node. That would be zero-config like the cert-less mode, but more secure.

I suppose it could also use a single self signed cert for all the processes on all the hosts, if they were all equally trusted.

@ckreibich
Copy link
Member

Yep, that was @0xxon's proposal too. I'm somewhat reluctant to add zeekctl-only functionality at this point, but it would be convenient.

@JustinAzoff
Copy link
Contributor

I was mostly thinking of what Robin said:

the default is cert-less, but only until certificates get configured on the Zeek side

In that case it really wouldn't be zeekctl only functionality, it would be a zeek feature that zeekctl helps you configure. I suppose the new zeek-client could also help bootstrap the tls certs.

@ckreibich
Copy link
Member

Yep. I think we're on the same page. I think the main question remains whether it's okay to go TLS-less by default, and I think your points align with "yes"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants