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

summon removes last newline from provider output: SSH private key broken #182

Open
marco-m opened this issue Nov 28, 2020 · 1 comment
Open

Comments

@marco-m
Copy link
Contributor

marco-m commented Nov 28, 2020

Hello,

up to now I was using summon with single-line secrets and everything was working fine.

But when I tried using multi-line secrets, I found that summon removes the last newline from a secret, sometimes making the secrets unrecognized by the program itself who created the secret! This is the case for example for SSH private keys.

How to reproduce

Generate a SSH key pair (we want to use it to sign SSH certificates):

$ ssh-keygen -t ed25519 -f host-ca-key -C 'Host CA' -N ""

Verify that the private key ends with a newline character:

$ cat host-ca-key | hexdump -s 370 -c
0000172   E   N   D       O   P   E   N   S   S   H       P   R   I   V
0000182   A   T   E       K   E   Y   -   -   -   -   -  \n

Move the keys to pass (or gopass):

$ cat host-ca-key     | pass insert -m foo/host-ca-key
$ cat host-ca-key.pub | pass insert -m foo/host-ca-key.pub

$ rm host-ca-key*

Verify that the output of pass (or gopass) contains the last newline:

$ pass foo/host-ca-key | hexdump -s 370 -c
0000172   E   N   D       O   P   E   N   S   S   H       P   R   I   V
0000182   A   T   E       K   E   Y   -   -   -   -   -  \n

Create a pass provider for summon:

$ cat <<EOT > pass-provider
#! /bin/sh
exec pass "\$1"
EOT
$ chmod +x pass-provider

Create a secrets.yml file:

$ cat <<EOT > secrets.yml
host_ca_key: !file:var foo/host-ca-key
EOT

Verify that the private key exported by summon lacks the last newline:

$ summon -p ./pass-provider sh -c 'cat $host_ca_key' | hexdump -s 370 -c
0000172   E   N   D       O   P   E   N   S   S   H       P   R   I   V
0000182   A   T   E       K   E   Y   -   -   -   -   -

We are almost here. Generate a host key pair to be signed by the previous key:

$ ssh-keygen -t rsa -f host-key -C 'Host key' -N ""

Try to use the private key via summon to sign the host key:

$ summon -p ./pass-provider sh -c \
  'ssh-keygen -h -s $host_ca_key -V "+30d" -I host-cert host-key.pub'
Load key "/Users/foo/.tmp010039550/.summon586653765": invalid format

Extract the same key using the pass provider and save it as a file:

$ ./pass-provider foo/host-ca-key > host_ca_key
$ chmod 400 host_ca_key

The only difference is that now the host_ca_key contains also the newline.

Attempt to sign again, using the host_ca_key:

$ ssh-keygen -h -s host_ca_key -V "+30d" -I host-cert host-key.pub
Signed host key host-key-cert.pub: id "host-cert" serial 0 valid from 2020-11-28T18:24:00 to 2020-12-28T18:25:46

Note that all usages of a SSH private key are broken, not only signing a certificate as shown here but also using the key for a sshd server.

Where is the problematic code

func Call(provider, specPath string) (string, error) {
var (
stdOut bytes.Buffer
stdErr bytes.Buffer
)
cmd := exec.Command(provider, specPath)
cmd.Stdout = &stdOut
cmd.Stderr = &stdErr
err := cmd.Run()
if err != nil {
errstr := err.Error()
if stdErr.Len() > 0 {
errstr += ": " + strings.TrimSpace(stdErr.String())
}
return "", fmt.Errorf(errstr)
}
return strings.TrimSpace(stdOut.String()), nil

I went through the git history and there has always been a strings.TrimSpace().

Further considerations

I think this is a tough one, because removing the trimming might break backwards compatibility.

Is there any workaround?

@sgnn7
Copy link
Contributor

sgnn7 commented Nov 30, 2020

Hey @marco-m - thank you for your exceptionally well-documented issue!

I can't speak for this code as it was here since the beginnings as you found as well but I am guessing that maybe the output of cmd.Run() includes a newline by default? Either way, while I won't be able to work on this issue as I'm in a different project, I do feel like this may be something to look into for the @cyberark/community-and-integrations-team.

PS: I wonder if the trim can be changed just to trim a single line break instead maybe which would keep 99% of backwards compatibility and allow you to have the extra newline?

If you have some ideas on this too, feel free to put them in here and we are always happy to review PRs!

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

No branches or pull requests

3 participants