iroh-relay: make it possible to disable captive portal #2178
Comments
|
Huh, yeah this is messy. It seems the captive portal can not be disabled at all as setting It seems that netcheck uses the It should be noted that the captive portal check is not really required to work. If it detects a captive portal it's a small optimisation which helps netcheck be a little bit more generic in finding a working connection, but in practice it probably doesn't do that much especially since we still only have 2 relay servers. This could make a little more difference once there are more than 2 relay servers, but even then. I think I agree with your assessment that setting this to a custom port is only really useful for a reverse proxy. But indeed the response is so trivial that you might as well configure the proxy to respond itself directly. It could be made more useful if we allowed a captive portal check on custom ports, maybe with a URL parameter like Anyway, a lot of words because I wanted to write down what I figured out looking at this. We should indeed make it possible to disable the captive portal on the relay server. |
…everse-proxy support (#2341) ## Description This re-architects the relay-server binary. There is now a struct with detailed configuration which runs the entire server and aborts the server on drop. This simplifies running the server in various situations, including tests. The configuration is now done using a declarative struct, which supports more control over how it runs so it can be more easily used behind a reverse proxy, without TLS etc. This is aiming to fix #2177, #2179 and #2178. ## Breaking Changes The configuration file format has changed, deployments will need to updated. For the full format see `struct Config` in `iroh-net/src/bin/iroh-relay.rs`. Here a summary: - The 3 parts of the server now have an independent enable setting: `enable_relay`, `enable_stun` and `enable_metrics`. If omitted they default to `true`. - The way to specify which addresses the server listens on has changed: `http_bind_addr` is for the relay server, `stun_bind_addr` for the STUN server, `metrics_bind_addr` is for the optional metrics server and `tls.https_bind_addr` is for when TLS is enabled. Note these are now all full socket addresses. All have sensible defaults if omitted. - There are new options in `tls.cert_path` and `tls.key_path` which allow more control over where the manual TLS keys are to be read from. - `iroh_net::defaults::DEFAULT_RELAY_STUN_PORT` has been renamed to `iroh_net::defaults::DEFAULT_STUN_PORT`. TBD: some APIs changed as well. Why are they not all private? ## Notes & open questions * The `iroh_net::relay::iroh_relay` crate name is a bit weird. But `iroh_net::relay::server` is already taken. Maybe `iroh_net::relay::bin` could work, but that would be weird when using it from code in other places. * The `ServerConfig` struct is a declarative way of controlling the new server interface. It's kind of nice to use. Bu it is a public API that will be a breaking change every time it changes, and it will change. Maybe it's worth creating a builder API for this. But maybe that's something to only tackle when it is a real demand. I feel like the `iroh_net::relay::server` builders are an attempt at doing this earlier than needed. ## Change checklist - [x] Self-review. - [x] Documentation updates if relevant. - [x] Tests if relevant. - [x] All breaking changes documented.
|
Fixed by #2341. If not feel free to reopen |
…everse-proxy support (n0-computer#2341) ## Description This re-architects the relay-server binary. There is now a struct with detailed configuration which runs the entire server and aborts the server on drop. This simplifies running the server in various situations, including tests. The configuration is now done using a declarative struct, which supports more control over how it runs so it can be more easily used behind a reverse proxy, without TLS etc. This is aiming to fix n0-computer#2177, n0-computer#2179 and n0-computer#2178. ## Breaking Changes The configuration file format has changed, deployments will need to updated. For the full format see `struct Config` in `iroh-net/src/bin/iroh-relay.rs`. Here a summary: - The 3 parts of the server now have an independent enable setting: `enable_relay`, `enable_stun` and `enable_metrics`. If omitted they default to `true`. - The way to specify which addresses the server listens on has changed: `http_bind_addr` is for the relay server, `stun_bind_addr` for the STUN server, `metrics_bind_addr` is for the optional metrics server and `tls.https_bind_addr` is for when TLS is enabled. Note these are now all full socket addresses. All have sensible defaults if omitted. - There are new options in `tls.cert_path` and `tls.key_path` which allow more control over where the manual TLS keys are to be read from. - `iroh_net::defaults::DEFAULT_RELAY_STUN_PORT` has been renamed to `iroh_net::defaults::DEFAULT_STUN_PORT`. TBD: some APIs changed as well. Why are they not all private? ## Notes & open questions * The `iroh_net::relay::iroh_relay` crate name is a bit weird. But `iroh_net::relay::server` is already taken. Maybe `iroh_net::relay::bin` could work, but that would be weird when using it from code in other places. * The `ServerConfig` struct is a declarative way of controlling the new server interface. It's kind of nice to use. Bu it is a public API that will be a breaking change every time it changes, and it will change. Maybe it's worth creating a builder API for this. But maybe that's something to only tackle when it is a real demand. I feel like the `iroh_net::relay::server` builders are an attempt at doing this earlier than needed. ## Change checklist - [x] Self-review. - [x] Documentation updates if relevant. - [x] Tests if relevant. - [x] All breaking changes documented.
There is an option
captive_portal_portbut in my case HTTP port is already taken byacmetool redirectorservice:iroh/iroh-net/src/bin/iroh-relay.rs
Line 228 in b07547b
Having captive portal on any port other than 80 does not really make sense if I understand correctly its purpose (maybe to put it behind reverse proxy, but then I can configure nginx or whatever reverse proxy to respond however I want myself), so would be better if there was an option to just disable it. As a workaround I have moved it to an arbitrary port.
The text was updated successfully, but these errors were encountered: