diff --git a/docs/openssh-compatibility-status.md b/docs/openssh-compatibility-status.md index 5922eb713..a16e1911b 100644 --- a/docs/openssh-compatibility-status.md +++ b/docs/openssh-compatibility-status.md @@ -85,3 +85,9 @@ Security regression coverage preserves trusted-file-only shell Match execution, The binding API proposal is [russh PR #794](https://github.com/Eugeny/russh/pull/794). Dependency adoption is conditional on upstream merge: only an immutable merged revision from the official upstream may be used temporarily, followed by a crates.io release. An unmerged contributor branch is not an approved dependency. Current code must continue to fail closed on unsupported agent constraints. Remaining control details include complete forwarding-channel enumeration, allocated remote-port replies, multiplexed `-W`, TTY-failure notifications and `~user` expansion. Unix-domain forwarding is implemented; raw mux-v4 interoperability is not. Eager `zlib` is validated; delayed `zlib@openssh.com` is not advertised. Non-pipe/socket character-device stdin still uses the existing blocking adapter; the SCP fix does not claim every stdin source is cancellation-safe. + +### Server security-key authorization + +bssh-server fails closed for `sk-ssh-ed25519@openssh.com` and `sk-ecdsa-sha2-nistp256@openssh.com` user keys: russh does not expose signature UP/UV flags to the authentication callback. An `authorized_keys` line must have `no-touch-required` and must not have `verify-required` to authorize either type. The default touch policy and every verification-required policy are rejected and logged at info level. Ordinary keys are unaffected. This server restriction is distinct from client-side security-key signing and host-key verification described above; [#340](https://github.com/lablup/bssh/issues/340) tracks actual signature-flag enforcement. Security-key user certificates remain out of scope. + +For duplicate user-key entries, the first matching `authorized_keys` line determines the security-key policy. A later `no-touch-required` entry cannot override an earlier matching line that requires touch or verification; unrelated keys are skipped. diff --git a/docs/security.md b/docs/security.md index 26d6f563e..8babfca45 100644 --- a/docs/security.md +++ b/docs/security.md @@ -55,6 +55,14 @@ auth: - Restrict authorized_keys file permissions to 0600 - Regularly rotate keys (annually or when personnel changes) +### Security-key user authentication + +bssh-server currently cannot inspect the user-presence (UP/touch) or user-verification (UV/PIN or biometric) flags of security-key signatures through russh. It therefore rejects `sk-ssh-ed25519@openssh.com` and `sk-ecdsa-sha2-nistp256@openssh.com` user keys by default. A matching `authorized_keys` entry is accepted only if it explicitly contains `no-touch-required` and does not contain `verify-required`. Adding both options still rejects the key. Rejections are logged at info level. + +Use an ordinary key or an SSH server that enforces UP/UV when touch or verification is required. Only use `no-touch-required` when deliberately authorizing authentication without either requirement; it is a relaxation of policy, not a way to enforce touch. Ordinary public keys are unaffected. Signature-flag enforcement is tracked in [#340](https://github.com/lablup/bssh/issues/340); security-key user certificates are outside this support. + +For duplicate user-key entries, the first matching `authorized_keys` line determines the security-key policy. A later `no-touch-required` entry cannot override an earlier matching line that requires touch or verification; unrelated keys are skipped. + ### Password Authentication If password authentication is required, use strong passwords with Argon2id hashing: diff --git a/src/server/auth/publickey.rs b/src/server/auth/publickey.rs index ea3d37244..5d080706e 100644 --- a/src/server/auth/publickey.rs +++ b/src/server/auth/publickey.rs @@ -213,6 +213,12 @@ pub struct AuthKeyOptions { /// Disable agent forwarding pub no_agent_forwarding: bool, + /// Permit security-key signatures without user presence (touch). + pub no_touch_required: bool, + + /// Require security-key user verification (PIN or biometric). + pub verify_required: bool, + /// Disable X11 forwarding pub no_x11_forwarding: bool, } @@ -286,6 +292,24 @@ impl PublicKeyVerifier { // Check if key matches any authorized key for authorized_key in &authorized_keys { if self.keys_match(key, authorized_key) { + // russh verifies the signature but does not expose its UP/UV flags + // to auth_publickey. Never authorize a policy we cannot enforce. + if matches!( + key.algorithm(), + russh::keys::ssh_key::Algorithm::SkEd25519 + | russh::keys::ssh_key::Algorithm::SkEcdsaSha2NistP256 + ) && (!authorized_key.options.no_touch_required + || authorized_key.options.verify_required) + { + tracing::info!( + user = %username, + key_type = %key.algorithm(), + "Security key rejected: user presence or verification policy cannot be enforced; only no-touch-required without verify-required is supported" + ); + // The first matching entry determines the policy; a later + // duplicate must not weaken its presence/verification requirement. + return Ok(false); + } tracing::info!( user = %username, key_type = %key.algorithm(), @@ -635,6 +659,8 @@ fn parse_key_options(options_str: &str) -> Result { } else { // Boolean option match option { + "no-touch-required" => options.no_touch_required = true, + "verify-required" => options.verify_required = true, "no-pty" => options.no_pty = true, "no-port-forwarding" => options.no_port_forwarding = true, "no-agent-forwarding" => options.no_agent_forwarding = true, diff --git a/tests/server_sk_policy_test.rs b/tests/server_sk_policy_test.rs new file mode 100644 index 000000000..02d12cc75 --- /dev/null +++ b/tests/server_sk_policy_test.rs @@ -0,0 +1,101 @@ +// Copyright 2025 Lablup Inc. and Jeongkyu Shin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use std::sync::Arc; + +use bssh::server::{config::ServerConfig, handler::SshHandler, session::SessionManager}; +use russh::keys::ssh_key::{ + PublicKey, + public::{Ed25519PublicKey, KeyData, SkEcdsaSha2NistP256, SkEd25519}, +}; +use russh::server::{Auth, Handler}; +use tokio::sync::RwLock; + +fn keys() -> [PublicKey; 3] { + let ed = Ed25519PublicKey([7; 32]); + let signing = p256::ecdsa::SigningKey::from_slice(&[1; 32]).unwrap(); + let point = signing.verifying_key().to_sec1_point(false); + [ + PublicKey::new(KeyData::SkEd25519(SkEd25519::new(ed, "ssh:")), ""), + PublicKey::new( + KeyData::SkEcdsaSha2NistP256(SkEcdsaSha2NistP256::new(point, "ssh:")), + "", + ), + PublicKey::new(KeyData::Ed25519(ed), ""), + ] +} + +async fn authenticate(key: &PublicKey, lines: &[String]) -> bool { + let dir = tempfile::tempdir().unwrap(); + let user_dir = dir.path().join("alice"); + std::fs::create_dir(&user_dir).unwrap(); + let path = user_dir.join("authorized_keys"); + std::fs::write(&path, lines.join("\n")).unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&user_dir, std::fs::Permissions::from_mode(0o700)).unwrap(); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600)).unwrap(); + } + let config = ServerConfig::builder() + .allow_publickey_auth(true) + .allow_password_auth(false) + .authorized_keys_dir(dir.path()) + .build(); + let mut handler = SshHandler::new( + None, + Arc::new(config), + Arc::new(RwLock::new(SessionManager::new())), + ); + matches!( + handler.auth_publickey("alice", key).await.unwrap(), + Auth::Accept + ) +} + +#[tokio::test] +async fn server_sk_policy_enforces_each_authorized_key_option_combination() { + for (index, key) in keys().iter().enumerate() { + for options in [ + "", + "no-touch-required", + "verify-required", + "no-touch-required,verify-required", + "verify-required,no-touch-required", + "no-pty,no-touch-required", + ] { + let line = format!("{options} {}", key.to_openssh().unwrap()); + let accepted = authenticate(key, &[line]).await; + let expected = index == 2 + || (options.contains("no-touch-required") && !options.contains("verify-required")); + assert_eq!(accepted, expected, "{}: {options}", key.algorithm()); + } + } +} + +#[tokio::test] +async fn server_sk_policy_uses_first_matching_entry() { + let keys = keys(); + for key in &keys[..2] { + let public = key.to_openssh().unwrap(); + let restricted = format!("verify-required {public}"); + let allowed = format!("no-touch-required {public}"); + assert!(!authenticate(key, &[restricted.clone(), allowed.clone()]).await); + assert!(authenticate(key, &[allowed.clone(), restricted]).await); + // An exception for a different key must never relax this key's policy. + let unrelated = format!("no-touch-required {}", keys[2].to_openssh().unwrap()); + assert!(!authenticate(key, &[public, unrelated.clone()]).await); + assert!(authenticate(key, &[unrelated, allowed]).await); + } +}