Add support for OpenSSH certificates, resolve #31 #901

luigidemasi · 2025-09-17T16:23:38Z

No description provided.

norrisjeremy

Can you run mvn formatter:format to ensure that everything is formatted correctly?
Does this support SSH certs for host keys? If not, we need to add support for that too, as I'm opposed to only adding support SSH certs for user pubkey auth, w/o also adding support for SSH certs as host keys as well at the same time.

src/main/java/com/jcraft/jsch/JSch.java

src/main/java/com/jcraft/jsch/Buffer.java

src/main/java/com/jcraft/jsch/JSch.java

src/main/java/com/jcraft/jsch/OpenSshCertificateAwareIdentityFile.java

src/main/java/com/jcraft/jsch/OpenSshCertificateUtil.java

src/main/java/com/jcraft/jsch/OpenSshCertificate.java

src/main/java/com/jcraft/jsch/OpenSshCertificateAwareIdentityFile.java

src/main/java/com/jcraft/jsch/OpenSshCertificateBuffer.java

src/main/java/com/jcraft/jsch/OpensshCertificateParser.java

src/main/java/com/jcraft/jsch/OpenSshCertificateUtil.java

src/main/java/com/jcraft/jsch/OpenSshCertificateBuffer.java

src/main/java/com/jcraft/jsch/OpenSshCertificateUtil.java

davsclaus · 2025-09-23T05:21:21Z

@norrisjeremy is there more feedback to this, as it would be nice to get to the finish line, thanks

norrisjeremy · 2025-09-23T10:08:10Z

@norrisjeremy is there more feedback to this, as it would be nice to get to the finish line, thanks

None of my earlier feedback appears to have actually been incorporated: the PR author simply marked them all as resolved w/o making any of the suggested changes.
To quote part of my earlier review: "Does this support SSH certs for host keys? If not, we need to add support for that too, as I'm opposed to only adding support SSH certs for user pubkey auth, w/o also adding support for SSH certs as host keys as well at the same time."

luigidemasi · 2025-09-23T10:57:35Z

@norrisjeremy is there more feedback to this, as it would be nice to get to the finish line, thanks

None of my earlier feedback appears to have actually been incorporated: the PR author simply marked them all as resolved w/o making any of the suggested changes.

To quote part of my earlier review: "Does this support SSH certs for host keys? If not, we need to add support for that too, as I'm opposed to only adding support SSH certs for user pubkey auth, w/o also adding support for SSH certs as host keys as well at the same time."

Hi @norrisjeremy, thanks for following up. I marked the comments as resolved on GitHub because I had already addressed them locally and didn’t want to lose track of your suggestions. I’m currently adding support for host keys as well (it’s nearly finished) and I’ll push everything together in the next update. Please let me know if you have any additional suggestions in the meantime.

norrisjeremy · 2025-09-23T11:01:08Z

Hi @norrisjeremy, thanks for following up. I marked the comments as resolved on GitHub because I had already addressed them locally and didn’t want to lose track of your suggestions. I’m currently adding support for host keys as well (it’s nearly finished) and I’ll push everything together in the next update. Please let me know if you have any additional suggestions in the meantime.

Hi @luigidemasi,

Great, thanks! We are excited to have someone step up and contribute this work!

Thanks,
Jeremy

luigidemasi · 2025-09-29T17:35:39Z

@norrisjeremy

Does this support SSH certs for host keys? If not, we need to add support for that too, as I'm opposed to only adding support SSH certs for user pubkey auth, w/o also adding support for SSH certs as host keys as well at the same time.

I added the support for Host Certificate, let me know wdyt.

davsclaus · 2025-10-02T11:28:28Z

Great to see progress on this one. If we are getting close to the finish line it would be good to do the last review and update the reported findings so fingers crossed we can get this merged and released. Thank you.

norrisjeremy · 2025-10-02T11:42:55Z

Great to see progress on this one. If we are getting close to the finish line it would be good to do the last review and update the reported findings so fingers crossed we can get this merged and released. Thank you.

I probably won't have time to start reviewing this again until next week.

davsclaus · 2025-10-02T11:51:09Z

Great to see progress on this one. If we are getting close to the finish line it would be good to do the last review and update the reported findings so fingers crossed we can get this merged and released. Thank you.

