From 42fe054d4634738035401b1521f303b16d1418ff Mon Sep 17 00:00:00 2001 From: Scott Lamb Date: Sat, 31 Dec 2022 12:08:26 -0500 Subject: [PATCH 1/3] make `GET /api/` return current permissions This is useful for e.g. deciding whether or not to present the user admin UI in navigation. As part of this change, I adjusted the casing in Permissions, and then all the toml stuff for consistency. Noted in changelog. --- CHANGELOG.md | 2 ++ design/api.md | 49 +++++++++++++++++++++++++---------- design/signal.md | 35 +++++++++++++++++++++++++ guide/build.md | 4 +-- guide/install.md | 4 +-- guide/secure.md | 4 +-- server/db/proto/schema.proto | 22 +++++----------- server/src/cmds/run/config.rs | 4 ++- server/src/cmds/run/mod.rs | 2 +- server/src/json.rs | 13 ++++++---- server/src/web/mod.rs | 1 + server/src/web/users.rs | 10 +++---- 12 files changed, 103 insertions(+), 47 deletions(-) create mode 100644 design/signal.md diff --git a/CHANGELOG.md b/CHANGELOG.md index eb702e7..4a3244f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ Each release is tagged in Git and on the Docker repository ## unreleased +* expect camelCase in `moonfire-nvr.toml` file, for consistency with the JSON + API. You'll need to adjust your config file when upgrading. * use Retina 0.4.3, which is newly compatible with rtsp-simple-server v0.19.3 and some TP-Link cameras. Fixes [#238](https://github.com/scottlamb/moonfire-nvr/issues/238). * expanded API interface for examining and updating users: diff --git a/design/api.md b/design/api.md index 0917b11..a19cc8d 100644 --- a/design/api.md +++ b/design/api.md @@ -2,8 +2,8 @@ Status: **current**. -* [Objective](#objective) -* [Detailed design](#detailed-design) +* [Summary](#summary) +* [Endpoints](#endpoints) * [Authentication](#authentication) * [`POST /api/login`](#post-apilogin) * [`POST /api/logout`](#post-apilogout) @@ -28,11 +28,13 @@ Status: **current**. * [`GET /api/users/`](#get-apiusersid) * [`POST /api/users/`](#post-apiusersid) * [`DELETE /api/users/`](#delete-apiusersid) +* [Types](#types) + * [Permissions](#permissions) -## Objective +## Summary -Allow a JavaScript-based web interface to list cameras and view recordings. -Support external analytics. +A JavaScript-based web interface to list cameras and view recordings. +Supports external analytics. In the future, this is likely to be expanded: @@ -40,8 +42,6 @@ In the future, this is likely to be expanded: * commandline tool over a UNIX-domain socket (at least for bootstrapping web authentication) -## Detailed design - *Note:* italicized terms in this document are defined in the [glossary](glossary.md). Currently the API is considered an internal contract between the server and the @@ -56,6 +56,8 @@ developed tools. All requests for JSON data should be sent with the header `Accept: application/json` (exactly). +## Endpoints + ### Authentication #### `POST /api/login` @@ -184,6 +186,7 @@ The `application/json` response will have a JSON object as follows: considered to have motion when this signal is in this state. * `color` (optional): a recommended color to use in UIs to represent this state, as in the [HTML specification](https://html.spec.whatwg.org/#colours). +* `permissions`: the caller's current `Permissions` object (defined below). * `user`: an object, present only when authenticated: * `name`: a human-readable name * `id`: an integer @@ -840,12 +843,13 @@ a `users` key with a map of id to username. Requires the `admin_users` permission. -Adds a user. Expects a JSON dictionary with the parameters for the user: +Adds a user. Expects a JSON object with the parameters for the user: * `username`: a string, which must be unique. -* `permissions`: a JSON dictionary of permissions. +* `permissions`: see `Permissions` below. * `password` (optional): a string. -* `preferences` (optional): a JSON dictionary. +* `preferences` (optional): an arbitrary JSON object. Interpretation is + up to clients. Returns status 204 (No Content) on success. @@ -856,11 +860,11 @@ not authenticated as the user in question. Returns a HTTP status 200 on success with a JSON dict: -* `preferences`: a JSON dictionary. +* `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`. +* `permissions`: see `Permissions` below. #### `POST /api/users/` @@ -876,11 +880,14 @@ Expects a JSON object: Currently the following fields are supported for `update` and `precondition`: -* `preferences`, a JSON dictionary. +* `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`, which always requires `admin_users` permission to update. +* `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. Returns HTTP status 204 (No Content) on success. @@ -890,6 +897,20 @@ Deletes the given user. Requires the `admin_users` permission. Returns HTTP status 204 (No Content) on success. +## Types + +### Permissions + +A JSON object of permissions to perform various actions: + +* `adminUsers`: bool +* `readCameraConfigs`: bool, read camera configs including credentials +* `updateSignals`: bool +* `viewVideo`: bool + +See endpoints above for more details on the contexts in which these are +required. + [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 diff --git a/design/signal.md b/design/signal.md new file mode 100644 index 0000000..7f6da69 --- /dev/null +++ b/design/signal.md @@ -0,0 +1,35 @@ +# Moonfire NVR Signals + +Status: **draft**. + +"Signals" are what Moonfire NVR uses to describe non-video timeseries data +such as "was motion detected?" or "what mode was my burglar alarm in?" They are +intended to be displayed in the UI with the video scrub bar to aid in finding +a relevant portion of video. + +## Objective + +Goals: + +* represent simple results of on-camera and on-NVR motion detection, e.g.: + `true`, `false`, or `unknown`. +* represent external signals such as burglar alarm state, e.g.: + `off`, `stay`, `away`, `alarm`, or `unknown`. + +Non-goals: + +* provide meaningful data when the NVR has inaccurate system time. +* support internal state necessary for on-NVR motion detection. (This will + be considered separately.) +* support fine-grained outputs such as "what are the bounding boxes of all + detected faces?", "what cells have motion?", audio volume, or audio + spectograms. + +## Overview + +hmm, two ideas: + +* just use timestamps everywhere. allow adding/updating historical data. +* only allow updating the current open. initially, just support setting + current time. then support extending from a previous request. no ability + to fill in while NVR is down. diff --git a/guide/build.md b/guide/build.md index d9f700a..03e7d85 100644 --- a/guide/build.md +++ b/guide/build.md @@ -295,11 +295,11 @@ You'll also need a `/etc/moonfire-nvr.toml`: ```toml [[binds]] ipv4 = "0.0.0.0:8080" -allow_unauthenticated_permissions = { view_video = true } +allowUnauthenticatedPermissions = { viewVideo = true } [[binds]] unix = "/var/lib/moonfire-nvr/sock" -own_uid_is_privileged = true +ownUidIsPrivileged = true ``` Note this configuration is insecure. You can change that via replacing the diff --git a/guide/install.md b/guide/install.md index 4d403f7..a1b06ab 100644 --- a/guide/install.md +++ b/guide/install.md @@ -71,11 +71,11 @@ $ sudo chmod a+rx /usr/local/bin/nvr ```toml [[binds]] ipv4 = "0.0.0.0:8080" -allow_unauthenticated_permissions = { view_video = true } +allowUnauthenticatedPermissions = { viewVideo = true } [[binds]] unix = "/var/lib/moonfire-nvr/sock" -own_uid_is_privileged = true +ownUidIsPrivileged = true ``` `/usr/local/bin/nvr`: diff --git a/guide/secure.md b/guide/secure.md index fc9858c..2ac531e 100644 --- a/guide/secure.md +++ b/guide/secure.md @@ -165,13 +165,13 @@ If you follow the recommended Docker setup, your `/etc/moonfire-nvr.json` will contain this line: ```toml -allow_unauthenticated_permissions = { view_video = true } +allowUnauthenticatedPermissions = { viewVideo = true } ``` Replace it with the following: ```toml -trust_forward_headers = true +trustForwardHeaders = true ``` This change has two effects: diff --git a/server/db/proto/schema.proto b/server/db/proto/schema.proto index 8b14a4e..a24e783 100644 --- a/server/db/proto/schema.proto +++ b/server/db/proto/schema.proto @@ -2,6 +2,11 @@ // Copyright (C) 2018 The Moonfire NVR Authors; see AUTHORS and LICENSE.txt. // SPDX-License-Identifier: GPL-v3.0-or-later WITH GPL-3.0-linking-exception.'; +// Protobuf portion of the Moonfire NVR schema. In general Moonfire's schema +// uses a SQLite3 database with some fields in JSON representation. The protobuf +// stuff is just high-cardinality things that must be compact, e.g. permissions +// that can be stuffed into every user session. + syntax = "proto3"; // Metadata stored in sample file dirs as `/meta`. This is checked @@ -55,25 +60,12 @@ message DirMeta { Open in_progress_open = 4; } -// Permissions to perform actions, currently all simple bools. +// Permissions to perform actions. See description in design/api.md. // -// These indicate actions which may be unnecessary in some contexts. Some -// basic access - like listing the cameras - is currently always allowed. -// See design/api.md for a description of what requires these permissions. -// -// These are used in a few contexts: -// * a session - affects what can be done when using that session to -// authenticate. -// * a user - when a new session is created, it inherits these permissions. -// * on the commandline - to specify what permissions are available for -// unauthenticated access. +// This protobuf form is stored in user and session rows. message Permissions { bool view_video = 1; bool read_camera_configs = 2; - bool update_signals = 3; - - // Administrate user accounts: create, delete accounts; modify passwords of - // accounts other than the caller's own. bool admin_users = 4; } diff --git a/server/src/cmds/run/config.rs b/server/src/cmds/run/config.rs index c8619d2..6913c0e 100644 --- a/server/src/cmds/run/config.rs +++ b/server/src/cmds/run/config.rs @@ -21,6 +21,7 @@ fn default_ui_dir() -> PathBuf { /// Top-level configuration file object. #[derive(Debug, Deserialize)] #[serde(deny_unknown_fields)] +#[serde(rename_all = "camelCase")] pub struct ConfigFile { pub binds: Vec, @@ -42,6 +43,7 @@ pub struct ConfigFile { /// Per-bind configuration. #[derive(Debug, Deserialize)] #[serde(deny_unknown_fields)] +#[serde(rename_all = "camelCase")] pub struct BindConfig { /// The address to bind to. #[serde(flatten)] @@ -70,8 +72,8 @@ pub struct BindConfig { } #[derive(Debug, Deserialize)] -#[serde(rename_all = "lowercase")] #[serde(deny_unknown_fields)] +#[serde(rename_all = "camelCase")] pub enum AddressConfig { /// IPv4 address such as `0.0.0.0:8080` or `127.0.0.1:8080`. Ipv4(std::net::SocketAddrV4), diff --git a/server/src/cmds/run/mod.rs b/server/src/cmds/run/mod.rs index 6a1ad90..ba26ba5 100644 --- a/server/src/cmds/run/mod.rs +++ b/server/src/cmds/run/mod.rs @@ -367,7 +367,7 @@ async fn inner( ui_dir: Some(&config.ui_dir), allow_unauthenticated_permissions: b .allow_unauthenticated_permissions - .as_ref() + .clone() .map(db::Permissions::from), trust_forward_hdrs: b.trust_forward_headers, time_zone_name: time_zone_name.clone(), diff --git a/server/src/json.rs b/server/src/json.rs index 803f4df..c15ce50 100644 --- a/server/src/json.rs +++ b/server/src/json.rs @@ -25,6 +25,8 @@ pub struct TopLevel<'a> { #[serde(serialize_with = "TopLevel::serialize_cameras")] pub cameras: (&'a db::LockedDatabase, bool, bool), + pub permissions: Permissions, + #[serde(skip_serializing_if = "Option::is_none")] pub user: Option, @@ -553,8 +555,9 @@ where } /// API/config analog of `Permissions` defined in `db/proto/schema.proto`. -#[derive(Debug, Default, Deserialize, Serialize, PartialEq, Eq)] +#[derive(Clone, Debug, Default, Deserialize, Serialize, PartialEq, Eq)] #[serde(deny_unknown_fields)] +#[serde(rename_all = "camelCase")] pub struct Permissions { #[serde(default)] view_video: bool, @@ -569,8 +572,8 @@ pub struct Permissions { admin_users: bool, } -impl From<&Permissions> for db::schema::Permissions { - fn from(p: &Permissions) -> Self { +impl From for db::schema::Permissions { + fn from(p: Permissions) -> Self { Self { view_video: p.view_video, read_camera_configs: p.read_camera_configs, @@ -581,8 +584,8 @@ impl From<&Permissions> for db::schema::Permissions { } } -impl From<&db::schema::Permissions> for Permissions { - fn from(p: &db::schema::Permissions) -> Self { +impl From for Permissions { + fn from(p: db::schema::Permissions) -> Self { Self { view_video: p.view_video, read_camera_configs: p.read_camera_configs, diff --git a/server/src/web/mod.rs b/server/src/web/mod.rs index 0978546..b48990f 100644 --- a/server/src/web/mod.rs +++ b/server/src/web/mod.rs @@ -347,6 +347,7 @@ impl Service { user: caller.user, signals: (&db, days), signal_types: &db, + permissions: caller.permissions.into(), }, ) } diff --git a/server/src/web/users.rs b/server/src/web/users.rs index a9fb0ce..7c08960 100644 --- a/server/src/web/users.rs +++ b/server/src/web/users.rs @@ -57,7 +57,7 @@ impl Service { if let Some(preferences) = r.preferences.take() { change.config.preferences = preferences; } - if let Some(ref permissions) = r.permissions.take() { + if let Some(permissions) = r.permissions.take() { change.permissions = permissions.into(); } if r != Default::default() { @@ -101,7 +101,7 @@ impl Service { } else { None }), - permissions: Some((&user.permissions).into()), + permissions: Some(user.permissions.clone().into()), }; serve_json(&req, &out) } @@ -145,7 +145,7 @@ impl Service { (_, _) => {} } if let Some(mut precondition) = r.precondition { - if matches!(precondition.username.take(), Some(n) if n != &user.username) { + if matches!(precondition.username.take(), Some(n) if n != user.username) { bail_t!(FailedPrecondition, "username mismatch"); } if matches!(precondition.preferences.take(), Some(ref p) if p != &user.config.preferences) @@ -158,7 +158,7 @@ impl Service { } } if let Some(p) = precondition.permissions.take() { - if user.permissions != db::Permissions::from(&p) { + if user.permissions != db::Permissions::from(p) { bail_t!(FailedPrecondition, "permissions mismatch"); } } @@ -193,7 +193,7 @@ impl Service { change.username = n.to_string(); } if let Some(permissions) = update.permissions.take() { - change.permissions = (&permissions).into(); + change.permissions = permissions.into(); } // Safety valve in case something is added to UserSubset and forgotten here. From dfa949815bde2c55fa5767ad08889fbdbc3e4b7d Mon Sep 17 00:00:00 2001 From: Scott Lamb Date: Thu, 5 Jan 2023 12:11:28 -0600 Subject: [PATCH 2/3] tweaks to api and docs In particular, the docs now talk about the CSRF protection. This is increasing relevant as we start having more mutation endpoints. And make the signals api expect a csrf for session auth to match the newer users api. --- CHANGELOG.md | 6 ++ design/api.md | 133 +++++++++++++++++++++++++------------- server/src/json.rs | 32 ++++++++- server/src/web/mod.rs | 10 +++ server/src/web/signals.rs | 5 +- server/src/web/users.rs | 43 ++++++------ 6 files changed, 162 insertions(+), 67 deletions(-) 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"); From 7fe2284cec86819e37552cabe8a437050787d365 Mon Sep 17 00:00:00 2001 From: Scott Lamb Date: Thu, 5 Jan 2023 12:44:19 -0600 Subject: [PATCH 3/3] fix docker build (untested) Forgot to update a symlink command in cdfb61f. I'm not able to test this right now because my build machine is down, but this fix should work. --- docker/dev.bash | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docker/dev.bash b/docker/dev.bash index 91c8711..c8735cf 100755 --- a/docker/dev.bash +++ b/docker/dev.bash @@ -90,7 +90,7 @@ time apt-get install --assume-yes --no-install-recommends "${packages[@]}" # https://doc.rust-lang.org/cargo/guide/build-cache.html if [[ -n "${rust_target}" ]]; then su moonfire-nvr -lc "rustup target install ${rust_target} && - ln -s target/${rust_target}/release/moonfire-nvr ." + ln -s target/${rust_target}/release-lto/moonfire-nvr ." underscore_rust_target="${rust_target//-/_}" uppercase_underscore_rust_target="${underscore_rust_target^^}" cat >> /var/lib/moonfire-nvr/.buildrc < /docker-build-debug/dev/var-cache-apt-after