SSHD-1168, SSHD-1171, SSHD-1172: OpenSshCertificate fixes #197
Conversation
Use Long.compareUnsigned() to compare them to determine whether a certificate is still valid. OpenSSH uses ~0ULL to indicate "no expiration time".
Previously these certificates had an expiration date of April 6, 2030. This would have made tests fail after that date, and actually hid bug SSHD-1172. Adapt the OpenSSHCertificateTest to check the expected exception message. This makes the test catch possible errors like comparing certificate timestamps as signed integers (because we always check expiration before checking principals).
Add checks that only host key certificates are used for host keys, and only client certificates are used in pubkey authentication. Also check the validity of the certificate in the server's UserAuthPublicKey.
|
I have a pending PR I was going to open tonight that touches some of this too, and in mine I changed the I also found that there is some additional invalid encoding in the OpenSshCertificate code from the original certkeys PR that has gone unnoticed with the Critical Options/Extensions data. My upcoming PR was to include code which can easily generate signed OpenSshCertificates, but ended up making the aforementioned fixes/changes as well (preview: https://github.com/alex-sherwin/mina-sshd2/pull/1) @tomaswolf Would you prefer if I un-do my change of using |
|
@alex-sherwin sorry about that. I had been thinking about Instant, too, but in the end I decided to just remove these two methods since I didn't need them. Converting to Instant would also have to take care of that unsigned long issue, and I didn't see a way to do that cleanly; best I could think of was to saturate the 64bit value at LONG_MAX. But if you have a clean solution I have nothing against using Instant at all. If your change undoes some of what I did and solves it differently that's no problem. |
2 participants
Fixes: