rework WebSocket error return protocol

This gives much better information to the UI layer, getting rid of a
whole troubleshooting guide entry. See #119 #132 #218 #219

I also restructured the code in anticipation of a new WebSocket event
stream (#40).
This commit is contained in:
Scott Lamb 2023-02-15 07:04:50 -08:00
parent 0ffda11d4b
commit 438de38202
No known key found for this signature in database
8 changed files with 237 additions and 219 deletions

View File

@ -34,6 +34,9 @@ even on minor releases, e.g. `0.7.5` -> `0.7.6`.
in `ref/api.md`, not an undocumented plaintext protobuf format.
* fix [#257](https://github.com/scottlamb/moonfire-nvr/issues/257):
Live View: select None Not Possible After Selecting a Camera.
* get rid of live view's dreaded `ws close: 1006` error altogether. The live
view WebSocket protocol now conveys errors in a way that allows the
Javscript UI to see them.
## 0.7.5 (2022-05-09)

View File

@ -18,8 +18,6 @@ need more help.
* [Incorrect timestamps](#incorrect-timestamps)
* [Configuration interface problems](#configuration-interface-problems)
* [`moonfire-nvr config` displays garbage](#moonfire-nvr-config-displays-garbage)
* [Browser user interface problems](#browser-user-interface-problems)
* [Live stream always fails with `ws close: 1006`](#live-stream-always-fails-with-ws-close-1006)
* [Errors in kernel logs](#errors-in-kernel-logs)
* [UAS errors](#uas-errors)
* [Filesystem errors](#filesystem-errors)
@ -357,35 +355,6 @@ configured your machine is configured to a non-UTF-8 locale, due to
gyscos/Cursive#13. As a workaround, try setting the environment variable
`LC_ALL=C.UTF-8`.
### Browser user interface problems
#### Live stream always fails with `ws close: 1006`
Moonfire NVR's UI uses a
[WebSocket](https://developer.mozilla.org/en-US/docs/Web/API/WebSockets_API)
connection to the server for the live view. If you see an alert in the lower
left corner of a live stream area that says `ws close: 1006`, this means that
the WebSocket connection failed. Unfortunately this is all the UI knows;
the WebSocket spec [deliberately withholds](https://html.spec.whatwg.org/multipage/web-sockets.html#closeWebSocket) additional debugging information
for security reasons.
You might be able to learn more through your browser's Javascript console.
If you consistently see this error when other parts of the UI work properly,
here are some things to check:
* If you are using Safari and haven't logged out since Moonfire NVR v0.6.3
was released, try logging out and back in. Safari apparently doesn't send
[`SameSite=Strict`
cookies](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite#strict)
on WebSocket requests. Since v0.6.3, Moonfire NVR uses `SameSite=Lax`
instead.
* If you are using a proxy server, check that it is properly configured for
Websockets. In particular, if you followed the [Securing Moonfire NVR
guide](schema.md) prior to 29 Feb 2020, look at [this
update](https://github.com/scottlamb/moonfire-nvr/commit/92266612b5c9163eb6096c580ba751280a403648#diff-e8bdd96dda101a25a541a6629675ea46bd6eaf670c6417c76662db5397c50c19)
to those instructions.
### Errors in kernel logs
#### UAS errors

View File

@ -568,10 +568,16 @@ Initiate a WebSocket stream for chunks of video. Expects the standard
WebSocket headers as described in [RFC 6455][rfc-6455] and (if authentication
is required) the `s` cookie.
The server will send a sequence of binary messages. Each message corresponds
to one or more frames of video. The first message is guaranteed to start with a
"key" (IDR) frame; others may not. The message will contain HTTP headers
followed by by a `.mp4` media segment. The following headers will be included:
The server will send messages as follows:
* text: a plaintext error message, followed by the end of stream.
* binary: video data, repeatedly, as described below.
* ping: every 30 seconds.
Each binary message corresponds to one or more frames of video. The first
message is guaranteed to start with a "key" (IDR) frame; others may not. The
message will contain HTTP headers followed by by a `.mp4` media segment. The
following headers will be included:
* `X-Video-Sample-Entry-Id`: An id to use when fetching an initialization segment.
* `X-Recording-Id`: the open id, a period, and the recording id of the
@ -585,10 +591,8 @@ followed by by a `.mp4` media segment. The following headers will be included:
* `X-Media-Time-Range`: the relative media start and end times of these
frames within the recording, as a half-open interval.
The server will also send pings, currently at 30-second intervals.
The WebSocket will always open immediately but will receive messages only while the
backing RTSP stream is connected.
The WebSocket will always open immediately but will receive messages only while
the backing RTSP stream is connected.
Example request URI:
@ -946,8 +950,9 @@ to a Unix socket with `ownUidIsPrivileged`):
[CORS](https://en.wikipedia.org/wiki/Cross-origin_resource_sharing) policy.
Thus, cross-domain Ajax requests (via `XMLHTTPRequest` or `fetch`) should
fail.
* WebSocket upgrade requests are rejected unless the `Origin` and `Host`
headers match.
* WebSocket upgrade requests are rejected if the `Origin` header is present
and does not match `Host`. This is the sole protection against
[Cross-Site WebSocket Hijacting (CSWSH)](https://christian-schneider.net/CrossSiteWebSocketHijacking.html).
The following additional protections apply only when using session
authentication:

View File

@ -167,17 +167,26 @@ macro_rules! bail_t {
/// Like `failure::format_err!`, but the first argument specifies a type as an `ErrorKind`.
///
/// Example:
/// Example with positional arguments:
/// ```
/// use moonfire_base::format_err_t;
/// let e = format_err_t!(Unauthenticated, "unknown user: {}", "slamb");
/// assert_eq!(e.kind(), moonfire_base::ErrorKind::Unauthenticated);
/// assert_eq!(e.to_string(), "Unauthenticated: unknown user: slamb");
/// ```
///
/// Example with named arguments:
/// ```
/// use moonfire_base::format_err_t;
/// let user = "slamb";
/// let e = format_err_t!(Unauthenticated, "unknown user: {user}");
/// assert_eq!(e.kind(), moonfire_base::ErrorKind::Unauthenticated);
/// assert_eq!(e.to_string(), "Unauthenticated: unknown user: slamb");
/// ```
#[macro_export]
macro_rules! format_err_t {
($t:ident, $e:expr) => {
Into::<$crate::Error>::into(failure::err_msg($e).context($crate::ErrorKind::$t))
($t:ident, $fmt:expr) => {
Into::<$crate::Error>::into(failure::err_msg(format!($fmt)).context($crate::ErrorKind::$t))
};
($t:ident, $fmt:expr, $($arg:tt)+) => {
Into::<$crate::Error>::into(failure::err_msg(format!($fmt, $($arg)+))

View File

@ -6,72 +6,25 @@
use std::sync::Arc;
use crate::body::Body;
use base::{bail_t, format_err_t};
use failure::Error;
use base::{bail_t, format_err_t, Error};
use futures::{future::Either, SinkExt, StreamExt};
use http::{header, Request, Response, StatusCode};
use log::{info, warn};
use tokio_tungstenite::tungstenite;
use http::header;
use tokio_tungstenite::{tungstenite, WebSocketStream};
use uuid::Uuid;
use crate::{mp4, web::plain_response};
use crate::mp4;
use super::{bad_req, Caller, ResponseResult, Service};
/// Checks the `Host` and `Origin` headers match, if the latter is supplied.
///
/// Web browsers must supply origin, according to [RFC 6455 section
/// 4.1](https://datatracker.ietf.org/doc/html/rfc6455#section-4.1).
/// It's not required for non-browser HTTP clients.
///
/// If present, verify it. Chrome doesn't honor the `s=` cookie's
/// `SameSite=Lax` setting for WebSocket requests, so this is the sole
/// protection against [CSWSH](https://christian-schneider.net/CrossSiteWebSocketHijacking.html).
fn check_origin(headers: &header::HeaderMap) -> Result<(), super::HttpError> {
let origin_hdr = match headers.get(http::header::ORIGIN) {
None => return Ok(()),
Some(o) => o,
};
let host_hdr = headers
.get(header::HOST)
.ok_or_else(|| bad_req("missing Host header"))?;
let host_str = host_hdr.to_str().map_err(|_| bad_req("bad Host header"))?;
// Currently this ignores the port number. This is easiest and I think matches the browser's
// rules for when it sends a cookie, so it probably doesn't cause great security problems.
let host = match host_str.split_once(':') {
Some((host, _port)) => host,
None => host_str,
};
let origin_url = origin_hdr
.to_str()
.ok()
.and_then(|o| url::Url::parse(o).ok())
.ok_or_else(|| bad_req("bad Origin header"))?;
let origin_host = origin_url
.host_str()
.ok_or_else(|| bad_req("bad Origin header"))?;
if host != origin_host {
bail_t!(
PermissionDenied,
"cross-origin request forbidden (request host {:?}, origin {:?})",
host_hdr,
origin_hdr
);
}
Ok(())
}
use super::{Caller, Service};
impl Service {
pub(super) fn stream_live_m4s(
pub(super) async fn stream_live_m4s(
self: Arc<Self>,
req: Request<::hyper::Body>,
caller: Caller,
ws: &mut WebSocketStream<hyper::upgrade::Upgraded>,
caller: Result<Caller, Error>,
uuid: Uuid,
stream_type: db::StreamType,
) -> ResponseResult {
check_origin(req.headers())?;
) -> Result<(), Error> {
let caller = caller?;
if !caller.permissions.view_video {
bail_t!(PermissionDenied, "view_video required");
}
@ -90,12 +43,11 @@ impl Service {
}
Some(o) => o.id,
};
let camera = db.get_camera(uuid).ok_or_else(|| {
plain_response(StatusCode::NOT_FOUND, format!("no such camera {uuid}"))
})?;
stream_id = camera.streams[stream_type.index()].ok_or_else(|| {
format_err_t!(NotFound, "no such stream {}/{}", uuid, stream_type)
})?;
let camera = db
.get_camera(uuid)
.ok_or_else(|| format_err_t!(NotFound, "no such camera {uuid}"))?;
stream_id = camera.streams[stream_type.index()]
.ok_or_else(|| format_err_t!(NotFound, "no such stream {uuid}/{stream_type}"))?;
db.watch_live(
stream_id,
Box::new(move |l| sub_tx.unbounded_send(l).is_ok()),
@ -103,54 +55,6 @@ impl Service {
.expect("stream_id refed by camera");
}
let response =
tungstenite::handshake::server::create_response_with_body(&req, hyper::Body::empty)
.map_err(|e| bad_req(e.to_string()))?;
let (parts, _) = response.into_parts();
tokio::spawn(self.stream_live_m4s_ws(stream_id, open_id, req, sub_rx));
Ok(Response::from_parts(parts, Body::from("")))
}
async fn stream_live_m4s_ws(
self: Arc<Self>,
stream_id: i32,
open_id: u32,
req: hyper::Request<hyper::Body>,
sub_rx: futures::channel::mpsc::UnboundedReceiver<db::LiveSegment>,
) {
let upgraded = match hyper::upgrade::on(req).await {
Ok(u) => u,
Err(e) => {
warn!("Unable to upgrade stream to websocket: {}", e);
return;
}
};
let ws = tokio_tungstenite::WebSocketStream::from_raw_socket(
upgraded,
tungstenite::protocol::Role::Server,
None,
)
.await;
if let Err(e) = self
.stream_live_m4s_ws_loop(stream_id, open_id, sub_rx, ws)
.await
{
info!("Dropping WebSocket after error: {}", e);
}
}
/// Helper for `stream_live_m4s_ws` that returns error when the stream is dropped.
/// The outer function logs the error.
async fn stream_live_m4s_ws_loop(
self: Arc<Self>,
stream_id: i32,
open_id: u32,
sub_rx: futures::channel::mpsc::UnboundedReceiver<db::LiveSegment>,
mut ws: tokio_tungstenite::WebSocketStream<hyper::upgrade::Upgraded>,
) -> Result<(), Error> {
let keepalive = tokio_stream::wrappers::IntervalStream::new(tokio::time::interval(
std::time::Duration::new(30, 0),
));
@ -169,18 +73,29 @@ impl Service {
.unwrap_or_else(|| unreachable!("timer stream never ends"));
match next {
Either::Left(live) => {
self.stream_live_m4s_chunk(open_id, stream_id, &mut ws, live, start_at_key)
.await?;
if !self
.stream_live_m4s_chunk(open_id, stream_id, ws, live, start_at_key)
.await?
{
return Ok(());
}
start_at_key = false;
}
Either::Right(_) => {
ws.send(tungstenite::Message::Ping(Vec::new())).await?;
if ws
.send(tungstenite::Message::Ping(Vec::new()))
.await
.is_err()
{
return Ok(());
}
}
}
}
}
/// Sends a single live segment chunk of a `live.m4s` stream.
/// Sends a single live segment chunk of a `live.m4s` stream, returning `Ok(false)` when
/// the connection is lost.
async fn stream_live_m4s_chunk(
&self,
open_id: u32,
@ -188,7 +103,7 @@ impl Service {
ws: &mut tokio_tungstenite::WebSocketStream<hyper::upgrade::Upgraded>,
live: db::LiveSegment,
start_at_key: bool,
) -> Result<(), Error> {
) -> Result<bool, Error> {
let mut builder = mp4::FileBuilder::new(mp4::Type::MediaSegment);
let mut row = None;
{
@ -200,11 +115,8 @@ impl Service {
builder.append(&db, r, live.media_off_90k.clone(), start_at_key)?;
Ok(())
})?;
if rows != 1 {
bail_t!(Internal, "unable to find {:?}", live);
}
}
let row = row.unwrap();
let row = row.ok_or_else(|| format_err_t!(Internal, "unable to find {:?}", live))?;
use http_serve::Entity;
let mp4 = builder.build(self.db.clone(), self.dirs_by_stream_id.clone())?;
let mut hdrs = header::HeaderMap::new();
@ -231,38 +143,6 @@ impl Service {
);
let mut v = hdr.into_bytes();
mp4.append_into_vec(&mut v).await?;
ws.send(tungstenite::Message::Binary(v)).await?;
Ok(())
}
}
#[cfg(test)]
mod tests {
use std::convert::TryInto;
use super::*;
#[test]
fn origin_port_8080_okay() {
// By default, Moonfire binds to port 8080. Make sure that specifying a port number works.
let mut hdrs = header::HeaderMap::new();
hdrs.insert(header::HOST, "nvr:8080".try_into().unwrap());
hdrs.insert(header::ORIGIN, "http://nvr:8080/".try_into().unwrap());
assert!(check_origin(&hdrs).is_ok());
}
#[test]
fn origin_missing_okay() {
let mut hdrs = header::HeaderMap::new();
hdrs.insert(header::HOST, "nvr".try_into().unwrap());
assert!(check_origin(&hdrs).is_ok());
}
#[test]
fn origin_mismatch_fails() {
let mut hdrs = header::HeaderMap::new();
hdrs.insert(header::HOST, "nvr".try_into().unwrap());
hdrs.insert(header::ORIGIN, "http://evil/".try_into().unwrap());
assert!(check_origin(&hdrs).is_err());
Ok(ws.send(tungstenite::Message::Binary(v)).await.is_ok())
}
}

View File

@ -10,6 +10,7 @@ mod signals;
mod static_file;
mod users;
mod view;
mod websocket;
use self::accept::ConnData;
use self::path::Path;
@ -237,15 +238,33 @@ impl Service {
}
/// Serves an HTTP request.
///
/// Note that the `serve` wrapper handles responses the same whether they
/// are `Ok` or `Err`. But returning `Err` here with the `?` operator is
/// convenient for error paths.
async fn serve_inner(
self: Arc<Self>,
req: Request<::hyper::Body>,
p: Path,
caller: Caller,
conn_data: ConnData,
) -> ResponseResult {
let p = Path::decode(req.uri().path());
let always_allow_unauthenticated = matches!(
p,
Path::NotFound | Path::Request | Path::Login | Path::Logout | Path::Static
);
let caller = self.authenticate(&req, &conn_data, always_allow_unauthenticated);
// WebSocket stuff is handled separately, because most authentication
// errors are returned as text messages over the protocol, rather than
// HTTP-level errors.
if let Path::StreamLiveMp4Segments(uuid, type_) = p {
return websocket::upgrade(req, move |ws| {
Box::pin(self.stream_live_m4s(ws, caller, uuid, type_))
});
}
let caller = caller?;
debug!("request on: {}: {:?}", req.uri(), p);
let (cache, mut response) = match p {
Path::InitSegment(sha1, debug) => (
CacheControl::PrivateStatic,
@ -266,10 +285,9 @@ impl Service {
CacheControl::PrivateStatic,
self.stream_view_mp4(&req, caller, uuid, type_, mp4::Type::MediaSegment, debug)?,
),
Path::StreamLiveMp4Segments(uuid, type_) => (
CacheControl::PrivateDynamic,
self.stream_live_m4s(req, caller, uuid, type_)?,
),
Path::StreamLiveMp4Segments(..) => {
unreachable!("StreamLiveMp4Segments should have already been handled")
}
Path::NotFound => return Err(not_found("path not understood")),
Path::Login => (CacheControl::PrivateDynamic, self.login(req).await?),
Path::Logout => (CacheControl::PrivateDynamic, self.logout(req).await?),
@ -313,18 +331,8 @@ impl Service {
req: Request<::hyper::Body>,
conn_data: ConnData,
) -> Result<Response<Body>, std::convert::Infallible> {
let p = Path::decode(req.uri().path());
let always_allow_unauthenticated = matches!(
p,
Path::NotFound | Path::Request | Path::Login | Path::Logout | Path::Static
);
debug!("request on: {}: {:?}", req.uri(), p);
let caller = match self.authenticate(&req, &conn_data, always_allow_unauthenticated) {
Ok(c) => c,
Err(e) => return Ok(from_base_error(e)),
};
Ok(self
.serve_inner(req, p, caller)
.serve_inner(req, conn_data)
.await
.unwrap_or_else(|e| e.0))
}
@ -575,7 +583,7 @@ impl Service {
// Log the specific reason this session is unauthenticated.
// Don't let the API client see it, as it may have a
// revocation reason that isn't for their eyes.
warn!("Session authentication failed: {:?}", &e);
warn!("Session authentication failed: {e}");
}
Err(e) => return Err(e),
};

136
server/src/web/websocket.rs Normal file
View File

@ -0,0 +1,136 @@
// This file is part of Moonfire NVR, a security camera network video recorder.
// Copyright (C) 2021 The Moonfire NVR Authors; see AUTHORS and LICENSE.txt.
// SPDX-License-Identifier: GPL-v3.0-or-later WITH GPL-3.0-linking-exception.
//! Common code for WebSockets, including the live view WebSocket and a future
//! WebSocket for watching database changes.
use std::pin::Pin;
use crate::body::Body;
use base::bail_t;
use futures::{Future, SinkExt};
use http::{header, Request, Response};
use tokio_tungstenite::{tungstenite, WebSocketStream};
use super::{bad_req, ResponseResult};
/// Upgrades to WebSocket and runs the supplied stream handler in a separate tokio task.
///
/// Fails on `Origin` mismatch with an HTTP-level error. If the handler returns
/// an error, tries to send it to the client before dropping the stream.
pub(super) fn upgrade<H>(req: Request<::hyper::Body>, handler: H) -> ResponseResult
where
for<'a> H: FnOnce(
&'a mut WebSocketStream<hyper::upgrade::Upgraded>,
) -> Pin<Box<dyn Future<Output = Result<(), base::Error>> + Send + 'a>>
+ Send
+ 'static,
{
// An `Origin` mismatch should be a HTTP-level error; this is likely a cross-site attack,
// and using HTTP-level errors avoids giving any information to the Javascript running in
// the browser.
check_origin(req.headers())?;
// Otherwise, upgrade and handle the rest in a separate task.
let response =
tungstenite::handshake::server::create_response_with_body(&req, hyper::Body::empty)
.map_err(|e| bad_req(e.to_string()))?;
let (parts, _) = response.into_parts();
tokio::spawn(async move {
let upgraded = match hyper::upgrade::on(req).await {
Ok(u) => u,
Err(e) => {
log::error!("WebSocket upgrade failed: {e}");
return;
}
};
let mut ws = tokio_tungstenite::WebSocketStream::from_raw_socket(
upgraded,
tungstenite::protocol::Role::Server,
None,
)
.await;
if let Err(e) = handler(&mut ws).await {
// TODO: use a nice JSON message format for errors.
log::error!("WebSocket stream terminating with error {e}");
let _ = ws.send(tungstenite::Message::Text(e.to_string())).await;
}
});
Ok(Response::from_parts(parts, Body::from("")))
}
/// Checks the `Host` and `Origin` headers match, if the latter is supplied.
///
/// Web browsers must supply origin, according to [RFC 6455 section
/// 4.1](https://datatracker.ietf.org/doc/html/rfc6455#section-4.1).
/// It's not required for non-browser HTTP clients.
///
/// If present, verify it. Chrome doesn't honor the `s=` cookie's
/// `SameSite=Lax` setting for WebSocket requests, so this is the sole
/// protection against [CSWSH](https://christian-schneider.net/CrossSiteWebSocketHijacking.html).
fn check_origin(headers: &header::HeaderMap) -> Result<(), super::HttpError> {
let origin_hdr = match headers.get(http::header::ORIGIN) {
None => return Ok(()),
Some(o) => o,
};
let host_hdr = headers
.get(header::HOST)
.ok_or_else(|| bad_req("missing Host header"))?;
let host_str = host_hdr.to_str().map_err(|_| bad_req("bad Host header"))?;
// Currently this ignores the port number. This is easiest and I think matches the browser's
// rules for when it sends a cookie, so it probably doesn't cause great security problems.
let host = match host_str.split_once(':') {
Some((host, _port)) => host,
None => host_str,
};
let origin_url = origin_hdr
.to_str()
.ok()
.and_then(|o| url::Url::parse(o).ok())
.ok_or_else(|| bad_req("bad Origin header"))?;
let origin_host = origin_url
.host_str()
.ok_or_else(|| bad_req("bad Origin header"))?;
if host != origin_host {
bail_t!(
PermissionDenied,
"cross-origin request forbidden (request host {:?}, origin {:?})",
host_hdr,
origin_hdr
);
}
Ok(())
}
#[cfg(test)]
mod tests {
use std::convert::TryInto;
use super::*;
#[test]
fn origin_port_8080_okay() {
// By default, Moonfire binds to port 8080. Make sure that specifying a port number works.
let mut hdrs = header::HeaderMap::new();
hdrs.insert(header::HOST, "nvr:8080".try_into().unwrap());
hdrs.insert(header::ORIGIN, "http://nvr:8080/".try_into().unwrap());
assert!(check_origin(&hdrs).is_ok());
}
#[test]
fn origin_missing_okay() {
let mut hdrs = header::HeaderMap::new();
hdrs.insert(header::HOST, "nvr".try_into().unwrap());
assert!(check_origin(&hdrs).is_ok());
}
#[test]
fn origin_mismatch_fails() {
let mut hdrs = header::HeaderMap::new();
hdrs.insert(header::HOST, "nvr".try_into().unwrap());
hdrs.insert(header::ORIGIN, "http://evil/".try_into().unwrap());
assert!(check_origin(&hdrs).is_err());
}
}

View File

@ -110,7 +110,10 @@ class LiveCameraDriver {
};
onWsClose = (e: CloseEvent) => {
this.error(`ws close: ${e.code} ${e.reason}`);
// e doesn't say much. code is likely 1006, reason is likely empty.
// See the warning here: https://websockets.spec.whatwg.org/#closeWebSocket
const cleanly = e.wasClean ? "cleanly" : "uncleanly";
this.error(`connection closed ${cleanly}`);
};
onWsOpen = (e: Event) => {
@ -118,13 +121,18 @@ class LiveCameraDriver {
};
onWsError = (e: Event) => {
console.error(`${this.camera.shortName}: ws error`);
console.error(`${this.camera.shortName}: ws error`, e);
};
onWsMessage = async (e: MessageEvent<Blob>) => {
onWsMessage = async (e: MessageEvent<any>) => {
if (typeof e.data === "string") {
// error message.
this.error(`server: ${e.data}`);
return;
}
let raw;
try {
raw = new Uint8Array(await e.data.arrayBuffer());
raw = new Uint8Array(await (e.data as Blob).arrayBuffer());
} catch (e) {
if (!(e instanceof DOMException)) {
throw e;