Enable endpoint identification (aka hostname verification) for TLS clients by default?

[DerGuteMoritz]

Netty currently doesn't enable endpoint identification / hostname verfication for TLS clients by default. This means that a TLS client will happily accept a cryptographically valid certificate presented by a server even if it mentions a different hostname than the client connected to, making it susceptible to a MitM attack. See CWE-297 for more info. This has been flagged as a Netty vulnerability in the past and Aleph of course inherits it. The recently published CVE-2023-4586 appears to be about the same issue.

There is a long-standing Netty issue about enabling it by default but it's still unresolved. However, other libraries built on Netty have opted to do it on their end (e.g. grpc-java and vert.x). We could decide to do the same in Aleph to be secure by default.

Note that this would be a breaking change for cases where users were connecting to misconfigured (or MitM'ed) servers. There might also be test setups which run in a safe environment where users don't care about this. To that end, we should probably provide a flag for restoring the old behavior.

Further note that it is possible to enable endpoint identification in user code via :pipeline-transform.

[KingMob]

For anyone looking for that Vert.x code, it was never merged because the author didn't realize Vert.x already handled it elsewhere: https://github.com/eclipse-vertx/vert.x/pull/3680

[danielcompton]

My vote would be for this to be enabled by default and to clearly signpost it as a possibly breaking change.

IMO, it is acceptable to break compatibility for security fixes where the previous behaviour is unsafe. We were surprised to realise that hostname verification wasn't on by default, and I think most (all?) production users of Aleph would prefer the safe approach.

The scope of this breaking change is fairly narrow: It only affects HTTPS requests to hosts which are serving an otherwise valid certificate for the wrong hostname. It doesn't affect invalid certificates or self-signed certificates which already fail. (https://badssl.com has some good test cases)

One scenario I could imagine this would break is if the user requested https://10.0.0.5/api instead of https://example.com/api, and the web server sent a cert for example.com. This seems fairly unlikely, as most other software would complain very loudly about this.

[DerGuteMoritz]

For anyone looking for that Vert.x code, it was never merged because the author didn't realize Vert.x already handled it elsewhere: eclipse-vertx/vert.x#3680

Well spotted, thanks! I also just realized that the Netty issue I linked to is about changing the default in Netty 5 while we're of course still on Netty 4.

FWIW, I concur with @danielcompton's analysis, so +1 for changing the default.

[KingMob]

OK. Want to fix this now, and cut a minor release? I think the http2 code will be under review for a bit, and this doesn't have to wait for it for any reason.

[danielcompton]

Just an update from production. While in theory this change shouldn't have broken anything, we had an internal web server which was redirecting from HTTP to HTTPS but serving the wrong cert. So when we started verifying the host, these requests started failing.

I still think it's worth making this change, but it might be good to point out that even if you're only making HTTP requests, this change can break things.

[KingMob]

@DerGuteMoritz Do you want to make a PR?

[DerGuteMoritz]

Yup, on it!

[DerGuteMoritz]

Here you go: https://github.com/clj-commons/aleph/pull/689

[KingMob]

I'm a bit busy, and it's the weekend, but I'll cut a minor 0.7.0-alpha2 release next week, probably Wednesday.