I probably won't have time to start reviewing this again until next week.

Thanks for the update, no problem. Just glad we are on path to the goal line.

davsclaus · 2025-10-14T12:38:49Z

Sorry to bother - but would be good to get this reviewed

norrisjeremy · 2025-10-14T20:05:55Z

Sorry to bother - but would be good to get this reviewed

HI @davsclaus,

Yes, I haven't forgotten, I will try to review it when I have some time available.

Thanks,
Jeremy

norrisjeremy

Just a few initial comments, I still have a lot left to review.

src/main/java/com/jcraft/jsch/Buffer.java

src/main/java/com/jcraft/jsch/KeyExchange.java

src/main/java/com/jcraft/jsch/Session.java

src/main/java/com/jcraft/jsch/UserAuthPublicKey.java

src/test/java/com/jcraft/jsch/HostCertificateIT.java

src/main/java/com/jcraft/jsch/Session.java

src/main/java/com/jcraft/jsch/OpenSshCertificateHostKeyVerifier.java

src/main/java/com/jcraft/jsch/Session.java

src/main/java/com/jcraft/jsch/SignatureWrapper.java

src/main/java/com/jcraft/jsch/OpenSshCertificateHostKeyVerifier.java

norrisjeremy · 2025-10-18T13:16:23Z

src/main/java/com/jcraft/jsch/Session.java

Do we need to change anything here to properly support host certs?

norrisjeremy · 2025-10-18T13:20:04Z

src/main/java/com/jcraft/jsch/Session.java

          if (type.equals("rsa-sha2-256") || type.equals("rsa-sha2-512")
              || type.equals("ssh-rsa-sha224@ssh.com") || type.equals("ssh-rsa-sha256@ssh.com")
              || type.equals("ssh-rsa-sha384@ssh.com") || type.equals("ssh-rsa-sha512@ssh.com")) {
            type = "ssh-rsa";
          }


For example, do we need to add any adjustments here to map rsa-sha2-512-cert-v01@openssh.com & rsa-sha2-256-cert-v01@openssh.com to ssh-rsa-cert-v01@openssh.com?

src/main/java/com/jcraft/jsch/Session.java

src/main/java/com/jcraft/jsch/OpenSshCertificateHostKeyVerifier.java

norrisjeremy · 2025-10-19T14:01:53Z

src/main/java/com/jcraft/jsch/OpenSshCertificateHostKeyVerifier.java

+              + certificateId);
+    }
+
+    checkSignature(certificate, caPublicKeyAlgorithm);


We need to add support for the CASignatureAlgorithms option and then allow enforcement of it at some point:

CASignatureAlgorithms Specifies which algorithms are allowed for signing of certificates by certificate authorities (CAs). The default is: ssh-ed25519,ecdsa-sha2-nistp256, ecdsa-sha2-nistp384,ecdsa-sha2-nistp521, sk-ssh-ed25519@openssh.com, sk-ecdsa-sha2-nistp256@openssh.com, rsa-sha2-512,rsa-sha2-256 If the specified list begins with a ‘+’ character, then the specified algorithms will be appended to the default set instead of replacing them. If the specified list begins with a ‘-’ character, then the specified algorithms (including wildcards) will be removed from the default set instead of replacing them. ssh(1) will not accept host certificates signed using algorithms other than those specified.

src/main/java/com/jcraft/jsch/OpenSshCertificateHostKeyVerifier.java

src/main/java/com/jcraft/jsch/OpenSshCertificateAwareIdentityFile.java

src/main/java/com/jcraft/jsch/JSch.java

src/main/java/com/jcraft/jsch/KeyExchange.java

src/main/java/com/jcraft/jsch/OpenSshCertificateAwareIdentityFile.java

src/main/java/com/jcraft/jsch/OpenSshCertificateBuffer.java

src/main/java/com/jcraft/jsch/OpenSshCertificateHostKeyVerifier.java

src/main/java/com/jcraft/jsch/OpenSshCertificateAwareIdentityFile.java

src/main/java/com/jcraft/jsch/OpenSshCertificateBuffer.java

src/main/java/com/jcraft/jsch/KeyExchange.java

