diff --git a/crates/buzz-relay/src/client_info.rs b/crates/buzz-relay/src/client_info.rs index 653147b44..445a2fb1b 100644 --- a/crates/buzz-relay/src/client_info.rs +++ b/crates/buzz-relay/src/client_info.rs @@ -1,11 +1,6 @@ //! Advisory parsing for the mobile `Buzz-Client` structured field. -use std::convert::Infallible; - -use axum::{ - extract::OptionalFromRequestParts, - http::{request::Parts, HeaderMap}, -}; +use axum::http::HeaderMap; use sfv::{BareItem, Dictionary, ListEntry, Parser}; /// Parsed, untrusted metadata supplied by a Buzz client. @@ -22,6 +17,8 @@ pub struct ClientInfo { pub platform: String, /// User-visible application version. pub app_version: String, + /// Bounded, normalized label derived from the application version. + metric_app_version: String, /// Platform build identifier, constrained to decimal digits. pub app_build: String, /// Coarse public operating-system version. @@ -65,10 +62,10 @@ impl ClientInfo { } let app_version = string(&dictionary, "app-version")?; + let metric_app_version = normalize_app_version(&app_version)?; let app_build = string(&dictionary, "app-build")?; let os_version = string(&dictionary, "os-version")?; - if app_version.is_empty() - || app_build.is_empty() + if app_build.is_empty() || !app_build.bytes().all(|byte| byte.is_ascii_digit()) || os_version.is_empty() { @@ -87,6 +84,7 @@ impl ClientInfo { app, platform, app_version, + metric_app_version, app_build, os_version, os_api, @@ -96,27 +94,43 @@ impl ClientInfo { /// Record a low-cardinality observation for a parsed client. pub fn record_observation(&self) { metrics::counter!( - "buzz_client_requests_total", + "buzz_client_connections_total", "app" => self.app.clone(), "platform" => self.platform.clone(), - "app_version" => self.app_version.clone(), + "app_version" => self.metric_app_version.clone(), ) .increment(1); } } -impl OptionalFromRequestParts for ClientInfo -where - S: Send + Sync, -{ - type Rejection = Infallible; +const MAX_APP_VERSION_COMPONENT_LENGTH: usize = 5; - async fn from_request_parts( - parts: &mut Parts, - _state: &S, - ) -> Result, Self::Rejection> { - Ok(Self::from_headers(&parts.headers)) +fn normalize_app_version(app_version: &str) -> Result { + let mut components = app_version.split('.'); + let Some(major) = components.next() else { + return Err(()); + }; + let Some(minor) = components.next() else { + return Err(()); + }; + let patch = components.next(); + if components.next().is_some() { + return Err(()); } + + if ![Some(major), Some(minor), patch] + .into_iter() + .flatten() + .all(|component| { + !component.is_empty() + && component.len() <= MAX_APP_VERSION_COMPONENT_LENGTH + && component.bytes().all(|byte| byte.is_ascii_digit()) + }) + { + return Err(()); + } + + Ok(format!("{major}.{minor}")) } fn bare_item<'a>(dictionary: &'a Dictionary, key: &str) -> Result<&'a BareItem, ()> { @@ -164,20 +178,36 @@ mod tests { use super::*; - fn parse_failures(recorder: &DebuggingRecorder) -> u64 { + fn metric_counter( + recorder: &DebuggingRecorder, + name: &str, + ) -> Vec<(Vec<(String, String)>, u64)> { recorder .snapshotter() .snapshot() .into_vec() .into_iter() - .find_map(|(key, _, _, value)| { - (key.key().name() == "buzz_client_header_parse_failures_total").then_some(value) + .filter_map(|(key, _, _, value)| { + (key.key().name() == name).then(|| { + let labels = key + .key() + .labels() + .map(|label| (label.key().to_owned(), label.value().to_owned())) + .collect(); + let DebugValue::Counter(value) = value else { + panic!("{name} must be a counter"); + }; + (labels, value) + }) }) - .map(|value| match value { - DebugValue::Counter(value) => value, - _ => panic!("parse failures must be a counter"), - }) - .unwrap_or_default() + .collect() + } + + fn parse_failures(recorder: &DebuggingRecorder) -> u64 { + metric_counter(recorder, "buzz_client_header_parse_failures_total") + .into_iter() + .map(|(_, value)| value) + .sum() } #[test] @@ -193,6 +223,7 @@ mod tests { app: "buzz-mobile".to_owned(), platform: "ios".to_owned(), app_version: "0.4.5".to_owned(), + metric_app_version: "0.4".to_owned(), app_build: "6".to_owned(), os_version: "18.5".to_owned(), os_api: None, @@ -206,6 +237,25 @@ mod tests { assert_eq!(android.os_api, Some(35)); } + #[test] + fn observations_bucket_versions_to_major_minor() { + let client = ClientInfo::parse( + r#"v=1, app=buzz-mobile, platform=ios, app-version="12.34.56", app-build="6", os-version="18.5""#, + ) + .expect("valid iOS header"); + let recorder = DebuggingRecorder::new(); + + metrics::with_local_recorder(&recorder, || client.record_observation()); + + let counters = metric_counter(&recorder, "buzz_client_connections_total"); + assert_eq!(counters.len(), 1); + let (labels, value) = &counters[0]; + assert_eq!(*value, 1); + assert!(labels.contains(&("app".to_owned(), "buzz-mobile".to_owned()))); + assert!(labels.contains(&("platform".to_owned(), "ios".to_owned()))); + assert!(labels.contains(&("app_version".to_owned(), "12.34".to_owned()))); + } + #[test] fn missing_header_is_absent_without_parse_failure() { let recorder = DebuggingRecorder::new(); @@ -222,6 +272,9 @@ mod tests { r#"v=2, app=buzz-mobile, platform=ios, app-version="1", app-build="1", os-version="18""#, r#"v=1, app=buzz-mobile, platform=android, app-version="1", app-build="1", os-version="15""#, r#"v=1, app=buzz-mobile, platform=ios, app-version="1", app-build="1.beta", os-version="18""#, + r#"v=1, app=buzz-mobile, platform=ios, app-version="random-connection-value", app-build="1", os-version="18""#, + r#"v=1, app=buzz-mobile, platform=ios, app-version="1.2.3.4", app-build="1", os-version="18""#, + r#"v=1, app=buzz-mobile, platform=ios, app-version="123456.2", app-build="1", os-version="18""#, ] { let recorder = DebuggingRecorder::new(); let mut headers = HeaderMap::new(); diff --git a/mobile/lib/features/pairing/pairing_provider.dart b/mobile/lib/features/pairing/pairing_provider.dart index f34698065..22eec3242 100644 --- a/mobile/lib/features/pairing/pairing_provider.dart +++ b/mobile/lib/features/pairing/pairing_provider.dart @@ -197,7 +197,9 @@ class PairingNotifier extends Notifier { qr.sourcePubkey, ); - // 4. Connect to relay with ephemeral keys. + // 4. Connect to the relay with ephemeral keys. The pairing payload has + // not been authenticated yet, so do not trust its relay URL as a + // configured origin for structured client metadata. final socket = _socketFactory( wsUrl: relayWsUrl, ephemeralPrivkey: _ephemeralPrivkey!, @@ -615,6 +617,8 @@ class PairingNotifier extends Notifier { final scheme = uri.scheme == 'https' ? 'wss' : 'ws'; final wsUrl = uri.replace(scheme: scheme).toString(); + // The credential payload is still untrusted, so only globally recognized + // Buzz origins may receive structured client metadata during this probe. final socket = RelaySocket( wsUrl: wsUrl, nsec: nsec, diff --git a/mobile/lib/main.dart b/mobile/lib/main.dart index bdfacf7d6..025455ea5 100644 --- a/mobile/lib/main.dart +++ b/mobile/lib/main.dart @@ -10,10 +10,13 @@ void main() async { WidgetsFlutterBinding.ensureInitialized(); // Pre-load preferences so the first frame uses the saved theme/accent. - final (prefs, clientHeaders) = await ( - SharedPreferences.getInstance(), - loadClientHeaders(), - ).wait; + final prefs = await SharedPreferences.getInstance(); + var clientHeaders = ClientHeaders.empty; + try { + clientHeaders = await loadClientHeaders(); + } catch (error) { + debugPrint('Could not load optional Buzz client headers: $error'); + } runApp( ProviderScope( diff --git a/mobile/lib/shared/client/client_headers.dart b/mobile/lib/shared/client/client_headers.dart index 24162bcb2..dca76ced0 100644 --- a/mobile/lib/shared/client/client_headers.dart +++ b/mobile/lib/shared/client/client_headers.dart @@ -15,12 +15,20 @@ class ClientHeaders { final String buzzClient; final String userAgent; + /// An inert value used when advisory identification cannot be loaded. + static const empty = ClientHeaders( + appVersion: '', + buzzClient: '', + userAgent: '', + ); + const ClientHeaders({ required this.appVersion, required this.buzzClient, required this.userAgent, }); + /// Both headers when they are fully initialized. Map get values { if (buzzClient.isEmpty && userAgent.isEmpty) return const {}; if (buzzClient.isEmpty || userAgent.isEmpty) { @@ -31,6 +39,15 @@ class ClientHeaders { _userAgentHeaderName: userAgent, }); } + + /// The coarse header safe to send to arbitrary HTTP and WebSocket origins. + Map get userAgentValue { + if (buzzClient.isEmpty && userAgent.isEmpty) return const {}; + if (buzzClient.isEmpty || userAgent.isEmpty) { + throw StateError('Buzz client headers must be initialized together'); + } + return Map.unmodifiable({_userAgentHeaderName: userAgent}); + } } /// Metadata used to construct [ClientHeaders]. @@ -53,6 +70,9 @@ class ClientHeaderMetadata { /// Builds the canonical structured `Buzz-Client` and display-only `User-Agent`. ClientHeaders buildClientHeaders(ClientHeaderMetadata metadata) { + if (!_isValidAppVersion(metadata.appVersion)) { + throw FormatException('App version must be numeric major.minor[.patch]'); + } if (metadata.platform != 'ios' && metadata.platform != 'android') { throw ArgumentError.value( metadata.platform, @@ -93,6 +113,20 @@ ClientHeaders buildClientHeaders(ClientHeaderMetadata metadata) { ); } +bool _isValidAppVersion(String version) { + final components = version.split('.'); + return components.length >= 2 && + components.length <= 3 && + components.every( + (component) => + component.isNotEmpty && + component.length <= 5 && + component.codeUnits.every( + (codeUnit) => codeUnit >= 0x30 && codeUnit <= 0x39, + ), + ); +} + String _structuredString(String value) { final escaped = StringBuffer('"'); for (final codeUnit in value.codeUnits) { @@ -175,14 +209,8 @@ Future loadClientHeaders() async { /// /// Tests and component previews do not execute [main], so the provider has an /// inert value until the app-level override installs real platform metadata. -const _uninitializedClientHeaders = ClientHeaders( - appVersion: '', - buzzClient: '', - userAgent: '', -); - final clientHeadersProvider = Provider( - (ref) => _uninitializedClientHeaders, + (ref) => ClientHeaders.empty, ); String _coarseOsVersion(String version) { @@ -225,18 +253,27 @@ bool isFirstPartyBuzzUrl( return false; } +/// Returns outbound identification headers for [targetUrl]. +/// +/// The coarse `User-Agent` is safe to send to arbitrary HTTP and WebSocket +/// origins. The structured `Buzz-Client` metadata is included only for a +/// recognized first-party Buzz origin. Map clientHeadersForUrl({ required ClientHeaders headers, required String targetUrl, String? relayUrl, bool allowLocalDevelopment = kDebugMode, }) { + final target = Uri.tryParse(targetUrl); + if (target == null || target.host.isEmpty || !_isHttpOrWebSocket(target)) { + return const {}; + } if (!isFirstPartyBuzzUrl( targetUrl, relayUrl: relayUrl, allowLocalDevelopment: allowLocalDevelopment, )) { - return const {}; + return headers.userAgentValue; } return headers.values; } diff --git a/mobile/lib/shared/relay/media_auth.dart b/mobile/lib/shared/relay/media_auth.dart index 4ca08899b..4a2111f8c 100644 --- a/mobile/lib/shared/relay/media_auth.dart +++ b/mobile/lib/shared/relay/media_auth.dart @@ -14,12 +14,13 @@ const _mediaGetAuthLifetimeSeconds = 600; /// request signed just before the boundary still lands well within validity. const _mediaGetAuthRefreshMarginSeconds = 60; -/// Builds BUD-01 Blossom `t=get` auth headers for relay-host media URLs. +/// Builds BUD-01 Blossom auth for relay-hosted media requests, alongside +/// outbound client identification headers. /// -/// Returns only first-party client-identification headers when no signing key -/// is available. Returns an empty map for non-relay URLs, so callers can safely -/// use this on arbitrary profile/custom-emoji URLs without leaking Buzz -/// metadata or credentials to third-party hosts. +/// The coarse `User-Agent` is returned for any valid remote HTTP(S) URL. +/// `Buzz-Client` and `Authorization` remain restricted to same-origin relay +/// media paths, so arbitrary profile and custom-emoji hosts receive neither +/// structured metadata nor credentials. /// /// The signed header is memoized until [_mediaGetAuthRefreshMarginSeconds] /// before expiry: repeated calls return the byte-identical map instead of @@ -46,11 +47,6 @@ class MediaGetAuthService { _now = now ?? DateTime.now; Map headersFor(String url) { - final uri = Uri.tryParse(url); - final relayUri = Uri.tryParse(_baseUrl); - if (uri == null || relayUri == null) return const {}; - if (!_isRelayMediaUrl(uri, relayUri)) return const {}; - final clientHeaders = _clientHeaders; final identificationHeaders = clientHeaders == null ? const {} @@ -59,6 +55,11 @@ class MediaGetAuthService { targetUrl: url, relayUrl: _baseUrl, ); + final uri = Uri.tryParse(url); + final relayUri = Uri.tryParse(_baseUrl); + if (uri == null || relayUri == null) return const {}; + if (!_isRelayMediaUrl(uri, relayUri)) return identificationHeaders; + final nsec = _nsec; if (nsec == null || nsec.isEmpty) return identificationHeaders; diff --git a/mobile/test/shared/client/client_headers_test.dart b/mobile/test/shared/client/client_headers_test.dart index 381450106..0a7791bc1 100644 --- a/mobile/test/shared/client/client_headers_test.dart +++ b/mobile/test/shared/client/client_headers_test.dart @@ -41,23 +41,37 @@ void main() { expect(headers.userAgent, 'Buzz/0.4.5 (android; build 6)'); }); - test('does not emit partially initialized header pairs', () { - expect( - const ClientHeaders( - appVersion: '', - buzzClient: '', - userAgent: '', - ).values, - isEmpty, - ); - expect( - () => const ClientHeaders( - appVersion: '1', - buzzClient: 'client', - userAgent: '', - ).values, - throwsStateError, + test('exposes an inert empty header value', () { + expect(ClientHeaders.empty.appVersion, isEmpty); + expect(ClientHeaders.empty.values, isEmpty); + expect(ClientHeaders.empty.userAgentValue, isEmpty); + }); + + test('partially initialized values fail loudly for every header view', () { + const partial = ClientHeaders( + appVersion: '1', + buzzClient: 'client', + userAgent: '', ); + expect(() => partial.values, throwsStateError); + expect(() => partial.userAgentValue, throwsStateError); + }); + + test('rejects app versions outside the relay metric contract', () { + for (final version in ['', '1', '1.2.3.4', '1.2-beta', '123456.2']) { + expect( + () => buildClientHeaders( + ClientHeaderMetadata( + platform: 'ios', + appVersion: version, + appBuild: '1', + osVersion: '18.0', + ), + ), + throwsFormatException, + reason: version, + ); + } }); test('rejects non-decimal app builds', () { @@ -140,14 +154,29 @@ void main() { } }); - test('rejects lookalikes, third-party hosts, and insecure hosted URLs', () { + test('sends only User-Agent to other HTTP and WebSocket servers', () { for (final url in [ - 'wss://communities.buzz.xyz.evil.example', - 'wss://evil.example', + 'https://cdn.cloudflare.example/attachment.png', + 'wss://relay.other.example/socket', + 'https://communities.buzz.xyz.evil.example/media/file', 'ws://pairing.buzz.xyz', - 'https://acme.communities.buzz.xyz.evil.example', 'wss://relay.example:444/socket', ]) { + expect( + clientHeadersForUrl( + headers: headers, + targetUrl: url, + relayUrl: 'https://relay.example', + allowLocalDevelopment: false, + ), + {'User-Agent': 'agent'}, + reason: url, + ); + } + }); + + test('does not send headers to malformed or non-network URLs', () { + for (final url in ['', 'not a URL', 'file:///tmp/media.png']) { expect( clientHeadersForUrl( headers: headers, diff --git a/mobile/test/shared/relay/media_image_test.dart b/mobile/test/shared/relay/media_image_test.dart index 1bc139dcd..61b5a2ff4 100644 --- a/mobile/test/shared/relay/media_image_test.dart +++ b/mobile/test/shared/relay/media_image_test.dart @@ -66,7 +66,7 @@ void main() { expect(refreshed['Authorization'], isNot(first['Authorization'])); }); - test('sends identification only for relay media URLs', () { + test('sends both identification headers only for relay media URLs', () { const clientHeaders = ClientHeaders( appVersion: '1.0', buzzClient: 'test-client', @@ -77,11 +77,29 @@ void main() { clientHeaders: clientHeaders, ); - expect( - auth.headersFor(_mediaUrl), - containsPair('Buzz-Client', 'test-client'), + final headers = auth.headersFor(_mediaUrl); + expect(headers['Buzz-Client'], 'test-client'); + expect(headers['User-Agent'], 'test-agent'); + expect(headers['Authorization'], startsWith('Nostr ')); + }); + + test('sends only User-Agent to third-party media hosts', () { + const clientHeaders = ClientHeaders( + appVersion: '1.0', + buzzClient: 'test-client', + userAgent: 'test-agent', ); - expect(auth.headersFor('https://elsewhere.com/media/abc.png'), isEmpty); + final auth = _auth( + nsec: nostr.Keys.generate().nsec, + clientHeaders: clientHeaders, + ); + + final headers = auth.headersFor( + 'https://cdn.cloudflare.example/attachments/abc.png', + ); + expect(headers, {'User-Agent': 'test-agent'}); + expect(headers, isNot(contains('Buzz-Client'))); + expect(headers, isNot(contains('Authorization'))); }); test('sends identification without a signing key', () {