fix(auth): backport dot-segment and redirect refusals

doc/bin/relay/auth.md

@@ -37,7 +37,10 @@ bytes as unpadded base64url), `role` the client declared at SETUP
 `name` (first SAN DNS name, else CN, else the fingerprint; a server cannot tell which), `fingerprint`
 (SHA-256 of the leaf), `expires`, `issuer`. Nothing is parsed on the server's
 behalf: the `jwt` query parameter is a convention of `moq auth serve`, not of
-the relay, and the relay forwards a SETUP token without reading it. An `end` adds `reason`, `duration` in seconds, and `bytes` sent and
+the relay, and the relay forwards a SETUP token without reading it. A path with
+a `.` or `..` segment, percent-encoded or not, is refused before the server is
+asked: a URL transport resolves those away, but a SETUP path arrives verbatim.
+An `end` adds `reason`, `duration` in seconds, and `bytes` sent and
 received.
 
 **Grant.** `publish` and `subscribe` as pattern unions (`foo/**` is a subtree,
@@ -120,7 +123,9 @@ only. HTTPS verifies the URL's hostname with system roots. For a private CA,
 set `auth.root` to PEM paths (or repeat `--auth-root`); these replace system
 trust for auth only and reload for new connections when rotated. Mesh roots,
 fingerprints, insecure mode, and hostname overrides never apply to auth.
-There is no shared secret.
+There is no shared secret. Auth requests never follow redirects, so a redirect
+cannot forward a credential to another destination. Point `--auth-url` at the
+server's final URL; a redirect is reported as an auth-server outage.
 
 **Patterns.** A grant is any pattern union and means exactly what it says:
 `foo/**` is a subtree, a bare `foo` is that one broadcast, `live/*` is every

rs/moq-auth/src/client.rs

@@ -32,7 +32,11 @@ impl Client {
 	/// caller's client identity and roots; `unix://` speaks HTTP over the socket at
 	/// the URL's path. Anything else is refused here rather than at the first session.
 	pub fn new(url: Url, tls: Option<rustls::ClientConfig>) -> crate::Result<Self> {
-		let builder = reqwest::Client::builder().timeout(TIMEOUT);
+		// Every request carries a credential in its body, which a followed 307 or
+		// 308 would resend to wherever it points, so refuse redirects.
+		let builder = reqwest::Client::builder()
+			.timeout(TIMEOUT)
+			.redirect(reqwest::redirect::Policy::none());
 
 		let (builder, url) = match url.scheme() {
 			"http" => {
@@ -87,29 +91,35 @@ trait Post: Clone + Send + Sync + 'static {
 }
 
 impl Post for Client {
-	/// A 2xx carries the grant, a 401 or 403 refuses, and everything else is an
-	/// outage the caller decides about.
+	/// A 2xx carries the grant, a 401 or 403 refuses, and everything else, a redirect
+	/// included, is an outage the caller decides about.
 	async fn post(&self, request: &Request) -> crate::Result<Grant> {
 		let response = self.http.post(self.url.clone()).json(request).send().await?;
 		let status = response.status();
 		if status == reqwest::StatusCode::UNAUTHORIZED || status == reqwest::StatusCode::FORBIDDEN {
 			return Err(Error::Refused);
 		}
-		if !status.is_success() {
-			return Err(Error::Unavailable(format!("auth server answered {status}")));
-		}
+		check(status)?;
 		Ok(response.json().await?)
 	}
 
 	async fn post_end(&self, request: &Request) -> crate::Result<()> {
-		self.http
-			.post(self.url.clone())
-			.json(request)
-			.send()
-			.await?
-			.error_for_status()?;
-		Ok(())
+		let response = self.http.post(self.url.clone()).json(request).send().await?;
+		check(response.status())
+	}
+}
+
+/// Refuse anything but a 2xx, naming a redirect as one, since it is not followed.
+fn check(status: reqwest::StatusCode) -> crate::Result<()> {
+	if status.is_redirection() {
+		return Err(Error::Unavailable(format!(
+			"auth server answered {status}; redirects are not followed"
+		)));
+	}
+	if !status.is_success() {
+		return Err(Error::Unavailable(format!("auth server answered {status}")));
 	}
+	Ok(())
 }
 
 /// Admit a session through `server` and spawn the driver behind its lease.
@@ -473,6 +483,53 @@ mod tests {
 		assert!(matches!(err, Error::Refused), "{err}");
 	}
 
+	#[tokio::test]
+	async fn redirects_are_refused_without_resending_the_credential() {
+		let foreign = MockServer::start().await;
+		Mock::given(wiremock::matchers::any())
+			.respond_with(ResponseTemplate::new(200).set_body_json(grant(None, None)))
+			.mount(&foreign)
+			.await;
+
+		for status in [307, 308, 301, 302, 303] {
+			let location = format!("{}/foreign", foreign.uri());
+			let auth = server(Log::default(), move |_| {
+				ResponseTemplate::new(status).insert_header("location", location.clone())
+			})
+			.await;
+			let client = client(&auth);
+			for event in [
+				Event::Connect,
+				Event::Revalidate,
+				Event::End {
+					reason: Reason::Dropped,
+					duration: Duration::ZERO,
+					bytes: Bytes::default(),
+				},
+			] {
+				let mut request = request();
+				request.query = Some("jwt=secret".into());
+				request.event = event;
+				let result = if matches!(request.event, Event::End { .. }) {
+					Post::post_end(&client, &request).await
+				} else {
+					Post::post(&client, &request).await.map(|_| ())
+				};
+				assert!(
+					foreign.received_requests().await.unwrap().is_empty(),
+					"{status} forwarded {:?} to the redirect destination",
+					request.event
+				);
+				let err = result.unwrap_err();
+				assert!(
+					matches!(&err, Error::Unavailable(message) if message.contains("redirects are not followed")),
+					"{status}: {err}"
+				);
+			}
+			assert_eq!(auth.received_requests().await.unwrap().len(), 3);
+		}
+	}
+
 	#[tokio::test]
 	async fn http_error_redacts_url() {
 		// A freed port refuses the connection, so reqwest fails with the dialed URL attached.

rs/moq-relay/src/auth.rs

@@ -632,7 +632,14 @@ impl Auth {
 	}
 
 	/// Admit a session: the lease it holds, carrying the scope the origin applies.
+	///
+	/// A path naming a `.` or `..` segment is refused before the decider sees it. A
+	/// URL transport resolves those away, but an in-band SETUP Path arrives verbatim,
+	/// and `/tenant-a/../tenant-b` would reach the decider spelled under `tenant-a`.
 	pub async fn admit(&self, request: Request) -> Result<Lease, Error> {
+		if navigates(&request.path) {
+			return Err(Error::Request(format!("path {:?} has a dot segment", request.path)));
+		}
 		let path = request.path.clone();
 		let (reply, answer) = oneshot::channel();
 		self.admissions
@@ -655,6 +662,12 @@ impl Auth {
 	}
 }
 
+/// Whether `path` has a segment URL resolution reads as `.` or `..`, percent-encoded or not.
+fn navigates(path: &str) -> bool {
+	path.split('/')
+		.any(|segment| matches!(segment.to_ascii_lowercase().replace("%2e", ".").as_str(), "." | ".."))
+}
+
 /// The sessions an embedded [`Auth`] is waiting to admit, in arrival order.
 ///
 /// The transport has already accepted each one; it waits for its answer, so a
@@ -779,6 +792,41 @@ mod tests {
 		assert!(grant.publish.is_empty());
 	}
 
+	#[tokio::test(start_paused = true)]
+	async fn admit_refuses_dot_segments_before_the_decider() {
+		let (auth, mut admissions) = Auth::embedded("relay-1");
+		for path in [
+			"/tenant-a/../tenant-b",
+			".",
+			"/..",
+			"/a/./b",
+			"/a/.",
+			"/%2e",
+			"/%2E/room",
+			"/a/.%2e",
+			"/a/%2E./b",
+			"/a/%2e%2E/b",
+		] {
+			let result = auth.admit(auth.request(moq_auth::Transport::Quic, path)).await;
+			assert!(
+				matches!(result, Err(Error::Request(_))),
+				"{path} was not refused as a bad path"
+			);
+			assert!(admissions.0.try_recv().is_err(), "{path} reached the decider");
+		}
+
+		// Names containing dots keep their exact spelling at the admission boundary.
+		for path in ["/a/...", "/.hidden", "/a/..b", "/a/%2e-hidden", "/a/%252e"] {
+			let decide = async {
+				let admission = admissions.next().await.unwrap();
+				assert_eq!(admission.request.path, path);
+				admission.grant(lease::Consumer::fixed(Grant::new(patterns(&["**"]), patterns(&["**"]))));
+			};
+			let (result, ()) = tokio::join!(auth.admit(auth.request(moq_auth::Transport::Quic, path)), decide);
+			assert!(result.is_ok(), "{path} was refused");
+		}
+	}
+
 	/// The public rules are rooted at `/`, like a token with an empty root. Bare `**`
 	/// reads the same either way, which is how rooting them at the dialed path went
 	/// unnoticed: `anon/**` at `/rooms/123` granted `rooms/123/anon/**`.