-
Notifications
You must be signed in to change notification settings - Fork 708
[JENKINS-76058] Fix OpenSSH Certificate Hostkey handling + Improve verifier #1141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Required by jenkinsci/mina-sshd-api-plugin#177 IIUC. |
| Properties.setThreadOverride(Properties.EMULATE_ORACLE, true); | ||
| // Generate MD5 fingerprint just like ssh-keygen | ||
| byte[] rawFingerprint = KeyUtils.getRawFingerprint(BuiltinDigests.md5.get(), serverKey); | ||
| return BufferUtils.toHex(':', rawFingerprint).toLowerCase(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommend Locale.ROOT as a general practice, though in this case I guess a hex string could not contain I.
| } catch (Exception e) { | ||
| return ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is some exception expected here? Seems like it should at least log a warning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not expected. It all goes does to implementation of Digest / decoder I guess.
We can add a warning.
|
Thanks @jglick |
olamy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
vwagh-dev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| return null; | ||
| } | ||
| AsymmetricKeyParameter parameters = PublicKeyFactory.createKey(serverKey.getEncoded()); | ||
| return new HostKey(serverKey.getAlgorithm(), OpenSSHPublicKeyUtil.encodePublicKey(parameters)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be serverKey.getAlgorithm() but sshAlgorithm. This is causing an issue https://issues.jenkins.io/browse/JENKINS-76077
OpenSshCertificatepublic key handling. If the aPublicKeyof typeOpenSsfCertificateis passed by SSHD verifier, then we must unwrap thegetCertPubKeyfor host key verification. (see bugfix: fix cert auth failed bug apache/mina-sshd#660)PublicKeyto aHostKeyif necessary (that would prevent future problem related to Algorithm type handling when No Verification is selected).Testing done
Submitter checklist