Skip to content

Commit

Permalink
Fix generation of ssh id25519 key pairs
Browse files Browse the repository at this point in the history
  • Loading branch information
@wallyworld
wallyworld committed Feb 9, 2024
1 parent 56659c6 commit b0823b9
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 27 deletions.
2 changes: 1 addition & 1 deletion ssh/clientkeys.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
"github.com/juju/utils/v4"
)

const clientKeyName = "juju_id_rsa"
const clientKeyName = "juju_id_ed25519"

// PublicKeySuffix is the file extension for public key files.
const PublicKeySuffix = ".pub"
Expand Down
25 changes: 13 additions & 12 deletions ssh/clientkeys_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ import (

gitjujutesting "github.com/juju/testing"
jc "github.com/juju/testing/checkers"
gc "gopkg.in/check.v1"

"github.com/juju/utils/v4"
"github.com/juju/utils/v4/ssh"
gc "gopkg.in/check.v1"
)

type ClientKeysSuite struct {
Expand All @@ -23,7 +24,7 @@ var _ = gc.Suite(&ClientKeysSuite{})
func (s *ClientKeysSuite) SetUpTest(c *gc.C) {
s.FakeHomeSuite.SetUpTest(c)
s.AddCleanup(func(*gc.C) { ssh.ClearClientKeys() })
generateKeyRestorer := overrideGenerateKey(c)
generateKeyRestorer := overrideGenerateKey()
s.AddCleanup(func(*gc.C) { generateKeyRestorer.Restore() })
}

Expand Down Expand Up @@ -51,7 +52,7 @@ func (s *ClientKeysSuite) TestPublicKeyFiles(c *gc.C) {
// and populate it with a key pair.
err := ssh.LoadClientKeys("~/.juju/ssh")
c.Assert(err, jc.ErrorIsNil)
checkPublicKeyFiles(c, "~/.juju/ssh/juju_id_rsa.pub")
checkPublicKeyFiles(c, "~/.juju/ssh/juju_id_ed25519.pub")
// All files ending with .pub in the client key dir get picked up.
priv, pub, err := ssh.GenerateKey("whatever")
c.Assert(err, jc.ErrorIsNil)
Expand All @@ -61,12 +62,12 @@ func (s *ClientKeysSuite) TestPublicKeyFiles(c *gc.C) {
c.Assert(err, jc.ErrorIsNil)
// The new public key won't be observed until the
// corresponding private key exists.
checkPublicKeyFiles(c, "~/.juju/ssh/juju_id_rsa.pub")
checkPublicKeyFiles(c, "~/.juju/ssh/juju_id_ed25519.pub")
err = ioutil.WriteFile(gitjujutesting.HomePath(".juju", "ssh", "whatever"), []byte(priv), 0600)
c.Assert(err, jc.ErrorIsNil)
err = ssh.LoadClientKeys("~/.juju/ssh")
c.Assert(err, jc.ErrorIsNil)
checkPublicKeyFiles(c, "~/.juju/ssh/juju_id_rsa.pub", "~/.juju/ssh/whatever.pub")
checkPublicKeyFiles(c, "~/.juju/ssh/juju_id_ed25519.pub", "~/.juju/ssh/whatever.pub")
}

func (s *ClientKeysSuite) TestPrivateKeyFiles(c *gc.C) {
Expand All @@ -75,7 +76,7 @@ func (s *ClientKeysSuite) TestPrivateKeyFiles(c *gc.C) {
// unless LoadClientKeys is called again.
err := ssh.LoadClientKeys("~/.juju/ssh")
c.Assert(err, jc.ErrorIsNil)
checkPrivateKeyFiles(c, "~/.juju/ssh/juju_id_rsa")
checkPrivateKeyFiles(c, "~/.juju/ssh/juju_id_ed25519")
priv, pub, err := ssh.GenerateKey("whatever")
c.Assert(err, jc.ErrorIsNil)
err = ioutil.WriteFile(gitjujutesting.HomePath(".juju", "ssh", "whatever"), []byte(priv), 0600)
Expand All @@ -84,22 +85,22 @@ func (s *ClientKeysSuite) TestPrivateKeyFiles(c *gc.C) {
c.Assert(err, jc.ErrorIsNil)
// The new private key won't be observed until the
// corresponding public key exists.
checkPrivateKeyFiles(c, "~/.juju/ssh/juju_id_rsa")
checkPrivateKeyFiles(c, "~/.juju/ssh/juju_id_ed25519")
err = ioutil.WriteFile(gitjujutesting.HomePath(".juju", "ssh", "whatever.pub"), []byte(pub), 0600)
c.Assert(err, jc.ErrorIsNil)
// new keys won't be reported until we call LoadClientKeys again
checkPublicKeyFiles(c, "~/.juju/ssh/juju_id_rsa.pub")
checkPrivateKeyFiles(c, "~/.juju/ssh/juju_id_rsa")
checkPublicKeyFiles(c, "~/.juju/ssh/juju_id_ed25519.pub")
checkPrivateKeyFiles(c, "~/.juju/ssh/juju_id_ed25519")
err = ssh.LoadClientKeys("~/.juju/ssh")
c.Assert(err, jc.ErrorIsNil)
checkPublicKeyFiles(c, "~/.juju/ssh/juju_id_rsa.pub", "~/.juju/ssh/whatever.pub")
checkPrivateKeyFiles(c, "~/.juju/ssh/juju_id_rsa", "~/.juju/ssh/whatever")
checkPublicKeyFiles(c, "~/.juju/ssh/juju_id_ed25519.pub", "~/.juju/ssh/whatever.pub")
checkPrivateKeyFiles(c, "~/.juju/ssh/juju_id_ed25519", "~/.juju/ssh/whatever")
}

func (s *ClientKeysSuite) TestLoadClientKeysDirExists(c *gc.C) {
err := os.MkdirAll(gitjujutesting.HomePath(".juju", "ssh"), 0755)
c.Assert(err, jc.ErrorIsNil)
err = ssh.LoadClientKeys("~/.juju/ssh")
c.Assert(err, jc.ErrorIsNil)
checkPrivateKeyFiles(c, "~/.juju/ssh/juju_id_rsa")
checkPrivateKeyFiles(c, "~/.juju/ssh/juju_id_ed25519")
}
9 changes: 2 additions & 7 deletions ssh/generate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package ssh
import (
"crypto/ed25519"
"crypto/rand"
"crypto/x509"
"encoding/pem"
"fmt"
"strings"
Expand All @@ -28,15 +27,11 @@ func GenerateKey(comment string) (private, public string, err error) {
return "", "", errors.Trace(err)
}

keyData, err := x509.MarshalPKCS8PrivateKey(privateKey)
pemBlock, err := ssh.MarshalPrivateKey(privateKey, comment)
if err != nil {
return "", "", errors.Trace(err)
}
identity := pem.EncodeToMemory(
&pem.Block{
Type: "PRIVATE KEY",
Bytes: keyData,
})
identity := pem.EncodeToMemory(pemBlock)

public, err = PublicKey(identity, comment)
if err != nil {
Expand Down
8 changes: 4 additions & 4 deletions ssh/generate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ var (

// overrideGenerateKey patches out rsa.GenerateKey to create a single testing
// key which is saved and used between tests to save computation time.
func overrideGenerateKey(c *gc.C) testing.Restorer {
func overrideGenerateKey() testing.Restorer {
restorer := testing.PatchValue(ssh.ED25519GenerateKey, func(random io.Reader) (ed25519.PublicKey, ed25519.PrivateKey, error) {
if pregeneratedKey != nil {
return ed25519.PublicKey{}, pregeneratedKey, nil
Expand Down Expand Up @@ -63,11 +63,11 @@ func generateDSAKey(random io.Reader) (*dsa.PrivateKey, error) {
}

func (s *GenerateSuite) TestGenerate(c *gc.C) {
defer overrideGenerateKey(c).Restore()
defer overrideGenerateKey().Restore()
private, public, err := ssh.GenerateKey("some-comment")
c.Check(err, jc.ErrorIsNil)
c.Check(private, jc.HasPrefix, "-----BEGIN PRIVATE KEY-----\n")
c.Check(private, jc.HasSuffix, "-----END PRIVATE KEY-----\n")
c.Check(private, jc.HasPrefix, "-----BEGIN OPENSSH PRIVATE KEY-----\n")
c.Check(private, jc.HasSuffix, "-----END OPENSSH PRIVATE KEY-----\n")
c.Check(public, jc.HasPrefix, "ssh-ed25519 ")
c.Check(public, jc.HasSuffix, " some-comment\n")
}
2 changes: 1 addition & 1 deletion ssh/ssh_gocrypto_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func (s *SSHGoCryptoCommandSuite) SetUpSuite(c *gc.C) {
func (s *SSHGoCryptoCommandSuite) SetUpTest(c *gc.C) {
s.IsolationSuite.SetUpTest(c)

generateKeyRestorer := overrideGenerateKey(c)
generateKeyRestorer := overrideGenerateKey()
s.AddCleanup(func(*gc.C) { generateKeyRestorer.Restore() })

client, err := ssh.NewGoCryptoClient()
Expand Down
4 changes: 2 additions & 2 deletions ssh/ssh_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,12 +205,12 @@ func (s *SSHCommandSuite) TestCopy(c *gc.C) {
}

func (s *SSHCommandSuite) TestCommandClientKeys(c *gc.C) {
defer overrideGenerateKey(c).Restore()
defer overrideGenerateKey().Restore()
clientKeysDir := c.MkDir()
defer ssh.ClearClientKeys()
err := ssh.LoadClientKeys(clientKeysDir)
c.Assert(err, jc.ErrorIsNil)
ck := filepath.Join(clientKeysDir, "juju_id_rsa")
ck := filepath.Join(clientKeysDir, "juju_id_ed25519")
var opts ssh.Options
opts.SetIdentities("x", "y")
s.assertCommandArgs(c, s.commandOptions([]string{echoCommand, "123"}, &opts),
Expand Down

0 comments on commit b0823b9

Please sign in to comment.