Merge lp:~thumper/juju-core/system-ssh-key into lp:~go-bot/juju-core/trunk

Proposed by Tim Penhey
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merged at revision: 2182
Proposed branch: lp:~thumper/juju-core/system-ssh-key
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 458 lines (+198/-22)
13 files modified
cloudinit/sshinit/configure_test.go (+1/-1)
dependencies.tsv (+1/-1)
doc/system-ssh-key.txt (+24/-0)
environs/cloudinit.go (+2/-1)
environs/cloudinit/cloudinit.go (+13/-0)
environs/cloudinit/cloudinit_test.go (+17/-11)
environs/manual/bootstrap.go (+7/-1)
provider/common/bootstrap.go (+29/-1)
provider/common/bootstrap_test.go (+12/-3)
provider/ec2/local_test.go (+2/-1)
provider/local/environ.go (+13/-2)
utils/ssh/generate.go (+49/-0)
utils/ssh/generate_test.go (+28/-0)
To merge this branch: bzr merge lp:~thumper/juju-core/system-ssh-key
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+199572@code.launchpad.net

Commit message

Create a system ssh identity on bootstrap

When an environment is bootstrapped, a system ssh
identity is created and written to the data directory.

The public key part is added to the authorized keys.

https://codereview.appspot.com/43730044/

Description of the change

Create a system ssh identity on bootstrap

When an environment is bootstrapped, a system ssh
identity is created and written to the data directory.

The public key part is added to the authorized keys.

https://codereview.appspot.com/43730044/

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

Reviewers: mp+199572_code.launchpad.net,

Message:
Please take a look.

Description:
Create a system ssh identity on bootstrap

When an environment is bootstrapped, a system ssh
identity is created and written to the data directory.

The public key part is added to the authorized keys.

https://code.launchpad.net/~thumper/juju-core/system-ssh-key/+merge/199572

(do not edit description out of merge proposal)

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

Affected files (+228, -19 lines):
   A [revision details]
   M cloudinit/sshinit/configure_test.go
   A doc/system-ssh-key.txt
   M environs/cloudinit.go
   M environs/cloudinit/cloudinit.go
   M environs/cloudinit/cloudinit_test.go
   M environs/manual/bootstrap.go
   M provider/common/bootstrap.go
   M provider/common/bootstrap_test.go
   M provider/ec2/local_test.go
   M provider/local/environ.go
   A utils/ssh/generate.go
   A utils/ssh/generate_test.go
   A utils/ssh/systemidentity.go
   A utils/ssh/systemidentity_test.go

Revision history for this message
Andrew Wilkins (axwalk) wrote :

https://codereview.appspot.com/43730044/diff/1/environs/cloudinit/cloudinit.go
File environs/cloudinit/cloudinit.go (right):

https://codereview.appspot.com/43730044/diff/1/environs/cloudinit/cloudinit.go#newcode262
environs/cloudinit/cloudinit.go:262: identityFile :=
ssh.SystemIdentityFilename(cfg.DataDir)
See comment on ssh.SystemIdentityFilename: the path here should be
obtained with path.Join, not filepath.Join. If you expose
ssh.SystemIdentity, you can just use cfg.dataFile(ssh.SystemIdentity)
here.

https://codereview.appspot.com/43730044/diff/1/provider/common/bootstrap.go
File provider/common/bootstrap.go (right):

