refraction-networking / conjure

Conjure Refraction Networking station code
https://refraction.network
Apache License 2.0
70 stars 21 forks source link

Port randomization fixes #239

Closed jmwample closed 1 year ago

jmwample commented 1 year ago

There is an issue wrt. destination port randomization that conflates subtly different booleans controlling whether port randomization is enabled for a given connection.

https://github.com/refraction-networking/gotapdance/pull/147

jmwample commented 1 year ago

Sessions using bidirectional registrars do have consistency between the client and the station wrt port randomization as regprocessor.go -- which handles the intake of bidirectional registrations, and generating the RegistrationResponse that is both sent to the client and attached to the registration forwarded to the -- handles port randomization properly. When a bidirectional registrar is used the stations use the port in the attached registration response which the client is supposed to use as well.

When picking the destination port the regprocesser uses the station side transport interface and provides the transport parameters which is where the client indicates whether port randomization is enabled or not.

jmwample commented 1 year ago

It turns out that making a copy of the params shared on init was not enough for item 3 because subsequent dials would still overwrite parameters within the Transport object if it is re-used, which we allow it to be.

The real solution was to add a public Parameters that is set by the user, which should be treated as immutable, unless it is explicitly set again using SetParams(). And then use an mutable internal parameter object that deep clones the user parameters on each dial. In this way internal state only persists for one session and each dial will start with consistent parameters matching caller expectations.

For things like bidirectional registration where it is necessary to overwrite some parameters within the session the function SetSessionParams() is used.

jmwample commented 1 year ago

In general I think this maybe motivates a different interface for Transports more similar to Tor's pluggable transports where you have a Transport, but then when dial is called a function like transport.ClientFactory() is called to build a new instance of the implemented protocol with/from the original immutable config.

jmwample commented 1 year ago

The 4th todo is not directly related to these fixes and should be handled separately as it is not required to get the fixed port randomization rolled. See #240