Merge lp:~axwalk/juju-core/ssh-client-keys into lp:~go-bot/juju-core/trunk

Proposed by Andrew Wilkins
Status: Merged
Approved by: Andrew Wilkins
Approved revision: no longer in the source branch.
Merged at revision: 2204
Proposed branch: lp:~axwalk/juju-core/ssh-client-keys
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 577 lines (+418/-27)
8 files modified
environs/config/authkeys.go (+16/-15)
environs/config/authkeys_test.go (+88/-0)
environs/config/config.go (+5/-1)
juju/conn.go (+7/-4)
juju/conn_test.go (+9/-6)
utils/ssh/authorisedkeys.go (+1/-1)
utils/ssh/clientkeys.go (+184/-0)
utils/ssh/clientkeys_test.go (+108/-0)
To merge this branch: bzr merge lp:~axwalk/juju-core/ssh-client-keys
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+201153@code.launchpad.net

Commit message

Introduce $JUJU_HOME/ssh for client keys

We introduce a new ssh directory under $JUJU_HOME,
in which we create an passphrase-less RSA key pair
for use by an SSH client. The user may add additional
key pairs to the directory. All public keys in the
directory (*.pub with a corresponding file without
the suffix) are added to the authorized-keys
environment config.

The private keys in the directory will be used in a
later CL that introduces a go.crypto/ssh-based client,
which will be used when openssh is not available.

https://codereview.appspot.com/49470046/

Description of the change

Introduce $JUJU_HOME/ssh for client keys

We introduce a new ssh directory under $JUJU_HOME,
in which we create an passphrase-less RSA key pair
for use by an SSH client. The user may add additional
key pairs to the directory. All public keys in the
directory (*.pub with a corresponding file without
the suffix) are added to the authorized-keys
environment config.

The private keys in the directory will be used in a
later CL that introduces a go.crypto/ssh-based client,
which will be used when openssh is not available.

https://codereview.appspot.com/49470046/

To post a comment you must log in.
Revision history for this message
Andrew Wilkins (axwalk) wrote :

Reviewers: mp+201153_code.launchpad.net,

Message:
Please take a look.

Description:
Introduce $JUJU_HOME/ssh for client keys

We introduce a new ssh directory under $JUJU_HOME,
in which we create an passphrase-less RSA key pair
for use by an SSH client. The user may add additional
key pairs to the directory. All public keys in the
directory (*.pub with a corresponding file without
the suffix) are added to the authorized-keys
environment config.

The private keys in the directory will be used in a
later CL that introduces a go.crypto/ssh-based client,
which will be used when openssh is not available.

https://code.launchpad.net/~axwalk/juju-core/ssh-client-keys/+merge/201153

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/49470046/

Affected files (+405, -27 lines):
   A [revision details]
   M environs/config/authkeys.go
   A environs/config/authkeys_test.go
   M environs/config/config.go
   M juju/conn.go
   M juju/conn_test.go
   M utils/ssh/authorisedkeys.go
   A utils/ssh/clientkeys.go
   A utils/ssh/clientkeys_test.go

Revision history for this message
Tim Penhey (thumper) wrote :

https://codereview.appspot.com/49470046/diff/1/environs/config/authkeys_test.go
File environs/config/authkeys_test.go (right):

