Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Commit bad19af

Browse files
committed
Migrate ssh key generation to use ecdsa if the ssh-keygen binary is not available
This repo used to use rsa keys on windows and ed25519 keys on not-windows. This behavior was controlled by compile time flags. This was non-optimal for two reasons. One, it used rsa keys which are not preferred. Two, it meant that windows users who had the ssh-keygen binary installed were not getting the benefits of using ed25519 keys even though their system does support generating them. Now it is determined at run time whether to use ssh-keygen to generate an ed25519 key or to generate an ecdsa key in pure go (without a dependency on ssh-keygen).
1 parent 24e284b commit bad19af

6 files changed

Lines changed: 86 additions & 67 deletions

File tree

src/keybaseca/sshutils/generate.go

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
package sshutils
2+
3+
import (
4+
"crypto/ecdsa"
5+
"crypto/elliptic"
6+
"crypto/rand"
7+
"crypto/x509"
8+
"encoding/pem"
9+
"fmt"
10+
"io/ioutil"
11+
"os"
12+
"os/exec"
13+
"strings"
14+
15+
"golang.org/x/crypto/ssh"
16+
17+
"github.com/keybase/bot-sshca/src/shared"
18+
)
19+
20+
// Generate a new SSH key and store the private key at filename and the public key at filename.pub
21+
// If the ssh-keygen binary exists, generates an ed25519 ssh key using ssh-keygen. Otherwise,
22+
// generates an ecdsa key using go's crypto library. Note that we use ecdsa rather than ed25519
23+
// in this case since go's crypto library does not support marshalling ed25519 keys into the format
24+
// expected by openssh. github.com/ScaleFT/sshkeys claims to support this but does not reliably
25+
// work with all versions of ssh.
26+
func generateNewSSHKey(filename string) error {
27+
if sshKeygenBinaryExists() {
28+
return generateNewSSHKeyEd25519(filename)
29+
}
30+
31+
return generateNewSSHKeyEcdsa(filename)
32+
}
33+
34+
// Returns true iff the ssh-keygen binary exists and is in the user's path
35+
func sshKeygenBinaryExists() bool {
36+
_, err := exec.LookPath("ssh-keygen")
37+
return err == nil
38+
}
39+
40+
// Generate an ed25519 ssh key via ssh-keygen. Stores the private key at filename and the public key at filename.pub
41+
func generateNewSSHKeyEd25519(filename string) error {
42+
cmd := exec.Command("ssh-keygen", "-t", "ed25519", "-f", filename, "-m", "PEM", "-N", "")
43+
bytes, err := cmd.CombinedOutput()
44+
if err != nil {
45+
return fmt.Errorf("ssh-keygen failed: %s (%v)", strings.TrimSpace(string(bytes)), err)
46+
}
47+
return nil
48+
}
49+
50+
// Generate an ecdsa ssh key in pure go code. Stores the private key at filename and the public key at filename.pub
51+
// Note that if you are editing this code, be careful to ensure you test it manually since the integration tests
52+
// run in an environment with ssh-keygen and thus do not call this function. This function is manually used on windows.
53+
func generateNewSSHKeyEcdsa(filename string) error {
54+
// ssh-keygen -t ecdsa uses P256 by default
55+
privateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
56+
if err != nil {
57+
return err
58+
}
59+
60+
// 0600 are the correct permissions for an ssh private key
61+
privateKeyFile, err := os.OpenFile(filename, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600)
62+
if err != nil {
63+
return err
64+
}
65+
defer privateKeyFile.Close()
66+
67+
bytes, err := x509.MarshalECPrivateKey(privateKey)
68+
if err != nil {
69+
return err
70+
}
71+
72+
privateKeyPEM := &pem.Block{Type: "EC PRIVATE KEY", Bytes: bytes}
73+
err = pem.Encode(privateKeyFile, privateKeyPEM)
74+
if err != nil {
75+
return err
76+
}
77+
78+
pub, err := ssh.NewPublicKey(&privateKey.PublicKey)
79+
if err != nil {
80+
return err
81+
}
82+
return ioutil.WriteFile(shared.KeyPathToPubKey(filename), ssh.MarshalAuthorizedKey(pub), 0600)
83+
}

src/keybaseca/sshutils/generate_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,5 +31,5 @@ func TestGenerateNewSSHKey(t *testing.T) {
3131
bytes, err = ioutil.ReadFile(shared.KeyPathToPubKey(filename))
3232
require.NoError(t, err)
3333
require.False(t, strings.Contains(string(bytes), "PRIVATE"))
34-
require.True(t, strings.HasPrefix(string(bytes), "ssh-"))
34+
require.True(t, strings.HasPrefix(string(bytes), "ssh-ed25519") || strings.HasPrefix(string(bytes), "ecdsa-sha2-nistp256"))
3535
}

src/keybaseca/sshutils/generate_unix.go

Lines changed: 0 additions & 21 deletions
This file was deleted.

src/keybaseca/sshutils/generate_windows.go

Lines changed: 0 additions & 43 deletions
This file was deleted.

tests/tests/lib.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ def outputs_audit_log(tc: TestConfig, filename: str, expected_number: int):
137137
cnt = 0
138138
for line in new_lines:
139139
line = line.decode('utf-8')
140-
if line and f"Processing SignatureRequest from user={tc.username}" in line and f"principals:{tc.subteam}.ssh.staging,{tc.subteam}.ssh.root_everywhere, expiration:+1h, pubkey:ssh-ed25519" in line:
140+
if line and f"Processing SignatureRequest from user={tc.username}" in line and f"principals:{tc.subteam}.ssh.staging,{tc.subteam}.ssh.root_everywhere, expiration:+1h, pubkey:" in line:
141141
cnt += 1
142142

143143
if cnt != expected_number:

tests/tests/test_env_1.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ def test_keybaseca_backup(self):
121121
if "----" in line and "PRIVATE" in line and "BEGIN" in line:
122122
add = True
123123
if add:
124-
keyLines.append(line)
124+
keyLines.append(line.strip())
125125
if "----" in line and "PRIVATE" in line and "END" in line:
126126
add = False
127127
key = '\n'.join(keyLines)

0 commit comments

Comments
 (0)