diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a3244f..44765ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,11 @@ changes, see Git history. Each release is tagged in Git and on the Docker repository [`scottlamb/moonfire-nvr`](https://hub.docker.com/r/scottlamb/moonfire-nvr). +Backwards-incompatible database schema changes happen on on major version +upgrades, e.g. `0.6.x` -> `0.7.x`. The config file format and +[API](design/api.md) currently have no stability guarantees, so they may change +even on minor releases, e.g. `0.7.5` -> `0.7.6`. + ## unreleased * expect camelCase in `moonfire-nvr.toml` file, for consistency with the JSON @@ -20,6 +25,7 @@ Each release is tagged in Git and on the Docker repository * expanded `POST /users/` endpoint, including password and permissions. * `DELETE /users/` endpoint to delete a user +* improved API documentation in `design/api.md`. ## 0.7.5 (2022-05-09) diff --git a/design/api.md b/design/api.md index a19cc8d..5b53379 100644 --- a/design/api.md +++ b/design/api.md @@ -23,13 +23,15 @@ Status: **current**. * [Request 2](#request-2) * [Request 3](#request-3) * [User management](#user-management) - * [`GET /api/users`](#get-apiusers) - * [`PUT /api/users`](#put-apiusers) + * [`GET /api/users/`](#get-apiusers) + * [`PUT /api/users/`](#put-apiusers) * [`GET /api/users/`](#get-apiusersid) * [`POST /api/users/`](#post-apiusersid) * [`DELETE /api/users/`](#delete-apiusersid) * [Types](#types) + * [UserSubset](#usersubset) * [Permissions](#permissions) +* [Cross-site request forgery (CSRF) protection](#cross-site-request-forgery-csrf-protection) ## Summary @@ -91,8 +93,7 @@ request parameters: should be included. * `cameraConfigs`: a boolean indicating if the `camera.config` and `camera.stream[].config` parameters described below should be included. - This requires the `read_camera_configs` permission as described in - `schema.proto`. + This requires the `readCameraConfigs` permission. Example request URI (with added whitespace between parameters): @@ -192,8 +193,6 @@ The `application/json` response will have a JSON object as follows: * `id`: an integer * `preferences`: a JSON object * `session`: an object, present only if authenticated via session cookie. - (In the future, it will be possible to instead authenticate via uid over - a Unix domain socket.) * `csrf`: a cross-site request forgery token for use in `POST` requests. Example response: @@ -430,7 +429,7 @@ Example response: ### `GET /api/cameras///view.mp4` -Requires the `view_video` permission. +Requires the `viewVideo` permission. Returns a `.mp4` file, with an etag and support for range requests. The MIME type will be `video/mp4`, with a `codecs` parameter as specified in @@ -720,7 +719,7 @@ This represents the following observations: ### `POST /api/signals` -Requires the `update_signals` permission. +Requires the `updateSignals` permission. Alters the state of a signal. @@ -738,6 +737,7 @@ last ran. These will specify beginning and end times. The request should have an `application/json` body describing the change to make. It should be a JSON object with these attributes: +* `csrf`: a CSRF token, required when using session authentication. * `signalIds`: a list of signal ids to change. Must be sorted. * `states`: a list (one per `signalIds` entry) of states to set. * `start`: the starting time of the change, as a JSON object of the form @@ -832,73 +832,80 @@ Response: ### User management -#### `GET /api/users` +#### `GET /api/users/` -Requires the `admin_users` permission. +Requires the `adminUsers` permission. Lists all users. Currently there's no paging. Returns a JSON object with -a `users` key with a map of id to username. +a `users` key with an array of objects, each with the following keys: -#### `PUT /api/users` +* `id`: a number. +* `username`: a string. -Requires the `admin_users` permission. +#### `PUT /api/users/` -Adds a user. Expects a JSON object with the parameters for the user: +Requires the `adminUsers` permission. -* `username`: a string, which must be unique. -* `permissions`: see `Permissions` below. -* `password` (optional): a string. -* `preferences` (optional): an arbitrary JSON object. Interpretation is - up to clients. +Adds a user. Expects a JSON object as follows: + +* `csrf`: a CSRF token, required when using session authentication. +* `user`: a `UserSubset` as defined below. Returns status 204 (No Content) on success. #### `GET /api/users/` -Retrieves the user. Requires the `admin_users` permission if the caller is +Retrieves the user. Requires the `adminUsers` permission if the caller is not authenticated as the user in question. -Returns a HTTP status 200 on success with a JSON dict: - -* `preferences`: a JSON object. -* `password`: absent (no password set) or a placeholder string to indicate - the password is set. Passwords are stored hashed, so the cleartext can not - be retrieved. -* `permissions`: see `Permissions` below. +Returns a HTTP status 200 on success with a JSON `UserSubset`. The `password` +will be absent (for no password) or a placeholder string to indicate the +password is set. Passwords are stored hashed, so the cleartext can not be +retrieved. #### `POST /api/users/` -Allows updating the given user. Requires the `admin_users` permission if the -caller is not authenticated as the user in question. +Updates the given user. Requires the `adminUsers` permission if the caller is +not authenticated as the user in question. Expects a JSON object: * `csrf`: a CSRF token, required when using session authentication. -* `update`: sets the provided fields -* `precondition`: forces the request to fail with HTTP status 412 - (Precondition failed) if the provided fields don't have the given value. - -Currently the following fields are supported for `update` and `precondition`: - -* `preferences`, a JSON object. -* `password`, a cleartext string. When updating the password, the previous - password must be supplied as a precondition, unless the caller has - `admin_users` permission. -* `permissions`, a `Permissions` as described below, which always requires - `admin_users` permission to update. Note that updating a user's permissions - currently neither adds nor limits permissions of existing sessions; it only - changes what is available to newly created sessions. +* `update`: `UserSubset`, sets the provided fields. Field-specific notes: + * `password`: when updating the password, the previous password must + be supplied as a precondition, unless the caller has `admin_users` + permission. + * `permissions`: requires `adminUsers` permission. Note that updating a + user's permissions currently neither adds nor limits permissions of + existing sessions; it only changes what is available to newly created + sessions. +* `precondition`: `UserSubset`, forces the request to fail with HTTP status + 412 (Precondition failed) if the provided fields don't have the given + values. Returns HTTP status 204 (No Content) on success. #### `DELETE /api/users/` -Deletes the given user. Requires the `admin_users` permission. +Deletes the given user. Requires the `adminUsers` permission. + +Expects a JSON object body with the following parameters: + +* `csrf`: a CSRF token, required when using session authentication. Returns HTTP status 204 (No Content) on success. ## Types +### UserSubset + +A JSON object with any of the following parameters: + +* `preferences`, a JSON object which the server stores without interpreting. + This field is meant for user-level preferences meaningful to the UI. +* `password`, a cleartext string. +* `permissions`, a `Permissions` as described below. + ### Permissions A JSON object of permissions to perform various actions: @@ -911,8 +918,46 @@ A JSON object of permissions to perform various actions: See endpoints above for more details on the contexts in which these are required. +## Cross-site request forgery (CSRF) protection + +The API includes several standard protections against [cross-site request +forgery](https://en.wikipedia.org/wiki/Cross-site_request_forgery) attacks, in +which a malicious third-party website convinces the user's browser to send +requests on its behalf. + +The following protections apply regardless of method of authentication (session, +no authentication + `allowUnauthenticatedPermissions`, or the browser proxying +to a Unix socket with `ownUidIsPrivileged`): + +* The `GET` method is always "safe". Actions that have significant side + effects require another method such as `DELETE`, `POST`, or `PUT`. This + prevents simple hyperlinks from causing damage. +* Mutations always require some non-default request header (e.g. + `Content-Type: application/json`) so that a `
` will be + rejected. +* The server does *not* override the default + [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. + +The following additional protections apply only when using session +authentication: + +* Session cookies are set with the [`SameSite=Lax` attribute](samesite-lax), + so that sufficiently modern web browsers will never send the session cookie + on subrequests. (Note they still send session cookies when following links + and on WebSocket upgrade. In these cases, we rely on the protections + described above.) +* Mutations use a `csrf` token, requiring the caller to prove it is able + to read the `GET /api/` response. (This is subect to change. We may decide + to implement these tokens in a way that doesn't require session + authentication or decide they're entirely unnecessary.) + [media-segment]: https://w3c.github.io/media-source/isobmff-byte-stream-format.html#iso-media-segments [init-segment]: https://w3c.github.io/media-source/isobmff-byte-stream-format.html#iso-init-segments [rfc-6381]: https://tools.ietf.org/html/rfc6381 [rfc-6455]: https://tools.ietf.org/html/rfc6455 [multipart-mixed-js]: https://github.com/scottlamb/multipart-mixed-js +[samesite-lax]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite#lax diff --git a/server/src/json.rs b/server/src/json.rs index c15ce50..cb5db23 100644 --- a/server/src/json.rs +++ b/server/src/json.rs @@ -9,7 +9,6 @@ use db::auth::SessionHash; use failure::{format_err, Error}; use serde::ser::{Error as _, SerializeMap, SerializeSeq, Serializer}; use serde::{Deserialize, Deserializer, Serialize}; -use std::collections::BTreeMap; use std::ops::Not; use uuid::Uuid; @@ -123,12 +122,15 @@ pub struct LoginRequest<'a> { #[derive(Deserialize)] #[serde(rename_all = "camelCase")] pub struct LogoutRequest<'a> { + #[serde(borrow)] pub csrf: &'a str, } #[derive(Deserialize)] #[serde(rename_all = "camelCase")] -pub struct PostSignalsRequest { +pub struct PostSignalsRequest<'a> { + #[serde(borrow)] + pub csrf: Option<&'a str>, pub signal_ids: Vec, pub states: Vec, pub start: PostSignalsTimeBase, @@ -516,15 +518,33 @@ pub struct ToplevelUser { pub session: Option, } +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +#[serde(deny_unknown_fields)] +pub struct PutUsers<'a> { + #[serde(borrow)] + pub csrf: Option<&'a str>, + pub user: UserSubset<'a>, +} + #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] #[serde(deny_unknown_fields)] pub struct PostUser<'a> { + #[serde(borrow)] pub csrf: Option<&'a str>, pub update: Option>, pub precondition: Option>, } +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +#[serde(deny_unknown_fields)] +pub struct DeleteUser<'a> { + #[serde(borrow)] + pub csrf: Option<&'a str>, +} + #[derive(Debug, Default, Deserialize, Serialize, PartialEq, Eq)] #[serde(rename_all = "camelCase")] #[serde(deny_unknown_fields)] @@ -598,7 +618,13 @@ impl From for Permissions { /// Response to `GET /api/users/`. #[derive(Serialize)] pub struct GetUsersResponse { - pub users: BTreeMap, + pub users: Vec, +} + +#[derive(Serialize)] +pub struct UserSummary { + pub id: i32, + pub username: String, } /// Response to `PUT /api/users/`. diff --git a/server/src/web/mod.rs b/server/src/web/mod.rs index b48990f..46a3bf8 100644 --- a/server/src/web/mod.rs +++ b/server/src/web/mod.rs @@ -153,6 +153,16 @@ async fn extract_json_body(req: &mut Request) -> Result) -> Result<(), base::Error> { + match (csrf, caller.user.as_ref().and_then(|u| u.session.as_ref())) { + (None, Some(_)) => bail_t!(Unauthenticated, "csrf must be supplied"), + (Some(csrf), Some(session)) if !csrf_matches(csrf, session.csrf) => { + bail_t!(Unauthenticated, "incorrect csrf"); + } + (_, _) => Ok(()), + } +} + pub struct Config<'a> { pub db: Arc, pub ui_dir: Option<&'a std::path::Path>, diff --git a/server/src/web/signals.rs b/server/src/web/signals.rs index b631454..ccee21a 100644 --- a/server/src/web/signals.rs +++ b/server/src/web/signals.rs @@ -12,8 +12,8 @@ use url::form_urlencoded; use crate::json; use super::{ - bad_req, extract_json_body, from_base_error, plain_response, serve_json, Caller, - ResponseResult, Service, + bad_req, extract_json_body, from_base_error, plain_response, require_csrf_if_session, + serve_json, Caller, ResponseResult, Service, }; use std::borrow::Borrow; @@ -42,6 +42,7 @@ impl Service { let r = extract_json_body(&mut req).await?; let r: json::PostSignalsRequest = serde_json::from_slice(&r).map_err(|e| bad_req(e.to_string()))?; + require_csrf_if_session(&caller, r.csrf)?; let now = recording::Time::new(self.db.clocks().realtime()); let mut l = self.db.lock(); let start = match r.start { diff --git a/server/src/web/users.rs b/server/src/web/users.rs index 7c08960..8f8671e 100644 --- a/server/src/web/users.rs +++ b/server/src/web/users.rs @@ -7,11 +7,11 @@ use base::{bail_t, format_err_t}; use http::{Method, Request, StatusCode}; -use crate::json::{self, PutUsersResponse, UserSubset}; +use crate::json::{self, PutUsersResponse, UserSubset, UserSummary}; use super::{ - bad_req, csrf_matches, extract_json_body, plain_response, serve_json, Caller, ResponseResult, - Service, + bad_req, extract_json_body, plain_response, require_csrf_if_session, serve_json, Caller, + ResponseResult, Service, }; impl Service { @@ -34,7 +34,10 @@ impl Service { .lock() .users_by_id() .iter() - .map(|(&id, user)| (id, user.username.clone())) + .map(|(&id, user)| UserSummary { + id, + username: user.username.clone(), + }) .collect(); serve_json(&req, &json::GetUsersResponse { users }) } @@ -44,23 +47,25 @@ impl Service { bail_t!(Unauthenticated, "must have admin_users permission"); } let r = extract_json_body(&mut req).await?; - let mut r: json::UserSubset = + let mut r: json::PutUsers = serde_json::from_slice(&r).map_err(|e| bad_req(e.to_string()))?; + require_csrf_if_session(&caller, r.csrf)?; let username = r + .user .username .take() .ok_or_else(|| format_err_t!(InvalidArgument, "username must be specified"))?; let mut change = db::UserChange::add_user(username.to_owned()); - if let Some(Some(pwd)) = r.password.take() { + if let Some(Some(pwd)) = r.user.password.take() { change.set_password(pwd.to_owned()); } - if let Some(preferences) = r.preferences.take() { + if let Some(preferences) = r.user.preferences.take() { change.config.preferences = preferences; } - if let Some(permissions) = r.permissions.take() { + if let Some(permissions) = r.user.permissions.take() { change.permissions = permissions.into(); } - if r != Default::default() { + if r.user != Default::default() { bail_t!(Unimplemented, "unsupported user fields: {:#?}", r); } let mut l = self.db.lock(); @@ -76,7 +81,7 @@ impl Service { ) -> ResponseResult { match *req.method() { Method::GET | Method::HEAD => self.get_user(req, caller, id).await, - Method::DELETE => self.delete_user(caller, id).await, + Method::DELETE => self.delete_user(req, caller, id).await, Method::POST => self.post_user(req, caller, id).await, _ => Err(plain_response( StatusCode::METHOD_NOT_ALLOWED, @@ -106,10 +111,18 @@ impl Service { serve_json(&req, &out) } - async fn delete_user(&self, caller: Caller, id: i32) -> ResponseResult { + async fn delete_user( + &self, + mut req: Request, + caller: Caller, + id: i32, + ) -> ResponseResult { if !caller.permissions.admin_users { bail_t!(Unauthenticated, "must have admin_users permission"); } + let r = extract_json_body(&mut req).await?; + let r: json::DeleteUser = serde_json::from_slice(&r).map_err(|e| bad_req(e.to_string()))?; + require_csrf_if_session(&caller, r.csrf)?; let mut l = self.db.lock(); l.delete_user(id)?; Ok(plain_response(StatusCode::NO_CONTENT, &b""[..])) @@ -137,13 +150,7 @@ impl Service { "to change password, must supply previous password or have admin_users permission" ); } - match (r.csrf, caller.user.and_then(|u| u.session)) { - (None, Some(_)) => bail_t!(Unauthenticated, "csrf must be supplied"), - (Some(csrf), Some(session)) if !csrf_matches(csrf, session.csrf) => { - bail_t!(Unauthenticated, "incorrect csrf"); - } - (_, _) => {} - } + require_csrf_if_session(&caller, r.csrf)?; if let Some(mut precondition) = r.precondition { if matches!(precondition.username.take(), Some(n) if n != user.username) { bail_t!(FailedPrecondition, "username mismatch");