goblin: bind the login callback host to the approved domain
The login URI parser checked the callback scheme but not that its host belongs to the displayed domain, so a site could show d=magick.market while pointing cb at its own host and harvest a signed login event for a domain it does not control. Apply the same domain binding the authorize URI already enforces: share authuri's domain_bound as pub(crate) and reject a login URI whose callback host is neither the domain nor a label-boundary subdomain of it. The http://localhost dev callback stays exempt. Adds binding tests.
This commit is contained in:
@@ -262,7 +262,10 @@ fn is_localhost_authority(rest: &str) -> bool {
|
||||
/// server legitimately serves any domain's flow. This is what makes the
|
||||
/// attacker-supplied `d` trustworthy: the signed event can only ever travel to
|
||||
/// a host that belongs to the domain the user approved.
|
||||
fn domain_bound(callback: &str, domain: &str) -> bool {
|
||||
///
|
||||
/// Shared with [`crate::nostr::loginuri`], which applies the exact same binding
|
||||
/// so a login callback can only reach a host belonging to the approved domain.
|
||||
pub(crate) fn domain_bound(callback: &str, domain: &str) -> bool {
|
||||
// The localhost dev callback is validated already and exempt from binding.
|
||||
if strip_prefix_ignore_case(callback, "http://").is_some() {
|
||||
return true;
|
||||
|
||||
@@ -116,6 +116,14 @@ pub fn parse(scanned: &str) -> Option<LoginUri> {
|
||||
let challenge = validate_challenge(challenge?)?;
|
||||
let domain = validate_domain(domain?)?;
|
||||
let callback = validate_callback(callback?)?;
|
||||
// Domain binding: the callback host must belong to the displayed domain, so a
|
||||
// site cannot show `d=magick.market` while pointing `cb` at its own host and
|
||||
// harvest a login event for a domain it does not control. Same rule as
|
||||
// authorize (case-insensitive, ports/userinfo stripped, label-boundary
|
||||
// subdomains allowed, http://localhost dev callback exempt).
|
||||
if !super::authuri::domain_bound(&callback, &domain) {
|
||||
return None;
|
||||
}
|
||||
Some(LoginUri {
|
||||
challenge,
|
||||
domain,
|
||||
@@ -356,6 +364,29 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn callback_domain_binding_enforced() {
|
||||
// d=magick.market with a callback on an unrelated host: rejected, so a
|
||||
// login event can never be harvested by a domain the site does not control.
|
||||
let bad = format!("goblin:login?c={C}&d=magick.market&cb=https://evil.com/x");
|
||||
assert_eq!(parse(&bad), None, "cross-domain callback must be rejected");
|
||||
// Host equals the domain: accepted.
|
||||
let ok = format!("goblin:login?c={C}&d=magick.market&cb=https://magick.market/cb");
|
||||
assert_eq!(parse(&ok).unwrap().callback, "https://magick.market/cb");
|
||||
// Subdomain on a label boundary: accepted.
|
||||
let ok = format!("goblin:login?c={C}&d=magick.market&cb=https://auth.magick.market/cb");
|
||||
assert!(
|
||||
parse(&ok).is_some(),
|
||||
"label-boundary subdomain must be accepted"
|
||||
);
|
||||
// Suffix without a label boundary: rejected.
|
||||
let bad = format!("goblin:login?c={C}&d=magick.market&cb=https://notmagick.market/cb");
|
||||
assert_eq!(parse(&bad), None, "non-boundary suffix must be rejected");
|
||||
// The http://localhost dev callback is exempt regardless of the domain.
|
||||
let ok = format!("goblin:login?c={C}&d=magick.market&cb=http://localhost:8080/cb");
|
||||
assert_eq!(parse(&ok).unwrap().callback, "http://localhost:8080/cb");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn percent_encoded_callback_decoded() {
|
||||
let cb = "https%3A%2F%2Fmagick.market%2Fapi%2Flogin%3Fs%3D1";
|
||||
|
||||
Reference in New Issue
Block a user