Skip to content

Commit

Permalink
rework WebSocket error return protocol
Browse files Browse the repository at this point in the history 
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).
  • Loading branch information
@scottlamb
scottlamb committed Feb 16, 2023
1 parent 0ffda11 commit 438de38
Show file tree
Hide file tree
Showing 8 changed files with 237 additions and 219 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
31 changes: 0 additions & 31 deletions guide/troubleshooting.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
25 changes: 15 additions & 10 deletions ref/api.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:

Expand Down Expand Up @@ -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:
Expand Down
15 changes: 12 additions & 3 deletions server/base/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)+))
Expand Down
186 changes: 33 additions & 153 deletions server/src/web/live.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand All @@ -90,67 +43,18 @@ 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()),
)
.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),
));
Expand All @@ -169,26 +73,37 @@ 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,
stream_id: i32,
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;
{
Expand All @@ -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();
Expand All @@ -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())
}
}
Loading

0 comments on commit 438de38

Please sign in to comment.