src/main/java/com/jcraft/jsch/OpenSshCertificateParser.java

src/main/java/com/jcraft/jsch/SignatureWrapper.java

src/main/java/com/jcraft/jsch/UserAuthPublicKey.java

src/test/java/com/jcraft/jsch/HostCertificateIT.java

src/test/java/com/jcraft/jsch/UserCertAuthIT.java

src/test/resources/certificates/host/Dockerfile

norrisjeremy

It makes me a bit nervous how much of the new parsing code for handling certs is converting everything to String objects, since much of the existing code for parsing regular public keys simply does it by operating on byte[] objects.

I worry this could potentially result in large changes of overall memory usage & garbage collection pressure by JSch.

src/main/java/com/jcraft/jsch/JSch.java

src/main/java/com/jcraft/jsch/OpenSshCertificateUtil.java

src/main/java/com/jcraft/jsch/OpenSshCertificateHostKeyVerifier.java

norrisjeremy · 2025-10-20T09:14:31Z

Hi @luigidemasi,

Why have you marked many of the comments I have made as resolved even though no changes have been pushed?

Thanks,
Jeremy

src/main/java/com/jcraft/jsch/SignatureWrapper.java

src/main/java/com/jcraft/jsch/OpenSshCertificateUtil.java

src/main/java/com/jcraft/jsch/OpenSshCertificateAwareIdentityFile.java

luigidemasi · 2025-10-20T11:41:21Z

Hi @luigidemasi,

Why have you marked many of the comments I have made as resolved even though no changes have been pushed?

Thanks, Jeremy

Hi @norrisjeremy, same as #901 (comment)

norrisjeremy · 2025-10-20T12:15:25Z

Are there integration tests covering cases in which CA key type doesn't match the user/host key type?

I.e., I believe it is valid for an ssh-ed25519 type CA key to be used as the signing key for an ecdsa-sha2-nistp256-cert-v01@openssh.com type cert.

Assuming my understanding is correct, we should construct various combinations of certs in which the cert algorithm doesn't match the CA key algorithm for integration tests.

luigidemasi · 2025-10-20T12:59:49Z

Are there integration tests covering cases in which CA key type doesn't match the user/host key type?

I.e., I believe it is valid for an ssh-ed25519 type CA key to be used as the signing key for an ecdsa-sha2-nistp256-cert-v01@openssh.com type cert.

Assuming my understanding is correct, we should construct various combinations of certs in which the cert algorithm doesn't match the CA key algorithm for integration tests.

You are 100% correct that SSH allows the CA key type to be different from the certificate key type it signs (i.e., cross-signing is permissible).

I have coverage for this specific scenario. In both HostCertificateIT and UserCertAuthIT, I use an ssh-ed25519 key as the Certificate Authority, and then I generated test certificates for every other supported key type (including ECDSA, RSA, etc.) to ensure successful signing and validation.

I believe this covers the various combinations you mentioned.

norrisjeremy · 2025-10-20T13:20:24Z

Are there integration tests covering cases in which CA key type doesn't match the user/host key type?
I.e., I believe it is valid for an ssh-ed25519 type CA key to be used as the signing key for an ecdsa-sha2-nistp256-cert-v01@openssh.com type cert.
Assuming my understanding is correct, we should construct various combinations of certs in which the cert algorithm doesn't match the CA key algorithm for integration tests.

You are 100% correct that SSH allows the CA key type to be different from the certificate key type it signs (i.e., cross-signing is permissible).

I have coverage for this specific scenario. In both HostCertificateIT and UserCertAuthIT, I use an ssh-ed25519 key as the Certificate Authority, and then I generated test certificates for every other supported key type (including ECDSA, RSA, etc.) to ensure successful signing and validation.

I believe this covers the various combinations you mentioned.

Excellent, thanks!

… code review

… Host Certificate

… code review for Host Certificate support

… code review for Host Certificate support - part2

sonarqubecloud · 2025-10-22T14:00:01Z

Quality Gate passed

Issues
178 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

norrisjeremy · 2025-10-22T14:20:22Z

src/main/java/com/jcraft/jsch/OpenSshCertificateHostKeyVerifier.java

