Skip to content

docs(adr): ADR-0050 and ADR-0051, RFC-0008's certificate-auth ADRs (Proposed) - #236

Merged
gonzafg2 merged 2 commits into
mainfrom
docs/adr-0050-certificate-auth
Oct 30, 2026
Merged

gonzafg2 merged 2 commits into
mainfrom
docs/adr-0050-certificate-auth

Conversation

@gonzafg2

Copy link
Copy Markdown
Owner

What

RFC-0008 (Accepted 2026-07-01) adopts certificate authentication in Phase 2 and leaves its specifics to implementing ADRs. Until now no ADR or code existed for it. This PR adds two, both Proposed:

  • ADR-0050, the feature. The server accepts OpenSSH user certificates for Ed25519 keys (ssh-ed25519-cert-v01@openssh.com), signed by a CA listed in a new optional [auth] trusted_user_ca_keys file.
    • Trust anchor. The anchor is the config key RFC-0010 reserved, not cert-authority in authorized_keys, which ADR-0040 keeps refusing (Alternative 1).
    • CA algorithm. ssh-ed25519 for now, as RFC-0008's interim. The RFC-0012 implementation PR switches it to ssh-mldsa44-ed25519 only, so 0.1.0 ships with the composite CA.
    • Verification. Seven ordered checks in Expect<UserAuth>, with explicit bounds (16 KiB blob, 256 principals, nonce of at least 16 bytes): trusted CA before signature work, user role, validity window, exact principal match against the requested user name, and no critical options.
    • Critical options. None are honoured in this first cut, so any one refuses the certificate (draft §2.4, ADR-0040's rule). source-address and force-command can follow in their own ADRs.
    • Extensions. permit-pty feeds the ADR-0038 §3 gate. The rest are ignored.
    • Failure. A failure looks like an unknown key on the wire. A fixed reason goes to a diagnostic record, and no certificate string is logged before the CA signature verifies.
    • Interop gate. The ADR-0020 gate gains hard checks: the exec round trip with a certificate, and refusals for a wrong principal and an expired certificate.
  • ADR-0051, the audit fields RFC-0014 §5–§6 leave to this lane. cert_key_id, cert_serial (a decimal string), cert_ca_fingerprint and cert_principal appear on auth.succeeded, exec.started and exec.finished, all four or none. authenticated_identity and auth.failed do not change, and schema_version stays 1.

docs/plans/phase2-scoping.md points its certificate row and banner at the two ADRs.

Why Proposed

As with ADR-0028/0029 and ADR-0047/0048, both flip to Accepted in the implementing PR, which carries the code, the config key, the interop checks and the docs.

Evidence (2026-10-30)

  • draft-ietf-sshm-cert is at -01 (2026-06-15). §2.1.4 names ssh-ed25519-cert as equivalent to the vendor name. OpenSSH V_10_6_P1 registers only ssh-ed25519-cert-v01@openssh.com (ssh-ed25519.c:302).
  • server-sig-algs does not change. sshd builds it with sshkey_alg_list(0, 1, 1, ',') (kex.c:332), whose plain_only argument drops certificate types. RFC-0015 (docs(rfc): RFC-0015 — negotiation profile version 1, the KEXINIT contract 0.1.0 freezes #223) is unaffected.
  • Principal matching follows sshd without AuthorizedPrincipalsFile: an exact match of the user name (auth2-pubkey.c:599, sshkey_cert_check_authority). The validity window is sshkey.c:2454-2461, and SSHKEY_CERT_MAX_PRINCIPALS is 256 (sshkey.h:96).
  • sshkey_type_is_valid_ca accepts every non-certificate type, mldsa44-ed25519 included, so ssh-keygen 10.6 can sign with a composite CA.
  • No ML-DSA SSH CA support was found in Vault, step-ca or Teleport. Support for ML-DSA keys smallstep/cli#1698 added ML-DSA keys but states that its SSH commands do not support them yet. ADR-0050 records this as a negative consequence.

Decisions taken by the lead

  • No critical options in the first cut.
  • Composite CA only at 0.1.0, rather than keeping Ed25519 next to it (ADR-0050 Alternative 3).

Not in this PR

  • The code, the trusted_user_ca_keys key, the interop checks, the threat model §5.3.2/§6.3 and operator-guide updates, and the Status flips.
  • The CA flip itself, which ADR-0050 §4 assigns to the RFC-0012 implementation PR.

Refs #41, #109

https://claude.ai/code/session_01QpBZ4qDMPzZWerWPccRsSG

…roposed)

ADR-0050 accepts OpenSSH user certificates for Ed25519 keys, signed by
a CA listed in the new `[auth] trusted_user_ca_keys` key that RFC-0010
reserves. `cert-authority` in authorized_keys stays refused (ADR-0040).
The first cut honours no critical options. The CA is Ed25519 until the
RFC-0012 implementation, which switches it to ssh-mldsa44-ed25519 only,
so 0.1.0 ships with the composite CA. `permit-pty` feeds the ADR-0038
§3 gate, and server-sig-algs does not change.

ADR-0051 names the audit fields RFC-0014 leaves to this lane:
cert_key_id, cert_serial (a decimal string), cert_ca_fingerprint and
cert_principal, all four or none, on auth.succeeded and exec.*.

Both stay Proposed and flip to Accepted in the implementing PR.

Refs #41, #109

Claude-Session: https://claude.ai/code/session_01QpBZ4qDMPzZWerWPccRsSG
Signed-off-by: Gonzalo Fleming <gonzafg2@gmail.com>
@gonzafg2 gonzafg2 added the documentation Improvements or additions to documentation label Oct 30, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 30, 2026 •

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-30T20:04:59.215589Z cb182ab Manual request
ℹ️ 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.

@gonzafg2

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 👍

Reviewed commit: 834f664991

ℹ️ 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".

@claude

claude Bot commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

PR Review: ADR-0050 and ADR-0051, RFC-0008's certificate-auth ADRs (Proposed) (#236)

Decision: 🔴 Blocked
Iteration: Review #2 (Codex first review: 0 findings)
Lenses applied: Design decision — docs/adr/0050-accept-openssh-user-certificates.md, docs/adr/0051-certificate-audit-fields.md

Executive summary

This PR adds two Proposed implementing ADRs for RFC-0008 (Accepted): ADR-0050 specifies how the server accepts OpenSSH user certificates, and ADR-0051 specifies four new audit log fields for certificate sessions. Both documents are well-structured and mostly correct: the type-state machine integrity is preserved (Expect<UserAuth>, no accept-and-branch), pre-auth size bounds are stated (16 KiB blob, 256 principals), the interim Ed25519 CA is correctly handled under RFC-0008's migration window, and ADR-0051's log schema extension is sound. Four issues need resolution before the ADRs can be accepted.

Previous issues

Codex reviewed commit 834f664 and found no findings. Nothing to carry forward.

New findings

🔴 Blocking (resolve before merge)

docs/adr/0050-accept-openssh-user-certificates.md:177 — missing gate coverage (confidence: 90)

Problem: §8 (Interop gate) adds two negative checks — wrong principal and expired certificate — but not one for a certificate signed by a CA absent from trusted_user_ca_keys. Step 2 is the primary access-control check and runs before any signature work. Without a CI negative case, a regression removing step 2 passes all existing interop tests while accepting any CA-signed certificate.
Fix: Add a third negative check: a certificate signed by a CA key not present in trusted_user_ca_keys must fail to authenticate.
Anchor: CLAUDE.md — "The pre-authentication path is the highest-trust surface (§4.1)"; "Fail closed."

🟡 Important (should be resolved)

docs/adr/0050-accept-openssh-user-certificates.md:79 — unaccepted RFCs cited as authority (confidence: 88)

Problem: §3 justifies schema_version = 1 with "Under RFC-0016 ([#224])"; §2 references "RFC-0015 ([#223])" for the negotiation profile being unchanged. Both PRs are currently open/draft — not accepted. CLAUDE.md requires ADRs to implement accepted RFCs; citing an open proposal as settled governance inverts that relationship. If either RFC is revised before merging, ADR-0050's stated rationale becomes unsupported.
Fix: Mark both as Proposed (e.g. "RFC-0016 ([#224], Proposed)"), and either condition ADR-0050's acceptance on both RFCs merging, or re-ground the reasoning in already-accepted documents (ADR-0021 for the negotiation profile; RFC-0010/ADR-0029 for additive config keys).
Anchor: CLAUDE.md — "ADR — records a decision taken, often implementing an accepted RFC."

docs/adr/0050-accept-openssh-user-certificates.md:164 — principal check does not limit access (confidence: 83)

Problem: §7 correctly states "A principal does not select an account," but Alternative 4's rejection ("any certificate from a trusted CA could then log in under any name") implies the principal check creates a meaningful access barrier. Under ADR-0016 it does not: the client freely supplies the username, so any certificate from a trusted CA whose principals include a string the client can send clears step 6. The ADR never states the actual access boundary (the CA trust file) and never recommends a host-dedicated CA.
Fix: Add a note in §7 or a Negative consequence that until Phase-3 privilege separation (#43), trusting a CA grants access to every certificate that CA has ever issued; operators should use a CA dedicated to this host or service.
Anchor: Threat model §5.3.2; CLAUDE.md — "The pre-authentication path is the highest-trust surface."

docs/adr/0050-accept-openssh-user-certificates.md:210 — lifetime deferral untracked (confidence: 82)

Problem: The Negative consequences state "A maximum-lifetime setting is not part of this ADR," but unlike every other deferral in ADR-0050 (critical options → "each needs its own ADR"; principal→UID mapping → #43), this gap names no tracking artifact. RFC-0008 relies on short-lived certificates as the §5.3.2 revocation mitigant; with no server-side cap and no path to implementation, the mitigant is documentation-only indefinitely.
Fix: Open a tracking issue for [auth] max_cert_lifetime and link it here, or add "A later ADR adds [auth] max_cert_lifetime" to match the deferral pattern used elsewhere in the same section.
Anchor: Threat model §5.3.2; RFC-0008 §Decision.

…nds itself in accepted documents

Review on #236:
- §2 and §3 rest on ADR-0021 and ADR-0029; RFC-0015 and RFC-0016 are cited as Drafts.
- §7 and a Negative consequence state that, under ADR-0016, the principal match fixes the login name but not access: until Phase 3 every certificate a trusted CA issued gets in, so the CA should be dedicated to the server. Alternative 4 is reworded to match.
- §8 adds a third refusal: a certificate from a CA not in trusted_user_ca_keys.
- The maximum-lifetime setting is left to a later ADR, like the other deferrals.

Claude-Session: https://claude.ai/code/session_01QpBZ4qDMPzZWerWPccRsSG
Signed-off-by: Gonzalo Fleming <gonzafg2@gmail.com>
gonzafg2 added a commit that referenced this pull request Oct 30, 2026
…0 §8)

Follows the #236 review: the gate now covers check 2, the CA trust anchor, with a certificate that a second, untrusted CA signs. The operator guide and threat model §6.3 state that, until Phase 3, the CA file is the access boundary, and recommend a CA dedicated to the server.

Claude-Session: https://claude.ai/code/session_01QpBZ4qDMPzZWerWPccRsSG
Signed-off-by: Gonzalo Fleming <gonzafg2@gmail.com>
@gonzafg2

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: cb182ab07a

ℹ️ 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".

@gonzafg2
gonzafg2 merged commit 8fd0314 into main Oct 30, 2026
22 checks passed
@gonzafg2
gonzafg2 deleted the docs/adr-0050-certificate-auth branch October 30, 2026 22:25
@gonzafg2
gonzafg2 restored the docs/adr-0050-certificate-auth branch October 30, 2026 22:26
@gonzafg2
gonzafg2 deleted the docs/adr-0050-certificate-auth branch October 30, 2026 22:26
@gonzafg2

gonzafg2 commented Nov 1, 2026

Copy link
Copy Markdown
Owner Author

/oc

@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown

PR Review: ADR-0050 and ADR-0051, RFC-0008's certificate-auth ADRs (Proposed) (#236)

Decision: 🟡 Minor changes
Iteration: Review #3 (4 resolved, 0 open)
Lenses applied: Design decision — docs/adr/0050-accept-openssh-user-certificates.md, docs/adr/0051-certificate-audit-fields.md, docs/plans/phase2-scoping.md. The diff is docs-only, so the Rust (crates/) and CI/workflow lenses have nothing to inspect.

Executive summary

This PR adds two Proposed ADRs implementing RFC-0008 (certificate authentication) and RFC-0014 §5–§6 (the certificate audit fields), plus a governance-banner update in the Phase-2 plan. The design is conservative and mostly sound: CA trust before signature work, bounded pre-auth parsing (16 KiB blob, 256 principals, ≥16-byte nonce), fail-closed critical options, type-state preserved (Expect<UserAuth>, no accept-and-branch), additive audit fields that keep the authenticated_identity/executing_uid mandate, and an Ed25519 CA that stays inside RFC-0007's migration window with a scheduled composite flip before 0.1.0. One gap remains in the verification contract for the composite-CA phase.

Previous issues

The previous workflow review was claude[bot] Review #2 on 834f664; Codex's two reviews raised no issues. All four earlier findings were verified against the current head (cb182ab), not against the replies.

# Issue Status Notes
1 ADR-0050 cited RFC-0015/RFC-0016 as settled governance ✅ cb182ab re-grounds §2 on ADR-0021 and §3 on ADR-0029; both citations are marked Draft (§2 lines 70–72, §3 lines 82–83, Links lines 280, 283–284).
2 §8 had no negative case for a CA absent from trusted_user_ca_keys ✅ Lines 185–188 now list the third refusal, matching §5 check 2.
3 §7 did not say the CA file is the access boundary ✅ Lines 173–178, Negative 222–224 and Alternative 4 260–268 now state it.
4 Maximum-lifetime deferral had no tracking artifact ✅ Lines 218–221 now say "A maximum-lifetime setting is left to a later ADR".

New findings

🟡 Important (should be resolved)

docs/adr/0050-accept-openssh-user-certificates.md:122-123 — composite-CA verification invariant missing (confidence: 84)

Problem: §5 presents the seven checks as the complete acceptance criteria ("A certificate is accepted only if every check passes", line 109). Check 3 is the only CA-signature check and specifies only verify_strict for Ed25519. §4 (lines 98–102) flips the CA to ssh-mldsa44-ed25519 at the RFC-0012 implementation, and §5.1 already sizes the blob for that CA, yet no check records RFC-0006's security-critical rule: a composite signature is valid only if both component signatures verify. Because ADR-0050 is the durable record for this path and, once Accepted, cannot gain a new rule by in-place edit (ADR-0015), an implementer of the flip has no normative "both halves" requirement in the ADR. The reference to the "composite verifier" (lines 199–201) points at ADR-0035, which specifies the ML-DSA half via verify_with_context, not the logical-AND rule.

Fix:

3. **The CA signature verifies** over every field up to and including
   the signature key (draft §2.1.1). For `ssh-ed25519`, `verify_strict`.
   For `ssh-mldsa44-ed25519`, the signature is valid only if both the
   ML-DSA-44 and Ed25519 components verify (RFC-0006's invariant;
   RFC-0012 §Reference-level); never accept one half. The implementing
   PR adds a negative test: a certificate whose ML-DSA-44 half is
   corrupted and whose Ed25519 half is valid is refused.

Anchor: RFC-0006 §"The verification invariant (the security-critical rule)" — "A composite verification MUST require both component signatures to be valid — a logical AND"; RFC-0012 "The verification invariant" (lines 151–154).


opencode v2.2.3 · opencode-go/deepseek-v4.1-flash · github run

@gonzafg2

gonzafg2 commented Nov 2, 2026

Copy link
Copy Markdown
Owner Author

Thanks. The invariant is right, and the code follows it. ADR-0050 is not edited, because the rule is already normative where ADR-0050 §4 points: RFC-0006 §"The verification invariant" and RFC-0012 §Reference-level ("a composite signature is valid only if both component signatures verify").

#242 implements the composite CA (478c98a):

  • composite::VerifyingKey::verify returns true only when the ML-DSA-44 half (verify_with_context, with the label as context) and the Ed25519 half (verify_strict) both verify over the same M'.
  • cert::verify checks the CA signature through it.
  • The certificate test you asked for is there: each_half_of_the_ca_signature_is_checked corrupts one byte in the ML-DSA half, then one in the Ed25519 half, and both are BadCaSignature.
  • a_signature_is_refused_unless_both_halves_verify covers a valid half paired with a foreign one at the primitive level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant