From ea4d755d7bf62bc11846dcdd2643721fa647f9c2 Mon Sep 17 00:00:00 2001 From: James Hodgkinson Date: Mon, 31 Jul 2023 10:49:59 +1000 Subject: [PATCH] chasing weirdness (#1910) * security headers, fixing error on empty username, handling login without SPN better * making deno happy * cleaning up windows build --- .github/workflows/windows_build.yml | 11 +--- book/src/DEVELOPER_README.md | 12 ++-- .../core/src/https/middleware/csp_headers.rs | 21 ------- server/core/src/https/middleware/mod.rs | 2 +- .../src/https/middleware/security_headers.rs | 58 +++++++++++++++++++ server/core/src/https/mod.rs | 2 +- server/core/src/https/v1.rs | 2 - server/lib/src/idm/event.rs | 28 ++++++--- tools/cli/src/cli/common.rs | 41 ++++++++++--- 9 files changed, 120 insertions(+), 57 deletions(-) delete mode 100644 server/core/src/https/middleware/csp_headers.rs create mode 100644 server/core/src/https/middleware/security_headers.rs diff --git a/.github/workflows/windows_build.yml b/.github/workflows/windows_build.yml index 41a5c8e87..78b1000b6 100644 --- a/.github/workflows/windows_build.yml +++ b/.github/workflows/windows_build.yml @@ -1,18 +1,13 @@ --- name: Windows Build and Test -# at the moment this only builds the kanidm client -# because @yaleman got tired but it's enough to prove -# it builds and be able to administer Kanidm from -# Windows-land - # Trigger the workflow on push to master or pull request "on": push: branches: - master pull_request: - + workflow_dispatch: # so you can run it manually env: SCCACHE_GHA_ENABLED: "true" RUSTC_WRAPPER: "sccache" @@ -36,6 +31,6 @@ jobs: uses: mozilla-actions/sccache-action@v0.0.3 with: version: "v0.4.2" - - run: cargo build -p kanidm_client -p kanidm_tools -p orca -p daemon + - run: cargo build -p kanidm_client -p kanidm_tools --bin kanidm # yamllint disable-line rule:line-length - - run: cargo test -p kanidm_client -p kanidm_tools -p orca -p daemon -p kanidmd_core + - run: cargo test -p kanidm_client -p kanidm_tools diff --git a/book/src/DEVELOPER_README.md b/book/src/DEVELOPER_README.md index aed2627cf..21dca8927 100644 --- a/book/src/DEVELOPER_README.md +++ b/book/src/DEVELOPER_README.md @@ -107,16 +107,16 @@ This is how it works in the automated build: 1. Enable use of installed packages for the user system-wide: -```bash -vcpkg integrate install -``` + ```bash + vcpkg integrate install + ``` 2. Install the openssl dependency, which compiles it from source. This downloads all sorts of dependencies, including perl for the build. -```bash -vcpkg install openssl:x64-windows-static-md -``` + ```bash + vcpkg install openssl:x64-windows-static-md + ``` There's a powershell script in the root directory of the repository which, in concert with `openssl` will generate a config file and certs for testing. diff --git a/server/core/src/https/middleware/csp_headers.rs b/server/core/src/https/middleware/csp_headers.rs deleted file mode 100644 index e575b4fbe..000000000 --- a/server/core/src/https/middleware/csp_headers.rs +++ /dev/null @@ -1,21 +0,0 @@ -use axum::extract::State; -use axum::http::Request; -use axum::middleware::Next; -use axum::response::Response; - -use crate::https::ServerState; - -pub async fn cspheaders_layer( - State(state): State, - request: Request, - next: Next, -) -> Response { - // wait for the middleware to come back - let mut response = next.run(request).await; - - // add the header - let headers = response.headers_mut(); - headers.insert("Content-Security-Policy", state.csp_header); - - response -} diff --git a/server/core/src/https/middleware/mod.rs b/server/core/src/https/middleware/mod.rs index 8e3b2d7e1..ac8c186b5 100644 --- a/server/core/src/https/middleware/mod.rs +++ b/server/core/src/https/middleware/mod.rs @@ -10,7 +10,7 @@ use uuid::Uuid; pub(crate) mod caching; pub(crate) mod compression; -pub(crate) mod csp_headers; +pub(crate) mod security_headers; pub(crate) mod hsts_header; // the version middleware injects diff --git a/server/core/src/https/middleware/security_headers.rs b/server/core/src/https/middleware/security_headers.rs new file mode 100644 index 000000000..bec908962 --- /dev/null +++ b/server/core/src/https/middleware/security_headers.rs @@ -0,0 +1,58 @@ +use axum::extract::State; +use axum::http::Request; +use axum::middleware::Next; +use axum::response::Response; +use http::header::X_CONTENT_TYPE_OPTIONS; +use http::HeaderValue; + +use crate::https::ServerState; + +const PERMISSIONS_POLICY_VALUE: &str = "fullscreen=(), geolocation=()"; +const X_CONTENT_TYPE_OPTIONS_VALUE: &str = "nosniff"; + +pub async fn security_headers_layer( + State(state): State, + request: Request, + next: Next, +) -> Response { + // wait for the middleware to come back + let mut response = next.run(request).await; + + // add the Content-Security-Policy header, which defines how contact will be accessed/run based on the source URL + let headers = response.headers_mut(); + headers.insert(http::header::CONTENT_SECURITY_POLICY, state.csp_header); + + // X-Content-Type-Options tells the browser if it's OK to "sniff" or guess the content type of a response + // + // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Content-Type-Options + // https://scotthelme.co.uk/hardening-your-http-response-headers/#x-content-type-options + #[allow(clippy::expect_used)] + headers.insert( + X_CONTENT_TYPE_OPTIONS, + HeaderValue::from_str(X_CONTENT_TYPE_OPTIONS_VALUE) + .expect("Failed to generate security header X-Content-Type-Options"), + ); + + // Permissions policy defines access to platform services like geolocation, fullscreen etc. + // + // https://www.w3.org/TR/permissions-policy-1/ + #[allow(clippy::expect_used)] + headers.insert( + "Permissions-Policy", + HeaderValue::from_str(PERMISSIONS_POLICY_VALUE) + .expect("Failed to generate security header Permissions-Policy"), + ); + + // Don't send a referrer header when the user is navigating to a non-HTTPS URL + // Ref: + // https://scotthelme.co.uk/a-new-security-header-referrer-policy/ + // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Referrer-Policy + #[allow(clippy::expect_used)] + headers.insert( + http::header::REFERRER_POLICY, + HeaderValue::from_str("no-referrer-when-downgrade") + .expect("Failed to generate Referer-Policy header"), + ); + + response +} diff --git a/server/core/src/https/mod.rs b/server/core/src/https/mod.rs index b0f68abd5..dc921ea87 100644 --- a/server/core/src/https/mod.rs +++ b/server/core/src/https/mod.rs @@ -234,7 +234,7 @@ pub async fn create_https_server( .merge(static_routes) .layer(from_fn_with_state( state.clone(), - middleware::csp_headers::cspheaders_layer, + middleware::security_headers::security_headers_layer, )) .layer(from_fn(middleware::version_middleware)) .layer(from_fn( diff --git a/server/core/src/https/v1.rs b/server/core/src/https/v1.rs index 41f077f21..e987cd8a6 100644 --- a/server/core/src/https/v1.rs +++ b/server/core/src/https/v1.rs @@ -1261,9 +1261,7 @@ pub async fn auth( .qe_r_ref .handle_auth(maybe_sessionid, obj, kopid.eventid, ip_addr) .await; - debug!("Auth result: {:?}", inter); - auth_session_state_management(state, inter) } diff --git a/server/lib/src/idm/event.rs b/server/lib/src/idm/event.rs index eb1fd160b..3a6bff443 100644 --- a/server/lib/src/idm/event.rs +++ b/server/lib/src/idm/event.rs @@ -302,17 +302,27 @@ pub enum AuthEventStep { impl AuthEventStep { fn from_authstep(aus: AuthStep, sid: Option) -> Result { match aus { - AuthStep::Init(username) => Ok(AuthEventStep::Init(AuthEventStepInit { - username, - issue: AuthIssueSession::Token, - })), + AuthStep::Init(username) => { + if username.trim().is_empty() { + Err(OperationError::EmptyRequest) + } else { + Ok(AuthEventStep::Init(AuthEventStepInit { + username, + issue: AuthIssueSession::Token, + })) + } + } AuthStep::Init2 { username, issue } => { - Ok(AuthEventStep::Init(AuthEventStepInit { username, issue })) + if username.trim().is_empty() { + Err(OperationError::EmptyRequest) + } else { + Ok(AuthEventStep::Init(AuthEventStepInit { username, issue })) + } } AuthStep::Begin(mech) => match sid { - Some(ssid) => Ok(AuthEventStep::Begin(AuthEventStepMech { - sessionid: ssid, + Some(sessionid) => Ok(AuthEventStep::Begin(AuthEventStepMech { + sessionid, mech, })), None => Err(OperationError::InvalidAuthState( @@ -320,8 +330,8 @@ impl AuthEventStep { )), }, AuthStep::Cred(cred) => match sid { - Some(ssid) => Ok(AuthEventStep::Cred(AuthEventStepCred { - sessionid: ssid, + Some(sessionid) => Ok(AuthEventStep::Cred(AuthEventStepCred { + sessionid, cred, })), None => Err(OperationError::InvalidAuthState( diff --git a/tools/cli/src/cli/common.rs b/tools/cli/src/cli/common.rs index 4227b544f..53da90eb2 100644 --- a/tools/cli/src/cli/common.rs +++ b/tools/cli/src/cli/common.rs @@ -110,20 +110,43 @@ impl CommonOpt { .get(filter_username) .map(|t| (filter_username.clone(), t.clone())) } else { - let filter_username = format!("{}@", filter_username); - // First, filter for tokens that match. + // first we try to find user@hostname + let filter_username_with_hostname = format!( + "{}@{}", + filter_username, + client.get_origin().host_str().unwrap_or("localhost") + ); + debug!( + "Looking for tokens matching {}", + filter_username_with_hostname + ); + let mut token_refs: Vec<_> = tokens .iter() - .filter(|(t, _)| t.starts_with(&filter_username)) + .filter(|(t, _)| *t == &filter_username_with_hostname) .map(|(k, v)| (k.clone(), v.clone())) .collect(); - match token_refs.len() { - 0 => None, - 1 => token_refs.pop(), - _ => { - error!("Multiple authentication tokens found for {}. Please specify the full spn to proceed", filter_username); - return Err(ToClientError::Other); + if token_refs.len() == 1 { + // return the token + token_refs.pop() + } else { + // otherwise let's try the fallback + let filter_username = format!("{}@", filter_username); + // Filter for tokens that match the pattern + let mut token_refs: Vec<_> = tokens + .into_iter() + .filter(|(t, _)| t.starts_with(&filter_username)) + .map(|(k, v)| (k, v)) + .collect(); + + match token_refs.len() { + 0 => None, + 1 => token_refs.pop(), + _ => { + error!("Multiple authentication tokens found for {}. Please specify the full spn to proceed", filter_username); + return Err(ToClientError::Other); + } } } };