Implement password checking using opaque

.gitignore

@@ -12,3 +12,6 @@
 *.db
 *.db-shm
 *.db-wal
+
+# Server private key
+server_key

Cargo.lock

@@ -698,6 +698,72 @@ dependencies = [
  "zeroize",
 ]
 
+[[package]]
+name = "darling"
+version = "0.12.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "5f2c43f534ea4b0b049015d00269734195e6d3f0f6635cb692251aca6f9f8b3c"
+dependencies = [
+ "darling_core",
+ "darling_macro",
+]
+
+[[package]]
+name = "darling_core"
+version = "0.12.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "8e91455b86830a1c21799d94524df0845183fa55bafd9aa137b01c7d1065fa36"
+dependencies = [
+ "fnv",
+ "ident_case",
+ "proc-macro2",
+ "quote",
+ "strsim",
+ "syn",
+]
+
+[[package]]
+name = "darling_macro"
+version = "0.12.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "29b5acf0dea37a7f66f7b25d2c5e93fd46f8f6968b1a5d7a3e02e97768afc95a"
+dependencies = [
+ "darling_core",
+ "quote",
+ "syn",
+]
+
+[[package]]
+name = "derive_builder"
+version = "0.10.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d13202debe11181040ae9063d739fa32cfcaaebe2275fe387703460ae2365b30"
+dependencies = [
+ "derive_builder_macro",
+]
+
+[[package]]
+name = "derive_builder_core"
+version = "0.10.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "66e616858f6187ed828df7c64a6d71720d83767a7f19740b2d1b6fe6327b36e5"
+dependencies = [
+ "darling",
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
+[[package]]
+name = "derive_builder_macro"
+version = "0.10.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "58a94ace95092c5acb1e97a7e846b310cfbd499652f72297da7493f618a98d73"
+dependencies = [
+ "derive_builder_core",
+ "syn",
+]
+
 [[package]]
 name = "derive_more"
 version = "0.99.14"
@@ -1088,6 +1154,12 @@ version = "1.4.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "f3a87b616e37e93c22fb19bcd386f02f3af5ea98a25670ad0fce773de23c5e68"
 
+[[package]]
+name = "ident_case"
+version = "1.0.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "b9e0384b61958566e926dc50660321d12159025e767c18e043daf26b70104c39"
+
 [[package]]
 name = "idna"
 version = "0.2.3"
@@ -1253,6 +1325,7 @@ dependencies = [
  "chrono",
  "clap",
  "cron",
+ "derive_builder",
  "figment",
  "futures",
  "futures-util",
@@ -1263,8 +1336,8 @@ dependencies = [
  "lldap_model",
  "log",
  "mockall",
+ "opaque-ke",
  "rand 0.8.3",
- "rust-argon2",
  "sea-query",
  "serde",
  "serde_json",

Cargo.toml

@@ -12,11 +12,11 @@ actix-server = "2.0.0-beta.3"
 actix-service = "2.0.0"
 actix-web = "4.0.0-beta.3"
 anyhow = "*"
-rust-argon2 = "0.8"
 async-trait = "0.1"
 chrono = { version = "*", features = [ "serde" ]}
 clap = "3.0.0-beta.2"
 cron = "*"
+derive_builder = "0.10.2"
 futures = "*"
 futures-util = "*"
 hmac = "0.10"
@@ -25,6 +25,7 @@ jwt = "0.13"
 ldap3_server = "*"
 lldap_model = { path = "model" }
 log = "*"
+opaque-ke = "0.5"
 serde = "*"
 serde_json = "1"
 sha2 = "0.9"

model/src/opaque.rs

@@ -9,6 +9,41 @@ pub enum AuthenticationError {
 
 pub type AuthenticationResult<T> = std::result::Result<T, AuthenticationError>;
 
+/// Wrapper around an opaque KeyPair to have type-checked public and private keys.
+#[derive(Debug, Clone)]
+pub struct KeyPair(pub opaque_ke::keypair::KeyPair<<DefaultSuite as CipherSuite>::Group>);
+
+pub struct PublicKey<'a>(&'a opaque_ke::keypair::Key);
+pub struct PrivateKey<'a>(&'a opaque_ke::keypair::Key);
+
+impl <'a> std::ops::Deref for PublicKey<'a> {
+    type Target = &'a opaque_ke::keypair::Key;
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl <'a> std::ops::Deref for PrivateKey<'a> {
+    type Target = &'a opaque_ke::keypair::Key;
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl KeyPair {
+    pub fn private(&self) -> PrivateKey<'_> {
+        PrivateKey(self.0.private())
+    }
+
+    pub fn public(&self) -> PublicKey<'_> {
+        PublicKey(self.0.public())
+    }
+
+    pub fn from_private_key_slice(input: &[u8]) -> std::result::Result<Self, opaque_ke::errors::InternalPakeError> {
+        opaque_ke::keypair::KeyPair::<<DefaultSuite as CipherSuite>::Group>::from_private_key_slice(input).map(Self)
+    }
+}
+
 /// A wrapper around argon2 to provide the [`opaque_ke::slow_hash::SlowHash`] trait.
 pub struct ArgonHasher;
 
@@ -56,11 +91,11 @@ impl CipherSuite for DefaultSuite {
 /// deserialized using the type's `deserialize` method.
 #[cfg(feature = "opaque_client")]
 pub mod client {
-    use super::*;
+    pub use super::*;
     /// Methods to register a new user, from the client side.
     pub mod registration {
-        use super::*;
+        pub use super::*;
-        use opaque_ke::{
+        pub use opaque_ke::{
             ClientRegistration, ClientRegistrationFinishParameters, ClientRegistrationFinishResult,
             ClientRegistrationStartResult, RegistrationResponse,
         };
@@ -91,8 +126,8 @@ pub mod client {
 
     /// Methods to login, from the client side.
     pub mod login {
-        use super::*;
+        pub use super::*;
-        use opaque_ke::{
+        pub use opaque_ke::{
             ClientLogin, ClientLoginFinishParameters, ClientLoginFinishResult,
             ClientLoginStartParameters, ClientLoginStartResult, CredentialResponse,
         };
@@ -123,24 +158,24 @@ pub mod client {
 /// intermediate results must be sent to the client using the serialized `.message`.
 #[cfg(feature = "opaque_server")]
 pub mod server {
-    use super::*;
+    pub use super::*;
-    use opaque_ke::{keypair::Key, ServerRegistration};
+    pub use opaque_ke::ServerRegistration;
     /// Methods to register a new user, from the server side.
     pub mod registration {
-        use super::*;
+        pub use super::*;
-        use opaque_ke::{RegistrationRequest, RegistrationUpload, ServerRegistrationStartResult};
+        pub use opaque_ke::{RegistrationRequest, RegistrationUpload, ServerRegistrationStartResult};
         /// Start a registration process, from a request sent by the client.
         ///
         /// The result must be kept for the next step.
         pub fn start_registration<R: RngCore + CryptoRng>(
             rng: &mut R,
             registration_request: RegistrationRequest<DefaultSuite>,
-            server_public_key: &Key,
+            server_public_key: PublicKey<'_>,
         ) -> AuthenticationResult<ServerRegistrationStartResult<DefaultSuite>> {
             Ok(ServerRegistration::<DefaultSuite>::start(
                 rng,
                 registration_request,
-                server_public_key,
+                *server_public_key,
             )?)
         }
 
@@ -155,8 +190,8 @@ pub mod server {
 
     /// Methods to handle user login, from the server-side.
     pub mod login {
-        use super::*;
+        pub use super::*;
-        use opaque_ke::{
+        pub use opaque_ke::{
             CredentialFinalization, CredentialRequest, ServerLogin, ServerLoginFinishResult,
             ServerLoginStartParameters, ServerLoginStartResult,
         };
@@ -167,13 +202,13 @@ pub mod server {
         pub fn start_login<R: RngCore + CryptoRng>(
             rng: &mut R,
             password_file: ServerRegistration<DefaultSuite>,
-            server_private_key: &Key,
+            server_private_key: PrivateKey<'_>,
             credential_request: CredentialRequest<DefaultSuite>,
         ) -> AuthenticationResult<ServerLoginStartResult<DefaultSuite>> {
             Ok(ServerLogin::start(
                 rng,
                 password_file,
-                server_private_key,
+                *server_private_key,
                 credential_request,
                 ServerLoginStartParameters::default(),
             )?)

src/domain/error.rs

@@ -6,6 +6,8 @@ pub enum Error {
     AuthenticationError(String),
     #[error("Database error: `{0}`")]
     DatabaseError(#[from] sqlx::Error),
+    #[error("Authentication protocol error for `{0}`")]
+    AuthenticationProtocolError(#[from] lldap_model::opaque::AuthenticationError),
 }
 
 pub type Result<T> = std::result::Result<T, Error>;

src/domain/sql_backend_handler.rs

@@ -3,6 +3,7 @@ use crate::infra::configuration::Configuration;
 use async_trait::async_trait;
 use futures_util::StreamExt;
 use futures_util::TryStreamExt;
+use lldap_model::opaque;
 use log::*;
 use sea_query::{Expr, Iden, Order, Query, SimpleExpr, Value};
 use sqlx::Row;
@@ -20,31 +21,55 @@ impl SqlBackendHandler {
     }
 }
 
-fn get_password_config(pepper: &str) -> argon2::Config {
+fn get_password_file(
-    argon2::Config {
+    clear_password: &str,
-        secret: pepper.as_bytes(),
+    server_public_key: opaque::PublicKey<'_>,
-        ..Default::default()
+) -> Result<opaque::server::ServerRegistration<opaque::DefaultSuite>> {
-    }
+    use opaque::{client, server};
+    let mut rng = rand::rngs::OsRng;
+    let client_register_start_result =
+        client::registration::start_registration(clear_password, &mut rng)?;
+
+    let server_register_start_result = server::registration::start_registration(
+        &mut rng,
+        client_register_start_result.message,
+        server_public_key,
+    )?;
+
+    let client_registration_result = client::registration::finish_registration(
+        client_register_start_result.state,
+        server_register_start_result.message,
+        &mut rng,
+    )?;
+
+    Ok(server::registration::get_password_file(
+        server_register_start_result.state,
+        client_registration_result.message,
+    )?)
 }
 
-fn hash_password(clear_password: &str, salt: &str, pepper: &str) -> String {
+fn passwords_match(
-    let config = get_password_config(pepper);
+    password_file_bytes: &[u8],
-    argon2::hash_encoded(clear_password.as_bytes(), salt.as_bytes(), &config)
+    clear_password: &str,
-        .map_err(|e| anyhow::anyhow!("Error encoding password: {}", e))
+    server_private_key: opaque::PrivateKey<'_>,
-        .unwrap()
+) -> Result<()> {
-}
+    use opaque::{client, client::login::*, server, server::login::*, DefaultSuite};
+    let mut rng = rand::rngs::OsRng;
+    let client_login_start_result = client::login::start_login(clear_password, &mut rng)?;
 
-fn passwords_match(encrypted_password: &str, clear_password: &str, pepper: &str) -> bool {
+    let password_file = ServerRegistration::<DefaultSuite>::deserialize(password_file_bytes)
-    argon2::verify_encoded_ext(
+        .map_err(opaque::AuthenticationError::ProtocolError)?;
-        encrypted_password,
+    let server_login_start_result = server::login::start_login(
-        clear_password.as_bytes(),
+        &mut rng,
-        pepper.as_bytes(),
+        password_file,
-        /*additional_data=*/ b"",
+        server_private_key,
-    )
+        client_login_start_result.message,
-    .unwrap_or_else(|e| {
+    )?;
-        log::error!("Error checking password: {}", e);
+    finish_login(
-        false
+        client_login_start_result.state,
-    })
+        server_login_start_result.message,
+    )?;
+    Ok(())
 }
 
 fn get_filter_expr(filter: RequestFilter) -> SimpleExpr {
@@ -85,14 +110,14 @@ impl BackendHandler for SqlBackendHandler {
             .and_where(Expr::col(Users::UserId).eq(request.name.as_str()))
             .to_string(DbQueryBuilder {});
         if let Ok(row) = sqlx::query(&query).fetch_one(&self.sql_pool).await {
-            if passwords_match(
+            if let Err(e) = passwords_match(
-                &row.get::<String, _>(&*Users::PasswordHash.to_string()),
+                &row.get::<Vec<u8>, _>(&*Users::PasswordHash.to_string()),
                 &request.password,
-                &self.config.secret_pepper,
+                self.config.get_server_keys().private(),
             ) {
-                return Ok(());
+                debug!(r#"Invalid password for "{}": {}"#, request.name, e);
             } else {
-                debug!(r#"Invalid password for "{}""#, request.name);
+                return Ok(());
             }
         } else {
             debug!(r#"No user found for "{}""#, request.name);
@@ -208,16 +233,9 @@ impl BackendHandler for SqlBackendHandler {
     }
 
     async fn create_user(&self, request: CreateUserRequest) -> Result<()> {
-        use rand::{distributions::Alphanumeric, rngs::SmallRng, Rng, SeedableRng};
+        let password_hash =
-        // TODO: Initialize the rng only once. Maybe Arc<Cell>?
+            get_password_file(&request.password, self.config.get_server_keys().public())?
-        let mut rng = SmallRng::from_entropy();
+                .serialize();
-        let salt: String = std::iter::repeat(())
-            .map(|()| rng.sample(Alphanumeric))
-            .map(char::from)
-            .take(32)
-            .collect();
-        // The salt is included in the password hash.
-        let password_hash = hash_password(&request.password, &salt, &self.config.secret_pepper);
         let query = Query::insert()
             .into_table(Users::Table)
             .columns(vec![
@@ -283,6 +301,14 @@ impl BackendHandler for SqlBackendHandler {
 mod tests {
     use super::*;
     use crate::domain::sql_tables::init_table;
+    use crate::infra::configuration::ConfigurationBuilder;
+
+    fn get_default_config() -> Configuration {
+        ConfigurationBuilder::default()
+            .verbose(true)
+            .build()
+            .unwrap()
+    }
 
     async fn get_in_memory_db() -> Pool {
         PoolOptions::new().connect("sqlite::memory:").await.unwrap()
@@ -328,11 +354,11 @@ mod tests {
     #[tokio::test]
     async fn test_bind_admin() {
         let sql_pool = get_in_memory_db().await;
-        let config = Configuration {
+        let config = ConfigurationBuilder::default()
-            ldap_user_dn: "admin".to_string(),
+            .ldap_user_dn("admin".to_string())
-            ldap_user_pass: "test".to_string(),
+            .ldap_user_pass("test".to_string())
-            ..Default::default()
+            .build()
-        };
+            .unwrap();
         let handler = SqlBackendHandler::new(config, sql_pool);
         handler
             .bind(BindRequest {
@@ -343,24 +369,10 @@ mod tests {
             .unwrap();
     }
 
-    #[test]
-    fn test_argon() {
-        let password = b"password";
-        let salt = b"randomsalt";
-        let pepper = b"pepper";
-        let config = argon2::Config {
-            secret: pepper,
-            ..Default::default()
-        };
-        let hash = argon2::hash_encoded(password, salt, &config).unwrap();
-        let matches = argon2::verify_encoded_ext(&hash, password, pepper, b"").unwrap();
-        assert!(matches);
-    }
-
     #[tokio::test]
     async fn test_bind_user() {
         let sql_pool = get_initialized_db().await;
-        let config = Configuration::default();
+        let config = get_default_config();
         let handler = SqlBackendHandler::new(config, sql_pool.clone());
         insert_user(&handler, "bob", "bob00").await;
 
@@ -390,7 +402,7 @@ mod tests {
     #[tokio::test]
     async fn test_list_users() {
         let sql_pool = get_initialized_db().await;
-        let config = Configuration::default();
+        let config = get_default_config();
         let handler = SqlBackendHandler::new(config, sql_pool);
         insert_user(&handler, "bob", "bob00").await;
         insert_user(&handler, "patrick", "pass").await;
@@ -455,7 +467,7 @@ mod tests {
     #[tokio::test]
     async fn test_list_groups() {
         let sql_pool = get_initialized_db().await;
-        let config = Configuration::default();
+        let config = get_default_config();
         let handler = SqlBackendHandler::new(config, sql_pool.clone());
         insert_user(&handler, "bob", "bob00").await;
         insert_user(&handler, "patrick", "pass").await;
@@ -484,7 +496,7 @@ mod tests {
     #[tokio::test]
     async fn test_get_user_groups() {
         let sql_pool = get_initialized_db().await;
-        let config = Configuration::default();
+        let config = get_default_config();
         let handler = SqlBackendHandler::new(config, sql_pool.clone());
         insert_user(&handler, "bob", "bob00").await;
         insert_user(&handler, "patrick", "pass").await;
@@ -519,7 +531,7 @@ mod tests {
     #[tokio::test]
     async fn test_delete_user() {
         let sql_pool = get_initialized_db().await;
-        let config = Configuration::default();
+        let config = get_default_config();
         let handler = SqlBackendHandler::new(config, sql_pool.clone());
 
         insert_user(&handler, "val", "s3np4i").await;

src/domain/sql_tables.rs

@@ -54,11 +54,7 @@ pub async fn init_table(pool: &Pool) -> sqlx::Result<()> {
             .col(ColumnDef::new(Users::LastName).string_len(255))
             .col(ColumnDef::new(Users::Avatar).binary())
             .col(ColumnDef::new(Users::CreationDate).date_time().not_null())
-            .col(
+            .col(ColumnDef::new(Users::PasswordHash).binary().not_null())
-                ColumnDef::new(Users::PasswordHash)
-                    .string_len(255)
-                    .not_null(),
-            )
             .col(ColumnDef::new(Users::TotpSecret).string_len(64))
             .col(ColumnDef::new(Users::MfaType).string_len(64))
             .to_string(DbQueryBuilder {}),

src/infra/configuration.rs

@@ -1,45 +1,61 @@
-use anyhow::Result;
+use anyhow::{anyhow, Result};
 use figment::{
     providers::{Env, Format, Serialized, Toml},
     Figment,
 };
+use lldap_model::{opaque, opaque::KeyPair};
 use serde::{Deserialize, Serialize};
 
 use crate::infra::cli::CLIOpts;
 
-#[derive(Clone, Debug, Deserialize, Serialize)]
+#[derive(Clone, Debug, Deserialize, Serialize, derive_builder::Builder)]
+#[builder(
+    pattern = "owned",
+    default = "Configuration::default()",
+    build_fn(name = "private_build", validate = "Self::validate")
+)]
 pub struct Configuration {
     pub ldap_port: u16,
     pub ldaps_port: u16,
     pub http_port: u16,
-    pub secret_pepper: String,
     pub jwt_secret: String,
     pub ldap_base_dn: String,
     pub ldap_user_dn: String,
     pub ldap_user_pass: String,
     pub database_url: String,
     pub verbose: bool,
+    pub key_file: String,
+    #[serde(skip)]
+    #[builder(field(private), setter(strip_option))]
+    server_keys: Option<KeyPair>,
 }
 
-impl Default for Configuration {
+impl ConfigurationBuilder {
-    fn default() -> Self {
+    #[cfg(test)]
-        Configuration {
+    pub fn build(self) -> Result<Configuration> {
-            ldap_port: 3890,
+        let server_keys = get_server_keys(
-            ldaps_port: 6360,
+            &self
-            http_port: 17170,
+                .key_file
-            secret_pepper: String::from("secretsecretpepper"),
+                .as_deref()
-            jwt_secret: String::from("secretjwtsecret"),
+                .unwrap_or("server_key"),
-            ldap_base_dn: String::from("dc=example,dc=com"),
+        )?;
-            // cn=admin,dc=example,dc=com
+        Ok(self.server_keys(server_keys).private_build()?)
-            ldap_user_dn: String::from("admin"),
+    }
-            ldap_user_pass: String::from("password"),
+
-            database_url: String::from("sqlite://users.db?mode=rwc"),
+    fn validate(&self) -> Result<(), String> {
-            verbose: false,
+        if self.server_keys.is_none() {
+            Err("Don't use `private_build`, use `build` instead".to_string())
+        } else {
+            Ok(())
         }
     }
 }
 
 impl Configuration {
+    pub fn get_server_keys(&self) -> &KeyPair {
+        self.server_keys.as_ref().unwrap()
+    }
+
     fn merge_with_cli(mut self: Configuration, cli_opts: CLIOpts) -> Configuration {
         if cli_opts.verbose {
             self.verbose = true;
@@ -55,6 +71,45 @@ impl Configuration {
 
         self
     }
+
+    pub(super) fn default() -> Self {
+        Configuration {
+            ldap_port: 3890,
+            ldaps_port: 6360,
+            http_port: 17170,
+            jwt_secret: String::from("secretjwtsecret"),
+            ldap_base_dn: String::from("dc=example,dc=com"),
+            // cn=admin,dc=example,dc=com
+            ldap_user_dn: String::from("admin"),
+            ldap_user_pass: String::from("password"),
+            database_url: String::from("sqlite://users.db?mode=rwc"),
+            verbose: false,
+            key_file: String::from("server_key"),
+            server_keys: None,
+        }
+    }
+}
+
+fn get_server_keys(file_path: &str) -> Result<KeyPair> {
+    use opaque_ke::ciphersuite::CipherSuite;
+    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)?)
+    } 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| {
+            anyhow!(
+                "Could not write the generated server keys to file `{}`: {}",
+                file_path,
+                e
+            )
+        })?;
+        Ok(KeyPair(keypair))
+    }
 }
 
 pub fn init(cli_opts: CLIOpts) -> Result<Configuration> {
@@ -65,6 +120,7 @@ pub fn init(cli_opts: CLIOpts) -> Result<Configuration> {
         .merge(Env::prefixed("LLDAP_"))
         .extract()?;
 
-    let config = config.merge_with_cli(cli_opts);
+    let mut config = config.merge_with_cli(cli_opts);
+    config.server_keys = Some(get_server_keys(&config.key_file)?);
     Ok(config)
 }

src/infra/tcp_server.rs

@@ -25,7 +25,9 @@ async fn index(req: HttpRequest) -> actix_web::Result<NamedFile> {
 
 pub(crate) fn error_to_http_response(error: DomainError) -> HttpResponse {
     match error {
-        DomainError::AuthenticationError(_) => HttpResponse::Unauthorized(),
+        DomainError::AuthenticationError(_) | DomainError::AuthenticationProtocolError(_) => {
+            HttpResponse::Unauthorized()
+        }
         DomainError::DatabaseError(_) => HttpResponse::InternalServerError(),
     }
     .body(error.to_string())