mirror of
https://github.com/jmcorgan/fips.git
synced 2026-07-30 19:46:15 +00:00
Fix Tor onion adverts missing port in Nostr overlay discovery
The Nostr overlay advert publisher serialized `transport: tor` endpoints as a bare `<onion>.onion` hostname with no port. The Tor address parser requires `<host>:<port>` form and rejected the bare shape with `expected host:port`. Any peer receiving a Tor-only advert went into a persistent retry-fail loop on jittered backoff until the advert aged out of the discovery cache. The bug had been latent for as long as Tor adverts have been published on Nostr, and was masked in deployments where every node also advertised a non-Tor transport (peers fell through to the working endpoint). Surfaced first on a deployment where Tor was the only advert path. Publisher now emits `<onion>.onion:<port>` using a new `transports.tor.advertised_port` config field that defaults to 443, matching the Tor `HiddenServicePort 443 127.0.0.1:<bind_port>` convention. Operators whose torrc uses a non-default virtual port can override. Adds a unit test that pins the publisher/parser contract: formats the advert exactly as the publisher does and asserts `parse_tor_addr` accepts the result; asserts the bare-onion form (the bug) does not parse, catching any future regression that drops the port again. Parser is unchanged (already correct).
This commit is contained in:
@@ -259,6 +259,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||||||
|
|
||||||
### Fixed
|
### Fixed
|
||||||
|
|
||||||
|
- Tor onion adverts published over Nostr overlay discovery now
|
||||||
|
include the public-facing port (`<onion>.onion:<port>`) instead of
|
||||||
|
just the bare onion hostname. The publisher previously emitted a
|
||||||
|
bare onion that the parser refused (`expected host:port`),
|
||||||
|
producing a persistent retry-fail loop on any peer whose Tor
|
||||||
|
advert was the only entry in the discovery cache. New
|
||||||
|
`transports.tor.advertised_port` config field (default `443`,
|
||||||
|
matching the Tor `HiddenServicePort` convention) controls the
|
||||||
|
advertised port; operators with non-default virtual ports can
|
||||||
|
override.
|
||||||
- Control socket path detection in fipsctl and fipstop now checks for
|
- Control socket path detection in fipsctl and fipstop now checks for
|
||||||
the `/run/fips/` directory instead of the socket file inside it, so
|
the `/run/fips/` directory instead of the socket file inside it, so
|
||||||
users not yet in the `fips` group get a clear "Permission denied"
|
users not yet in the `fips` group get a clear "Permission denied"
|
||||||
|
|||||||
@@ -467,6 +467,7 @@ Requires an external Tor daemon providing a SOCKS5 proxy. Three modes:
|
|||||||
| `transports.tor.max_inbound_connections` | usize | `64` | Maximum inbound connections via onion service. |
|
| `transports.tor.max_inbound_connections` | usize | `64` | Maximum inbound connections via onion service. |
|
||||||
| `transports.tor.directory_service.hostname_file` | string | `"/var/lib/tor/fips_onion_service/hostname"` | Path to Tor-managed hostname file containing the `.onion` address. |
|
| `transports.tor.directory_service.hostname_file` | string | `"/var/lib/tor/fips_onion_service/hostname"` | Path to Tor-managed hostname file containing the `.onion` address. |
|
||||||
| `transports.tor.directory_service.bind_addr` | string | `"127.0.0.1:8443"` | Local bind address for the listener that Tor forwards inbound connections to. Must match `HiddenServicePort` target in `torrc`. |
|
| `transports.tor.directory_service.bind_addr` | string | `"127.0.0.1:8443"` | Local bind address for the listener that Tor forwards inbound connections to. Must match `HiddenServicePort` target in `torrc`. |
|
||||||
|
| `transports.tor.advertised_port` | u16 | `443` | Public-facing onion port published in Nostr overlay adverts. Must match the virtual port in torrc's `HiddenServicePort <port> 127.0.0.1:<bind_port>` directive — that is the port other peers will use to reach this onion. |
|
||||||
|
|
||||||
**Named instances.** Like other transports, multiple Tor instances can
|
**Named instances.** Like other transports, multiple Tor instances can
|
||||||
be configured with named sub-keys for different SOCKS5 proxy endpoints.
|
be configured with named sub-keys for different SOCKS5 proxy endpoints.
|
||||||
|
|||||||
@@ -441,6 +441,10 @@ const DEFAULT_HOSTNAME_FILE: &str = "/var/lib/tor/fips_onion_service/hostname";
|
|||||||
/// Default directory mode bind address.
|
/// Default directory mode bind address.
|
||||||
const DEFAULT_DIRECTORY_BIND_ADDR: &str = "127.0.0.1:8443";
|
const DEFAULT_DIRECTORY_BIND_ADDR: &str = "127.0.0.1:8443";
|
||||||
|
|
||||||
|
/// Default advertised onion port for Nostr overlay discovery. Matches the
|
||||||
|
/// Tor convention of `HiddenServicePort 443 127.0.0.1:<bind_port>` in torrc.
|
||||||
|
const DEFAULT_TOR_ADVERTISED_PORT: u16 = 443;
|
||||||
|
|
||||||
/// Tor transport instance configuration.
|
/// Tor transport instance configuration.
|
||||||
///
|
///
|
||||||
/// Supports three modes:
|
/// Supports three modes:
|
||||||
@@ -503,6 +507,13 @@ pub struct TorConfig {
|
|||||||
/// Default: false.
|
/// Default: false.
|
||||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||||
pub advertise_on_nostr: Option<bool>,
|
pub advertise_on_nostr: Option<bool>,
|
||||||
|
|
||||||
|
/// Public-facing onion port published in Nostr overlay adverts. Must
|
||||||
|
/// match the virtual port in torrc's `HiddenServicePort <port>
|
||||||
|
/// 127.0.0.1:<bind_port>` directive — that is the port other peers
|
||||||
|
/// will use to reach this onion. Default: 443.
|
||||||
|
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||||
|
pub advertised_port: Option<u16>,
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Directory-mode onion service configuration.
|
/// Directory-mode onion service configuration.
|
||||||
@@ -595,6 +606,12 @@ impl TorConfig {
|
|||||||
pub fn advertise_on_nostr(&self) -> bool {
|
pub fn advertise_on_nostr(&self) -> bool {
|
||||||
self.advertise_on_nostr.unwrap_or(false)
|
self.advertise_on_nostr.unwrap_or(false)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Public-facing onion port published in Nostr overlay adverts.
|
||||||
|
/// Default: 443.
|
||||||
|
pub fn advertised_port(&self) -> u16 {
|
||||||
|
self.advertised_port.unwrap_or(DEFAULT_TOR_ADVERTISED_PORT)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// ============================================================================
|
// ============================================================================
|
||||||
|
|||||||
@@ -1341,7 +1341,7 @@ impl Node {
|
|||||||
if let Some(addr) = handle.onion_address() {
|
if let Some(addr) = handle.onion_address() {
|
||||||
endpoints.push(OverlayEndpointAdvert {
|
endpoints.push(OverlayEndpointAdvert {
|
||||||
transport: OverlayTransportKind::Tor,
|
transport: OverlayTransportKind::Tor,
|
||||||
addr: addr.to_string(),
|
addr: format!("{}:{}", addr, cfg.advertised_port()),
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1446,6 +1446,41 @@ mod tests {
|
|||||||
assert_eq!(config.socks5_addr(), "127.0.0.1:9050");
|
assert_eq!(config.socks5_addr(), "127.0.0.1:9050");
|
||||||
assert_eq!(config.connect_timeout_ms(), 120000);
|
assert_eq!(config.connect_timeout_ms(), 120000);
|
||||||
assert_eq!(config.mtu(), 1400);
|
assert_eq!(config.mtu(), 1400);
|
||||||
|
assert_eq!(config.advertised_port(), 443);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_advertised_port_override() {
|
||||||
|
let config = TorConfig {
|
||||||
|
advertised_port: Some(9001),
|
||||||
|
..Default::default()
|
||||||
|
};
|
||||||
|
assert_eq!(config.advertised_port(), 9001);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Pins the publisher/parser contract for Tor overlay adverts.
|
||||||
|
/// `build_overlay_advert` formats Tor endpoints as `<onion>:<port>`;
|
||||||
|
/// `parse_tor_addr` must accept that exact form back. A bare onion
|
||||||
|
/// (no port) was the production bug — assert it does not parse.
|
||||||
|
#[test]
|
||||||
|
fn test_advert_address_round_trips_through_parser() {
|
||||||
|
let onion = "mwvj6q3pnsiaky7i6wg5s42xlfurt5uqr3qzckrlw2graa2ugcgwhiqd.onion";
|
||||||
|
let cfg = TorConfig::default();
|
||||||
|
let advertised = format!("{}:{}", onion, cfg.advertised_port());
|
||||||
|
|
||||||
|
let parsed = parse_tor_addr(&TransportAddr::from_string(&advertised)).unwrap();
|
||||||
|
match parsed {
|
||||||
|
TorAddr::Onion(host, port) => {
|
||||||
|
assert_eq!(host, onion);
|
||||||
|
assert_eq!(port, 443);
|
||||||
|
}
|
||||||
|
other => panic!("expected Onion variant, got {:?}", other),
|
||||||
|
}
|
||||||
|
|
||||||
|
// Sanity-check the inverse: the bare-onion form (the bug) must
|
||||||
|
// not parse, so any future regression in the publisher will be
|
||||||
|
// caught by the round-trip test above.
|
||||||
|
assert!(parse_tor_addr(&TransportAddr::from_string(onion)).is_err());
|
||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
|
|||||||
Reference in New Issue
Block a user