fix(rpc): reject unsigned v2 control mutations (#6905)

Co-authored-by: heihutu <heihutu@gmail.com>
This commit is contained in:
houseme
2026-08-31 02:17:43 +08:00
committed by GitHub
parent 37b23a16da
commit 16af688a7a
5 changed files with 126 additions and 6 deletions
+2 -2
View File
@@ -461,8 +461,8 @@ pub mod rpc {
tonic_boot_epoch_challenge, tonic_boot_epoch_response_headers, tonic_rpc_auth_failure_reason, tonic_boot_epoch_challenge, tonic_boot_epoch_response_headers, tonic_rpc_auth_failure_reason,
verify_ns_scanner_capability, verify_ns_scanner_capability_with_tier_registry_generation, verify_put_file_auth_trailer, verify_ns_scanner_capability, verify_ns_scanner_capability_with_tier_registry_generation, verify_put_file_auth_trailer,
verify_put_file_capability, verify_rpc_signature, verify_tonic_boot_epoch_response, verify_tonic_canonical_body_digest, verify_put_file_capability, verify_rpc_signature, verify_tonic_boot_epoch_response, verify_tonic_canonical_body_digest,
verify_tonic_mutation_body_digest, verify_tonic_rpc_response_proof, verify_tonic_rpc_signature, verify_tonic_mutation_body_digest, verify_tonic_mutation_body_digest_reject_unsigned, verify_tonic_rpc_response_proof,
verify_tonic_rpc_signature_with_bootstrap, verify_tonic_rpc_signature, verify_tonic_rpc_signature_with_bootstrap,
}; };
} }
@@ -1307,6 +1307,32 @@ pub fn verify_tonic_mutation_body_digest<T>(request: &tonic::Request<T>, canonic
verify_tonic_mutation_body_digest_with_strictness(request, canonical_body, internode_rpc_body_digest_strict()) verify_tonic_mutation_body_digest_with_strictness(request, canonical_body, internode_rpc_body_digest_strict())
} }
/// Verify a non-disk mutation without accepting a newly-generated unsigned v2 body.
///
/// The disk mutation lane has a rolling-upgrade exception for `UNSIGNED-PAYLOAD`
/// while peer replay-cache capability is being discovered. Historical v2 peers
/// used the fixed `unsigned` nonce before body-digest rollout; preserve that
/// exact marker for mixed-version compatibility, but reject unsigned v2
/// requests that omit it or present a different nonce.
pub fn verify_tonic_mutation_body_digest_reject_unsigned<T>(
request: &tonic::Request<T>,
canonical_body: &[u8],
) -> std::io::Result<()> {
let version = request
.metadata()
.get(RPC_AUTH_VERSION_HEADER)
.and_then(|value| value.to_str().ok());
let digest = request
.metadata()
.get(RPC_CONTENT_SHA256_HEADER)
.and_then(|value| value.to_str().ok());
let nonce = request.metadata().get(RPC_NONCE_HEADER).and_then(|value| value.to_str().ok());
if version == Some(RPC_AUTH_VERSION_V2) && digest == Some(UNSIGNED_PAYLOAD) && nonce != Some("unsigned") {
return Err(std::io::Error::other("RPC mutation requires a body-bound v2 signature"));
}
verify_tonic_mutation_body_digest(request, canonical_body)
}
/// [`verify_tonic_mutation_body_digest`] with the strict gate injected as a parameter, so both /// [`verify_tonic_mutation_body_digest`] with the strict gate injected as a parameter, so both
/// rollout postures are unit-testable without racing on process-global environment variables. /// rollout postures are unit-testable without racing on process-global environment variables.
fn verify_tonic_mutation_body_digest_with_strictness<T>( fn verify_tonic_mutation_body_digest_with_strictness<T>(
+2 -2
View File
@@ -39,8 +39,8 @@ pub use http_auth::{
sign_tonic_rpc_response_proof, tonic_boot_epoch_challenge, tonic_boot_epoch_response_headers, tonic_rpc_auth_failure_reason, sign_tonic_rpc_response_proof, tonic_boot_epoch_challenge, tonic_boot_epoch_response_headers, tonic_rpc_auth_failure_reason,
verify_ns_scanner_capability, verify_ns_scanner_capability_with_tier_registry_generation, verify_put_file_auth_trailer, verify_ns_scanner_capability, verify_ns_scanner_capability_with_tier_registry_generation, verify_put_file_auth_trailer,
verify_put_file_capability, verify_rpc_signature, verify_tonic_boot_epoch_response, verify_tonic_canonical_body_digest, verify_put_file_capability, verify_rpc_signature, verify_tonic_boot_epoch_response, verify_tonic_canonical_body_digest,
verify_tonic_mutation_body_digest, verify_tonic_rpc_response_proof, verify_tonic_rpc_signature, verify_tonic_mutation_body_digest, verify_tonic_mutation_body_digest_reject_unsigned, verify_tonic_rpc_response_proof,
verify_tonic_rpc_signature_with_bootstrap, verify_tonic_rpc_signature, verify_tonic_rpc_signature_with_bootstrap,
}; };
#[cfg(test)] #[cfg(test)]
pub(crate) use internode_data_transport::TcpHttpInternodeDataTransport; pub(crate) use internode_data_transport::TcpHttpInternodeDataTransport;
+87 -1
View File
@@ -31,6 +31,7 @@ use crate::storage::storage_api::rpc_consumer::node_service::{
use crate::storage::storage_api::runtime_sources_consumer::{EndpointServerPools, runtime_sources}; use crate::storage::storage_api::runtime_sources_consumer::{EndpointServerPools, runtime_sources};
use crate::storage::storage_api::{ use crate::storage::storage_api::{
sign_tonic_rpc_response_proof, verify_tonic_canonical_body_digest, verify_tonic_mutation_body_digest, sign_tonic_rpc_response_proof, verify_tonic_canonical_body_digest, verify_tonic_mutation_body_digest,
verify_tonic_mutation_body_digest_reject_unsigned,
}; };
use bytes::Bytes; use bytes::Bytes;
use futures::Stream; use futures::Stream;
@@ -123,6 +124,15 @@ fn verify_node_mutation_body<T: CanonicalMutationBody>(request: &Request<T>, ope
.map_err(|err| Status::permission_denied(format!("{operation} authentication failed: {err}"))) .map_err(|err| Status::permission_denied(format!("{operation} authentication failed: {err}")))
} }
fn verify_node_signal_body<T: CanonicalMutationBody>(request: &Request<T>, operation: &'static str) -> Result<(), Status> {
let canonical_body = request
.get_ref()
.canonical_body()
.map_err(|_| Status::invalid_argument(format!("{operation} request length cannot be represented")))?;
verify_tonic_mutation_body_digest_reject_unsigned(request, &canonical_body)
.map_err(|err| Status::permission_denied(format!("{operation} authentication failed: {err}")))
}
fn start_decommission_failure_response(err: Error) -> StartDecommissionResponse { fn start_decommission_failure_response(err: Error) -> StartDecommissionResponse {
match err { match err {
Error::InvalidArgument(_, _, reason) => StartDecommissionResponse { Error::InvalidArgument(_, _, reason) => StartDecommissionResponse {
@@ -1839,7 +1849,7 @@ impl Node for NodeService {
} }
async fn signal_service(&self, request: Request<SignalServiceRequest>) -> Result<Response<SignalServiceResponse>, Status> { async fn signal_service(&self, request: Request<SignalServiceRequest>) -> Result<Response<SignalServiceResponse>, Status> {
verify_node_mutation_body(&request, "signal service")?; verify_node_signal_body(&request, "signal service")?;
let request = request.into_inner(); let request = request.into_inner();
let vars = match request.vars { let vars = match request.vars {
Some(vars) => vars.value, Some(vars) => vars.value,
@@ -4744,6 +4754,34 @@ mod tests {
assert!(refresh_response.error_info.is_some()); assert!(refresh_response.error_info.is_some());
} }
#[tokio::test]
async fn lock_rolling_unsigned_v2_remains_compatible_for_unknown_peer() {
let service = create_test_node_service();
let unsigned_request = || {
let mut request = Request::new(GenerallyLockRequest {
args: "invalid json".to_string(),
});
request
.metadata_mut()
.insert("x-rustfs-rpc-auth-version", "2".parse().expect("valid metadata value"));
request
.metadata_mut()
.insert("x-rustfs-content-sha256", "UNSIGNED-PAYLOAD".parse().expect("valid metadata value"));
request
};
let lock = service
.lock(unsigned_request())
.await
.expect("unsigned lock must pass the rolling body gate");
assert!(!lock.into_inner().success, "invalid test lock args should fail in the lock handler");
let unlock = service
.un_lock(unsigned_request())
.await
.expect("unsigned unlock must pass the rolling body gate");
assert!(!unlock.into_inner().success, "invalid test unlock args should fail in the unlock handler");
}
/// Premise guard for the no-object-layer RPC tests (backlog#1830): they /// Premise guard for the no-object-layer RPC tests (backlog#1830): they
/// assert the error surface returned while the global object layer is /// assert the error surface returned while the global object layer is
/// absent. Under nextest — the authoritative runner — every test owns its /// absent. Under nextest — the authoritative runner — every test owns its
@@ -5561,6 +5599,54 @@ mod tests {
assert_eq!(response.error_info.as_deref(), Some("unsupported service signal: 99")); assert_eq!(response.error_info.as_deref(), Some("unsupported service signal: 99"));
} }
#[tokio::test]
async fn signal_service_rejects_explicitly_unsigned_v2_body() {
let service = create_test_node_service();
let request = SignalServiceRequest {
vars: Some(Mss {
value: HashMap::from([(PEER_RESTSIGNAL.to_string(), "99".to_string())]),
}),
};
let mut request = Request::new(request);
request
.metadata_mut()
.insert("x-rustfs-rpc-auth-version", "2".parse().expect("valid metadata value"));
request
.metadata_mut()
.insert("x-rustfs-content-sha256", "UNSIGNED-PAYLOAD".parse().expect("valid metadata value"));
let error = service
.signal_service(request)
.await
.expect_err("an explicitly unsigned v2 signal must fail before handler logic");
assert_eq!(error.code(), tonic::Code::PermissionDenied);
}
#[tokio::test]
async fn signal_service_accepts_historical_unsigned_v2_marker_during_rollout() {
let service = create_test_node_service();
let mut request = Request::new(SignalServiceRequest {
vars: Some(Mss {
value: HashMap::from([(PEER_RESTSIGNAL.to_string(), "99".to_string())]),
}),
});
request
.metadata_mut()
.insert("x-rustfs-rpc-auth-version", "2".parse().expect("valid metadata value"));
request
.metadata_mut()
.insert("x-rustfs-content-sha256", "UNSIGNED-PAYLOAD".parse().expect("valid metadata value"));
request
.metadata_mut()
.insert("x-rustfs-rpc-nonce", "unsigned".parse().expect("valid metadata value"));
let response = service
.signal_service(request)
.await
.expect("historical unsigned v2 marker must remain compatible during rollout");
assert!(!response.into_inner().success, "invalid signal fixture should reach handler validation");
}
#[tokio::test] #[tokio::test]
async fn every_non_disk_mutation_rejects_a_mismatched_body_digest() { async fn every_non_disk_mutation_rejects_a_mismatched_body_digest() {
let service = create_test_node_service(); let service = create_test_node_service();
+9 -1
View File
@@ -539,7 +539,8 @@ pub(crate) mod ecstore_rpc {
sign_ns_scanner_capability_with_tier_registry_generation, sign_put_file_capability, sign_tonic_rpc_response_proof, sign_ns_scanner_capability_with_tier_registry_generation, sign_put_file_capability, sign_tonic_rpc_response_proof,
tonic_boot_epoch_challenge, tonic_boot_epoch_response_headers, tonic_rpc_auth_failure_reason, tonic_boot_epoch_challenge, tonic_boot_epoch_response_headers, tonic_rpc_auth_failure_reason,
verify_put_file_auth_trailer, verify_rpc_signature, verify_tonic_canonical_body_digest, verify_put_file_auth_trailer, verify_rpc_signature, verify_tonic_canonical_body_digest,
verify_tonic_mutation_body_digest, verify_tonic_rpc_signature_with_bootstrap, verify_tonic_mutation_body_digest, verify_tonic_mutation_body_digest_reject_unsigned,
verify_tonic_rpc_signature_with_bootstrap,
}; };
#[cfg(test)] #[cfg(test)]
pub(crate) use rustfs_ecstore::api::rpc::{ pub(crate) use rustfs_ecstore::api::rpc::{
@@ -1903,6 +1904,13 @@ pub(crate) fn verify_tonic_mutation_body_digest<T>(request: &tonic::Request<T>,
ecstore_rpc::verify_tonic_mutation_body_digest(request, canonical_body) ecstore_rpc::verify_tonic_mutation_body_digest(request, canonical_body)
} }
pub(crate) fn verify_tonic_mutation_body_digest_reject_unsigned<T>(
request: &tonic::Request<T>,
canonical_body: &[u8],
) -> std::io::Result<()> {
ecstore_rpc::verify_tonic_mutation_body_digest_reject_unsigned(request, canonical_body)
}
#[cfg(test)] #[cfg(test)]
pub(crate) fn set_tonic_canonical_body_digest<T>(request: &mut tonic::Request<T>, canonical_body: &[u8]) -> std::io::Result<()> { pub(crate) fn set_tonic_canonical_body_digest<T>(request: &mut tonic::Request<T>, canonical_body: &[u8]) -> std::io::Result<()> {
ecstore_rpc::set_tonic_canonical_body_digest(request, canonical_body) ecstore_rpc::set_tonic_canonical_body_digest(request, canonical_body)