Skip to content

fix: correlate base answers and bind peer identity in sm and peer - #123

Merged
gomaja merged 1 commit into
mainfrom
fix/i108a-answer-correlation
Oct 15, 2026
Merged

gomaja merged 1 commit into
mainfrom
fix/i108a-answer-correlation

Conversation

@gomaja

@gomaja gomaja commented Oct 15, 2026 •

Copy link
Copy Markdown
Owner

Part 1 of the protocol contract decided in #108. sm.StateMachine,
sm.Client and peer.Manager differed in how strictly they matched base
answers to their requests and in which peer they accepted watchdog and
disconnect requests from.

  • CEA: the CER is tracked per connection generation with its Hop-by-Hop
    and End-to-End identifiers. A CEA that does not match, malformed or
    not, is discarded with a Warn record and a notification, and the
    handshake keeps waiting until its original deadline; the Manager no
    longer fails the connection on an identifier mismatch, and the Client
    learns the peer's identity from the matching CEA (RFC 6733 §§3, 5.3,
    6.2.1).
  • DPA: an answer completes a disconnect only if a DPR is pending on that
    generation and the command, both identifiers and the bound peer host
    match, checked before its Result-Code is read (§§5.4.2, 6.2.1).
  • StateMachine no longer answers DWR or DPR from a host other than the
    admitted peer, and such a DPR does not start Closing, as in the
    Manager (§6.3).
  • DWA: StateMachine validates a successful DWA as the Manager does
    (single identities and Result-Code, bound host and realm). Received
    traffic resets Tw, but an invalid, rejected or mismatched DWA neither
    clears the pending probe nor restores health (RFC 3539 §3.4.1,
    Appendix A).

Refs #108

Validation

  • make check passes locally for all modules: format, test (Go 1.27.2 and
    1.26.9), race, vet, lint, vuln, windows, secrets and codeql.
  • Wire tests over TCP, written to fail on main first, cover each row:
    identifier and host mismatches, a malformed uncorrelated CEA followed by the
    right one or by nothing, duplicate and stale answers, wrong-host DWR and DPR,
    and invalid, rejected or mismatched DWAs while SUSPECT. Captures show the
    wrong-host requests answered on main and unanswered here.
  • An independent review found one major gap (the malformed-CEA path) and a
    missing log record; both are fixed with their own tests, and mutating any
    row fails a named test.

Summary by cubic

Correlates base protocol answers in sm and peer so a peer cannot consume a pending request with an unrelated or malformed answer, and binds watchdog and disconnect requests to the admitted peer identity.

  • CEA must match the CER's identifiers on the same connection generation; mismatched or malformed CEAs are discarded with a Warn record and notification, and the handshake keeps its original deadline instead of failing (RFC 6733 §§3, 5.3, 6.2.1). The Client now learns the peer identity from the matching CEA.
  • DPA completes a disconnect only when a DPR is pending for that generation and command, both identifiers match, and Origin-Host matches the bound peer — all checked before the Result-Code (RFC 6733 §§5.4.2, 6.2.1).
  • StateMachine mirrors the Manager in rejecting DWR and DPR from a non-admitted host; such DPR also no longer enters Closing (RFC 6733 §6.3).
  • The StateMachine validates successful DWAs like the Manager (single Result-Code and identities bound to the peer), and while received traffic still resets Tw, an invalid, rejected, or mismatched DWA neither clears the pending probe nor restores health (RFC 3539 §3.4.1, Appendix A).

Refs #108.

Written for commit 82ddcc3. Summary will update on new commits.

View guided diff Turn on auto-fix

Part 1 of the protocol contract decided in #108. sm.StateMachine,
sm.Client and peer.Manager differed in how strictly they matched base
answers to their requests and in which peer they accepted watchdog and
disconnect requests from.

- CEA: the CER is tracked per connection generation with its Hop-by-Hop
  and End-to-End identifiers. A CEA that does not match, malformed or
  not, is discarded with a Warn record and a notification, and the
  handshake keeps waiting until its original deadline; the Manager no
  longer fails the connection on an identifier mismatch, and the Client
  learns the peer's identity from the matching CEA (RFC 6733 §§3, 5.3,
  6.2.1).
- DPA: an answer completes a disconnect only if a DPR is pending on that
  generation and the command, both identifiers and the bound peer host
  match, checked before its Result-Code is read (§§5.4.2, 6.2.1).
- StateMachine no longer answers DWR or DPR from a host other than the
  admitted peer, and such a DPR does not start Closing, as in the
  Manager (§6.3).
- DWA: StateMachine validates a successful DWA as the Manager does
  (single identities and Result-Code, bound host and realm). Received
  traffic resets Tw, but an invalid, rejected or mismatched DWA neither
  clears the pending probe nor restores health (RFC 3539 §3.4.1,
  Appendix A).

Refs #108
Copilot AI balanced review requested due to automatic review settings October 15, 2026 00:21

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Oct 15, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e315f286-0db5-4b0b-ac50-a557c242755e
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-15T00:24:22.326725Z 82ddcc3 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@gomaja
gomaja merged commit 41ef037 into main Oct 15, 2026
2 checks passed
@gomaja
gomaja deleted the fix/i108a-answer-correlation branch October 15, 2026 00:25
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