Skip to content
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

FIX support SSH CA user signed keys in ssh_agent #1

Closed

Conversation

auphofBSF
Copy link

PR mscdex#808 publickey support for SSH certificates is great but I found it was failing with when the keys and signed certificates where in ssh_agent

The changes I propose in this PR enable signed certs ssh-rsa-cert-v01@openssh.com to be correctly handled by the SSH client when retrieved from SSH_AGENT. This is the default certificate type returned by Hashicorp Vault SSH CA signing.

This PR does not handle the multitude of possible cert types and robustly handle the errors if this where the case. I don't like submitting something not fully complete but I am a Newbie to this wonderful world of PublicKey and Security so would hope someone with suitable experience would more confidently and robustly fill in the general cases. Thansk to @TimWolla for original work implementing Publickey Certificate support and hope @mscdex will review and merge soon

@TimWolla
Copy link
Owner

TimWolla commented Aug 3, 2020

Hi @auphofBSF. Thank you for the contribution and the interest in my SSH CA patches. However I don't feel confident integrating this PR into my branch.

  • It contains quite a few TODOs (as you already acknowledged).
  • The commit message does not follow the style of the upstream project.
  • The code style itself does not appear to be 100% fitting either.

Additionally I can't really test your changes, the original PR was written to fit my use case and I intentionally made a minimally invasive patch to improve the chances for merging. Unfortunately it already looks like this is not going to happen any time soon and taking your commit probably would not improve chances. I suggest to either hold off your patch until my PR is merged or to create a concurring PR containing both my commit and your addition.

@TimWolla TimWolla closed this Aug 3, 2020
@auphofBSF
Copy link
Author

Thanks for your comments @TimWolla , I knew It would not go through but put it there as documentation for people like myself who are trying to use SSH certificates via the ssh-agent. I will raise it as an issue on mscdex:ssh2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants