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

Support multiple SSH keys #14

Merged
merged 2 commits into from
Jan 14, 2020
Merged

Support multiple SSH keys #14

merged 2 commits into from
Jan 14, 2020

Conversation

mpdude
Copy link
Member

@mpdude mpdude commented Jan 10, 2020

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.

@bradmartin
Copy link

What would the expected input be fore this using the action?

I'm getting errors with the yml file when trying:

          ssh-private-key: ${{ secrets.SSH_PRIVATE_KEY | secrets.SQLITE_ENCRYPTED_SSH_TOKEN | secrets.SQLITE_COMMERCIAL_SSH_TOKEN }}

also , to split them and you can't use multiple ssh-private-key on the with so I'm unsure how to input multiples into the action. Any advice?

@bradmartin
Copy link

bradmartin commented Jan 10, 2020

So looks like

    with:
          ssh-private-key: ${{ secrets.SSH_PRIVATE_KEY }}, |
            ${{ secrets.SQLITE_ENCRYPTED_SSH_TOKEN }} |
            ${{ secrets.SQLITE_COMMERCIAL_SSH_TOKEN }}

is valid as well as using a , to split them. I've just got an issue with how I uploaded the keys (invalid format) to work through to confirm.

Here's my current action output for this branch:

  Setup SSH-Agent2s
##[error]Node run failed with exit code 1
Run webfactory/ssh-agent@support-multiple-keys
  with:
    ssh-private-key: ***, | *** | ***
    ssh-auth-sock: /tmp/ssh-auth.sock
Adding GitHub.com keys to /Users/runner/.ssh/known_hosts
Starting ssh-agent
Adding private key to agent
Error loading key "(stdin)": invalid format
##[error]Command failed: ssh-add -
Error loading key "(stdin)": invalid format

##[error]Node run failed with exit code 1

@bradmartin
Copy link

bradmartin commented Jan 10, 2020

I've swapped out the KNOWN WORKING value:

          ssh-private-key: ${{ secrets.SSH_PRIVATE_KEY }}

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 });

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.

@webknjaz
Copy link

webknjaz commented Jan 10, 2020

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

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.

@mpdude
Copy link
Member Author

mpdude commented Jan 10, 2020

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.

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.

@mpdude
Copy link
Member Author

mpdude commented Jan 11, 2020

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

@webknjaz
Copy link

In fact, there is already some output for each key loaded

Ah, ok. Then you could just add ssh-add -l -E md5 after the loop and it'll print out the whole list with hashes. The keys will be in the load order.

@mpdude
Copy link
Member Author

mpdude commented Jan 11, 2020

@webknjaz weird... I have re-generated the demo keys and now ssh-add fails, although the input seems right.

Do you know if the keys need to be in a particular format?

@webknjaz
Copy link

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.
I suspect that those comments are getting added to your keys which may also depend on the YAML parser they use in GitHub Actions.

  1. You could remove the comments and see what happens because I suspect you're getting
    -----BEGIN ... PRIVATE KEY-----
    ...
    xxx
    ...
    -----END ... PRIVATE KEY-----       # 4096 bit RSA key created with `ssh-keygen -t rsa -b 4096 -o -f ...`
    -----BEGIN ... PRIVATE KEY-----
    ...
    yyy
    ...
    -----END ... PRIVATE KEY-----     # ED25519 key created with `ssh-keygen -t ed25519 -a 100 -f ...`
    and since you're only splitting the contents by the start string, you may get some unwanted bytes in the end. Consider using both start and end markers.
  2. I've seen similar errors when I tried to integrate this action earlier so maybe something in the envs/VMs has changed over time. Have you checked this command locally with the same keys?

@webknjaz
Copy link

This https://stackoverflow.com/a/59595773/595220 hints that there should be LF in the end maybe...

@webknjaz
Copy link

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-----

@bradmartin
Copy link

Just wanted to confirm the | for multi line args in the YML file worked as suggested by @webknjaz 👍

As of now looks like all good to go with following output in my action:

***
***
***
***
***
***
***
***
***
***
***
***
***
***
***
***
***
***
***
***
***
Keys added:
4096 [REDACTED] (stdin) (RSA)
4096 [REDACTED (stdin) (RSA)
4096 [REDACTED] (stdin) (RSA)

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 [REDACTED] however, they do print the values, just want to be sure that's not exposing anything to someone who might come across a build on a public repo seeing those 🤔

@mpdude
Copy link
Member Author

mpdude commented Jan 13, 2020

@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.

@bradmartin
Copy link

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 💯

@mpdude
Copy link
Member Author

mpdude commented Jan 13, 2020

@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.

@mpdude mpdude force-pushed the support-multiple-keys branch 2 times, most recently from 6fcf3aa to c13b857 Compare January 13, 2020 21:57
@webknjaz
Copy link

@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?

@webknjaz
Copy link

your personal keys would never show up here

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...

@bradmartin
Copy link

bradmartin commented Jan 13, 2020

Yea my environment:

Brads-MBP:~ bradmartin$ ssh -V
OpenSSH_7.9p1, LibreSSL 2.7.3
Brads-MBP:~ bradmartin$ ssh-keygen -q -N '' -b 4096 -f test-key

The keygen command runs with no output there, assuming that's correct for the args supplied to it.

@mpdude mpdude force-pushed the support-multiple-keys branch 2 times, most recently from 479f787 to f58bd3c Compare January 13, 2020 22:45
@mpdude
Copy link
Member Author

mpdude commented Jan 13, 2020

Finally, I believe I have sorted this out: ssh-agent seems to be very picky about leading or trailing newlines. I now strip them, add a trailing newline again and things seem to work.

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.

@mpdude mpdude merged commit 6cf6299 into master Jan 14, 2020
@mpdude mpdude deleted the support-multiple-keys branch January 14, 2020 09:21
@webknjaz
Copy link

ssh-agent seems to be very picky about leading or trailing newlines

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) {

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

cardoe added a commit to cardoe/ssh-agent that referenced this pull request Mar 2, 2023
…types/node-18.14.1

Bump @types/node from 18.14.0 to 18.14.1
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.

3 participants