test: NTS wire-format interop tests + fix Next Protocol critical bit

[PenguinzTech]

Stacked on #54. Third branch in the chain.

Interop fix

RFC 8915 §4.1.5 requires the NTS Next Protocol Negotiation record's Critical Bit to be set. _build_nts_ke_response encoded it critical=False — strict clients (ntpsec, chrony) reject a response whose Next Protocol record isn't critical. Fixed to critical=True.

Interop tests (tests/test_nts_wire_interop.py, 24 tests)

Cross-checks the wire format against independent, spec-faithful references (no shared code with server.py) rather than self-consistency:

• NTS-KE records (RFC 8915 §4.1): response byte layout + critical bits parsed by an independent parser; server parses an independently-encoded chrony-style request; record encoding byte-identical to a reference encoder across odd body lengths.
• AES-SIV (RFC 5297): known-answer vector — the exact primitive chrony/ntpsec implement — so any AEAD drift is caught. Authenticator proven equal to AES-SIV(S2C).encrypt(empty, [AAD])[:16] (RFC 8915 §5.7); unsupported-AEAD IDs rejected (no MAC downgrade).
• Extension fields (RFC 7822): framing/padding cross-checked by an independent EF parser.
• End-to-end: a peer holding the S2C key authenticates the NTP response by recomputing the tag over the AAD; Unique Identifier echo (replay binding) verified.

Full ntp-server suite: 51 passing (27 existing + 24 new).

🤖 Generated with Claude Code

Summary by Sourcery

Ensure NTS wire-format interoperability by fixing the Next Protocol critical bit and adding cross-implementation tests for NTS-KE, AES-SIV, extension fields, and end-to-end authentication.

Bug Fixes:

• Set the NTS Next Protocol Negotiation record critical bit to comply with RFC 8915 and interoperate with strict NTS clients.

Tests:

• Add independent, spec-faithful interop tests for NTS-KE record layout and critical bits, AES-SIV authentication behavior, NTP extension-field framing, and end-to-end NTS response verification.

[sourcery-ai]

Reviewer's Guide

Fixes the NTS Next Protocol Negotiation record to set the Critical Bit per RFC 8915 for interop with strict clients, and adds a focused test suite that cross-checks the server’s NTS wire format, AES-SIV usage, and NTP extension-field framing against independent, spec-faithful reference implementations and vectors.

Sequence diagram for building NTS-KE response with critical Next Protocol record

sequenceDiagram
    participant NTPServer
    participant NTSKERecord

    NTPServer->>NTPServer: _build_nts_ke_response(aead_id, cookies)
    NTPServer->>NTSKERecord: encode(NTSKERecordType.NEXT_PROTOCOL, next_proto_body, critical=True)
    NTSKERecord-->>NTPServer: next_protocol_record
    NTPServer-->>NTPServer: assemble_response_with_next_protocol_record

Loading

File-Level Changes

Change	Details	Files
Ensure NTS Next Protocol Negotiation record is encoded as critical to comply with RFC 8915 and interoperate with strict NTS clients.	Update _build_nts_ke_response to pass critical=True when encoding the Next Protocol record. Document in comments that the Next Protocol record’s Critical Bit MUST be set and that strict clients reject non-critical responses.	ntp-server/bins/server.py
Introduce an interop-focused NTS wire-format test suite that validates encoding/decoding and cryptographic behavior against independent reference implementations and RFC vectors.	Add independent reference encoder/parser for NTS-KE records and NTP extension fields to avoid self-consistency masking bugs. Verify NTS-KE response record ordering, critical bits, and specific field values (Next Protocol, AEAD algorithm, End of Message) using the reference parser. Check that server-side NTSKERecord.encode output is byte-identical to the reference encoder across various body lengths and padding scenarios. Anchor AES-SIV behavior to the RFC 5297 known-answer vector and to the NTS authenticator definition, ensuring tags match an independent AESSIV implementation. Confirm unsupported AEAD IDs are rejected rather than silently downgraded, preserving MAC strength. Validate NTP extension-field framing and padding against an independent RFC 7822 parser for multiple payload sizes. Add an end-to-end test where a client recomputes the authenticator tag over the AAD using the S2C key, and verifies Unique Identifier echoing for replay binding.	ntp-server/tests/test_nts_wire_interop.py

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've left some high level feedback:

• The tests reach into private helpers like _build_nts_ke_response, _build_ef, and _compute_authenticator; consider exercising these behaviors via public interfaces where possible to avoid coupling tests to internal implementation details.
• The test module manipulates sys.path to import server; you may want to switch to package-relative imports (e.g., ntp_server.bins.server) to keep test imports robust to layout changes.
• Several AEAD- and type-related values (e.g., 15 for AES_SIV_CMAC_256) are hard-coded in tests; using the existing enum constants directly wherever possible would reduce the risk of divergence if the IDs change.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The tests reach into private helpers like `_build_nts_ke_response`, `_build_ef`, and `_compute_authenticator`; consider exercising these behaviors via public interfaces where possible to avoid coupling tests to internal implementation details.
- The test module manipulates `sys.path` to import `server`; you may want to switch to package-relative imports (e.g., `ntp_server.bins.server`) to keep test imports robust to layout changes.
- Several AEAD- and type-related values (e.g., `15` for AES_SIV_CMAC_256) are hard-coded in tests; using the existing enum constants directly wherever possible would reduce the risk of divergence if the IDs change.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}