mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(mobile): harden advisory client headers
Co-authored-by: npub1ux8n2yfs8qfvgd75s7kyhar2mztac355v6vmrz4juc9l3msw4pgstums9e <e18f3511303812c437d487ac4bf46ad897dc46946699b18ab2e60bf8ee0ea851@buzz.block.builderlab.xyz> Signed-off-by: npub1ux8n2yfs8qfvgd75s7kyhar2mztac355v6vmrz4juc9l3msw4pgstums9e <e18f3511303812c437d487ac4bf46ad897dc46946699b18ab2e60bf8ee0ea851@buzz.block.builderlab.xyz>
This commit is contained in:
parent
442bffdc15
commit
b5d067036e
@@ -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<S> OptionalFromRequestParts<S> 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<Option<Self>, Self::Rejection> {
|
||||
Ok(Self::from_headers(&parts.headers))
|
||||
fn normalize_app_version(app_version: &str) -> Result<String, ()> {
|
||||
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();
|
||||
|
||||
@@ -197,7 +197,9 @@ class PairingNotifier extends Notifier<PairingState> {
|
||||
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<PairingState> {
|
||||
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,
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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<String, String> 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<String, String> 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<ClientHeaders> 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<ClientHeaders>(
|
||||
(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<String, String> 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;
|
||||
}
|
||||
|
||||
@@ -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<String, String> 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 <String, String>{}
|
||||
@@ -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;
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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', () {
|
||||
|
||||
Reference in New Issue
Block a user