Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

user authentication #26

Open
10 of 13 tasks
scottlamb opened this issue Mar 9, 2018 · 5 comments
Open
10 of 13 tasks

user authentication #26

scottlamb opened this issue Mar 9, 2018 · 5 comments
Assignees
Labels
enhancement javascript Pull requests that update Javascript code rust Rust backend work required schema change Requires a database schema change (see new-schema branch)
Milestone

Comments

@scottlamb
Copy link
Owner

scottlamb commented Mar 9, 2018

Current status: functional but some features missing:

  • schema for users and sessions
  • moonfire-nvr config for users
  • API endpoints to log in and out
  • check authentication status on API requests
  • moonfire-nvr login command to make a new session from the CLI (mostly for tools to use)
  • add the Varies: cookie or whatever headers to ensure authenticated and unauthenticated requests don't get mixed up in caches.
  • document how to set up a nginx proxy properly. (I have it working on my setup, though.)
  • consider tighter security (__Host + SameSite=strict) when using tls. currently it uses Secure; SameSite=Lax. (Update: I'm happy with the status quo.)
  • add an endpoint that shows what the auth request looks like, to ensure the nginx configuration + --trust-forward-hdrs are working together properly.
  • add ability to invalidate existing sessions.
  • add endpoint to see existing sessions associated with an account + use in JS
  • add a password change endpoint + use in JS
  • add some restriction on when insecure cookies can be served. They're useful for development with a WebPack proxy server (yarn start) but shouldn't ever be served in production. Maybe restrict via the Host header.

Original issue text:

I want proper user authentication; this (along with some manner of https) is a precondition of exposing Moonfire NVR to the Internet.

As @dolfs said, a temporary measure could be to do basic HTTP with a apache/nginx frontend. But I think it shouldn't be too bad to do a proper built-in thing.

Here's my proposal.

https is a prerequisite. (either built-in or through a proxy.)

You can create/delete/modify users through moonfire-nvr config.

Creating a session could be simply password-based. Some things that I consider out of scope:

  • two-factor authentication (maybe later)
  • email-based password recovery (maybe later)
  • web-based password changes (later)
  • OAuth2 (probably never; I want it to work without an Internet connection)
  • protection against online brute force attacks: rate limiting and/or auto password disable after too many failed guesses (maybe later; you've gotta pick a good password for now)
  • even logging of auth failures (likewise; pick a really good password for now)
  • roles / permissions (for now, the web interface is read-only, and everyone can read everything)
  • user/device settings, such as UI preferences stored on the server side (maybe later)

Sessions could simply be a long random number, stored as a hash for the same reason people hash passwords: so a database leak doesn't trivially compromise the credentials. (They should need to edit the database for to gain access, which may be harder to do than getting an old backup, and which leaves more evidence behind.)

Straw man schema:

create table users (
  id integer primary key,
  username unique not null,

  -- If set, a hash for password authentication, as generated by `libpasta::hash_password`.
  password_hash text,
  password_id integer not null default 0,  -- increasing with password sets/clears.
  password_failure_count integer not null,  -- updated lazily; reset when password_id incremented.

  -- If set, a Unix UID that is accepted for authentication when using HTTP over
  -- a Unix domain socket. (Additionally, the UID running Moonfire NVR can authenticate
  -- as anyone; there's no point in trying to do otherwise.) This might be an easy
  -- bootstrap method once configuration happens through a web UI rather than text UI.
  unix_uid integer
);

create table user_sessions (
  user_id integer references user (id),
  hashed_session_id blob unique not null,  -- hash function TBD
  creation_password_id integer not null, -- the id it was created from
  creation_peer_addr blob not null,  -- IPv4 or IPv6 address
  creation_time integer not null,  -- sec since epoch
  creation_user_agent text,
  revocation_time integer, -- likewise
  description text, -- probably a device name
  last_use_time integer,  -- updated lazily on flush
  use_count not null -- likewise
);

Typical sessions should probably be represented as cookies with the HttpOnly flag set (meaning not accessible to Javascript). If a hit on /api/... lacks the cookie, it should return a HTTP 401 which the Javascript knows means to redirect the browser to /login, a simple HTML login form which on success sets the cookie and redirects back.

The cookies should also be secure (meaning only sent over https, not http).

There should also be a way of getting a session id for use by some automated thing. Example: a program that dumps events into the database based on my Elk security system's zone status. Something you can paste into its config file. Better to have separate, high-entropy credentials where possible so you can see what was compromised and disable it independently of the others.

@scottlamb scottlamb added this to the 1.0 milestone Mar 9, 2018
@scottlamb scottlamb added the schema change Requires a database schema change (see new-schema branch) label Mar 11, 2018
@dolfs
Copy link
Contributor

dolfs commented Mar 13, 2018

There should also be a solution for mobile devices using the API. The standard web based approach for that is cumbersome. Consider supporting oauth based approach. Tokens handed out should perhaps be revokable through the web interface, but should otherwise be fairly long term expiration, or we need to also implement the token refresh portion.

scottlamb added a commit that referenced this issue Mar 22, 2018
This is only the database schema, which I'm adding now in the hopes of
freezing schema version 3. There's no way yet to create users, much less
actually authenticate.
@scottlamb scottlamb self-assigned this Nov 2, 2018
scottlamb added a commit that referenced this issue Nov 27, 2018
Some caveats:

  * it doesn't record the peer IP yet, which makes it harder to verify
    sessions are valid. This is a little annoying to do in hyper now
    (see hyperium/hyper#1410). The direct peer might not be what we want
    right now anyway because there's no TLS support yet (see #27).  In
    the meantime, the sane way to expose Moonfire NVR to the Internet is
    via a proxy server, and recording the proxy's IP is not useful.
    Maybe better to interpret a RFC 7239 Forwarded header (and/or
    the older X-Forwarded-{For,Proto} headers).

  * it doesn't ever use Secure (https-only) cookies, for a similar reason.
    It's not safe to use even with a tls proxy until this is fixed.

  * there's no "moonfire-nvr config" support for inspecting/invalidating
    sessions yet.

  * in debug builds, logging in is crazy slow. See libpasta/libpasta#9.

Some notes:

  * I removed the Javascript "no-use-before-defined" lint, as some of
    the functions form a cycle.

  * Fixed #20 along the way. I needed to add support for properly
    returning non-OK HTTP statuses to signal unauthorized and such.

  * I removed the Access-Control-Allow-Origin header support, which was
    at odds with the "SameSite=lax" in the cookie header. The "yarn
    start" method for running a local proxy server accomplishes the same
    thing as the Access-Control-Allow-Origin support in a more secure
    manner.
@scottlamb
Copy link
Owner Author

scottlamb commented Dec 1, 2018

Finally getting there. Some remaining things to do:

[moved to first comment to make the issue more skimmable]

It'll be a lot more slick once we support built-in https (see #27), but I think if we get those checkboxes ticked we can reasonably say it has auth support.

scottlamb added a commit that referenced this issue Dec 2, 2018
I initially chose SameSite=Lax because I thought if a user followed a
link to the landing page, the landing page's ajax requests wouldn't send
the cookie. But I just did an experiment, and that's not true. Only the
initial page load (of a .html file) lacks the cookie. All of its
resources and ajax requests send the cookie. I'm not sure about
document.cookie accesses, but my cookie is HttpOnly anyway, so it's
irrelevant. So no reason to be lax.
scottlamb added a commit that referenced this issue Dec 27, 2018
The guide is not as quick to follow and amateur-friendly as I'd like. A
few things that might improve matters:

   * complete #27 (built-in https+letsencrypt), so that when not sharing
     the port, users don't need to use nginx or certbot.
   * more ubiquitous IPv6 (out of my control but should happen over
     time) to reduce need to share the port
   * embed a dynamic DNS client
   * support UPnP Internet Gateway Device Control Protocol (if common
     routers have this enabled? probably not for security reasons.)

It's progress, though. Enough that I think I'll merge the auth branch
into master shortly.
@scottlamb
Copy link
Owner Author

I still don't have all the checkboxes ticked, but it works okay for me, and it shouldn't urgently require schema changes, so I merged it to master.

@scottlamb scottlamb added js rust Rust backend work required labels Jul 11, 2019
@clydebarrow
Copy link
Contributor

Possible enhancement - assign roles to users. At a minimum this would be admin vs user, where an admin can do everything, a user can only view. This would simply need a boolean added to the user table.

More fine-grained roles would be nice. This could be achieved with a minimal change to the schema by adding a single column to the user table that holds JSON-encoded data, rather than an extra table. Implementing checks for roles throughout the server code would be most of the work.

WRT OAuth - this would be a nice option, especially integrating to external providers like Google or Apple ID, but username/password will need to be available for systems without continuous internet connectivity.

@scottlamb
Copy link
Owner Author

The user table has a permissions column which is a similar idea. Currently there aren't a lot of permissions:

message Permissions {
  bool view_video = 1;
  bool read_camera_configs = 2;

  bool update_signals = 3;
}

but we can expand as needed.

This column is also in the user_session table so a privileged user can create less-privileged sessions, but this isn't really used (yet?).

@scottlamb scottlamb added javascript Pull requests that update Javascript code and removed js labels Sep 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement javascript Pull requests that update Javascript code rust Rust backend work required schema change Requires a database schema change (see new-schema branch)
Projects
None yet
Development

No branches or pull requests

3 participants