From 8b73de0df7701c53d247383b9cf5606bff0552b0 Mon Sep 17 00:00:00 2001 From: Valentin Tolmer Date: Wed, 23 Jun 2021 20:33:36 +0200 Subject: [PATCH] Update opaque and implement it without DB --- Cargo.lock | 65 +++++++-- Cargo.toml | 5 +- app/src/login.rs | 2 +- model/Cargo.toml | 2 +- model/src/lib.rs | 27 ++-- model/src/opaque.rs | 34 ++--- src/domain/error.rs | 6 + src/domain/sql_backend_handler.rs | 14 +- src/domain/sql_opaque_handler.rs | 232 ++++++++++-------------------- src/domain/sql_tables.rs | 95 ------------ src/infra/configuration.rs | 33 +++-- src/infra/db_cleaner.rs | 30 +--- src/infra/tcp_server.rs | 7 +- 13 files changed, 209 insertions(+), 343 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b92fac9..b817c37 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -728,6 +728,22 @@ dependencies = [ "subtle", ] +[[package]] +name = "crypto-mac" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "25fab6889090c8133f3deb8f73ba3c65a7f456f66436fc012a1b1e272b1e103e" +dependencies = [ + "generic-array", + "subtle", +] + +[[package]] +name = "ct-codecs" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f3b7eb4404b8195a9abb6356f4ac07d8ba267045c8d6d220ac4dc992e6cc75df" + [[package]] name = "curve25519-dalek" version = "3.1.0" @@ -737,6 +753,7 @@ dependencies = [ "byteorder", "digest", "rand_core 0.5.1", + "serde", "subtle", "zeroize", ] @@ -1056,6 +1073,7 @@ version = "0.14.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "501466ecc8a30d1d3b7fc9229b122b2ce8ed6e9d9223f1138d4babb253e51817" dependencies = [ + "serde", "typenum", "version_check", ] @@ -1210,12 +1228,12 @@ checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" [[package]] name = "hkdf" -version = "0.10.0" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "51ab2f639c231793c5f6114bdb9bbe50a7dbbfcd7c7c6bd8475dec2d991e964f" +checksum = "01706d578d5c281058480e673ae4086a9f4710d8df1ad80a5b03e39ece5f886b" dependencies = [ "digest", - "hmac", + "hmac 0.11.0", ] [[package]] @@ -1224,7 +1242,17 @@ version = "0.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c1441c6b1e930e2817404b5046f1f989899143a12bf92de603b69f4e0aee1e15" dependencies = [ - "crypto-mac", + "crypto-mac 0.10.0", + "digest", +] + +[[package]] +name = "hmac" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2a2a2320eb7ec0ebe8da8f744d7812d9fc4cb4d09344ac01898dbcb6a20ae69b" +dependencies = [ + "crypto-mac 0.11.0", "digest", ] @@ -1324,9 +1352,9 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "86e46349d67dc03bdbdb28da0337a355a53ca1d5156452722c36fe21d0e6389b" dependencies = [ "base64", - "crypto-mac", + "crypto-mac 0.10.0", "digest", - "hmac", + "hmac 0.10.1", "serde", "serde_json", "sha2", @@ -1419,6 +1447,8 @@ dependencies = [ "actix-web-httpauth", "anyhow", "async-trait", + "base64", + "bincode", "chrono", "clap", "cron", @@ -1426,7 +1456,7 @@ dependencies = [ "figment", "futures", "futures-util", - "hmac", + "hmac 0.10.1", "http", "jwt", "ldap3_server", @@ -1434,6 +1464,7 @@ dependencies = [ "log", "mockall", "opaque-ke", + "orion", "rand 0.8.3", "sea-query", "serde", @@ -1804,8 +1835,8 @@ checksum = "624a8340c38c1b80fd549087862da4ba43e08858af025b236e509b6649fc13d5" [[package]] name = "opaque-ke" -version = "0.5.1-pre.1" -source = "git+https://github.com/novifinancial/opaque-ke?rev=98f1821897cd2800e5bffb2a70541056145e99cc#98f1821897cd2800e5bffb2a70541056145e99cc" +version = "0.6.0-pre.1" +source = "git+https://github.com/novifinancial/opaque-ke?rev=eb59676a940b15f77871aefe1e46d7b5bf85f40a#eb59676a940b15f77871aefe1e46d7b5bf85f40a" dependencies = [ "base64", "curve25519-dalek", @@ -1814,7 +1845,7 @@ dependencies = [ "generic-array", "generic-bytes", "hkdf", - "hmac", + "hmac 0.11.0", "rand 0.8.3", "serde", "subtle", @@ -1871,6 +1902,18 @@ dependencies = [ "thiserror", ] +[[package]] +name = "orion" +version = "0.16.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "118f94b4ca56d1eb99466a26a216b87fad822a51af8b308264c88a9337eb0a15" +dependencies = [ + "ct-codecs", + "getrandom 0.2.3", + "subtle", + "zeroize", +] + [[package]] name = "os_str_bytes" version = "2.4.0" @@ -2525,7 +2568,7 @@ dependencies = [ "generic-array", "hashlink", "hex", - "hmac", + "hmac 0.10.1", "itoa", "libc", "libsqlite3-sys", diff --git a/Cargo.toml b/Cargo.toml index 666d53a..fec7f74 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,6 +16,8 @@ actix-service = "2.0.0" actix-web = "4.0.0-beta.3" anyhow = "*" async-trait = "0.1" +base64 = "0.13" +bincode = "1.3" chrono = { version = "*", features = [ "serde" ]} clap = "3.0.0-beta.2" cron = "*" @@ -28,6 +30,7 @@ jwt = "0.13" ldap3_server = "*" lldap_model = { path = "model" } log = "*" +orion = "0.16" serde = "*" serde_json = "1" sha2 = "0.9" @@ -45,7 +48,7 @@ rand = { version = "0.8", features = ["small_rng", "getrandom"] } # TODO: update to 0.6 when out. [dependencies.opaque-ke] git = "https://github.com/novifinancial/opaque-ke" -rev = "98f1821897cd2800e5bffb2a70541056145e99cc" +rev = "eb59676a940b15f77871aefe1e46d7b5bf85f40a" [dependencies.sqlx] version = "0.5.1" diff --git a/app/src/login.rs b/app/src/login.rs index 080c563..33ae52c 100644 --- a/app/src/login.rs +++ b/app/src/login.rs @@ -92,7 +92,7 @@ impl LoginForm { Ok(l) => l, }; let req = login::ClientLoginFinishRequest { - login_key: res.login_key, + server_data: res.server_data, credential_finalization: login_finish.message, }; self.call_backend( diff --git a/model/Cargo.toml b/model/Cargo.toml index 40b1989..4eec5a9 100644 --- a/model/Cargo.toml +++ b/model/Cargo.toml @@ -23,7 +23,7 @@ thiserror = "*" # TODO: update to 0.6 when out. [dependencies.opaque-ke] git = "https://github.com/novifinancial/opaque-ke" -rev = "98f1821897cd2800e5bffb2a70541056145e99cc" +rev = "eb59676a940b15f77871aefe1e46d7b5bf85f40a" [dependencies.chrono] version = "*" diff --git a/model/src/lib.rs b/model/src/lib.rs index b84d74f..1fd861d 100644 --- a/model/src/lib.rs +++ b/model/src/lib.rs @@ -14,6 +14,12 @@ pub struct BindRequest { pub mod login { use super::*; + #[derive(Serialize, Deserialize, Clone)] + pub struct ServerData { + pub username: String, + pub server_login: opaque::server::login::ServerLogin, + } + #[derive(Serialize, Deserialize, Clone)] pub struct ClientLoginStartRequest { pub username: String, @@ -22,15 +28,15 @@ pub mod login { #[derive(Serialize, Deserialize, Clone)] pub struct ServerLoginStartResponse { - /// A randomly-generated temporary key that corresponds to this login attempt. - pub login_key: String, + /// Base64, encrypted ServerData to be passed back to the server. + pub server_data: String, pub credential_response: opaque::client::login::CredentialResponse, } #[derive(Serialize, Deserialize, Clone)] pub struct ClientLoginFinishRequest { - /// The key returned by the server in the previous step. - pub login_key: String, + /// Encrypted ServerData from the previous step. + pub server_data: String, pub credential_finalization: opaque::client::login::CredentialFinalization, } } @@ -39,6 +45,11 @@ pub mod login { pub mod registration { use super::*; + #[derive(Serialize, Deserialize, Clone)] + pub struct ServerData { + pub username: String, + } + #[derive(Serialize, Deserialize, Clone)] pub struct ClientRegistrationStartRequest { pub username: String, @@ -47,15 +58,15 @@ pub mod registration { #[derive(Serialize, Deserialize, Clone)] pub struct ServerRegistrationStartResponse { - /// A randomly-generated temporary key that corresponds to this registration attempt. - pub registration_key: String, + /// Base64, encrypted ServerData to be passed back to the server. + pub server_data: String, pub registration_response: opaque::client::registration::RegistrationResponse, } #[derive(Serialize, Deserialize, Clone)] pub struct ClientRegistrationFinishRequest { - /// The key returned by the server in the previous step. - pub registration_key: String, + /// Encrypted ServerData from the previous step. + pub server_data: String, pub registration_upload: opaque::server::registration::RegistrationUpload, } } diff --git a/model/src/opaque.rs b/model/src/opaque.rs index c46b81d..2371c0a 100644 --- a/model/src/opaque.rs +++ b/model/src/opaque.rs @@ -100,18 +100,14 @@ pub mod client { pub type ClientLoginStartResult = opaque_ke::ClientLoginStartResult; pub type CredentialResponse = opaque_ke::CredentialResponse; pub type CredentialFinalization = opaque_ke::CredentialFinalization; - pub use opaque_ke::{ClientLoginFinishParameters, ClientLoginStartParameters}; + pub use opaque_ke::ClientLoginFinishParameters; /// Initiate the login negotiation. pub fn start_login( password: &str, rng: &mut R, ) -> AuthenticationResult { - Ok(ClientLogin::start( - rng, - password.as_bytes(), - ClientLoginStartParameters::default(), - )?) + Ok(ClientLogin::start(rng, password.as_bytes())?) } /// Finalize the client login negotiation. @@ -130,6 +126,7 @@ pub mod client { pub mod server { pub use super::*; pub type ServerRegistration = opaque_ke::ServerRegistration; + pub type ServerSetup = opaque_ke::ServerSetup; /// Methods to register a new user, from the server side. pub mod registration { pub use super::*; @@ -140,24 +137,21 @@ pub mod server { /// Start a registration process, from a request sent by the client. /// /// The result must be kept for the next step. - pub fn start_registration( - rng: &mut R, + pub fn start_registration( + server_setup: &ServerSetup, registration_request: RegistrationRequest, - server_public_key: &PublicKey, + username: &str, ) -> AuthenticationResult { Ok(ServerRegistration::start( - rng, + server_setup, registration_request, - server_public_key, + username.as_bytes(), )?) } /// Finish to register a new user, and get the data to store in the database. - pub fn get_password_file( - registration_start: ServerRegistration, - registration_upload: RegistrationUpload, - ) -> AuthenticationResult { - Ok(registration_start.finish(registration_upload)?) + pub fn get_password_file(registration_upload: RegistrationUpload) -> ServerRegistration { + ServerRegistration::finish(registration_upload) } } @@ -176,15 +170,17 @@ pub mod server { /// The result must be kept for the next step. pub fn start_login( rng: &mut R, - password_file: ServerRegistration, - server_private_key: &PrivateKey, + server_setup: &ServerSetup, + password_file: Option, credential_request: CredentialRequest, + username: &str, ) -> AuthenticationResult { Ok(ServerLogin::start( rng, + server_setup, password_file, - server_private_key, credential_request, + username.as_bytes(), ServerLoginStartParameters::default(), )?) } diff --git a/src/domain/error.rs b/src/domain/error.rs index 0a77d18..13c223c 100644 --- a/src/domain/error.rs +++ b/src/domain/error.rs @@ -9,6 +9,12 @@ pub enum DomainError { DatabaseError(#[from] sqlx::Error), #[error("Authentication protocol error for `{0}`")] AuthenticationProtocolError(#[from] lldap_model::opaque::AuthenticationError), + #[error("Unknown crypto error: `{0}`")] + UnknownCryptoError(#[from] orion::errors::UnknownCryptoError), + #[error("Binary serialization error: `{0}`")] + BinarySerializationError(#[from] bincode::Error), + #[error("Invalid base64: `{0}`")] + Base64DecodeError(#[from] base64::DecodeError), #[error("Internal error: `{0}`")] InternalError(String), } diff --git a/src/domain/sql_backend_handler.rs b/src/domain/sql_backend_handler.rs index 55b9f5f..ecce733 100644 --- a/src/domain/sql_backend_handler.rs +++ b/src/domain/sql_backend_handler.rs @@ -22,7 +22,8 @@ impl SqlBackendHandler { pub fn get_password_file( clear_password: &str, - server_public_key: &opaque::PublicKey, + server_setup: &opaque::server::ServerSetup, + username: &str, ) -> Result { use opaque::{client, server}; let mut rng = rand::rngs::OsRng; @@ -30,9 +31,9 @@ pub fn get_password_file( client::registration::start_registration(clear_password, &mut rng)?; let server_register_start_result = server::registration::start_registration( - &mut rng, + server_setup, client_register_start_result.message, - server_public_key, + username, )?; let client_registration_result = client::registration::finish_registration( @@ -42,9 +43,8 @@ pub fn get_password_file( )?; Ok(server::registration::get_password_file( - server_register_start_result.state, client_registration_result.message, - )?) + )) } fn get_filter_expr(filter: RequestFilter) -> SimpleExpr { @@ -187,7 +187,7 @@ impl BackendHandler for SqlBackendHandler { Users::CreationDate, ]; let mut values = vec![ - request.user_id.into(), + request.user_id.clone().into(), request.email.into(), request.display_name.map(Into::into).unwrap_or(Value::Null), request.first_name.map(Into::into).unwrap_or(Value::Null), @@ -197,7 +197,7 @@ impl BackendHandler for SqlBackendHandler { if let Some(pass) = request.password { columns.push(Users::PasswordHash); values.push( - get_password_file(&pass, self.config.get_server_keys().public())? + get_password_file(&pass, self.config.get_server_setup(), &request.user_id)? .serialize() .into(), ); diff --git a/src/domain/sql_opaque_handler.rs b/src/domain/sql_opaque_handler.rs index ee61970..c39f064 100644 --- a/src/domain/sql_opaque_handler.rs +++ b/src/domain/sql_opaque_handler.rs @@ -5,25 +5,16 @@ use super::{ use async_trait::async_trait; use lldap_model::{opaque, BindRequest}; use log::*; -use rand::{CryptoRng, RngCore}; use sea_query::{Expr, Iden, Query}; use sqlx::Row; type SqlOpaqueHandler = SqlBackendHandler; -fn generate_random_id(rng: &mut R) -> String { - use rand::{distributions::Alphanumeric, Rng}; - std::iter::repeat(()) - .map(|()| rng.sample(Alphanumeric)) - .map(char::from) - .take(32) - .collect() -} - fn passwords_match( password_file_bytes: &[u8], clear_password: &str, - server_private_key: &opaque::PrivateKey, + server_setup: &opaque::server::ServerSetup, + username: &str, ) -> Result<()> { use opaque::{client, server}; let mut rng = rand::rngs::OsRng; @@ -33,9 +24,10 @@ fn passwords_match( .map_err(opaque::AuthenticationError::ProtocolError)?; let server_login_start_result = server::login::start_login( &mut rng, - password_file, - server_private_key, + server_setup, + Some(password_file), client_login_start_result.message, + username, )?; client::login::finish_login( client_login_start_result.state, @@ -44,6 +36,40 @@ fn passwords_match( Ok(()) } +impl SqlBackendHandler { + fn get_orion_secret_key(&self) -> Result { + Ok(orion::aead::SecretKey::from_slice( + self.config.get_server_keys().private(), + )?) + } + + async fn get_password_file_for_user( + &self, + username: &str, + ) -> Result> { + // Fetch the previously registered password file from the DB. + let password_file_bytes = { + let query = Query::select() + .column(Users::PasswordHash) + .from(Users::Table) + .and_where(Expr::col(Users::UserId).eq(username)) + .to_string(DbQueryBuilder {}); + if let Some(row) = sqlx::query(&query).fetch_optional(&self.sql_pool).await? { + row.get::>, _>(&*Users::PasswordHash.to_string()) + // If no password, always fail. + .ok_or_else(|| DomainError::AuthenticationError(username.to_string()))? + } else { + return Ok(None); + } + }; + opaque::server::ServerRegistration::deserialize(&password_file_bytes) + .map(Option::Some) + .map_err(|_| { + DomainError::InternalError(format!("Corrupted password file for {}", username)) + }) + } +} + #[async_trait] impl LoginHandler for SqlBackendHandler { async fn bind(&self, request: BindRequest) -> Result<()> { @@ -67,7 +93,8 @@ impl LoginHandler for SqlBackendHandler { if let Err(e) = passwords_match( &password_hash, &request.password, - self.config.get_server_keys().private(), + self.config.get_server_setup(), + &request.name, ) { debug!(r#"Invalid password for "{}": {}"#, request.name, e); } else { @@ -89,99 +116,44 @@ impl OpaqueHandler for SqlOpaqueHandler { &self, request: login::ClientLoginStartRequest, ) -> Result { - // Fetch the previously registered password file from the DB. - let password_file_bytes = { - let query = Query::select() - .column(Users::PasswordHash) - .from(Users::Table) - .and_where(Expr::col(Users::UserId).eq(request.username.as_str())) - .to_string(DbQueryBuilder {}); - sqlx::query(&query) - .fetch_one(&self.sql_pool) - .await? - .get::>, _>(&*Users::PasswordHash.to_string()) - // If no password, always fail. - .ok_or_else(|| DomainError::AuthenticationError(request.username.clone()))? - }; - let password_file = opaque::server::ServerRegistration::deserialize(&password_file_bytes) - .map_err(|_| { - DomainError::InternalError(format!("Corrupted password file for {}", request.username)) - })?; + let maybe_password_file = self.get_password_file_for_user(&request.username).await?; let mut rng = rand::rngs::OsRng; let start_response = opaque::server::login::start_login( &mut rng, - password_file, - self.config.get_server_keys().private(), + self.config.get_server_setup(), + maybe_password_file, request.login_start_request, + &request.username, )?; - let login_attempt_id = generate_random_id(&mut rng); - - { - // Insert the current login attempt in the DB. - let query = Query::insert() - .into_table(LoginAttempts::Table) - .columns(vec![ - LoginAttempts::RandomId, - LoginAttempts::UserId, - LoginAttempts::ServerLoginData, - LoginAttempts::Timestamp, - ]) - .values_panic(vec![ - login_attempt_id.as_str().into(), - request.username.as_str().into(), - start_response.state.serialize().into(), - chrono::Utc::now().naive_utc().into(), - ]) - .to_string(DbQueryBuilder {}); - sqlx::query(&query).execute(&self.sql_pool).await?; - } + let secret_key = self.get_orion_secret_key()?; + let server_data = login::ServerData { + username: request.username, + server_login: start_response.state, + }; + let encrypted_state = orion::aead::seal(&secret_key, &bincode::serialize(&server_data)?)?; Ok(login::ServerLoginStartResponse { - login_key: login_attempt_id, + server_data: base64::encode(&encrypted_state), credential_response: start_response.message, }) } async fn login_finish(&self, request: login::ClientLoginFinishRequest) -> Result { - // Fetch the previous data from this login attempt. - let row = { - let query = Query::select() - .column(LoginAttempts::UserId) - .column(LoginAttempts::ServerLoginData) - .from(LoginAttempts::Table) - .and_where(Expr::col(LoginAttempts::RandomId).eq(request.login_key.as_str())) - .and_where( - Expr::col(LoginAttempts::Timestamp) - .gt(chrono::Utc::now().naive_utc() - chrono::Duration::minutes(5)), - ) - .to_string(DbQueryBuilder {}); - sqlx::query(&query).fetch_one(&self.sql_pool).await? - }; - let username = row.get::(&*LoginAttempts::UserId.to_string()); - let login_data = opaque::server::login::ServerLogin::deserialize( - &row.get::, _>(&*LoginAttempts::ServerLoginData.to_string()), - ) - .map_err(|_| { - DomainError::InternalError(format!( - "Corrupted login data for user `{}` [id `{}`]", - username, request.login_key - )) - })?; + let secret_key = self.get_orion_secret_key()?; + let login::ServerData { + username, + server_login, + } = bincode::deserialize(&orion::aead::open( + &secret_key, + &base64::decode(&request.server_data)?, + )?)?; // Finish the login: this makes sure the client data is correct, and gives a session key we // don't need. let _session_key = - opaque::server::login::finish_login(login_data, request.credential_finalization)? + opaque::server::login::finish_login(server_login, request.credential_finalization)? .session_key; - { - // Login was successful, we can delete the login attempt from the table. - let delete_query = Query::delete() - .from_table(LoginAttempts::Table) - .and_where(Expr::col(LoginAttempts::RandomId).eq(request.login_key)) - .to_string(DbQueryBuilder {}); - sqlx::query(&delete_query).execute(&self.sql_pool).await?; - } Ok(username) } @@ -189,36 +161,19 @@ impl OpaqueHandler for SqlOpaqueHandler { &self, request: registration::ClientRegistrationStartRequest, ) -> Result { - let mut rng = rand::rngs::OsRng; // Generate the server-side key and derive the data to send back. let start_response = opaque::server::registration::start_registration( - &mut rng, + self.config.get_server_setup(), request.registration_start_request, - self.config.get_server_keys().public(), + &request.username, )?; - // Unique ID to identify the registration attempt. - let registration_attempt_id = generate_random_id(&mut rng); - { - // Write the registration attempt to the DB for the later turn. - let query = Query::insert() - .into_table(RegistrationAttempts::Table) - .columns(vec![ - RegistrationAttempts::RandomId, - RegistrationAttempts::UserId, - RegistrationAttempts::ServerRegistrationData, - RegistrationAttempts::Timestamp, - ]) - .values_panic(vec![ - registration_attempt_id.as_str().into(), - request.username.as_str().into(), - start_response.state.serialize().into(), - chrono::Utc::now().naive_utc().into(), - ]) - .to_string(DbQueryBuilder {}); - sqlx::query(&query).execute(&self.sql_pool).await?; - } + let secret_key = self.get_orion_secret_key()?; + let server_data = registration::ServerData { + username: request.username, + }; + let encrypted_state = orion::aead::seal(&secret_key, &bincode::serialize(&server_data)?)?; Ok(registration::ServerRegistrationStartResponse { - registration_key: registration_attempt_id, + server_data: base64::encode(encrypted_state), registration_response: start_response.message, }) } @@ -227,37 +182,14 @@ impl OpaqueHandler for SqlOpaqueHandler { &self, request: registration::ClientRegistrationFinishRequest, ) -> Result<()> { - // Fetch the previous state. - let row = { - let query = Query::select() - .column(RegistrationAttempts::UserId) - .column(RegistrationAttempts::ServerRegistrationData) - .from(RegistrationAttempts::Table) - .and_where( - Expr::col(RegistrationAttempts::RandomId).eq(request.registration_key.as_str()), - ) - .and_where( - Expr::col(RegistrationAttempts::Timestamp) - .gt(chrono::Utc::now().naive_utc() - chrono::Duration::minutes(5)), - ) - .to_string(DbQueryBuilder {}); - sqlx::query(&query).fetch_one(&self.sql_pool).await? - }; - let username = row.get::(&*RegistrationAttempts::UserId.to_string()); - let registration_data = opaque::server::registration::ServerRegistration::deserialize( - &row.get::, _>(&*RegistrationAttempts::ServerRegistrationData.to_string()), - ) - .map_err(|_| { - DomainError::InternalError(format!( - "Corrupted registration data for user `{}` [id `{}`]", - username, request.registration_key - )) - })?; + let secret_key = self.get_orion_secret_key()?; + let registration::ServerData { username } = bincode::deserialize(&orion::aead::open( + &secret_key, + &base64::decode(&request.server_data)?, + )?)?; - let password_file = opaque::server::registration::get_password_file( - registration_data, - request.registration_upload, - )?; + let password_file = + opaque::server::registration::get_password_file(request.registration_upload); { // Set the user password to the new password. let update_query = Query::update() @@ -270,14 +202,6 @@ impl OpaqueHandler for SqlOpaqueHandler { .to_string(DbQueryBuilder {}); sqlx::query(&update_query).execute(&self.sql_pool).await?; } - { - // Delete the registration attempt. - let delete_query = Query::delete() - .from_table(RegistrationAttempts::Table) - .and_where(Expr::col(RegistrationAttempts::RandomId).eq(request.registration_key)) - .to_string(DbQueryBuilder {}); - sqlx::query(&delete_query).execute(&self.sql_pool).await?; - } Ok(()) } } @@ -341,7 +265,7 @@ mod tests { )?; opaque_handler .login_finish(ClientLoginFinishRequest { - login_key: start_response.login_key, + server_data: start_response.server_data, credential_finalization: login_finish.message, }) .await?; @@ -370,7 +294,7 @@ mod tests { )?; opaque_handler .registration_finish(ClientRegistrationFinishRequest { - registration_key: start_response.registration_key, + server_data: start_response.server_data, registration_upload: registration_finish.message, }) .await diff --git a/src/domain/sql_tables.rs b/src/domain/sql_tables.rs index 16d991e..70b3dd8 100644 --- a/src/domain/sql_tables.rs +++ b/src/domain/sql_tables.rs @@ -34,27 +34,6 @@ pub enum Memberships { GroupId, } -/// Contains the temporary data that needs to be kept between the first and second message when -/// logging in with the OPAQUE protocol. -#[derive(Iden)] -pub enum LoginAttempts { - Table, - RandomId, - UserId, - ServerLoginData, - Timestamp, -} - -/// Same for registration. -#[derive(Iden)] -pub enum RegistrationAttempts { - Table, - RandomId, - UserId, - ServerRegistrationData, - Timestamp, -} - pub async fn init_table(pool: &Pool) -> sqlx::Result<()> { // SQLite needs this pragma to be turned on. Other DB might not understand this, so ignore the // error. @@ -135,80 +114,6 @@ pub async fn init_table(pool: &Pool) -> sqlx::Result<()> { .execute(pool) .await?; - sqlx::query( - &Table::create() - .table(LoginAttempts::Table) - .if_not_exists() - .col( - ColumnDef::new(LoginAttempts::RandomId) - .string_len(32) - .primary_key(), - ) - .col( - ColumnDef::new(LoginAttempts::UserId) - .string_len(255) - .not_null(), - ) - .col( - ColumnDef::new(LoginAttempts::ServerLoginData) - .binary() - .not_null(), - ) - .col( - ColumnDef::new(LoginAttempts::Timestamp) - .date_time() - .not_null(), - ) - .foreign_key( - ForeignKey::create() - .name("LoginAttemptsUserIdForeignKey") - .table(LoginAttempts::Table, Users::Table) - .col(LoginAttempts::UserId, Users::UserId) - .on_delete(ForeignKeyAction::Cascade) - .on_update(ForeignKeyAction::Cascade), - ) - .to_string(DbQueryBuilder {}), - ) - .execute(pool) - .await?; - - sqlx::query( - &Table::create() - .table(RegistrationAttempts::Table) - .if_not_exists() - .col( - ColumnDef::new(RegistrationAttempts::RandomId) - .string_len(32) - .primary_key(), - ) - .col( - ColumnDef::new(RegistrationAttempts::UserId) - .string_len(255) - .not_null(), - ) - .col( - ColumnDef::new(RegistrationAttempts::ServerRegistrationData) - .binary() - .not_null(), - ) - .col( - ColumnDef::new(RegistrationAttempts::Timestamp) - .date_time() - .not_null(), - ) - .foreign_key( - ForeignKey::create() - .name("RegistrationAttemptsUserIdForeignKey") - .table(RegistrationAttempts::Table, Users::Table) - .col(RegistrationAttempts::UserId, Users::UserId) - .on_delete(ForeignKeyAction::Cascade) - .on_update(ForeignKeyAction::Cascade), - ) - .to_string(DbQueryBuilder {}), - ) - .execute(pool) - .await?; - Ok(()) } diff --git a/src/infra/configuration.rs b/src/infra/configuration.rs index 3525bf9..730d98a 100644 --- a/src/infra/configuration.rs +++ b/src/infra/configuration.rs @@ -3,7 +3,7 @@ use figment::{ providers::{Env, Format, Serialized, Toml}, Figment, }; -use lldap_model::{opaque, opaque::KeyPair}; +use lldap_model::opaque::{server::ServerSetup, KeyPair}; use serde::{Deserialize, Serialize}; use crate::infra::cli::CLIOpts; @@ -27,18 +27,18 @@ pub struct Configuration { pub key_file: String, #[serde(skip)] #[builder(field(private), setter(strip_option))] - server_keys: Option, + server_setup: Option, } impl ConfigurationBuilder { #[cfg(test)] pub fn build(self) -> Result { - let server_keys = get_server_keys(self.key_file.as_deref().unwrap_or("server_key"))?; - Ok(self.server_keys(server_keys).private_build()?) + let server_setup = get_server_setup(self.key_file.as_deref().unwrap_or("server_key"))?; + Ok(self.server_setup(server_setup).private_build()?) } fn validate(&self) -> Result<(), String> { - if self.server_keys.is_none() { + if self.server_setup.is_none() { Err("Don't use `private_build`, use `build` instead".to_string()) } else { Ok(()) @@ -47,8 +47,12 @@ impl ConfigurationBuilder { } impl Configuration { + pub fn get_server_setup(&self) -> &ServerSetup { + self.server_setup.as_ref().unwrap() + } + pub fn get_server_keys(&self) -> &KeyPair { - self.server_keys.as_ref().unwrap() + self.get_server_setup().keypair() } fn merge_with_cli(mut self: Configuration, cli_opts: CLIOpts) -> Configuration { @@ -80,30 +84,29 @@ impl Configuration { database_url: String::from("sqlite://users.db?mode=rwc"), verbose: false, key_file: String::from("server_key"), - server_keys: None, + server_setup: None, } } } -fn get_server_keys(file_path: &str) -> Result { - use opaque_ke::ciphersuite::CipherSuite; +fn get_server_setup(file_path: &str) -> Result { use std::path::Path; let path = Path::new(file_path); if path.exists() { let bytes = std::fs::read(file_path) .map_err(|e| anyhow!("Could not read key file `{}`: {}", file_path, e))?; - Ok(KeyPair::from_private_key_slice(&bytes)?) + Ok(ServerSetup::deserialize(&bytes)?) } else { let mut rng = rand::rngs::OsRng; - let keypair = opaque::DefaultSuite::generate_random_keypair(&mut rng); - std::fs::write(path, keypair.private().as_slice()).map_err(|e| { + let server_setup = ServerSetup::new(&mut rng); + std::fs::write(path, server_setup.serialize()).map_err(|e| { anyhow!( - "Could not write the generated server keys to file `{}`: {}", + "Could not write the generated server setup to file `{}`: {}", file_path, e ) })?; - Ok(keypair) + Ok(server_setup) } } @@ -116,6 +119,6 @@ pub fn init(cli_opts: CLIOpts) -> Result { .extract()?; let mut config = config.merge_with_cli(cli_opts); - config.server_keys = Some(get_server_keys(&config.key_file)?); + config.server_setup = Some(get_server_setup(&config.key_file)?); Ok(config) } diff --git a/src/infra/db_cleaner.rs b/src/infra/db_cleaner.rs index a90a4ad..5fc68f3 100644 --- a/src/infra/db_cleaner.rs +++ b/src/infra/db_cleaner.rs @@ -1,5 +1,5 @@ use crate::{ - domain::sql_tables::{DbQueryBuilder, LoginAttempts, Pool, RegistrationAttempts}, + domain::sql_tables::{DbQueryBuilder, Pool}, infra::jwt_sql_tables::{JwtRefreshStorage, JwtStorage}, }; use actix::prelude::*; @@ -70,34 +70,6 @@ impl Scheduler { { log::error!("DB error while cleaning up JWT storage: {}", e); }; - if let Err(e) = sqlx::query( - &Query::delete() - .from_table(LoginAttempts::Table) - .and_where( - Expr::col(LoginAttempts::Timestamp) - .lt(Local::now().naive_utc() - chrono::Duration::minutes(5)), - ) - .to_string(DbQueryBuilder {}), - ) - .execute(&sql_pool) - .await - { - log::error!("DB error while cleaning up login attempts: {}", e); - }; - if let Err(e) = sqlx::query( - &Query::delete() - .from_table(RegistrationAttempts::Table) - .and_where( - Expr::col(RegistrationAttempts::Timestamp) - .lt(Local::now().naive_utc() - chrono::Duration::minutes(5)), - ) - .to_string(DbQueryBuilder {}), - ) - .execute(&sql_pool) - .await - { - log::error!("DB error while cleaning up registration attempts: {}", e); - }; log::info!("DB cleaned!"); } diff --git a/src/infra/tcp_server.rs b/src/infra/tcp_server.rs index dd6686c..f3d16ff 100644 --- a/src/infra/tcp_server.rs +++ b/src/infra/tcp_server.rs @@ -32,8 +32,11 @@ pub(crate) fn error_to_http_response(error: DomainError) -> HttpResponse { DomainError::AuthenticationError(_) | DomainError::AuthenticationProtocolError(_) => { HttpResponse::Unauthorized() } - DomainError::DatabaseError(_) | DomainError::InternalError(_) => { - HttpResponse::InternalServerError() + DomainError::DatabaseError(_) + | DomainError::InternalError(_) + | DomainError::UnknownCryptoError(_) => HttpResponse::InternalServerError(), + DomainError::Base64DecodeError(_) | DomainError::BinarySerializationError(_) => { + HttpResponse::BadRequest() } } .body(error.to_string())