diff --git a/Cargo.toml b/Cargo.toml index b73ef89..1aca7cd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,6 +28,7 @@ base64 = "0.23" clap = { version = "4.6", features = ["derive"] } hex = "0.4" hmac = "0.13" +jsonwebtoken = { version = "11", features = ["rust_crypto"] } reqwest = { version = "0.13", default-features = false, features = ["blocking", "json", "native-tls"] } rsa = { version = "0.9", features = ["sha2"] } rustls = { version = "0.23", default-features = false, features = ["aws-lc-rs"] } @@ -43,9 +44,6 @@ tracing = "0.1" tracing-subscriber = { version = "0.3", features = ["env-filter", "json"] } wildmatch = "2.4" -[dev-dependencies] -jsonwebtoken = { version = "11", features = ["rust_crypto"] } - [[example]] name = "kms_sign_jwt" path = "examples/kms_sign_jwt.rs" diff --git a/README.md b/README.md index 49072a6..5d8171f 100644 --- a/README.md +++ b/README.md @@ -138,6 +138,48 @@ curl -X GET \ http://localhost:8080/proxy/deployments/repos/github_user/repo_name/contents/README.md ``` +### The human token path + +A person authenticated by Teleport, holding an approved access request, can obtain a token for one +exact repository: + +```sh +curl -X POST \ + -H "Authorization: Bearer $TELEPORT_JWT" \ + https://idcat.example.invalid/human/installation-token/source-reader/myorg/pilot +``` + +This route is separate from `/installation-token/...` in every respect that matters: + +| | `/installation-token/...` | `/human/installation-token/...` | +| --- | --- | --- | +| Validated by | `authzoo`, against `[[role]]` | idcat's own validator, against `[[human-role]]` | +| Authorized by | `allowed-roles` or `[[installation-policy]]` | `[[human-policy]]` only | +| Repository | may be a glob | exactly one, no wildcards | +| Permissions | optional | required | +| On no match | falls back to `allowed-roles` | refused | +| `--disable-auth` | honoured | not honoured | +| Token cache | shared | partitioned by validated subject and access request | + +A request that does not match exactly one `[[human-policy]]` is refused before the token is +validated and before GitHub is contacted. A workload token cannot satisfy a human policy, and a +human token cannot satisfy a workload one. + +Each allow and deny is recorded at `info` as a structured event carrying the subject, the Teleport +request, the App, the exact repository, the exact permissions, the expiry and a truncated digest of +the token. The token itself, the Authorization header and the App private key are never logged. + +This route must be reachable only through the Teleport Application Service; it is not safe to +expose to the internet. Two things are worth stating plainly to whoever operates it: + +* A short Teleport approval window stops new sessions from starting, and an installation token + expires within the hour. Neither removes source that has already been cloned to a machine, nor + the history that came with it. +* idcat opts token requests into GitHub's stateless (JWT-format) installation tokens, which are + verified from their own signature. Whether revoking a token, or suspending the App installation, + affects one that has already been issued is worth confirming against GitHub's documentation + before relying on either as an emergency stop. + ## Webhooks idcat can receive GitHub webhook callbacks and bridge them into NATS. GitHub diff --git a/examples/human_route_load_test.rs b/examples/human_route_load_test.rs new file mode 100644 index 0000000..9e1dba6 --- /dev/null +++ b/examples/human_route_load_test.rs @@ -0,0 +1,252 @@ +// SPDX-License-Identifier: MIT +// SPDX-FileCopyrightText: The idcat contributors + +//! Bounded invalid-token load test for the human installation-token route. +//! +//! # Why this exists +//! +//! `authzoo` validates a token once per configured role, and the OpenID discovery request it makes +//! when a role has no `validation-key` is not cached — only the resulting JWKS key is. An invalid +//! token can therefore cost several blocking outbound requests on a worker thread. The human route +//! avoids that (it selects one issuer from configuration and parses once, against a cached JWKS), +//! but the *existing* workload consumers still run through `authzoo`, and they share a runtime with +//! the new route. This measures whether load on the new route degrades them. +//! +//! # This will not run without owner-supplied values +//! +//! The request ceiling, permitted latency change, duration and environment are not guessable, and +//! guessing them would either make the test meaningless or turn it into an outage. Every one of +//! them is a required argument, and the target is refused if it looks like production. +//! +//! ```text +//! cargo run --example human_route_load_test -- \ +//! --environment \ +//! --human-url https:///human/installation-token/// \ +//! --workload-url https:///installation-token/// \ +//! --workload-token-file \ +//! --requests-per-second \ +//! --duration-seconds \ +//! --max-latency-increase-ms \ +//! --i-am-the-named-owner +//! ``` +//! +//! Exits non-zero if the measured latency increase exceeds the supplied threshold, which is the +//! signal to complete the wider validator repair before the pilot goes live. + +use clap::Parser; +use std::sync::Arc; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::time::{Duration, Instant}; + +/// Environment names that must never be a load-test target, however the flags are set. +const FORBIDDEN_ENVIRONMENT_MARKERS: [&str; 4] = ["prod", "live", "production", "public"]; + +#[derive(Debug, Parser)] +#[command( + about = "Bounded invalid-token load test for the idcat human token route", + long_about = None +)] +struct Cli { + /// The non-production environment named by the owner who authorised this test. + #[arg(long)] + environment: String, + + /// The human installation-token route to load with invalid tokens. + #[arg(long)] + human_url: String, + + /// An existing workload installation-token route, whose latency is the thing being protected. + #[arg(long)] + workload_url: String, + + /// File holding a non-production workload token. Passed as a path, never on the command line, + /// so the token cannot appear in process arguments or shell history. + #[arg(long)] + workload_token_file: std::path::PathBuf, + + /// The request ceiling supplied by the owner. There is no default. + #[arg(long)] + requests_per_second: u32, + + /// How long to sustain the load, supplied by the owner. There is no default. + #[arg(long)] + duration_seconds: u64, + + /// The largest acceptable increase in workload p95 latency, supplied by the owner. + #[arg(long)] + max_latency_increase_ms: u64, + + /// Affirms that the caller is the named owner permitted to run this. + #[arg(long)] + i_am_the_named_owner: bool, +} + +#[tokio::main] +async fn main() -> anyhow::Result<()> { + let cli = Cli::parse(); + guard(&cli)?; + + let workload_token = std::fs::read_to_string(&cli.workload_token_file)? + .trim() + .to_string(); + if workload_token.is_empty() { + anyhow::bail!("workload token file is empty"); + } + + let client = reqwest::Client::builder() + .timeout(Duration::from_secs(10)) + .build()?; + + println!("measuring workload latency with no additional load"); + let baseline = measure_workload(&client, &cli.workload_url, &workload_token, 20).await; + report("baseline", &baseline); + + println!( + "applying {} invalid requests/second to the human route for {}s", + cli.requests_per_second, cli.duration_seconds + ); + let sent = Arc::new(AtomicU64::new(0)); + let load = tokio::spawn(apply_load( + client.clone(), + cli.human_url.clone(), + cli.requests_per_second, + Duration::from_secs(cli.duration_seconds), + Arc::clone(&sent), + )); + + let under_load = measure_workload(&client, &cli.workload_url, &workload_token, 20).await; + load.await??; + report("under load", &under_load); + println!("invalid requests sent: {}", sent.load(Ordering::Relaxed)); + + let increase = under_load.p95.saturating_sub(baseline.p95); + println!( + "workload p95 increased by {}ms (permitted: {}ms)", + increase.as_millis(), + cli.max_latency_increase_ms + ); + if increase > Duration::from_millis(cli.max_latency_increase_ms) { + anyhow::bail!( + "workload p95 latency increase exceeded the permitted change; complete the wider \ + validator repair before the pilot goes live" + ); + } + println!("within the permitted latency change"); + Ok(()) +} + +/// Refuses to run unless the owner has supplied every bound and the target is non-production. +fn guard(cli: &Cli) -> anyhow::Result<()> { + if !cli.i_am_the_named_owner { + anyhow::bail!( + "this test may only be run by the named owner; pass --i-am-the-named-owner to affirm that" + ); + } + if cli.requests_per_second == 0 || cli.duration_seconds == 0 { + anyhow::bail!("requests-per-second and duration-seconds must be greater than zero"); + } + let environment = cli.environment.to_ascii_lowercase(); + for marker in FORBIDDEN_ENVIRONMENT_MARKERS { + if environment.contains(marker) { + anyhow::bail!( + "environment '{}' looks like production; this test runs only in the named \ + non-production environment", + cli.environment + ); + } + for url in [&cli.human_url, &cli.workload_url] { + if url.to_ascii_lowercase().contains(marker) { + anyhow::bail!( + "target '{url}' looks like production; this test runs only in the named \ + non-production environment" + ); + } + } + } + Ok(()) +} + +struct Latencies { + p50: Duration, + p95: Duration, + p99: Duration, + errors: usize, +} + +fn report(label: &str, latencies: &Latencies) { + println!( + "{label}: p50={}ms p95={}ms p99={}ms errors={}", + latencies.p50.as_millis(), + latencies.p95.as_millis(), + latencies.p99.as_millis(), + latencies.errors + ); +} + +async fn measure_workload( + client: &reqwest::Client, + url: &str, + token: &str, + samples: usize, +) -> Latencies { + let mut observed = Vec::with_capacity(samples); + let mut errors = 0; + for _ in 0..samples { + let started = Instant::now(); + match client.post(url).bearer_auth(token).send().await { + Ok(response) if response.status().is_success() => observed.push(started.elapsed()), + Ok(_) | Err(_) => errors += 1, + } + } + observed.sort_unstable(); + Latencies { + p50: percentile(&observed, 50), + p95: percentile(&observed, 95), + p99: percentile(&observed, 99), + errors, + } +} + +fn percentile(sorted: &[Duration], percentile: usize) -> Duration { + if sorted.is_empty() { + return Duration::ZERO; + } + let index = (sorted.len() * percentile / 100).min(sorted.len() - 1); + sorted[index] +} + +/// Sends structurally valid but cryptographically worthless tokens. Nothing here is a credential. +async fn apply_load( + client: reqwest::Client, + url: String, + requests_per_second: u32, + duration: Duration, + sent: Arc, +) -> anyhow::Result<()> { + let deadline = Instant::now() + duration; + let interval = Duration::from_secs_f64(1.0 / f64::from(requests_per_second)); + let token = synthetic_invalid_token(); + while Instant::now() < deadline { + let client = client.clone(); + let url = url.clone(); + let token = token.clone(); + let sent = Arc::clone(&sent); + tokio::spawn(async move { + let _ = client.post(&url).bearer_auth(&token).send().await; + sent.fetch_add(1, Ordering::Relaxed); + }); + tokio::time::sleep(interval).await; + } + Ok(()) +} + +/// A well-formed JWT whose signature is meaningless, so it exercises the full validation path and +/// is rejected. It carries no real claims and is not a credential. +fn synthetic_invalid_token() -> String { + use base64::{Engine, engine::general_purpose::URL_SAFE_NO_PAD}; + let header = URL_SAFE_NO_PAD.encode(br#"{"alg":"RS256","typ":"JWT","kid":"load-test"}"#); + let claims = URL_SAFE_NO_PAD + .encode(br#"{"sub":"load-test","iss":"load-test","aud":"load-test","exp":9999999999}"#); + let signature = URL_SAFE_NO_PAD.encode([0u8; 256]); + format!("{header}.{claims}.{signature}") +} diff --git a/git-credential-helper/README.md b/git-credential-helper/README.md index 8df8ef0..49aa4d1 100644 --- a/git-credential-helper/README.md +++ b/git-credential-helper/README.md @@ -46,21 +46,71 @@ token-path = "/var/run/secrets/tokens/idcat" `github-app` selects the GitHub App configured in idcat. `idcat-endpoint` is the base URL of the idcat service. -Exactly one token source must be configured: +Exactly one token source must be configured. + +Read the token from a file: ```toml token-path = "/var/run/secrets/tokens/idcat" ``` -or: +Or run a program and use its standard output: ```toml -token-command = "kubectl create token idcat-client" +token-executable = "/usr/bin/kubectl" +token-args = ["create", "token", "idcat-client"] ``` `token-path` reads a bearer token accepted by idcat from the filesystem, such as a mounted -Kubernetes service account token. `token-command` invokes a shell command and uses its standard -output as the bearer token. +Kubernetes service account token. `token-executable` runs a fixed executable with an explicit +argument array. No shell is involved, so nothing in the configuration is word-split, globbed or +substituted, and each argument reaches the program exactly as written. + +### Migrating from `token-command` (deprecated) + +`token-command` ran its value through `/bin/sh -c`. It still works and existing configuration keeps +working unchanged, but it logs a deprecation warning at startup and will be removed. Rewrite it as +an executable and an argument array: + +```toml +# Before +token-command = "kubectl create token idcat-client" + +# After +token-executable = "/usr/bin/kubectl" +token-args = ["create", "token", "idcat-client"] +``` + +Setting more than one of `token-path`, `token-executable` and `token-command` is an error, as is +setting `token-args` without `token-executable`. + +### Restricting the helper to one repository + +```toml +repository = "myorg/pilot" +``` + +When `repository` is set, the helper produces a credential for that repository and nothing else. +For any other repository it prints nothing, so Git receives no credential from it. Owner and +repository names are compared case-insensitively, as GitHub treats them. When `repository` is +unset the helper answers for any GitHub repository, which is the pre-existing behaviour; a +deployment that mints tokens for a *person* should set it. + +Scope Git's own configuration to match, so the helper is offered only the requests it is for, and +so no other helper is consulted for them: + +```sh +# Match credentials by full path, not just by host, so this block applies to one repository. +git config --global credential.useHttpPath true + +# Clear any inherited helpers for this repository, then add only this one. +git config --global --unset-all credential.https://github.com/myorg/pilot.helper +git config --global --add credential.https://github.com/myorg/pilot.helper "" +git config --global --add credential.https://github.com/myorg/pilot.helper idcat +``` + +The empty first value clears helpers inherited from broader scopes, so a global `store` or +`osxkeychain` helper is neither consulted for this repository nor offered the token. When Git accesses `https://github.com/OWNER/REPO.git`, the helper calls: @@ -71,6 +121,13 @@ POST {idcat-endpoint}/installation-token/{github-app}/OWNER/REPO with the configured token as the bearer token. The response body is returned to Git as the password, with `x-access-token` as the username. +## What the helper does not do + +`store` and `erase` are accepted and do nothing: the helper holds no state, and a token it returned +is not written anywhere. The token is never passed as a command-line argument, so it cannot appear +in process listings or shell history, and it is never logged — a failing token source is reported +by exit status alone, without echoing what it printed. + ## License MIT diff --git a/git-credential-helper/src/config.rs b/git-credential-helper/src/config.rs index 1b6d19e..a1b18e2 100644 --- a/git-credential-helper/src/config.rs +++ b/git-credential-helper/src/config.rs @@ -3,27 +3,51 @@ use serde::Deserialize; use std::env; use std::io::ErrorKind; use std::path::PathBuf; +use tracing::warn; #[derive(Debug, Deserialize)] #[serde(rename_all = "kebab-case")] struct RawConfig { github_app: String, idcat_endpoint: String, + /// The one repository this helper will produce a credential for, as `owner/name`. When unset + /// the helper answers for any GitHub repository, which is the pre-existing behaviour; a + /// deployment that mints tokens for a person must set it. + repository: Option, token_path: Option, + /// Deprecated. Runs through `/bin/sh -c`, which puts the command — and anything it + /// interpolates — through a shell. Replaced by `token-executable` and `token-args`. token_command: Option, + token_executable: Option, + #[serde(default)] + token_args: Vec, } #[derive(Debug)] pub struct Config { pub github_app: String, pub idcat_endpoint: String, + pub repository: Option, pub token_source: TokenSource, } +#[derive(Debug, Eq, PartialEq)] +pub struct Repository { + pub owner: String, + pub name: String, +} + #[derive(Debug)] pub enum TokenSource { Path(PathBuf), - Command(String), + /// A fixed executable invoked with an explicit argument array. No shell is involved, so + /// nothing in the configuration is subject to word splitting, globbing or substitution. + Program { + executable: PathBuf, + args: Vec, + }, + /// Deprecated shell form, accepted during the migration window. + ShellCommand(String), } impl Config { @@ -51,23 +75,87 @@ impl TryFrom for Config { type Error = anyhow::Error; fn try_from(config: RawConfig) -> Result { - let token_source = match (config.token_path, config.token_command) { - (Some(path), None) => TokenSource::Path(path), - (None, Some(command)) => TokenSource::Command(command), - (None, None) => bail!("configuration must set either token-path or token-command"), - (Some(_), Some(_)) => { - bail!("configuration must not set both token-path and token-command") + let sources_set = [ + config.token_path.is_some(), + config.token_command.is_some(), + config.token_executable.is_some(), + ] + .into_iter() + .filter(|set| *set) + .count(); + if sources_set == 0 { + bail!("configuration must set one of token-path, token-executable or token-command"); + } + if sources_set > 1 { + bail!( + "configuration must set only one of token-path, token-executable or token-command" + ); + } + if !config.token_args.is_empty() && config.token_executable.is_none() { + bail!("token-args requires token-executable"); + } + + let token_source = match ( + config.token_path, + config.token_executable, + config.token_command, + ) { + (Some(path), None, None) => TokenSource::Path(path), + (None, Some(executable), None) => { + if executable.as_os_str().is_empty() { + bail!("token-executable must not be empty"); + } + TokenSource::Program { + executable, + args: config.token_args, + } + } + (None, None, Some(command)) => { + if command.is_empty() { + bail!("token-command must not be empty"); + } + warn!( + "token-command is deprecated and runs through /bin/sh; \ + replace it with token-executable and token-args" + ); + TokenSource::ShellCommand(command) } + _ => unreachable!("the count check above rejects every other combination"), }; + let repository = config.repository.map(parse_repository).transpose()?; + if repository.is_none() { + warn!( + "no repository is configured; this helper will answer for any GitHub repository. \ + Set repository = \"owner/name\" to scope it to one." + ); + } + Ok(Self { github_app: config.github_app, idcat_endpoint: config.idcat_endpoint, + repository, token_source, }) } } +fn parse_repository(repository: String) -> anyhow::Result { + let Some((owner, name)) = repository.split_once('/') else { + bail!("repository must be in owner/name form, got '{repository}'"); + }; + if owner.is_empty() || name.is_empty() || name.contains('/') { + bail!("repository must be in owner/name form, got '{repository}'"); + } + if repository.contains('*') || repository.contains('?') { + bail!("repository must name one exact repository, got '{repository}'"); + } + Ok(Repository { + owner: owner.to_owned(), + name: name.to_owned(), + }) +} + fn default_config_path() -> anyhow::Result { let home = env::var_os("HOME").ok_or_else(|| anyhow!("HOME is not set"))?; Ok(PathBuf::from(home) @@ -84,8 +172,11 @@ mod tests { RawConfig { github_app: "deployments".to_owned(), idcat_endpoint: "https://idcat.example.test".to_owned(), + repository: None, token_path: None, token_command: None, + token_executable: None, + token_args: Vec::new(), } } @@ -102,15 +193,55 @@ mod tests { } #[test] - fn accepts_token_command() { + fn accepts_token_executable_with_args() { let config = RawConfig { - token_command: Some("cat /var/run/secrets/tokens/idcat".to_owned()), + token_executable: Some("/usr/local/bin/tsh".into()), + token_args: vec![ + "apps".to_owned(), + "config".to_owned(), + "--format=json".to_owned(), + ], ..raw_config() }; let config = Config::try_from(config).expect("config validates"); - assert!(matches!(config.token_source, TokenSource::Command(_))); + match config.token_source { + TokenSource::Program { executable, args } => { + assert_eq!(executable, PathBuf::from("/usr/local/bin/tsh")); + assert_eq!(args, ["apps", "config", "--format=json"]); + } + other => panic!("expected a Program token source, got {other:?}"), + } + } + + #[test] + fn accepts_token_executable_without_args() { + let config = RawConfig { + token_executable: Some("/usr/local/bin/print-token".into()), + ..raw_config() + }; + + let config = Config::try_from(config).expect("config validates"); + + match config.token_source { + TokenSource::Program { args, .. } => assert!(args.is_empty()), + other => panic!("expected a Program token source, got {other:?}"), + } + } + + #[test] + fn still_accepts_the_deprecated_token_command() { + // The migration is backwards compatible: an existing configuration keeps working, with a + // warning, rather than failing at startup. + let config = RawConfig { + token_command: Some("cat /var/run/secrets/tokens/idcat".to_owned()), + ..raw_config() + }; + + let config = Config::try_from(config).expect("the deprecated form must keep working"); + + assert!(matches!(config.token_source, TokenSource::ShellCommand(_))); } #[test] @@ -120,7 +251,8 @@ mod tests { assert!( error .to_string() - .contains("must set either token-path or token-command") + .contains("must set one of token-path, token-executable or token-command"), + "{error}" ); } @@ -134,10 +266,83 @@ mod tests { let error = Config::try_from(config).expect_err("config should be rejected"); + assert!(error.to_string().contains("only one of"), "{error}"); + } + + #[test] + fn rejects_token_executable_combined_with_token_command() { + let config = RawConfig { + token_executable: Some("/usr/local/bin/tsh".into()), + token_command: Some("tsh apps config".to_owned()), + ..raw_config() + }; + + let error = Config::try_from(config).expect_err("config should be rejected"); + + assert!(error.to_string().contains("only one of"), "{error}"); + } + + #[test] + fn rejects_token_args_without_token_executable() { + let config = RawConfig { + token_path: Some("/var/run/secrets/tokens/idcat".into()), + token_args: vec!["--format=json".to_owned()], + ..raw_config() + }; + + let error = Config::try_from(config).expect_err("config should be rejected"); + assert!( error .to_string() - .contains("must not set both token-path and token-command") + .contains("token-args requires token-executable"), + "{error}" ); } + + #[test] + fn accepts_an_exact_repository() { + let config = RawConfig { + repository: Some("myorg/pilot".to_owned()), + token_path: Some("/var/run/secrets/tokens/idcat".into()), + ..raw_config() + }; + + let config = Config::try_from(config).expect("config validates"); + + assert_eq!( + config.repository, + Some(Repository { + owner: "myorg".to_owned(), + name: "pilot".to_owned() + }) + ); + } + + #[test] + fn rejects_a_repository_that_is_not_owner_name() { + for repository in ["pilot", "myorg/", "/pilot", "myorg/pilot/extra"] { + let config = RawConfig { + repository: Some(repository.to_owned()), + token_path: Some("/var/run/secrets/tokens/idcat".into()), + ..raw_config() + }; + + assert!( + Config::try_from(config).is_err(), + "repository '{repository}' must be rejected" + ); + } + } + + #[test] + fn rejects_a_wildcard_repository() { + let config = RawConfig { + repository: Some("myorg/*".to_owned()), + token_path: Some("/var/run/secrets/tokens/idcat".into()), + ..raw_config() + }; + + assert!(Config::try_from(config).is_err()); + } } diff --git a/git-credential-helper/src/idcat.rs b/git-credential-helper/src/idcat.rs index 4122cb5..7435a3b 100644 --- a/git-credential-helper/src/idcat.rs +++ b/git-credential-helper/src/idcat.rs @@ -75,7 +75,8 @@ mod tests { let config = Config { github_app: "deployments".to_owned(), idcat_endpoint: "https://idcat.example.test/base".to_owned(), - token_source: TokenSource::Command("unused".to_owned()), + repository: None, + token_source: TokenSource::ShellCommand("unused".to_owned()), }; let repo = Repo { owner: "noa".to_owned(), diff --git a/git-credential-helper/src/main.rs b/git-credential-helper/src/main.rs index 39af389..ab088a1 100644 --- a/git-credential-helper/src/main.rs +++ b/git-credential-helper/src/main.rs @@ -3,9 +3,9 @@ mod credential; mod idcat; mod token_source; -use crate::config::Config; +use crate::config::{Config, Repository}; use crate::credential::{ - is_github_https_request, read_credential_from_stdin, repo_from_credential, + Repo, is_github_https_request, read_credential_from_stdin, repo_from_credential, }; use crate::idcat::fetch_installation_token; use crate::token_source::read_token; @@ -54,6 +54,17 @@ fn run() -> anyhow::Result<()> { } }; + if !answers_for(config.repository.as_ref(), &repo) { + // Print nothing. Git treats an empty response as "this helper has no credential", moves on + // to whatever else is configured, and never sees a value for this repository. + info!( + requested_owner = %repo.owner, + requested_repo = %repo.name, + "exiting without output because this helper is configured for a different repository" + ); + return Ok(()); + } + info!("obtaining bearer token"); let oidc_token = read_token(&config.token_source)?; info!("bearer token obtained"); @@ -66,6 +77,22 @@ fn run() -> anyhow::Result<()> { Ok(()) } +/// Whether this helper should produce a credential for `requested`. +/// +/// A configured repository is a hard boundary: the helper prints nothing for anything else, so +/// Git receives no credential from it and falls through to whatever else is configured. GitHub +/// owner and repository names are case-insensitive, so the comparison is too — otherwise a clone +/// URL differing only in case would silently get no credential. +fn answers_for(configured: Option<&Repository>, requested: &Repo) -> bool { + match configured { + None => true, + Some(configured) => { + configured.owner.eq_ignore_ascii_case(&requested.owner) + && configured.name.eq_ignore_ascii_case(&requested.name) + } + } +} + #[derive(Debug, Parser)] struct Cli { #[arg(long = "config", short = 'c')] @@ -94,3 +121,67 @@ fn init_logging() { ) .init(); } + +#[cfg(test)] +mod tests { + use super::*; + + fn repo(owner: &str, name: &str) -> Repo { + Repo { + owner: owner.to_owned(), + name: name.to_owned(), + } + } + + fn configured(owner: &str, name: &str) -> Repository { + Repository { + owner: owner.to_owned(), + name: name.to_owned(), + } + } + + #[test] + fn answers_for_the_configured_repository() { + assert!(answers_for( + Some(&configured("myorg", "pilot")), + &repo("myorg", "pilot") + )); + } + + #[test] + fn does_not_answer_for_another_repository_in_the_same_org() { + assert!(!answers_for( + Some(&configured("myorg", "pilot")), + &repo("myorg", "other") + )); + } + + #[test] + fn does_not_answer_for_the_same_repository_name_under_another_owner() { + assert!(!answers_for( + Some(&configured("myorg", "pilot")), + &repo("otherorg", "pilot") + )); + } + + #[test] + fn does_not_answer_for_a_repository_whose_name_merely_starts_the_same() { + assert!(!answers_for( + Some(&configured("myorg", "pilot")), + &repo("myorg", "pilot-internal") + )); + } + + #[test] + fn matches_case_insensitively_as_github_does() { + assert!(answers_for( + Some(&configured("MyOrg", "Pilot")), + &repo("myorg", "pilot") + )); + } + + #[test] + fn answers_for_anything_when_no_repository_is_configured() { + assert!(answers_for(None, &repo("myorg", "anything"))); + } +} diff --git a/git-credential-helper/src/token_source.rs b/git-credential-helper/src/token_source.rs index 65a5212..df7c13e 100644 --- a/git-credential-helper/src/token_source.rs +++ b/git-credential-helper/src/token_source.rs @@ -1,12 +1,14 @@ use crate::config::TokenSource; use anyhow::{Context, bail}; +use std::ffi::OsStr; use std::path::Path; use std::process::Command; pub fn read_token(source: &TokenSource) -> anyhow::Result { match source { TokenSource::Path(path) => read_token_path(path), - TokenSource::Command(command) => run_token_command(command), + TokenSource::Program { executable, args } => run_token_program(executable, args), + TokenSource::ShellCommand(command) => run_token_shell_command(command), } } @@ -16,16 +18,36 @@ fn read_token_path(path: &Path) -> anyhow::Result { trim_token(token, "token-path produced an empty token") } -fn run_token_command(command: &str) -> anyhow::Result { +/// Runs a fixed executable with an explicit argument array. Nothing is passed through a shell, so +/// no part of the configuration is word-split, globbed or substituted. +fn run_token_program(executable: &Path, args: &[String]) -> anyhow::Result { + let output = Command::new(executable) + .args(args.iter().map(OsStr::new)) + .output() + .with_context(|| { + format!( + "failed to execute token-executable: {}", + executable.display() + ) + })?; + token_from_output(output, "token-executable") +} + +fn run_token_shell_command(command: &str) -> anyhow::Result { let output = Command::new("/bin/sh") .arg("-c") .arg(command) .output() .with_context(|| format!("failed to execute token-command: {command}"))?; + token_from_output(output, "token-command") +} +fn token_from_output(output: std::process::Output, source: &str) -> anyhow::Result { if !output.status.success() { + // Deliberately does not include stdout: on some failures a token source still prints a + // usable token before exiting non-zero, and that must not reach a log. bail!( - "token-command exited with status {}", + "{source} exited with status {}", output .status .code() @@ -33,8 +55,9 @@ fn run_token_command(command: &str) -> anyhow::Result { ); } - let token = String::from_utf8(output.stdout).context("token-command output was not UTF-8")?; - trim_token(token, "token-command produced an empty token") + let token = String::from_utf8(output.stdout) + .with_context(|| format!("{source} output was not UTF-8"))?; + trim_token(token, &format!("{source} produced an empty token")) } fn trim_token(token: String, empty_message: &str) -> anyhow::Result { @@ -45,3 +68,86 @@ fn trim_token(token: String, empty_message: &str) -> anyhow::Result { Ok(token.to_owned()) } + +#[cfg(test)] +mod tests { + use super::*; + use std::path::PathBuf; + + #[test] + fn reads_a_token_from_a_program_and_its_argument_array() { + let source = TokenSource::Program { + executable: PathBuf::from("/bin/echo"), + args: vec!["synthetic-token".to_owned()], + }; + + assert_eq!(read_token(&source).unwrap(), "synthetic-token"); + } + + #[test] + fn passes_each_argument_verbatim_without_a_shell() { + // Under `/bin/sh -c` this would be substituted; as an argv element it must arrive intact. + let source = TokenSource::Program { + executable: PathBuf::from("/bin/echo"), + args: vec!["$(echo injected)".to_owned()], + }; + + assert_eq!(read_token(&source).unwrap(), "$(echo injected)"); + } + + #[test] + fn does_not_split_an_argument_on_whitespace() { + let source = TokenSource::Program { + executable: PathBuf::from("/bin/echo"), + args: vec!["-n".to_owned(), "one two".to_owned()], + }; + + assert_eq!(read_token(&source).unwrap(), "one two"); + } + + #[test] + fn rejects_an_empty_program_token() { + let source = TokenSource::Program { + executable: PathBuf::from("/bin/echo"), + args: vec!["-n".to_owned(), String::new()], + }; + + let error = read_token(&source).unwrap_err(); + + assert!(error.to_string().contains("empty token"), "{error}"); + } + + #[test] + fn reports_a_failing_program_without_echoing_its_output() { + let source = TokenSource::Program { + executable: PathBuf::from("/bin/sh"), + args: vec!["-c".to_owned(), "echo ghs_leaked; exit 3".to_owned()], + }; + + let error = read_token(&source).unwrap_err(); + + assert!(error.to_string().contains("status 3"), "{error}"); + assert!( + !error.to_string().contains("ghs_leaked"), + "a failure message must not echo what the token source printed: {error}" + ); + } + + #[test] + fn the_deprecated_shell_command_still_works() { + let source = TokenSource::ShellCommand("echo synthetic-token".to_owned()); + + assert_eq!(read_token(&source).unwrap(), "synthetic-token"); + } + + #[test] + fn reads_a_token_from_a_file() { + let path = std::env::temp_dir().join("idcat-token-source-test"); + std::fs::write(&path, "synthetic-token\n").unwrap(); + + let token = read_token(&TokenSource::Path(path.clone())).unwrap(); + + std::fs::remove_file(&path).ok(); + assert_eq!(token, "synthetic-token"); + } +} diff --git a/idcat.toml.example b/idcat.toml.example index 1c98276..c026008 100644 --- a/idcat.toml.example +++ b/idcat.toml.example @@ -162,3 +162,63 @@ allow-self-access = true [owner-policy.permissions] packages = "read" +# --------------------------------------------------------------------------- +# The human token path +# --------------------------------------------------------------------------- +# +# `[[human-role]]` and `[[human-policy]]` let a *person*, authenticated by +# Teleport and holding an approved access request, obtain a token for one exact +# repository. They are deliberately separate from `[[role]]` and +# `[[installation-policy]]`: +# +# * a workload token can never satisfy a human policy, and vice versa — the +# two are validated by different code against different issuers, and +# neither namespace may name the other's roles; +# * there is no `allowed-roles` fallback. A request that does not match +# exactly one `[[human-policy]]` is refused, not retried more broadly; +# * there are no wildcards. One policy names one repository; +# * `permissions` is required, since it is the only thing keeping the token +# narrower than the whole App installation; +# * `--disable-auth` does not apply. The human route has no unauthenticated +# mode. +# +# Requests arrive at: +# +# POST /human/installation-token/{github-app}/{owner}/{repo} +# +# This route must be reachable only through the Teleport Application Service. +# +# Every value below is an example. Replace them with the real issuer, +# application uri, JWKS URL, Teleport role, App and repository. +# +# [[human-role]] +# name = "pilot-repo-reader" +# # The exact `iss` the token carries. +# issuer = "https://teleport.example.com" +# # The exact `aud`. This is the Teleport APPLICATION URI, which is not +# # necessarily the hostname a person types. +# audience = "https://idcat.teleport.example.com:443" +# # Given explicitly. idcat does not use OpenID discovery for this issuer. +# # Must end in /.well-known/jwks.json. +# jwks-url = "https://teleport.example.com/.well-known/jwks.json" +# # The exact Teleport role string. Membership is tested against the JSON array +# # in `roles-claim` by exact string equality: a role that merely contains this +# # name does not match, and a scalar claim of the same name is refused +# # outright. +# teleport-role = "github-app-token-pilot-repo" +# # Defaults to "roles". +# roles-claim = "roles" +# # Asymmetric algorithms only; an HMAC value will not parse. +# algorithms = ["RS256"] +# # Optional. A scalar claim naming the Teleport access request or session, +# # recorded on issuance so an operator can join the approval to the token. +# request-id-claim = "teleport_request_id" +# +# [[human-policy]] +# github-app = "source-reader" +# # Exactly one repository, in owner/name form. Globs are refused at startup. +# repository = "myorg/pilot" +# human-role = "pilot-repo-reader" +# +# [human-policy.permissions] +# contents = "read" diff --git a/src/audit.rs b/src/audit.rs new file mode 100644 index 0000000..43e57b6 --- /dev/null +++ b/src/audit.rs @@ -0,0 +1,279 @@ +// SPDX-License-Identifier: MIT +// SPDX-FileCopyrightText: The idcat contributors + +//! The person-level issuance record for the human token path. +//! +//! GitHub's own App record shows that *the App* minted a token; it cannot show which person asked +//! for it. These events are the only place that link is made, so they are emitted at `info` — the +//! level a production deployment actually keeps — for denials as well as grants. +//! +//! Nothing here may take a bearer token, an Authorization header, an App private key or a +//! credential-helper payload. The only token-derived value recorded is a truncated digest, which +//! lets an operator correlate two records without being able to reconstruct the token. + +use crate::service::HumanIdentity; +use sha2::{Digest, Sha256}; +use std::collections::BTreeMap; +use tracing::info; + +/// The number of hex characters of the token digest recorded. 16 hex characters (64 bits) is +/// ample to correlate records, and useless for recovering the token. +const FINGERPRINT_LENGTH: usize = 16; + +/// A non-secret, stable identifier for a token. Correlates an idcat issuance record with another +/// record of the same token without disclosing it. +pub fn token_fingerprint(token: &str) -> String { + let digest = Sha256::digest(token.as_bytes()); + hex::encode(digest)[..FINGERPRINT_LENGTH].to_string() +} + +fn permissions_label(permissions: &BTreeMap) -> String { + permissions + .iter() + .map(|(permission, value)| format!("{permission}={value}")) + .collect::>() + .join(",") +} + +/// Records that a person was issued a token for exactly one repository. +pub fn record_issuance( + identity: &HumanIdentity, + github_app: &str, + repository: &str, + permissions: &BTreeMap, + expires_at: &str, + token: &str, +) { + info!( + event = "human_installation_token", + decision = "allow", + subject = %identity.subject, + teleport_request = identity.request_id.as_deref().unwrap_or("unknown"), + human_role = %identity.human_role, + github_app = %github_app, + repository = %repository, + permissions = %permissions_label(permissions), + expires_at = %expires_at, + token_fingerprint = %token_fingerprint(token), + "issued a GitHub App installation token to a person" + ); +} + +/// Records that a person's request was refused. `subject` is absent when the token itself failed +/// validation, since nothing in an unvalidated token may be treated as an identity. +pub fn record_denial(subject: Option<&str>, github_app: &str, repository: &str, reason: &str) { + info!( + event = "human_installation_token", + decision = "deny", + subject = subject.unwrap_or("unauthenticated"), + github_app = %github_app, + repository = %repository, + reason = %reason, + "refused a GitHub App installation token request from a person" + ); +} + +#[cfg(test)] +mod tests { + use super::*; + use std::io::Write; + use std::sync::{Arc, Mutex}; + + /// Captures what the tracing layer actually writes, so the assertions below are about the + /// emitted log line rather than about the arguments we believe we passed. + #[derive(Clone, Default)] + struct CapturedLog(Arc>>); + + impl CapturedLog { + fn contents(&self) -> String { + String::from_utf8(self.0.lock().unwrap().clone()).unwrap() + } + } + + impl Write for CapturedLog { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + self.0.lock().unwrap().extend_from_slice(buf); + Ok(buf.len()) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } + } + + impl<'a> tracing_subscriber::fmt::MakeWriter<'a> for CapturedLog { + type Writer = Self; + + fn make_writer(&'a self) -> Self::Writer { + self.clone() + } + } + + /// Runs `emit` with a subscriber at the level a production deployment keeps, and returns what + /// was logged. + fn capture_at_info(emit: impl FnOnce()) -> String { + use tracing_subscriber::layer::SubscriberExt; + + let captured = CapturedLog::default(); + let subscriber = tracing_subscriber::registry() + .with(tracing_subscriber::filter::LevelFilter::INFO) + .with( + tracing_subscriber::fmt::layer() + .with_writer(captured.clone()) + .with_ansi(false), + ); + tracing::subscriber::with_default(subscriber, emit); + captured.contents() + } + + fn identity() -> HumanIdentity { + HumanIdentity { + subject: "alex.mason@example.invalid".to_string(), + request_id: Some("req-1".to_string()), + human_role: "pilot-repo-reader".to_string(), + } + } + + const SYNTHETIC_TOKEN: &str = "ghs_synthetic_token_value_for_tests"; + + #[test] + fn an_issuance_record_is_emitted_at_info() { + let logged = capture_at_info(|| { + record_issuance( + &identity(), + "source-reader", + "myorg/pilot", + &BTreeMap::from([("contents".to_string(), "read".to_string())]), + "2099-01-01T00:00:00Z", + SYNTHETIC_TOKEN, + ); + }); + + assert!(!logged.is_empty(), "the record must survive an INFO filter"); + } + + #[test] + fn an_issuance_record_carries_everything_needed_to_join_it_to_the_approval() { + let logged = capture_at_info(|| { + record_issuance( + &identity(), + "source-reader", + "myorg/pilot", + &BTreeMap::from([("contents".to_string(), "read".to_string())]), + "2099-01-01T00:00:00Z", + SYNTHETIC_TOKEN, + ); + }); + + for expected in [ + "alex.mason@example.invalid", + "req-1", + "pilot-repo-reader", + "source-reader", + "myorg/pilot", + "contents=read", + "2099-01-01T00:00:00Z", + &token_fingerprint(SYNTHETIC_TOKEN), + "allow", + ] { + assert!( + logged.contains(expected), + "issuance record is missing '{expected}': {logged}" + ); + } + } + + #[test] + fn an_issuance_record_never_contains_the_token() { + let logged = capture_at_info(|| { + record_issuance( + &identity(), + "source-reader", + "myorg/pilot", + &BTreeMap::from([("contents".to_string(), "read".to_string())]), + "2099-01-01T00:00:00Z", + SYNTHETIC_TOKEN, + ); + }); + + assert!( + !logged.contains(SYNTHETIC_TOKEN), + "the bearer token must never reach a log: {logged}" + ); + assert!( + !logged.contains("ghs_"), + "no GitHub token prefix may appear in a log: {logged}" + ); + } + + #[test] + fn a_denial_is_emitted_at_info_without_inventing_an_identity() { + let logged = capture_at_info(|| { + record_denial( + None, + "source-reader", + "myorg/other", + "no human-policy authorizes", + ); + }); + + assert!(logged.contains("deny"), "{logged}"); + assert!(logged.contains("myorg/other"), "{logged}"); + assert!( + logged.contains("unauthenticated"), + "an unvalidated token must not be reported as an identity: {logged}" + ); + } + + #[test] + fn a_denial_after_validation_names_the_person() { + let logged = capture_at_info(|| { + record_denial( + Some("alex.mason@example.invalid"), + "source-reader", + "myorg/pilot", + "GitHub rejected the request", + ); + }); + + assert!(logged.contains("alex.mason@example.invalid"), "{logged}"); + assert!(logged.contains("deny"), "{logged}"); + } + + #[test] + fn fingerprint_is_stable_and_short() { + assert_eq!( + token_fingerprint("ghs_example"), + token_fingerprint("ghs_example") + ); + assert_eq!(token_fingerprint("ghs_example").len(), FINGERPRINT_LENGTH); + } + + #[test] + fn fingerprint_differs_between_tokens() { + assert_ne!(token_fingerprint("ghs_one"), token_fingerprint("ghs_two")); + } + + #[test] + fn fingerprint_does_not_contain_the_token() { + let token = "ghs_averyrecognisablesecretvalue"; + + assert!( + !token_fingerprint(token).contains("averyrecognisable"), + "a fingerprint that echoed the token would defeat its purpose" + ); + } + + #[test] + fn permissions_label_is_ordered_and_complete() { + let permissions = BTreeMap::from([ + ("metadata".to_string(), "read".to_string()), + ("contents".to_string(), "read".to_string()), + ]); + + assert_eq!( + permissions_label(&permissions), + "contents=read,metadata=read" + ); + } +} diff --git a/src/config.rs b/src/config.rs index 6576751..d0f148d 100644 --- a/src/config.rs +++ b/src/config.rs @@ -49,6 +49,10 @@ pub struct Config { pub installation_policies: Vec, #[serde(rename = "owner-policy", default)] pub owner_policies: Vec, + #[serde(rename = "human-role", default)] + pub human_roles: Vec, + #[serde(rename = "human-policy", default)] + pub human_policies: Vec, } #[derive(Debug, Clone, Deserialize, Eq, PartialEq)] @@ -223,6 +227,100 @@ impl OwnerPolicyConfig { } } +/// A Teleport issuer whose tokens are presented by a *person*. Kept separate from `[[role]]` +/// (which `authzoo` validates for workload identities) so that a workload token can never satisfy +/// a human policy and a human token can never satisfy a workload one: the two are validated by +/// different code against different issuers, and neither namespace can name the other's roles. +#[derive(Debug, Clone, Deserialize)] +#[serde(deny_unknown_fields, rename_all = "kebab-case")] +pub struct HumanRoleConfig { + /// The idcat-local name a `[[human-policy]]` refers to. + pub name: String, + /// The exact `iss` the token must carry. + pub issuer: String, + /// The exact `aud` the token must carry. This is the Teleport *application uri*, which is not + /// necessarily the hostname a person types. + pub audience: String, + /// The JWKS endpoint, given explicitly rather than discovered from the issuer. + pub jwks_url: String, + /// The exact Teleport role string that must appear in `roles-claim`. + pub teleport_role: String, + #[serde(default = "default_roles_claim")] + pub roles_claim: String, + #[serde(default = "default_human_algorithms")] + pub algorithms: Vec, + /// A scalar claim naming the Teleport access request or session, recorded on issuance so an + /// operator can join the approval to the token that followed it. + pub request_id_claim: Option, + #[serde(rename = "required-claims", default)] + pub required_claims: BTreeMap, +} + +/// Asymmetric signature algorithms only. A human role must never be validated against a shared +/// secret: anything able to verify such a token could also mint one. +#[derive(Debug, Clone, Copy, Deserialize, Eq, PartialEq)] +pub enum HumanJwtAlgorithm { + #[serde(rename = "RS256")] + Rs256, + #[serde(rename = "RS384")] + Rs384, + #[serde(rename = "RS512")] + Rs512, + #[serde(rename = "PS256")] + Ps256, + #[serde(rename = "PS384")] + Ps384, + #[serde(rename = "PS512")] + Ps512, + #[serde(rename = "ES256")] + Es256, + #[serde(rename = "ES384")] + Es384, + #[serde(rename = "EdDSA")] + EdDsa, +} + +impl HumanJwtAlgorithm { + pub fn to_algorithm(self) -> jsonwebtoken::Algorithm { + use jsonwebtoken::Algorithm; + match self { + Self::Rs256 => Algorithm::RS256, + Self::Rs384 => Algorithm::RS384, + Self::Rs512 => Algorithm::RS512, + Self::Ps256 => Algorithm::PS256, + Self::Ps384 => Algorithm::PS384, + Self::Ps512 => Algorithm::PS512, + Self::Es256 => Algorithm::ES256, + Self::Es384 => Algorithm::ES384, + Self::EdDsa => Algorithm::EdDSA, + } + } +} + +/// Binds one exact repository to one exact Teleport role. Unlike `[[installation-policy]]` there +/// is no repository glob and no `allowed-roles` fallback: if this does not match, the request is +/// refused rather than retried against a broader path. +#[derive(Debug, Clone, Deserialize)] +#[serde(deny_unknown_fields, rename_all = "kebab-case")] +pub struct HumanPolicyConfig { + pub github_app: String, + /// Exactly one repository in `owner/name` form. Globs are rejected at startup. + pub repository: String, + /// The `[[human-role]]` name that authorises this repository. + pub human_role: String, + // Keys are GitHub permission names (snake_case), not kebab-case. + #[serde(default)] + pub permissions: BTreeMap, +} + +fn default_roles_claim() -> String { + "roles".to_string() +} + +fn default_human_algorithms() -> Vec { + vec![HumanJwtAlgorithm::Rs256] +} + impl Config { pub fn load(path: &str) -> anyhow::Result { let content = std::fs::read_to_string(path) @@ -276,8 +374,10 @@ impl Config { } } let role_validator = authzoo::TokenValidator::new(self.roles.clone())?; - if !disable_auth && self.roles.is_empty() { - anyhow::bail!("at least one [[role]] entry is required"); + // A human-only deployment configures `[[human-role]]` and no workload `[[role]]`, so + // either kind satisfies this. + if !disable_auth && self.roles.is_empty() && self.human_roles.is_empty() { + anyhow::bail!("at least one [[role]] or [[human-role]] entry is required"); } if self.github_apps.is_empty() { @@ -566,6 +666,9 @@ impl Config { } } } + // Run the human checks first, so a misreferenced app or role reports the specific + // problem rather than the generic "this app authorizes nothing" that follows from it. + self.validate_human_config(&github_apps, &role_validator)?; for github_app in &self.github_apps { let has_installation_policy = self .installation_policies @@ -575,12 +678,17 @@ impl Config { .owner_policies .iter() .any(|owner_policy| owner_policy.github_app == github_app.name); + let has_human_policy = self + .human_policies + .iter() + .any(|human_policy| human_policy.github_app == github_app.name); if github_app.allowed_roles.is_empty() && !has_installation_policy && !has_owner_policy + && !has_human_policy { anyhow::bail!( - "github-app '{}' must define at least one allowed-role, installation-policy or owner-policy", + "github-app '{}' must define at least one allowed-role, installation-policy, owner-policy or human-policy", github_app.name ); } @@ -588,6 +696,219 @@ impl Config { } Ok(()) } + + /// Startup checks for the human path. Every one of these is a fail-closed condition: rather + /// than narrowing a loose policy at request time, idcat refuses to start. + fn validate_human_config( + &self, + github_apps: &std::collections::HashSet, + role_validator: &authzoo::TokenValidator, + ) -> anyhow::Result<()> { + let mut human_role_names = HashSet::new(); + for human_role in &self.human_roles { + if human_role.name.is_empty() { + anyhow::bail!("human-role names must not be empty"); + } + if !human_role_names.insert(human_role.name.clone()) { + anyhow::bail!("duplicate human-role '{}'", human_role.name); + } + // A shared name across the two namespaces would make `role = "x"` ambiguous to a + // reader, and makes the allowed-roles overlap check below unreadable. + if role_validator.roles().contains_key(&human_role.name) { + anyhow::bail!( + "human-role '{}' has the same name as a [[role]]; workload and human role names must not overlap", + human_role.name + ); + } + if human_role.issuer.is_empty() { + anyhow::bail!("human-role '{}' must define issuer", human_role.name); + } + if human_role.audience.is_empty() { + anyhow::bail!( + "human-role '{}' must define audience (the Teleport application uri)", + human_role.name + ); + } + if !human_role.jwks_url.starts_with("https://") { + anyhow::bail!( + "human-role '{}' jwks-url must be an https URL", + human_role.name + ); + } + if !human_role + .jwks_url + .ends_with(crate::human::TELEPORT_JWKS_PATH) + { + anyhow::bail!( + "human-role '{}' jwks-url must end in '{}'", + human_role.name, + crate::human::TELEPORT_JWKS_PATH + ); + } + if human_role.teleport_role.is_empty() { + anyhow::bail!("human-role '{}' must define teleport-role", human_role.name); + } + if human_role.teleport_role.contains('*') || human_role.teleport_role.contains('?') { + anyhow::bail!( + "human-role '{}' teleport-role must be an exact role name, not a pattern", + human_role.name + ); + } + if human_role.roles_claim.is_empty() { + anyhow::bail!( + "human-role '{}' roles-claim must not be empty", + human_role.name + ); + } + if human_role.algorithms.is_empty() { + anyhow::bail!( + "human-role '{}' must allow at least one algorithm", + human_role.name + ); + } + if matches!(human_role.request_id_claim.as_deref(), Some("")) { + anyhow::bail!( + "human-role '{}' request-id-claim must not be empty when set", + human_role.name + ); + } + for (claim, requirement) in &human_role.required_claims { + if claim.is_empty() { + anyhow::bail!( + "human-role '{}' required-claim names must not be empty", + human_role.name + ); + } + if claim == &human_role.roles_claim { + anyhow::bail!( + "human-role '{}' required-claim '{}' duplicates roles-claim; role membership is checked against the array, not as a scalar", + human_role.name, + claim + ); + } + requirement.validate(&human_role.name, claim)?; + } + } + + let mut seen_mappings: HashSet<(String, String, String)> = HashSet::new(); + let mut seen_repositories: HashSet<(String, String)> = HashSet::new(); + for human_policy in &self.human_policies { + let repository = &human_policy.repository; + if !github_apps.contains(&human_policy.github_app) { + anyhow::bail!( + "human-policy references unknown github-app '{}'", + human_policy.github_app + ); + } + if repository.contains('*') || repository.contains('?') { + anyhow::bail!( + "human-policy for github-app '{}' repository '{}' must name one exact repository; wildcards are not allowed on the human path", + human_policy.github_app, + repository + ); + } + match repository.split_once('/') { + Some((owner, name)) + if !owner.is_empty() && !name.is_empty() && !name.contains('/') => {} + _ => anyhow::bail!( + "human-policy for github-app '{}' repository '{}' must be in owner/name form", + human_policy.github_app, + repository + ), + } + if !human_role_names.contains(&human_policy.human_role) { + anyhow::bail!( + "human-policy for repository '{}' references unknown human-role '{}'", + repository, + human_policy.human_role + ); + } + // Without this, a human policy would inherit whatever the App's installation grants, + // which is exactly the broad path the pilot is meant to avoid. + if human_policy.permissions.is_empty() { + anyhow::bail!( + "human-policy for github-app '{}' repository '{}' must define at least one permission", + human_policy.github_app, + repository + ); + } + for (permission, value) in &human_policy.permissions { + if permission.is_empty() { + anyhow::bail!( + "human-policy for github-app '{}' repository '{}' permission names must not be empty", + human_policy.github_app, + repository + ); + } + if value.is_empty() { + anyhow::bail!( + "human-policy for github-app '{}' repository '{}' permission '{}' must define a value", + human_policy.github_app, + repository, + permission + ); + } + // Same rules and wording as the installation-policy and owner-policy checks, so + // the three read alike; GitHub stays authoritative and the value is forwarded. + if !known_github_permissions().contains(permission.as_str()) { + warn!( + github_app = %human_policy.github_app, + repository = %repository, + human_role = %human_policy.human_role, + permission = %permission, + "permission '{permission}' is not a recognised GitHub permission. If this is intended, consider updating the permissions list." + ); + } + if !KNOWN_PERMISSION_VALUES.contains(&value.as_str()) { + warn!( + github_app = %human_policy.github_app, + repository = %repository, + human_role = %human_policy.human_role, + permission = %permission, + value = %value, + "'{value}' is not a recognised access level (expected read, write or admin) for permission '{permission}'. If this is intended, it will still be forwarded to GitHub." + ); + } + } + // A GitHub App that grants the same role a broad token through `allowed-roles` would + // make the narrow human policy pointless, since the broad path is checked first. + let github_app = self + .github_apps + .iter() + .find(|github_app| github_app.name == human_policy.github_app) + .expect("github-app presence was checked above"); + if github_app.allowed_roles.contains(&human_policy.human_role) { + anyhow::bail!( + "github-app '{}' grants human-role '{}' through allowed-roles; a human-policy must be the only path to that role", + github_app.name, + human_policy.human_role + ); + } + let mapping = ( + human_policy.github_app.clone(), + repository.clone(), + human_policy.human_role.clone(), + ); + if !seen_mappings.insert(mapping) { + anyhow::bail!( + "duplicate human-policy for github-app '{}' repository '{}' human-role '{}'", + human_policy.github_app, + repository, + human_policy.human_role + ); + } + // Two roles reaching the same repository, or one role reaching it through two Apps, + // makes "who approved this" ambiguous in the issuance record. + if !seen_repositories.insert((human_policy.github_app.clone(), repository.clone())) { + anyhow::bail!( + "github-app '{}' has more than one human-policy for repository '{}'; each repository must map to exactly one human-role", + human_policy.github_app, + repository + ); + } + } + Ok(()) + } } fn is_valid_owner_pattern(pattern: &str) -> bool { @@ -1529,7 +1850,7 @@ secret-key = "private-key.pem" let error = config.validate(false).unwrap_err(); assert_eq!( error.to_string(), - "github-app 'default' must define at least one allowed-role, installation-policy or owner-policy" + "github-app 'default' must define at least one allowed-role, installation-policy, owner-policy or human-policy" ); } @@ -1757,3 +2078,369 @@ secret-key = "private-key.pem" config.validate(true).unwrap(); } } + +/// Startup validation of the human path. Every case here is a configuration idcat must refuse to +/// start with, rather than narrow silently at request time. +#[cfg(test)] +mod human_config_tests { + use super::*; + + const BASE: &str = r#" +[[github-app]] +name = "source-reader" +app-id = 42 +secret-key = "private-key.pem" + +[[human-role]] +name = "pilot-repo-reader" +issuer = "https://teleport.example.invalid" +audience = "https://idcat.teleport.example.invalid:443" +jwks-url = "https://teleport.example.invalid/.well-known/jwks.json" +teleport-role = "github-app-token-pilot-repo" +"#; + + fn config(extra: &str) -> anyhow::Result<()> { + let config: Config = toml::from_str(&format!("{BASE}{extra}"))?; + config.validate(false) + } + + const VALID_POLICY: &str = r#" +[[human-policy]] +github-app = "source-reader" +repository = "myorg/pilot" +human-role = "pilot-repo-reader" + +[human-policy.permissions] +contents = "read" +"#; + + #[test] + fn accepts_one_exact_repository_with_a_permission_map() { + config(VALID_POLICY).unwrap(); + } + + #[test] + fn rejects_a_wildcard_repository() { + for repository in ["*", "myorg/*", "myorg/pilot?"] { + let error = config(&format!( + r#" +[[human-policy]] +github-app = "source-reader" +repository = "{repository}" +human-role = "pilot-repo-reader" + +[human-policy.permissions] +contents = "read" +"# + )) + .unwrap_err(); + assert!( + error.to_string().contains("one exact repository"), + "repository '{repository}' must be refused, got: {error}" + ); + } + } + + #[test] + fn rejects_a_repository_that_is_not_in_owner_name_form() { + let error = config( + r#" +[[human-policy]] +github-app = "source-reader" +repository = "pilot" +human-role = "pilot-repo-reader" + +[human-policy.permissions] +contents = "read" +"#, + ) + .unwrap_err(); + + assert!(error.to_string().contains("owner/name form"), "{error}"); + } + + #[test] + fn rejects_an_empty_permission_map() { + let error = config( + r#" +[[human-policy]] +github-app = "source-reader" +repository = "myorg/pilot" +human-role = "pilot-repo-reader" +"#, + ) + .unwrap_err(); + + assert!( + error.to_string().contains("at least one permission"), + "an empty permission map would inherit the whole installation: {error}" + ); + } + + #[test] + fn rejects_a_second_policy_for_the_same_repository() { + let error = config(&format!( + r#"{VALID_POLICY} +[[human-policy]] +github-app = "source-reader" +repository = "myorg/pilot" +human-role = "pilot-repo-reader" + +[human-policy.permissions] +contents = "write" +"# + )) + .unwrap_err(); + + assert!( + error.to_string().contains("duplicate human-policy"), + "{error}" + ); + } + + #[test] + fn rejects_two_roles_reaching_the_same_repository() { + let error = config(&format!( + r#" +[[human-role]] +name = "second-reader" +issuer = "https://teleport.example.invalid" +audience = "https://idcat.teleport.example.invalid:443" +jwks-url = "https://teleport.example.invalid/.well-known/jwks.json" +teleport-role = "another-teleport-role" +{VALID_POLICY} +[[human-policy]] +github-app = "source-reader" +repository = "myorg/pilot" +human-role = "second-reader" + +[human-policy.permissions] +contents = "read" +"# + )) + .unwrap_err(); + + assert!( + error + .to_string() + .contains("more than one human-policy for repository"), + "{error}" + ); + } + + #[test] + fn rejects_a_github_app_that_also_grants_the_role_through_allowed_roles() { + // `allowed-roles` is checked before any policy on the workload path, so an App granting + // the same name there would make the narrow human policy unreachable and pointless. + let error = toml::from_str::(&format!( + r#" +[[role]] +name = "pilot-repo-reader" +audience = "idcat" +issuer = "https://token.actions.githubusercontent.com" +validation-key = "secret" +algorithms = ["HS256"] + +[[github-app]] +name = "source-reader" +app-id = 42 +secret-key = "private-key.pem" +allowed-roles = ["pilot-repo-reader"] + +[[human-role]] +name = "pilot-repo-reader" +issuer = "https://teleport.example.invalid" +audience = "https://idcat.teleport.example.invalid:443" +jwks-url = "https://teleport.example.invalid/.well-known/jwks.json" +teleport-role = "github-app-token-pilot-repo" +{VALID_POLICY}"# + )) + .unwrap() + .validate(false) + .unwrap_err(); + + assert!( + error.to_string().contains("must not overlap"), + "a name shared by [[role]] and [[human-role]] must be refused: {error}" + ); + } + + #[test] + fn rejects_an_app_whose_allowed_roles_name_the_human_role() { + // No `[[role]]` of this name exists, so `allowed-roles` cannot resolve it. Together with + // `rejects_a_github_app_that_also_grants_the_role_through_allowed_roles` (which covers the + // case where such a `[[role]]` does exist) this closes both halves: an App can never grant + // a broad token under the name a human-policy uses. + let error = config(&format!( + r#" +[[github-app]] +name = "second-app" +app-id = 43 +secret-key = "private-key.pem" +allowed-roles = ["pilot-repo-reader"] +{VALID_POLICY}"# + )) + .unwrap_err(); + + assert!( + error.to_string().contains("unknown role"), + "an allowed-role naming a human-role must not resolve: {error}" + ); + } + + #[test] + fn rejects_a_jwks_url_that_is_not_the_teleport_jwks_path() { + for jwks_url in [ + "https://teleport.example.invalid/.well-known/openid-configuration", + "https://teleport.example.invalid/jwks", + "http://teleport.example.invalid/.well-known/jwks.json", + ] { + let error = toml::from_str::(&format!( + r#" +[[github-app]] +name = "source-reader" +app-id = 42 +secret-key = "private-key.pem" + +[[human-role]] +name = "pilot-repo-reader" +issuer = "https://teleport.example.invalid" +audience = "https://idcat.teleport.example.invalid:443" +jwks-url = "{jwks_url}" +teleport-role = "github-app-token-pilot-repo" +{VALID_POLICY}"# + )) + .unwrap() + .validate(false) + .unwrap_err(); + + assert!( + error.to_string().contains("jwks-url"), + "jwks-url '{jwks_url}' must be refused, got: {error}" + ); + } + } + + #[test] + fn rejects_a_teleport_role_pattern() { + let error = toml::from_str::(&format!( + r#" +[[github-app]] +name = "source-reader" +app-id = 42 +secret-key = "private-key.pem" + +[[human-role]] +name = "pilot-repo-reader" +issuer = "https://teleport.example.invalid" +audience = "https://idcat.teleport.example.invalid:443" +jwks-url = "https://teleport.example.invalid/.well-known/jwks.json" +teleport-role = "github-app-token-*" +{VALID_POLICY}"# + )) + .unwrap() + .validate(false) + .unwrap_err(); + + assert!(error.to_string().contains("exact role name"), "{error}"); + } + + #[test] + fn rejects_an_unknown_human_role_reference() { + let error = config( + r#" +[[human-policy]] +github-app = "source-reader" +repository = "myorg/pilot" +human-role = "no-such-role" + +[human-policy.permissions] +contents = "read" +"#, + ) + .unwrap_err(); + + assert!(error.to_string().contains("unknown human-role"), "{error}"); + } + + #[test] + fn rejects_an_unknown_github_app_reference() { + let error = config( + r#" +[[human-policy]] +github-app = "no-such-app" +repository = "myorg/pilot" +human-role = "pilot-repo-reader" + +[human-policy.permissions] +contents = "read" +"#, + ) + .unwrap_err(); + + assert!(error.to_string().contains("unknown github-app"), "{error}"); + } + + #[test] + fn rejects_an_hmac_algorithm_for_a_human_role() { + // A shared secret able to verify a human token could also mint one. + let error = toml::from_str::(&format!( + r#" +[[github-app]] +name = "source-reader" +app-id = 42 +secret-key = "private-key.pem" + +[[human-role]] +name = "pilot-repo-reader" +issuer = "https://teleport.example.invalid" +audience = "https://idcat.teleport.example.invalid:443" +jwks-url = "https://teleport.example.invalid/.well-known/jwks.json" +teleport-role = "github-app-token-pilot-repo" +algorithms = ["HS256"] +{VALID_POLICY}"# + )) + .unwrap_err(); + + assert!( + error.to_string().contains("HS256") || error.to_string().contains("unknown variant"), + "an HMAC algorithm must not parse for a human role: {error}" + ); + } + + #[test] + fn rejects_a_required_claim_that_shadows_the_roles_claim() { + let error = toml::from_str::(&format!( + r#" +[[github-app]] +name = "source-reader" +app-id = 42 +secret-key = "private-key.pem" + +[[human-role]] +name = "pilot-repo-reader" +issuer = "https://teleport.example.invalid" +audience = "https://idcat.teleport.example.invalid:443" +jwks-url = "https://teleport.example.invalid/.well-known/jwks.json" +teleport-role = "github-app-token-pilot-repo" + +[human-role.required-claims] +roles = "github-app-token-pilot-repo" +{VALID_POLICY}"# + )) + .unwrap() + .validate(false) + .unwrap_err(); + + assert!( + error.to_string().contains("duplicates roles-claim"), + "a scalar requirement on the roles claim must be refused: {error}" + ); + } + + #[test] + fn a_github_app_may_be_authorized_by_a_human_policy_alone() { + // Without this, adding a human-only App would demand a pointless workload role. + config(VALID_POLICY).unwrap(); + } +} diff --git a/src/github.rs b/src/github.rs index 8b1c550..f1d6251 100644 --- a/src/github.rs +++ b/src/github.rs @@ -75,6 +75,7 @@ struct InstallationTokenCacheKey { github_app: String, target: InstallationTarget, scope: crate::service::TokenScope, + requester: crate::service::Requester, } #[derive(Clone)] @@ -160,11 +161,13 @@ impl GithubClient { signer: &dyn Signer, target: &InstallationTarget, scope: crate::service::TokenScope, + requester: crate::service::Requester, ) -> anyhow::Result { let token_cache_key = InstallationTokenCacheKey { github_app: github_app.name.clone(), target: target.clone(), scope, + requester, }; if let Some(token) = self.cached_installation_token(&token_cache_key).await { debug!( @@ -659,6 +662,7 @@ mod tests { &StubSigner, &repository_target(), broad_scope(), + crate::service::Requester::Workload, ) .await .unwrap(); @@ -671,6 +675,7 @@ mod tests { &StubSigner, &repository_target(), narrow_scope(&[]), + crate::service::Requester::Workload, ) .await .unwrap(); @@ -686,6 +691,153 @@ mod tests { ); } + fn person(subject: &str, request_id: &str) -> crate::service::Requester { + crate::service::Requester::Person { + subject: subject.to_owned(), + request_id: Some(request_id.to_owned()), + } + } + + #[tokio::test] + async fn two_people_are_never_served_each_others_token() { + let stub = spawn_stub_github_api(&["myorg"]).await; + let mut client = GithubClient::new().unwrap(); + client.api_url = stub.url; + let github_app = test_github_app(); + + let first = client + .create_installation_token( + &github_app, + &StubSigner, + &repository_target(), + narrow_scope(&[("contents", "read")]), + person("first@example.invalid", "req-1"), + ) + .await + .unwrap(); + // Same App, same repository, same permissions: everything about the request matches except + // who is asking. Each person must still receive their own token. + let second = client + .create_installation_token( + &github_app, + &StubSigner, + &repository_target(), + narrow_scope(&[("contents", "read")]), + person("second@example.invalid", "req-2"), + ) + .await + .unwrap(); + + assert_ne!( + first.token, second.token, + "two people were served the same cached token" + ); + assert_eq!(stub.mint_count.load(Ordering::SeqCst), 2); + } + + #[tokio::test] + async fn a_second_approval_for_one_person_mints_a_fresh_token() { + let stub = spawn_stub_github_api(&["myorg"]).await; + let mut client = GithubClient::new().unwrap(); + client.api_url = stub.url; + let github_app = test_github_app(); + + let first = client + .create_installation_token( + &github_app, + &StubSigner, + &repository_target(), + narrow_scope(&[("contents", "read")]), + person("alex@example.invalid", "req-1"), + ) + .await + .unwrap(); + let second = client + .create_installation_token( + &github_app, + &StubSigner, + &repository_target(), + narrow_scope(&[("contents", "read")]), + person("alex@example.invalid", "req-2"), + ) + .await + .unwrap(); + + assert_ne!( + first.token, second.token, + "a fresh approval must not revive the token issued under the previous one" + ); + assert_eq!(stub.mint_count.load(Ordering::SeqCst), 2); + } + + #[tokio::test] + async fn one_person_reuses_their_own_cached_token_within_an_approval() { + let stub = spawn_stub_github_api(&["myorg"]).await; + let mut client = GithubClient::new().unwrap(); + client.api_url = stub.url; + let github_app = test_github_app(); + + let first = client + .create_installation_token( + &github_app, + &StubSigner, + &repository_target(), + narrow_scope(&[("contents", "read")]), + person("alex@example.invalid", "req-1"), + ) + .await + .unwrap(); + let again = client + .create_installation_token( + &github_app, + &StubSigner, + &repository_target(), + narrow_scope(&[("contents", "read")]), + person("alex@example.invalid", "req-1"), + ) + .await + .unwrap(); + + assert_eq!(first.token, again.token); + assert_eq!( + stub.mint_count.load(Ordering::SeqCst), + 1, + "partitioning must not defeat caching within a single approval" + ); + } + + #[tokio::test] + async fn a_person_is_never_served_the_shared_workload_token() { + let stub = spawn_stub_github_api(&["myorg"]).await; + let mut client = GithubClient::new().unwrap(); + client.api_url = stub.url; + let github_app = test_github_app(); + + let workload = client + .create_installation_token( + &github_app, + &StubSigner, + &repository_target(), + narrow_scope(&[("contents", "read")]), + crate::service::Requester::Workload, + ) + .await + .unwrap(); + let human = client + .create_installation_token( + &github_app, + &StubSigner, + &repository_target(), + narrow_scope(&[("contents", "read")]), + person("alex@example.invalid", "req-1"), + ) + .await + .unwrap(); + + assert_ne!(workload.token, human.token); + assert_eq!(stub.mint_count.load(Ordering::SeqCst), 2); + } + #[tokio::test] async fn caller_is_not_served_a_cached_token_with_different_permissions() { let stub = spawn_stub_github_api(&["myorg"]).await; @@ -700,6 +852,7 @@ mod tests { &StubSigner, &repository_target(), narrow_scope(&[("contents", "read")]), + crate::service::Requester::Workload, ) .await .unwrap(); @@ -711,6 +864,7 @@ mod tests { &StubSigner, &repository_target(), narrow_scope(&[("contents", "write")]), + crate::service::Requester::Workload, ) .await .unwrap(); @@ -743,6 +897,7 @@ mod tests { .into_iter() .collect(), }, + crate::service::Requester::Workload, ) .await .unwrap(); @@ -773,6 +928,7 @@ mod tests { &StubSigner, &InstallationTarget::Owner("noa".to_string()), broad_scope(), + crate::service::Requester::Workload, ) .await .unwrap(); @@ -798,11 +954,18 @@ mod tests { &StubSigner, &repository_target(), broad_scope(), + crate::service::Requester::Workload, ) .await .unwrap(); let owner_wide = client - .create_installation_token(&github_app, &StubSigner, &owner_target(), broad_scope()) + .create_installation_token( + &github_app, + &StubSigner, + &owner_target(), + broad_scope(), + crate::service::Requester::Workload, + ) .await .unwrap(); diff --git a/src/human.rs b/src/human.rs new file mode 100644 index 0000000..f6602c0 --- /dev/null +++ b/src/human.rs @@ -0,0 +1,645 @@ +// SPDX-License-Identifier: MIT +// SPDX-FileCopyrightText: The idcat contributors + +//! Validation of Teleport-issued JWTs presented by a *person*, as opposed to the workload tokens +//! handled by [`crate::service::TokenValidator`] (which delegates to `authzoo`). +//! +//! This exists as an idcat-local validator rather than a change to `authzoo` because the human +//! path needs four things `authzoo` 0.1.4 does not offer, and which cannot be expressed through +//! its `RoleConfig` (`#[serde(deny_unknown_fields)]`): +//! +//! * **Array-valued role claims.** Teleport puts a user's roles in a JSON array. `authzoo`'s +//! `ValidatedClaims::claim_value` ends in `.and_then(Value::as_str)`, so an array claim reads +//! back as absent and can never match. A *scalar* claim of the same name must not be accepted +//! as a substitute, or a token minted by a different issuer shape could impersonate a role. +//! * **An explicitly configured JWKS URL.** `authzoo` only reaches a JWKS through +//! `GET {issuer}/.well-known/openid-configuration`. Teleport publishes a JWKS directly, and the +//! pilot must not depend on OpenID discovery for this issuer. +//! * **The validated claims, not just role names.** `authzoo::TokenValidator::validate` returns +//! `Vec` of role names, discarding `sub`. A person-level issuance record needs the +//! subject. +//! * **One parse per request.** `authzoo` validates once per configured role, and idcat's +//! `validate_role_with_claims` builds a fresh validator per candidate policy, so an invalid +//! token costs N parses and N key lookups. Here the issuer is chosen from configuration first +//! and the token is parsed exactly once. + +use anyhow::Context; +use jsonwebtoken::jwk::JwkSet; +use jsonwebtoken::{Algorithm, DecodingKey, Validation, decode, decode_header}; +use serde::Deserialize; +use serde_json::Value; +use std::collections::BTreeMap; +use std::sync::Arc; +use std::time::{Duration, Instant}; +use tokio::sync::RwLock; +use tracing::debug; + +/// The path a Teleport JWKS is published under. An operator-supplied `jwks-url` must end in this, +/// so a typo cannot silently point key discovery at an arbitrary endpoint. +pub const TELEPORT_JWKS_PATH: &str = "/.well-known/jwks.json"; + +/// How long a fetched JWKS is reused before it is fetched again. +const JWKS_CACHE_TTL: Duration = Duration::from_secs(15 * 60); + +/// The shortest interval between two JWKS fetches provoked by an unrecognised `kid`. Without this +/// floor, a stream of tokens carrying junk `kid` values would become a fetch amplifier against the +/// Teleport proxy. +const JWKS_REFETCH_FLOOR: Duration = Duration::from_secs(60); + +#[derive(Debug, thiserror::Error, Eq, PartialEq)] +pub enum HumanTokenError { + #[error("token header could not be decoded")] + MalformedHeader, + #[error("token algorithm {0:?} is not allowed for this issuer")] + AlgorithmNotAllowed(Algorithm), + #[error("no validation key matched the token's key id")] + UnknownKeyId, + #[error("token signature, issuer, audience, expiry or not-before check failed")] + TokenRejected, + #[error("claim '{claim}' is not a JSON array of strings")] + RoleClaimNotAnArray { claim: String }, + #[error("required role '{role}' is not present in claim '{claim}'")] + RoleNotHeld { claim: String, role: String }, + #[error("claim '{claim}' did not match the configured requirement")] + ClaimMismatch { claim: String }, + #[error("validation keys could not be retrieved")] + KeysUnavailable, +} + +/// The claims of a token that has passed every check, kept whole rather than reduced to a role +/// name so that an issuance record can name the person. +#[derive(Clone, Debug)] +pub struct HumanClaims { + subject: String, + claims: BTreeMap, +} + +impl HumanClaims { + pub fn subject(&self) -> &str { + &self.subject + } + + /// A scalar claim, for joining an issuance record to the Teleport access request or session + /// that authorised it. Returns `None` for an absent or non-string claim. + pub fn claim_str(&self, claim: &str) -> Option<&str> { + self.claims.get(claim).and_then(Value::as_str) + } +} + +#[derive(Debug, Deserialize)] +struct RawClaims { + sub: String, + #[serde(flatten)] + claims: BTreeMap, +} + +/// Where validation keys come from. +#[derive(Clone)] +enum JwksSource { + /// Fetched from a configured URL and cached. + Http { + url: String, + client: reqwest::Client, + }, + /// Supplied directly, so a test never reaches the network. + #[cfg(test)] + Static(Arc), +} + +struct CachedJwks { + jwks: Arc, + fetched_at: Instant, +} + +#[derive(Clone)] +pub struct HumanTokenValidator { + issuer: String, + audience: String, + algorithms: Vec, + source: JwksSource, + cache: Arc>>, +} + +impl HumanTokenValidator { + pub fn from_http( + issuer: impl Into, + audience: impl Into, + jwks_url: impl Into, + algorithms: Vec, + ) -> anyhow::Result { + let client = reqwest::Client::builder() + .timeout(Duration::from_secs(5)) + .connect_timeout(Duration::from_secs(2)) + .build() + .context("failed to build the Teleport JWKS client")?; + Ok(Self::new( + issuer, + audience, + algorithms, + JwksSource::Http { + url: jwks_url.into(), + client, + }, + )) + } + + #[cfg(test)] + pub fn from_static_jwks( + issuer: impl Into, + audience: impl Into, + jwks: JwkSet, + algorithms: Vec, + ) -> Self { + Self::new( + issuer, + audience, + algorithms, + JwksSource::Static(Arc::new(jwks)), + ) + } + + fn new( + issuer: impl Into, + audience: impl Into, + algorithms: Vec, + source: JwksSource, + ) -> Self { + Self { + issuer: issuer.into(), + audience: audience.into(), + algorithms, + source, + cache: Arc::new(RwLock::new(None)), + } + } + + /// Validates `token` once: signature, exact issuer, exact audience, expiry and not-before, + /// then the configured role membership and any additional scalar claim requirements. + pub async fn validate( + &self, + token: &str, + roles_claim: &str, + required_role: &str, + required_claims: &BTreeMap, + ) -> Result { + let header = decode_header(token).map_err(|_| HumanTokenError::MalformedHeader)?; + if !self.algorithms.contains(&header.alg) { + return Err(HumanTokenError::AlgorithmNotAllowed(header.alg)); + } + + let key = self.decoding_key(header.kid.as_deref(), header.alg).await?; + + let mut validation = Validation::new(header.alg); + validation.set_issuer(&[&self.issuer]); + validation.set_audience(&[&self.audience]); + validation.set_required_spec_claims(&["exp", "iss", "aud", "sub"]); + validation.validate_exp = true; + validation.validate_nbf = true; + // The pilot's approval window is one hour; a clock-skew grace period long enough to matter + // would eat into it, and both ends are NTP-disciplined. + validation.leeway = 0; + + let decoded = decode::(token, &key, &validation).map_err(|error| { + debug!(error = %error, "human token rejected during signature or registered-claim validation"); + HumanTokenError::TokenRejected + })?; + + let claims = decoded.claims; + let roles = role_members(&claims.claims, roles_claim)?; + // Exact string equality against each element: a role that merely *contains* the required + // name, or a scalar claim that happens to spell it, must not grant access. + if !roles.iter().any(|held| held == required_role) { + return Err(HumanTokenError::RoleNotHeld { + claim: roles_claim.to_string(), + role: required_role.to_string(), + }); + } + + for (claim, requirement) in required_claims { + let value = claims.claims.get(claim).and_then(Value::as_str); + if !requirement.matches(value) { + return Err(HumanTokenError::ClaimMismatch { + claim: claim.clone(), + }); + } + } + + Ok(HumanClaims { + subject: claims.sub, + claims: claims.claims, + }) + } + + async fn decoding_key( + &self, + kid: Option<&str>, + algorithm: Algorithm, + ) -> Result { + let jwks = self.jwks(false).await?; + if let Some(key) = select_key(&jwks, kid, algorithm) { + return Ok(key); + } + // An unrecognised key id most often means the issuer has rotated, so try once more with a + // forced fetch before rejecting. + let jwks = self.jwks(true).await?; + select_key(&jwks, kid, algorithm).ok_or(HumanTokenError::UnknownKeyId) + } + + async fn jwks(&self, force: bool) -> Result, HumanTokenError> { + let (url, client) = match &self.source { + #[cfg(test)] + JwksSource::Static(jwks) => return Ok(Arc::clone(jwks)), + JwksSource::Http { url, client } => (url, client), + }; + + { + let cached = self.cache.read().await; + if let Some(cached) = cached.as_ref() { + let age = cached.fetched_at.elapsed(); + let usable = if force { + age < JWKS_REFETCH_FLOOR + } else { + age < JWKS_CACHE_TTL + }; + if usable { + return Ok(Arc::clone(&cached.jwks)); + } + } + } + + let mut cache = self.cache.write().await; + // Another task may have refreshed while this one waited for the write lock. + if let Some(cached) = cache.as_ref() { + let age = cached.fetched_at.elapsed(); + let usable = if force { + age < JWKS_REFETCH_FLOOR + } else { + age < JWKS_CACHE_TTL + }; + if usable { + return Ok(Arc::clone(&cached.jwks)); + } + } + + debug!(jwks_url = %url, "fetching Teleport JWKS"); + let fetched: JwkSet = client + .get(url) + .send() + .await + .and_then(reqwest::Response::error_for_status) + .map_err(|error| { + debug!(jwks_url = %url, error = %error, "Teleport JWKS request failed"); + HumanTokenError::KeysUnavailable + })? + .json() + .await + .map_err(|error| { + debug!(jwks_url = %url, error = %error, "Teleport JWKS could not be parsed"); + HumanTokenError::KeysUnavailable + })?; + + let fetched = Arc::new(fetched); + *cache = Some(CachedJwks { + jwks: Arc::clone(&fetched), + fetched_at: Instant::now(), + }); + Ok(fetched) + } +} + +/// Reads `roles_claim` as a JSON array of strings. A scalar claim of the same name is an error +/// rather than a single-element array: accepting it would let a token shaped like a different +/// issuer's satisfy a Teleport role requirement. +fn role_members( + claims: &BTreeMap, + roles_claim: &str, +) -> Result, HumanTokenError> { + let value = claims + .get(roles_claim) + .ok_or_else(|| HumanTokenError::RoleClaimNotAnArray { + claim: roles_claim.to_string(), + })?; + let Value::Array(entries) = value else { + return Err(HumanTokenError::RoleClaimNotAnArray { + claim: roles_claim.to_string(), + }); + }; + entries + .iter() + .map(|entry| { + entry + .as_str() + .map(str::to_string) + .ok_or_else(|| HumanTokenError::RoleClaimNotAnArray { + claim: roles_claim.to_string(), + }) + }) + .collect() +} + +fn select_key(jwks: &JwkSet, kid: Option<&str>, algorithm: Algorithm) -> Option { + let jwk = match kid { + Some(kid) => jwks.find(kid)?, + // Teleport stamps a `kid`, but a JWKS holding exactly one key is unambiguous without one. + None => match jwks.keys.as_slice() { + [only] => only, + _ => return None, + }, + }; + if let Some(key_algorithm) = jwk.common.key_algorithm + && key_algorithm.to_string() != format!("{algorithm:?}") + { + return None; + } + DecodingKey::from_jwk(jwk).ok() +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::testkeys::{TeleportClaims, other_keys, test_keys}; + use serde_json::json; + + const ISSUER: &str = "https://teleport.example.invalid"; + // The audience is the Teleport *application uri*, which is deliberately not the human-facing + // proxy hostname; conflating the two is the mistake this constant exists to make visible. + const AUDIENCE: &str = "https://idcat.teleport.example.invalid:443"; + const ROLES_CLAIM: &str = "roles"; + const ROLE: &str = "github-app-token-pilot-repo"; + + fn validator() -> HumanTokenValidator { + HumanTokenValidator::from_static_jwks( + ISSUER, + AUDIENCE, + test_keys().jwks.clone(), + vec![Algorithm::RS256], + ) + } + + async fn validate(token: &str) -> Result { + validator() + .validate(token, ROLES_CLAIM, ROLE, &BTreeMap::new()) + .await + } + + fn claims(roles: serde_json::Value) -> TeleportClaims { + TeleportClaims::new("alex.mason@example.invalid", ISSUER, AUDIENCE, roles) + } + + #[tokio::test] + async fn accepts_a_token_whose_roles_array_contains_the_required_role() { + let token = test_keys().sign(&claims(json!(["some-other-role", ROLE]))); + + let validated = validate(&token).await.unwrap(); + + assert_eq!(validated.subject(), "alex.mason@example.invalid"); + } + + #[tokio::test] + async fn returns_the_subject_so_an_issuance_record_can_name_the_person() { + let mut raw = claims(json!([ROLE])); + raw.sub = "someone.else@example.invalid".to_string(); + let token = test_keys().sign(&raw); + + assert_eq!( + validate(&token).await.unwrap().subject(), + "someone.else@example.invalid", + "a successful validation must not be reduced to role names" + ); + } + + #[test] + fn role_members_reads_a_json_array_of_strings() { + let claims = BTreeMap::from([(ROLES_CLAIM.to_string(), json!(["some-other-role", ROLE]))]); + + assert_eq!( + role_members(&claims, ROLES_CLAIM).unwrap(), + ["some-other-role", ROLE] + ); + } + + #[tokio::test] + async fn rejects_a_scalar_claim_that_merely_looks_like_the_roles_array() { + let token = test_keys().sign(&claims(json!(ROLE))); + + assert_eq!( + validate(&token).await.unwrap_err(), + HumanTokenError::RoleClaimNotAnArray { + claim: ROLES_CLAIM.to_string() + } + ); + } + + #[tokio::test] + async fn rejects_a_role_that_is_only_a_substring_of_a_held_role() { + let token = test_keys().sign(&claims(json!([format!("{ROLE}-readonly")]))); + + assert_eq!( + validate(&token).await.unwrap_err(), + HumanTokenError::RoleNotHeld { + claim: ROLES_CLAIM.to_string(), + role: ROLE.to_string() + } + ); + } + + #[tokio::test] + async fn rejects_a_role_the_required_name_merely_contains() { + let token = test_keys().sign(&claims(json!(["github-app-token"]))); + + assert!(matches!( + validate(&token).await.unwrap_err(), + HumanTokenError::RoleNotHeld { .. } + )); + } + + #[tokio::test] + async fn rejects_a_roles_array_holding_a_non_string_entry() { + let token = test_keys().sign(&claims(json!([ROLE, 7]))); + + assert!(matches!( + validate(&token).await.unwrap_err(), + HumanTokenError::RoleClaimNotAnArray { .. } + )); + } + + #[tokio::test] + async fn rejects_a_missing_roles_claim() { + #[derive(serde::Serialize)] + struct NoRoles { + sub: &'static str, + iss: &'static str, + aud: &'static str, + exp: u64, + } + let token = test_keys().sign(&NoRoles { + sub: "alex.mason@example.invalid", + iss: ISSUER, + aud: AUDIENCE, + exp: crate::testkeys::now() + 3600, + }); + + assert!(matches!( + validate(&token).await.unwrap_err(), + HumanTokenError::RoleClaimNotAnArray { .. } + )); + } + + #[tokio::test] + async fn rejects_a_wrong_audience() { + let mut raw = claims(json!([ROLE])); + raw.aud = "https://idcat.teleport.example.invalid".to_string(); + let token = test_keys().sign(&raw); + + assert_eq!( + validate(&token).await.unwrap_err(), + HumanTokenError::TokenRejected, + "the audience must match the Teleport application uri exactly, not a prefix of it" + ); + } + + #[tokio::test] + async fn rejects_the_human_route_hostname_used_as_an_audience() { + let mut raw = claims(json!([ROLE])); + raw.aud = "https://idcat.example.invalid".to_string(); + let token = test_keys().sign(&raw); + + assert_eq!( + validate(&token).await.unwrap_err(), + HumanTokenError::TokenRejected + ); + } + + #[tokio::test] + async fn rejects_a_wrong_issuer() { + let mut raw = claims(json!([ROLE])); + raw.iss = "https://teleport.other.invalid".to_string(); + let token = test_keys().sign(&raw); + + assert_eq!( + validate(&token).await.unwrap_err(), + HumanTokenError::TokenRejected + ); + } + + #[tokio::test] + async fn rejects_a_signature_from_another_key() { + let token = other_keys().sign(&claims(json!([ROLE]))); + + assert_eq!( + validate(&token).await.unwrap_err(), + HumanTokenError::TokenRejected + ); + } + + #[tokio::test] + async fn rejects_an_expired_token_without_leeway() { + let mut raw = claims(json!([ROLE])); + raw.exp = crate::testkeys::now() - 1; + let token = test_keys().sign(&raw); + + assert_eq!( + validate(&token).await.unwrap_err(), + HumanTokenError::TokenRejected + ); + } + + #[tokio::test] + async fn rejects_a_token_that_is_not_yet_valid() { + let mut raw = claims(json!([ROLE])); + raw.nbf = crate::testkeys::now() + 600; + let token = test_keys().sign(&raw); + + assert_eq!( + validate(&token).await.unwrap_err(), + HumanTokenError::TokenRejected + ); + } + + #[tokio::test] + async fn rejects_an_unsigned_token() { + let mut raw = claims(json!([ROLE])); + raw.exp = crate::testkeys::now() + 3600; + let unsigned = jsonwebtoken::encode( + &jsonwebtoken::Header::new(Algorithm::HS256), + &raw, + &jsonwebtoken::EncodingKey::from_secret(b"not-the-issuer-key"), + ) + .unwrap(); + + assert_eq!( + validate(&unsigned).await.unwrap_err(), + HumanTokenError::AlgorithmNotAllowed(Algorithm::HS256), + "an HMAC token must be refused on algorithm, never checked against an RSA public key" + ); + } + + #[tokio::test] + async fn rejects_a_malformed_token() { + assert_eq!( + validate("not-a-jwt").await.unwrap_err(), + HumanTokenError::MalformedHeader + ); + } + + #[tokio::test] + async fn rejects_a_token_signed_by_a_key_absent_from_the_jwks() { + let validator = HumanTokenValidator::from_static_jwks( + ISSUER, + AUDIENCE, + other_keys().jwks.clone(), + vec![Algorithm::RS256], + ); + let token = test_keys().sign(&claims(json!([ROLE]))); + + // Both key sets stamp the same kid, so selection succeeds and the signature check is what + // rejects this. The distinct-kid case is covered by `rejects_an_unknown_key_id`. + assert_eq!( + validator + .validate(&token, ROLES_CLAIM, ROLE, &BTreeMap::new()) + .await + .unwrap_err(), + HumanTokenError::TokenRejected + ); + } + + #[tokio::test] + async fn rejects_an_unknown_key_id() { + let validator = HumanTokenValidator::from_static_jwks( + ISSUER, + AUDIENCE, + crate::testkeys::TestKeyPair::generate("a-different-kid").jwks, + vec![Algorithm::RS256], + ); + let token = test_keys().sign(&claims(json!([ROLE]))); + + assert_eq!( + validator + .validate(&token, ROLES_CLAIM, ROLE, &BTreeMap::new()) + .await + .unwrap_err(), + HumanTokenError::UnknownKeyId + ); + } + + #[tokio::test] + async fn enforces_additional_scalar_claim_requirements() { + let mut raw = claims(json!([ROLE])); + raw.traits = Some(json!("contractor")); + let token = test_keys().sign(&raw); + let required = BTreeMap::from([( + "traits".to_string(), + authzoo::ClaimRequirement::equals("employee"), + )]); + + assert_eq!( + validator() + .validate(&token, ROLES_CLAIM, ROLE, &required) + .await + .unwrap_err(), + HumanTokenError::ClaimMismatch { + claim: "traits".to_string() + } + ); + } +} diff --git a/src/main.rs b/src/main.rs index cb0f5ce..a97abef 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,9 +1,11 @@ // SPDX-License-Identifier: MIT // SPDX-FileCopyrightText: The idcat contributors +mod audit; mod config; mod error; mod github; +mod human; mod jwt; #[cfg(feature = "kms")] #[allow(dead_code)] @@ -12,12 +14,14 @@ mod nats; mod secret; mod service; mod signer; +#[cfg(test)] +mod testkeys; mod webhook; use crate::config::{Config, GithubAppConfig}; use crate::error::AppError; use crate::github::{InstallationTarget, InstallationTokenResponse}; -use crate::service::{AppState, TokenScope, build_app_state}; +use crate::service::{AppState, Requester, TokenScope, build_app_state}; use anyhow::Context; use axum::body::{Body, Bytes}; use axum::extract::{OriginalUri, Path, State}; @@ -121,6 +125,10 @@ async fn run(cli: Cli) -> anyhow::Result<()> { "/installation-token/{github_app}/{owner}/{repo}", post(installation_token), ) + .route( + "/human/installation-token/{github_app}/{owner}/{repo}", + post(human_installation_token), + ) .route( "/proxy/{github_app}/repos/{owner}/{repo}", any(proxy_repo_root), @@ -263,6 +271,19 @@ async fn owner_installation_token( Ok(token.token) } +/// The human token route. Separate from [`installation_token`] so that it can be reached only +/// through the Teleport Application Service, and so that a workload caller cannot arrive here by +/// accident: the two routes share no authorization code. +async fn human_installation_token( + Path((github_app, owner, repo)): Path<(String, String, String)>, + State(state): State, + headers: HeaderMap, +) -> Result { + let repo = format!("{owner}/{repo}"); + let token = create_human_installation_token(&github_app, &repo, &state, &headers).await?; + Ok(token.token) +} + async fn proxy_repo_root( Path((github_app, owner, repo)): Path<(String, String, String)>, State(state): State, @@ -447,12 +468,84 @@ async fn create_installation_token( debug!(github_app = %github_app_name, target = %target, ?token_scope, "requesting GitHub installation access token"); let token = state .github - .create_installation_token(github_app, signer.as_ref(), target, token_scope) + .create_installation_token( + github_app, + signer.as_ref(), + target, + token_scope, + Requester::Workload, + ) .await?; debug!(github_app = %github_app_name, target = %target, expires_at = %token.expires_at, "GitHub installation access token created"); Ok(token) } +async fn create_human_installation_token( + github_app_name: &str, + repo: &str, + state: &AppState, + headers: &HeaderMap, +) -> Result { + // Unlike the workload path there is no `--disable-auth` branch: a missing or malformed + // Authorization header is a denial, recorded as one. + let bearer_token = extract_bearer_token(headers).inspect_err(|error| { + audit::record_denial(None, github_app_name, repo, &error.to_string()); + })?; + + let github_app = state.github_app(github_app_name).inspect_err(|error| { + audit::record_denial(None, github_app_name, repo, &error.to_string()); + })?; + + let (token_scope, identity) = state + .authorize_human(github_app, repo, Some(bearer_token.as_str())) + .await + .inspect_err(|error| { + audit::record_denial(None, github_app_name, repo, &error.to_string()); + })?; + + let signer = state.signer(&github_app.secret_key).map_err(|error| { + audit::record_denial( + Some(&identity.subject), + github_app_name, + repo, + &error.to_string(), + ); + AppError::from(error) + })?; + + let token = state + .github + .create_installation_token( + github_app, + signer.as_ref(), + // A human policy names one exact repository, so the target is always a repository + // target. The human path has no owner-wide equivalent and must never acquire one. + &InstallationTarget::Repository(repo.to_string()), + token_scope.clone(), + Requester::person(&identity), + ) + .await + .map_err(|error| { + audit::record_denial( + Some(&identity.subject), + github_app_name, + repo, + &error.to_string(), + ); + AppError::from(error) + })?; + + audit::record_issuance( + &identity, + github_app_name, + repo, + &token_scope.permissions, + &token.expires_at, + &token.token, + ); + Ok(token) +} + fn extract_bearer_token(headers: &HeaderMap) -> Result { let value = headers .get(header::AUTHORIZATION) @@ -501,6 +594,8 @@ mod tests { github_apps: Arc::new(github_apps), installation_policies: Arc::new(Vec::new()), owner_policies: Arc::new(Vec::new()), + human_policies: Arc::new(Vec::new()), + human_roles: Arc::new(std::collections::BTreeMap::new()), token_validator: TokenValidator::new(Vec::new(), true).unwrap(), github: GithubClient::new().unwrap(), webhook_publisher: None, diff --git a/src/service.rs b/src/service.rs index fba0a2c..fd8076c 100644 --- a/src/service.rs +++ b/src/service.rs @@ -2,11 +2,12 @@ // SPDX-FileCopyrightText: The idcat contributors use crate::config::{ - Config, GithubAppConfig, InstallationPolicyConfig, KeySource, OwnerPolicyConfig, - SELF_ACCESS_OWNER_CLAIM, WebhookTarget, + Config, GithubAppConfig, HumanPolicyConfig, HumanRoleConfig, InstallationPolicyConfig, + KeySource, OwnerPolicyConfig, SELF_ACCESS_OWNER_CLAIM, WebhookTarget, }; use crate::error::AppError; use crate::github::GithubClient; +use crate::human::{HumanTokenError, HumanTokenValidator}; use crate::nats::WebhookPublisher; use crate::secret::FilePrivateKeyStore; use crate::signer::{LocalSigner, Signer}; @@ -40,6 +41,8 @@ pub struct AppState { pub github_apps: Arc>, pub installation_policies: Arc>, pub owner_policies: Arc>, + pub human_policies: Arc>, + pub human_roles: Arc>, pub token_validator: TokenValidator, pub github: GithubClient, pub webhook_publisher: Option, @@ -69,6 +72,8 @@ pub async fn build_app_state(config: &Config, disable_auth: bool) -> anyhow::Res github_apps: Arc::new(config.github_apps.clone()), installation_policies: Arc::new(config.installation_policies.clone()), owner_policies: Arc::new(config.owner_policies.clone()), + human_policies: Arc::new(config.human_policies.clone()), + human_roles: Arc::new(build_human_roles(&config.human_roles)?), token_validator: TokenValidator::new(config.roles.clone(), disable_auth)?, github: GithubClient::new()?, webhook_publisher, @@ -209,6 +214,99 @@ impl AppState { ))) } + /// Authorizes a person's request for a token covering exactly `repo`. + /// + /// This path is deliberately not a variant of [`Self::authorize_github_app`]. It never + /// consults `allowed-roles`, never widens the scope, and never retries against another + /// policy: a request that does not match exactly one `[[human-policy]]` is refused. The + /// repository is resolved from configuration *before* the token is validated and long before + /// GitHub is contacted, so a request for the wrong repository costs no upstream call. + pub async fn authorize_human( + &self, + github_app: &GithubAppConfig, + repo: &str, + bearer_token: Option<&str>, + ) -> Result<(TokenScope, HumanIdentity), AppError> { + // `--disable-auth` is a local development affordance for the workload path. Honouring it + // here would turn the human route into an unauthenticated token minter, so it does not + // apply: the human path has no unauthenticated mode. + let bearer_token = bearer_token + .ok_or_else(|| AppError::Unauthorized("missing Authorization header".to_string()))?; + + let Some(human_policy) = self.human_policies.iter().find(|human_policy| { + human_policy.github_app == github_app.name && human_policy.repository == repo + }) else { + debug!( + github_app = %github_app.name, + repo = %repo, + "no human-policy names this github-app and repository" + ); + return Err(AppError::Unauthorized(format!( + "no human-policy authorizes github-app '{}' for repository '{}'", + github_app.name, repo + ))); + }; + + let human_role = self + .human_roles + .get(&human_policy.human_role) + .ok_or_else(|| { + // Startup validation rejects this, so reaching it means the state was built by + // some path that skipped validation. + AppError::Internal(format!( + "human-policy references unknown human-role '{}'", + human_policy.human_role + )) + })?; + + let claims = human_role + .validator + .validate( + bearer_token, + &human_role.config.roles_claim, + &human_role.config.teleport_role, + &human_role.config.required_claims, + ) + .await + .map_err(|error| match error { + HumanTokenError::KeysUnavailable => AppError::Internal( + "Teleport validation keys could not be retrieved".to_string(), + ), + error => { + debug!( + github_app = %github_app.name, + repo = %repo, + human_role = %human_policy.human_role, + error = %error, + "human token rejected" + ); + AppError::Unauthorized(format!( + "token did not satisfy human-role '{}' for repository '{}'", + human_policy.human_role, repo + )) + } + })?; + + let identity = HumanIdentity { + subject: claims.subject().to_string(), + request_id: human_role + .config + .request_id_claim + .as_deref() + .and_then(|claim| claims.claim_str(claim)) + .map(str::to_string), + human_role: human_policy.human_role.clone(), + }; + + Ok(( + TokenScope { + repositories: RepoScope::OnlyRequested, + permissions: human_policy.permissions.clone(), + }, + identity, + )) + } + pub fn signer(&self, secret_key: &str) -> anyhow::Result> { match self.key_source { KeySource::Local => { @@ -235,6 +333,80 @@ impl AppState { } } +/// Who a token is being minted for. This is part of the installation-token cache key, so a token +/// minted for one person is never handed to another, and a person's token is never served from +/// the shared workload entry. +#[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)] +pub enum Requester { + /// A workload identity. These legitimately share a cache entry: the token they receive depends + /// only on the App, repository and scope, and no person is accountable for it. + Workload, + /// A person. Partitioned by validated subject *and* the access request that authorised them, + /// so a second approval for the same person yields a fresh token rather than reviving the one + /// issued under the previous approval. + Person { + subject: String, + request_id: Option, + }, +} + +impl Requester { + pub fn person(identity: &HumanIdentity) -> Self { + Self::Person { + subject: identity.subject.clone(), + request_id: identity.request_id.clone(), + } + } +} + +/// A configured Teleport issuer together with the validator built for it. The validator holds the +/// JWKS cache, so it is built once at startup rather than per request. +#[derive(Clone)] +pub struct HumanRole { + pub config: HumanRoleConfig, + pub validator: HumanTokenValidator, +} + +/// Who idcat believes is asking, carried from authorization through to the token request so that +/// the issuance record names a person and the token cache cannot be shared between people. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct HumanIdentity { + /// The validated `sub` of the Teleport token. + pub subject: String, + /// The Teleport access request or session the approval came from, when the issuer supplies it. + pub request_id: Option, + /// The `[[human-role]]` that authorised this request. + pub human_role: String, +} + +fn build_human_roles( + human_roles: &[HumanRoleConfig], +) -> anyhow::Result> { + human_roles + .iter() + .map(|config| { + let algorithms = config + .algorithms + .iter() + .map(|algorithm| algorithm.to_algorithm()) + .collect(); + let validator = HumanTokenValidator::from_http( + &config.issuer, + &config.audience, + &config.jwks_url, + algorithms, + )?; + Ok(( + config.name.clone(), + HumanRole { + config: config.clone(), + validator, + }, + )) + }) + .collect() +} + fn claims_for_request( installation_policy: &InstallationPolicyConfig, request_repo: &str, @@ -354,6 +526,8 @@ mod tests { github_apps: Arc::new(github_apps), installation_policies: Arc::new(installation_policies), owner_policies: Arc::new(owner_policies), + human_policies: Arc::new(Vec::new()), + human_roles: Arc::new(BTreeMap::new()), token_validator: TokenValidator::new(Vec::new(), true).unwrap(), github: GithubClient::new().unwrap(), webhook_publisher: None, @@ -820,3 +994,402 @@ mod tests { assert!(matches!(error, AppError::Unauthorized(_))); } } + +/// The pilot's human token path: one exact repository, one exact Teleport role, no fallback. +/// +/// Every token here is synthetic and signed with a key generated in-process. Nothing in this +/// module may be given a real Teleport token. +#[cfg(test)] +mod human_tests { + use super::*; + use crate::config::{HumanJwtAlgorithm, HumanPolicyConfig, HumanRoleConfig}; + use crate::human::HumanTokenValidator; + use crate::secret::FilePrivateKeyStore; + use crate::testkeys::{TeleportClaims, other_keys, test_keys}; + use serde_json::json; + + const ISSUER: &str = "https://teleport.example.invalid"; + const AUDIENCE: &str = "https://idcat.teleport.example.invalid:443"; + const TELEPORT_ROLE: &str = "github-app-token-pilot-repo"; + const HUMAN_ROLE: &str = "pilot-repo-reader"; + const APP: &str = "source-reader"; + const PILOT_REPO: &str = "myorg/pilot"; + const OTHER_REPO: &str = "myorg/other"; + const REQUEST_ID_CLAIM: &str = "teleport_request_id"; + + fn human_role_config() -> HumanRoleConfig { + HumanRoleConfig { + name: HUMAN_ROLE.to_string(), + issuer: ISSUER.to_string(), + audience: AUDIENCE.to_string(), + jwks_url: format!("{ISSUER}/.well-known/jwks.json"), + teleport_role: TELEPORT_ROLE.to_string(), + roles_claim: "roles".to_string(), + algorithms: vec![HumanJwtAlgorithm::Rs256], + request_id_claim: Some(REQUEST_ID_CLAIM.to_string()), + required_claims: BTreeMap::new(), + } + } + + fn human_policy(repository: &str, permissions: &[(&str, &str)]) -> HumanPolicyConfig { + HumanPolicyConfig { + github_app: APP.to_string(), + repository: repository.to_string(), + human_role: HUMAN_ROLE.to_string(), + permissions: permissions + .iter() + .map(|(name, value)| (name.to_string(), value.to_string())) + .collect(), + } + } + + fn github_app(allowed_roles: Vec) -> GithubAppConfig { + GithubAppConfig { + name: APP.to_string(), + app_id: 42, + secret_key: "private-key.pem".to_string(), + webhook_target: None, + webhook_validation_secret_file: None, + allowed_roles, + } + } + + /// Builds state whose human role validates against an in-process JWKS, so no test reaches the + /// network. + fn state(policies: Vec, app: GithubAppConfig) -> AppState { + let config = human_role_config(); + let validator = HumanTokenValidator::from_static_jwks( + &config.issuer, + &config.audience, + test_keys().jwks.clone(), + vec![jsonwebtoken::Algorithm::RS256], + ); + AppState { + github_apps: Arc::new(vec![app]), + installation_policies: Arc::new(Vec::new()), + owner_policies: Arc::new(Vec::new()), + human_policies: Arc::new(policies), + human_roles: Arc::new(BTreeMap::from([( + HUMAN_ROLE.to_string(), + HumanRole { config, validator }, + )])), + token_validator: TokenValidator::new(Vec::new(), true).unwrap(), + github: GithubClient::new().unwrap(), + webhook_publisher: None, + key_source: KeySource::Local, + private_key_store: FilePrivateKeyStore::new("/var/run/secrets/idcat"), + #[cfg(feature = "kms")] + kms_signers: None, + } + } + + fn default_state() -> AppState { + state( + vec![human_policy(PILOT_REPO, &[("contents", "read")])], + github_app(Vec::new()), + ) + } + + fn claims(subject: &str) -> TeleportClaims { + let mut claims = TeleportClaims::new(subject, ISSUER, AUDIENCE, json!([TELEPORT_ROLE])); + claims.traits = None; + claims + } + + /// A Teleport token with a request id, matching what the Application Service issues after an + /// approved access request. + fn approved_token(subject: &str, request_id: &str) -> String { + let claims = claims(subject); + let mut value = serde_json::to_value(&claims).unwrap(); + value[REQUEST_ID_CLAIM] = json!(request_id); + test_keys().sign(&value) + } + + async fn authorize( + state: &AppState, + repo: &str, + token: &str, + ) -> Result<(TokenScope, HumanIdentity), AppError> { + let app = state.github_app(APP).unwrap().clone(); + state.authorize_human(&app, repo, Some(token)).await + } + + #[tokio::test] + async fn exact_person_role_app_and_repository_is_authorized() { + let state = default_state(); + let token = approved_token("alex.mason@example.invalid", "req-1"); + + let (scope, identity) = authorize(&state, PILOT_REPO, &token).await.unwrap(); + + assert_eq!(scope.repositories, RepoScope::OnlyRequested); + assert_eq!( + scope.permissions, + BTreeMap::from([("contents".to_string(), "read".to_string())]) + ); + assert_eq!(identity.subject, "alex.mason@example.invalid"); + assert_eq!(identity.request_id.as_deref(), Some("req-1")); + assert_eq!(identity.human_role, HUMAN_ROLE); + } + + #[tokio::test] + async fn right_person_and_role_but_wrong_repository_is_denied() { + let state = default_state(); + let token = approved_token("alex.mason@example.invalid", "req-1"); + + let error = authorize(&state, OTHER_REPO, &token).await.unwrap_err(); + + // The policy lookup fails before the token is validated and long before GitHub is asked + // for anything, so this denial costs no upstream call. + assert!( + matches!(&error, AppError::Unauthorized(message) + if message.contains("no human-policy authorizes")), + "expected an unauthorized policy-lookup failure, got {error:?}" + ); + } + + #[tokio::test] + async fn approval_for_one_repository_cannot_mint_for_a_renamed_repository() { + let state = default_state(); + let token = approved_token("alex.mason@example.invalid", "req-1"); + + assert!( + authorize(&state, "myorg/pilot-renamed", &token) + .await + .is_err() + ); + assert!(authorize(&state, "myorg/Pilot", &token).await.is_err()); + assert!(authorize(&state, "otherorg/pilot", &token).await.is_err()); + } + + #[tokio::test] + async fn the_minted_scope_is_the_policy_permissions_not_the_installation() { + let state = default_state(); + let token = approved_token("alex.mason@example.invalid", "req-1"); + + let (scope, _) = authorize(&state, PILOT_REPO, &token).await.unwrap(); + + assert_eq!(scope.repositories, RepoScope::OnlyRequested); + assert!( + !scope.permissions.contains_key("administration"), + "a human token must carry only the permissions its policy names" + ); + assert_eq!(scope.permissions.len(), 1); + } + + #[tokio::test] + async fn a_request_through_a_different_github_app_is_denied() { + let other_app = GithubAppConfig { + name: "another-app".to_string(), + ..github_app(Vec::new()) + }; + let state = AppState { + github_apps: Arc::new(vec![github_app(Vec::new()), other_app]), + ..default_state() + }; + let token = approved_token("alex.mason@example.invalid", "req-1"); + let app = state.github_app("another-app").unwrap().clone(); + + assert!( + state + .authorize_human(&app, PILOT_REPO, Some(&token)) + .await + .is_err(), + "the policy names one App; another App installation must not satisfy it" + ); + } + + #[tokio::test] + async fn a_token_without_the_exact_teleport_role_is_denied() { + let state = default_state(); + let mut raw = claims("alex.mason@example.invalid"); + raw.roles = json!(["some-other-role"]); + let token = test_keys().sign(&raw); + + assert!(authorize(&state, PILOT_REPO, &token).await.is_err()); + } + + #[tokio::test] + async fn a_wrong_audience_issuer_signature_or_expiry_is_denied() { + let state = default_state(); + let cases: Vec<(&str, String)> = vec![ + ("wrong audience", { + let mut raw = claims("a@example.invalid"); + raw.aud = "https://idcat.example.invalid".to_string(); + test_keys().sign(&raw) + }), + ("wrong issuer", { + let mut raw = claims("a@example.invalid"); + raw.iss = "https://teleport.other.invalid".to_string(); + test_keys().sign(&raw) + }), + ( + "wrong signature", + other_keys().sign(&claims("a@example.invalid")), + ), + ("expired", { + let mut raw = claims("a@example.invalid"); + raw.exp = crate::testkeys::now() - 1; + test_keys().sign(&raw) + }), + ]; + + for (case, token) in cases { + let error = authorize(&state, PILOT_REPO, &token).await.unwrap_err(); + assert!( + matches!(error, AppError::Unauthorized(_)), + "{case} must be refused outright, not retried against another path" + ); + } + } + + #[tokio::test] + async fn a_missing_authorization_header_is_denied() { + let state = default_state(); + let app = state.github_app(APP).unwrap().clone(); + + assert!(state.authorize_human(&app, PILOT_REPO, None).await.is_err()); + } + + #[tokio::test] + async fn the_human_path_never_falls_back_to_allowed_roles() { + // The App grants a workload role a broad token. A human request that fails its policy must + // not be rescued by that path. + let state = state( + vec![human_policy(PILOT_REPO, &[("contents", "read")])], + github_app(vec!["some-workload-role".to_string()]), + ); + let mut raw = claims("alex.mason@example.invalid"); + raw.roles = json!(["not-the-pilot-role"]); + let token = test_keys().sign(&raw); + + let error = authorize(&state, PILOT_REPO, &token).await.unwrap_err(); + + assert!(matches!(error, AppError::Unauthorized(_))); + } + + #[tokio::test] + async fn a_wrong_repository_is_denied_even_when_the_app_has_broad_allowed_roles() { + // The lookup-miss branch must not be rescued by `allowed-roles` either. Without this the + // fail-closed property only holds for Apps that happen to have no workload roles. + let state = state( + vec![human_policy(PILOT_REPO, &[("contents", "read")])], + github_app(vec!["some-workload-role".to_string()]), + ); + let token = approved_token("alex.mason@example.invalid", "req-1"); + + let error = authorize(&state, OTHER_REPO, &token).await.unwrap_err(); + + assert!( + matches!(&error, AppError::Unauthorized(message) + if message.contains("no human-policy authorizes")), + "expected a fail-closed denial, got {error:?}" + ); + } + + #[tokio::test] + async fn an_unknown_repository_is_denied_when_no_human_policy_exists_at_all() { + let state = state( + Vec::new(), + github_app(vec!["some-workload-role".to_string()]), + ); + let token = approved_token("alex.mason@example.invalid", "req-1"); + + assert!(authorize(&state, PILOT_REPO, &token).await.is_err()); + } + + #[tokio::test] + async fn a_workload_token_cannot_satisfy_a_human_policy() { + let state = default_state(); + // A GitHub Actions style HS256 token, of the shape the workload path accepts. + let workload_token = jsonwebtoken::encode( + &jsonwebtoken::Header::new(jsonwebtoken::Algorithm::HS256), + &json!({ + "sub": "repo:myorg/pilot:ref:refs/heads/main", + "aud": "idcat", + "iss": "https://token.actions.githubusercontent.com", + "exp": 4_102_444_800u64, + "roles": [TELEPORT_ROLE], + }), + &jsonwebtoken::EncodingKey::from_secret(b"secret"), + ) + .unwrap(); + + assert!( + authorize(&state, PILOT_REPO, &workload_token) + .await + .is_err(), + "a workload token must not reach the human path even when it spells the right role" + ); + } + + #[tokio::test] + async fn a_human_token_cannot_satisfy_a_workload_policy() { + let mut state = default_state(); + state.token_validator = TokenValidator::new( + vec![authzoo::RoleConfig { + name: "github-workflow".to_string(), + audience: "idcat".to_string(), + issuer: "https://token.actions.githubusercontent.com".to_string(), + validation_key: Some("secret".to_string()), + algorithms: vec![authzoo::JwtAlgorithm::Hs256], + claims: BTreeMap::new(), + }], + false, + ) + .unwrap(); + let app = GithubAppConfig { + allowed_roles: vec!["github-workflow".to_string()], + ..github_app(Vec::new()) + }; + let token = approved_token("alex.mason@example.invalid", "req-1"); + + assert!( + state + .authorize_github_app(&app, PILOT_REPO, Some(&token)) + .is_err(), + "a Teleport human token must not satisfy a workload allowed-role" + ); + } + + #[tokio::test] + async fn two_people_are_given_distinct_cache_partitions() { + let state = default_state(); + let first = approved_token("first@example.invalid", "req-1"); + let second = approved_token("second@example.invalid", "req-2"); + + let (_, first_identity) = authorize(&state, PILOT_REPO, &first).await.unwrap(); + let (_, second_identity) = authorize(&state, PILOT_REPO, &second).await.unwrap(); + + assert_ne!( + Requester::person(&first_identity), + Requester::person(&second_identity), + "two people must never share an installation-token cache entry" + ); + } + + #[tokio::test] + async fn a_second_approval_for_the_same_person_is_a_new_cache_partition() { + let state = default_state(); + let first = approved_token("alex.mason@example.invalid", "req-1"); + let second = approved_token("alex.mason@example.invalid", "req-2"); + + let (_, first_identity) = authorize(&state, PILOT_REPO, &first).await.unwrap(); + let (_, second_identity) = authorize(&state, PILOT_REPO, &second).await.unwrap(); + + assert_ne!( + Requester::person(&first_identity), + Requester::person(&second_identity), + "a fresh approval must not revive the token issued under the previous one" + ); + } + + #[tokio::test] + async fn a_person_is_never_served_the_shared_workload_partition() { + let state = default_state(); + let token = approved_token("alex.mason@example.invalid", "req-1"); + + let (_, identity) = authorize(&state, PILOT_REPO, &token).await.unwrap(); + + assert_ne!(Requester::person(&identity), Requester::Workload); + } +} diff --git a/src/testkeys.rs b/src/testkeys.rs new file mode 100644 index 0000000..7bd014e --- /dev/null +++ b/src/testkeys.rs @@ -0,0 +1,120 @@ +// SPDX-License-Identifier: MIT +// SPDX-FileCopyrightText: The idcat contributors + +//! Locally generated RSA keys and synthetic signed JWTs for tests. +//! +//! Nothing here is a credential: the key pair is generated in-process, lives only for the lifetime +//! of the test binary, and is never written to disk. Tests must never be given a real Teleport +//! token, GitHub token or App private key. + +use base64::{Engine, engine::general_purpose::URL_SAFE_NO_PAD}; +use jsonwebtoken::jwk::JwkSet; +use jsonwebtoken::{Algorithm, EncodingKey, Header}; +use rsa::pkcs8::{EncodePrivateKey, LineEnding}; +use rsa::traits::PublicKeyParts; +use rsa::{RsaPrivateKey, rand_core::OsRng}; +use serde::Serialize; +use std::sync::{Arc, OnceLock}; +use std::time::{SystemTime, UNIX_EPOCH}; + +pub const TEST_KID: &str = "idcat-test-key"; + +/// A generated key pair, together with the JWKS a validator would fetch for it. +pub struct TestKeyPair { + pub private_key_pem: String, + pub jwks: JwkSet, +} + +/// Generating a 2048-bit key is the slowest thing in the test suite, so do it once per binary. +fn shared() -> &'static Arc { + static SHARED: OnceLock> = OnceLock::new(); + SHARED.get_or_init(|| Arc::new(TestKeyPair::generate(TEST_KID))) +} + +pub fn test_keys() -> Arc { + Arc::clone(shared()) +} + +/// A second, unrelated key pair, for proving that a token signed by the wrong issuer is rejected. +pub fn other_keys() -> Arc { + static OTHER: OnceLock> = OnceLock::new(); + Arc::clone(OTHER.get_or_init(|| Arc::new(TestKeyPair::generate(TEST_KID)))) +} + +impl TestKeyPair { + pub fn generate(kid: &str) -> Self { + let private_key = + RsaPrivateKey::new(&mut OsRng, 2048).expect("test RSA key generation must succeed"); + let private_key_pem = private_key + .to_pkcs8_pem(LineEnding::LF) + .expect("test RSA key must encode as PKCS#8 PEM") + .to_string(); + + let modulus = URL_SAFE_NO_PAD.encode(private_key.n().to_bytes_be()); + let exponent = URL_SAFE_NO_PAD.encode(private_key.e().to_bytes_be()); + let jwks = serde_json::from_value(serde_json::json!({ + "keys": [{ + "kty": "RSA", + "use": "sig", + "alg": "RS256", + "kid": kid, + "n": modulus, + "e": exponent, + }] + })) + .expect("generated JWKS must parse"); + + Self { + private_key_pem, + jwks, + } + } + + pub fn encoding_key(&self) -> EncodingKey { + EncodingKey::from_rsa_pem(self.private_key_pem.as_bytes()) + .expect("generated PEM must parse as an RSA signing key") + } + + /// Signs `claims` as an RS256 JWT carrying [`TEST_KID`]. + pub fn sign(&self, claims: &impl Serialize) -> String { + let mut header = Header::new(Algorithm::RS256); + header.kid = Some(TEST_KID.to_string()); + jsonwebtoken::encode(&header, claims, &self.encoding_key()) + .expect("synthetic test token must encode") + } +} + +pub fn now() -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("system clock must be after the unix epoch") + .as_secs() +} + +/// The claim shape of a Teleport application JWT: `roles` is an array, and `sub` names the person. +#[derive(Debug, Serialize)] +pub struct TeleportClaims { + pub sub: String, + pub iss: String, + pub aud: String, + pub exp: u64, + pub nbf: u64, + pub roles: serde_json::Value, + #[serde(skip_serializing_if = "Option::is_none")] + pub traits: Option, +} + +impl TeleportClaims { + pub fn new(sub: &str, issuer: &str, audience: &str, roles: serde_json::Value) -> Self { + let now = now(); + Self { + sub: sub.to_string(), + iss: issuer.to_string(), + aud: audience.to_string(), + exp: now + 3600, + nbf: now - 5, + roles, + traits: None, + } + } +}