Support wildcard host certificate principals

[tc-tlv]

See #1107

[tc-tlv]

@norrisjeremy , would you also like me to look into the SonarQube remark?

[norrisjeremy]

Hi @tc-tlv,

No, I don't think any changes are needed to for SonarQube.

Thanks!
Jeremy

[norrisjeremy]

I don't see any immediate issues with this.

@luigidemasi, do you see any problems with this change?

[norrisjeremy]

Does principal matching in OpenSSH also support other types of match patterns, such as:

host?.example.com => host1.example.com
192.168.1.* => 192.168.1.1

If so, should tests covering those also be included?

[tc-tlv]

Hey Jeremy, I looked at the code base of OpenSSH to find similar tests and I added them here.

[norrisjeremy]

Thanks!

[norrisjeremy]

@luigidemasi,

Do you see any issues with these changes?

Thanks!
Jeremy

[luigidemasi]

Hi @norrisjeremy,

I reviewed the changes in depth, including a comparison with the OpenSSH 10.3 sources, and I don't see any security issue with this PR.

What I checked:

• Order of checks is safe. Principal matching runs after the CA trust check and the certificate signature verification, so the patterns evaluated by the matcher always come from a certificate signed by a CA the user already trusts — a MITM can never get its own patterns evaluated. Acceptance is also still bounded by the host pattern of the @cert-authority line in known_hosts, which is checked independently of the principals.
• Semantics match OpenSSH 10.3. In V_10_3_P1, sshkey_cert_check_host() enables wildcard matching unconditionally for host certificates and calls match_pattern(name, principals[i]) — principal as pattern, lowercased hostname as string, which is exactly what this PR does. Like OpenSSH's match_pattern(), the jsch matcher is case-sensitive, supports only * and ?, and has no negation or comma-list handling (those exist only in match_pattern_list(), which OpenSSH does not use for cert principals). The existing empty-principals rejection — which 10.3 also hardened on the OpenSSH side — is preserved before the new loop, and wildcards stay host-cert-only, same as OpenSSH.
• Matcher equivalence and DoS. I differential-tested HostKey.matchesWildcardPattern against ports of OpenSSH's match_pattern (both the pre-10.5 recursive version and the current NFA): all patterns up to length 6 over {a,b,*,?} against all strings up to length 6, ~700k pairs, plus 200k randomized longer pairs — zero divergences, and all 69 test vectors in this PR agree with both. Runtime is linear: on a*a*a*a*…b against 100k characters it takes ~0.2 ms, an input on which the old recursive OpenSSH matcher blows up exponentially (that is what OpenSSH fixed in 10.5 with the NFA rewrite). So jsch is not exposed to that backtracking DoS at all.
• No regression. A glob pattern always matches itself, so every principal that matched exactly before still matches after this change, and no legal hostname contains * or ?.

One gap worth closing before merge — not a security issue: the new loop in checkHostCertificate is not exercised by any test (the added testPrincipalPatternMatching calls HostKey.matchesWildcardPattern directly, which is also why SonarCloud reports 0% coverage on new code). It is cheap to close without Docker: the repo's own CA key (src/test/resources/certificates/ca/ca_jsch_key) is the signer of the host fixtures, so a fixture minted with ssh-keygen -s ca_jsch_key -h -n '*.example.com' … plus a @cert-authority *.example.com … known_hosts line lets a plain unit test drive both the wildcard-accept and the reject branch through checkHostCertificate, and would also pin the "lowercase the hostname, principals as-is" behavior end to end.

Minor/optional: the pattern-level vectors test a HostKey method, so they might fit better in HostKeyTest next to the existing isWildcardMatched coverage; and a one-line javadoc note on checkHostCertificate that principals are matched as wildcard patterns (OpenSSH ≥ 10.3) would help future readers.

[norrisjeremy]

Hi @luigidemasi,

Thank you for the review!

@tc-tlv, can you review the comments from @luigidemasi and try to address them?

Thanks!
Jeremy

[tc-tlv]

I will look into it this week. Thanks guys.

[norrisjeremy]

Hi @tc-tlv,

Thank you for the additional changes!

@luigidemasi,
Can you confirm that the comments you made previously are addressed with the latest changes?

Thanks!
Jeremy

[tc-tlv]

I was wondering how the merge process happens, is this something initiated by one of you, or is this something I have to do? I am asking because I don't know. This is my first PR.

[luigidemasi]

@norrisjeremy @tc-tlv LGTM! Ready to merge from my side.
Non-blocking suggestion / nitpick: It might be worth adding an acceptance test covering the scenario where the first principal fails and a subsequent one matches, explicitly verifying full-list iteration.

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud