From 4ce2be789de9ba1b9a39de3c310ae9fa35fef5f7 Mon Sep 17 00:00:00 2001 From: Lewis Date: Mon, 27 Jul 2026 08:34:07 +0000 Subject: [PATCH] knot2: serve legacy basic-auth admin endpoint if secret set Lewis: May this revision serve well! --- Cargo.lock | 2 ++ Cargo.toml | 1 + knot2/example.toml | 3 +++ knot2/justfile | 25 +++++++------------------ knot2/crates/knot-xrpc/Cargo.toml | 2 ++ knot2/crates/knot-config/src/lib.rs | 28 +++++++++++++++++++++++++++- knot2/crates/knot-lexicons/src/lib.rs | 11 +---------- knot2/crates/knot-server/src/main.rs | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++--- knot2/crates/knot-xrpc/src/legacy_admin.rs | 147 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ knot2/crates/knot-xrpc/src/lib.rs | 39 +++++++++++++++++++++++++++++++++++---- knot2/crates/knot-xrpc/src/members.rs | 32 +++++++++++++++++++++----------- knot2/crates/knot-xrpc/src/tests.rs | 149 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 12 file(s) changed, 446 insertion(s)(+), 47 deletion(s)(-) diff --git a/Cargo.lock b/Cargo.lock --- a/Cargo.lock +++ b/Cargo.lock @@ -4967,6 +4967,7 @@ "serde", "serde_json", "sha2 0.11.0", + "subtle", "tempfile", "thiserror 2.0.18", "tokio", @@ -4977,6 +4978,7 @@ "tower-http 0.7.0", "tracing", "url", + "zeroize", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml --- a/Cargo.toml +++ b/Cargo.toml @@ -144,6 +144,7 @@ base64 = "0.22" bs58 = "0.5" zeroize = { version = "1", features = ["derive"] } +subtle = "2.6" wiremock = "0.6" tempfile = "3" diff --git a/knot2/example.toml b/knot2/example.toml --- a/knot2/example.toml +++ b/knot2/example.toml @@ -107,6 +107,9 @@ # Default value: "closed" #admission = "closed" +# Can also be specified via environment variable `KNOT_LEGACY_ADMIN_SECRET_ENV`. +#legacy_admin_secret_env = + [repo] # Can also be specified via environment variable `KNOT_SCAN_PATH`. # Required! This value must be specified. diff --git a/knot2/justfile b/knot2/justfile --- a/knot2/justfile +++ b/knot2/justfile @@ -7,10 +7,10 @@ cargo run -p knot-server -- config-template > example.toml fmt: - cargo fmt + cargo fmt --all fmt-check: - cargo fmt --check + cargo fmt --all --check clippy: cargo clippy -p 'knot-*' --all-targets -- -D warnings @@ -45,7 +45,7 @@ ci: fmt-check clippy test gates bench-gate fuzz-ci -gates: gate-no-subprocess gate-no-sql gate-no-native-git gate-no-string-ids gate-no-unguarded-receive gate-fuzz-targets-enumerated +gates: gate-no-subprocess (gate-no-banned-deps "no-sql" "an embedded database" "rusqlite|libsqlite3-sys|sqlx|sqlx-core|sled|fjall|redb") (gate-no-banned-deps "no-native-git" "a native git or TLS shim" "git2|libgit2-sys|openssl-sys|zlib-ng|zlib-ng-sys") gate-no-string-ids gate-no-unguarded-receive gate-fuzz-targets-enumerated gate-no-subprocess: #!/usr/bin/env bash @@ -58,27 +58,16 @@ fi echo "ok: no process spawning in server source" -gate-no-sql: +gate-no-banned-deps name subject pattern: #!/usr/bin/env bash set -euo pipefail - hits=$(grep -inE '^name = "(rusqlite|libsqlite3-sys|sqlx|sqlx-core|sled|fjall|redb)"' ../Cargo.lock || true) + hits=$(cargo tree -p knot-server --edges normal,build --prefix none | sort -u | grep -iE '^({{pattern}}) v' || true) if [ -n "$hits" ]; then - echo "no-sql gate failed: an embedded database is in the dependency tree" >&2 + echo "{{name}} gate failed: {{subject}} is in the knot-server dependency tree" >&2 echo "$hits" >&2 exit 1 fi - echo "ok: no embedded database in the dependency tree" - -gate-no-native-git: - #!/usr/bin/env bash - set -euo pipefail - hits=$(grep -inE '^name = "(git2|libgit2-sys|openssl-sys|zlib-ng|zlib-ng-sys)"' ../Cargo.lock || true) - if [ -n "$hits" ]; then - echo "no-native-git gate failed: a native git or TLS shim is in the dependency tree" >&2 - echo "$hits" >&2 - exit 1 - fi - echo "ok: no native git or TLS shim in the dependency tree" + echo "ok: {{subject}} isn't in the knot-server dependency tree" gate-no-string-ids: #!/usr/bin/env bash diff --git a/knot2/crates/knot-xrpc/Cargo.toml b/knot2/crates/knot-xrpc/Cargo.toml --- a/knot2/crates/knot-xrpc/Cargo.toml +++ b/knot2/crates/knot-xrpc/Cargo.toml @@ -43,6 +43,8 @@ chrono = { workspace = true } tempfile = { workspace = true } sha2 = { workspace = true } +subtle = { workspace = true } +zeroize = { workspace = true } url = { workspace = true } [dev-dependencies] diff --git a/knot2/crates/knot-config/src/lib.rs b/knot2/crates/knot-config/src/lib.rs --- a/knot2/crates/knot-config/src/lib.rs +++ b/knot2/crates/knot-config/src/lib.rs @@ -53,6 +53,9 @@ pub struct AclConfig { #[config(env = "KNOT_ADMISSION", default = "closed")] pub admission: AdmissionPolicy, + + #[config(env = "KNOT_LEGACY_ADMIN_SECRET_ENV")] + pub legacy_admin_secret_env: Option, } #[derive(Debug, Config)] @@ -800,6 +803,14 @@ .as_ref() .filter(|header| !is_http_token(header)) .map(|_| "xrpc.trusted_proxy_header isn't valid HTTP header name".to_string()), + self.acl + .legacy_admin_secret_env + .as_deref() + .filter(|name| !is_env_var_name(name)) + .map(|_| { + "acl.legacy_admin_secret_env must be valid environment variable name" + .to_string() + }), match self.homepage.source() { HomepageSource::File(path) if !path.is_absolute() => { Some("homepage.path must be absolute path".to_string()) @@ -1094,7 +1105,7 @@ assert_eq!( template(), include_str!("../../../example.toml"), - "regenerate example.toml from knot_config::template() after changing config" + "regenerate example.toml with `just gen-config` after changing config" ); } @@ -1133,6 +1144,7 @@ }, acl: AclConfig { admission: AdmissionPolicy::Closed, + legacy_admin_secret_env: None, }, repo: RepoConfig { scan_path: PathBuf::from("/srv/git"), @@ -1266,6 +1278,15 @@ false, false, ), + ( + "a_legacy_admin_secret_env_var", + |config| { + config.acl.legacy_admin_secret_env = + Some("KNOT_LEGACY_ADMIN_SECRET".to_string()) + }, + false, + false, + ), ]; cases .iter() @@ -1346,6 +1367,11 @@ "empty_admin_list", |config| config.server.admins = Vec::new(), "admins", + ), + ( + "a_malformed_legacy_admin_secret_env_var_name", + |config| config.acl.legacy_admin_secret_env = Some("9_NOT_A_VAR".to_string()), + "acl.legacy_admin_secret_env", ), ( "a_zero_maintenance_interval", diff --git a/knot2/crates/knot-lexicons/src/lib.rs b/knot2/crates/knot-lexicons/src/lib.rs --- a/knot2/crates/knot-lexicons/src/lib.rs +++ b/knot2/crates/knot-lexicons/src/lib.rs @@ -2,16 +2,7 @@ #[path = "_lex/lib.rs"] #[allow(non_snake_case, unused_imports, unused_extern_crates)] -#[allow( - clippy::absurd_extreme_comparisons, - clippy::collapsible_if, - clippy::manual_strip, - clippy::needless_update, - clippy::new_ret_no_self, - clippy::new_without_default, - clippy::should_implement_trait, - clippy::type_complexity -)] +#[allow(clippy::all)] #[rustfmt::skip] mod _lex; diff --git a/knot2/crates/knot-server/src/main.rs b/knot2/crates/knot-server/src/main.rs --- a/knot2/crates/knot-server/src/main.rs +++ b/knot2/crates/knot-server/src/main.rs @@ -74,15 +74,34 @@ .init(); } +fn subcommand(name: &str) -> Option> { + match name { + "config-template" => { + print!("{}", knot_config::template()); + Some(Ok(())) + } + "validate" => Some( + knot_config::load(std::env::args().nth(2).map(PathBuf::from).as_deref()) + .context("load configuration") + .and_then(|config| { + config + .verify_environment() + .context("verify runtime environment") + }) + .map(|()| println!("configuration is valid")), + ), + _ => None, + } +} + #[tokio::main] async fn main() -> anyhow::Result<()> { #[cfg(target_os = "linux")] rustix::process::set_dumpable_behavior(rustix::process::DumpableBehavior::NotDumpable) .context("disable core dumps and ptrace attachment")?; - if std::env::args().nth(1).as_deref() == Some("config-template") { - print!("{}", knot_config::template()); - return Ok(()); + if let Some(result) = std::env::args().nth(1).as_deref().and_then(subcommand) { + return result; } init_tracing(); @@ -414,6 +433,7 @@ let catalog = Arc::new( knot_messages::Catalog::parse(&config.messages).context("parse message templates")?, ); + let legacy_admin = legacy_admin_secret(&config); knot_config::init(config); @@ -526,6 +546,13 @@ xrpc_state.knot_hostname.clone(), Arc::new(SystemClock), ); + let legacy_admin_routes = legacy_admin.map(|secret| { + tracing::warn!( + route = knot_xrpc::legacy_admin::ADD_MEMBER_ROUTE, + "serving the legacy basic-auth admin route" + ); + knot_xrpc::legacy_admin::router(Arc::clone(&xrpc_state), secret) + }); let base_router = write_routes.merge(knot_xrpc::router(xrpc_state)).route( "/.well-known/did.json", get(move || { @@ -533,6 +560,10 @@ async move { Json(document) } }), ); + let base_router = match legacy_admin_routes { + Some(routes) => base_router.merge(routes), + None => base_router, + }; let base_router = match homepage { HomepageSource::Disabled => base_router, HomepageSource::Default => base_router.route("/", get(|| async { Html(DEFAULT_HOMEPAGE) })), @@ -622,6 +653,23 @@ Edge(Result, tokio::task::JoinError>), Ssh(Result, tokio::task::JoinError>), Signal, +} + +fn legacy_admin_secret( + config: &knot_config::Validated, +) -> Option { + let name = config.acl.legacy_admin_secret_env.as_deref()?; + let value = zeroize::Zeroizing::new(std::env::var(name).unwrap_or_default()); + match knot_xrpc::legacy_admin::LegacyAdminSecret::new(&value) { + Ok(secret) => Some(secret), + Err(_) => { + tracing::warn!( + secret_env = name, + "the environment variable in acl.legacy_admin_secret_env is unset or empty, so we won't serve the admin route" + ); + None + } + } } fn build_tls_setup( diff --git a/knot2/crates/knot-xrpc/src/legacy_admin.rs b/knot2/crates/knot-xrpc/src/legacy_admin.rs new file mode 100644 --- /dev/null +++ b/knot2/crates/knot-xrpc/src/legacy_admin.rs @@ -0,0 +1,147 @@ +use std::sync::Arc; + +use axum::Router; +use axum::body::Bytes; +use axum::extract::{DefaultBodyLimit, State}; +use axum::middleware::from_fn_with_state; +use axum::response::Response; +use axum::routing::post; +use http::HeaderMap; +use subtle::ConstantTimeEq; +use zeroize::Zeroizing; + +use knot_cobs::Grant; +use knot_runtime::{Clock, HttpTransport}; + +use crate::error::XrpcError; +use crate::members::{SubjectInput, grant_membership}; +use crate::{XrpcState, basic_credentials, decode, enforce_pre_auth_limit}; + +pub const ADD_MEMBER_ROUTE: &str = "/admin/addMember"; + +const BASIC_USER: &str = "admin"; + +#[derive(Debug, thiserror::Error)] +#[error("legacy admin secret mustn't be empty")] +pub struct EmptySecret; + +pub struct LegacyAdminSecret(Zeroizing); + +impl LegacyAdminSecret { + pub fn new(value: &str) -> Result { + let value = value.trim(); + match value.is_empty() { + true => Err(EmptySecret), + false => Ok(Self(Zeroizing::new(value.to_string()))), + } + } + + fn authorize(&self, headers: &HeaderMap) -> Result<(), XrpcError> { + let denied = || XrpcError::auth_required("invalid admin credentials"); + let credentials = headers + .get(http::header::AUTHORIZATION) + .and_then(|value| value.to_str().ok()) + .and_then(basic_credentials) + .ok_or_else(denied)?; + let admitted = credentials.user.matches(BASIC_USER) + && bool::from(credentials.password.as_bytes().ct_eq(self.0.as_bytes())); + match admitted { + true => Ok(()), + false => Err(denied()), + } + } +} + +struct LegacyAdmin { + state: Arc>, + secret: LegacyAdminSecret, +} + +pub fn router( + state: Arc>, + secret: LegacyAdminSecret, +) -> Router { + let limits = state.byte_limits.body.get(); + let limiter = Arc::clone(&state); + Router::new() + .route(ADD_MEMBER_ROUTE, post(add_member::)) + .layer(DefaultBodyLimit::max(limits)) + .layer(from_fn_with_state(limiter, enforce_pre_auth_limit::)) + .with_state(Arc::new(LegacyAdmin { state, secret })) +} + +async fn add_member( + State(admin): State>>, + headers: HeaderMap, + body: Bytes, +) -> Result { + admin.secret.authorize(&headers)?; + let SubjectInput { subject } = decode(&body)?; + tracing::warn!( + route = ADD_MEMBER_ROUTE, + %subject, + "legacy admin route authorized a member grant" + ); + grant_membership( + &admin.state, + Grant { + subject, + added_by: admin.state.service_owner.clone(), + created_at: admin.state.now(), + }, + ) + .await +} + +#[cfg(test)] +mod tests { + use super::*; + + use base64::Engine; + + fn header(value: &str) -> HeaderMap { + let mut headers = HeaderMap::new(); + headers.insert( + http::header::AUTHORIZATION, + value.parse().expect("header is ascii"), + ); + headers + } + + fn basic(scheme: &str, user: &str, password: &str) -> HeaderMap { + let encoded = base64::engine::general_purpose::STANDARD + .encode(format!("{user}:{password}").as_bytes()); + header(&format!("{scheme} {encoded}")) + } + + #[test] + fn a_trimmed_secret_admits_only_the_admin_user_sending_it_exactly() { + assert!(LegacyAdminSecret::new(" ").is_err()); + let secret = LegacyAdminSecret::new("\tnekomilk2\n").expect("secret is non-empty"); + + let admitted = + ["Basic", "basic", "BASIC"].map(|scheme| basic(scheme, "admin", "nekomilk2")); + let refused = [ + basic("Basic", "admin", "\tnekomilk2\n"), + basic("Basic", "admin", "nope"), + basic("Basic", "root", "nekomilk2"), + basic("Basic", "admin", ""), + header("Basic !!!not-base64!!!"), + header("Bearer nekomilk2"), + header("nekomilk2"), + HeaderMap::new(), + ]; + assert!( + admitted + .iter() + .all(|headers| secret.authorize(headers).is_ok()), + "authorize admits the trimmed secret under any case of the Basic scheme" + ); + assert!( + refused + .iter() + .all(|headers| secret.authorize(headers).is_err()), + "authorize refuses an untrimmed, wrong, empty or malformed credential" + ); + } +} diff --git a/knot2/crates/knot-xrpc/src/lib.rs b/knot2/crates/knot-xrpc/src/lib.rs --- a/knot2/crates/knot-xrpc/src/lib.rs +++ b/knot2/crates/knot-xrpc/src/lib.rs @@ -6,6 +6,7 @@ mod error; mod events; mod forks; +pub mod legacy_admin; mod lfs; mod lists; mod locks; @@ -329,7 +330,7 @@ .with_state(state) } -async fn enforce_pre_auth_limit( +pub(crate) async fn enforce_pre_auth_limit( State(state): State>>, socket: SocketPeer, request: Request, @@ -392,7 +393,28 @@ .ok_or_else(|| XrpcError::auth_required("missing or malformed Bearer authorization header")) } -fn strip_basic(value: &str) -> Option { +pub(crate) struct BasicUser(String); + +impl BasicUser { + pub(crate) fn matches(&self, expected: &str) -> bool { + self.0 == expected + } +} + +pub(crate) struct BasicPassword(String); + +impl BasicPassword { + pub(crate) fn as_bytes(&self) -> &[u8] { + self.0.as_bytes() + } +} + +pub(crate) struct BasicCredentials { + pub(crate) user: BasicUser, + pub(crate) password: BasicPassword, +} + +pub(crate) fn basic_credentials(value: &str) -> Option { let (scheme, rest) = value.split_once(' ')?; if !scheme.eq_ignore_ascii_case("Basic") { return None; @@ -401,8 +423,17 @@ .decode(rest.trim()) .ok()?; let text = String::from_utf8(decoded).ok()?; - let (_user, password) = text.split_once(':')?; - (!password.is_empty()).then(|| password.to_string()) + let (user, password) = text.split_once(':')?; + Some(BasicCredentials { + user: BasicUser(user.to_string()), + password: BasicPassword(password.to_string()), + }) +} + +fn strip_basic(value: &str) -> Option { + basic_credentials(value) + .map(|credentials| credentials.password.0) + .filter(|password| !password.is_empty()) } fn push_credential(headers: &HeaderMap) -> Result { diff --git a/knot2/crates/knot-xrpc/src/members.rs b/knot2/crates/knot-xrpc/src/members.rs --- a/knot2/crates/knot-xrpc/src/members.rs +++ b/knot2/crates/knot-xrpc/src/members.rs @@ -23,8 +23,8 @@ pub(crate) const REMOVE_ROUTE: &str = "/xrpc/sh.tangled.knot.removeMember"; #[derive(Deserialize)] -struct SubjectInput { - subject: AccountDid, +pub(crate) struct SubjectInput { + pub(crate) subject: AccountDid, } pub(crate) async fn add_member( @@ -40,19 +40,29 @@ } let SubjectInput { subject } = decode(&body)?; - if state.admins.contains(&subject) - || matches!(state.index.is_member(&subject), Resolved::Ready(true)) + grant_membership( + &state, + Grant { + subject, + added_by: actor, + created_at: state.now(), + }, + ) + .await +} + +pub(crate) async fn grant_membership( + state: &Arc>, + grant: Grant, +) -> Result { + if state.admins.contains(&grant.subject) + || matches!(state.index.is_member(&grant.subject), Resolved::Ready(true)) { return Ok(ok_empty()); } - let now = state.now(); - let event_subject = subject.clone(); - let grant = Grant { - subject, - added_by: actor, - created_at: now, - }; + let now = grant.created_at; + let event_subject = grant.subject.clone(); let signer = state.secrets.signer(&state.knot_did)?; let meta_path = state.meta_path.clone(); let index = Arc::clone(&state.index); diff --git a/knot2/crates/knot-xrpc/src/tests.rs b/knot2/crates/knot-xrpc/src/tests.rs --- a/knot2/crates/knot-xrpc/src/tests.rs +++ b/knot2/crates/knot-xrpc/src/tests.rs @@ -3042,3 +3042,152 @@ assert_eq!(fork.find_ref(&main_ref()).unwrap(), Some(new_tip)); } } + +mod legacy_admin_route { + use super::*; + use crate::legacy_admin::{ADD_MEMBER_ROUTE, LegacyAdminSecret}; + use tower::ServiceExt; + + const SECRET: &str = "nekomilk2"; + + async fn call(router: &axum::Router, user: &str, password: &str, subject: &str) -> StatusCode { + let encoded = base64::engine::general_purpose::STANDARD + .encode(format!("{user}:{password}").as_bytes()); + let request = http::Request::builder() + .method("POST") + .uri(ADD_MEMBER_ROUTE) + .header(AUTHORIZATION, format!("Basic {encoded}")) + .header(http::header::CONTENT_TYPE, "application/json") + .body(axum::body::Body::from( + json!({ "subject": subject }).to_string(), + )) + .unwrap(); + router.clone().oneshot(request).await.unwrap().status() + } + + #[tokio::test] + async fn the_legacy_route_admits_a_member_only_with_the_configured_credentials() { + let world = World::new(); + let router = crate::router(Arc::clone(&world.state)).merge(crate::legacy_admin::router( + Arc::clone(&world.state), + LegacyAdminSecret::new(SECRET).unwrap(), + )); + let subject = format!("did:web:{MEMBER_HOST}"); + + let version = http::Request::builder() + .method("GET") + .uri(crate::service::VERSION_ROUTE) + .body(axum::body::Body::empty()) + .unwrap(); + assert_eq!( + router.clone().oneshot(version).await.unwrap().status(), + StatusCode::OK, + "merging the legacy route leaves the xrpc routes reachable" + ); + + let refused = futures::future::join_all( + [("admin", "nope"), ("root", SECRET), ("admin", "")] + .map(|(user, password)| call(&router, user, password, &subject)), + ) + .await; + assert!( + refused + .iter() + .all(|status| *status == StatusCode::UNAUTHORIZED), + "the knot refuses a wrong user or secret, got {refused:?}" + ); + assert_eq!( + call( + &router, + "admin", + SECRET, + &"n".repeat(world.state.byte_limits.body.get() + 1) + ) + .await, + StatusCode::PAYLOAD_TOO_LARGE + ); + assert_eq!( + world.state.index.is_member(&account(MEMBER_HOST)), + Resolved::Ready(false), + "a refused call grants nothing" + ); + + assert_eq!( + call(&router, "admin", SECRET, &subject).await, + StatusCode::OK + ); + assert_eq!( + world.state.index.is_member(&account(MEMBER_HOST)), + Resolved::Ready(true) + ); + let added = last_event(&world, "sh.tangled.knot.memberUpdate"); + assert_eq!(added.payload["op"], "add"); + assert_eq!(added.payload["subject"], account(MEMBER_HOST).to_string()); + let Resolved::Ready(members) = world.state.index.member_entries() else { + panic!("the member roster is warm in this test"); + }; + assert_eq!( + members + .iter() + .find(|grant| grant.subject == account(MEMBER_HOST)) + .expect("the member is in the roster") + .added_by, + world.state.service_owner, + "the legacy grant records the service owner as the granter" + ); + + let baseline = event_count(&world); + assert_eq!( + call(&router, "admin", SECRET, &subject).await, + StatusCode::OK, + "the legacy route is idempotent, matching the Go knot" + ); + assert_eq!( + event_count(&world), + baseline, + "re-adding an existing member emits no event" + ); + } + + #[tokio::test] + async fn the_legacy_route_sheds_a_pre_auth_flood_from_one_peer() { + use axum::extract::ConnectInfo; + use std::net::SocketAddr; + + let world = World::new(); + let router = crate::legacy_admin::router( + Arc::clone(&world.state), + LegacyAdminSecret::new(SECRET).unwrap(), + ); + let peer = SocketAddr::from(([203, 0, 113, 9], 5555)); + + let statuses: Vec = futures::stream::iter(0..22) + .then(|_| { + let router = router.clone(); + async move { + let mut request = http::Request::builder() + .method("POST") + .uri(ADD_MEMBER_ROUTE) + .body(axum::body::Body::empty()) + .unwrap(); + request.extensions_mut().insert(ConnectInfo(peer)); + router.oneshot(request).await.unwrap().status() + } + }) + .collect() + .await; + + assert!( + statuses[..20] + .iter() + .all(|status| *status == StatusCode::UNAUTHORIZED), + "the knot admits the per-peer burst and then fails it on the missing credentials, got {statuses:?}" + ); + assert!( + statuses[20..] + .iter() + .all(|status| *status == StatusCode::TOO_MANY_REQUESTS), + "past the burst the knot sheds the guess flood before it reaches the secret comparison, got {statuses:?}" + ); + } +} -- tangled.sh