-
Notifications
You must be signed in to change notification settings - Fork 255
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
Support multiple SSH keys #14
Conversation
What would the expected input be fore this using the action? I'm getting errors with the yml file when trying:
also |
So looks like
is valid as well as using a Here's my current action output for this branch:
|
I've swapped out the KNOWN WORKING value:
with a second token that I assumed was wrong and it does work, so I've confirmed my TOKENS are valid on my main repo. So I'm still uncertain how to provide the multiple values to this 😞 |
index.js
Outdated
@@ -17,7 +17,9 @@ try { | |||
core.exportVariable('SSH_AUTH_SOCK', authSock); | |||
|
|||
console.log("Adding private key to agent"); | |||
child_process.execSync('ssh-add -', { input: core.getInput('ssh-private-key') }); | |||
core.getInput('ssh-private-key').split(/(?=-----BEGIN)/).forEach(function(key) { | |||
child_process.execSync('ssh-add -', { input: key }); |
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.
It'd be nice to calculate a fingerprint and log it so that people would understand which key gets loaded. It's good for troubleshooting as there's no other ways to see what's going on.
Use something like this maybe: https://developer.github.com/apps/building-github-apps/authenticating-with-github-apps/#verifying-private-keys.
try this: with:
ssh-private-key: |
${{ secrets.SSH_PRIVATE_KEY }}
${{ secrets.SQLITE_ENCRYPTED_SSH_TOKEN }}
${{ secrets.SQLITE_COMMERCIAL_SSH_TOKEN }} Pipe will preserve the new lines in YAML. |
Nice idea that you can keep the keys in separate secrets and concatenate them this way – makes it waaaay easier if you need to rotate one key only. |
README.md
Outdated
@@ -36,6 +36,11 @@ jobs: | |||
``` | |||
4. If, for some reason, you need to change the location of the SSH agent socket, you can use the `ssh-auth-sock` input to provide a path. | |||
|
|||
If you need to use multiple SSH keys (for example because you're using deployment keys which belong to one repository each), you can put all those keys into the secret value one after another. | |||
This action will add all the keys to the `ssh-agent` one-by-one. `ssh` will then automatically figure out the right keys for each connection. |
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.
I feel like this needs some disclaimer that if you add too many keys, ssh-agent will feed all of them to ssh servers in the order they are added. Most servers have a limit of attempts to authenticate with a key. For example, if the server has a limit of 5 keys and the correct one is number 6 in the sequence, ssh client will use 5 first keys and then ssh server will interrupt the auth flow and that right one won't even be tried out.
f3175fa
to
8aa4b4b
Compare
In fact, there is already some output for each key loaded: https://github.com/webfactory/ssh-agent/pull/14/checks?check_run_id=384036303#step:4:7 |
Ah, ok. Then you could just add |
@webknjaz weird... I have re-generated the demo keys and now Do you know if the keys need to be in a particular format? |
Just PEM should be okay. Have you checked that there's no extra garbage added? Are you talking about https://github.com/webfactory/ssh-agent/pull/14/files#diff-d66f8d1a4662d10fdd69fc1726011da9R15-R16? I'm not sure if you can put comments in the middle of a string literal block in YAML.
|
This https://stackoverflow.com/a/59595773/595220 hints that there should be LF in the end maybe... |
Oh, and sometimes if you copy the key from a terminal you can accidentally copy trailing spaces at the end of each line. Have you checked this assumption? I mean: ------BEGIN ... PRIVATE KEY-----
-somesecretbytessomesecretbytessomesecretbytes
-somesecretbytessomesecretbytessomesecretbytes
-somesecretbytessomesecretbytessomesecretbytes
-somesecretbytessomesecretbytessomesecretbytes
-somesecretbytessomesecretbytessomesecretbytes
------END ... PRIVATE KEY-----
+-----BEGIN ... PRIVATE KEY-----
+somesecretbytessomesecretbytessomesecretbytes
+somesecretbytessomesecretbytessomesecretbytes
+somesecretbytessomesecretbytessomesecretbytes
+somesecretbytessomesecretbytessomesecretbytes
+-----END ... PRIVATE KEY----- |
Just wanted to confirm the As of now looks like all good to go with following output in my action:
Is there any security issue that could come up with printing the values in the build like it's doing currently ^^^? I've removed them from this message and replaced with |
@bradmartin yeah, that’s just debugging output because right now my own keys do no longer work :-( That will of course be removed before we merge this. The fingerprints will stay to help you see what’s going on, but they’re not sensitive data. |
Sorry you're running into trouble with that, it's all a PITA to me 😄 . However, thank you for your work on this because I know without this I'd still be struggling to get this sorted... badly 💯 |
@bradmartin BTW your personal keys would never show up here. They are secrets in your repo and only used when the action runs for you – all the output then stays with you. The keys/secrets in this repo here are demo keys without access to anything. |
6fcf3aa
to
c13b857
Compare
@mpdude @bradmartin I think we all should compare our envs, ssh versions, and commands for generating SSH keys... $ ssh -V
OpenSSH_8.1p1, OpenSSL 1.1.1d 10 Sep 2019
$ ssh-keygen -q -N '' -b 4096 -f test-key
Any difference from what you run? |
Well, better safe than sorry. GH Actions may have matching bugs like any other software. Plus Actions also may have bugs. You can output a part of the key which may be enough to guess the last char or you may put spaces between bytes and their matcher won't help... |
Yea my environment:
The keygen command runs with no output there, assuming that's correct for the args supplied to it. |
479f787
to
f58bd3c
Compare
Finally, I believe I have sorted this out: What I meant above with the key output is not that GH will mask all outputs that are equal to one of the secrets, but rather that a secret stored in @bradmartin's repo would never be used when running an action in one of my repos. This has been designed that way to prevent secrets from leaking accidentally. |
f58bd3c
to
1bcbe13
Compare
1bcbe13
to
1bc48df
Compare
5f52758
to
1bc48df
Compare
Oh, so my suggestion here #14 (comment) was correct after all :) |
@@ -17,7 +17,11 @@ try { | |||
core.exportVariable('SSH_AUTH_SOCK', authSock); | |||
|
|||
console.log("Adding private key to agent"); | |||
child_process.execSync('ssh-add -', { input: core.getInput('ssh-private-key') }); | |||
core.getInput('ssh-private-key').split(/(?=-----BEGIN)/).forEach(function(key) { |
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.
@mpdude why didn't you add an end-marker here? It'd be way more precise than trimming \n
this way... People would even be able to put all the keys in one line if they wanted to
…types/node-18.14.1 Bump @types/node from 18.14.0 to 18.14.1
Multiple SSH keys can be put into the secret value and will be split by the action.
Hopefully this can easily be tried by using
uses: webfactory/ssh-agent@support-multiple-keys
in the workflow definition.Leave a comment or 👍 here if it works for you.