mirror of
https://github.com/Such-Software/smirk-backend-core.git
synced 2026-10-17 07:21:32 +02:00
fix(relay): harden admission (fail-safe, loopback-only, bounded, size cap) [review]
Adversarial review of the relay admission surface: - FAIL-SAFE on admission death: nostr-rs-relay fails OPEN if its gRPC admission server is unreachable, so a dead admission = an unrestricted relay. The backend now EXITS if the admission service stops (supervisor restarts a clean pair); documented in RELAY.md. Pin the relay image to a digest (compose note). - Registration-oracle protection: validate() now refuses a non-loopback RELAY_ADMISSION_BIND unless RELAY_ADMISSION_ALLOW_PUBLIC=true (+ test). - No N+1 on crafted events: recipient p-tags are capped (20, deduped) and resolved in ONE batched query (db.any_registered_npub) instead of one-per-tag. - Defence-in-depth size cap: admission denies events over max_event_bytes before any DB work (the relay enforces its own limit too). - Docs: RELAY.md Security section (fail-open, loopback oracle, default PoW=0 spam footgun, client sender-verification); config.toml relay port 8088 (not 8080); Docker Hub image; .env.example RELAY_ADMISSION_ALLOW_PUBLIC. Note: the client-side sender-impersonation blocker is fixed separately in the monorepo (verifying gift-wrap unwrap).
This commit is contained in:
+5
-1
@@ -154,8 +154,12 @@ RELAY_WRITE_POLICY=inbox-outbox
|
||||
# inbound events; 0 disables the PoW gate.
|
||||
RELAY_INBOUND_POW_BITS=0
|
||||
# Loopback address the gRPC event-admission service binds (the relay calls it).
|
||||
# Keep it loopback; the bundled compose reaches it via the host network.
|
||||
# It answers "is this npub registered?" (a registration oracle) so it MUST stay
|
||||
# loopback; the bundled compose reaches it via the host network.
|
||||
RELAY_ADMISSION_BIND=127.0.0.1:8090
|
||||
# Allow a NON-loopback admission bind (advanced; e.g. a relay container without
|
||||
# host networking). Off by default; you must firewall it if you enable it.
|
||||
RELAY_ADMISSION_ALLOW_PUBLIC=false
|
||||
RELAY_MAX_EVENT_BYTES=65536
|
||||
RELAY_RETENTION_DAYS=30
|
||||
|
||||
|
||||
@@ -27,7 +27,8 @@ event_admission_server = "http://127.0.0.1:8090"
|
||||
|
||||
[network]
|
||||
address = "0.0.0.0"
|
||||
port = 8080
|
||||
# NOT 8080 — the Smirk backend uses that. Front this with your TLS terminator.
|
||||
port = 8088
|
||||
|
||||
# Conservative, self-host-friendly resource ceilings (the anti-abuse floor even
|
||||
# for the `open` policy). Tune to your box.
|
||||
|
||||
@@ -16,7 +16,9 @@
|
||||
# version you pin (https://github.com/scsibug/nostr-rs-relay).
|
||||
services:
|
||||
relay:
|
||||
image: ghcr.io/scsibug/nostr-rs-relay:latest # pin a digest in production
|
||||
# SECURITY: pin a digest in production — the relay's admission fail-open
|
||||
# behavior can change across versions (see docs/operations/RELAY.md).
|
||||
image: scsibug/nostr-rs-relay:latest
|
||||
restart: unless-stopped
|
||||
network_mode: host # reach the backend admission socket on 127.0.0.1
|
||||
user: "1000:1000"
|
||||
|
||||
@@ -78,6 +78,28 @@ author allowlist. Only the `inbox-outbox` external-inbound branch is gated.
|
||||
event → rejected (`inbox-outbox`); an under-PoW cross-ecosystem event →
|
||||
rejected (when `RELAY_INBOUND_POW_BITS > 0`).
|
||||
|
||||
## Security notes (read before exposing a public relay)
|
||||
|
||||
- **The relay fails OPEN if admission is unreachable.** `nostr-rs-relay` treats an
|
||||
unreachable gRPC admission server as PERMIT. So a down/crashed admission service
|
||||
= an unrestricted relay. Two defences: (1) the backend **exits** if its admission
|
||||
service stops (so your supervisor restarts a clean pair — don't run them
|
||||
independently), and (2) run the relay + backend as one supervised unit so the
|
||||
relay isn't serving while admission is down. Pin the relay image to a **digest**,
|
||||
not `:latest` — the fail-open semantics can change across versions.
|
||||
- **The admission socket is a registration oracle** (it answers "is this npub
|
||||
registered?"). It MUST stay loopback; `validate()` refuses a non-loopback
|
||||
`RELAY_ADMISSION_BIND` unless you set `RELAY_ADMISSION_ALLOW_PUBLIC=true` and
|
||||
firewall it yourself.
|
||||
- **Default `RELAY_INBOUND_POW_BITS=0` has no spam friction.** Under `inbox-outbox`
|
||||
a `p` tag is public, so anyone can address gift-wraps to a registered user's
|
||||
inbox. A PUBLIC operator should raise the PoW bits (e.g. 8–16) to blunt inbox
|
||||
flooding; content is always encrypted, so this is a storage/spam concern, not a
|
||||
confidentiality one.
|
||||
- **Clients verify the sender.** A gift-wrap's inner author is only trusted after
|
||||
the wallet checks the seal signature + `seal.pubkey == rumor.pubkey`, so a
|
||||
relay/sender cannot forge the displayed "from".
|
||||
|
||||
## Modularity
|
||||
|
||||
`nostr-rs-relay` is the first adapter behind the `RelayProvider` seam
|
||||
|
||||
+43
-1
@@ -537,8 +537,13 @@ pub struct RelayConfig {
|
||||
/// author) inbound events; `0` disables the PoW gate.
|
||||
pub inbound_pow_bits: u8,
|
||||
/// Loopback address the gRPC event-admission service binds (nostr-rs-relay
|
||||
/// calls it per event). Only used when `write_policy != open`.
|
||||
/// calls it per event). Only used when `write_policy != open`. It answers
|
||||
/// "is this npub registered?", so it MUST stay loopback (a registration
|
||||
/// oracle) unless explicitly opted out.
|
||||
pub admission_bind: String,
|
||||
/// Allow a NON-loopback `admission_bind` (e.g. a relay in a container without
|
||||
/// host networking). Off by default; the operator must firewall it.
|
||||
pub admission_allow_public: bool,
|
||||
/// Reject events larger than this many bytes (the relay enforces it too).
|
||||
pub max_event_bytes: usize,
|
||||
/// Event retention (days) — advisory for operator housekeeping/advertising.
|
||||
@@ -738,6 +743,7 @@ impl Config {
|
||||
write_policy: env_or("RELAY_WRITE_POLICY", "inbox-outbox").to_lowercase(),
|
||||
inbound_pow_bits: env_parse("RELAY_INBOUND_POW_BITS", 0u8)?,
|
||||
admission_bind: env_or("RELAY_ADMISSION_BIND", "127.0.0.1:8090"),
|
||||
admission_allow_public: env_bool("RELAY_ADMISSION_ALLOW_PUBLIC", false),
|
||||
max_event_bytes: env_parse("RELAY_MAX_EVENT_BYTES", 65536usize)?,
|
||||
retention_days: env_parse("RELAY_RETENTION_DAYS", 30u32)?,
|
||||
},
|
||||
@@ -1022,6 +1028,26 @@ impl Config {
|
||||
"RELAY_ADMISSION_BIND is required for a non-open RELAY_WRITE_POLICY",
|
||||
));
|
||||
}
|
||||
// The admission service answers "is this npub registered?" per event —
|
||||
// a registration oracle. Require a loopback bind unless the operator
|
||||
// explicitly opts out (and firewalls it).
|
||||
if r.write_policy != "open" && !r.admission_allow_public {
|
||||
use std::net::ToSocketAddrs;
|
||||
let loopback = r
|
||||
.admission_bind
|
||||
.to_socket_addrs()
|
||||
.map(|addrs| {
|
||||
let addrs: Vec<_> = addrs.collect();
|
||||
!addrs.is_empty() && addrs.iter().all(|a| a.ip().is_loopback())
|
||||
})
|
||||
.unwrap_or(false);
|
||||
if !loopback {
|
||||
return Err(cfg_err(
|
||||
"RELAY_ADMISSION_BIND must be loopback (it is a registration oracle); \
|
||||
set RELAY_ADMISSION_ALLOW_PUBLIC=true to override + firewall it",
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
@@ -1166,6 +1192,7 @@ mod tests {
|
||||
write_policy: "inbox-outbox".into(),
|
||||
inbound_pow_bits: 0,
|
||||
admission_bind: "127.0.0.1:8090".into(),
|
||||
admission_allow_public: false,
|
||||
max_event_bytes: 65536,
|
||||
retention_days: 30,
|
||||
},
|
||||
@@ -1458,6 +1485,7 @@ mod tests {
|
||||
write_policy: "inbox-outbox".into(),
|
||||
inbound_pow_bits: 20,
|
||||
admission_bind: "127.0.0.1:8090".into(),
|
||||
admission_allow_public: false,
|
||||
max_event_bytes: 65536,
|
||||
retention_days: 30,
|
||||
}
|
||||
@@ -1519,6 +1547,20 @@ mod tests {
|
||||
assert!(c.validate().is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn relay_rejects_non_loopback_admission_bind() {
|
||||
let mut c = valid();
|
||||
c.messaging.relay = valid_relay();
|
||||
c.messaging.relay.admission_bind = "0.0.0.0:8090".into();
|
||||
assert!(
|
||||
c.validate().is_err(),
|
||||
"non-loopback admission bind rejected"
|
||||
);
|
||||
// …unless explicitly opted out.
|
||||
c.messaging.relay.admission_allow_public = true;
|
||||
assert!(c.validate().is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn payment_requires_https_provider_url_in_production() {
|
||||
let mut c = valid();
|
||||
|
||||
@@ -128,6 +128,23 @@ impl Database {
|
||||
Ok(exists)
|
||||
}
|
||||
|
||||
/// Whether ANY of `pubkeys` (canonical lowercase hex) is a registered npub —
|
||||
/// a single batched query, so resolving an event's recipient `p` tags costs
|
||||
/// one round-trip instead of one-per-tag (no N+1 blowup on a crafted event).
|
||||
#[instrument(skip(self, pubkeys))]
|
||||
pub async fn any_registered_npub(&self, pubkeys: &[String]) -> Result<bool, AppError> {
|
||||
if pubkeys.is_empty() {
|
||||
return Ok(false);
|
||||
}
|
||||
let exists = sqlx::query_scalar::<_, bool>(
|
||||
"SELECT EXISTS (SELECT 1 FROM users WHERE nostr_pubkey = ANY($1))",
|
||||
)
|
||||
.bind(pubkeys)
|
||||
.fetch_one(self.pool())
|
||||
.await?;
|
||||
Ok(exists)
|
||||
}
|
||||
|
||||
/// Replace a user's `pubkey_hash` (derivation-scheme rotation, keyed by the
|
||||
/// unchanged `seed_fingerprint`). Peppered.
|
||||
#[instrument(skip(self, new_pubkey_hash))]
|
||||
|
||||
@@ -38,6 +38,9 @@ pub trait RelayProvider: Send + Sync {
|
||||
fn write_policy(&self) -> WritePolicy;
|
||||
/// NIP-13 PoW bits required on cross-ecosystem inbound (0 = off).
|
||||
fn inbound_pow_bits(&self) -> u8;
|
||||
/// Max event size (bytes) the admission service enforces as defence-in-depth
|
||||
/// (the relay enforces its own limit too).
|
||||
fn max_event_bytes(&self) -> usize;
|
||||
/// NIPs advertised as supported.
|
||||
fn supported_nips(&self) -> &'static [u16] {
|
||||
SUPPORTED_NIPS
|
||||
@@ -69,6 +72,7 @@ pub fn from_config(cfg: &Config) -> Result<Option<Arc<dyn RelayProvider>>, AppEr
|
||||
advertised_url: r.advertised_url.clone(),
|
||||
policy,
|
||||
inbound_pow_bits: r.inbound_pow_bits,
|
||||
max_event_bytes: r.max_event_bytes,
|
||||
}),
|
||||
other => {
|
||||
return Err(AppError::ConfigError(format!(
|
||||
@@ -86,6 +90,7 @@ struct NostrRelayProvider {
|
||||
advertised_url: String,
|
||||
policy: WritePolicy,
|
||||
inbound_pow_bits: u8,
|
||||
max_event_bytes: usize,
|
||||
}
|
||||
|
||||
impl RelayProvider for NostrRelayProvider {
|
||||
@@ -101,4 +106,7 @@ impl RelayProvider for NostrRelayProvider {
|
||||
fn inbound_pow_bits(&self) -> u8 {
|
||||
self.inbound_pow_bits
|
||||
}
|
||||
fn max_event_bytes(&self) -> usize {
|
||||
self.max_event_bytes
|
||||
}
|
||||
}
|
||||
|
||||
@@ -63,6 +63,12 @@ impl Authorization for AdmissionService {
|
||||
return Ok(Response::new(deny("missing event")));
|
||||
};
|
||||
|
||||
// Defence-in-depth size cap (the relay enforces its own limit too): reject
|
||||
// an oversized event before any DB work.
|
||||
if event.content.len() > self.relay.max_event_bytes() {
|
||||
return Ok(Response::new(deny("event exceeds max size")));
|
||||
}
|
||||
|
||||
// Canonical lowercase hex (matches how we store/verify npubs).
|
||||
let author_hex = hex::encode(&event.pubkey);
|
||||
let id_hex = hex::encode(&event.id);
|
||||
@@ -74,20 +80,28 @@ impl Authorization for AdmissionService {
|
||||
.await
|
||||
.unwrap_or(false);
|
||||
|
||||
// Recipient registration: any `p` tag that is a registered npub. Only
|
||||
// resolved when needed (skipped once the author is already registered).
|
||||
let mut recipient_registered = false;
|
||||
if !author_registered {
|
||||
// Recipient registration (only when the author isn't already registered):
|
||||
// collect up to MAX_P_TAGS distinct `p` tags and resolve them in ONE
|
||||
// batched query — bounds the work so a crafted event stuffed with `p` tags
|
||||
// can't trigger an unbounded per-tag query storm.
|
||||
let recipient_registered = if author_registered {
|
||||
false
|
||||
} else {
|
||||
const MAX_P_TAGS: usize = 20;
|
||||
let mut p_tags: Vec<String> = Vec::new();
|
||||
for tag in &event.tags {
|
||||
if tag.values.len() >= 2 && tag.values[0] == "p" {
|
||||
let p = tag.values[1].to_lowercase();
|
||||
if self.db.is_registered_npub(&p).await.unwrap_or(false) {
|
||||
recipient_registered = true;
|
||||
if p_tags.len() >= MAX_P_TAGS {
|
||||
break;
|
||||
}
|
||||
if tag.values.len() >= 2 && tag.values[0] == "p" {
|
||||
let p = tag.values[1].to_lowercase();
|
||||
if !p_tags.contains(&p) {
|
||||
p_tags.push(p);
|
||||
}
|
||||
}
|
||||
}
|
||||
self.db.any_registered_npub(&p_tags).await.unwrap_or(false)
|
||||
};
|
||||
|
||||
let meta = EventMeta {
|
||||
author_pubkey: &author_hex,
|
||||
|
||||
+11
-5
@@ -247,11 +247,17 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
|
||||
let db = state.db.clone();
|
||||
tracing::info!("relay event-admission (nauthz) listening on {addr}");
|
||||
tokio::spawn(async move {
|
||||
if let Err(e) =
|
||||
smirk_backend_core::infra::relay::nauthz::serve(addr, relay, db).await
|
||||
{
|
||||
tracing::error!(error = %e, "relay admission service exited");
|
||||
}
|
||||
let result = smirk_backend_core::infra::relay::nauthz::serve(addr, relay, db).await;
|
||||
// nostr-rs-relay fails OPEN if the admission service is unreachable
|
||||
// (accepts every event). So a dead admission = an unrestricted
|
||||
// relay. Fail SAFE: take the backend down so the operator's
|
||||
// supervisor restarts a clean relay+admission pair, rather than
|
||||
// leaving an open relay running behind a healthy-looking API.
|
||||
tracing::error!(
|
||||
?result,
|
||||
"relay admission service stopped — exiting to avoid an unrestricted relay"
|
||||
);
|
||||
std::process::exit(1);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user