feat(auth): add scoped temporary credentials - #5
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50f54c6b87
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
acking-you
left a comment
acking-you
left a comment
There was a problem hiding this comment.
Review
The slot table, derived secrets, namespace scoping, and WAL-before-expose path are in good shape. Temporary keys cannot reach admin operations, and same-name services in different namespaces do not collide.
Two issues should be fixed before merge: root rotation does not bind the decrypted first frame to the current administrator key, and first-frame replay protection is too weak for non-idempotent admin mutations.
Note on the existing inline comment in src/pb_server/client.rs: subscriber revocation is already raced at the subscribe dispatch site (tokio::select! around handle_client_conn). That path looks correct. Please still add a regression where credential A only connects to a service registered by B, then A is revoked — the current data-stream test uses one credential for both sides.
Suggested test additions
- In-flight V2 admin handshake that has already sent the prefix / derived the old key, then
root-key rotate, then completes the frame: must fail, must not bind the new admin lease. - Replay of a captured
key issuefirst frame after the bloom window (or after a relay restart): must not mint a second credential. - Cross-credential subscribe revoke, as above.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17217795df
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54e28d756c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if [ ! -s "$ADMIN_KEY_PATH" ] && [ -s "$LEGACY_KEY_PATH" ]; then | ||
| install -m 0600 "$LEGACY_KEY_PATH" "$ADMIN_KEY_PATH" |
There was a problem hiding this comment.
On an upgrade where /etc/pb-mapper/server.env explicitly supplies a new MSG_HEADER_KEY, the legacy key file still exists, and admin.key does not, this installer copies the legacy key before starting the service. The runtime then gives the newly created admin.key precedence over the environment credential, locking out clients configured with the intended key. The Docker entrypoint's equivalent migration now checks MSG_HEADER_KEY, but both GitHub and Gitee install scripts still omit that guard; leave migration to the runtime or account for the configured environment file.
Useful? React with 👍 / 👎.
| ), | ||
| response_session: None, | ||
| })?; | ||
| if !valid_checksum(datalen, checksum) || datalen > MAX_MSG_LEN { |
There was a problem hiding this comment.
When the Flutter embedded relay is running with its isolated app-local administrator key and the UI process has a different outbound credential, this check validates legacy framing with valid_checksum, which reads the process-global credential hash rather than the relay's key loaded immediately above. Consequently, a legacy client using the embedded relay's correct administrator key is rejected before decryption unless the unrelated outbound credential happens to have the same checksum. Compute the legacy checksum from the isolated relay key instead of global process state.
Useful? React with 👍 / 👎.
| const DEFAULT_REPLAY_WINDOW_SECONDS: u64 = 60; | ||
| const DEFAULT_REPLAY_FILTER_BYTES: usize = 1024 * 1024; | ||
| const MAX_INITIAL_PLAINTEXT_LEN: u32 = 64 * 1024; | ||
| const MAX_CONNECTION_CLOCK_SKEW_SECONDS: u64 = 5 * 60; |
There was a problem hiding this comment.
The two 60-second Bloom windows forget an accepted first flight after at most 120 seconds, while the same frame remains timestamp-valid for 300 seconds. An on-path observer can therefore wait for the filters to rotate and replay a captured Subcribe request without knowing the credential; for the default non-codec service, the attacker can discard the encrypted setup response and then use the raw forwarded connection to reach the registered service. Make replay retention cover the entire accepted clock-skew interval, or reduce timestamp acceptance to the cache's guaranteed lifetime.
Useful? React with 👍 / 👎.
| }; | ||
|
|
||
| pub const ADMIN_NAMESPACE: u64 = 0; | ||
| pub const DEFAULT_AUTH_STATE_DIR: &str = "/var/lib/pb-mapper/auth"; |
There was a problem hiding this comment.
When the released macOS or Windows binary is run as a normal user with pb-mapper server --port 7666, the new default attempts to create /var/lib/pb-mapper/auth; on macOS this is a root-owned system location, and the analogous drive-root path on Windows is not a reliable user-writable application directory. Authentication initialization therefore fails before the listener binds, making the default server command unusable on supported non-Linux release targets unless users discover and supply --auth-state-dir. Select a platform-appropriate writable state directory outside Linux system-service deployments.
Useful? React with 👍 / 👎.
1 participant
Summary
Add expiring, renewable temporary credentials without changing the existing persistent-control and on-demand data-connection topology. The administrator key remains the sole root credential, while temporary credentials can register, connect to, and inspect only their own namespace.
Related Issues
No linked issue.
Change Type
What Changed
Verification
cargo fmt --allcargo clippy --all-targets -- -D warningsCommands run:
cargo fmt --all cargo test --workspace --all-targets cargo clippy --workspace --all-targets -- -D warnings bash -n scripts/install-server-github.sh scripts/install-server-gitee.sh scripts/release/entrypoint/pb-mapper.shAdditional checks:
quick_validate.py.toastificationdependency, which requires Flutter 3.38 or newer; CI uses Flutter 3.44.9.0700auth state directory and0600administrator-key and instance-ID files.Compatibility / Risk
/var/lib/pb-mapper-server/msg_header_keyis migrated automatically when no new administrator key or environment override exists.Screenshots / Logs
Not applicable; the user-facing change is limited to credential validation text and the setup flow.