test(security): GHSA-named regression tests for 3p3x and r5qv (backlog#1151 sec-6) (#4707)

test(security): add GHSA-named regression tests for 3p3x and r5qv (backlog#1151 sec-6)

Anchor the two fixed advisories to discoverable, named regression tests so
`rg -i "ghsa|3p3x|r5qv"` finds a guard for each, and future fixes are forced
to update pinned behavior (red -> green).

GHSA-3p3x-734c-h5vx (constant-time WebDAV/FTPS secret comparison, rustfs#4403):
- ftps_core.rs: new `assert_ftps_ghsa_3p3x_wrong_credentials_rejected` drives
  the `ct_eq` reject branch in FtpsAuthenticator::authenticate; asserts wrong
  password and unknown user are both rejected (530) and indistinguishable.
- webdav_core.rs: the auth-failure block now sends a valid access key with a
  wrong secret (exercising the `ct_eq` branch, not just the unknown-access-key
  path) plus an unknown user, asserting both 401 and indistinguishable.
- Module doc comments map advisory -> tests -> fix PR on both files.

GHSA-r5qv-rc46-hv8q (internode RPC fail-closed, rustfs#4402):
- http_auth.rs: renamed the default-fallback rejection test to
  `ghsa_r5qv_resolve_shared_secret_rejects_default_fallback` (and broadened it
  to cover default env secret + blank secrets), and added
  `ghsa_r5qv_verify_rpc_signature_fails_closed_on_missing_or_invalid_auth`
  pinning the exact advisory scenario (missing/forged/cross-URL signature is
  rejected; a correctly signed request still passes). File-level doc maps the
  advisory.

Docs: new docs/testing/security-regressions.md with the advisory -> test
mapping table and where each layer runs; linked from docs/testing/README.md.
sec-14 will formalize the written policy in AGENTS.md.

The unit-level ghsa_r5qv_* tests run in the default CI pass. The WebDAV/FTPS
e2e live in the protocols suite (fixed ports, --test-threads=1); they cannot
join the e2e-smoke profile and are wired into CI by sec-5.

Refs: rustfs/backlog#1151 (sec-6), rustfs/backlog#1155

crates/e2e_test/src/protocols/ftps_core.rs

@@ -13,6 +13,23 @@
 // limitations under the License.
 
 //! Core FTPS tests
+//!
+//! # Security regression coverage
+//!
+//! GHSA-3p3x-734c-h5vx (constant-time secret comparison on the WebDAV/FTPS
+//! password login path, fixed in rustfs/rustfs#4403) is anchored end-to-end
+//! by [`assert_ftps_ghsa_3p3x_wrong_credentials_rejected`], invoked from
+//! [`test_ftps_core_operations`]. It exercises the `ct_eq` rejection branch in
+//! `FtpsAuthenticator::authenticate` (`crates/protocols/src/ftps/server.rs`),
+//! proving that a wrong password and an unknown user are both rejected and are
+//! indistinguishable at the protocol layer (both return FTP `530 Not logged
+//! in`). The sibling WebDAV assertion lives in `webdav_core.rs`; the internode
+//! RPC fail-closed advisory (GHSA-r5qv-rc46-hv8q, rustfs/rustfs#4402) is
+//! anchored by the `ghsa_r5qv_*` unit tests in
+//! `crates/ecstore/src/cluster/rpc/http_auth.rs`. See
+//! `docs/testing/security-regressions.md` for the full advisory -> test map.
+//!
+//! Advisory: <https://github.com/rustfs/rustfs/security/advisories/GHSA-3p3x-734c-h5vx>
 
 use crate::common::rustfs_binary_path_with_features;
 use crate::protocols::test_env::{DEFAULT_ACCESS_KEY, DEFAULT_SECRET_KEY, ProtocolTestEnvironment};
@@ -24,8 +41,8 @@ use rustls::{ClientConfig, DigitallySignedStruct, Error as RustlsError, Signatur
 use std::io::Cursor;
 use std::path::PathBuf;
 use std::sync::Arc;
-use suppaftp::RustlsConnector;
-use suppaftp::RustlsFtpStream;
+use suppaftp::types::Response;
+use suppaftp::{FtpError, RustlsConnector, RustlsFtpStream, Status};
 use tokio::process::Command;
 use tracing::info;
 
@@ -73,6 +90,78 @@ impl ServerCertVerifier for AcceptAnyServerCertVerifier {
     }
 }
 
+/// Open a fresh, unauthenticated FTPS control connection to the test server
+/// and upgrade it to TLS. The caller is responsible for logging in.
+///
+/// Assumes the process-wide rustls crypto provider has already been installed
+/// (done once at the top of [`test_ftps_core_operations`]).
+fn ftps_connect_secure() -> Result<RustlsFtpStream> {
+    let config = ClientConfig::builder()
+        .dangerous()
+        .with_custom_certificate_verifier(Arc::new(AcceptAnyServerCertVerifier))
+        .with_no_client_auth();
+
+    let tls_connector = RustlsConnector::from(Arc::new(config));
+
+    let ftp_stream = RustlsFtpStream::connect(FTPS_ADDRESS).map_err(|e| anyhow::anyhow!("Failed to connect: {}", e))?;
+
+    ftp_stream
+        .into_secure(tls_connector, "127.0.0.1")
+        .map_err(|e| anyhow::anyhow!("Failed to upgrade to TLS: {}", e))
+}
+
+/// Extract the FTP reply status from a failed login, asserting the failure is a
+/// protocol-level rejection (`FtpError::UnexpectedResponse`) rather than a
+/// transport error.
+fn login_rejection_status(user: &str, password: &str) -> Result<Status> {
+    let mut ftp_stream = ftps_connect_secure()?;
+    match ftp_stream.login(user, password) {
+        Ok(()) => anyhow::bail!("login unexpectedly succeeded for user '{user}'"),
+        Err(FtpError::UnexpectedResponse(Response { status, .. })) => Ok(status),
+        Err(other) => anyhow::bail!("expected a protocol rejection, got transport error: {other}"),
+    }
+}
+
+/// Security regression for GHSA-3p3x-734c-h5vx.
+///
+/// FTPS password verification must reject invalid credentials via the
+/// constant-time `ct_eq` branch in `FtpsAuthenticator::authenticate`
+/// (`crates/protocols/src/ftps/server.rs`, fixed in rustfs/rustfs#4403). This
+/// asserts, purely on behavior (no timing assertions):
+///
+/// 1. A valid user with a wrong password is rejected (`530`), exercising the
+///    secret-mismatch branch that the advisory hardened.
+/// 2. An unknown user is rejected (`530`).
+/// 3. The two failures are indistinguishable at the protocol layer — both
+///    return the same FTP status, so an attacker cannot use the reply code to
+///    tell "user exists, wrong password" from "no such user".
+async fn assert_ftps_ghsa_3p3x_wrong_credentials_rejected() -> Result<()> {
+    info!("Testing FTPS (GHSA-3p3x): wrong password is rejected");
+    let wrong_password_status = login_rejection_status(DEFAULT_ACCESS_KEY, "definitely-not-the-secret")?;
+    assert_eq!(
+        wrong_password_status,
+        Status::NotLoggedIn,
+        "valid user + wrong password must be rejected with 530, got {wrong_password_status:?}"
+    );
+    info!("PASS: FTPS wrong password rejected with 530");
+
+    info!("Testing FTPS (GHSA-3p3x): unknown user is rejected");
+    let unknown_user_status = login_rejection_status("no-such-access-key", DEFAULT_SECRET_KEY)?;
+    assert_eq!(
+        unknown_user_status,
+        Status::NotLoggedIn,
+        "unknown user must be rejected with 530, got {unknown_user_status:?}"
+    );
+    info!("PASS: FTPS unknown user rejected with 530");
+
+    assert_eq!(
+        wrong_password_status, unknown_user_status,
+        "invalid-user and invalid-password failures must be indistinguishable at the protocol layer"
+    );
+    info!("PASS: FTPS invalid-user and invalid-password failures are indistinguishable");
+    Ok(())
+}
+
 /// Test FTPS: put, ls, mkdir, rmdir, delete operations
 pub async fn test_ftps_core_operations() -> Result<()> {
     let env = ProtocolTestEnvironment::new().map_err(|e| anyhow::anyhow!("{}", e))?;
@@ -116,26 +205,18 @@ pub async fn test_ftps_core_operations() -> Result<()> {
             .await
             .map_err(|e| anyhow::anyhow!("{}", e))?;
 
-        // Build ServerConfig with SNI support
+        // Install the default crypto provider once for this process before any
+        // TLS handshake. Subsequent connections reuse it via `ftps_connect_secure`.
         rustls::crypto::aws_lc_rs::default_provider()
             .install_default()
             .map_err(|e| anyhow::anyhow!("Failed to install crypto provider: {:?}", e))?;
 
-        let config = ClientConfig::builder()
-            .dangerous()
-            .with_custom_certificate_verifier(Arc::new(AcceptAnyServerCertVerifier))
-            .with_no_client_auth();
+        // Security regression (GHSA-3p3x-734c-h5vx): wrong credentials must be
+        // rejected before any successful login is attempted below.
+        assert_ftps_ghsa_3p3x_wrong_credentials_rejected().await?;
 
-        // Wrap in suppaftp's RustlsConnector
-        let tls_connector = RustlsConnector::from(Arc::new(config));
-
-        // Connect to FTPS server
-        let ftp_stream = RustlsFtpStream::connect(FTPS_ADDRESS).map_err(|e| anyhow::anyhow!("Failed to connect: {}", e))?;
-
-        // Upgrade to secure connection
-        let mut ftp_stream = ftp_stream
-            .into_secure(tls_connector, "127.0.0.1")
-            .map_err(|e| anyhow::anyhow!("Failed to upgrade to TLS: {}", e))?;
+        // Connect and log in with valid credentials for the functional flow.
+        let mut ftp_stream = ftps_connect_secure()?;
         ftp_stream.login(DEFAULT_ACCESS_KEY, DEFAULT_SECRET_KEY)?;
 
         info!("Testing FTPS: mkdir bucket");

crates/e2e_test/src/protocols/webdav_core.rs

@@ -13,6 +13,23 @@
 // limitations under the License.
 
 //! Core WebDAV tests
+//!
+//! # Security regression coverage
+//!
+//! GHSA-3p3x-734c-h5vx (constant-time secret comparison on the WebDAV/FTPS
+//! password login path, fixed in rustfs/rustfs#4403) is anchored end-to-end by
+//! the authentication-failure block in [`test_webdav_core_operations`], marked
+//! with a `GHSA-3p3x` comment. It drives the `ct_eq` rejection branch in
+//! `WebDavAuth::authenticate` (`crates/protocols/src/webdav/server.rs`): a
+//! valid access key with a wrong secret is rejected (the exact branch the
+//! advisory hardened), and an unknown user is rejected with the same status so
+//! the two failures are indistinguishable. The sibling FTPS assertion lives in
+//! `ftps_core.rs`; the internode RPC fail-closed advisory
+//! (GHSA-r5qv-rc46-hv8q, rustfs/rustfs#4402) is anchored by the `ghsa_r5qv_*`
+//! unit tests in `crates/ecstore/src/cluster/rpc/http_auth.rs`. See
+//! `docs/testing/security-regressions.md` for the full advisory -> test map.
+//!
+//! Advisory: <https://github.com/rustfs/rustfs/security/advisories/GHSA-3p3x-734c-h5vx>
 
 use crate::common::local_http_client;
 use crate::common::rustfs_binary_path_with_features;
@@ -653,15 +670,38 @@ pub async fn test_webdav_core_operations() -> Result<()> {
         );
         info!("PASS: DELETE bucket '{}' successful", bucket_name);
 
-        // Test authentication failure
-        info!("Testing WebDAV: Authentication failure");
-        let resp = client
+        // Security regression (GHSA-3p3x-734c-h5vx): constant-time secret
+        // comparison on the WebDAV password login path (fixed in
+        // rustfs/rustfs#4403). A valid access key with a wrong secret must be
+        // rejected via the `ct_eq` branch in `WebDavAuth::authenticate`, and an
+        // unknown user must be rejected with the same status so the two are
+        // indistinguishable. Behavior-only assertion (no timing checks).
+        info!("Testing WebDAV (GHSA-3p3x): valid access key + wrong secret is rejected");
+        let wrong_secret_resp = client
             .request(reqwest::Method::from_bytes(b"PROPFIND").unwrap(), &base_url)
-            .header("Authorization", "Basic aW52YWxpZDppbnZhbGlk") // invalid:invalid
+            .header("Authorization", basic_auth_header_for(DEFAULT_ACCESS_KEY, "definitely-not-the-secret"))
             .send()
             .await?;
-        assert_eq!(resp.status().as_u16(), 401, "Invalid auth should return 401, got: {}", resp.status());
-        info!("PASS: Authentication failure test successful");
+        let wrong_secret_status = wrong_secret_resp.status().as_u16();
+        assert_eq!(
+            wrong_secret_status, 401,
+            "valid access key + wrong secret should return 401, got: {wrong_secret_status}"
+        );
+        info!("PASS: WebDAV wrong secret rejected with 401");
+
+        info!("Testing WebDAV (GHSA-3p3x): unknown user is rejected");
+        let unknown_user_resp = client
+            .request(reqwest::Method::from_bytes(b"PROPFIND").unwrap(), &base_url)
+            .header("Authorization", basic_auth_header_for("no-such-access-key", "definitely-not-the-secret"))
+            .send()
+            .await?;
+        let unknown_user_status = unknown_user_resp.status().as_u16();
+        assert_eq!(unknown_user_status, 401, "unknown user should return 401, got: {unknown_user_status}");
+        assert_eq!(
+            wrong_secret_status, unknown_user_status,
+            "invalid-user and invalid-secret failures must be indistinguishable at the protocol layer"
+        );
+        info!("PASS: WebDAV authentication failure (GHSA-3p3x) test successful");
 
         info!("WebDAV core tests passed");
         Ok(())

crates/ecstore/src/cluster/rpc/http_auth.rs

@@ -12,6 +12,19 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+//! Internode RPC HMAC authentication.
+//!
+//! # Security regression coverage
+//!
+//! GHSA-r5qv-rc46-hv8q (internode RPC authentication must fail closed, fixed in
+//! rustfs/rustfs#4402) is anchored by the `ghsa_r5qv_*` tests in the module
+//! below, plus the broader negative-signature suite. The advisory class is: a
+//! node must never accept an RPC whose auth is missing, malformed, replayed, or
+//! signed with the default/empty shared secret. See
+//! `docs/testing/security-regressions.md` for the full advisory -> test map.
+//!
+//! Advisory: <https://github.com/rustfs/rustfs/security/advisories/GHSA-r5qv-rc46-hv8q>
+
 use crate::cluster::rpc::context_propagation::{inject_request_id_into_http_headers, inject_trace_context_into_http_headers};
 use base64::Engine as _;
 use base64::engine::general_purpose;
@@ -227,13 +240,70 @@ mod tests {
         runtime_sources::ensure_test_rpc_secret();
     }
 
+    /// Security regression for GHSA-r5qv-rc46-hv8q (internode RPC fail-closed,
+    /// fixed in rustfs/rustfs#4402): secret resolution must never silently fall
+    /// back to a default/empty shared secret. Missing and default secrets both
+    /// resolve to an error, so a misconfigured node cannot come up with a
+    /// predictable, attacker-known RPC key.
     #[test]
-    fn test_resolve_shared_secret_rejects_default_fallback() {
+    fn ghsa_r5qv_resolve_shared_secret_rejects_default_fallback() {
         let err = resolve_shared_secret(None, None).expect_err("default fallback must be rejected");
         assert_eq!(err.to_string(), RPC_SECRET_REQUIRED_MESSAGE);
 
         let err = resolve_shared_secret(None, Some(DEFAULT_SECRET_KEY)).expect_err("default global secret must be rejected");
         assert_eq!(err.to_string(), RPC_SECRET_REQUIRED_MESSAGE);
+
+        let err = resolve_shared_secret(Some(DEFAULT_SECRET_KEY), None).expect_err("default env secret must be rejected");
+        assert_eq!(err.to_string(), RPC_SECRET_REQUIRED_MESSAGE);
+
+        let err = resolve_shared_secret(Some("   "), Some("   ")).expect_err("blank secrets must be rejected");
+        assert_eq!(err.to_string(), RPC_SECRET_REQUIRED_MESSAGE);
+    }
+
+    /// Security regression for GHSA-r5qv-rc46-hv8q: `verify_rpc_signature` must
+    /// fail closed for every shape of missing or malformed authentication.
+    /// Consolidates the advisory's exact scenario (no valid signature/timestamp
+    /// pair => rejected, never silently allowed) into one named test so the
+    /// advisory maps to a discoverable regression.
+    #[test]
+    fn ghsa_r5qv_verify_rpc_signature_fails_closed_on_missing_or_invalid_auth() {
+        ensure_test_rpc_secret();
+        let url = "http://example.com/api/test";
+        let method = Method::GET;
+
+        // No auth headers at all.
+        let empty = HeaderMap::new();
+        assert!(
+            verify_rpc_signature(url, &method, &empty).is_err(),
+            "request with no auth headers must be rejected"
+        );
+
+        // Signature header present but garbage; timestamp is current.
+        let mut forged = HeaderMap::new();
+        let now = OffsetDateTime::now_utc().unix_timestamp();
+        forged.insert(SIGNATURE_HEADER, HeaderValue::from_static("not-a-real-signature"));
+        forged.insert(TIMESTAMP_HEADER, HeaderValue::from_str(&now.to_string()).unwrap());
+        assert!(
+            verify_rpc_signature(url, &method, &forged).is_err(),
+            "request with a forged signature must be rejected"
+        );
+
+        // A validly signed request for a *different* URL must not authorize this one.
+        let mut cross = HeaderMap::new();
+        build_auth_headers("http://example.com/api/other", &method, &mut cross).expect("auth headers should build");
+        assert!(
+            verify_rpc_signature(url, &method, &cross).is_err(),
+            "a signature bound to a different URL must not authorize this request"
+        );
+
+        // Control: a correctly signed request for this URL still succeeds, so the
+        // gate is fail-closed rather than fail-everything.
+        let mut valid = HeaderMap::new();
+        build_auth_headers(url, &method, &mut valid).expect("auth headers should build");
+        assert!(
+            verify_rpc_signature(url, &method, &valid).is_ok(),
+            "a correctly signed request must be accepted"
+        );
     }
 
     #[test]

docs/testing/README.md

@@ -11,6 +11,13 @@
 _TODO (infra-11): unit / e2e-smoke / e2e-full / e2e-nightly / protocols /
 s3-tests / fuzz / perf, with naming conventions._
 
+### Security advisory regression tests
+
+Fixed GHSA advisories map to named, discoverable regression tests. The
+advisory -> test map lives in
+[`docs/testing/security-regressions.md`](security-regressions.md). sec-14
+(backlog#1151) formalizes the written admission policy in `AGENTS.md`.
+
 ## Serial groups & CI profiles
 
 _TODO (infra-11): document the `[test-groups]` mechanism and the `default` vs

docs/testing/security-regressions.md

@@ -0,0 +1,48 @@
+# Security Advisory Regression Tests
+
+Every fixed RustFS GitHub Security Advisory (GHSA) should map to at least one
+named, discoverable regression test. The convention is: name the test (or a
+helper / doc comment on the exact assertion) after the advisory so that
+
+```bash
+rg -i "ghsa|3p3x|r5qv"
+```
+
+finds the guard for any advisory, and a future fix of a still-open advisory is
+forced to update its pinned test (red -> green).
+
+> This is the lightweight inventory. sec-14 (backlog#1151) formalizes the
+> written admission policy in `AGENTS.md`; keep this file as the map.
+
+## Advisory -> test mapping
+
+| Advisory | Class | Fix PR | Named regression tests | Layer |
+| --- | --- | --- | --- | --- |
+| [GHSA-3p3x-734c-h5vx](https://github.com/rustfs/rustfs/security/advisories/GHSA-3p3x-734c-h5vx) | Constant-time secret comparison on WebDAV/FTPS password login | rustfs/rustfs#4403 | `assert_ftps_ghsa_3p3x_wrong_credentials_rejected` (`crates/e2e_test/src/protocols/ftps_core.rs`); `GHSA-3p3x` auth-failure block in `test_webdav_core_operations` (`crates/e2e_test/src/protocols/webdav_core.rs`) | e2e (protocols suite) |
+| [GHSA-r5qv-rc46-hv8q](https://github.com/rustfs/rustfs/security/advisories/GHSA-r5qv-rc46-hv8q) | Internode RPC authentication must fail closed | rustfs/rustfs#4402 | `ghsa_r5qv_resolve_shared_secret_rejects_default_fallback`, `ghsa_r5qv_verify_rpc_signature_fails_closed_on_missing_or_invalid_auth` (`crates/ecstore/src/cluster/rpc/http_auth.rs`) | unit |
+| [GHSA-m77q-r63m-pj89](https://github.com/rustfs/rustfs/security/advisories/GHSA-m77q-r63m-pj89) | STS JWTs signed with shared root secret (intentionally unfixed) | n/a | `test_created_sts_credentials_authorize_with_session_token_claims` (`crates/iam/src/sys.rs`) — pins current behavior; sec-7 adds GHSA naming | unit |
+
+## Where these run
+
+- **Unit tests** (`ghsa_r5qv_*` in `crates/ecstore`) run automatically in the
+  default CI pass via `cargo nextest run` / `cargo test` — no special wiring.
+- **Protocol e2e tests** (WebDAV/FTPS constant-time login rejection) live in the
+  `e2e_test` protocols suite, which binds **fixed ports** and requires
+  `--test-threads=1`. Because of the fixed ports they **cannot** join the
+  `e2e-smoke` nextest profile (parallel, dynamic ports). They run manually today:
+
+  ```bash
+  RUSTFS_BUILD_FEATURES=ftps,webdav cargo test --package e2e_test \
+    test_protocol_core_suite -- --test-threads=1 --nocapture
+  ```
+
+  Wiring the security e2e lane into CI is tracked separately (sec-5, backlog#1151).
+
+## Adding a new advisory guard
+
+1. Reproduce the advisory's bypass form as a focused negative test.
+2. Name the test (or the helper/assertion) `ghsa_<id>_*`, or attach a
+   `GHSA-<id>` doc comment with the advisory URL and fix PR.
+3. Add a row to the table above.
+4. Land the unit-level guard where the default CI pass runs it; land any e2e
+   guard in the protocols suite (and register it with sec-5's filterset seam).