From 6818c0c48873dc64c5ed98fafdcddfa378208772 Mon Sep 17 00:00:00 2001 From: Tran Duc Anh Dang Date: Thu, 12 Nov 2026 01:38:10 +1100 Subject: [PATCH] feat(auth): sign the device app label into register and login A device may name its app (planner or engineering). It then signs the v2 register or login message, which adds its trimmed name, platform and app to the v1 bytes. The server stores the label and lists it; a device with no label signs the v1 messages as before. --- crates/orbyn-auth-client/src/lib.rs | 130 +++++++ crates/orbyn-client/tests/sync.rs | 1 + crates/orbyn-contracts/bindings/Device.ts | 7 +- crates/orbyn-contracts/bindings/DeviceApp.ts | 9 + crates/orbyn-contracts/bindings/NewDevice.ts | 9 +- crates/orbyn-contracts/src/auth.rs | 42 +++ crates/orbyn-contracts/src/devices.rs | 6 +- crates/orbyn-core/src/accounts/device_app.rs | 79 +++++ crates/orbyn-core/src/accounts/devices.rs | 6 +- crates/orbyn-core/src/accounts/email.rs | 7 +- .../src/accounts/email/verification.rs | 10 +- crates/orbyn-core/src/accounts/login.rs | 15 +- crates/orbyn-core/src/accounts/mod.rs | 12 +- crates/orbyn-core/src/accounts/register.rs | 10 +- .../orbyn-core/src/accounts/second_factor.rs | 6 +- crates/orbyn-device-client/src/client.rs | 60 +++- crates/orbyn-device-client/src/signup.rs | 6 +- crates/orbyn-device-client/tests/client.rs | 41 ++- crates/orbyn-device-client/tests/limits.rs | 1 + .../orbyn-device-client/tests/local_stack.rs | 3 +- crates/orbyn-device-client/tests/notices.rs | 2 + .../orbyn-device-client/tests/support/mod.rs | 65 +++- crates/orbyn-server/src/accounts/support.rs | 16 + crates/orbyn-server/tests/accounts.rs | 332 ++++++++++++++++++ crates/orbyn-server/tests/device_client.rs | 3 + docs/architecture.md | 2 +- docs/device-client.md | 2 +- migrations/0019_device_app.sql | 5 + 28 files changed, 822 insertions(+), 65 deletions(-) create mode 100644 crates/orbyn-contracts/bindings/DeviceApp.ts create mode 100644 crates/orbyn-core/src/accounts/device_app.rs create mode 100644 migrations/0019_device_app.sql diff --git a/crates/orbyn-auth-client/src/lib.rs b/crates/orbyn-auth-client/src/lib.rs index 2daae38..02afd20 100644 --- a/crates/orbyn-auth-client/src/lib.rs +++ b/crates/orbyn-auth-client/src/lib.rs @@ -43,6 +43,10 @@ pub const LOGIN_CONTEXT: &[u8] = b"orbyn/opaque-login/v1"; pub const DEVICE_REGISTER_CONTEXT: &str = "orbyn/device-register/v1"; /// What a device signs at sign-in: [`login_message`]. pub const DEVICE_LOGIN_CONTEXT: &str = "orbyn/device-login/v1"; +/// What a device that names its app signs at sign-up: [`register_message_v2`]. +pub const DEVICE_REGISTER_V2_CONTEXT: &str = "orbyn/device-register/v2"; +/// What a device that names its app signs at sign-in: [`login_message_v2`]. +pub const DEVICE_LOGIN_V2_CONTEXT: &str = "orbyn/device-login/v2"; /// What a trusted device signs to approve another: [`approval_message`]. pub const DEVICE_APPROVAL_CONTEXT: &str = "orbyn/device-approval/v1"; /// See [`approval_message_v2`]. @@ -254,6 +258,48 @@ pub fn login_message(login_id: &[u8; 16], keys: &DevicePublic) -> Vec { out } +/// What a device says about itself in the v2 messages: the name it shows (trimmed), its platform +/// and its app, as the `snake_case` words the wire uses (`linux`, `engineering`). +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct Descriptor<'a> { + pub name: &'a str, + pub platform: &'a str, + pub app: &'a str, +} + +fn push_descriptor(out: &mut Vec, d: &Descriptor<'_>) { + push_len_prefixed(out, d.name.as_bytes()); + push_len_prefixed(out, d.platform.as_bytes()); + push_len_prefixed(out, d.app.as_bytes()); +} + +/// What a device that names its app signs at sign-up, under [`DEVICE_REGISTER_V2_CONTEXT`]: the +/// [`register_message`] bytes followed by its [`Descriptor`]. The label, the name and the platform +/// are then the device's own signed claim: the server cannot attach another to this key at +/// sign-up, and a swap of any of them breaks the signature. +pub fn register_message_v2( + email: &str, + upload: &[u8], + keys: &DevicePublic, + descriptor: &Descriptor<'_>, +) -> Vec { + let mut out = register_message(email, upload, keys); + push_descriptor(&mut out, descriptor); + out +} + +/// What a device that names its app signs at sign-in, under [`DEVICE_LOGIN_V2_CONTEXT`]: the +/// [`login_message`] bytes followed by its [`Descriptor`]. +pub fn login_message_v2( + login_id: &[u8; 16], + keys: &DevicePublic, + descriptor: &Descriptor<'_>, +) -> Vec { + let mut out = login_message(login_id, keys); + push_descriptor(&mut out, descriptor); + out +} + /// What a device that is already trusted signs when it starts a sign-in, under /// [`TRUSTED_START_CONTEXT`]: the normalised email, this sign-in's credential /// request and the time. Bound to the request, so a copy cannot be reused for @@ -730,3 +776,87 @@ mod protection_tests { assert_ne!(a, approval_message(&pairing)); } } + +#[cfg(test)] +mod descriptor_tests { + use super::*; + use orbyn_crypto::DeviceKeys; + + fn descriptor() -> Descriptor<'static> { + Descriptor { + name: "Work laptop", + platform: "linux", + app: "engineering", + } + } + + #[test] + fn the_v2_messages_extend_the_v1_ones_and_bind_name_platform_and_app() { + let keys = DeviceKeys::generate().expect("keys").public(); + let v1 = register_message("a@example.test", b"upload", &keys); + let v2 = register_message_v2("a@example.test", b"upload", &keys, &descriptor()); + assert!(v2.starts_with(&v1) && v2.len() > v1.len()); + let other_name = Descriptor { + name: "Other", + ..descriptor() + }; + let other_platform = Descriptor { + platform: "macos", + ..descriptor() + }; + let other_app = Descriptor { + app: "planner", + ..descriptor() + }; + for other in [other_name, other_platform, other_app] { + assert_ne!( + v2, + register_message_v2("a@example.test", b"upload", &keys, &other) + ); + } + let id = [9u8; 16]; + let login_v2 = login_message_v2(&id, &keys, &descriptor()); + assert!(login_v2.starts_with(&login_message(&id, &keys))); + for other in [other_name, other_platform, other_app] { + assert_ne!(login_v2, login_message_v2(&id, &keys, &other)); + } + } + + #[test] + fn a_field_cannot_slide_into_the_next() { + // Length prefixes keep ("ab", "c") apart from ("a", "bc"). + let keys = DeviceKeys::generate().expect("keys").public(); + let one = Descriptor { + name: "ab", + platform: "c", + app: "engineering", + }; + let two = Descriptor { + name: "a", + platform: "bc", + app: "engineering", + }; + assert_ne!( + register_message_v2("e", b"u", &keys, &one), + register_message_v2("e", b"u", &keys, &two) + ); + } + + #[test] + fn a_v1_signature_is_not_a_v2_signature_and_the_reverse() { + let device = DeviceKeys::generate().expect("keys"); + let keys = device.public(); + let v1 = register_message("a@example.test", b"upload", &keys); + let v2 = register_message_v2("a@example.test", b"upload", &keys, &descriptor()); + let signed_v1 = device.sign(DEVICE_REGISTER_CONTEXT, &v1); + let signed_v2 = device.sign(DEVICE_REGISTER_V2_CONTEXT, &v2); + assert!(orbyn_crypto::verify(&keys, DEVICE_REGISTER_CONTEXT, &v1, &signed_v1).is_ok()); + assert!(orbyn_crypto::verify(&keys, DEVICE_REGISTER_V2_CONTEXT, &v2, &signed_v2).is_ok()); + // Neither passes for the other's context or message, so a label cannot be added to or + // stripped from a signed claim. + assert!(orbyn_crypto::verify(&keys, DEVICE_REGISTER_V2_CONTEXT, &v1, &signed_v1).is_err()); + assert!(orbyn_crypto::verify(&keys, DEVICE_REGISTER_CONTEXT, &v2, &signed_v2).is_err()); + assert!(orbyn_crypto::verify(&keys, DEVICE_REGISTER_CONTEXT, &v1, &signed_v2).is_err()); + assert!(orbyn_crypto::verify(&keys, DEVICE_REGISTER_V2_CONTEXT, &v2, &signed_v1).is_err()); + } +} diff --git a/crates/orbyn-client/tests/sync.rs b/crates/orbyn-client/tests/sync.rs index 600c029..7d5c64a 100644 --- a/crates/orbyn-client/tests/sync.rs +++ b/crates/orbyn-client/tests/sync.rs @@ -57,6 +57,7 @@ fn entry(id: &str, keys: &DeviceKeys, state: wire::DeviceState) -> wire::Device id: id.into(), name: id.into(), platform: wire::DevicePlatform::Web, + app: None, state, signing_key: b64(&public.signing), agreement_key: b64(&public.agreement), diff --git a/crates/orbyn-contracts/bindings/Device.ts b/crates/orbyn-contracts/bindings/Device.ts index ce79a64..f5230e7 100644 --- a/crates/orbyn-contracts/bindings/Device.ts +++ b/crates/orbyn-contracts/bindings/Device.ts @@ -1,11 +1,16 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { DeviceApp } from "./DeviceApp"; import type { DevicePlatform } from "./DevicePlatform"; import type { DeviceState } from "./DeviceState"; /** * A device of the account. */ -export type Device = { id: string, name: string, platform: DevicePlatform, state: DeviceState, signing_key: string, agreement_key: string, +export type Device = { id: string, name: string, platform: DevicePlatform, +/** + * The app the device signed itself in as, if it said (see [`DeviceApp`]). + */ +app?: DeviceApp, state: DeviceState, signing_key: string, agreement_key: string, /** * RFC 3339. */ diff --git a/crates/orbyn-contracts/bindings/DeviceApp.ts b/crates/orbyn-contracts/bindings/DeviceApp.ts new file mode 100644 index 0000000..7cdcb1b --- /dev/null +++ b/crates/orbyn-contracts/bindings/DeviceApp.ts @@ -0,0 +1,9 @@ +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +/** + * Which Orbyn app a device is. It is **signed into the device's register and login messages** + * (`orbyn/device-register/v2` and `orbyn/device-login/v2`, with the device's name and platform), + * so the server checks the device itself claimed it, and the device list shows it. A device that + * sends no label signs the earlier messages and has none. + */ +export type DeviceApp = "planner" | "engineering"; diff --git a/crates/orbyn-contracts/bindings/NewDevice.ts b/crates/orbyn-contracts/bindings/NewDevice.ts index 2f3cd91..8497fe6 100644 --- a/crates/orbyn-contracts/bindings/NewDevice.ts +++ b/crates/orbyn-contracts/bindings/NewDevice.ts @@ -1,4 +1,5 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { DeviceApp } from "./DeviceApp"; import type { DevicePlatform } from "./DevicePlatform"; /** @@ -16,4 +17,10 @@ signing_key: string, /** * The X25519 public key (32 bytes). */ -agreement_key: string, }; +agreement_key: string, +/** + * The app this device is, when it says. With a label the device's signature covers its + * trimmed name, its platform and the label (`orbyn/device-register/v2`, + * `orbyn/device-login/v2`); without one it signs the v1 messages, as before. + */ +app?: DeviceApp, }; diff --git a/crates/orbyn-contracts/src/auth.rs b/crates/orbyn-contracts/src/auth.rs index a2f1e83..5403f2d 100644 --- a/crates/orbyn-contracts/src/auth.rs +++ b/crates/orbyn-contracts/src/auth.rs @@ -16,6 +16,42 @@ pub enum DevicePlatform { Linux, } +impl DevicePlatform { + /// The wire name, as signed into a v2 device message. + pub fn as_str(self) -> &'static str { + match self { + Self::Web => "web", + Self::Ios => "ios", + Self::Android => "android", + Self::Macos => "macos", + Self::Windows => "windows", + Self::Linux => "linux", + } + } +} + +/// Which Orbyn app a device is. It is **signed into the device's register and login messages** +/// (`orbyn/device-register/v2` and `orbyn/device-login/v2`, with the device's name and platform), +/// so the server checks the device itself claimed it, and the device list shows it. A device that +/// sends no label signs the earlier messages and has none. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize, TS)] +#[serde(rename_all = "snake_case")] +#[ts(export)] +pub enum DeviceApp { + Planner, + Engineering, +} + +impl DeviceApp { + /// The wire name, as signed into a v2 device message. + pub fn as_str(self) -> &'static str { + match self { + Self::Planner => "planner", + Self::Engineering => "engineering", + } + } +} + /// Whether a device may use the account. A device that signs in is `pending` /// until a trusted device approves it. #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize, TS)] @@ -37,6 +73,12 @@ pub struct NewDevice { pub signing_key: String, /// The X25519 public key (32 bytes). pub agreement_key: String, + /// The app this device is, when it says. With a label the device's signature covers its + /// trimmed name, its platform and the label (`orbyn/device-register/v2`, + /// `orbyn/device-login/v2`); without one it signs the v1 messages, as before. + #[serde(default)] + #[ts(optional)] + pub app: Option, } /// `POST /v1/accounts/register/start`. Byte fields are base64url without padding. diff --git a/crates/orbyn-contracts/src/devices.rs b/crates/orbyn-contracts/src/devices.rs index b4f3092..27df996 100644 --- a/crates/orbyn-contracts/src/devices.rs +++ b/crates/orbyn-contracts/src/devices.rs @@ -1,6 +1,6 @@ //! Device lists, removals and approval wire types. -use super::{DevicePlatform, DeviceState}; +use super::{DeviceApp, DevicePlatform, DeviceState}; use serde::{Deserialize, Serialize}; use ts_rs::TS; @@ -11,6 +11,10 @@ pub struct Device { pub id: String, pub name: String, pub platform: DevicePlatform, + /// The app the device signed itself in as, if it said (see [`DeviceApp`]). + #[serde(default)] + #[ts(optional)] + pub app: Option, pub state: DeviceState, pub signing_key: String, pub agreement_key: String, diff --git a/crates/orbyn-core/src/accounts/device_app.rs b/crates/orbyn-core/src/accounts/device_app.rs new file mode 100644 index 0000000..e33feaf --- /dev/null +++ b/crates/orbyn-core/src/accounts/device_app.rs @@ -0,0 +1,79 @@ +//! The app label a device may sign into its introduction. + +use super::{AccountError, NewDevice, Result}; +use orbyn_auth_client::{ + DEVICE_LOGIN_CONTEXT, DEVICE_LOGIN_V2_CONTEXT, DEVICE_REGISTER_CONTEXT, + DEVICE_REGISTER_V2_CONTEXT, Descriptor, login_message, login_message_v2, register_message, + register_message_v2, +}; + +/// The product a device belongs to, when it says so. It is signed with the device's +/// introduction, so a server or relay cannot relabel a device afterwards. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum App { + Planner, + Engineering, +} + +impl App { + pub fn as_str(self) -> &'static str { + match self { + Self::Planner => "planner", + Self::Engineering => "engineering", + } + } + + pub(super) fn parse(text: &str) -> Result { + Ok(match text { + "planner" => Self::Planner, + "engineering" => Self::Engineering, + _ => return Err(AccountError::Internal), + }) + } + + pub(super) fn parse_opt(text: Option<&str>) -> Result> { + text.map(Self::parse).transpose() + } +} + +impl NewDevice { + /// The context and bytes this device's sign-up signature must cover. A device that names its + /// app signs the v2 message, which adds the trimmed name, platform and app; one that does not + /// signs the v1 message, so older clients keep working. + pub(super) fn register_proof( + &self, + name: &str, + email: &str, + upload: &[u8], + ) -> (&'static str, Vec) { + match self.app { + Some(app) => ( + DEVICE_REGISTER_V2_CONTEXT, + register_message_v2(email, upload, &self.keys, &self.descriptor(name, app)), + ), + None => ( + DEVICE_REGISTER_CONTEXT, + register_message(email, upload, &self.keys), + ), + } + } + + /// As [`Self::register_proof`], for a sign-in. + pub(super) fn login_proof(&self, name: &str, login_id: &[u8; 16]) -> (&'static str, Vec) { + match self.app { + Some(app) => ( + DEVICE_LOGIN_V2_CONTEXT, + login_message_v2(login_id, &self.keys, &self.descriptor(name, app)), + ), + None => (DEVICE_LOGIN_CONTEXT, login_message(login_id, &self.keys)), + } + } + + fn descriptor<'a>(&self, name: &'a str, app: App) -> Descriptor<'a> { + Descriptor { + name, + platform: self.platform.as_str(), + app: app.as_str(), + } + } +} diff --git a/crates/orbyn-core/src/accounts/devices.rs b/crates/orbyn-core/src/accounts/devices.rs index dba9862..5fd0143 100644 --- a/crates/orbyn-core/src/accounts/devices.rs +++ b/crates/orbyn-core/src/accounts/devices.rs @@ -19,7 +19,7 @@ //! Removing a device carries the remover's signature, which every device checks. use super::{ - AccountError, Accounts, Approval, ApprovalView, AuditAction, Client, Device, DeviceState, + AccountError, Accounts, App, Approval, ApprovalView, AuditAction, Client, Device, DeviceState, Platform, Removal, Result, Session, limits, public_keys, }; use chrono::{DateTime, Utc}; @@ -52,6 +52,7 @@ struct DeviceRow { id: Uuid, name: String, platform: String, + app: Option, state: String, signing_key: Vec, agreement_key: Vec, @@ -91,7 +92,7 @@ struct ApprovalRow { macro_rules! select_devices { ($tail:literal) => { concat!( - "SELECT id, name, platform, state, signing_key, agreement_key, created_at, ", + "SELECT id, name, platform, app, state, signing_key, agreement_key, created_at, ", "approved_by, approval_signature, approval_id, approval_created_at, ", "approval_endorsement_hash FROM devices ", $tail @@ -117,6 +118,7 @@ impl TryFrom for Device { id: row.id, name: row.name, platform: Platform::parse(&row.platform)?, + app: App::parse_opt(row.app.as_deref())?, state: DeviceState::parse(&row.state).map_err(|_| AccountError::Internal)?, keys: public_keys(&row.signing_key, &row.agreement_key)?, created_at: row.created_at, diff --git a/crates/orbyn-core/src/accounts/email.rs b/crates/orbyn-core/src/accounts/email.rs index 0f7aab4..c2ac202 100644 --- a/crates/orbyn-core/src/accounts/email.rs +++ b/crates/orbyn-core/src/accounts/email.rs @@ -28,7 +28,7 @@ //! in the code's life. A wrong guess at a dead code counts and fails like any other. use super::{ - AccountError, Accounts, AuditAction, Client, DeviceState, NewDevice, NewSession, Result, + AccountError, Accounts, App, AuditAction, Client, DeviceState, NewDevice, NewSession, Result, limits, new_account_id, sessions, }; use crate::mail::{self, Kind, Mail}; @@ -359,9 +359,9 @@ impl Accounts { self.step("insert"); sqlx::query( "INSERT INTO pending_signups - (id, email, decoy, password_file, device_name, device_platform, + (id, email, decoy, password_file, device_name, device_platform, device_app, device_signing_key, device_agreement_key, expires_at, invite_id, prefix_digest) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8, now() + make_interval(mins => $9), $10, $11)", + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, now() + make_interval(mins => $10), $11, $12)", ) .bind(id) .bind(email) @@ -369,6 +369,7 @@ impl Accounts { .bind(password_file) .bind(name) .bind(device.platform.as_str()) + .bind(device.app.map(App::as_str)) .bind(device.keys.signing.as_slice()) .bind(device.keys.agreement.as_slice()) .bind(SIGNUP_MINUTES) diff --git a/crates/orbyn-core/src/accounts/email/verification.rs b/crates/orbyn-core/src/accounts/email/verification.rs index 7f67ade..709510e 100644 --- a/crates/orbyn-core/src/accounts/email/verification.rs +++ b/crates/orbyn-core/src/accounts/email/verification.rs @@ -133,6 +133,7 @@ impl Accounts { Vec, String, String, + Option, Vec, Vec, Option, @@ -141,14 +142,14 @@ impl Accounts { // second finds it gone. self.step("row-lock"); let row: Option = sqlx::query_as( - "SELECT email, decoy, password_file, device_name, device_platform, + "SELECT email, decoy, password_file, device_name, device_platform, device_app, device_signing_key, device_agreement_key, invite_id FROM pending_signups WHERE id = $1 AND expires_at > now() FOR UPDATE", ) .bind(signup_id) .fetch_optional(&mut *tx) .await?; - let (email, decoy, password_file, name, platform, signing, agreement, invite_id) = + let (email, decoy, password_file, name, platform, app, signing, agreement, invite_id) = row.ok_or(AccountError::InvalidEmailCode)?; self.step("code-lock"); let record: Option<(Vec, i32)> = sqlx::query_as( @@ -235,13 +236,14 @@ impl Accounts { } let device_id = Uuid::new_v4(); sqlx::query( - "INSERT INTO devices (id, account_id, name, platform, signing_key, agreement_key, state, approved_at) - VALUES ($1, $2, $3, $4, $5, $6, 'trusted', now())", + "INSERT INTO devices (id, account_id, name, platform, app, signing_key, agreement_key, state, approved_at) + VALUES ($1, $2, $3, $4, $5, $6, $7, 'trusted', now())", ) .bind(device_id) .bind(account_id) .bind(&name) .bind(&platform) + .bind(&app) .bind(&signing) .bind(&agreement) .execute(&mut *tx) diff --git a/crates/orbyn-core/src/accounts/login.rs b/crates/orbyn-core/src/accounts/login.rs index 2ae43e1..6692894 100644 --- a/crates/orbyn-core/src/accounts/login.rs +++ b/crates/orbyn-core/src/accounts/login.rs @@ -185,13 +185,9 @@ impl Accounts { return Err(AccountError::LoginFailed); } }; - orbyn_crypto::verify( - &device.keys, - DEVICE_LOGIN_CONTEXT, - &login_message(login_id.as_bytes(), &device.keys), - signature, - ) - .map_err(|_| AccountError::InvalidSignature)?; + let (context, message) = device.login_proof(&name, login_id.as_bytes()); + orbyn_crypto::verify(&device.keys, context, &message, signature) + .map_err(|_| AccountError::InvalidSignature)?; // The password was right. An account with a second factor gets no session and no device // row yet, unless a trusted device proved itself at the start: that device holds a device @@ -275,13 +271,14 @@ impl Accounts { } let id = Uuid::new_v4(); sqlx::query( - "INSERT INTO devices (id, account_id, name, platform, signing_key, agreement_key, state) - VALUES ($1, $2, $3, $4, $5, $6, 'pending')", + "INSERT INTO devices (id, account_id, name, platform, app, signing_key, agreement_key, state) + VALUES ($1, $2, $3, $4, $5, $6, $7, 'pending')", ) .bind(id) .bind(account_id) .bind(name) .bind(device.platform.as_str()) + .bind(device.app.map(App::as_str)) .bind(device.keys.signing.as_slice()) .bind(device.keys.agreement.as_slice()) .execute(&mut *tx) diff --git a/crates/orbyn-core/src/accounts/mod.rs b/crates/orbyn-core/src/accounts/mod.rs index 7dabc5b..0fcb909 100644 --- a/crates/orbyn-core/src/accounts/mod.rs +++ b/crates/orbyn-core/src/accounts/mod.rs @@ -17,6 +17,7 @@ mod delete; mod delete_unused; +mod device_app; mod devices; mod email; pub mod email_code; @@ -36,6 +37,7 @@ pub const TOMBSTONE_DAYS: i32 = 400; pub use delete::{FRESH_SIGN_IN_SECS, signed_in_recently}; pub use delete_unused::{DELETE_UNUSED_TTL, REQUEST_JOB_KIND as DELETE_UNUSED_JOB_KIND}; +pub use device_app::App; pub use limits::{Limit, Limits}; pub use push_tokens::PUSH_TOKEN_DAYS; pub use second_factor::{ManageProof, TotpEnrolment}; @@ -46,15 +48,11 @@ use orbyn_auth_client::opaque_ke::{ CredentialFinalization, CredentialRequest, RegistrationRequest, RegistrationUpload, ServerLogin, ServerLoginParameters, ServerRegistration, }; -use orbyn_auth_client::{ - DEVICE_LOGIN_CONTEXT, DEVICE_REGISTER_CONTEXT, LOGIN_CONTEXT, OsRng, Suite, login_message, - normalize_email, register_message, -}; +use orbyn_auth_client::{LOGIN_CONTEXT, OsRng, Suite, normalize_email}; use orbyn_crypto::{DevicePublic, Sealed}; use sqlx::{PgPool, Postgres, Transaction}; use std::net::IpAddr; -use std::sync::Arc; -use std::time::Duration; +use std::{sync::Arc, time::Duration}; use uuid::Uuid; /// How long the server keeps its half of a sign-in. @@ -258,6 +256,7 @@ impl DeviceState { pub struct NewDevice { pub name: String, pub platform: Platform, + pub app: Option, pub keys: DevicePublic, } @@ -323,6 +322,7 @@ pub struct Device { pub id: Uuid, pub name: String, pub platform: Platform, + pub app: Option, pub state: DeviceState, pub keys: DevicePublic, pub created_at: DateTime, diff --git a/crates/orbyn-core/src/accounts/register.rs b/crates/orbyn-core/src/accounts/register.rs index a533b83..67559c9 100644 --- a/crates/orbyn-core/src/accounts/register.rs +++ b/crates/orbyn-core/src/accounts/register.rs @@ -75,13 +75,9 @@ impl Accounts { let invite_id = self.check_invite(client, invite).await?; let record = RegistrationUpload::::deserialize(upload) .map_err(|_| AccountError::InvalidRequest)?; - orbyn_crypto::verify( - &device.keys, - DEVICE_REGISTER_CONTEXT, - ®ister_message(&email, upload, &device.keys), - signature, - ) - .map_err(|_| AccountError::InvalidSignature)?; + let (context, message) = device.register_proof(&name, &email, upload); + orbyn_crypto::verify(&device.keys, context, &message, signature) + .map_err(|_| AccountError::InvalidSignature)?; let password_file = ServerRegistration::::finish(record) .serialize() .to_vec(); diff --git a/crates/orbyn-core/src/accounts/second_factor.rs b/crates/orbyn-core/src/accounts/second_factor.rs index 0decc6a..da2868e 100644 --- a/crates/orbyn-core/src/accounts/second_factor.rs +++ b/crates/orbyn-core/src/accounts/second_factor.rs @@ -11,8 +11,8 @@ use super::totp::{self, RECOVERY_CODES, SECRET_LEN}; use super::{ - AccountError, Accounts, AuditAction, Client, LOGIN_STATE_TTL, NewDevice, NewSession, Platform, - Result, Session, limits, public_keys, + AccountError, Accounts, App, AuditAction, Client, LOGIN_STATE_TTL, NewDevice, NewSession, + Platform, Result, Session, limits, public_keys, }; use base64::Engine; use base64::engine::general_purpose::URL_SAFE_NO_PAD; @@ -125,6 +125,7 @@ impl Accounts { let body = serde_json::json!({ "name": name, "platform": device.platform.as_str(), + "app": device.app.map(App::as_str), "signing": URL_SAFE_NO_PAD.encode(device.keys.signing), "agreement": URL_SAFE_NO_PAD.encode(device.keys.agreement), }); @@ -181,6 +182,7 @@ impl Accounts { let device = NewDevice { name: name.clone(), platform: Platform::parse(body["platform"].as_str().ok_or(AccountError::Internal)?)?, + app: App::parse_opt(body["app"].as_str())?, keys: public_keys(&field("signing")?, &field("agreement")?)?, }; Ok((device, name)) diff --git a/crates/orbyn-device-client/src/client.rs b/crates/orbyn-device-client/src/client.rs index 2150bb5..93d172e 100644 --- a/crates/orbyn-device-client/src/client.rs +++ b/crates/orbyn-device-client/src/client.rs @@ -49,6 +49,10 @@ pub struct ClientConfig { /// A name the person chose for this device, 1 to 100 characters. pub device_name: String, pub platform: wire::DevicePlatform, + /// Which Orbyn app this is. When set, the sign-up and sign-in signatures cover the trimmed + /// name, the platform and this label (`orbyn/device-register/v2`, `orbyn/device-login/v2`), so + /// the server and the device list can trust it; when `None` the earlier messages are signed. + pub app: Option, } /// What a sign-up needs. @@ -197,13 +201,61 @@ impl DeviceClient { pub(crate) fn new_device(&self, keys: &DevicePublic) -> wire::NewDevice { wire::NewDevice { - name: self.config.device_name.clone(), + name: self.config.device_name.trim().to_owned(), platform: self.config.platform, + app: self.config.app, signing_key: encode(&keys.signing), agreement_key: encode(&keys.agreement), } } + /// The context and bytes this device signs to introduce itself at sign-up. + pub(crate) fn register_message( + &self, + email: &str, + upload: &[u8], + keys: &DevicePublic, + ) -> (&'static str, Vec) { + let name = self.config.device_name.trim(); + match self.config.app { + Some(app) => ( + auth::DEVICE_REGISTER_V2_CONTEXT, + auth::register_message_v2(email, upload, keys, &self.descriptor(name, app)), + ), + None => ( + auth::DEVICE_REGISTER_CONTEXT, + auth::register_message(email, upload, keys), + ), + } + } + + /// As [`Self::register_message`], for a sign-in. + pub(crate) fn login_message( + &self, + login_id: &[u8; 16], + keys: &DevicePublic, + ) -> (&'static str, Vec) { + let name = self.config.device_name.trim(); + match self.config.app { + Some(app) => ( + auth::DEVICE_LOGIN_V2_CONTEXT, + auth::login_message_v2(login_id, keys, &self.descriptor(name, app)), + ), + None => ( + auth::DEVICE_LOGIN_CONTEXT, + auth::login_message(login_id, keys), + ), + } + } + + fn descriptor<'a>(&self, name: &'a str, app: wire::DeviceApp) -> auth::Descriptor<'a> { + auth::Descriptor { + name, + platform: self.config.platform.as_str(), + app: app.as_str(), + } + } + pub(crate) async fn call( &self, method: Method, @@ -358,10 +410,8 @@ impl DeviceClient { let login_id: [u8; 16] = *uuid::Uuid::parse_str(&started.login_id) .map_err(|_| protocol())? .as_bytes(); - let signature = keys.sign( - auth::DEVICE_LOGIN_CONTEXT, - &auth::login_message(&login_id, &public), - ); + let (context, message) = self.login_message(&login_id, &public); + let signature = keys.sign(context, &message); let response = self .call( Method::Post, diff --git a/crates/orbyn-device-client/src/signup.rs b/crates/orbyn-device-client/src/signup.rs index 3578735..97a8c1e 100644 --- a/crates/orbyn-device-client/src/signup.rs +++ b/crates/orbyn-device-client/src/signup.rs @@ -72,10 +72,8 @@ impl DeviceClient { drop(request.password); let keys = DeviceKeys::generate().map_err(|_| DeviceError::Crypto)?; let public = keys.public(); - let signature = keys.sign( - auth::DEVICE_REGISTER_CONTEXT, - &auth::register_message(&email, &finish.upload, &public), - ); + let (context, message) = self.register_message(&email, &finish.upload, &public); + let signature = keys.sign(context, &message); let mut body = wire::RegisterFinish { email: email.clone(), registration_upload: encode(&finish.upload), diff --git a/crates/orbyn-device-client/tests/client.rs b/crates/orbyn-device-client/tests/client.rs index 75921d5..c36c87b 100644 --- a/crates/orbyn-device-client/tests/client.rs +++ b/crates/orbyn-device-client/tests/client.rs @@ -3,7 +3,7 @@ mod support; -use orbyn_contracts::{DevicePlatform, DeviceState}; +use orbyn_contracts::{DeviceApp, DevicePlatform, DeviceState}; use orbyn_device_client::{ ClientConfig, Clock, DeviceApi, DeviceClient, DeviceError, HttpApi, KeyStore, MemoryStore, Method, Request, Response, SecondFactor, SignInOutcome, SignUpRequest, StoredIdentity, @@ -36,6 +36,7 @@ fn client(engine: &FakeEngine, name: &str) -> Client { ClientConfig { device_name: name.to_owned(), platform: DevicePlatform::Linux, + app: Some(DeviceApp::Engineering), }, ) } @@ -541,3 +542,41 @@ async fn the_http_transport_does_not_follow_redirects_and_reads_retry_after() { .expect("answer"); assert_eq!((response.status, response.retry_after), (429, Some(7))); } + +#[tokio::test(flavor = "multi_thread")] +async fn the_app_label_is_signed_at_sign_up_and_sign_in_and_a_client_with_none_signs_the_old_messages() + { + let engine = FakeEngine::new(); + // The fake checks the v2 message when a label is sent and the v1 message when not, and + // panics on a bad signature, so each of these passes only if the client signed the right one. + let a = client(&engine, " Laptop "); + let mut pending = a.begin_sign_up(request(None)).await.expect("begin"); + let code = engine.code_for(NORMAL); + a.verify_sign_up(&mut pending, &code).await.expect("verify"); + let plain = DeviceClient::new( + engine.clone(), + MemoryStore::new(), + SharedClock(engine.now.clone()), + ClientConfig { + device_name: " Tablet ".to_owned(), + platform: DevicePlatform::Linux, + app: None, + }, + ); + plain + .sign_in(NORMAL, &password(PASSWORD)) + .await + .expect("an unlabelled device signs in with the v1 message"); + let b = client(&engine, "Phone"); + b.sign_in(NORMAL, &password(PASSWORD)) + .await + .expect("a labelled device signs in with the v2 message"); + assert_eq!( + engine.device_apps(NORMAL), + vec![ + Some(DeviceApp::Engineering), + None, + Some(DeviceApp::Engineering) + ] + ); +} diff --git a/crates/orbyn-device-client/tests/limits.rs b/crates/orbyn-device-client/tests/limits.rs index d488f39..c34190d 100644 --- a/crates/orbyn-device-client/tests/limits.rs +++ b/crates/orbyn-device-client/tests/limits.rs @@ -134,6 +134,7 @@ fn client(upload: Upload, fetch: Fetch) -> Client { ClientConfig { device_name: "Laptop".to_owned(), platform: wire::DevicePlatform::Linux, + app: None, }, ) } diff --git a/crates/orbyn-device-client/tests/local_stack.rs b/crates/orbyn-device-client/tests/local_stack.rs index ddc6de8..74b5c5b 100644 --- a/crates/orbyn-device-client/tests/local_stack.rs +++ b/crates/orbyn-device-client/tests/local_stack.rs @@ -10,7 +10,7 @@ //! local stack, and there the test prints a note and passes without doing anything. //! `ORBYN_LOCAL_URL` changes the address (default `http://127.0.0.1:8080`). -use orbyn_contracts::{DevicePlatform, DeviceState}; +use orbyn_contracts::{DeviceApp, DevicePlatform, DeviceState}; use orbyn_device_client::{ ClientConfig, DeviceClient, DeviceError, HttpApi, MemoryStore, SignInOutcome, SignUpRequest, SystemClock, @@ -39,6 +39,7 @@ fn client(name: &str) -> DeviceClient { ClientConfig { device_name: name.to_owned(), platform: DevicePlatform::Linux, + app: Some(DeviceApp::Engineering), }, ) } diff --git a/crates/orbyn-device-client/tests/notices.rs b/crates/orbyn-device-client/tests/notices.rs index 8ea5d48..f42d4af 100644 --- a/crates/orbyn-device-client/tests/notices.rs +++ b/crates/orbyn-device-client/tests/notices.rs @@ -34,6 +34,7 @@ async fn client_for(address: &str) -> DeviceClient, +} + +/// The descriptor a v2 message must cover: what the server would build from the request. +fn descriptor(device: &wire::NewDevice, app: wire::DeviceApp) -> Descriptor<'_> { + Descriptor { + name: device.name.trim(), + platform: device.platform.as_str(), + app: app.as_str(), + } } pub struct Account { @@ -52,6 +65,7 @@ struct Pending { email: String, file: Vec, keys: DevicePublic, + app: Option, code: String, invite: Option, } @@ -206,6 +220,15 @@ impl FakeEngine { .len() } + /// The app label each device of the account signed, in the order they joined. + pub fn device_apps(&self, email: &str) -> Vec> { + self.state.lock().expect("lock").accounts[email] + .devices + .iter() + .map(|d| d.app) + .collect() + } + /// Every request body the fake has seen, as `(path, body text)`. pub fn log(&self) -> Vec<(String, String)> { self.state.lock().expect("lock").log.clone() @@ -322,13 +345,18 @@ fn register_finish(s: &mut State, body: Value) -> Result let signature: [u8; 64] = unb64(&finish.device_signature) .try_into() .expect("signature"); - orbyn_crypto::verify( - &keys, - DEVICE_REGISTER_CONTEXT, - ®ister_message(&email, &upload, &keys), - &signature, - ) - .expect("the device signed the registration"); + let (context, message) = match finish.device.app { + Some(app) => ( + DEVICE_REGISTER_V2_CONTEXT, + register_message_v2(&email, &upload, &keys, &descriptor(&finish.device, app)), + ), + None => ( + DEVICE_REGISTER_CONTEXT, + register_message(&email, &upload, &keys), + ), + }; + orbyn_crypto::verify(&keys, context, &message, &signature) + .expect("the device signed the registration"); if let Some((challenge, bits)) = s.proof { let solved = finish.proof.as_ref().is_some_and(|p| { p.challenge == hex(&challenge) @@ -360,6 +388,7 @@ fn register_finish(s: &mut State, body: Value) -> Result email, file, keys, + app: finish.device.app, code, invite, }, @@ -392,6 +421,7 @@ fn email_verify(s: &mut State, body: &Value, now: i64) -> Result Result ( + DEVICE_LOGIN_V2_CONTEXT, + login_message_v2(&login_id, &keys, &descriptor(&finish.device, app)), + ), + None => (DEVICE_LOGIN_CONTEXT, login_message(&login_id, &keys)), + }; + orbyn_crypto::verify(&keys, context, &message, &signature) + .expect("the device signed the login"); let account = s.accounts.get_mut(&login.email).expect("account"); let existing = account.devices.iter().position(|d| d.keys == keys); let (device_id, trusted) = match existing { @@ -543,6 +575,7 @@ fn login_finish(s: &mut State, body: &Value, now: i64) -> Result wire::DevicePlatform { } } +pub(super) fn app(a: wire::DeviceApp) -> accounts::App { + match a { + wire::DeviceApp::Planner => accounts::App::Planner, + wire::DeviceApp::Engineering => accounts::App::Engineering, + } +} + +pub(super) fn wire_app(a: accounts::App) -> wire::DeviceApp { + match a { + accounts::App::Planner => wire::DeviceApp::Planner, + accounts::App::Engineering => wire::DeviceApp::Engineering, + } +} + pub(super) fn wire_state(s: accounts::DeviceState) -> wire::DeviceState { match s { accounts::DeviceState::Pending => wire::DeviceState::Pending, @@ -296,6 +310,7 @@ pub(super) fn new_device(device: &wire::NewDevice) -> Answer wire::Devi id: device.id.to_string(), name: device.name, platform: wire_platform(device.platform), + app: device.app.map(wire_app), state: wire_state(device.state), signing_key: encode(&device.keys.signing), agreement_key: encode(&device.keys.agreement), diff --git a/crates/orbyn-server/tests/accounts.rs b/crates/orbyn-server/tests/accounts.rs index 59b8e16..4a8ccbc 100644 --- a/crates/orbyn-server/tests/accounts.rs +++ b/crates/orbyn-server/tests/accounts.rs @@ -9753,3 +9753,335 @@ async fn deleting_one_unused_account_never_releases_another_accounts_invite() { (StatusCode::BAD_REQUEST, json!("invite_invalid")) ); } + +// ---- The device's app label, signed into its register and login messages ----------------------- + +/// What a labelled device says about itself and what it signed: the body's name, platform and app, +/// and the descriptor its signature covers (which a test can make differ from the body). +struct Label<'a> { + sent_name: &'a str, + sent_app: Option<&'a str>, + sent_platform: &'a str, + signed: Option<(&'a str, &'a str, &'a str)>, +} + +fn labelled_device(label: &Label<'_>, keys: &DevicePublic) -> Value { + let mut device = json!({ + "name": label.sent_name, + "platform": label.sent_platform, + "signing_key": b64(&keys.signing), + "agreement_key": b64(&keys.agreement), + }); + if let Some(app) = label.sent_app { + device["app"] = json!(app); + } + device +} + +fn descriptor<'a>(signed: (&'a str, &'a str, &'a str)) -> auth::Descriptor<'a> { + auth::Descriptor { + name: signed.0, + platform: signed.1, + app: signed.2, + } +} + +impl Server { + async fn register_labelled( + &self, + email: &str, + upload: &[u8], + keys: &DeviceKeys, + label: &Label<'_>, + ) -> (StatusCode, Value) { + let normalized = auth::normalize_email(email).expect("email"); + let public = keys.public(); + let signature = match label.signed { + Some(signed) => keys.sign( + auth::DEVICE_REGISTER_V2_CONTEXT, + &auth::register_message_v2(&normalized, upload, &public, &descriptor(signed)), + ), + None => keys.sign( + auth::DEVICE_REGISTER_CONTEXT, + &auth::register_message(&normalized, upload, &public), + ), + }; + self.call( + Method::POST, + "/v1/accounts/register/finish", + None, + Some(json!({ + "email": email, + "registration_upload": b64(upload), + "device": labelled_device(label, &public), + "device_signature": b64(&signature), + })), + ) + .await + } + + async fn login_labelled( + &self, + email: &str, + password: &[u8], + keys: &DeviceKeys, + label: &Label<'_>, + ) -> (StatusCode, Value) { + let (login, status, started) = self.login_start(email, password).await; + assert_eq!(status, StatusCode::OK, "{started}"); + let finish = login + .finish(password, &unb64(&started["credential_response"])) + .expect("right password"); + let login_id = uuid_of(&started["login_id"]); + let public = keys.public(); + let signature = match label.signed { + Some(signed) => keys.sign( + auth::DEVICE_LOGIN_V2_CONTEXT, + &auth::login_message_v2(login_id.as_bytes(), &public, &descriptor(signed)), + ), + None => keys.sign( + auth::DEVICE_LOGIN_CONTEXT, + &auth::login_message(login_id.as_bytes(), &public), + ), + }; + self.call( + Method::POST, + "/v1/sessions/login/finish", + None, + Some(json!({ + "login_id": started["login_id"], + "credential_finalization": b64(&finish.finalization), + "device": labelled_device(label, &public), + "device_signature": b64(&signature), + })), + ) + .await + } + + async fn device_list(&self, session: &Value) -> Vec { + let (status, list) = self + .call(Method::GET, "/v1/devices", Some(token(session)), None) + .await; + assert_eq!(status, StatusCode::OK, "{list}"); + list["devices"].as_array().expect("devices").clone() + } +} + +fn labelled<'a>(name: &'a str, app: &'a str) -> Label<'a> { + Label { + sent_name: name, + sent_app: Some(app), + sent_platform: "macos", + signed: Some((name.trim(), "macos", app)), + } +} + +fn invalid_signature(answer: &(StatusCode, Value)) -> bool { + answer.0 == StatusCode::BAD_REQUEST && answer.1["code"] == "invalid_signature" +} + +#[tokio::test] +#[ignore = "needs DATABASE_URL"] +async fn a_device_that_names_its_app_signs_it_and_the_list_shows_it() { + let _turn = TURN.lock().await; + let server = server().await; + let email = email(); + let upload = server.registration_upload(&email, b"pw").await; + let keys = DeviceKeys::generate().expect("keys"); + + // The name is signed trimmed, as the server stores it. + let (status, pending) = server + .register_labelled( + &email, + &upload, + &keys, + &labelled(" Laptop ", "engineering"), + ) + .await; + assert_eq!(status, StatusCode::CREATED, "{pending}"); + let code = server.code_for(&email).await; + let (status, session) = server.verify(&pending["signup_id"], &code).await; + assert_eq!(status, StatusCode::CREATED, "{session}"); + let devices = server.device_list(&session).await; + assert_eq!(devices[0]["app"], "engineering"); + assert_eq!(devices[0]["name"], "Laptop"); + + // A new device signs in under the same rule and is listed with its label while pending. + let phone = DeviceKeys::generate().expect("keys"); + let (status, phone_session) = server + .login_labelled(&email, b"pw", &phone, &labelled("Phone", "planner")) + .await; + assert_eq!(status, StatusCode::OK, "{phone_session}"); + assert_eq!(phone_session["device_state"], "pending"); + let devices = server.device_list(&session).await; + let labels: Vec<_> = devices.iter().map(|d| d["app"].clone()).collect(); + assert!(labels.contains(&json!("planner")), "{labels:?}"); + assert!(labels.contains(&json!("engineering")), "{labels:?}"); + + // A known device that signs in again keeps what is stored; the claim does not rewrite it. + let (status, again) = server + .login_labelled(&email, b"pw", &keys, &labelled("Renamed", "planner")) + .await; + assert_eq!(status, StatusCode::OK, "{again}"); + let devices = server.device_list(&session).await; + let laptop = devices + .iter() + .find(|d| d["current"] == true) + .expect("laptop"); + assert_eq!( + (&laptop["name"], &laptop["app"]), + (&json!("Laptop"), &json!("engineering")) + ); +} + +#[tokio::test] +#[ignore = "needs DATABASE_URL"] +async fn the_server_refuses_a_label_the_device_did_not_sign() { + let _turn = TURN.lock().await; + let server = server().await; + let email = email(); + let upload = server.registration_upload(&email, b"pw").await; + let keys = DeviceKeys::generate().expect("keys"); + let base = labelled("Laptop", "engineering"); + + // Each field of the descriptor is covered: the name, the platform and the app. + let cases = [ + Label { + sent_name: "Other", + ..labelled("Laptop", "engineering") + }, + Label { + sent_platform: "linux", + ..labelled("Laptop", "engineering") + }, + Label { + sent_app: Some("planner"), + ..labelled("Laptop", "engineering") + }, + // A label the signature does not mention: the v1 message under a labelled body. + Label { + signed: None, + ..labelled("Laptop", "engineering") + }, + // A v2 signature under a body with no label: the server checks the v1 message. + Label { + sent_app: None, + ..labelled("Laptop", "engineering") + }, + ]; + for (i, case) in cases.iter().enumerate() { + let answer = server.register_labelled(&email, &upload, &keys, case).await; + assert!(invalid_signature(&answer), "case {i}: {answer:?}"); + } + // The same device with an honest label is accepted. + let (status, body) = server + .register_labelled(&email, &upload, &keys, &base) + .await; + assert_eq!(status, StatusCode::CREATED, "{body}"); + let code = server.code_for(&email).await; + let (status, session) = server.verify(&body["signup_id"], &code).await; + assert_eq!(status, StatusCode::CREATED, "{session}"); + + // Sign-in is covered the same way. + let phone = DeviceKeys::generate().expect("keys"); + let cases = [ + Label { + sent_name: "Other", + ..labelled("Phone", "planner") + }, + Label { + sent_platform: "linux", + ..labelled("Phone", "planner") + }, + Label { + sent_app: Some("engineering"), + ..labelled("Phone", "planner") + }, + Label { + signed: None, + ..labelled("Phone", "planner") + }, + Label { + sent_app: None, + ..labelled("Phone", "planner") + }, + ]; + for (i, case) in cases.iter().enumerate() { + let answer = server.login_labelled(&email, b"pw", &phone, case).await; + assert!(invalid_signature(&answer), "login case {i}: {answer:?}"); + } + assert_eq!(server.device_list(&session).await.len(), 1); +} + +#[tokio::test] +#[ignore = "needs DATABASE_URL"] +async fn a_device_with_no_label_still_signs_the_earlier_messages_and_has_none() { + let _turn = TURN.lock().await; + let server = server().await; + let email = email(); + let keys = DeviceKeys::generate().expect("keys"); + let session = server.register(&email, b"pw", &keys).await; + let devices = server.device_list(&session).await; + assert_eq!(devices[0]["app"], Value::Null); + let phone = DeviceKeys::generate().expect("keys"); + let signed_in = server.login(&email, b"pw", &phone).await; + assert_eq!(signed_in["device_state"], "pending"); + assert!( + server + .device_list(&session) + .await + .iter() + .all(|d| d["app"].is_null()) + ); +} + +#[tokio::test] +#[ignore = "needs DATABASE_URL"] +async fn the_label_survives_the_second_factor_step() { + let _turn = TURN.lock().await; + let server = server().await; + let email = email(); + let keys = DeviceKeys::generate().expect("keys"); + let session = server.register(&email, b"pw", &keys).await; + let (secret, _) = server.enable_totp(&session, &keys).await; + + // A new device with a label meets the factor; the label rides in the sealed state. + let phone = DeviceKeys::generate().expect("keys"); + let (login, status, started) = server.login_start(&email, b"pw").await; + assert_eq!(status, StatusCode::OK, "{started}"); + let finish = login + .finish(b"pw", &unb64(&started["credential_response"])) + .expect("password"); + let login_id = uuid_of(&started["login_id"]); + let public = phone.public(); + let signature = phone.sign( + auth::DEVICE_LOGIN_V2_CONTEXT, + &auth::login_message_v2( + login_id.as_bytes(), + &public, + &descriptor(("Phone", "macos", "engineering")), + ), + ); + let (status, body) = server + .call( + Method::POST, + "/v1/sessions/login/finish", + None, + Some(json!({ + "login_id": started["login_id"], + "credential_finalization": b64(&finish.finalization), + "device": labelled_device(&labelled("Phone", "engineering"), &public), + "device_signature": b64(&signature), + })), + ) + .await; + assert_eq!(status, StatusCode::UNAUTHORIZED, "{body}"); + let (status, signed_in) = server + .second_factor(&body["login_id"], Some(&code_for(&secret, 1)), None) + .await; + assert_eq!(status, StatusCode::OK, "{signed_in}"); + let devices = server.device_list(&session).await; + let labels: Vec<_> = devices.iter().map(|d| d["app"].clone()).collect(); + assert!(labels.contains(&json!("engineering")), "{labels:?}"); + assert_eq!(devices.len(), 2); +} diff --git a/crates/orbyn-server/tests/device_client.rs b/crates/orbyn-server/tests/device_client.rs index c44d1ce..27c83ff 100644 --- a/crates/orbyn-server/tests/device_client.rs +++ b/crates/orbyn-server/tests/device_client.rs @@ -105,6 +105,7 @@ impl Server { ClientConfig { device_name: name.to_owned(), platform: wire::DevicePlatform::Linux, + app: Some(wire::DeviceApp::Engineering), }, ) } @@ -527,6 +528,7 @@ async fn a_failed_network_keeps_the_work_and_says_offline() { ClientConfig { device_name: "Laptop".to_owned(), platform: wire::DevicePlatform::Linux, + app: Some(wire::DeviceApp::Engineering), }, ); lost.store() @@ -706,6 +708,7 @@ async fn a_conflict_during_an_upload_is_merged_and_the_upload_goes_again() { ClientConfig { device_name: "Laptop".to_owned(), platform: wire::DevicePlatform::Linux, + app: Some(wire::DeviceApp::Engineering), }, ); racing diff --git a/docs/architecture.md b/docs/architecture.md index aa96f42..3f1d84a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -81,7 +81,7 @@ Later phases add local storage for `orbyn-client` (SQLite on desktop and mobile) - **Verification failures are all alike.** Any failure, for any id, real or made up, is 400 `invalid_email_code` with the same body: a wrong code, an expired sign-up, a decoy, a dead code and a refusal by a limit. A real sign-up and a decoy take exactly the same steps (the same limiters, the same address lock, the same row and code-record locks, the same update; tests compare the steps and the timing). Wrong guesses are counted **with the code, for its whole life**: 10 in all (a counter on the code record, kept under the address lock) and **5 from one IP prefix** (a limiter keyed on the code's salt and the prefix, whose window equals the code's life); an address's ids share the one budget, for a free address and a taken one alike, and a start or resend never renews it. A dead code is replaced by the next start or resend after the minute, within the ceiling above; burning guesses costs the attacker ten guesses from two prefixes each time, and can delay the owner (an hour per spent code past the third in a day) **for as long as the attack runs** (see the owner's wait above): each start is still answered and stored, but no new code is made until the gap is over. Wrong codes are also limited to 30 an hour per IP address, for any id. The code is bound to the address. Starts, the making of an address's code and the verifications that create its account are serialized per address (an advisory lock). - The server's half of a sign-in is sealed (XChaCha20-Poly1305, bound to the login id) and kept in Postgres for five minutes, so any replica can finish it. Finishing deletes it, so it works once. - A session token is 32 random bytes in base64url, returned once. Postgres keeps its SHA-256 digest with the account, device, creation, last use and expiry. Each use moves the expiry to 30 days later, never past one year after sign-in. A device can sign out itself; a trusted device can sign out every session. -- Each device registers its Ed25519 and X25519 public keys with a name and platform, and signs a message that names the sign-up or the login id, so knowing the password is not enough to act as another device. The first device is trusted at sign-up. A device that signs in later is `pending` and can only see itself, answer an approval and sign out. +- Each device registers its Ed25519 and X25519 public keys with a name and platform, and signs a message that names the sign-up or the login id, so knowing the password is not enough to act as another device. A device may also name its app (`planner` or `engineering`): it then signs `orbyn/device-register/v2` or `orbyn/device-login/v2`, which are the v1 bytes followed by its trimmed name, platform and app, each length-prefixed. The server checks the v2 message exactly when the request carries an `app` and the v1 message when it does not (so older clients work unchanged), stores the label with the device and lists it. A known device that signs in again keeps the name and label it has; a claim at sign-in never rewrites a stored device. The first device is trusted at sign-up. A device that signs in later is `pending` and can only see itself, answer an approval and sign out. - **Approving a device is a two-way code comparison**, so neither device takes the server's word for the other's keys. Every message binds the account id, the approval id, the approval time (Unix seconds, chosen by the approver), both device ids and both devices' public keys (`orbyn_auth_client::Pairing`). In this exact order, each step once: 1. The approver A picks the approval id, the time and a 32-byte nonce `r_A`, and sends `POST /v1/devices/approvals` with only the commitment `SHA-256(orbyn/approval-commit/v1 ‖ pairing ‖ r_A)`. The server refuses a time more than five minutes from its own clock or a used id. 2. The new device D, which then holds the commitment and has seen no reveal, sends its 32-byte nonce `r_D` (`POST …/{id}/nonce`). It refuses an approval older than ten minutes. diff --git a/docs/device-client.md b/docs/device-client.md index 34822d6..b0fa56e 100644 --- a/docs/device-client.md +++ b/docs/device-client.md @@ -16,7 +16,7 @@ It ships in three pull requests: | Secrets | `KeyStore` | `load`, `save`, `clear` of one `StoredIdentity`. The OS keychain in the app; `MemoryStore` in tests. | | Time | `Clock` | Unix seconds, so a test fixes the time a trusted start is signed at. `SystemClock` is the real one. | -`DeviceClient` holds the three and a `ClientConfig` (the device's name and platform). +`DeviceClient` holds the three and a `ClientConfig` (the device's name, platform and optional `app` label). With an `app`, the sign-up and sign-in signatures are the v2 messages, which cover the trimmed name, the platform and the app; the client sends the trimmed name so the server verifies the same bytes. Without one it signs the v1 messages. ### What the key store holds, and what it does not diff --git a/migrations/0019_device_app.sql b/migrations/0019_device_app.sql new file mode 100644 index 0000000..1866a4d --- /dev/null +++ b/migrations/0019_device_app.sql @@ -0,0 +1,5 @@ +-- Which Orbyn app a device is (planner or engineering), when the device said so. The device's own +-- signature covers it (orbyn/device-register/v2 and orbyn/device-login/v2), so the server only +-- stores what the device claimed with its key. Existing devices have none. +ALTER TABLE devices ADD COLUMN app text CHECK (app IN ('planner', 'engineering')); +ALTER TABLE pending_signups ADD COLUMN device_app text CHECK (device_app IN ('planner', 'engineering'));