https://codereview.appspot.com/43730044/diff/1/provider/common/bootstrap.go#newcode90
provider/common/bootstrap.go:90: func GenerateSystemSSHKey(env
environs.Environ) (privateKey string, err error) {
Doc comment please. Please mention that the function records the public
key in the environment config.

https://codereview.appspot.com/43730044/diff/1/provider/common/bootstrap.go#newcode90
provider/common/bootstrap.go:90: func GenerateSystemSSHKey(env
environs.Environ) (privateKey string, err error) {
Can you please add a test for this function in bootstrap_test.go, to
make sure the public key is added to the environment's config?

https://codereview.appspot.com/43730044/diff/1/provider/common/bootstrap.go#newcode95
provider/common/bootstrap.go:95: return "", fmt.Errorf("failed to create
system key: %v", err)
I'm told the errors should be worded like "creating system key", so it
ends up as
error: creating system key: <thing>

(I've traditionally done it the same way as you have here, mind.)

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

https://codereview.appspot.com/43730044/diff/1/utils/ssh/generate.go#newcode19
utils/ssh/generate.go:19: func GenerateKey(comment string) (private,
public string, err error) {
Doc comment please. What are the properties of the keys? It's RSA,
2048-bits, unencrypted (i.e. no passphrase), with the private key
encoded as PEM and the public key encoded in authorized_keys format. I
think it'd be good to spell all of that out.

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

https://codereview.appspot.com/43730044/diff/1/utils/ssh/systemidentity.go#newcode15
utils/ssh/systemidentity.go:15: func WriteSystemIdentity(dataDir string,
privateKey string) error {
This seems a bit too high-level for the utils/ssh package. It probably
ought to go in environs/ssh, as a "system identity" is a Juju-level
concept rather than an SSH-level one.

I'm probably being a bit too anal though, so I won't insist.

https://codereview.appspot.com/43730044/diff/1/utils/ssh/systemidentity.go#newcode28
utils/ssh/systemidentity.go:28: return filepath.Join(dataDir,
systemIdentity)
Using filepath.Join isn't correct in the case of bootstrapping from
Windows. I'd say just expose the systemIdentity constant, and use the
appropriate path/filepath.Join depending on the context.

https://codereview.appspot.com/43730044/

Revision history for this message
Roger Peppe (rogpeppe) wrote :

Looks reasonable if we actually want to do this, but I have my
reservations. I'd like more information on why we're doing this.

https://codereview.appspot.com/43730044/diff/1/doc/system-ssh-key.txt
File doc/system-ssh-key.txt (right):

https://codereview.appspot.com/43730044/diff/1/doc/system-ssh-key.txt#newcode2
doc/system-ssh-key.txt:2: potential use-cases.
I'd like to see a few of those use cases outlined here.

Also, we already have a system private key. Can't we just use that?

https://codereview.appspot.com/43730044/diff/1/doc/system-ssh-key.txt#newcode5
doc/system-ssh-key.txt:5: as part of the clout-init machine creation
process. The public key part is
s/clout/cloud/

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

https://codereview.appspot.com/43730044/diff/1/utils/ssh/systemidentity.go#newcode15
utils/ssh/systemidentity.go:15: func WriteSystemIdentity(dataDir string,
privateKey string) error {
On 2013/12/19 01:50:05, axw wrote:
> This seems a bit too high-level for the utils/ssh package. It probably
ought to
> go in environs/ssh, as a "system identity" is a Juju-level concept
rather than
> an SSH-level one.

+1
This definitely feels like the wrong place to me. Doesn't
environs/cloudinit encapsulate most of this kind of knowledge?

https://codereview.appspot.com/43730044/

Revision history for this message
Tim Penhey (thumper) wrote :
Download full text (4.0 KiB)

Please take a look.

https://codereview.appspot.com/43730044/diff/1/doc/system-ssh-key.txt
File doc/system-ssh-key.txt (right):

https://codereview.appspot.com/43730044/diff/1/doc/system-ssh-key.txt#newcode2
doc/system-ssh-key.txt:2: potential use-cases.
On 2013/12/19 11:59:17, rog wrote:
> I'd like to see a few of those use cases outlined here.

> Also, we already have a system private key. Can't we just use that?

Added some. Also made the decision to use different keys for different
uses rather than overloading use.

https://codereview.appspot.com/43730044/diff/1/doc/system-ssh-key.txt#newcode5
doc/system-ssh-key.txt:5: as part of the clout-init machine creation
process. The public key part is
On 2013/12/19 11:59:17, rog wrote:
> s/clout/cloud/

Done.

https://codereview.appspot.com/43730044/diff/1/environs/cloudinit/cloudinit.go
File environs/cloudinit/cloudinit.go (right):

https://codereview.appspot.com/43730044/diff/1/environs/cloudinit/cloudinit.go#newcode262
environs/cloudinit/cloudinit.go:262: identityFile :=
ssh.SystemIdentityFilename(cfg.DataDir)
On 2013/12/19 01:50:05, axw wrote:
> See comment on ssh.SystemIdentityFilename: the path here should be
obtained with
> path.Join, not filepath.Join. If you expose ssh.SystemIdentity, you
can just use
> cfg.dataFile(ssh.SystemIdentity) here.

Done.

https://codereview.appspot.com/43730044/diff/1/provider/common/bootstrap.go
File provider/common/bootstrap.go (right):

https://codereview.appspot.com/43730044/diff/1/provider/common/bootstrap.go#newcode90
provider/common/bootstrap.go:90: func GenerateSystemSSHKey(env
environs.Environ) (privateKey string, err error) {
On 2013/12/19 01:50:05, axw wrote:
> Doc comment please. Please mention that the function records the
public key in
> the environment config.

Done.

https://codereview.appspot.com/43730044/diff/1/provider/common/bootstrap.go#newcode95
provider/common/bootstrap.go:95: return "", fmt.Errorf("failed to create
system key: %v", err)
On 2013/12/19 01:50:05, axw wrote:
> I'm told the errors should be worded like "creating system key", so it
ends up
> as
> error: creating system key: <thing>

> (I've traditionally done it the same way as you have here, mind.)

I'd say that this is dumb.

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

https://codereview.appspot.com/43730044/diff/1/utils/ssh/generate.go#newcode19
utils/ssh/generate.go:19: func GenerateKey(comment string) (private,
public string, err error) {
On 2013/12/19 01:50:05, axw wrote:
> Doc comment please. What are the properties of the keys? It's RSA,
2048-bits,
> unencrypted (i.e. no passphrase), with the private key encoded as PEM
and the
> public key encoded in authorized_keys format. I think it'd be good to
spell all
> of that out.

OK

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

https://codereview.appspot.com/43730044/diff/1/utils/ssh/systemidentity.go#newcode15
utils/ssh/systemidentity.go:15: func WriteSystemIdentity(dataDir string,
privateKey string) error {
On 2013/12/19 01:50:05, axw wrote:
> This seems a bit too high-level for the utils/s...

Read more...

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

On 2013/12/19 22:16:10, thumper wrote:
> Please take a look.

> https://codereview.appspot.com/43730044/diff/1/doc/system-ssh-key.txt
> File doc/system-ssh-key.txt (right):

https://codereview.appspot.com/43730044/diff/1/doc/system-ssh-key.txt#newcode2
> doc/system-ssh-key.txt:2: potential use-cases.
> On 2013/12/19 11:59:17, rog wrote:
> > I'd like to see a few of those use cases outlined here.
> >
> > Also, we already have a system private key. Can't we just use that?

> Added some. Also made the decision to use different keys for different
uses
> rather than overloading use.

https://codereview.appspot.com/43730044/diff/1/doc/system-ssh-key.txt#newcode5
> doc/system-ssh-key.txt:5: as part of the clout-init machine creation
process.
> The public key part is
> On 2013/12/19 11:59:17, rog wrote:
> > s/clout/cloud/

> Done.

https://codereview.appspot.com/43730044/diff/1/environs/cloudinit/cloudinit.go
> File environs/cloudinit/cloudinit.go (right):

https://codereview.appspot.com/43730044/diff/1/environs/cloudinit/cloudinit.go#newcode262
> environs/cloudinit/cloudinit.go:262: identityFile :=
> ssh.SystemIdentityFilename(cfg.DataDir)
> On 2013/12/19 01:50:05, axw wrote:
> > See comment on ssh.SystemIdentityFilename: the path here should be
obtained
> with
> > path.Join, not filepath.Join. If you expose ssh.SystemIdentity, you
can just
> use
> > cfg.dataFile(ssh.SystemIdentity) here.

> Done.

https://codereview.appspot.com/43730044/diff/1/provider/common/bootstrap.go
> File provider/common/bootstrap.go (right):

https://codereview.appspot.com/43730044/diff/1/provider/common/bootstrap.go#newcode90
> provider/common/bootstrap.go:90: func GenerateSystemSSHKey(env
environs.Environ)
> (privateKey string, err error) {
> On 2013/12/19 01:50:05, axw wrote:
> > Doc comment please. Please mention that the function records the
public key in
> > the environment config.

> Done.

https://codereview.appspot.com/43730044/diff/1/provider/common/bootstrap.go#newcode95
> provider/common/bootstrap.go:95: return "", fmt.Errorf("failed to
create system
> key: %v", err)
> On 2013/12/19 01:50:05, axw wrote:
> > I'm told the errors should be worded like "creating system key", so
it ends up
> > as
> > error: creating system key: <thing>
> >
> > (I've traditionally done it the same way as you have here, mind.)

> I'd say that this is dumb.

> https://codereview.appspot.com/43730044/diff/1/utils/ssh/generate.go
> File utils/ssh/generate.go (right):

https://codereview.appspot.com/43730044/diff/1/utils/ssh/generate.go#newcode19
> utils/ssh/generate.go:19: func GenerateKey(comment string) (private,
public
> string, err error) {
> On 2013/12/19 01:50:05, axw wrote:
> > Doc comment please. What are the properties of the keys? It's RSA,
2048-bits,
> > unencrypted (i.e. no passphrase), with the private key encoded as
PEM and the
> > public key encoded in authorized_keys format. I think it'd be good
to spell
> all
> > of that out.

> OK

https://codereview.appspot.com/43730044/diff/1/utils/ssh/systemidentity.go
> File utils/ssh/systemidentity.go (right):

https://codereview.appspot.com/43730044/diff/1/utils/ssh/systemidentity.go#newcode15
> utils/ssh/systemidentity.g...

Read more...

Revision history for this message
Go Bot (go-bot) wrote :

The attempt to merge lp:~thumper/juju-core/system-ssh-key into lp:juju-core failed. Below is the output from the failed tests.

# launchpad.net/juju-core/utils/ssh
utils/ssh/generate.go:38: undefined: ssh.ParsePrivateKey

Revision history for this message
Go Bot (go-bot) wrote :

Attempt to merge into lp:juju-core failed due to conflicts:

text conflict in provider/common/bootstrap_test.go

Revision history for this message
Go Bot (go-bot) wrote :

The attempt to merge lp:~thumper/juju-core/system-ssh-key into lp:juju-core failed. Below is the output from the failed tests.

# launchpad.net/juju-core/utils/ssh
utils/ssh/generate.go:38: undefined: ssh.ParsePrivateKey

Revision history for this message
Roger Peppe (rogpeppe) wrote :

https://codereview.appspot.com/43730044/diff/1/provider/common/bootstrap.go
File provider/common/bootstrap.go (right):

https://codereview.appspot.com/43730044/diff/1/provider/common/bootstrap.go#newcode95
provider/common/bootstrap.go:95: return "", fmt.Errorf("failed to create
system key: %v", err)
On 2013/12/19 22:16:11, thumper wrote:
> On 2013/12/19 01:50:05, axw wrote:
> > I'm told the errors should be worded like "creating system key", so
it ends up
> > as
> > error: creating system key: <thing>
> >
> > (I've traditionally done it the same way as you have here, mind.)

> I'd say that this is dumb.

I agree with both of you here. I think it's good if the error message
still makes sense even when the subsidiary error after the colon is
removed.

https://codereview.appspot.com/43730044/diff/20001/doc/system-ssh-key.txt
File doc/system-ssh-key.txt (right):

https://codereview.appspot.com/43730044/diff/20001/doc/system-ssh-key.txt#newcode10
doc/system-ssh-key.txt:10: Juju already creates a private key to connect
to the mongo database. It was an
Strictly speaking it creates a private key for *serving* the mongo
database and the API server.

https://codereview.appspot.com/43730044/diff/20001/doc/system-ssh-key.txt#newcode12
doc/system-ssh-key.txt:12: different purposes just seems like a more
robust idea.
I'd like a less hand-wavy justification than this.
Every extra secret lying around is another potential system
vulnerability.

On the other hand, if there *is* a good reason for using different keys
for different purposes, perhaps we should consider using different keys
for serving the API server and for the mongo server.

https://codereview.appspot.com/43730044/diff/20001/environs/ssh/systemidentity.go
File environs/ssh/systemidentity.go (right):

https://codereview.appspot.com/43730044/diff/20001/environs/ssh/systemidentity.go#newcode18
environs/ssh/systemidentity.go:18: func WriteSystemIdentity(filename
string, privateKey string) error {
We're creating an entire new package for a single constant and a
function that's semantically almost identical to ioutil.WriteFile?

Please let's just define SystemIdentityFilename in environs/cloudinit
and call WriteFile directly inside provider/local, the only caller.

https://codereview.appspot.com/43730044/diff/20001/environs/ssh/systemidentity.go#newcode21
environs/ssh/systemidentity.go:21: logger.Errorf("failed writing system
identity: %v", err)
If we've got an error return, please return the more informative error
rather than logging it. The error should be logged later at a higher
level.

https://codereview.appspot.com/43730044/

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

Please take a look.

https://codereview.appspot.com/43730044/diff/20001/doc/system-ssh-key.txt
File doc/system-ssh-key.txt (right):

https://codereview.appspot.com/43730044/diff/20001/doc/system-ssh-key.txt#newcode12
doc/system-ssh-key.txt:12: different purposes just seems like a more
robust idea.
On 2013/12/20 09:19:42, rog wrote:
> I'd like a less hand-wavy justification than this.
> Every extra secret lying around is another potential system
vulnerability.

> On the other hand, if there *is* a good reason for using different
keys for
> different purposes, perhaps we should consider using different keys
for serving
> the API server and for the mongo server.

When I originally wrote this, I wasn't really intending to commit it to
the tree, so the prose was somewhat fast and loose.

https://codereview.appspot.com/43730044/diff/20001/environs/ssh/systemidentity.go
File environs/ssh/systemidentity.go (right):

https://codereview.appspot.com/43730044/diff/20001/environs/ssh/systemidentity.go#newcode18
environs/ssh/systemidentity.go:18: func WriteSystemIdentity(filename
string, privateKey string) error {
On 2013/12/20 09:19:42, rog wrote:
> We're creating an entire new package for a single constant and a
function that's
> semantically almost identical to ioutil.WriteFile?

> Please let's just define SystemIdentityFilename in environs/cloudinit
and call
> WriteFile directly inside provider/local, the only caller.

Yeah, I have done this now.

Originally I was going to have this module abstract away more of the
information of the system identity file, but now as you can see it
doesn't do much. I have now removed this and just call write from the
local provider.

https://codereview.appspot.com/43730044/

Revision history for this message
Go Bot (go-bot) wrote :

The attempt to merge lp:~thumper/juju-core/system-ssh-key into lp:juju-core failed. Below is the output from the failed tests.

# launchpad.net/juju-core/utils/ssh
utils/ssh/generate.go:38: undefined: ssh.ParsePrivateKey

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'cloudinit/sshinit/configure_test.go'
--- cloudinit/sshinit/configure_test.go 2013-12-10 14:43:21 +0000
+++ cloudinit/sshinit/configure_test.go 2014-01-05 22:29:00 +0000
@@ -53,7 +53,7 @@
53func (s *configureSuite) getCloudConfig(c *gc.C, stateServer bool, vers version.Binary) *cloudinit.Config {53func (s *configureSuite) getCloudConfig(c *gc.C, stateServer bool, vers version.Binary) *cloudinit.Config {
54 var mcfg *envcloudinit.MachineConfig54 var mcfg *envcloudinit.MachineConfig
55 if stateServer {55 if stateServer {
56 mcfg = environs.NewBootstrapMachineConfig("http://whatever/dotcom")56 mcfg = environs.NewBootstrapMachineConfig("http://whatever/dotcom", "private-key")
57 } else {57 } else {
58 mcfg = environs.NewMachineConfig("0", "ya", nil, nil)58 mcfg = environs.NewMachineConfig("0", "ya", nil, nil)
59 }59 }
6060
=== modified file 'dependencies.tsv'
--- dependencies.tsv 2013-12-19 03:18:49 +0000
+++ dependencies.tsv 2014-01-05 22:29:00 +0000
@@ -1,4 +1,4 @@
1code.google.com/p/go.crypto hg 1747226a2f43c3f146f77dede1adf7073461b616 1411code.google.com/p/go.crypto hg 6478cc9340cbbe6c04511280c5007722269108e9 184
2code.google.com/p/go.net hg 3591c18acabc99439c783463ef00e6dc277eee39 772code.google.com/p/go.net hg 3591c18acabc99439c783463ef00e6dc277eee39 77
3labix.org/v2/mgo bzr gustavo@niemeyer.net-20130830194015-zk51jnp704tqmp7n 2403labix.org/v2/mgo bzr gustavo@niemeyer.net-20130830194015-zk51jnp704tqmp7n 240
4launchpad.net/gnuflag bzr roger.peppe@canonical.com-20121003093437-zcyyw0lpvj2nifpk 124launchpad.net/gnuflag bzr roger.peppe@canonical.com-20121003093437-zcyyw0lpvj2nifpk 12
55
=== added file 'doc/system-ssh-key.txt'
--- doc/system-ssh-key.txt 1970-01-01 00:00:00 +0000
+++ doc/system-ssh-key.txt 2014-01-05 22:29:00 +0000
@@ -0,0 +1,24 @@
1The idea of having a system SSH key is to support a number of real and
2potential use-cases.
3 * The system ssh key could be used to monitor the bootstrap process, and this
4 would benefit the new users that don't have an existing SSH key
5 * Allows the api server machines to ssh to other machines in the environment
6 * could be used to set up ssh tunnels through a single public facing IP
7 address on the server
8 * allows juju-run commands to be run on remote machiens
9
10Juju already creates a private key for serving the mongo database. It was an
11option to also use this key, but in the end, having different keys for
12different purposes just seems like a more robust idea.
13
14A system key is generated when the environment is bootstrapped, and uploaded
15as part of the cloud-init machine creation process. The public key part is
16added to the authorized keys list.
17
18This means that we need to generate an identity file and the authorized key
19line prior to creating the new machine.
20
21If subsequent state server machines are created, they also need to have the
22system identity file on them. Actually, it is most likely the API server jobs
23that we really care about.
24
025
=== modified file 'environs/cloudinit.go'
--- environs/cloudinit.go 2013-12-03 06:04:43 +0000
+++ environs/cloudinit.go 2014-01-05 22:29:00 +0000
@@ -42,12 +42,13 @@
42// bootstrap node. You'll still need to supply more information, but this42// bootstrap node. You'll still need to supply more information, but this
43// takes care of the fixed entries and the ones that are always needed.43// takes care of the fixed entries and the ones that are always needed.
44// stateInfoURL is the storage URL for the environment's state file.44// stateInfoURL is the storage URL for the environment's state file.
45func NewBootstrapMachineConfig(stateInfoURL string) *cloudinit.MachineConfig {45func NewBootstrapMachineConfig(stateInfoURL string, privateSystemSSHKey string) *cloudinit.MachineConfig {
46 // For a bootstrap instance, FinishMachineConfig will provide the46 // For a bootstrap instance, FinishMachineConfig will provide the
47 // state.Info and the api.Info. The machine id must *always* be "0".47 // state.Info and the api.Info. The machine id must *always* be "0".
48 mcfg := NewMachineConfig("0", state.BootstrapNonce, nil, nil)48 mcfg := NewMachineConfig("0", state.BootstrapNonce, nil, nil)
49 mcfg.StateServer = true49 mcfg.StateServer = true
50 mcfg.StateInfoURL = stateInfoURL50 mcfg.StateInfoURL = stateInfoURL
51 mcfg.SystemPrivateSSHKey = privateSystemSSHKey
51 return mcfg52 return mcfg
52}53}
5354
5455
=== modified file 'environs/cloudinit/cloudinit.go'
--- environs/cloudinit/cloudinit.go 2013-12-18 15:15:23 +0000
+++ environs/cloudinit/cloudinit.go 2014-01-05 22:29:00 +0000
@@ -33,6 +33,9 @@
33// is bootstrapping.33// is bootstrapping.
34const BootstrapStateURLFile = "/tmp/provider-state-url"34const BootstrapStateURLFile = "/tmp/provider-state-url"
3535
36// SystemIdentity is the name of the file where the environment SSH key is kept.
37const SystemIdentity = "system-identity"
38
36// fileSchemePrefix is the prefix for file:// URLs.39// fileSchemePrefix is the prefix for file:// URLs.
37const fileSchemePrefix = "file://"40const fileSchemePrefix = "file://"
3841
@@ -118,6 +121,11 @@
118 // DisableSSLHostnameVerification can be set to true to tell cloud-init121 // DisableSSLHostnameVerification can be set to true to tell cloud-init
119 // that it shouldn't verify SSL certificates122 // that it shouldn't verify SSL certificates
120 DisableSSLHostnameVerification bool123 DisableSSLHostnameVerification bool
124
125 // SystemPrivateSSHKey is created at bootstrap time and recorded on every
126 // node that has an API server. At this stage, that is any machine where
127 // StateServer (member above) is set to true.
128 SystemPrivateSSHKey string
121}129}
122130
123func base64yaml(m *config.Config) string {131func base64yaml(m *config.Config) string {
@@ -270,6 +278,8 @@
270 cfg.MaybeAddCloudArchiveCloudTools(c)278 cfg.MaybeAddCloudArchiveCloudTools(c)
271279
272 if cfg.StateServer {280 if cfg.StateServer {
281 identityFile := cfg.dataFile(SystemIdentity)
282 c.AddFile(identityFile, cfg.SystemPrivateSSHKey, 0600)
273 // Disable the default mongodb installed by the mongodb-server package.283 // Disable the default mongodb installed by the mongodb-server package.
274 // Only do this if the file doesn't exist already, so users can run284 // Only do this if the file doesn't exist already, so users can run
275 // their own mongodb server if they wish to.285 // their own mongodb server if they wish to.
@@ -607,6 +617,9 @@
607 if cfg.APIPort == 0 {617 if cfg.APIPort == 0 {
608 return fmt.Errorf("missing API port")618 return fmt.Errorf("missing API port")
609 }619 }
620 if cfg.SystemPrivateSSHKey == "" {
621 return fmt.Errorf("missing system ssh identity")
622 }
610 } else {623 } else {
611 if len(cfg.StateInfo.Addrs) == 0 {624 if len(cfg.StateInfo.Addrs) == 0 {
612 return fmt.Errorf("missing state hosts")625 return fmt.Errorf("missing state hosts")
613626
=== modified file 'environs/cloudinit/cloudinit_test.go'
--- environs/cloudinit/cloudinit_test.go 2013-12-17 12:13:50 +0000
+++ environs/cloudinit/cloudinit_test.go 2014-01-05 22:29:00 +0000
@@ -81,9 +81,10 @@
81 Password: "bletch",81 Password: "bletch",
82 CACert: []byte("CA CERT\n" + testing.CACert),82 CACert: []byte("CA CERT\n" + testing.CACert),
83 },83 },
84 Constraints: envConstraints,84 Constraints: envConstraints,
85 DataDir: environs.DataDir,85 DataDir: environs.DataDir,
86 StateInfoURL: "some-url",86 StateInfoURL: "some-url",
87 SystemPrivateSSHKey: "private rsa key",
87 },88 },
88 setEnvConfig: true,89 setEnvConfig: true,
89 expectScripts: `90 expectScripts: `
@@ -112,6 +113,8 @@
112install -m 600 /dev/null '/var/lib/juju/agents/machine-0/agent\.conf'113install -m 600 /dev/null '/var/lib/juju/agents/machine-0/agent\.conf'
113printf '%s\\n' '.*' > '/var/lib/juju/agents/machine-0/agent\.conf'114printf '%s\\n' '.*' > '/var/lib/juju/agents/machine-0/agent\.conf'
114ln -s /var/lib/juju/tools/machine-0/jujud /usr/local/bin/juju-run115ln -s /var/lib/juju/tools/machine-0/jujud /usr/local/bin/juju-run
116install -D -m 600 /dev/null '/var/lib/juju/system-identity'
117printf '%s\\n' '.*' > '/var/lib/juju/system-identity'
115install -D -m 600 /dev/null '/var/lib/juju/server\.pem'118install -D -m 600 /dev/null '/var/lib/juju/server\.pem'
116printf '%s\\n' 'SERVER CERT\\n[^']*SERVER KEY\\n[^']*' > '/var/lib/juju/server\.pem'119printf '%s\\n' 'SERVER CERT\\n[^']*SERVER KEY\\n[^']*' > '/var/lib/juju/server\.pem'
117mkdir -p /var/lib/juju/db/journal120mkdir -p /var/lib/juju/db/journal
@@ -159,9 +162,10 @@
159 Password: "bletch",162 Password: "bletch",
160 CACert: []byte("CA CERT\n" + testing.CACert),163 CACert: []byte("CA CERT\n" + testing.CACert),
161 },164 },
162 Constraints: envConstraints,165 Constraints: envConstraints,
163 DataDir: environs.DataDir,166 DataDir: environs.DataDir,
164 StateInfoURL: "some-url",167 StateInfoURL: "some-url",
168 SystemPrivateSSHKey: "private rsa key",
165 },169 },
166 setEnvConfig: true,170 setEnvConfig: true,
167 inexactMatch: true,171 inexactMatch: true,
@@ -320,8 +324,9 @@
320 Password: "bletch",324 Password: "bletch",
321 CACert: []byte("CA CERT\n" + testing.CACert),325 CACert: []byte("CA CERT\n" + testing.CACert),
322 },326 },
323 DataDir: environs.DataDir,327 DataDir: environs.DataDir,
324 StateInfoURL: "some-url",328 StateInfoURL: "some-url",
329 SystemPrivateSSHKey: "private rsa key",
325 },330 },
326 setEnvConfig: true,331 setEnvConfig: true,
327 inexactMatch: true,332 inexactMatch: true,
@@ -721,9 +726,10 @@
721 Addrs: []string{"host:9999"},726 Addrs: []string{"host:9999"},
722 CACert: []byte(testing.CACert),727 CACert: []byte(testing.CACert),
723 },728 },
724 Config: minimalConfig(c),729 Config: minimalConfig(c),
725 DataDir: environs.DataDir,730 DataDir: environs.DataDir,
726 MachineNonce: "FAKE_NONCE",731 MachineNonce: "FAKE_NONCE",
732 SystemPrivateSSHKey: "private rsa key",
727 }733 }
728 // check that the base configuration does not give an error734 // check that the base configuration does not give an error
729 ci := coreCloudinit.New()735 ci := coreCloudinit.New()
730736
=== modified file 'environs/manual/bootstrap.go'
--- environs/manual/bootstrap.go 2013-12-19 08:40:11 +0000
+++ environs/manual/bootstrap.go 2014-01-05 22:29:00 +0000
@@ -12,6 +12,7 @@
12 "launchpad.net/juju-core/environs/bootstrap"12 "launchpad.net/juju-core/environs/bootstrap"
13 envtools "launchpad.net/juju-core/environs/tools"13 envtools "launchpad.net/juju-core/environs/tools"
14 "launchpad.net/juju-core/instance"14 "launchpad.net/juju-core/instance"
15 "launchpad.net/juju-core/provider/common"
15 "launchpad.net/juju-core/tools"16 "launchpad.net/juju-core/tools"
16 "launchpad.net/juju-core/worker/localstorage"17 "launchpad.net/juju-core/worker/localstorage"
17)18)
@@ -121,9 +122,14 @@
121 return err122 return err
122 }123 }
123124
125 privateKey, err := common.GenerateSystemSSHKey(args.Environ)
126 if err != nil {
127 return err
128 }
129
124 // Finally, provision the machine agent.130 // Finally, provision the machine agent.
125 stateFileURL := fmt.Sprintf("file://%s/%s", storageDir, bootstrap.StateFile)131 stateFileURL := fmt.Sprintf("file://%s/%s", storageDir, bootstrap.StateFile)
126 mcfg := environs.NewBootstrapMachineConfig(stateFileURL)132 mcfg := environs.NewBootstrapMachineConfig(stateFileURL, privateKey)
127 if args.DataDir != "" {133 if args.DataDir != "" {
128 mcfg.DataDir = args.DataDir134 mcfg.DataDir = args.DataDir
129 }135 }
130136
=== modified file 'provider/common/bootstrap.go'
--- provider/common/bootstrap.go 2013-12-20 02:38:56 +0000
+++ provider/common/bootstrap.go 2014-01-05 22:29:00 +0000
@@ -46,7 +46,12 @@
46 if err != nil {46 if err != nil {
47 return err47 return err
48 }48 }
49 machineConfig := environs.NewBootstrapMachineConfig(stateFileURL)49
50 privateKey, err := GenerateSystemSSHKey(env)
51 if err != nil {
52 return err
53 }
54 machineConfig := environs.NewBootstrapMachineConfig(stateFileURL, privateKey)
5055
51 selectedTools, err := EnsureBootstrapTools(env, env.Config().DefaultSeries(), cons.Arch)56 selectedTools, err := EnsureBootstrapTools(env, env.Config().DefaultSeries(), cons.Arch)
52 if err != nil {57 if err != nil {
@@ -76,6 +81,29 @@
76 return FinishBootstrap(ctx, inst, machineConfig)81 return FinishBootstrap(ctx, inst, machineConfig)
77}82}
7883
84// GenerateSystemSSHKey creates a new key for the system identity. The
85// authorized_keys in the environment config is updated to include the public
86// key for the generated key.
87func GenerateSystemSSHKey(env environs.Environ) (privateKey string, err error) {
88 logger.Debugf("generate a system ssh key")
89 // Create a new system ssh key and add that to the authorized keys.
90 privateKey, publicKey, err := ssh.GenerateKey("juju-system-key")
91 if err != nil {
92 return "", fmt.Errorf("failed to create system key: %v", err)
93 }
94 authorized_keys := env.Config().AuthorizedKeys() + publicKey
95 newConfig, err := env.Config().Apply(map[string]interface{}{
96 "authorized-keys": authorized_keys,
97 })
98 if err != nil {
99 return "", fmt.Errorf("failed to create new config: %v", err)
100 }
101 if err = env.SetConfig(newConfig); err != nil {
102 return "", fmt.Errorf("failed to set new config: %v", err)
103 }
104 return privateKey, nil
105}
106
79// handelBootstrapError cleans up after a failed bootstrap.107// handelBootstrapError cleans up after a failed bootstrap.
80func handleBootstrapError(err error, ctx environs.BootstrapContext, inst instance.Instance, env environs.Environ) {108func handleBootstrapError(err error, ctx environs.BootstrapContext, inst instance.Instance, env environs.Environ) {
81 if err == nil {109 if err == nil {
82110
=== modified file 'provider/common/bootstrap_test.go'
--- provider/common/bootstrap_test.go 2013-12-20 09:25:57 +0000
+++ provider/common/bootstrap_test.go 2014-01-05 22:29:00 +0000
@@ -107,7 +107,7 @@
107 instance.Instance, *instance.HardwareCharacteristics, error,107 instance.Instance, *instance.HardwareCharacteristics, error,
108 ) {108 ) {
109 c.Assert(cons, gc.DeepEquals, checkCons)109 c.Assert(cons, gc.DeepEquals, checkCons)
110 c.Assert(mcfg, gc.DeepEquals, environs.NewBootstrapMachineConfig(checkURL))110 c.Assert(mcfg, gc.DeepEquals, environs.NewBootstrapMachineConfig(checkURL, mcfg.SystemPrivateSSHKey))
111 return nil, nil, fmt.Errorf("meh, not started")111 return nil, nil, fmt.Errorf("meh, not started")
112 }112 }
113113
@@ -209,11 +209,15 @@
209 checkURL = mcfg.StateInfoURL209 checkURL = mcfg.StateInfoURL
210 return &mockInstance{id: checkInstanceId}, &checkHardware, nil210 return &mockInstance{id: checkInstanceId}, &checkHardware, nil
211 }211 }
212212 var mocksConfig = minimalConfig(c)
213 var getConfigCalled int213 var getConfigCalled int
214 getConfig := func() *config.Config {214 getConfig := func() *config.Config {
215 getConfigCalled++215 getConfigCalled++
216 return minimalConfig(c)216 return mocksConfig
217 }
218 setConfig := func(c *config.Config) error {
219 mocksConfig = c
220 return nil
217 }221 }
218222
219 restore := envtesting.DisableFinishBootstrap()223 restore := envtesting.DisableFinishBootstrap()
@@ -223,7 +227,9 @@
223 storage: stor,227 storage: stor,
224 startInstance: startInstance,228 startInstance: startInstance,
225 config: getConfig,229 config: getConfig,
230 setConfig: setConfig,
226 }231 }
232 originalAuthKeys := env.Config().AuthorizedKeys()
227 ctx, _ := bootstrapContext(c)233 ctx, _ := bootstrapContext(c)
228 err := common.Bootstrap(ctx, env, constraints.Value{})234 err := common.Bootstrap(ctx, env, constraints.Value{})
229 c.Assert(err, gc.IsNil)235 c.Assert(err, gc.IsNil)
@@ -234,6 +240,9 @@
234 StateInstances: []instance.Id{instance.Id(checkInstanceId)},240 StateInstances: []instance.Id{instance.Id(checkInstanceId)},
235 Characteristics: []instance.HardwareCharacteristics{checkHardware},241 Characteristics: []instance.HardwareCharacteristics{checkHardware},
236 })242 })
243 authKeys := env.Config().AuthorizedKeys()
244 c.Assert(authKeys, gc.Not(gc.Equals), originalAuthKeys)
245 c.Assert(authKeys, jc.HasSuffix, "juju-system-key\n")
237}246}
238247
239type neverRefreshes struct {248type neverRefreshes struct {
240249
=== modified file 'provider/ec2/local_test.go'
--- provider/ec2/local_test.go 2013-12-20 02:38:56 +0000
+++ provider/ec2/local_test.go 2014-01-05 22:29:00 +0000
@@ -240,11 +240,12 @@
240 var userDataMap map[interface{}]interface{}240 var userDataMap map[interface{}]interface{}
241 err = goyaml.Unmarshal(userData, &userDataMap)241 err = goyaml.Unmarshal(userData, &userDataMap)
242 c.Assert(err, gc.IsNil)242 c.Assert(err, gc.IsNil)
243 expectedAuthKeys := strings.TrimSpace(env.Config().AuthorizedKeys())
243 c.Assert(userDataMap, gc.DeepEquals, map[interface{}]interface{}{244 c.Assert(userDataMap, gc.DeepEquals, map[interface{}]interface{}{
244 "output": map[interface{}]interface{}{245 "output": map[interface{}]interface{}{
245 "all": "| tee -a /var/log/cloud-init-output.log",246 "all": "| tee -a /var/log/cloud-init-output.log",
246 },247 },
247 "ssh_authorized_keys": []interface{}{"my-keys"},248 "ssh_authorized_keys": []interface{}{expectedAuthKeys},
248 "runcmd": []interface{}{249 "runcmd": []interface{}{
249 "install -D -m 644 /dev/null '/var/lib/juju/nonce.txt'",250 "install -D -m 644 /dev/null '/var/lib/juju/nonce.txt'",
250 "printf '%s\\n' 'user-admin:bootstrap' > '/var/lib/juju/nonce.txt'",251 "printf '%s\\n' 'user-admin:bootstrap' > '/var/lib/juju/nonce.txt'",
251252
=== modified file 'provider/local/environ.go'
--- provider/local/environ.go 2013-12-19 08:40:11 +0000
+++ provider/local/environ.go 2014-01-05 22:29:00 +0000
@@ -114,6 +114,10 @@
114 }114 }
115115
116 // TODO(thumper): check that the constraints don't include "container=lxc" for now.116 // TODO(thumper): check that the constraints don't include "container=lxc" for now.
117 privateKey, err := common.GenerateSystemSSHKey(env)
118 if err != nil {
119 return err
120 }
117121
118 cert, key, err := env.setupLocalMongoService()122 cert, key, err := env.setupLocalMongoService()
119 if err != nil {123 if err != nil {
@@ -151,7 +155,7 @@
151 return err155 return err
152 }156 }
153157
154 return env.setupLocalMachineAgent(cons, selectedTools)158 return env.setupLocalMachineAgent(cons, selectedTools, privateKey)
155}159}
156160
157// StateInfo is specified in the Environ interface.161// StateInfo is specified in the Environ interface.
@@ -459,11 +463,18 @@
459 return cert, key, nil463 return cert, key, nil
460}464}
461465
462func (env *localEnviron) setupLocalMachineAgent(cons constraints.Value, possibleTools tools.List) error {466func (env *localEnviron) setupLocalMachineAgent(cons constraints.Value, possibleTools tools.List, privateKey string) error {
463 dataDir := env.config.rootDir()467 dataDir := env.config.rootDir()
464 // unpack the first tools into the agent dir.468 // unpack the first tools into the agent dir.
465 agentTools := possibleTools[0]469 agentTools := possibleTools[0]
466 logger.Debugf("tools: %#v", agentTools)470 logger.Debugf("tools: %#v", agentTools)
471 // save the system identity file
472 systemIdentityFilename := filepath.Join(dataDir, cloudinit.SystemIdentity)
473 logger.Debugf("writing system identity to %s", systemIdentityFilename)
474 if err := ioutil.WriteFile(systemIdentityFilename, []byte(privateKey), 0600); err != nil {
475 return fmt.Errorf("failed to write system identity: %v", err)
476 }
477
467 // brutally abuse our knowledge of storage to directly open the file478 // brutally abuse our knowledge of storage to directly open the file
468 toolsUrl, err := url.Parse(agentTools.URL)479 toolsUrl, err := url.Parse(agentTools.URL)
469 if err != nil {480 if err != nil {
470481
=== added file 'utils/ssh/generate.go'
--- utils/ssh/generate.go 1970-01-01 00:00:00 +0000
+++ utils/ssh/generate.go 2014-01-05 22:29:00 +0000
@@ -0,0 +1,49 @@
1// Copyright 2013 Canonical Ltd.
2// Licensed under the AGPLv3, see LICENCE file for details.
3
4package ssh
5
6import (
7 "crypto/rand"
8 "crypto/rsa"
9 "crypto/x509"
10 "encoding/pem"
11 "fmt"
12 "strings"
13
14 "code.google.com/p/go.crypto/ssh"
15)
16
17// KeyBits is used to determine the number of bits to use for the RSA keys
18// created using the GenerateKey function.
19var KeyBits = 2048
20
21// GenerateKey makes a 2048 bit RSA no-passphrase SSH capable key. The bit
22// size is actually controlled by the KeyBits var. The private key returned is
23// encoded to ASCII using the PKCS1 encoding. The public key is suitable to
24// be added into an authorized_keys file, and has the comment passed in as the
25// comment part of the key.
26func GenerateKey(comment string) (private, public string, err error) {
27 key, err := rsa.GenerateKey(rand.Reader, KeyBits)
28 if err != nil {
29 return "", "", err
30 }
31
32 identity := pem.EncodeToMemory(
33 &pem.Block{
34 Type: "RSA PRIVATE KEY",
35 Bytes: x509.MarshalPKCS1PrivateKey(key),
36 })
37
38 signer, err := ssh.ParsePrivateKey(identity)
39 if err != nil {
40 return "", "", fmt.Errorf("failed to load key: %v", err)
41 }
42
43 auth_key := string(ssh.MarshalAuthorizedKey(signer.PublicKey()))
44 // Strip off the trailing new line so we can add a comment.
45 auth_key = strings.TrimSpace(auth_key)
46 public = fmt.Sprintf("%s %s\n", auth_key, comment)
47
48 return string(identity), public, nil
49}
050
=== added file 'utils/ssh/generate_test.go'
--- utils/ssh/generate_test.go 1970-01-01 00:00:00 +0000
+++ utils/ssh/generate_test.go 2014-01-05 22:29:00 +0000
@@ -0,0 +1,28 @@
1// Copyright 2013 Canonical Ltd.
2// Licensed under the AGPLv3, see LICENCE file for details.
3
4package ssh_test
5
6import (
7 gc "launchpad.net/gocheck"
8
9 jc "launchpad.net/juju-core/testing/checkers"
10 "launchpad.net/juju-core/testing/testbase"
11 "launchpad.net/juju-core/utils/ssh"
12)
13
14type GenerateSuite struct {
15 testbase.LoggingSuite
16}
17
18var _ = gc.Suite(&GenerateSuite{})
19
20func (s *GenerateSuite) TestGenerate(c *gc.C) {
21 private, public, err := ssh.GenerateKey("some-comment")
22
23 c.Check(err, gc.IsNil)
24 c.Check(private, jc.HasPrefix, "-----BEGIN RSA PRIVATE KEY-----\n")
25 c.Check(private, jc.HasSuffix, "-----END RSA PRIVATE KEY-----\n")
26 c.Check(public, jc.HasPrefix, "ssh-rsa ")
27 c.Check(public, jc.HasSuffix, " some-comment\n")
28}

Subscribers

People subscribed via source and target branches

to status/vote changes: