Skip to content

fix(state-node): relay reads to members with member-side authorization; fix read auth + phantom history - #54

Merged
somasekimoto merged 17 commits into
mainfrom
fix/state-node-read-relay
Aug 13, 2026
Merged

somasekimoto merged 17 commits into
mainfrom
fix/state-node-read-relay

Conversation

@somasekimoto

@somasekimoto somasekimoto commented Jul 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes the read path for content a gateway-facing node does not replicate (bug #93 follow-up). After #53, write placement picks members by capacity/XOR distance, so the node the gateway talks to is often not a member of a content network. Writes relay correctly, but reads were broken in three stacked ways — found while E2E-testing the Drive UI (#47) against the 4-node AWS deployment:

  1. /state/* reads always 401'd. The SDK signs write requests itself via monas-account (Authorization: user:<hex> + P-256 signature), but forwarded the caller's (empty) auth headers for reads. → The SDK now signs read requests via the account, exactly like the write path. Explicit caller Authorization is still passed through.

  2. Phantom history defeated local-presence checks. crsl-lib's linear_history(genesis) returns [genesis] even when the node holds nothing, so ensure_content_local never pulled, and sync_from_peers requested ops since=genesis — which providers answer by skipping the Create operation, so a member that missed the initial push synced "0 operations from 2 providers" forever (observed on node4). → Both now gate on has_genesis (actual DAG node lookup).

  3. Non-member reads had no authorized path. ensure_content_local pulled ops via the read-only FetchOperations RPC, but members (correctly) reject that pull for non-members — the two #93-era fixes contradicted each other. Rather than dropping the membership gate, this implements the hardened read-relay the old SECURITY NOTE planned:

    • New ReadContent / ReadHistory relay RPCs carry the original caller's token, request signature and timestamp.
    • The member re-authenticates the caller and enforces the access policy (StateNodeService::authorize_read, now shared with the HTTP read path) before serving data / version / history.
    • Read endpoints relay when has_genesis is false; ensure_content_local is removed. Operations are never copied to non-members, and FetchOperations keeps its membership gate.
    • Member verdicts map back to typed errors so the relaying node returns the member's 401/403/404.

Guarantees, and what this PR does not cover

This PR makes reads reachable and authorized. It does not make relayed responses verifiable — a relaying node still has to take the member's word for the bytes. Response verification is PR #56, which stacks on this branch.

Do not merge #54 on its own. Without #56 there is no client-side check that the returned payload matches the requested version, so a hostile member can serve forged ciphertext. The two are a single safe unit; merge #54 → #56 in order.

Known limits tracked separately:

Deep-review fixes

Round 1 (207f0af)

  • Typed relay verdicts: relayed-read failures cross the wire as ContentResponse::ReadFailed { kind, message } (RelayReadErrorKind in the port layer) — no more substring matching on stringified errors.
  • authorize_read fails closed on a policy-store error instead of falling through to the no-policy allow.
  • get_version scoped to the content series: the version CID must belong to the authorized content's DAG, closing a cross-content version read (applied to local and relayed paths).
  • In-repo tests for previously E2E-only paths: relay-read happy path, typed 403 mapping, authorize_read owner/no-policy/deny/fail-closed, phantom-history sync gating, cross-content get_version rejection, fetch_encrypted contract, revoke-by-remote_content_id.

Round 2 — missing access policy is now a denial (c5afba0)

authorize_read allowed the read when the content had no access policy at all. That state is not proof the content is public: it is indistinguishable from "the owner policy operation has not replicated here yet", which is reachable because genesis and owner policy are separate operations (#64). A replica in that state served ciphertext and history to anyone. Missing policy now denies, with an error that tells the caller to retry or read from a node that has the policy. Tests cover both a missing policy and a genesis-only replica.

Round 3 — unproven DHT peers are no longer trusted as members (9e05c59)

A relay hands the caller's token and request signature to whatever resolve_members returns, then interprets the answer. That list has two very different origins and the code treated them identically:

  • a local ContentNetwork record, built from this content's ContentCreated event — the nodes the network actually assigned
  • DHT neighbours of sha256(content_id) — nothing ties them to the content, so anyone who can place a Peer ID near the key lands in the list

resolve_members now reports which it produced, and that provenance decides two things.

Auth verdicts from unproven peers no longer end the member loop. A 401/403 used to end it unconditionally. From an unproven peer that proves nothing — one hostile node squatting near the DHT key could deny every read and write by answering 403 first, and an honest but partially-synced replica can return 403 from a policy it has not finished replicating, stopping failover to a healthy one. Such a verdict is now remembered as the answer of last resort while the loop continues. (At this point the early exit was kept for local-record members; Round 5 removes it there too — see below.)

Credentials are still forwarded to every candidate, and that is correct. The member is the one that evaluates authorization, and it cannot do that without the caller's token and signature. It is also safe: authorization is proof-of-possession — the token is either a self-contained key id (a public key) or a delegated JWT whose aud is likewise a public key, and the request signature is verified against that aud key. A peer that captures both cannot mint a new request without the private key, and the captured signature is bound to one operation, resource, body and timestamp.

An earlier revision of this PR capped how many unproven candidates were tried. That was removed: a peer sitting first in DHT distance order receives the credentials regardless of any cap, so the cap only cut failover to legitimate members — an availability cost for no confidentiality gain.

The real remaining gap is member discovery itself — see Round 4, which closes part of it. What is left is #63, and docs/design.md states it rather than implying it is solved.

Round 4 — the publisher of a membership event is authenticated (8201797)

Round 3 leaned on the local ContentNetwork record being a better basis than a DHT guess. Checking that assumption showed it did not hold: ContentCreated and ContentNetworkManagerAdded were applied with no origin check at all, so any peer could plant a record on this node by naming it in member_nodes — and that record then read back as the trusted kind, which is what decides whether a 403 ends the member loop. ContentUpdated already called verify_source_peer_id; these two arms did not.

The event's publisher is now bound:

  • ContentCreated must come from the creator_node_id it names. create_content always publishes with creator_node_id == local_node_id, so every legitimate event satisfies this.
  • A membership change must come from a node already in the network it changes. ContentNetworkManagerAdded names no publisher (added_node_id is the node being added, usually us), so membership is the check that fits. With no local record there is nothing to check against and nothing to overwrite, so it is accepted as bootstrap.

ReceivedEvent::source now carries gossipsub's Message::source — the authenticated publisher — instead of propagation_source, the peer that forwarded it. The mesh forwards over multiple hops, so binding on the forwarder would reject honest multi-hop delivery and accept forged origins. Under MessageAuthenticity::Signed + ValidationMode::Strict the author field is required and signature-verified before delivery, so a forwarder cannot alter it.

MemberProvenance::Attested is renamed to LocalRecord. The old name claimed a cryptographic attestation that does not exist: events carry no owner signature, so the member set is the publisher's own claim even now that the publisher is authenticated. Two gaps remain, both needing owner-signed membership — a protocol change tracked in #63: the first record for a content has no prior membership to check against, and an authenticated member can still declare a member set of its choosing.

Round 5 — the remaining two arms, and the verdict that rested on them (746d1ad)

Round 4 hardened the two arms a review had named and stopped there. Enumerating all six arms of handle_sync_event found two more that mutate the ContentNetwork record with no publisher authorization:

  • ContentNetworkManagerRemoved had no check whatsoever. Any peer could delete our record by naming us as removed_node_id.
  • ContentDeleted called verify_source_peer_id against deleted_by_node_id — but the event names its own publisher, so any authenticated peer satisfied it and could delete our record.

Both now require the publisher to be a member of the network being changed, the same rule as the Added arm.

auth_verdict_is_authoritative now always returns false. It returned true for LocalRecord on the grounds that a listed member had evaluated the caller against the real access policy. That reasoning does not survive the above: the first record for a content is accepted with no prior membership to check against, so an attacker who wins that race lands in the candidate list, and its 403 would end the failover loop — a permanent denial of service against a legitimate caller, which is precisely the attack the DhtGuess case already guards against. The denial is still kept and returned when no candidate produces anything better; only the early exit is gone. The cost is bounded — one extra round trip per remaining candidate on a genuine denial — and errs toward availability, which is the right direction: the caller is refused either way, just later. Restoring the early exit needs owner-signed membership (#63).

provenance is kept and marked #[allow(dead_code)] rather than deleted — it is what #63 will read to restore the distinction.

The test named attested_member_auth_verdict_is_final passed a hardcoded true, so it never exercised the policy at all, only record_relay_read_error's mechanics. Renamed to say what it actually checks, and to record that no caller passes true today.

Verification

  • cargo test --workspace: all green, no failing test binaries.
  • Rust 1.97 cargo clippy --workspace --all-targets --deny warnings: clean.
  • Every new negative test in Rounds 4 and 5 was confirmed to fail with its check disabled — none are vacuous.
  • Rounds 4 and 5 verified on four real nodes (merged into feat(read-integrity): verified read path — payload authenticity, sender-authenticated CEK envelopes, single-use mutation signatures #56, which has the 4-node script): e2e 8/8, auth script 9/9, content propagated to all four, and zero membership-rejection warnings across all four node logs — the tightened checks accept every legitimate event in a real multi-hop mesh.
  • Deployed to the 4-node AWS environment and verified end-to-end through the Drive UI (feat(example-ui): minimal Drive-like UI on monas-sdk/gateway #47) with Playwright: create-in-folder (the old 408 repro), preview with version history, verify-integrity via relayed version read on a non-member node, edit, HPKE share/revoke, delete. (This predates Round 4.)

Note: this branch's start-local-nodes.sh still starts 3 nodes, so the local e2e script cannot satisfy min_replication_factor = 3 (the creator is excluded from the member set, leaving 2). The 4-node standardisation lives in #56; the full local e2e passes there. This is pre-existing on this branch and not introduced by the review fixes.

Note on edd429f (also in this PR): it adds two public SDK API fields — VerifyIntegrityInput.local_content_id (verify against the locally stored ciphertext; the state node never sees plaintext) and RevokeShareInput.remote_content_id (the state node only knows the series id). Both optional, backward compatible.

🤖 Generated with Claude Code

somasekimoto and others added 4 commits July 5, 2026 00:02
/state/latest-version, /state/history and /state/verify-integrity forwarded
the caller's Authorization header as-is, but the UI (and any client that
relies on the SDK's own signing, like create/update/delete already do) sends
only X-Request-Timestamp — so the state node's verify_read_access rejected
every read with 401 'Authorization header is required'.

Build the read auth the same way the write path does: sign
read:content:<timestamp> via monas-account and send
Authorization: user:<hex(pubkey)>. An explicitly provided Authorization is
still forwarded untouched.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
crsl-lib's linear_history(genesis) returns [genesis] even when the node
holds nothing for that content. Two paths relied on history emptiness and
broke multi-node reads:

- ensure_content_local treated the phantom [genesis] as 'already local' and
  never pulled from members, so reads on a non-member node returned
  phantom history plus 404 for data/version.
- sync_from_peers passed the phantom genesis as since_version; providers
  skip the Create operation for since==genesis, so a member that missed the
  initial push fetched '0 operations from 2 providers' forever and never
  converged (observed on node4).

Gate both on has_genesis, which checks the actual DAG node.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ization (bug #93 hardened read path)

A gateway-facing node that does not replicate a content (post-#53 routing
picks members by capacity/XOR, so the receiving node is often excluded)
could not serve reads: the previous approach pulled operations via the
read-only FetchOperations RPC, but members reject that pull for
non-members ('Peer ... is not a member'), leaving data/version reads
returning 404 while writes worked. This was the read-relay follow-up the
ensure_content_local SECURITY NOTE tracked.

Implement the hardened design instead of loosening FetchOperations:

- protocol: new ReadContent / ReadHistory relay RPCs carrying the original
  caller's auth token, request signature and timestamp; new HistoryData
  response.
- member side: the relay handler re-authenticates the caller and enforces
  the access policy (new StateNodeService::authorize_read, shared with the
  HTTP read path) before serving data/version/history from the local repo.
- caller side: read endpoints relay to resolve_members() targets when
  has_genesis is false, instead of pulling operations; ensure_content_local
  is removed. Member errors are mapped back to typed StateNodeError so the
  HTTP layer returns the member's verdict (401/403/404).
- reads never copy operations to non-members; FetchOperations keeps its
  membership gate.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tate node by series id

Two UI-blocking semantic bugs found in the Drive UI E2E:

- verify_integrity compared the caller's plaintext with the state node's
  stored bytes — but the state node only ever holds the ciphertext the SDK
  sent, so the check could never pass. VerifyIntegrityInput gains an
  optional local_content_id; when present the SDK loads its locally stored
  ciphertext (new ContentService::fetch_encrypted) and byte-compares it
  with the state node's version data. Without it the old comparison is
  kept for backward compatibility.

- revoke_share used the SDK-local version id for the post-revoke
  re-encryption PUT to the state node, which only knows the series id —
  the sync failed and the whole revoke rolled back. RevokeShareInput gains
  remote_content_id (same local/remote distinction UpdateContentInput
  already makes), falling back to content_id when absent.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@somasekimoto

Copy link
Copy Markdown
Contributor Author

Added edd429f with two more SDK fixes found while completing the UI E2E against the deployed 4-node environment:

  1. verify_integrity compared the caller's plaintext with the state node's stored bytes — the state node only ever holds the ciphertext the SDK sent, so the check could never return valid. VerifyIntegrityInput gains an optional local_content_id; when present the SDK byte-compares the state node's version data against its locally stored ciphertext (new ContentService::fetch_encrypted).
  2. revoke_share addressed the post-revoke re-encryption PUT with the SDK-local version id, which the state node doesn't know — the sync failed and the revoke rolled back. RevokeShareInput gains remote_content_id (same local/remote split as UpdateContentInput).

Final verification: full Drive-UI Playwright E2E against the redeployed nodes — 16/16 steps green, zero error toasts (account → folder → create-in-folder → history via relay → integrity valid → edit → share w/ HPKE decrypt proof → revoke → delete).

somasekimoto and others added 2 commits July 5, 2026 12:20
…pe_complexity

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…uthz, version scoping

Addresses deep-review round-1 findings on the read-relay path:

- Structured relay errors: relayed-read failures now cross the wire as
  ContentResponse::ReadFailed { kind, message } (RelayReadErrorKind in the
  port layer), replacing the substring matching on stringified errors that
  turned any message containing 'not found' into a 404 and silently coupled
  HTTP semantics to thiserror message wording. The member loop treats auth
  verdicts (401/403) as authoritative and short-circuits, so a member's
  policy decision can no longer be overwritten by a later member's
  transport failure; NotFound is preferred over transport errors otherwise.

- authorize_read fails closed: an error loading the access policy now
  denies (StorageError) instead of falling through to the no-policy allow.

- get_version is scoped to the content series: the version CID must belong
  to the authorized content's DAG (repo.get_genesis check), closing a
  cross-content read where any caller passing authorize_read for one
  content (or any no-policy content) could fetch arbitrary version CIDs.
  Applied to both the local HTTP path and the relayed path.

- Test coverage for the paths the review flagged as E2E-only: relay read
  happy path + typed 403 mapping, authorize_read owner/no-policy/deny/
  fail-closed branches, phantom-history sync gating (since=None vs
  incremental), cross-content get_version rejection, fetch_encrypted
  contract, and revoke syncing the state node by remote_content_id.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@somasekimoto
somasekimoto requested a review from Yu-da-1 July 9, 2026 09:23
@Yu-da-1

Yu-da-1 commented Jul 9, 2026

Copy link
Copy Markdown
Member

[セキュリティ] DHTフォールバックで得た未検証ピアに実credentialを転送し、応答を無検証で信頼している

問題

resolve_members は、ローカルに ContentNetwork レコードがない場合、Kademlia DHT の近接ピア (find_closest_peers) を「そのコンテンツのメンバー」として扱います。ここには暗号学的な検証やメンバーシップの確認が一切ありません。

let mut members: Vec<String> = match local_record {
    Some(network) => network.member_nodes_as_strings(),
    None => {
        let key = compute_dht_key(content_id);
        self.peer_network
            .find_closest_peers(key, self.min_replication_factor + 1)
            .await
            ...
    }
};

relay_read_data はこの members に対して、呼び出し元の本物の auth_token/request_signature をそのまま転送し(L637)、最初に成功した応答をそのまま信頼します(L640 Ok(result) => return Ok(result))。応答データに対する署名検証・completeness検証はどこにも存在しません(libp2p_network.rs の応答ハンドラでも素通し、relay_read_content/relay_read_history の実装含め sign/verify/Signature は0件でした)。

再現方法

実は、既存のハッピーパステスト自体がこの挙動を証明しています:

async fn test_relay_read_data_returns_member_payload() {
// Happy path: the member serves the read and the payload comes back.
let node_registry = MockNodeRegistry::new();
let content_repo = Arc::new(RwLock::new(MockContentNetworkRepository::new()));
let peer_network = Arc::new(
MockPeerNetwork::new()
.with_local_peer_id("node-1")
.with_closest_peers(vec!["node-2".to_string()])
.with_relay_read_data(b"cipher".to_vec(), "v1"),
);
let event_publisher = MockEventPublisher::new();
let crdt_repo = Arc::new(MockContentRepository::new());
let service: TestService = StateNodeService::new(
node_registry,
content_repo,
peer_network,
event_publisher,
crdt_repo,
"node-1".to_string(),
);
let result = service
.relay_read_data(
"content-1",
None,
&test_token(),
Some(&test_request_signature()),
None,
)
.await
.expect("relayed read should succeed");
assert_eq!(result, (b"cipher".to_vec(), "v1".to_string()));
}

async fn test_relay_read_data_returns_member_payload() {
    let content_repo = Arc::new(RwLock::new(MockContentNetworkRepository::new())); // ← "content-1" のContentNetworkレコードは無い
    let peer_network = Arc::new(
        MockPeerNetwork::new()
            .with_local_peer_id("node-1")
            .with_closest_peers(vec!["node-2".to_string()]) // ← DHTフォールバックで返る「メンバー」
            .with_relay_read_data(b"cipher".to_vec(), "v1"),
    );
    ...
    let result = service.relay_read_data("content-1", None, &test_token(), Some(&test_request_signature()), None).await
        .expect("relayed read should succeed");
    assert_eq!(result, (b"cipher".to_vec(), "v1".to_string()));
}

このテストでは content-1 の正規メンバーが誰かを一度も検証せず、DHTで返ってきた node-2 の回答をそのまま採用しています。node-2 を攻撃者ノードに、with_relay_read_data の内容を攻撃者が用意した任意のバイト列に置き換えても、コード上は全く同じパスを通り、同じように成功します。つまり以下を機械的に示せます:

  1. content-1 のContentNetworkレコードを持たないノードAが読み取りリクエストを受ける
  2. resolve_members がDHT近接性だけで選ばれた「メンバー」を返す(実際にそのコンテンツを保持しているかは未検証)
  3. ノードAはそのピアに、リクエスト元の生きた auth_token/request_signature を転送する(別ホストへのcredential漏洩)
  4. そのピアが返す (data, version) をノードAは無条件に信頼し、クライアントへ返す

再現テストとして、以下のように既存パターンをそのまま流用して「攻撃者ノード」を明示した形にできます(差分は node-2 → attacker-node のリネームと意図を示すコメントのみ):

#[tokio::test]
async fn test_relay_read_data_trusts_unverified_dht_fallback_peer() {
    // resolve_members が DHT フォールバックで返すピアは、実際にそのコンテンツの
    // 正規メンバーであることが一度も検証されない。この攻撃者ノードが返す
    // 任意のペイロードが、そのまま relay_read_data の戻り値として信頼されることを示す。
    let content_repo = Arc::new(RwLock::new(MockContentNetworkRepository::new())); // 正規メンバー情報なし
    let peer_network = Arc::new(
        MockPeerNetwork::new()
            .with_local_peer_id("node-1")
            .with_closest_peers(vec!["attacker-node".to_string()])
            .with_relay_read_data(b"FABRICATED_DATA".to_vec(), "fake-version-cid"),
    );
    let service: TestService = StateNodeService::new(
        MockNodeRegistry::new(), content_repo, peer_network,
        MockEventPublisher::new(), Arc::new(MockContentRepository::new()),
        "node-1".to_string(),
    );

    let result = service
        .relay_read_data("content-1", None, &test_token(), Some(&test_request_signature()), None)
        .await
        .expect("relayed read succeeds even though attacker-node was never a verified member");

    // 検証も membership チェックも無いまま、攻撃者の偽データがそのまま返る。
    assert_eq!(result, (b"FABRICATED_DATA".to_vec(), "fake-version-cid".to_string()));
}

「versionはCID(ハッシュ)だから偽装は自動検出できる」への補足

version/genesis_cid は crsl-lib の Node::content_id()(SHA-256ベースのCID、crdt_repository.rs:527-529)です。正規メンバーの内部ストレージは content-addressed なので正規メンバー自身は嘘をつけませんが、これは relay 境界(ネットワーク層)には及びません。受信側 (relay_read_data, libp2p_network.rs の relay_read_content) は返ってきた (data, version) に対して一度もハッシュ再計算・検証をしていません。ワイヤプロトコル上は ContentResponse::Data{data, version} を直接送るだけなので、攻撃者は正規のcrsl-lib DAGを経由する必要がありません。ハッシュ整合性チェックを追加しても「Mの回答の内部整合性」しか証明できず、「そのversionがX系列の正当な履歴(genesisからの親子チェーン)に属する」ことまでは証明できない点にも注意が必要です。

提案する修正

  • DHTフォールバックで得たピアは、relay対象にする前に「本当にそのコンテンツを保持する正当な権利があるか」を検証する(例: ContentNetwork 作成時の正当なメンバー証明、または応答データが genesis からの正当な履歴に属することの検証)。
  • 少なくとも、DHTフォールバック経由の未検証ピアには実 auth_token/request_signature を転送しない。

somasekimoto and others added 3 commits July 10, 2026 01:26
…th AES-GCM

Addresses the PR #54 review finding that relayed reads forward live
credentials to unverified DHT-fallback peers and trust their responses
blindly (#54 review comment).

Read signature binding:
- The read request signature message is now `read:{content_id}:{timestamp}`
  instead of the generic `read:content:{timestamp}`, matching the delete
  path which already signs over the content id. A signature forwarded to a
  relay member (or captured by a non-member node) can no longer be replayed
  to read other content; exposure is limited to the single content id
  within the 5-minute timestamp window.

Content encryption:
- Replace Aes256CtrContentEncryption with Aes256GcmContentEncryption
  (AEAD). Format changes from [iv16 || ciphertext] to
  [nonce12 || ciphertext || tag16]. Forged or bit-flipped payloads —
  including data substituted by an untrusted relay peer — now fail GCM
  authentication at the client instead of silently decrypting to
  attacker-controlled bytes (CTR was malleable and unauthenticated).
- BREAKING: previously stored CTR ciphertext can no longer be decrypted;
  there is no production data to migrate at this stage.

Membership verification itself (signed ContentNetwork records for the
gossip and DHT-fallback paths) is tracked separately in #55.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… race

CI moved from Rust 1.96.1 to 1.97.0 (released 2026-07-07), whose clippy
newly flags `for (_, v) in map.iter()` in monas-event-manager — iterate
map.values() instead. Unrelated to this PR's changes; fixed here so the
branch can go green.

Also retry the sled reopen in test_persistence: sled releases its file
lock asynchronously on Drop, so an immediate reopen can transiently fail
with WouldBlock on slow CI runners.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rmatting)

Second toolchain-drift lint hidden behind the first failure: clippy 1.97
flags redundant `&` in println! arguments in test_auth_generator. Full
workspace clippy now verified clean locally on 1.97.0.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
somasekimoto added a commit that referenced this pull request Jul 17, 2026
… handoff (#55)

Persist the task list (TaskCreate state doesn't carry across sessions),
the user-confirmed constraints (1 PR, no backward-compat, base = #54, must
build a real usable read path not just verification, member proof
dropped), and the open CEK question for the real read endpoint.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
somasekimoto added a commit that referenced this pull request Jul 18, 2026
…member creator (#55)

e2e は常に 4 ノード(creator + 3 members, MIN_REPLICATION_FACTOR=3)で行う。
create_content は creator を意図的に member から除外し local CRDT も持たせない
ため、3 ノードでは member 定足数を満たせず、また非 member 経由の relay read
経路(#54/#55 の本題)が一度も検証されない。

- start-local-nodes.sh: 3 -> 4 ノード起動(ci-e2e.sh と同一トポロジに統一)
- e2e-test.sh: 全ノードループを NODE_PORTS(4 ポート)に統一。Step 2.6 を新設し、
  非 member である creator ノード経由の read が data と version を返すことを
  明示的に assert(relay read の直接の回帰テスト。スモーク範囲に含まれるので
  CI でも毎回検証される)
- read 応答が Node CBOR になったため、ログはバイナリを直接表示せず
  サイズのみ出す。relay がある以上「data が返る = member」ではないので
  誤解を招くラベルも修正
- cleanup/test-local-nodes/test-with-auth も 4 ノード対応

検証: 4 ノードを別ポートで実起動し、ポート置換のみのコピーでスモーク実行
(3/3 成功)。member 3 ノード停止後は creator への read が失敗することも確認
(= creator はローカル非保持で、成功していた read は relay 経由だった証明)。

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…elay

# Conflicts:
#	monas-state-node/src/bin/test_auth_generator.rs
#	monas-state-node/src/test_utils.rs
@somasekimoto

Copy link
Copy Markdown
Contributor Author

ディープレビュー(2026-07-26)の blocker を実コードで検証し、本 PR のスコープでは解決しない設計課題として issue 化しました(セルフメモ)。

いずれも「認証された member discovery」「genesis への policy 原子的包含」という設計変更が必要で、本 PR(relay 到達性と member 側認可の修正)の範囲を超えます。

なお #63 の影響範囲は PR #56 のリクエスト署名統一により縮小しています。署名が operation / resource / timestamp / body digest に束縛されたため、転送された署名を別 content や別操作へ転用することはできなくなりました(残るのは同一 content・同一操作の 5 分以内の再実行)。

AES-CTR → AES-GCM への移行が design.md に反映されていない点も指摘を受けたので確認します。

…osed)

closes #64 の fail-open 部分。

authorize_read は policy load の I/O エラーには fail-closed だったが、
正常に None が返る経路は「まだ policy が無いだけ」として許可していた。
これは理論上の状態ではない: create genesis の Create payload は
access_policy: None で作られ、owner policy は別の Update operation として
届く。受信側の apply_operations は個々の失敗をログして継続し部分適用でも
成功を返し得るため、「genesis はあるが policy が無い」レプリカが成立する。

そのレプリカは認可契約が無いまま、任意の認証済み caller に暗号文と履歴を
渡してしまう。平文は CEK が守るが、メタデータ(サイズ・更新頻度・履歴)の
漏洩と、将来の暗号/鍵管理バグの影響拡大につながる。

policy 欠落を AuthorizationFailed として拒否し、エラーメッセージで
「このノードにまだ複製されていない可能性」を示して retry / 別ノードでの
read を案内する。

テスト: policy 欠落時の拒否と、genesis のみを持つレプリカでの拒否。
実機 4 ノードでフル e2e(8/8)が通ることも確認済み。

genesis へ owner policy を原子的に含める修正(部分適用を成功扱いしない件を
含む)は #64 に残す。

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
somasekimoto added a commit that referenced this pull request Jul 28, 2026
…ca test

#54 の fail-closed テストを取り込む際、#56 の timestamp 構造的必須化と
噛み合うよう明示的な timestamp を渡す。

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
somasekimoto and others added 4 commits July 28, 2026 18:19
…ementation)

commit 7880384 で AES-256-CTR から AES-256-GCM(AEAD)へ移行済みだが、
design.md のコンポーネント表と DDD レイヤー記述が CTR のままだった。
暗号文の保存形式(nonce || ciphertext || tag)も含めて実態に合わせる。

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A relay hands the caller's token and request signature to whoever `resolve_members`
returns, then interprets the answer. But that list has two very different origins
and the code treated them identically:

- a local `ContentNetwork` record, built from this content's `ContentCreated`
  event — the nodes the network actually assigned
- DHT neighbours of `sha256(content_id)` — nothing ties them to the content, so
  anyone who can place a Peer ID near the key lands in the list

`resolve_members` now reports which it produced, and that provenance decides two
things.

**Auth verdicts are only authoritative from attested members.** A 401/403 used to
end the member loop unconditionally. From an attested member that is right: it
evaluated the caller against the real policy, no other member would answer
differently, and continuing would leak the content's existence to someone already
refused. From an unproven peer it proves nothing — one hostile node squatting
near the DHT key could deny every read and write by answering 403 first, and an
honest but partially-synced replica can return 403 from a policy it has not
finished replicating, stopping failover to a healthy one. Such a verdict is now
remembered as the answer of last resort (it beats NotFound and transport errors,
and is what the caller sees if nothing better turns up) while the loop continues.

**Credentials reach a bounded number of unproven peers.** Every candidate tried is
one more unidentified party handed a live token and signature, so the failover
loop no longer walks the whole list — two keeps one retry for the ordinary
offline-or-lagging case. Attested members stay uncapped; they are the assigned
nodes and availability should use all of them.

This mitigates the disclosure rather than removing it. Eliminating it needs
capabilities that can be down-scoped to a single relay, operation and resource,
which is a protocol change out of scope here. The design doc states the boundary
rather than leaving it implied.

Tests: an unproven peer's verdict does not end the loop but is still returned if
nothing better arrives, and a later NotFound does not overwrite it; an attested
member's verdict is final; unproven candidate lists are capped while attested ones
are not. All three fail without the fix.
Forwarding the caller's token and request signature to a relay candidate is the
design, not a leak: the member is the one that evaluates authorization, and it
cannot do that without the credentials. Capping how many unproven candidates
receive them was the wrong response to that.

It also bought nothing. Authorization is proof-of-possession — the token is
either a self-contained key id (a public key) or a delegated JWT whose `aud` is
likewise a public key, and the request signature is verified against that `aud`
key. A peer that captures both cannot mint a new request without the private
key, and the captured signature is bound to one operation, resource, body and
timestamp. Meanwhile a peer that sits first in DHT distance order receives the
credentials regardless of any cap, so the cap only cut failover to legitimate
members — a real availability cost for no confidentiality gain.

What provenance still decides is unchanged and is the part that matters: a
negative authorization verdict is only authoritative from an attested member.

The remaining gap is member discovery itself — `ContentNetwork` records come
from `ContentCreated` events that are not signature-verified, so the
attested/unproven split is an approximation of a check that does not exist yet.
Stated in the design doc rather than implied.
`ContentCreated` and `ContentNetworkManagerAdded` were applied without any
origin check, so any peer could plant a `ContentNetwork` record on this node
by naming it in `member_nodes`. That record then read back as
`MemberProvenance::Attested`, which decides whether a 401/403 from a listed
peer is treated as final. `ContentUpdated` already called
`verify_source_peer_id`; these two arms did not.

Bind the authenticated publisher:

- `ContentCreated` must come from the `creator_node_id` it names.
  `create_content` always publishes with `creator_node_id == local_node_id`,
  so every legitimate event satisfies this.
- `ContentNetworkManagerAdded` names no publisher (`added_node_id` is the node
  being added, usually us), so the check that fits is membership: only a node
  already in the network we hold may change that network's member set. With no
  local record there is nothing to check against and nothing to overwrite, so
  the event is accepted as bootstrap.

`ReceivedEvent::source` now carries gossipsub's `Message::source` — the
authenticated *publisher* — instead of `propagation_source`, the peer that
forwarded it. The mesh forwards over multiple hops, so binding on the
forwarder would reject honest multi-hop delivery while accepting forged
origins. Under `MessageAuthenticity::Signed` + `ValidationMode::Strict` the
author field is required and signature-verified before delivery, so a
forwarder cannot alter it. The field is `Option<String>`; `None` means the
origin is unverifiable and origin-bound checks are skipped rather than trusted.

Rename `MemberProvenance::Attested` to `LocalRecord`. The old name claimed a
cryptographic attestation that does not exist: events carry no owner
signature, so the member set is the publisher's own claim even now that the
publisher is authenticated. Two gaps remain, both needing owner-signed
membership (a protocol change, tracked separately): the first record for a
content has no prior membership to check against, and an authenticated member
can still declare a member set of its choosing. design.md's "relay先の信頼度"
section is corrected to match.

Both new negative tests were confirmed to fail with the checks disabled.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S8xhYR7ZuFkiFMS91NUW6c
somasekimoto and others added 2 commits July 29, 2026 04:10
…trusting a planted record's verdict

The previous commit hardened `ContentCreated` and `ContentNetworkManagerAdded`
but stopped at the two arms that had been reported. Enumerating all six arms of
`handle_sync_event` found two more that mutate the `ContentNetwork` record with
no publisher authorization:

- `ContentNetworkManagerRemoved` had no check whatsoever. Any peer could delete
  our record by naming us as `removed_node_id`.
- `ContentDeleted` called `verify_source_peer_id` against `deleted_by_node_id`,
  but the event names its own publisher — so any authenticated peer satisfied
  it and could delete our record.

Both now require the publisher to be a member of the network being changed, the
same rule already applied to the Added arm.

`auth_verdict_is_authoritative` now always returns false. It returned true for
`LocalRecord`, on the grounds that a peer in a local record had evaluated the
caller against the real access policy. That reasoning does not survive the
above: the first record for a content is accepted with no prior membership to
check against, so an attacker who wins that race lands in the candidate list,
and its 403 would end the failover loop — a permanent denial of service against
a legitimate caller, which is precisely the attack the DhtGuess case already
guards against. The denial is still kept and returned when no candidate
produces anything better; only the early exit is gone. The cost is bounded (one
extra round trip per remaining candidate on a genuine denial) and errs toward
availability, which is the right direction here. Restoring the early exit needs
owner-signed membership, tracked in #63.

`provenance` is kept and marked `#[allow(dead_code)]` rather than deleted: it
is what #63 will read to restore the distinction.

The test named `attested_member_auth_verdict_is_final` passed a hardcoded
`true`, so it never exercised the policy — only `record_relay_read_error`'s
mechanics. Renamed to say what it actually checks, and to record that no caller
passes `true` today.

Both new tests were confirmed to fail with their checks disabled.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S8xhYR7ZuFkiFMS91NUW6c
The bullet list named two event types. The code binds the publisher on four:
`ContentNetworkManagerRemoved` also goes through `verify_source_is_existing_member`
— and it is the arm that *deletes* the local record, so strictly more
destructive than the `Added` arm that was listed — while `ContentDeleted`
needs both checks because it names its own publisher, and `ContentUpdated`
binds against `updated_node_id`.

No behaviour change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S8xhYR7ZuFkiFMS91NUW6c
@somasekimoto

Copy link
Copy Markdown
Contributor Author

@Yu-da-1 さん

指摘ありがとうございます。既存のハッピーパステストがそのまま攻撃の証明になっている、という示し方が特に的確でした。実際そのテストは今もリネームなしで通ります。

指摘を 3 つに分けて、それぞれ現状を書きます。2 つは未解決のままです。

1. 未検証ピアの応答を無条件に信頼する → #56 で対応、ただし保証範囲は限定的

PR #56 でクライアント側の検証を入れました。返ってきた Node の CID を再計算して要求した版 CID と照合し、AES-GCM で復号します。CEK を持たない相手は復号可能な偽 payload を作れません。

ただしご指摘の但し書きがそのまま当たっています。「ハッシュ整合性チェックを追加しても『M の回答の内部整合性』しか証明できず、『その version が X 系列の正当な履歴に属する』ことまでは証明できない」— その通りで、#56 が保証するのは payload の真正性までです。正規の writer が書いたか、canonical head か、本当に最新かは保証しません。

一度クライアント側に単調性チェック(last-seen 版より後退したら拒否)を実装しましたが、撤去しました。偽の parents を詰めた版で bypass できて防御にならない一方、結果整合性のもとでは正当な sync 遅延と攻撃を応答単体で区別できず、正規の read を壊す誤検知が残るためです。版の真正性には owner/writer 署名の trust anchor が必要で、crsl-lib の Node フォーマットに及ぶプロトコル変更になります。issue #59 で追跡しています。

なお relay 境界そのものは今も無検証で、検証はクライアント側です。これは「state node を信頼しない」設計に沿った配置ですが、relay が嘘をつけないことを意味しません。

2. 未検証ピアへの credential 転送 → 転送は続けます

ここはご提案と違う結論にしました。理由を書きます。

relay は access policy を持たないので、認可判断は member 側が実 policy に対して行います。credential を転送しなければ member 側で認可できず、この経路自体が成立しません。

そのうえで転送が安全と判断したのは、認可が Proof of Possession だからです。token は自己完結型の鍵 ID(公開鍵そのもの)か委譲 JWT で、後者の aud もまた自己完結型の鍵 ID です。リクエスト署名はその aud の鍵に対して検証されるので、token と署名の両方を傍受した相手も aud の秘密鍵を持たない以上、新しいリクエストを作れません。傍受した署名自体も操作・リソース・body digest・timestamp に束縛され、mutation はさらに使い切りです。

途中で「未検証候補は 2 件まで」という上限を実装しましたが、撤去しました。DHT 距離順で先頭に来る相手は上限があろうと credential を受け取るので機密性は改善せず、正当な member への failover を減らして可用性を落とすだけだったためです。

3. 未検証ピアを member 扱いする → 部分的に対応、根本は未解決

resolve_members が出自を報告するようにして、扱いを分けました。さらに調べたところ、ご指摘より一段悪い状態が見つかりました — ローカルの ContentNetwork レコード側も安全ではありませんでした。ContentCreated などの membership イベントが発行元を一切検証せずに適用されていたので、任意のピアが member_nodes にこちらを名指しするだけでレコードを植え付けられました。

現在は gossipsub の Message::source(署名検証済みの発行元)に束縛し、membership を変える全イベントで発行元を認可しています。また 401/403 による早期打ち切りは、出自によらず全廃しました — レコード自体が最初の 1 通で植え付けられる以上、そこに載った peer の 403 も最終判断にはできないためです。

ただし根本は未解決です。 発行元は認証できても、member 集合そのものは発行元の自己申告のままです。ご提案の「正当なメンバー証明」= owner 署名付き membership が必要で、issue #63 で追跡しています。

まとめ

指摘 状態
応答を無検証で信頼 #56 で payload 真正性まで。版・writer・latest は #59
credential 転送 転送を継続(PoP なので安全と判断)
未検証ピアを member 扱い 発行元認証と verdict 非採用まで。member 証明は #63

design.md に保証範囲を明記し、未達の部分は「解決済みに見える書き方」を避けました。2 と 3 についてご異論があれば伺いたいです。特に 2 は判断が分かれうる箇所だと思っています。

somasekimoto added a commit that referenced this pull request Aug 3, 2026
… read

Brings the Drive UI up to the read-integrity stack so every operation works
from the browser again.

Contract changes absorbed (all breaking, all previously silent 4xx/5xx):

- `/share` and `/share/revoke` now require `sender_private_key` — the CEK is
  wrapped with HPKE in **Auth mode**, which mixes the sender's private key in.
- `/share/decrypt` takes `sender_public_key` instead of the self-asserted
  `sender_key_id`; the recipient TOFU-pins it and rejects later envelopes that
  don't match.
- `KeyEnvelope.key_epoch` is carried through untouched. Revoke rotates the CEK
  and bumps the epoch, and recipients reject older epochs as rollback replay.
- Revoke returns `reissued_envelopes` for the *surviving* recipients. The UI
  swaps them into its registry — a recipient left on the pre-rotation envelope
  can no longer decrypt — and reports `token_invalidated_at`, which also voids
  tokens held by recipients that were not revoked.

New capability surfaced:

- `POST /state/read` (verified read) was entirely unused by the UI. The preview
  modal can now read any version back from the state node — relayed to a member
  when the contacted node isn't one — showing plaintext only after CID
  recomputation, AES-GCM decryption and a plain-CID recheck. The panel is
  explicit that this proves payload authenticity but *not* version freshness
  (issue #59).

Also corrects the UI's crypto claims: content encryption moved to AES-256-GCM
in #54, but 8 places still advertised the unauthenticated AES-256-CTR.

The registry key moves to v3. There is nothing to migrate — the GCM switch
invalidates every pre-existing ciphertext, so v2 entries could only fail on
open.

Verified against a real 4-node local cluster (gateway pointed at the non-member
node so reads exercise the relay path): e2e-verify.mjs 17/17, zero error
toasts, zero page errors. `tsc --noEmit` and `vite build` clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants