-
Notifications
You must be signed in to change notification settings - Fork 8
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
feat(ssh): allow to use pubkey auth #111
feat(ssh): allow to use pubkey auth #111
Conversation
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.
Hi, thanks a lot.
Overall it looks good, I suggest some small code changes to make it coherent with the way we declare plugin properties.
Signed-off-by: AlexandreBrg <[email protected]>
c6cf542
to
87c598a
Compare
Thanks for the review, I applied the changes |
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, thanks a lot.
Signed-off-by: AlexandreBrg <[email protected]>
5a8d22f
to
658310f
Compare
Signed-off-by: AlexandreBrg <[email protected]>
658310f
to
f48d805
Compare
@loicmathieu Tests should now be passing properly :) |
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 thanks a lot
What changes are being made and why?
👋
SSH plugin command was missing authentication with asymmetric keys. I added an option to determine which option will be used in the SSH command (
PASSWORD
vsPUBLIC_KEY
), with the ability to give a private key and a passphrase for it.Moreover, JSch has been replaced by this fork. It's a drop-in replacement, with enhanced capabilities, especially in cryptography. Original version was only able to run
ssh-rsa
keys, which is a method deprecated nowadays in OpenSSL, and even more, it's disabled by default. (see here).Based on my light research, this lib wasn't used by any other component, meaning it shouldn't impact other plugins. A little note here to talk about available algorithms, this lib will load various (most commons) algo depending on the java version is it built with (doc). If I remember well, Kestra must be built & run with at least java 17, so it should be good :D
How the changes have been QAed?
I'm running a simple docker-compose as described here, with the below flow and this additional docker compose service:
Setup Instructions
ssh-keygen -t ed255_19 -f keypair -P "" -N ""