fix(video): keep signed stream tokens out of logs and traces
The access log and the tracing root span's http.target both recorded the full /video/original/stream?token=… URL, i.e. a credential for a possibly sensitive file, in the journal and OTel exports. An outermost middleware now moves the token into request extensions and rewrites the URI to token=redacted before either logger sees it; the handler reads the token from extensions and fails closed (401) without the middleware. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
40d9dcdbb6
commit
c0e6a68ca4
2 files changed
+125
-10
No files matched your search
+120
-10
@@ -8,13 +8,18 @@ use tracing_opentelemetry::OpenTelemetrySpanExt;
|
|||||||
|
|
||||||
use actix_files::NamedFile;
|
use actix_files::NamedFile;
|
||||||
use actix_web::{
|
use actix_web::{
|
||||||
HttpRequest, HttpResponse, Responder, get, post,
|
HttpMessage, HttpRequest, HttpResponse, Responder,
|
||||||
|
body::MessageBody,
|
||||||
|
dev::{ServiceRequest, ServiceResponse},
|
||||||
|
get,
|
||||||
|
middleware::Next,
|
||||||
|
post,
|
||||||
web::{self, Data},
|
web::{self, Data},
|
||||||
};
|
};
|
||||||
use log::{debug, error, info, warn};
|
use log::{debug, error, info, warn};
|
||||||
use opentelemetry::KeyValue;
|
use opentelemetry::KeyValue;
|
||||||
use opentelemetry::trace::Status;
|
use opentelemetry::trace::Status;
|
||||||
use serde::{Deserialize, Serialize};
|
use serde::Serialize;
|
||||||
|
|
||||||
use crate::content_hash;
|
use crate::content_hash;
|
||||||
use crate::data::{
|
use crate::data::{
|
||||||
@@ -301,29 +306,71 @@ pub async fn original_video_url(
|
|||||||
|
|
||||||
span.set_status(Status::Ok);
|
span.set_status(Status::Ok);
|
||||||
HttpResponse::Ok().json(OriginalVideoUrlResponse {
|
HttpResponse::Ok().json(OriginalVideoUrlResponse {
|
||||||
url: format!("/video/original/stream?token={}", token),
|
url: format!("{}?token={}", ORIGINAL_STREAM_PATH, token),
|
||||||
expires_at,
|
expires_at,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Deserialize)]
|
const ORIGINAL_STREAM_PATH: &str = "/video/original/stream";
|
||||||
struct OriginalStreamQuery {
|
|
||||||
token: String,
|
/// A stream token lifted out of the request URI by
|
||||||
|
/// [`redact_original_stream_token`].
|
||||||
|
struct OriginalStreamToken(String);
|
||||||
|
|
||||||
|
/// Moves `token=` off `/video/original/stream` requests into request
|
||||||
|
/// extensions and rewrites the URI to `token=redacted`, so neither the
|
||||||
|
/// access log nor the tracing span's `http.target` (exported to OTel)
|
||||||
|
/// records a credential for a possibly sensitive file. Must be the
|
||||||
|
/// outermost `.wrap()` so it runs before both loggers; without it the
|
||||||
|
/// stream handler finds no token and 401s (fails closed).
|
||||||
|
pub async fn redact_original_stream_token(
|
||||||
|
mut req: ServiceRequest,
|
||||||
|
next: Next<impl MessageBody>,
|
||||||
|
) -> Result<ServiceResponse<impl MessageBody>, actix_web::Error> {
|
||||||
|
if req.path() == ORIGINAL_STREAM_PATH
|
||||||
|
&& let Some((token, redacted_query)) = take_token_param(req.query_string())
|
||||||
|
&& let Ok(uri) = format!("{}?{}", ORIGINAL_STREAM_PATH, redacted_query).parse()
|
||||||
|
{
|
||||||
|
req.head_mut().uri = uri;
|
||||||
|
req.extensions_mut().insert(OriginalStreamToken(token));
|
||||||
|
}
|
||||||
|
next.call(req).await
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Split the first `token=` value out of a query string, returning it and
|
||||||
|
/// the query with that value replaced by `redacted`.
|
||||||
|
fn take_token_param(query: &str) -> Option<(String, String)> {
|
||||||
|
let mut token = None;
|
||||||
|
let redacted: Vec<&str> = query
|
||||||
|
.split('&')
|
||||||
|
.map(|pair| match pair.strip_prefix("token=") {
|
||||||
|
Some(value) if token.is_none() => {
|
||||||
|
token = Some(value.to_string());
|
||||||
|
"token=redacted"
|
||||||
|
}
|
||||||
|
_ => pair,
|
||||||
|
})
|
||||||
|
.collect();
|
||||||
|
token.map(|t| (t, redacted.join("&")))
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Stream an original video named by a token from `POST /video/original`.
|
/// Stream an original video named by a token from `POST /video/original`.
|
||||||
/// Authenticated by the token alone (no Claims). `NamedFile` honours Range,
|
/// Authenticated by the token alone (no Claims), which
|
||||||
/// so players can seek.
|
/// [`redact_original_stream_token`] has already moved out of the URI.
|
||||||
|
/// `NamedFile` honours Range, so players can seek.
|
||||||
#[get("/video/original/stream")]
|
#[get("/video/original/stream")]
|
||||||
#[tracing::instrument(name = "stream_original_video", skip_all)]
|
#[tracing::instrument(name = "stream_original_video", skip_all)]
|
||||||
pub async fn stream_original_video(
|
pub async fn stream_original_video(
|
||||||
request: HttpRequest,
|
request: HttpRequest,
|
||||||
query: web::Query<OriginalStreamQuery>,
|
|
||||||
app_state: Data<AppState>,
|
app_state: Data<AppState>,
|
||||||
) -> impl Responder {
|
) -> impl Responder {
|
||||||
let span = tracing::Span::current();
|
let span = tracing::Span::current();
|
||||||
|
|
||||||
let Some(claims) = original_url::verify(&query.token) else {
|
let claims = request
|
||||||
|
.extensions()
|
||||||
|
.get::<OriginalStreamToken>()
|
||||||
|
.and_then(|t| original_url::verify(&t.0));
|
||||||
|
let Some(claims) = claims else {
|
||||||
span.set_status(Status::error("bad token"));
|
span.set_status(Status::error("bad token"));
|
||||||
return HttpResponse::Unauthorized().finish();
|
return HttpResponse::Unauthorized().finish();
|
||||||
};
|
};
|
||||||
@@ -745,6 +792,7 @@ mod tests {
|
|||||||
write_library_file(&app_state, "clips/a.mp4", b"0123456789");
|
write_library_file(&app_state, "clips/a.mp4", b"0123456789");
|
||||||
let app = actix_web::test::init_service(
|
let app = actix_web::test::init_service(
|
||||||
App::new()
|
App::new()
|
||||||
|
.wrap(actix_web::middleware::from_fn(redact_original_stream_token))
|
||||||
.service(original_video_url)
|
.service(original_video_url)
|
||||||
.service(stream_original_video)
|
.service(stream_original_video)
|
||||||
.app_data(app_state),
|
.app_data(app_state),
|
||||||
@@ -796,12 +844,74 @@ mod tests {
|
|||||||
assert_eq!(actix_web::test::call_service(&app, req).await.status(), 401);
|
assert_eq!(actix_web::test::call_service(&app, req).await.status(), 401);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn take_token_param_redacts_only_the_token() {
|
||||||
|
assert_eq!(
|
||||||
|
take_token_param("token=abc.def.ghi"),
|
||||||
|
Some(("abc.def.ghi".into(), "token=redacted".into()))
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
take_token_param("x=1&token=abc&y=2"),
|
||||||
|
Some(("abc".into(), "x=1&token=redacted&y=2".into()))
|
||||||
|
);
|
||||||
|
assert_eq!(take_token_param("x=1&tokens=abc"), None);
|
||||||
|
assert_eq!(take_token_param(""), None);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// What the loggers (and handler) see after the middleware runs.
|
||||||
|
#[get("/video/original/stream")]
|
||||||
|
async fn echo_uri_and_token(request: HttpRequest) -> HttpResponse {
|
||||||
|
let token = request
|
||||||
|
.extensions()
|
||||||
|
.get::<OriginalStreamToken>()
|
||||||
|
.map(|t| t.0.clone())
|
||||||
|
.unwrap_or_default();
|
||||||
|
HttpResponse::Ok().body(format!("{} {}", request.uri(), token))
|
||||||
|
}
|
||||||
|
|
||||||
|
#[actix_rt::test]
|
||||||
|
async fn redact_middleware_strips_token_from_uri() {
|
||||||
|
let app = actix_web::test::init_service(
|
||||||
|
App::new()
|
||||||
|
.wrap(actix_web::middleware::from_fn(redact_original_stream_token))
|
||||||
|
.service(echo_uri_and_token),
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
let req = actix_web::test::TestRequest::get()
|
||||||
|
.uri("/video/original/stream?token=secret.jwt.value")
|
||||||
|
.to_request();
|
||||||
|
let body = actix_web::test::call_and_read_body(&app, req).await;
|
||||||
|
assert_eq!(
|
||||||
|
std::str::from_utf8(&body).unwrap(),
|
||||||
|
"/video/original/stream?token=redacted secret.jwt.value"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[actix_rt::test]
|
||||||
|
async fn stream_original_video_fails_closed_without_middleware() {
|
||||||
|
let app_state = Data::new(AppState::test_state());
|
||||||
|
write_library_file(&app_state, "clips/a.mp4", b"0123456789");
|
||||||
|
let app = actix_web::test::init_service(
|
||||||
|
App::new()
|
||||||
|
.service(stream_original_video)
|
||||||
|
.app_data(app_state),
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
let (token, _) =
|
||||||
|
original_url::sign("1", crate::libraries::PRIMARY_LIBRARY_ID, "clips/a.mp4");
|
||||||
|
let req = actix_web::test::TestRequest::get()
|
||||||
|
.uri(&format!("/video/original/stream?token={}", token))
|
||||||
|
.to_request();
|
||||||
|
assert_eq!(actix_web::test::call_service(&app, req).await.status(), 401);
|
||||||
|
}
|
||||||
|
|
||||||
#[actix_rt::test]
|
#[actix_rt::test]
|
||||||
async fn stream_original_video_rejects_bad_tokens() {
|
async fn stream_original_video_rejects_bad_tokens() {
|
||||||
let app_state = Data::new(AppState::test_state());
|
let app_state = Data::new(AppState::test_state());
|
||||||
write_library_file(&app_state, "clips/a.mp4", b"0123456789");
|
write_library_file(&app_state, "clips/a.mp4", b"0123456789");
|
||||||
let app = actix_web::test::init_service(
|
let app = actix_web::test::init_service(
|
||||||
App::new()
|
App::new()
|
||||||
|
.wrap(actix_web::middleware::from_fn(redact_original_stream_token))
|
||||||
.service(stream_original_video)
|
.service(stream_original_video)
|
||||||
.app_data(app_state),
|
.app_data(app_state),
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -318,6 +318,11 @@ fn main() -> std::io::Result<()> {
|
|||||||
.wrap(middleware::Logger::default())
|
.wrap(middleware::Logger::default())
|
||||||
.wrap(cors)
|
.wrap(cors)
|
||||||
.wrap(tracing_actix_web::TracingLogger::default())
|
.wrap(tracing_actix_web::TracingLogger::default())
|
||||||
|
// Outermost: strips signed stream tokens from the URI before
|
||||||
|
// either logger above records it.
|
||||||
|
.wrap(middleware::from_fn(
|
||||||
|
handlers::video::redact_original_stream_token,
|
||||||
|
))
|
||||||
.service(
|
.service(
|
||||||
web::resource("/login")
|
web::resource("/login")
|
||||||
.wrap(Governor::new(&governor_conf))
|
.wrap(Governor::new(&governor_conf))
|
||||||
|
|||||||
Reference in new issue
Block a user