wolfsshd: bind FPKI certificate UPN realm to AuthorizedUPNDomains

[yosuke-wolfssl]

wolfsshd: bind FPKI certificate UPN realm to AuthorizedUPNDomains

Summary

When WOLFSSL_FPKI is enabled, wolfsshd validated a client certificate's UPN
SAN by comparing only the local part (the text before @) to the requested SSH
username and discarding the realm/domain. A client holding a valid certificate
from a trusted CA for alice@other-domain.example could therefore
authenticate as local user alice. This is most dangerous on the CA-only path
(no AuthorizedKeysFile set), where the CA-verified chain plus the local-part
match are the only gates.

This PR adds an opt-in allowlist of permitted UPN realms, AuthorizedUPNDomains.
When set, a certificate's UPN realm must exactly match one of the configured
domains (case-insensitive) in addition to the existing local-part match. When
unset, behavior is unchanged for backward compatibility and a warning is logged
so operators know the domain is not being checked.

Addressed by f_5580.

Problem

RequestAuthentication (apps/wolfsshd/auth.c) scanned the UPN SAN up to @
and matched only the local part:

for (idx = 0; idx < current->len; idx++) {
    if (current->name[idx] == '@') break;
}
if ((int)XSTRLEN(usr) == idx && XSTRNCMP(usr, current->name, idx) == 0) {
    usrMatch = 1;
}

The realm after @ was explicitly ignored, so any trusted-CA certificate whose
local part equalled the target username was accepted regardless of its domain.

Solution

• New config option AuthorizedUPNDomains — a whitespace/comma separated
  list of permitted UPN realms. Plumbed like the other WOLFSSHD_CONFIG string
  options (struct field, option table, copy/free, getter) and scoped to Match
  blocks via the existing config copy.
• New helper MatchUPNToUser() that performs the local-part match and, when
  an allowlist is configured, requires the certificate's realm to be present and
  to match a listed domain exactly (case-insensitive). It is length-bounded
  (nameSz) because certificate altName buffers are not guaranteed to be
  NUL-terminated.
• The multi-value ForceCommand/AuthorizedUPNDomains storage was de-duplicated
  into a shared SetListString() helper instead of a per-option handler.

Example configuration:

# /etc/ssh/sshd_config
TrustedUserCAKeys /etc/ssh/ca-cert-ecc.pem
HostCertificate   /etc/ssh/server-cert.pem

# only accept certificate UPNs in these realms
AuthorizedUPNDomains corp.example partner.example

Behavior:

AuthorizedUPNDomains	Certificate UPN	Result
unset	alice@anything	accepted (local part only) + warning logged
corp.example	alice@corp.example	accepted
corp.example	alice@other.example	rejected
corp.example	alice (no realm)	rejected

Backward compatibility

Unset is the default and preserves the prior local-part-only behavior, so
existing FPKI deployments are unaffected until they opt in. A WS_LOG_WARN is
emitted on each authentication attempt in which a certificate UPN matches while
the option is unset, making the unchecked-domain gap visible in logs.

The warning is deliberately per-attempt rather than one-time-per-process: a
one-time flag would require shared mutable state, which is racy in threaded
builds (Windows), and the log volume is bounded by the rate of successful
certificate logins on a deployment that has chosen not to opt in.

Security guidance

AuthorizedUPNDomains is opt-in, so the default remains fail-open with respect
to the certificate realm. FPKI deployments that rely on the CA-only path (no
AuthorizedKeysFile) should always set AuthorizedUPNDomains, since the
CA-verified chain and the local-part match are otherwise the only gates on
identity. The per-attempt warning exists to make an un-opted-in deployment
visible in the logs.

Files changed

• apps/wolfsshd/configuration.c, apps/wolfsshd/configuration.h — new option,
  getter, and shared SetListString() helper.
• apps/wolfsshd/auth.c, apps/wolfsshd/auth.h — MatchUPNToUser() and its
  use in RequestAuthentication; helper exported under WOLFSSHD_UNIT_TEST
  via WOLFSSHD_STATIC.
• apps/wolfsshd/wolfsshd.c — wolfsshd -? advertises a Build features: FPKI certificate UPN domain checking line so test scripts can detect the feature
  deterministically.
• apps/wolfsshd/test/test_configuration.c — unit tests for option parsing,
  Match-block copy, and MatchUPNToUser (match/mismatch, missing/empty realm,
  multi/comma/tab/CR-LF separators, case-insensitivity, and length-bounded
  buffers).
• apps/wolfsshd/test/create_sshd_config.sh,
  apps/wolfsshd/test/sshd_x509_upn_fail.sh,
  apps/wolfsshd/test/run_all_sshd_tests.sh — end-to-end negative test: a cert
  with realm example is rejected by a daemon configured with
  AuthorizedUPNDomains other.example.

Testing

• Unit tests (apps/wolfsshd/test/test_configuration): all pass. New
  test_MatchUPNToUser covers the separator set, case-insensitivity, and the
  length-bounded (non-NUL-terminated) buffer path. Negative controls confirm the
  vectors fail when enforcement is removed or when the helper reads past
  nameSz.
• Integration test: sshd_x509_upn_fail.sh gates on FPKI by grepping the
  daemon's help output, then asserts the daemon-side rejection reason
  (incorrect user cert sent) from the log with a before/after match count, so
  neither a non-FPKI build, a stale log line, nor an unrelated client failure
  can produce a false pass. Skips cleanly (exit 77) when FPKI is not compiled in.
• The existing positive test sshd_config_test_x509 now also sets
  AuthorizedUPNDomains example, matching the fixture certificate's UPN realm
  (keys/renewcerts.cnf: otherName = msUPN;UTF8:fred@example). This gives the
  allowlist-accept path end-to-end coverage, which it otherwise lacked.

[Copilot]

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

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1079

Scan targets checked: wolfssh-bugs, wolfssh-src

No new issues found in the changed files. ✅

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1079

Scan targets checked: wolfssh-bugs, wolfssh-src

No new issues found in the changed files. ✅

[aidangarske]

🐺 Skoll Code Review

Overall recommendation: COMMENT
Findings: 5 total — 5 posted, 0 skipped

Posted findings

• [Low] AuthorizedUPNDomains opt-in default keeps realm-unchecked (fail-open) UPN matching and warns on every cert auth attempt — apps/wolfsshd/auth.c:1619-1632
• [Medium] Existing positive x509 UPN test now depends on fixture cert realm being exactly 'example' — apps/wolfsshd/test/create_sshd_config.sh:32
• [Low] Unquoted variable expansions in new test script reduce robustness — apps/wolfsshd/test/sshd_x509_upn_fail.sh:6
• [Info] Multi-line function header comment on MatchUPNToUser — apps/wolfsshd/auth.c:1390-1392
• [Info] WSTRNCASECMP degrades to case-sensitive strncmp on MICROCHIP targets — apps/wolfsshd/auth.c:1441

Review generated by Skoll.

[yosuke-wolfssl]

Hello @aidangarske ,
Could you review this again please ?

[aidangarske]

Looks good i would like @ejohnstown to look over this though.