https://codereview.appspot.com/49470046/diff/1/environs/config/authkeys_test.go#newcode27
environs/config/authkeys_test.go:27: func (s *AuthKeysSuite) SetUpTest(c
*gc.C) {
You need to make sure you call
  s.LoggingSuite.SetUpTest(c)

https://codereview.appspot.com/49470046/diff/1/environs/config/authkeys_test.go#newcode44
environs/config/authkeys_test.go:44: func writeFile(c *gc.C, filename
string, contents string) {
I've seen this so often I feel like we should add it to the a base test
suite... not expecting you to do this, just commenting.

https://codereview.appspot.com/49470046/diff/1/environs/config/authkeys_test.go#newcode62
environs/config/authkeys_test.go:62: defer ssh.LoadClientKeys("")
Why do this?

https://codereview.appspot.com/49470046/diff/1/utils/ssh/clientkeys.go
File utils/ssh/clientkeys.go (right):

https://codereview.appspot.com/49470046/diff/1/utils/ssh/clientkeys.go#newcode57
utils/ssh/clientkeys.go:57: // If the directory exists without keys,
While I understand the desire here, could we not just change the return
to only return if err != nil and len(clientKeys) > 0, otherwise create?

https://codereview.appspot.com/49470046/diff/1/utils/ssh/clientkeys.go#newcode82
utils/ssh/clientkeys.go:82: clientPrivateKey, err :=
ssh.ParsePrivateKey([]byte(private))
is this really a key? or a signer?

https://codereview.appspot.com/49470046/diff/1/utils/ssh/clientkeys.go#newcode104
utils/ssh/clientkeys.go:104: filename =
filename[:len(filename)-len(".pub")]
Can we put ".pub" as a file constant?
  PUB_SUFFIX
or something?

https://codereview.appspot.com/49470046/diff/1/utils/ssh/clientkeys.go#newcode121
utils/ssh/clientkeys.go:121: signers = append(signers, key)
can this panic?

https://codereview.appspot.com/49470046/diff/1/utils/ssh/clientkeys_test.go
File utils/ssh/clientkeys_test.go (right):

https://codereview.appspot.com/49470046/diff/1/utils/ssh/clientkeys_test.go#newcode29
utils/ssh/clientkeys_test.go:29: fakeHome :=
testing.MakeFakeHomeNoEnvironments(c)
MakeFakeHomeNoEnvironments just makes a home and overrides env vars,
with no .ssh dir.

MakeEmptyFakeHome also makes sure that ~/.juju exists.

https://codereview.appspot.com/49470046/diff/1/utils/ssh/clientkeys_test.go#newcode31
utils/ssh/clientkeys_test.go:31: s.AddCleanup(func(*gc.C) {
ssh.LoadClientKeys("") })
I do wonder if since LoadClientKeys("") is only used for tests, we
should instead have a method called ClearClientKeys()

https://codereview.appspot.com/49470046/diff/1/utils/ssh/clientkeys_test.go#newcode65
utils/ssh/clientkeys_test.go:65: err =
ioutil.WriteFile(testing.HomePath(".juju", "ssh", "whatever.pub"),
[]byte(pub), 0600)
why not use config.JujuHomePath?

https://codereview.appspot.com/49470046/

Revision history for this message
Andrew Wilkins (axwalk) wrote :
Download full text (3.5 KiB)

Please take a look.

https://codereview.appspot.com/49470046/diff/1/environs/config/authkeys_test.go
File environs/config/authkeys_test.go (right):

https://codereview.appspot.com/49470046/diff/1/environs/config/authkeys_test.go#newcode27
environs/config/authkeys_test.go:27: func (s *AuthKeysSuite) SetUpTest(c
*gc.C) {
On 2014/01/14 02:01:37, thumper wrote:
> You need to make sure you call
> s.LoggingSuite.SetUpTest(c)

Done.

https://codereview.appspot.com/49470046/diff/1/environs/config/authkeys_test.go#newcode62
environs/config/authkeys_test.go:62: defer ssh.LoadClientKeys("")
On 2014/01/14 02:01:37, thumper wrote:
> Why do this?

To clear the keys cached in memory, so other tests get a clean slate.
I've moved it to TearDownTest.

https://codereview.appspot.com/49470046/diff/1/utils/ssh/clientkeys.go
File utils/ssh/clientkeys.go (right):

https://codereview.appspot.com/49470046/diff/1/utils/ssh/clientkeys.go#newcode57
utils/ssh/clientkeys.go:57: // If the directory exists without keys,
On 2014/01/14 02:01:37, thumper wrote:
> While I understand the desire here, could we not just change the
return to only
> return if err != nil and len(clientKeys) > 0, otherwise create?

Done, but I made it return if (err != nil || len(clientKeys) > 0). I
guess that's what you really meant.

https://codereview.appspot.com/49470046/diff/1/utils/ssh/clientkeys.go#newcode82
utils/ssh/clientkeys.go:82: clientPrivateKey, err :=
ssh.ParsePrivateKey([]byte(private))
On 2014/01/14 02:01:37, thumper wrote:
> is this really a key? or a signer?

Both. It's a key type that implements (and is exposed as) the Signer
interface. I find using Signer in the method names to be a bit awkward,
because it's talking about the go.crypto/ssh API rather than the SSH
concept.

https://codereview.appspot.com/49470046/diff/1/utils/ssh/clientkeys.go#newcode104
utils/ssh/clientkeys.go:104: filename =
filename[:len(filename)-len(".pub")]
On 2014/01/14 02:01:37, thumper wrote:
> Can we put ".pub" as a file constant?
> PUB_SUFFIX
> or something?

Done.

https://codereview.appspot.com/49470046/diff/1/utils/ssh/clientkeys.go#newcode121
utils/ssh/clientkeys.go:121: signers = append(signers, key)
On 2014/01/14 02:01:37, thumper wrote:
> can this panic?

:)

Deferred Unlock.

Yes it can, but TBH in this case if it did the whole program would
probably be buggered (Go does not deal well with OOM).

https://codereview.appspot.com/49470046/diff/1/utils/ssh/clientkeys_test.go
File utils/ssh/clientkeys_test.go (right):

https://codereview.appspot.com/49470046/diff/1/utils/ssh/clientkeys_test.go#newcode29
utils/ssh/clientkeys_test.go:29: fakeHome :=
testing.MakeFakeHomeNoEnvironments(c)
On 2014/01/14 02:01:37, thumper wrote:
> MakeFakeHomeNoEnvironments just makes a home and overrides env vars,
with no
> .ssh dir.

> MakeEmptyFakeHome also makes sure that ~/.juju exists.

Done.

https://codereview.appspot.com/49470046/diff/1/utils/ssh/clientkeys_test.go#newcode31
utils/ssh/clientkeys_test.go:31: s.AddCleanup(func(*gc.C) {
ssh.LoadClientKeys("") })
On 2014/01/14 02:01:37, thumper wrote:
> I do wonder if since LoadClientKeys("") is only used for tests, we
should
> instead have a method called ClearC...

Read more...

Revision history for this message
Tim Penhey (thumper) wrote :

LGTM - thanks for the updates

https://codereview.appspot.com/49470046/

Revision history for this message
Go Bot (go-bot) wrote :
Download full text (14.1 KiB)

The attempt to merge lp:~axwalk/juju-core/ssh-client-keys into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core/agent 1.272s
ok launchpad.net/juju-core/agent/tools 0.222s
ok launchpad.net/juju-core/bzr 7.810s
ok launchpad.net/juju-core/cert 3.138s
ok launchpad.net/juju-core/charm 0.592s
? launchpad.net/juju-core/charm/hooks [no test files]
ok launchpad.net/juju-core/cloudinit 0.033s
ok launchpad.net/juju-core/cloudinit/sshinit 1.052s
ok launchpad.net/juju-core/cmd 0.229s
ok launchpad.net/juju-core/cmd/charm-admin 0.837s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/juju 218.655s
ok launchpad.net/juju-core/cmd/jujud 56.763s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 9.658s
? launchpad.net/juju-core/cmd/plugins/juju-restore [no test files]
ok launchpad.net/juju-core/constraints 0.028s
ok launchpad.net/juju-core/container 0.055s
ok launchpad.net/juju-core/container/factory 0.049s
ok launchpad.net/juju-core/container/kvm 0.374s
ok launchpad.net/juju-core/container/kvm/mock 0.073s
? launchpad.net/juju-core/container/kvm/testing [no test files]
ok launchpad.net/juju-core/container/lxc 0.294s
? launchpad.net/juju-core/container/lxc/mock [no test files]
? launchpad.net/juju-core/container/lxc/testing [no test files]
? launchpad.net/juju-core/container/testing [no test files]
ok launchpad.net/juju-core/downloader 5.287s
ok launchpad.net/juju-core/environs 3.166s
ok launchpad.net/juju-core/environs/bootstrap 4.549s
ok launchpad.net/juju-core/environs/cloudinit 0.517s
ok launchpad.net/juju-core/environs/config 2.302s
ok launchpad.net/juju-core/environs/configstore 0.055s
ok launchpad.net/juju-core/environs/filestorage 0.032s
ok launchpad.net/juju-core/environs/httpstorage 0.892s
ok launchpad.net/juju-core/environs/imagemetadata 0.549s
? launchpad.net/juju-core/environs/imagemetadata/testing [no test files]
ok launchpad.net/juju-core/environs/instances 0.055s
ok launchpad.net/juju-core/environs/jujutest 0.308s
ok launchpad.net/juju-core/environs/manual 10.288s
ok launchpad.net/juju-core/environs/simplestreams 0.343s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 1.167s
ok launchpad.net/juju-core/environs/storage 1.210s
ok launchpad.net/juju-core/environs/sync 32.572s
ok launchpad.net/juju-core/environs/testing 0.211s
ok launchpad.net/juju-core/environs/tools 6.973s
? launchpad.net/juju-core/environs/tools/testing [no test files]
ok launchpad.net/juju-core/errors 0.025s
ok launchpad.net/juju-core/instance 0.023s
? launchpad.net/juju-core/instance/testing [no test files]
ok launchpad.net/juju-core/juju 22.830s
ok launchpad.net/juju-core/juju/osenv 0.019s
? launchpad.net/juju-core/juju/testing [no test files]
ok launchpad.net/juju-core/log 0.016s
ok launchpad.net/juju-core/log/syslog 0.025s
ok launchpad.net/juju-core/names 0.038s
? launchpad.net/juju-core/provider [no test files]
? launchpad.net/juju-core/provide...

Revision history for this message
Go Bot (go-bot) wrote :
Download full text (28.6 KiB)

The attempt to merge lp:~axwalk/juju-core/ssh-client-keys into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core/agent 1.325s
ok launchpad.net/juju-core/agent/tools 0.227s
ok launchpad.net/juju-core/bzr 7.846s
ok launchpad.net/juju-core/cert 3.297s
ok launchpad.net/juju-core/charm 0.636s
? launchpad.net/juju-core/charm/hooks [no test files]
ok launchpad.net/juju-core/cloudinit 0.035s
ok launchpad.net/juju-core/cloudinit/sshinit 1.030s
ok launchpad.net/juju-core/cmd 0.284s
ok launchpad.net/juju-core/cmd/charm-admin 0.890s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/juju 225.380s
ok launchpad.net/juju-core/cmd/jujud 56.575s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 12.666s
? launchpad.net/juju-core/cmd/plugins/juju-restore [no test files]
ok launchpad.net/juju-core/constraints 0.035s
ok launchpad.net/juju-core/container 0.036s
ok launchpad.net/juju-core/container/factory 0.049s
ok launchpad.net/juju-core/container/kvm 0.327s
ok launchpad.net/juju-core/container/kvm/mock 0.035s
? launchpad.net/juju-core/container/kvm/testing [no test files]
ok launchpad.net/juju-core/container/lxc 0.325s
? launchpad.net/juju-core/container/lxc/mock [no test files]
? launchpad.net/juju-core/container/lxc/testing [no test files]
? launchpad.net/juju-core/container/testing [no test files]
ok launchpad.net/juju-core/downloader 5.296s
ok launchpad.net/juju-core/environs 3.226s
ok launchpad.net/juju-core/environs/bootstrap 4.501s
ok launchpad.net/juju-core/environs/cloudinit 0.527s
ok launchpad.net/juju-core/environs/config 2.615s
ok launchpad.net/juju-core/environs/configstore 0.042s
ok launchpad.net/juju-core/environs/filestorage 0.031s
ok launchpad.net/juju-core/environs/httpstorage 0.908s
ok launchpad.net/juju-core/environs/imagemetadata 0.512s
? launchpad.net/juju-core/environs/imagemetadata/testing [no test files]
ok launchpad.net/juju-core/environs/instances 0.057s
ok launchpad.net/juju-core/environs/jujutest 0.243s
ok launchpad.net/juju-core/environs/manual 9.618s
ok launchpad.net/juju-core/environs/simplestreams 0.368s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 1.234s
ok launchpad.net/juju-core/environs/storage 1.169s
ok launchpad.net/juju-core/environs/sync 32.487s
ok launchpad.net/juju-core/environs/testing 0.260s
ok launchpad.net/juju-core/environs/tools 7.036s
? launchpad.net/juju-core/environs/tools/testing [no test files]
ok launchpad.net/juju-core/errors 0.016s
ok launchpad.net/juju-core/instance 0.022s
? launchpad.net/juju-core/instance/testing [no test files]
ok launchpad.net/juju-core/juju 23.814s
ok launchpad.net/juju-core/juju/osenv 0.019s
? launchpad.net/juju-core/juju/testing [no test files]
ok launchpad.net/juju-core/log 0.016s
ok launchpad.net/juju-core/log/syslog 0.040s
ok launchpad.net/juju-core/names 0.026s
? launchpad.net/juju-core/provider [no test files]
? launchpad.net/juju-core/provide...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'environs/config/authkeys.go'
2--- environs/config/authkeys.go 2014-01-03 03:35:25 +0000
3+++ environs/config/authkeys.go 2014-01-14 03:36:18 +0000
4@@ -9,23 +9,14 @@
5 "fmt"
6 "io/ioutil"
7 "os"
8- "path"
9 "path/filepath"
10- "strings"
11
12 "launchpad.net/juju-core/cert"
13 "launchpad.net/juju-core/juju/osenv"
14+ "launchpad.net/juju-core/utils"
15+ "launchpad.net/juju-core/utils/ssh"
16 )
17
18-func expandTilde(f string) string {
19- // TODO expansion of other user's home directories.
20- // Q what characters are valid in a user name?
21- if strings.HasPrefix(f, "~"+string(filepath.Separator)) {
22- return path.Join(osenv.Home(), f[2:])
23- }
24- return f
25-}
26-
27 // ReadAuthorizedKeys implements the standard juju behaviour for finding
28 // authorized_keys. It returns a set of keys in in authorized_keys format
29 // (see sshd(8) for a description). If path is non-empty, it names the
30@@ -33,17 +24,27 @@
31 // Home directory expansion will be performed on the path if it starts with
32 // a ~; if the expanded path is relative, it will be interpreted relative
33 // to $HOME/.ssh.
34+//
35+// The result of utils/ssh.PublicKeyFiles will always be prepended to the
36+// result. In practice, this means ReadAuthorizedKeys never returns an
37+// error when the call originates in the CLI.
38 func ReadAuthorizedKeys(path string) (string, error) {
39- var files []string
40+ files := ssh.PublicKeyFiles()
41 if path == "" {
42- files = []string{"id_dsa.pub", "id_rsa.pub", "identity.pub"}
43+ files = append(files, "id_dsa.pub", "id_rsa.pub", "identity.pub")
44 } else {
45- files = []string{path}
46+ files = append(files, path)
47 }
48 var firstError error
49 var keyData []byte
50 for _, f := range files {
51- f = expandTilde(f)
52+ f, err := utils.NormalizePath(f)
53+ if err != nil {
54+ if firstError == nil {
55+ firstError = err
56+ }
57+ continue
58+ }
59 if !filepath.IsAbs(f) {
60 f = filepath.Join(osenv.Home(), ".ssh", f)
61 }
62
63=== added file 'environs/config/authkeys_test.go'
64--- environs/config/authkeys_test.go 1970-01-01 00:00:00 +0000
65+++ environs/config/authkeys_test.go 2014-01-14 03:36:18 +0000
66@@ -0,0 +1,88 @@
67+// Copyright 2014 Canonical Ltd.
68+// Licensed under the AGPLv3, see LICENCE file for details.
69+
70+package config_test
71+
72+import (
73+ "io/ioutil"
74+ "os"
75+ "path/filepath"
76+ "strings"
77+
78+ gc "launchpad.net/gocheck"
79+
80+ "launchpad.net/juju-core/environs/config"
81+ "launchpad.net/juju-core/juju/osenv"
82+ "launchpad.net/juju-core/testing/testbase"
83+ "launchpad.net/juju-core/utils/ssh"
84+)
85+
86+type AuthKeysSuite struct {
87+ testbase.LoggingSuite
88+ dotssh string // ~/.ssh
89+}
90+
91+var _ = gc.Suite(&AuthKeysSuite{})
92+
93+func (s *AuthKeysSuite) SetUpTest(c *gc.C) {
94+ s.LoggingSuite.SetUpTest(c)
95+ old := osenv.Home()
96+ newhome := c.MkDir()
97+ osenv.SetHome(newhome)
98+ s.AddCleanup(func(*gc.C) { osenv.SetHome(old) })
99+ s.dotssh = filepath.Join(newhome, ".ssh")
100+ err := os.Mkdir(s.dotssh, 0755)
101+ c.Assert(err, gc.IsNil)
102+}
103+
104+func (s *AuthKeysSuite) TearDownTest(c *gc.C) {
105+ ssh.ClearClientKeys()
106+ s.LoggingSuite.TearDownTest(c)
107+}
108+
109+func (s *AuthKeysSuite) TestReadAuthorizedKeysErrors(c *gc.C) {
110+ _, err := config.ReadAuthorizedKeys("")
111+ c.Assert(err, gc.ErrorMatches, "no public ssh keys found")
112+ _, err = config.ReadAuthorizedKeys(filepath.Join(s.dotssh, "notthere.pub"))
113+ c.Assert(err, gc.ErrorMatches, "no public ssh keys found")
114+}
115+
116+func writeFile(c *gc.C, filename string, contents string) {
117+ err := ioutil.WriteFile(filename, []byte(contents), 0644)
118+ c.Assert(err, gc.IsNil)
119+}
120+
121+func (s *AuthKeysSuite) TestReadAuthorizedKeys(c *gc.C) {
122+ writeFile(c, filepath.Join(s.dotssh, "id_rsa.pub"), "id_rsa")
123+ writeFile(c, filepath.Join(s.dotssh, "identity.pub"), "identity")
124+ writeFile(c, filepath.Join(s.dotssh, "test.pub"), "test")
125+ keys, err := config.ReadAuthorizedKeys("")
126+ c.Assert(err, gc.IsNil)
127+ c.Assert(keys, gc.Equals, "id_rsa\nidentity\n")
128+ keys, err = config.ReadAuthorizedKeys("test.pub") // relative to ~/.ssh
129+ c.Assert(err, gc.IsNil)
130+ c.Assert(keys, gc.Equals, "test\n")
131+}
132+
133+func (s *AuthKeysSuite) TestReadAuthorizedKeysClientKeys(c *gc.C) {
134+ keydir := filepath.Join(s.dotssh, "juju")
135+ err := ssh.LoadClientKeys(keydir) // auto-generates a key pair
136+ c.Assert(err, gc.IsNil)
137+ pubkeyFiles := ssh.PublicKeyFiles()
138+ c.Assert(pubkeyFiles, gc.HasLen, 1)
139+ data, err := ioutil.ReadFile(pubkeyFiles[0])
140+ c.Assert(err, gc.IsNil)
141+ prefix := strings.Trim(string(data), "\n") + "\n"
142+
143+ writeFile(c, filepath.Join(s.dotssh, "id_rsa.pub"), "id_rsa")
144+ writeFile(c, filepath.Join(s.dotssh, "test.pub"), "test")
145+ keys, err := config.ReadAuthorizedKeys("")
146+ c.Assert(err, gc.IsNil)
147+ c.Assert(keys, gc.Equals, prefix+"id_rsa\n")
148+ keys, err = config.ReadAuthorizedKeys("test.pub")
149+ c.Assert(err, gc.IsNil)
150+ c.Assert(keys, gc.Equals, prefix+"test\n")
151+ keys, err = config.ReadAuthorizedKeys("notthere.pub")
152+ c.Assert(err, gc.IsNil)
153+ c.Assert(keys, gc.Equals, prefix)
154+}
155
156=== modified file 'environs/config/config.go'
157--- environs/config/config.go 2014-01-03 03:35:25 +0000
158+++ environs/config/config.go 2014-01-14 03:36:18 +0000
159@@ -18,6 +18,7 @@
160 "launchpad.net/juju-core/charm"
161 "launchpad.net/juju-core/juju/osenv"
162 "launchpad.net/juju-core/schema"
163+ "launchpad.net/juju-core/utils"
164 "launchpad.net/juju-core/version"
165 )
166
167@@ -318,7 +319,10 @@
168 }
169 path = defaultPath
170 }
171- path = expandTilde(path)
172+ path, err := utils.NormalizePath(path)
173+ if err != nil {
174+ return err
175+ }
176 if !filepath.IsAbs(path) {
177 path = JujuHomePath(path)
178 }
179
180=== modified file 'juju/conn.go'
181--- juju/conn.go 2013-12-19 21:17:26 +0000
182+++ juju/conn.go 2014-01-14 03:36:18 +0000
183@@ -12,7 +12,6 @@
184 "io/ioutil"
185 "net/url"
186 "os"
187- "path/filepath"
188 "time"
189
190 "launchpad.net/juju-core/charm"
191@@ -24,6 +23,7 @@
192 "launchpad.net/juju-core/log"
193 "launchpad.net/juju-core/state"
194 "launchpad.net/juju-core/utils"
195+ "launchpad.net/juju-core/utils/ssh"
196 )
197
198 // Conn holds a connection to a juju environment and its
199@@ -253,8 +253,8 @@
200 return sch, nil
201 }
202
203-// InitJujuHome initializes the charm and environs/config packages to use
204-// default paths based on the $JUJU_HOME or $HOME environment variables.
205+// InitJujuHome initializes the charm, environs/config and utils/ssh packages
206+// to use default paths based on the $JUJU_HOME or $HOME environment variables.
207 // This function should be called before calling NewConn or Conn.Deploy.
208 func InitJujuHome() error {
209 jujuHome := osenv.JujuHomeDir()
210@@ -263,6 +263,9 @@
211 "cannot determine juju home, required environment variables are not set")
212 }
213 config.SetJujuHome(jujuHome)
214- charm.CacheDir = filepath.Join(jujuHome, "charmcache")
215+ charm.CacheDir = config.JujuHomePath("charmcache")
216+ if err := ssh.LoadClientKeys(config.JujuHomePath("ssh")); err != nil {
217+ return fmt.Errorf("cannot load ssh client keys: %v", err)
218+ }
219 return nil
220 }
221
222=== modified file 'juju/conn_test.go'
223--- juju/conn_test.go 2013-12-20 09:25:57 +0000
224+++ juju/conn_test.go 2014-01-14 03:36:18 +0000
225@@ -657,18 +657,20 @@
226 }
227
228 func (s *InitJujuHomeSuite) TestJujuHome(c *gc.C) {
229- os.Setenv("JUJU_HOME", "/my/juju/home")
230+ jujuHome := c.MkDir()
231+ os.Setenv("JUJU_HOME", jujuHome)
232 err := juju.InitJujuHome()
233 c.Assert(err, gc.IsNil)
234- c.Assert(config.JujuHome(), gc.Equals, "/my/juju/home")
235+ c.Assert(config.JujuHome(), gc.Equals, jujuHome)
236 }
237
238 func (s *InitJujuHomeSuite) TestHome(c *gc.C) {
239+ osHome := c.MkDir()
240 os.Setenv("JUJU_HOME", "")
241- osenv.SetHome("/my/home/")
242+ osenv.SetHome(osHome)
243 err := juju.InitJujuHome()
244 c.Assert(err, gc.IsNil)
245- c.Assert(config.JujuHome(), gc.Equals, "/my/home/.juju")
246+ c.Assert(config.JujuHome(), gc.Equals, filepath.Join(osHome, ".juju"))
247 }
248
249 func (s *InitJujuHomeSuite) TestError(c *gc.C) {
250@@ -679,9 +681,10 @@
251 }
252
253 func (s *InitJujuHomeSuite) TestCacheDir(c *gc.C) {
254- os.Setenv("JUJU_HOME", "/foo/bar")
255+ jujuHome := c.MkDir()
256+ os.Setenv("JUJU_HOME", jujuHome)
257 c.Assert(charm.CacheDir, gc.Equals, "")
258 err := juju.InitJujuHome()
259 c.Assert(err, gc.IsNil)
260- c.Assert(charm.CacheDir, gc.Equals, "/foo/bar/charmcache")
261+ c.Assert(charm.CacheDir, gc.Equals, filepath.Join(jujuHome, "charmcache"))
262 }
263
264=== modified file 'utils/ssh/authorisedkeys.go'
265--- utils/ssh/authorisedkeys.go 2014-01-10 02:42:02 +0000
266+++ utils/ssh/authorisedkeys.go 2014-01-14 03:36:18 +0000
267@@ -20,7 +20,7 @@
268 "launchpad.net/juju-core/utils"
269 )
270
271-var logger = loggo.GetLogger("juju.ssh")
272+var logger = loggo.GetLogger("juju.utils.ssh")
273
274 type ListMode bool
275
276
277=== added file 'utils/ssh/clientkeys.go'
278--- utils/ssh/clientkeys.go 1970-01-01 00:00:00 +0000
279+++ utils/ssh/clientkeys.go 2014-01-14 03:36:18 +0000
280@@ -0,0 +1,184 @@
281+// Copyright 2014 Canonical Ltd.
282+// Licensed under the AGPLv3, see LICENCE file for details.
283+
284+package ssh
285+
286+import (
287+ "fmt"
288+ "io/ioutil"
289+ "os"
290+ "path/filepath"
291+ "strings"
292+ "sync"
293+
294+ "code.google.com/p/go.crypto/ssh"
295+
296+ "launchpad.net/juju-core/utils"
297+ "launchpad.net/juju-core/utils/set"
298+)
299+
300+const clientKeyName = "juju_id_rsa"
301+
302+// PublicKeySuffix is the file extension for public key files.
303+const PublicKeySuffix = ".pub"
304+
305+var (
306+ clientKeysMutex sync.Mutex
307+
308+ // clientKeys is a cached map of private key filenames
309+ // to ssh.Signers. The private keys are those loaded
310+ // from the client key directory, passed to LoadClientKeys.
311+ clientKeys map[string]ssh.Signer
312+)
313+
314+// LoadClientKeys loads the client SSH keys from the
315+// specified directory, and caches them as a process-wide
316+// global. If the directory does not exist, it is created;
317+// if the directory did not exist, or contains no keys, it
318+// is populated with a new key pair.
319+//
320+// If the directory exists, then all pairs of files where one
321+// has the same name as the other + ".pub" will be loaded as
322+// private/public key pairs.
323+//
324+// Calls to LoadClientKeys will clear the previously loaded
325+// keys, and recompute the keys.
326+func LoadClientKeys(dir string) error {
327+ clientKeysMutex.Lock()
328+ defer clientKeysMutex.Unlock()
329+ dir, err := utils.NormalizePath(dir)
330+ if err != nil {
331+ return err
332+ }
333+ if _, err := os.Stat(dir); err == nil {
334+ keys, err := loadClientKeys(dir)
335+ if err != nil {
336+ return err
337+ } else if len(keys) > 0 {
338+ clientKeys = keys
339+ return nil
340+ }
341+ // Directory exists but contains no keys;
342+ // fall through and create one.
343+ }
344+ if err := os.MkdirAll(dir, 0700); err != nil {
345+ return err
346+ }
347+ keyfile, key, err := generateClientKey(dir)
348+ if err != nil {
349+ os.RemoveAll(dir)
350+ return err
351+ }
352+ clientKeys = map[string]ssh.Signer{keyfile: key}
353+ return nil
354+}
355+
356+// ClearClientKeys clears the client keys cached in memory.
357+func ClearClientKeys() {
358+ clientKeysMutex.Lock()
359+ defer clientKeysMutex.Unlock()
360+ clientKeys = nil
361+}
362+
363+func generateClientKey(dir string) (keyfile string, key ssh.Signer, err error) {
364+ private, public, err := GenerateKey("juju-client-key")
365+ if err != nil {
366+ return "", nil, err
367+ }
368+ clientPrivateKey, err := ssh.ParsePrivateKey([]byte(private))
369+ if err != nil {
370+ return "", nil, err
371+ }
372+ privkeyFilename := filepath.Join(dir, clientKeyName)
373+ if err = ioutil.WriteFile(privkeyFilename, []byte(private), 0600); err != nil {
374+ return "", nil, err
375+ }
376+ if err := ioutil.WriteFile(privkeyFilename+PublicKeySuffix, []byte(public), 0600); err != nil {
377+ os.Remove(privkeyFilename)
378+ return "", nil, err
379+ }
380+ return privkeyFilename, clientPrivateKey, nil
381+}
382+
383+func loadClientKeys(dir string) (map[string]ssh.Signer, error) {
384+ publicKeyFiles, err := publicKeyFiles(dir)
385+ if err != nil {
386+ return nil, err
387+ }
388+ keys := make(map[string]ssh.Signer, len(publicKeyFiles))
389+ for _, filename := range publicKeyFiles {
390+ filename = filename[:len(filename)-len(PublicKeySuffix)]
391+ data, err := ioutil.ReadFile(filename)
392+ if err != nil {
393+ return nil, err
394+ }
395+ keys[filename], err = ssh.ParsePrivateKey(data)
396+ if err != nil {
397+ return nil, fmt.Errorf("parsing key file %q: %v", filename, err)
398+ }
399+ }
400+ return keys, nil
401+}
402+
403+// privateKeys returns the private keys loaded by LoadClientKeys.
404+func privateKeys() (signers []ssh.Signer) {
405+ clientKeysMutex.Lock()
406+ defer clientKeysMutex.Unlock()
407+ for _, key := range clientKeys {
408+ signers = append(signers, key)
409+ }
410+ return signers
411+}
412+
413+// PrivateKeyFiles returns the filenames of private SSH keys loaded by
414+// LoadClientKeys.
415+func PrivateKeyFiles() []string {
416+ clientKeysMutex.Lock()
417+ defer clientKeysMutex.Unlock()
418+ keyfiles := make([]string, 0, len(clientKeys))
419+ for f := range clientKeys {
420+ keyfiles = append(keyfiles, f)
421+ }
422+ return keyfiles
423+}
424+
425+// PublicKeyFiles returns the filenames of public SSH keys loaded by
426+// LoadClientKeys.
427+func PublicKeyFiles() []string {
428+ privkeys := PrivateKeyFiles()
429+ pubkeys := make([]string, len(privkeys))
430+ for i, priv := range privkeys {
431+ pubkeys[i] = priv + PublicKeySuffix
432+ }
433+ return pubkeys
434+}
435+
436+// publicKeyFiles returns the filenames of public SSH keys
437+// in the specified directory (all the files ending with .pub).
438+func publicKeyFiles(clientKeysDir string) ([]string, error) {
439+ if clientKeysDir == "" {
440+ return nil, nil
441+ }
442+ var keys []string
443+ dir, err := os.Open(clientKeysDir)
444+ if err != nil {
445+ return nil, err
446+ }
447+ names, err := dir.Readdirnames(-1)
448+ dir.Close()
449+ if err != nil {
450+ return nil, err
451+ }
452+ candidates := set.NewStrings(names...)
453+ for _, name := range names {
454+ if !strings.HasSuffix(name, PublicKeySuffix) {
455+ continue
456+ }
457+ // If the private key filename also exists, add the file.
458+ priv := name[:len(name)-len(PublicKeySuffix)]
459+ if candidates.Contains(priv) {
460+ keys = append(keys, filepath.Join(dir.Name(), name))
461+ }
462+ }
463+ return keys, nil
464+}
465
466=== added file 'utils/ssh/clientkeys_test.go'
467--- utils/ssh/clientkeys_test.go 1970-01-01 00:00:00 +0000
468+++ utils/ssh/clientkeys_test.go 2014-01-14 03:36:18 +0000
469@@ -0,0 +1,108 @@
470+// Copyright 2013 Canonical Ltd.
471+// Licensed under the AGPLv3, see LICENCE file for details.
472+
473+package ssh_test
474+
475+import (
476+ "io/ioutil"
477+ "os"
478+
479+ gc "launchpad.net/gocheck"
480+
481+ "launchpad.net/juju-core/environs/config"
482+ "launchpad.net/juju-core/testing"
483+ jc "launchpad.net/juju-core/testing/checkers"
484+ "launchpad.net/juju-core/testing/testbase"
485+ "launchpad.net/juju-core/utils"
486+ "launchpad.net/juju-core/utils/ssh"
487+)
488+
489+type ClientKeysSuite struct {
490+ testbase.LoggingSuite
491+}
492+
493+var _ = gc.Suite(&ClientKeysSuite{})
494+
495+func (s *ClientKeysSuite) SetUpTest(c *gc.C) {
496+ s.LoggingSuite.SetUpTest(c)
497+ fakeHome := testing.MakeEmptyFakeHome(c)
498+ s.AddCleanup(func(*gc.C) { fakeHome.Restore() })
499+ s.AddCleanup(func(*gc.C) { ssh.ClearClientKeys() })
500+}
501+
502+func checkFiles(c *gc.C, obtained, expected []string) {
503+ var err error
504+ for i, e := range expected {
505+ expected[i], err = utils.NormalizePath(e)
506+ c.Assert(err, gc.IsNil)
507+ }
508+ c.Assert(obtained, jc.SameContents, expected)
509+}
510+
511+func checkPublicKeyFiles(c *gc.C, expected ...string) {
512+ keys := ssh.PublicKeyFiles()
513+ checkFiles(c, keys, expected)
514+}
515+
516+func checkPrivateKeyFiles(c *gc.C, expected ...string) {
517+ keys := ssh.PrivateKeyFiles()
518+ checkFiles(c, keys, expected)
519+}
520+
521+func (s *ClientKeysSuite) TestPublicKeyFiles(c *gc.C) {
522+ // LoadClientKeys will create the specified directory
523+ // and populate it with a key pair.
524+ err := ssh.LoadClientKeys("~/.juju/ssh")
525+ c.Assert(err, gc.IsNil)
526+ checkPublicKeyFiles(c, "~/.juju/ssh/juju_id_rsa.pub")
527+ // All files ending with .pub in the client key dir get picked up.
528+ priv, pub, err := ssh.GenerateKey("whatever")
529+ c.Assert(err, gc.IsNil)
530+ err = ioutil.WriteFile(config.JujuHomePath("ssh", "whatever.pub"), []byte(pub), 0600)
531+ c.Assert(err, gc.IsNil)
532+ err = ssh.LoadClientKeys("~/.juju/ssh")
533+ c.Assert(err, gc.IsNil)
534+ // The new public key won't be observed until the
535+ // corresponding private key exists.
536+ checkPublicKeyFiles(c, "~/.juju/ssh/juju_id_rsa.pub")
537+ err = ioutil.WriteFile(config.JujuHomePath("ssh", "whatever"), []byte(priv), 0600)
538+ c.Assert(err, gc.IsNil)
539+ err = ssh.LoadClientKeys("~/.juju/ssh")
540+ c.Assert(err, gc.IsNil)
541+ checkPublicKeyFiles(c, "~/.juju/ssh/juju_id_rsa.pub", "~/.juju/ssh/whatever.pub")
542+}
543+
544+func (s *ClientKeysSuite) TestPrivateKeyFiles(c *gc.C) {
545+ // Create/load client keys. They will be cached in memory:
546+ // any files added to the directory will not be considered
547+ // unless LoadClientKeys is called again.
548+ err := ssh.LoadClientKeys("~/.juju/ssh")
549+ c.Assert(err, gc.IsNil)
550+ checkPrivateKeyFiles(c, "~/.juju/ssh/juju_id_rsa")
551+ priv, pub, err := ssh.GenerateKey("whatever")
552+ c.Assert(err, gc.IsNil)
553+ err = ioutil.WriteFile(config.JujuHomePath("ssh", "whatever"), []byte(priv), 0600)
554+ c.Assert(err, gc.IsNil)
555+ err = ssh.LoadClientKeys("~/.juju/ssh")
556+ c.Assert(err, gc.IsNil)
557+ // The new private key won't be observed until the
558+ // corresponding public key exists.
559+ checkPrivateKeyFiles(c, "~/.juju/ssh/juju_id_rsa")
560+ err = ioutil.WriteFile(config.JujuHomePath("ssh", "whatever.pub"), []byte(pub), 0600)
561+ c.Assert(err, gc.IsNil)
562+ // new keys won't be reported until we call LoadClientKeys again
563+ checkPublicKeyFiles(c, "~/.juju/ssh/juju_id_rsa.pub")
564+ checkPrivateKeyFiles(c, "~/.juju/ssh/juju_id_rsa")
565+ err = ssh.LoadClientKeys("~/.juju/ssh")
566+ c.Assert(err, gc.IsNil)
567+ checkPublicKeyFiles(c, "~/.juju/ssh/juju_id_rsa.pub", "~/.juju/ssh/whatever.pub")
568+ checkPrivateKeyFiles(c, "~/.juju/ssh/juju_id_rsa", "~/.juju/ssh/whatever")
569+}
570+
571+func (s *ClientKeysSuite) TestLoadClientKeysDirExists(c *gc.C) {
572+ err := os.MkdirAll(config.JujuHomePath("ssh"), 0755)
573+ c.Assert(err, gc.IsNil)
574+ err = ssh.LoadClientKeys("~/.juju/ssh")
575+ c.Assert(err, gc.IsNil)
576+ checkPrivateKeyFiles(c, "~/.juju/ssh/juju_id_rsa")
577+}

Subscribers

People subscribed via source and target branches

to status/vote changes: