From 68119e1067b6b6aa84d62c077c390305a184e31d Mon Sep 17 00:00:00 2001 From: James Hodgkinson Date: Sat, 19 Oct 2024 09:51:45 +1000 Subject: [PATCH] more errors for the people (#3121) --- Cargo.lock | 1 + proto/src/internal/error.rs | 56 +++++++++++++------ unix_integration/common/Cargo.toml | 9 +-- unix_integration/common/src/unix_proto.rs | 3 +- unix_integration/pam_kanidm/src/core.rs | 5 +- .../resolver/src/bin/kanidm-unix.rs | 5 +- .../resolver/src/bin/kanidm_unixd.rs | 27 +++++---- 7 files changed, 70 insertions(+), 36 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c69dc064b..cec5cefb2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3369,6 +3369,7 @@ dependencies = [ "csv", "futures", "kanidm_build_profiles", + "kanidm_proto", "serde", "serde_json", "serde_with", diff --git a/proto/src/internal/error.rs b/proto/src/internal/error.rs index 7e4e79c5f..5e3d66854 100644 --- a/proto/src/internal/error.rs +++ b/proto/src/internal/error.rs @@ -143,6 +143,11 @@ pub enum OperationError { // Specific internal errors. + // Kanidm Generic Errors + KG001TaskTimeout, + KG002TaskCommFailure, + KG003CacheClearFailed, + // What about something like this for unique errors? // Credential Update Errors CU0001WebauthnAttestationNotTrusted, @@ -230,6 +235,14 @@ pub enum OperationError { // Web UI UI0001ChallengeSerialisation, UI0002InvalidState, + + // Unixd Things + KU001InitWhileSessionActive, + KU002ContinueWhileSessionInActive, + KU003PamAuthFailed, + KU004PamInitFailed, + KU005ErrorCheckingAccount, + KU006OnlyRootAllowed, } impl PartialEq for OperationError { @@ -324,29 +337,16 @@ impl OperationError { Self::TransactionAlreadyCommitted => None, Self::ValueDenyName => None, Self::DatabaseLockAcquisitionTimeout => Some("Unable to acquire a database lock - the current server may be too busy. Try again later.".into()), + Self::CU0001WebauthnAttestationNotTrusted => None, Self::CU0002WebauthnRegistrationError => None, Self::CU0003WebauthnUserNotVerified => Some("User Verification bit not set while registering credential, you may need to configure a PIN on this device.".into()), - Self::CU0001WebauthnAttestationNotTrusted => None, - Self::VS0001IncomingReplSshPublicKey => None, - Self::VS0003CertificateDerDecode => Some("Decoding the stored certificate from DER failed.".into()), - Self::VS0002CertificatePublicKeyDigest | - Self::VS0004CertificatePublicKeyDigest | - Self::VS0005CertificatePublicKeyDigest => Some("The certificates public key is unabled to be digested.".into()), - Self::VL0001ValueSshPublicKeyString => None, - Self::LD0001AnonymousNotAllowed => Some("Anonymous is not allowed to access LDAP with this method.".into()), - Self::SC0001IncomingSshPublicKey => None, - Self::MG0001InvalidReMigrationLevel => None, - Self::MG0002RaiseDomainLevelExceedsMaximum => None, - Self::MG0003ServerPhaseInvalidForMigration => None, Self::DB0001MismatchedRestoreVersion => None, Self::DB0002MismatchedRestoreVersion => None, Self::DB0003FilterResolveCacheBuild => None, Self::DB0004DatabaseTooOld => Some("The database is too old to be migrated.".into()), - Self::MG0004DomainLevelInDevelopment => None, - Self::MG0005GidConstraintsNotMet => None, - Self::MG0006SKConstraintsNotMet => Some("Migration Constraints Not Met - Security Keys should not be present.".into()), - Self::MG0007Oauth2StrictConstraintsNotMet => Some("Migration Constraints Not Met - All OAuth2 clients must have strict-redirect-uri mode enabled.".into()), - Self::MG0008SkipUpgradeAttempted => Some("Skip Upgrade Attempted.".into()), + Self::KG001TaskTimeout => Some("Task timed out".into()), + Self::KG002TaskCommFailure => Some("Inter-Task communication failure".into()), + Self::KG003CacheClearFailed => Some("Failed to clear cache".into()), Self::KP0001KeyProviderNotLoaded => None, Self::KP0002KeyProviderInvalidClass => None, Self::KP0003KeyProviderInvalidType => None, @@ -392,9 +392,31 @@ impl OperationError { Self::KP0042KeyObjectNoActiveEncryptionKeys => None, Self::KP0043KeyObjectJweA128GCMEncryption => None, Self::KP0044KeyObjectJwsPublicJwk => None, + Self::KU001InitWhileSessionActive => Some("The session was active when the init function was called.".into()), + Self::KU002ContinueWhileSessionInActive => Some("Attempted to continue auth session while current session is inactive".into()), + Self::KU003PamAuthFailed => Some("Failed PAM account authentication step".into()), + Self::KU004PamInitFailed => Some("Failed to initialise PAM authentication".into()), + Self::KU005ErrorCheckingAccount => Some("Error checking account".into()), + Self::KU006OnlyRootAllowed => Some("Only root is allowed to perform this operation".into()), + Self::LD0001AnonymousNotAllowed => Some("Anonymous is not allowed to access LDAP with this method.".into()), + Self::MG0001InvalidReMigrationLevel => None, + Self::MG0002RaiseDomainLevelExceedsMaximum => None, + Self::MG0003ServerPhaseInvalidForMigration => None, + Self::MG0004DomainLevelInDevelopment => None, + Self::MG0005GidConstraintsNotMet => None, + Self::MG0006SKConstraintsNotMet => Some("Migration Constraints Not Met - Security Keys should not be present.".into()), + Self::MG0007Oauth2StrictConstraintsNotMet => Some("Migration Constraints Not Met - All OAuth2 clients must have strict-redirect-uri mode enabled.".into()), + Self::MG0008SkipUpgradeAttempted => Some("Skip Upgrade Attempted.".into()), Self::PL0001GidOverlapsSystemRange => None, + Self::SC0001IncomingSshPublicKey => None, Self::UI0001ChallengeSerialisation => Some("The WebAuthn challenge was unable to be serialised.".into()), Self::UI0002InvalidState => Some("The credential update process returned an invalid state transition.".into()), + Self::VL0001ValueSshPublicKeyString => None, + Self::VS0001IncomingReplSshPublicKey => None, + Self::VS0002CertificatePublicKeyDigest | + Self::VS0003CertificateDerDecode => Some("Decoding the stored certificate from DER failed.".into()), + Self::VS0004CertificatePublicKeyDigest | + Self::VS0005CertificatePublicKeyDigest => Some("The certificates public key is unabled to be digested.".into()), } } } diff --git a/unix_integration/common/Cargo.toml b/unix_integration/common/Cargo.toml index 5c6a5ff0a..ceab00183 100644 --- a/unix_integration/common/Cargo.toml +++ b/unix_integration/common/Cargo.toml @@ -25,14 +25,15 @@ doctest = false bytes = { workspace = true } csv = { workspace = true } futures = { workspace = true } +kanidm_proto = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } serde_with = { workspace = true } -toml = { workspace = true } -tokio = { workspace = true, features = ["time","net","macros"] } -tokio-util = { workspace = true, features = ["codec"] } -tracing = { workspace = true } sha-crypt = { workspace = true } +tokio = { workspace = true, features = ["time", "net", "macros"] } +tokio-util = { workspace = true, features = ["codec"] } +toml = { workspace = true } +tracing = { workspace = true } [build-dependencies] kanidm_build_profiles = { workspace = true } diff --git a/unix_integration/common/src/unix_proto.rs b/unix_integration/common/src/unix_proto.rs index 8ccbcd75c..1f8dbd779 100644 --- a/unix_integration/common/src/unix_proto.rs +++ b/unix_integration/common/src/unix_proto.rs @@ -1,4 +1,5 @@ use crate::unix_passwd::{EtcGroup, EtcUser}; +use kanidm_proto::internal::OperationError; use serde::{Deserialize, Serialize}; #[derive(Serialize, Deserialize, Debug)] @@ -177,7 +178,7 @@ pub enum ClientResponse { ProviderStatus(Vec), Ok, - Error, + Error(OperationError), } impl From for ClientResponse { diff --git a/unix_integration/pam_kanidm/src/core.rs b/unix_integration/pam_kanidm/src/core.rs index b30331363..cb5954ceb 100644 --- a/unix_integration/pam_kanidm/src/core.rs +++ b/unix_integration/pam_kanidm/src/core.rs @@ -298,8 +298,11 @@ pub fn sm_authenticate_connected( continue; } + ClientResponse::Error(err) => { + error!("Error from kanidm-unixd: {}", err); + return PamResultCode::PAM_AUTH_ERR; + } ClientResponse::Ok - | ClientResponse::Error | ClientResponse::SshKeys(_) | ClientResponse::NssAccounts(_) | ClientResponse::NssAccount(_) diff --git a/unix_integration/resolver/src/bin/kanidm-unix.rs b/unix_integration/resolver/src/bin/kanidm-unix.rs index 690eb7285..145dd2498 100644 --- a/unix_integration/resolver/src/bin/kanidm-unix.rs +++ b/unix_integration/resolver/src/bin/kanidm-unix.rs @@ -103,6 +103,10 @@ async fn main() -> ExitCode { }); continue; } + ClientResponse::Error(err) => { + error!("Error from kanidm-unixd: {}", err); + break; + } ClientResponse::PamAuthenticateStepResponse(_) | ClientResponse::SshKeys(_) | ClientResponse::NssAccounts(_) @@ -111,7 +115,6 @@ async fn main() -> ExitCode { | ClientResponse::NssGroups(_) | ClientResponse::ProviderStatus(_) | ClientResponse::Ok - | ClientResponse::Error | ClientResponse::PamStatus(_) => { // unexpected response. error!("Error: unexpected response -> {:?}", r); diff --git a/unix_integration/resolver/src/bin/kanidm_unixd.rs b/unix_integration/resolver/src/bin/kanidm_unixd.rs index 247fbb525..da3889704 100644 --- a/unix_integration/resolver/src/bin/kanidm_unixd.rs +++ b/unix_integration/resolver/src/bin/kanidm_unixd.rs @@ -26,6 +26,7 @@ use clap::{Arg, ArgAction, Command}; use futures::{SinkExt, StreamExt}; use kanidm_client::KanidmClientBuilder; use kanidm_proto::constants::DEFAULT_CLIENT_CONFIG_PATH; +use kanidm_proto::internal::OperationError; use kanidm_unix_common::constants::DEFAULT_CONFIG_PATH; use kanidm_unix_common::unix_passwd::{parse_etc_group, parse_etc_passwd, parse_etc_shadow}; use kanidm_unix_common::unix_proto::{ClientRequest, ClientResponse, TaskRequest, TaskResponse}; @@ -281,7 +282,7 @@ async fn handle_client( warn!("Attempt to init auth session while current session is active"); // Clean the former session, something is wrong. pam_auth_session_state = None; - ClientResponse::Error + ClientResponse::Error(OperationError::KU001InitWhileSessionActive) } None => { let current_time = OffsetDateTime::now_utc(); @@ -299,7 +300,7 @@ async fn handle_client( pam_auth_session_state = Some(auth_session); pam_auth_response.into() } - Err(_) => ClientResponse::Error, + Err(_) => ClientResponse::Error(OperationError::KU004PamInitFailed), } } } @@ -309,17 +310,19 @@ async fn handle_client( .pam_account_authenticate_step(auth_session, pam_next_req) .await .map(|pam_auth_response| pam_auth_response.into()) - .unwrap_or(ClientResponse::Error), + .unwrap_or(ClientResponse::Error(OperationError::KU003PamAuthFailed)), None => { warn!("Attempt to continue auth session while current session is inactive"); - ClientResponse::Error + ClientResponse::Error(OperationError::KU002ContinueWhileSessionInActive) } }, ClientRequest::PamAccountAllowed(account_id) => cachelayer .pam_account_allowed(account_id.as_str()) .await .map(ClientResponse::PamStatus) - .unwrap_or(ClientResponse::Error), + .unwrap_or(ClientResponse::Error( + OperationError::KU005ErrorCheckingAccount, + )), ClientRequest::PamAccountBeginSession(account_id) => { match cachelayer .pam_account_beginsession(account_id.as_str()) @@ -349,13 +352,13 @@ async fn handle_client( } _ => { // Timeout or other error. - ClientResponse::Error + ClientResponse::Error(OperationError::KG001TaskTimeout) } } } Err(_) => { // We could not submit the req. Move on! - ClientResponse::Error + ClientResponse::Error(OperationError::KG002TaskCommFailure) } } } @@ -363,24 +366,24 @@ async fn handle_client( // The session can begin, but we do not need to create the home dir. ClientResponse::Ok } - _ => ClientResponse::Error, + Err(_) => ClientResponse::Error(OperationError::KU005ErrorCheckingAccount), } } ClientRequest::InvalidateCache => cachelayer .invalidate() .await .map(|_| ClientResponse::Ok) - .unwrap_or(ClientResponse::Error), + .unwrap_or(ClientResponse::Error(OperationError::KG003CacheClearFailed)), ClientRequest::ClearCache => { if ucred.uid() == 0 { cachelayer .clear_cache() .await .map(|_| ClientResponse::Ok) - .unwrap_or(ClientResponse::Error) + .unwrap_or(ClientResponse::Error(OperationError::KG003CacheClearFailed)) } else { - error!("Only root may clear the cache"); - ClientResponse::Error + error!("{}", OperationError::KU006OnlyRootAllowed); + ClientResponse::Error(OperationError::KU006OnlyRootAllowed) } } ClientRequest::Status => {