+    // any principal of the specified type."
+    // Empty principals in a host certificate mean the certificate is valid for any host.
+    Collection<String> principals = certificate.getPrincipals();
+    if (principals != null && !principals.isEmpty()) {


I'm a bit confused: is this allowing host certs with empty or missing principals?
Doesn't that contradict draft-miller-ssh-cert-05 section 3.1 & draft-miller-ssh-cert-05 section 3.3?
Or am I misunderstanding?

3.1. Accepting certificates ... * MUST verify that at least one of the listed certificate principals is accepted ...

3.3. Use in host authentication ... Certificate host keys list hostnames and/or IP addresses for the host in their "principals" section. Clients MUST match the hostname or address thay are using for the host against the principals in this list and refuse to authorise the certificate if it is absent. ...

It's here: https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/Attic/PROTOCOL.certkeys?annotate=HEAD
but I agree with you that probably it's better to do the opposite, no principals means validation fails.

Interesting.
Can you validate how OpenSSH behaves?
Does it's ssh client reject a host that presents a host cert with empty/missing principals (aligning with draft-miller-ssh-cert-05)?
Or does it accept it (aligning with the original PROTOCOL.certkeys)?

I think OpenSSH rejects host certs with empty principals if I'm reading through their code correct:
https://github.com/openssh/openssh-portable/blob/0ffb76c6590800958777cd0f7b1aaae19c74fa3f/sshkey.c#L2419.

I think OpenSSH rejects host certs with empty principals if I'm reading through their code correct: https://github.com/openssh/openssh-portable/blob/0ffb76c6590800958777cd0f7b1aaae19c74fa3f/sshkey.c#L2419.

nope, require_principal is 0 in sshkey_cert_check_host method:

sshkey_cert_check_host(host_key, hostname, 0, options.ca_sign_algorithms, &fail_reason)

https://github.com/openssh/openssh-portable/blob/0ffb76c6590800958777cd0f7b1aaae19c74fa3f/sshkey.c#L2470

while is 1 when validating a user certificate:

if (sshkey_cert_check_authority_now(key, 0, 1, 0, use_authorized_principals ? NULL : pw->pw_name, &reason) != 0) goto fail_reason;

https://github.com/openssh/openssh-portable/blob/0ffb76c6590800958777cd0f7b1aaae19c74fa3f/auth2-pubkey.c#L565

To summarize:

Empty principal lists are formally valid for both user and host certificates

An empty principal list creates a "wildcard certificate" - valid for any principal

There's no RFC requirement that says user certificates must have principals
any certificate with empty principal list is formally valid, but are wildcard certificates and can be used by anyone.

OpenSSH makes a security-minded choice that goes beyond the specification, but this is OpenSSH policy, not protocol specification.

Hi @norrisjeremy,

I’m just circling back following your note from earlier this month. I completely understand that things can get busy, so no worries at all if you haven't had the chance to dig back into this yet.

However, since I’m keen to unblock this PR, would it help to offload this review to another maintainer? Is there someone else you trust to take a look?

Luigi.

Hi @luigidemasi,

Apologies, it's been a busy time lately, I'm hoping to find time to circle back to this soon.

Thanks,
Jeremy

thanks, we are close to the finish line for this PR. Maybe any extra work can be in a new tricket/PR to avoid this being stalled for very long time.

Hi @davsclaus,

I know you and other folks are extremely anxious to see this land, but I would ask that you remain patient.
I've been busy lately with some other work, but I'm hoping to find time soon to dedicate to reviewing this more.

Thanks,
Jeremy

Hi @norrisjeremy

Sorry to bother you, but we are coming to a close to 2025 and this PR has been open for 3+ months now.

Are you considering to meet us half-way or something? I know its ungrateful work to maintain open source, but it also adds bad vibes when PRs are dragged along for a long time.

To my understanding this is new feature that is designed to not affect any existing functionality (forgive me if I got that wrong). And if so could this new feature then be documented that "there is a spec clarification in progress .. so use this with that in mind, that it can be changed when the clarification is provided".

Regards

Claus Ibsen

norrisjeremy suggested changes Sep 17, 2025

View reviewed changes

norrisjeremy reviewed Sep 17, 2025

View reviewed changes

norrisjeremy reviewed Sep 18, 2025

View reviewed changes