Libera-Chat / sable

GNU Affero General Public License v3.0
78 stars 8 forks source link

NetworkTaskState should check peer address before accepting TLS handshake #81

Open progval opened 12 months ago

progval commented 12 months ago

It's cheaper to reject spammy connection based on just the IP address instead of accepting the TLS handshake and rejecting. Additionally, it lowers the risk of vulnerabilities in rustls being exploited.

<spb> val: the NetworkTaskState could build a big list of authorised IP addresses and have handle_connection test against that
[...]
<spb> i'm not sure it's a massive priority right now, but might be nice to improve
<spb> though i think if i were doing that, i'd be inclined to rework that a bit more
<val> it's a little annoying because I first need to check in handle_connection() that it's in the allowlist, but read_and_handle_message still needs to check it's the right address for that particular peer
<val> so yeah it needs more rearchitecturing
<spb> that's what i meant by reworking more
<spb> read_and_handle_message isn't really the right place for that because it's also called to handle responses after we send an outbound message
<spb> so it ends up doing all the peer authentication on a connection that we opened ourselves, after we sent network events to it