feat(rpc): bind canonical body digest into internode mutating disk RPC signatures (#5234)

* feat(rpc): bind canonical body digest into internode mutating disk RPC signatures Binds a domain-separated, length-prefixed canonical request-body digest into the v2 HMAC signature scope for every mutating NodeService disk RPC, so an on-path attacker on the default-plaintext internode channel can no longer tamper with a mutation payload (or strip the msgpack `_bin` field to force the JSON fallback decode) without invalidating the signature. Covers 13 mutating disk RPCs: RenameData, DeleteVersion, DeleteVersions, WriteMetadata, UpdateMetadata, WriteAll, Delete, DeletePaths, RenameFile, RenamePart, DeleteVolume, MakeVolume, MakeVolumes. The digest covers both the msgpack `_bin` payloads and their JSON compatibility copies. Gated fail-open by default (RUSTFS_INTERNODE_RPC_BODY_DIGEST_STRICT) with a convergence counter, so rolling upgrades are byte-for-byte unaffected; the replay-cache capacity is now configurable and overflow fails closed with a metric. Refs https://github.com/rustfs/backlog/issues/1327 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(rpc): satisfy architecture-migration compat-marker guard Put the removal condition on the RUSTFS_COMPAT_TODO marker line itself, and stop backticking env-var/metric names in the cleanup-register entry so the guard's id extractor only sees the task-id. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-08-08 14:23:13 +00:00 · 2026-07-25 22:56:27 +08:00
parent 6974963e20
commit 61d4e04d65
11 changed files with 1198 additions and 18 deletions
@@ -2393,6 +2393,238 @@ mod tests {
        request
    }

+    fn delete_request_message(options: &str) -> DeleteRequest {
+        DeleteRequest {
+            disk: "http://node-a:9000/data/rustfs0".to_string(),
+            volume: "bucket".to_string(),
+            path: "object".to_string(),
+            options: options.to_string(),
+        }
+    }
+
+    #[tokio::test]
+    async fn disk_mutation_body_digest_gate_runs_before_disk_lookup() {
+        let service = make_server();
+
+        // A digestless mutation stays accepted through the default fail-open gate (rolling
+        // upgrade posture) and proceeds to the disk lookup.
+        let digestless = service
+            .delete(Request::new(delete_request_message("{}")))
+            .await
+            .expect("a digestless mutation must stay accepted while the strict gate is off");
+        assert!(!digestless.into_inner().success, "the unknown test disk cannot resolve");
+
+        // A digest bound to different request contents must be rejected before any disk work.
+        let mut tampered = Request::new(delete_request_message("{}"));
+        let other_body = rustfs_protos::canonical_delete_request_body(&delete_request_message("{\"recursive\":true}"))
+            .expect("small request should encode");
+        set_tonic_canonical_body_digest(&mut tampered, &other_body).expect("digest metadata should encode");
+        mark_v2_authenticated(&mut tampered);
+        let tampered = service
+            .delete(tampered)
+            .await
+            .expect_err("a tampered mutation must fail closed");
+        assert_eq!(tampered.code(), tonic::Code::PermissionDenied);
+
+        // A digest matching the received wire fields authenticates and proceeds to the disk lookup.
+        let mut signed = Request::new(delete_request_message("{}"));
+        let body = rustfs_protos::canonical_delete_request_body(signed.get_ref()).expect("small request should encode");
+        set_tonic_canonical_body_digest(&mut signed, &body).expect("digest metadata should encode");
+        mark_v2_authenticated(&mut signed);
+        let signed = service
+            .delete(signed)
+            .await
+            .expect("a correctly body-bound mutation must pass the digest gate");
+        assert!(!signed.into_inner().success, "the unknown test disk cannot resolve");
+    }
+
+    /// Per-handler wiring check for every mutating disk RPC. A mismatched digest must be rejected
+    /// (catches a handler that omits its `verify_disk_mutation_digest` gate) and a correctly
+    /// body-bound digest must pass the gate (catches a handler wired to the wrong
+    /// `canonical_*_request_body`, which would reject legitimate traffic). Both failure modes are
+    /// realistic across these copy-pasted call sites and are otherwise invisible to the digestless
+    /// fail-open tests.
+    #[tokio::test]
+    async fn every_mutating_handler_enforces_its_body_digest() {
+        let service = make_server();
+        let disk = "http://node-a:9000/data/rustfs0".to_string();
+
+        macro_rules! assert_gated {
+            ($method:ident, $msg:expr, $canonical:path) => {{
+                let msg = $msg;
+
+                // Correct digest: the gate passes and the handler proceeds to the (unknown) disk
+                // lookup, so it must NOT fail with PermissionDenied.
+                let mut ok = Request::new(msg.clone());
+                let body = $canonical(ok.get_ref()).expect("canonical body should encode");
+                set_tonic_canonical_body_digest(&mut ok, &body).expect("digest metadata should encode");
+                mark_v2_authenticated(&mut ok);
+                if let Err(status) = service.$method(ok).await {
+                    assert_ne!(
+                        status.code(),
+                        tonic::Code::PermissionDenied,
+                        concat!(stringify!($method), ": a correctly body-bound request must pass the digest gate"),
+                    );
+                }
+
+                // Mismatched digest: the gate must reject before any disk work.
+                let mut bad = Request::new(msg);
+                set_tonic_canonical_body_digest(&mut bad, b"unrelated-canonical-body").expect("digest metadata should encode");
+                mark_v2_authenticated(&mut bad);
+                let err = service
+                    .$method(bad)
+                    .await
+                    .expect_err(concat!(stringify!($method), ": a tampered body must be rejected"));
+                assert_eq!(
+                    err.code(),
+                    tonic::Code::PermissionDenied,
+                    concat!(stringify!($method), " must fail closed on a body-digest mismatch"),
+                );
+            }};
+        }
+
+        assert_gated!(
+            rename_data,
+            RenameDataRequest {
+                disk: disk.clone(),
+                src_volume: "src".into(),
+                src_path: "sp".into(),
+                file_info: "{}".into(),
+                dst_volume: "dst".into(),
+                dst_path: "dp".into(),
+                file_info_bin: vec![0x80].into(),
+            },
+            rustfs_protos::canonical_rename_data_request_body
+        );
+        assert_gated!(
+            delete_version,
+            DeleteVersionRequest {
+                disk: disk.clone(),
+                volume: "v".into(),
+                path: "p".into(),
+                file_info: "{}".into(),
+                force_del_marker: false,
+                opts: "{}".into(),
+                file_info_bin: vec![0x80].into(),
+                opts_bin: vec![0x80].into(),
+            },
+            rustfs_protos::canonical_delete_version_request_body
+        );
+        assert_gated!(
+            delete_versions,
+            DeleteVersionsRequest {
+                disk: disk.clone(),
+                volume: "v".into(),
+                versions: vec!["a".into()],
+                opts: "{}".into(),
+                versions_bin: vec![vec![0x80].into()],
+                opts_bin: vec![0x80].into(),
+            },
+            rustfs_protos::canonical_delete_versions_request_body
+        );
+        assert_gated!(
+            write_metadata,
+            WriteMetadataRequest {
+                disk: disk.clone(),
+                volume: "v".into(),
+                path: "p".into(),
+                file_info: "{}".into(),
+                file_info_bin: vec![0x80].into(),
+            },
+            rustfs_protos::canonical_write_metadata_request_body
+        );
+        assert_gated!(
+            update_metadata,
+            UpdateMetadataRequest {
+                disk: disk.clone(),
+                volume: "v".into(),
+                path: "p".into(),
+                file_info: "{}".into(),
+                opts: "{}".into(),
+                file_info_bin: vec![0x80].into(),
+                opts_bin: vec![0x80].into(),
+            },
+            rustfs_protos::canonical_update_metadata_request_body
+        );
+        assert_gated!(
+            write_all,
+            WriteAllRequest {
+                disk: disk.clone(),
+                volume: "v".into(),
+                path: "p".into(),
+                data: vec![0x01, 0x02].into(),
+            },
+            rustfs_protos::canonical_write_all_request_body
+        );
+        assert_gated!(
+            delete,
+            DeleteRequest {
+                disk: disk.clone(),
+                volume: "v".into(),
+                path: "p".into(),
+                options: "{}".into(),
+            },
+            rustfs_protos::canonical_delete_request_body
+        );
+        assert_gated!(
+            delete_paths,
+            DeletePathsRequest {
+                disk: disk.clone(),
+                volume: "v".into(),
+                paths: vec!["a".into()],
+            },
+            rustfs_protos::canonical_delete_paths_request_body
+        );
+        assert_gated!(
+            rename_file,
+            RenameFileRequest {
+                disk: disk.clone(),
+                src_volume: "src".into(),
+                src_path: "sp".into(),
+                dst_volume: "dst".into(),
+                dst_path: "dp".into(),
+            },
+            rustfs_protos::canonical_rename_file_request_body
+        );
+        assert_gated!(
+            rename_part,
+            RenamePartRequest {
+                disk: disk.clone(),
+                src_volume: "src".into(),
+                src_path: "sp".into(),
+                dst_volume: "dst".into(),
+                dst_path: "dp".into(),
+                meta: vec![0x03].into(),
+            },
+            rustfs_protos::canonical_rename_part_request_body
+        );
+        assert_gated!(
+            delete_volume,
+            DeleteVolumeRequest {
+                disk: disk.clone(),
+                volume: "v".into(),
+                force: true,
+            },
+            rustfs_protos::canonical_delete_volume_request_body
+        );
+        assert_gated!(
+            make_volume,
+            MakeVolumeRequest {
+                disk: disk.clone(),
+                volume: "v".into(),
+            },
+            rustfs_protos::canonical_make_volume_request_body
+        );
+        assert_gated!(
+            make_volumes,
+            MakeVolumesRequest {
+                disk,
+                volumes: vec!["v".into()],
+            },
+            rustfs_protos::canonical_make_volumes_request_body
+        );
+    }
+
    #[tokio::test]
    async fn heal_control_requires_body_bound_auth_before_topology_validation() {
        let service = make_heal_control_server();
@@ -18,6 +18,7 @@ use crate::storage::storage_api::rpc_consumer::node_service::{
    ReadMultipleResp, ReadOptions, StorageDiskRpcExt as _, UpdateMetadataOpts, validate_batch_read_version_item_count,
 };
 use crate::storage::storage_api::runtime_sources_consumer::runtime_sources;
+use crate::storage::storage_api::verify_tonic_mutation_body_digest;
 use bytes::Bytes;
 use rustfs_filemeta::FileInfo;
 use rustfs_io_metrics::internode_metrics::{
@@ -81,6 +82,24 @@ fn encode_msgpack_named<T: serde::Serialize>(value: &T, value_name: &str) -> std
    Ok(serializer.into_inner())
 }

+/// Enforce the signature-bound canonical body digest on a mutating disk RPC (backlog#1327).
+///
+/// Digest-bearing requests are verified against the canonical bytes rebuilt from the received
+/// wire fields — which cover both the msgpack `_bin` payloads and their JSON compatibility
+/// copies, so tampering with either encoding (or stripping `_bin` to force the JSON fallback
+/// decode) is rejected. Digestless requests fall back per
+/// `RUSTFS_INTERNODE_RPC_BODY_DIGEST_STRICT` (default: accept + convergence counter).
+fn verify_disk_mutation_digest<T>(
+    request: &Request<T>,
+    canonical_body: std::result::Result<Vec<u8>, std::num::TryFromIntError>,
+    op: &'static str,
+) -> std::result::Result<(), Status> {
+    let canonical_body =
+        canonical_body.map_err(|_| Status::invalid_argument(format!("{op} request length cannot be represented")))?;
+    verify_tonic_mutation_body_digest(request, &canonical_body)
+        .map_err(|err| Status::permission_denied(format!("{op} authentication failed: {err}")))
+}
+
 /// JSON compatibility string for a dual-encoded response field. Returns an empty string only when
 /// msgpack-only mode and its explicit fleet confirmation guard are both enabled; otherwise the
 /// legacy JSON encoding is retained for old peers. The paired `_bin` field is always sent.
@@ -171,6 +190,11 @@ impl NodeService {
        &self,
        request: Request<DeleteVolumeRequest>,
    ) -> Result<Response<DeleteVolumeResponse>, Status> {
+        verify_disk_mutation_digest(
+            &request,
+            rustfs_protos::canonical_delete_volume_request_body(request.get_ref()),
+            "delete_volume",
+        )?;
        let request = request.into_inner();
        if let Some(disk) = self.find_disk(&request.disk).await {
            match disk.delete_volume(&request.volume, request.force).await {
@@ -325,6 +349,11 @@ impl NodeService {
        &self,
        request: Request<DeleteVersionsRequest>,
    ) -> Result<Response<DeleteVersionsResponse>, Status> {
+        verify_disk_mutation_digest(
+            &request,
+            rustfs_protos::canonical_delete_versions_request_body(request.get_ref()),
+            "delete_versions",
+        )?;
        let request = request.into_inner();
        if let Some(disk) = self.find_disk(&request.disk).await {
            let mut versions = Vec::with_capacity(request.versions.len());
@@ -380,6 +409,11 @@ impl NodeService {
        &self,
        request: Request<DeleteVersionRequest>,
    ) -> Result<Response<DeleteVersionResponse>, Status> {
+        verify_disk_mutation_digest(
+            &request,
+            rustfs_protos::canonical_delete_version_request_body(request.get_ref()),
+            "delete_version",
+        )?;
        let request = request.into_inner();
        if let Some(disk) = self.find_disk(&request.disk).await {
            let file_info = match decode_msgpack_or_json::<FileInfo>(&request.file_info_bin, &request.file_info, "FileInfo") {
@@ -544,6 +578,11 @@ impl NodeService {
        &self,
        request: Request<WriteMetadataRequest>,
    ) -> Result<Response<WriteMetadataResponse>, Status> {
+        verify_disk_mutation_digest(
+            &request,
+            rustfs_protos::canonical_write_metadata_request_body(request.get_ref()),
+            "write_metadata",
+        )?;
        let request = request.into_inner();
        if let Some(disk) = self.find_disk(&request.disk).await {
            let file_info = match decode_msgpack_or_json::<FileInfo>(&request.file_info_bin, &request.file_info, "FileInfo") {
@@ -577,6 +616,11 @@ impl NodeService {
        &self,
        request: Request<UpdateMetadataRequest>,
    ) -> Result<Response<UpdateMetadataResponse>, Status> {
+        verify_disk_mutation_digest(
+            &request,
+            rustfs_protos::canonical_update_metadata_request_body(request.get_ref()),
+            "update_metadata",
+        )?;
        let request = request.into_inner();
        if let Some(disk) = self.find_disk(&request.disk).await {
            let file_info = match decode_msgpack_or_json::<FileInfo>(&request.file_info_bin, &request.file_info, "FileInfo") {
@@ -648,6 +692,11 @@ impl NodeService {
        &self,
        request: Request<DeletePathsRequest>,
    ) -> Result<Response<DeletePathsResponse>, Status> {
+        verify_disk_mutation_digest(
+            &request,
+            rustfs_protos::canonical_delete_paths_request_body(request.get_ref()),
+            "delete_paths",
+        )?;
        let request = request.into_inner();
        if let Some(disk) = self.find_disk(&request.disk).await {
            match disk.delete_paths(&request.volume, &request.paths).await {
@@ -750,6 +799,11 @@ impl NodeService {
        &self,
        request: Request<MakeVolumeRequest>,
    ) -> Result<Response<MakeVolumeResponse>, Status> {
+        verify_disk_mutation_digest(
+            &request,
+            rustfs_protos::canonical_make_volume_request_body(request.get_ref()),
+            "make_volume",
+        )?;
        let request = request.into_inner();
        if let Some(disk) = self.find_disk(&request.disk).await {
            match disk.make_volume(&request.volume).await {
@@ -774,6 +828,11 @@ impl NodeService {
        &self,
        request: Request<MakeVolumesRequest>,
    ) -> Result<Response<MakeVolumesResponse>, Status> {
+        verify_disk_mutation_digest(
+            &request,
+            rustfs_protos::canonical_make_volumes_request_body(request.get_ref()),
+            "make_volumes",
+        )?;
        let request = request.into_inner();
        if let Some(disk) = self.find_disk(&request.disk).await {
            match disk.make_volumes(request.volumes.iter().map(|s| &**s).collect()).await {
@@ -798,6 +857,11 @@ impl NodeService {
        &self,
        request: Request<RenameDataRequest>,
    ) -> Result<Response<RenameDataResponse>, Status> {
+        verify_disk_mutation_digest(
+            &request,
+            rustfs_protos::canonical_rename_data_request_body(request.get_ref()),
+            "rename_data",
+        )?;
        let request = request.into_inner();
        if let Some(disk) = self.find_disk(&request.disk).await {
            let file_info = match decode_msgpack_or_json::<FileInfo>(&request.file_info_bin, &request.file_info, "FileInfo") {
@@ -888,6 +952,11 @@ impl NodeService {
        &self,
        request: Request<RenameFileRequest>,
    ) -> Result<Response<RenameFileResponse>, Status> {
+        verify_disk_mutation_digest(
+            &request,
+            rustfs_protos::canonical_rename_file_request_body(request.get_ref()),
+            "rename_file",
+        )?;
        let request = request.into_inner();
        if let Some(disk) = self.find_disk(&request.disk).await {
            match disk
@@ -915,6 +984,11 @@ impl NodeService {
        &self,
        request: Request<RenamePartRequest>,
    ) -> Result<Response<RenamePartResponse>, Status> {
+        verify_disk_mutation_digest(
+            &request,
+            rustfs_protos::canonical_rename_part_request_body(request.get_ref()),
+            "rename_part",
+        )?;
        let request = request.into_inner();
        if let Some(disk) = self.find_disk(&request.disk).await {
            match disk
@@ -1083,6 +1157,7 @@ impl NodeService {
    }

    pub(super) async fn handle_delete(&self, request: Request<DeleteRequest>) -> Result<Response<DeleteResponse>, Status> {
+        verify_disk_mutation_digest(&request, rustfs_protos::canonical_delete_request_body(request.get_ref()), "delete")?;
        let request = request.into_inner();
        if let Some(disk) = self.find_disk(&request.disk).await {
            let options = match serde_json::from_str::<DeleteOptions>(&request.options) {
@@ -1113,6 +1188,7 @@ impl NodeService {
    }

    pub(super) async fn handle_write_all(&self, request: Request<WriteAllRequest>) -> Result<Response<WriteAllResponse>, Status> {
+        verify_disk_mutation_digest(&request, rustfs_protos::canonical_write_all_request_body(request.get_ref()), "write_all")?;
        let request = request.into_inner();
        let data_len = request.data.len();
        let metrics = runtime_sources::current_internode_metrics();
@@ -496,7 +496,7 @@ pub(crate) mod ecstore_rpc {
        LocalPeerS3Client, PEER_RESTDRY_RUN, PEER_RESTSIGNAL, PEER_RESTSUB_SYS, PeerRestClient, PeerS3Client,
        SERVICE_SIGNAL_REFRESH_CONFIG, SERVICE_SIGNAL_RELOAD_DYNAMIC, TONIC_RPC_PREFIX, normalize_tonic_rpc_audience,
        sign_ns_scanner_capability, sign_tonic_rpc_response_proof, verify_rpc_signature, verify_tonic_canonical_body_digest,
-        verify_tonic_rpc_signature,
+        verify_tonic_mutation_body_digest, verify_tonic_rpc_signature,
    };
    #[cfg(test)]
    pub(crate) use rustfs_ecstore::api::rpc::{
@@ -1567,6 +1567,10 @@ pub(crate) fn verify_tonic_canonical_body_digest<T>(request: &tonic::Request<T>,
    ecstore_rpc::verify_tonic_canonical_body_digest(request, canonical_body)
 }

+pub(crate) fn verify_tonic_mutation_body_digest<T>(request: &tonic::Request<T>, canonical_body: &[u8]) -> std::io::Result<()> {
+    ecstore_rpc::verify_tonic_mutation_body_digest(request, canonical_body)
+}
+
 #[cfg(test)]
 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)