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

[Tim Penhey]

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/

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/

[Tim Penhey (thumper)]

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

[Andrew Wilkins (axwalk)]

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/

[Roger Peppe (rogpeppe)]

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/

[Tim Penhey (thumper)]

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...

[Andrew Wilkins (axwalk)]

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...

[Go Bot (go-bot)]

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

[Go Bot (go-bot)]

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

text conflict in provider/common/bootstrap_test.go

[Go Bot (go-bot)]

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

[Roger Peppe (rogpeppe)]

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/

[Tim Penhey (thumper)]

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/

[Go Bot (go-bot)]

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

=== 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 @@
 func (s *configureSuite) getCloudConfig(c *gc.C, stateServer bool, vers version.Binary) *cloudinit.Config {
 	var mcfg *envcloudinit.MachineConfig
 	if stateServer {
-		mcfg = environs.NewBootstrapMachineConfig("http://whatever/dotcom")
+		mcfg = environs.NewBootstrapMachineConfig("http://whatever/dotcom", "private-key")
 	} else {
 		mcfg = environs.NewMachineConfig("0", "ya", nil, nil)
 	}

=== 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 @@
-code.google.com/p/go.crypto	hg	1747226a2f43c3f146f77dede1adf7073461b616	141
+code.google.com/p/go.crypto	hg	6478cc9340cbbe6c04511280c5007722269108e9        184
 code.google.com/p/go.net	hg	3591c18acabc99439c783463ef00e6dc277eee39	77
 labix.org/v2/mgo	bzr	[e-mail]-20130830194015-zk51jnp704tqmp7n	240
 launchpad.net/gnuflag	bzr	[e-mail]-20121003093437-zcyyw0lpvj2nifpk	12

=== 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 @@
+The idea of having a system SSH key is to support a number of real and
+potential use-cases.
+ * The system ssh key could be used to monitor the bootstrap process, and this
+   would benefit the new users that don't have an existing SSH key
+ * Allows the api server machines to ssh to other machines in the environment
+   * could be used to set up ssh tunnels through a single public facing IP
+     address on the server
+   * allows juju-run commands to be run on remote machiens
+
+Juju already creates a private key for serving the mongo database. It was an
+option to also use this key, but in the end, having different keys for
+different purposes just seems like a more robust idea.
+
+A system key is generated when the environment is bootstrapped, and uploaded
+as part of the cloud-init machine creation process. The public key part is
+added to the authorized keys list.
+
+This means that we need to generate an identity file and the authorized key
+line prior to creating the new machine.
+
+If subsequent state server machines are created, they also need to have the
+system identity file on them. Actually, it is most likely the API server jobs
+that we really care about.
+

=== 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 @@
 // bootstrap node.  You'll still need to supply more information, but this
 // takes care of the fixed entries and the ones that are always needed.
 // stateInfoURL is the storage URL for the environment's state file.
-func NewBootstrapMachineConfig(stateInfoURL string) *cloudinit.MachineConfig {
+func NewBootstrapMachineConfig(stateInfoURL string, privateSystemSSHKey string) *cloudinit.MachineConfig {
 	// For a bootstrap instance, FinishMachineConfig will provide the
 	// state.Info and the api.Info. The machine id must *always* be "0".
 	mcfg := NewMachineConfig("0", state.BootstrapNonce, nil, nil)
 	mcfg.StateServer = true
 	mcfg.StateInfoURL = stateInfoURL
+	mcfg.SystemPrivateSSHKey = privateSystemSSHKey
 	return mcfg
 }
 

=== 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 @@
 // is bootstrapping.
 const BootstrapStateURLFile = "/tmp/provider-state-url"
 
+// SystemIdentity is the name of the file where the environment SSH key is kept.
+const SystemIdentity = "system-identity"
+
 // fileSchemePrefix is the prefix for file:// URLs.
 const fileSchemePrefix = "file://"
 
@@ -118,6 +121,11 @@
 	// DisableSSLHostnameVerification can be set to true to tell cloud-init
 	// that it shouldn't verify SSL certificates
 	DisableSSLHostnameVerification bool
+
+	// SystemPrivateSSHKey is created at bootstrap time and recorded on every
+	// node that has an API server. At this stage, that is any machine where
+	// StateServer (member above) is set to true.
+	SystemPrivateSSHKey string
 }
 
 func base64yaml(m *config.Config) string {
@@ -270,6 +278,8 @@
 	cfg.MaybeAddCloudArchiveCloudTools(c)
 
 	if cfg.StateServer {
+		identityFile := cfg.dataFile(SystemIdentity)
+		c.AddFile(identityFile, cfg.SystemPrivateSSHKey, 0600)
 		// Disable the default mongodb installed by the mongodb-server package.
 		// Only do this if the file doesn't exist already, so users can run
 		// their own mongodb server if they wish to.
@@ -607,6 +617,9 @@
 		if cfg.APIPort == 0 {
 			return fmt.Errorf("missing API port")
 		}
+		if cfg.SystemPrivateSSHKey == "" {
+			return fmt.Errorf("missing system ssh identity")
+		}
 	} else {
 		if len(cfg.StateInfo.Addrs) == 0 {
 			return fmt.Errorf("missing state hosts")

=== 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 @@
 				Password: "bletch",
 				CACert:   []byte("CA CERT\n" + testing.CACert),
 			},
-			Constraints:  envConstraints,
-			DataDir:      environs.DataDir,
-			StateInfoURL: "some-url",
+			Constraints:         envConstraints,
+			DataDir:             environs.DataDir,
+			StateInfoURL:        "some-url",
+			SystemPrivateSSHKey: "private rsa key",
 		},
 		setEnvConfig: true,
 		expectScripts: `
@@ -112,6 +113,8 @@
 install -m 600 /dev/null '/var/lib/juju/agents/machine-0/agent\.conf'
 printf '%s\\n' '.*' > '/var/lib/juju/agents/machine-0/agent\.conf'
 ln -s /var/lib/juju/tools/machine-0/jujud /usr/local/bin/juju-run
+install -D -m 600 /dev/null '/var/lib/juju/system-identity'
+printf '%s\\n' '.*' > '/var/lib/juju/system-identity'
 install -D -m 600 /dev/null '/var/lib/juju/server\.pem'
 printf '%s\\n' 'SERVER CERT\\n[^']*SERVER KEY\\n[^']*' > '/var/lib/juju/server\.pem'
 mkdir -p /var/lib/juju/db/journal
@@ -159,9 +162,10 @@
 				Password: "bletch",
 				CACert:   []byte("CA CERT\n" + testing.CACert),
 			},
-			Constraints:  envConstraints,
-			DataDir:      environs.DataDir,
-			StateInfoURL: "some-url",
+			Constraints:         envConstraints,
+			DataDir:             environs.DataDir,
+			StateInfoURL:        "some-url",
+			SystemPrivateSSHKey: "private rsa key",
 		},
 		setEnvConfig: true,
 		inexactMatch: true,
@@ -320,8 +324,9 @@
 				Password: "bletch",
 				CACert:   []byte("CA CERT\n" + testing.CACert),
 			},
-			DataDir:      environs.DataDir,
-			StateInfoURL: "some-url",
+			DataDir:             environs.DataDir,
+			StateInfoURL:        "some-url",
+			SystemPrivateSSHKey: "private rsa key",
 		},
 		setEnvConfig: true,
 		inexactMatch: true,
@@ -721,9 +726,10 @@
 			Addrs:  []string{"host:9999"},
 			CACert: []byte(testing.CACert),
 		},
-		Config:       minimalConfig(c),
-		DataDir:      environs.DataDir,
-		MachineNonce: "FAKE_NONCE",
+		Config:              minimalConfig(c),
+		DataDir:             environs.DataDir,
+		MachineNonce:        "FAKE_NONCE",
+		SystemPrivateSSHKey: "private rsa key",
 	}
 	// check that the base configuration does not give an error
 	ci := coreCloudinit.New()

=== 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 @@
 	"launchpad.net/juju-core/environs/bootstrap"
 	envtools "launchpad.net/juju-core/environs/tools"
 	"launchpad.net/juju-core/instance"
+	"launchpad.net/juju-core/provider/common"
 	"launchpad.net/juju-core/tools"
 	"launchpad.net/juju-core/worker/localstorage"
 )
@@ -121,9 +122,14 @@
 		return err
 	}
 
+	privateKey, err := common.GenerateSystemSSHKey(args.Environ)
+	if err != nil {
+		return err
+	}
+
 	// Finally, provision the machine agent.
 	stateFileURL := fmt.Sprintf("file://%s/%s", storageDir, bootstrap.StateFile)
-	mcfg := environs.NewBootstrapMachineConfig(stateFileURL)
+	mcfg := environs.NewBootstrapMachineConfig(stateFileURL, privateKey)
 	if args.DataDir != "" {
 		mcfg.DataDir = args.DataDir
 	}

=== 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 @@
 	if err != nil {
 		return err
 	}
-	machineConfig := environs.NewBootstrapMachineConfig(stateFileURL)
+
+	privateKey, err := GenerateSystemSSHKey(env)
+	if err != nil {
+		return err
+	}
+	machineConfig := environs.NewBootstrapMachineConfig(stateFileURL, privateKey)
 
 	selectedTools, err := EnsureBootstrapTools(env, env.Config().DefaultSeries(), cons.Arch)
 	if err != nil {
@@ -76,6 +81,29 @@
 	return FinishBootstrap(ctx, inst, machineConfig)
 }
 
+// GenerateSystemSSHKey creates a new key for the system identity. The
+// authorized_keys in the environment config is updated to include the public
+// key for the generated key.
+func GenerateSystemSSHKey(env environs.Environ) (privateKey string, err error) {
+	logger.Debugf("generate a system ssh key")
+	// Create a new system ssh key and add that to the authorized keys.
+	privateKey, publicKey, err := ssh.GenerateKey("juju-system-key")
+	if err != nil {
+		return "", fmt.Errorf("failed to create system key: %v", err)
+	}
+	authorized_keys := env.Config().AuthorizedKeys() + publicKey
+	newConfig, err := env.Config().Apply(map[string]interface{}{
+		"authorized-keys": authorized_keys,
+	})
+	if err != nil {
+		return "", fmt.Errorf("failed to create new config: %v", err)
+	}
+	if err = env.SetConfig(newConfig); err != nil {
+		return "", fmt.Errorf("failed to set new config: %v", err)
+	}
+	return privateKey, nil
+}
+
 // handelBootstrapError cleans up after a failed bootstrap.
 func handleBootstrapError(err error, ctx environs.BootstrapContext, inst instance.Instance, env environs.Environ) {
 	if err == nil {

=== 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 @@
 		instance.Instance, *instance.HardwareCharacteristics, error,
 	) {
 		c.Assert(cons, gc.DeepEquals, checkCons)
-		c.Assert(mcfg, gc.DeepEquals, environs.NewBootstrapMachineConfig(checkURL))
+		c.Assert(mcfg, gc.DeepEquals, environs.NewBootstrapMachineConfig(checkURL, mcfg.SystemPrivateSSHKey))
 		return nil, nil, fmt.Errorf("meh, not started")
 	}
 
@@ -209,11 +209,15 @@
 		checkURL = mcfg.StateInfoURL
 		return &mockInstance{id: checkInstanceId}, &checkHardware, nil
 	}
-
+	var mocksConfig = minimalConfig(c)
 	var getConfigCalled int
 	getConfig := func() *config.Config {
 		getConfigCalled++
-		return minimalConfig(c)
+		return mocksConfig
+	}
+	setConfig := func(c *config.Config) error {
+		mocksConfig = c
+		return nil
 	}
 
 	restore := envtesting.DisableFinishBootstrap()
@@ -223,7 +227,9 @@
 		storage:       stor,
 		startInstance: startInstance,
 		config:        getConfig,
+		setConfig:     setConfig,
 	}
+	originalAuthKeys := env.Config().AuthorizedKeys()
 	ctx, _ := bootstrapContext(c)
 	err := common.Bootstrap(ctx, env, constraints.Value{})
 	c.Assert(err, gc.IsNil)
@@ -234,6 +240,9 @@
 		StateInstances:  []instance.Id{instance.Id(checkInstanceId)},
 		Characteristics: []instance.HardwareCharacteristics{checkHardware},
 	})
+	authKeys := env.Config().AuthorizedKeys()
+	c.Assert(authKeys, gc.Not(gc.Equals), originalAuthKeys)
+	c.Assert(authKeys, jc.HasSuffix, "juju-system-key\n")
 }
 
 type neverRefreshes struct {

=== 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 @@
 	var userDataMap map[interface{}]interface{}
 	err = goyaml.Unmarshal(userData, &userDataMap)
 	c.Assert(err, gc.IsNil)
+	expectedAuthKeys := strings.TrimSpace(env.Config().AuthorizedKeys())
 	c.Assert(userDataMap, gc.DeepEquals, map[interface{}]interface{}{
 		"output": map[interface{}]interface{}{
 			"all": "| tee -a /var/log/cloud-init-output.log",
 		},
-		"ssh_authorized_keys": []interface{}{"my-keys"},
+		"ssh_authorized_keys": []interface{}{expectedAuthKeys},
 		"runcmd": []interface{}{
 			"install -D -m 644 /dev/null '/var/lib/juju/nonce.txt'",
 			"printf '%s\\n' 'user-admin:bootstrap' > '/var/lib/juju/nonce.txt'",

=== 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 @@
 	}
 
 	// TODO(thumper): check that the constraints don't include "container=lxc" for now.
+	privateKey, err := common.GenerateSystemSSHKey(env)
+	if err != nil {
+		return err
+	}
 
 	cert, key, err := env.setupLocalMongoService()
 	if err != nil {
@@ -151,7 +155,7 @@
 		return err
 	}
 
-	return env.setupLocalMachineAgent(cons, selectedTools)
+	return env.setupLocalMachineAgent(cons, selectedTools, privateKey)
 }
 
 // StateInfo is specified in the Environ interface.
@@ -459,11 +463,18 @@
 	return cert, key, nil
 }
 
-func (env *localEnviron) setupLocalMachineAgent(cons constraints.Value, possibleTools tools.List) error {
+func (env *localEnviron) setupLocalMachineAgent(cons constraints.Value, possibleTools tools.List, privateKey string) error {
 	dataDir := env.config.rootDir()
 	// unpack the first tools into the agent dir.
 	agentTools := possibleTools[0]
 	logger.Debugf("tools: %#v", agentTools)
+	// save the system identity file
+	systemIdentityFilename := filepath.Join(dataDir, cloudinit.SystemIdentity)
+	logger.Debugf("writing system identity to %s", systemIdentityFilename)
+	if err := ioutil.WriteFile(systemIdentityFilename, []byte(privateKey), 0600); err != nil {
+		return fmt.Errorf("failed to write system identity: %v", err)
+	}
+
 	// brutally abuse our knowledge of storage to directly open the file
 	toolsUrl, err := url.Parse(agentTools.URL)
 	if err != nil {

=== 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 @@
+// Copyright 2013 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package ssh
+
+import (
+	"crypto/rand"
+	"crypto/rsa"
+	"crypto/x509"
+	"encoding/pem"
+	"fmt"
+	"strings"
+
+	"code.google.com/p/go.crypto/ssh"
+)
+
+// KeyBits is used to determine the number of bits to use for the RSA keys
+// created using the GenerateKey function.
+var KeyBits = 2048
+
+// GenerateKey makes a 2048 bit RSA no-passphrase SSH capable key.  The bit
+// size is actually controlled by the KeyBits var. The private key returned is
+// encoded to ASCII using the PKCS1 encoding.  The public key is suitable to
+// be added into an authorized_keys file, and has the comment passed in as the
+// comment part of the key.
+func GenerateKey(comment string) (private, public string, err error) {
+	key, err := rsa.GenerateKey(rand.Reader, KeyBits)
+	if err != nil {
+		return "", "", err
+	}
+
+	identity := pem.EncodeToMemory(
+		&pem.Block{
+			Type:  "RSA PRIVATE KEY",
+			Bytes: x509.MarshalPKCS1PrivateKey(key),
+		})
+
+	signer, err := ssh.ParsePrivateKey(identity)
+	if err != nil {
+		return "", "", fmt.Errorf("failed to load key: %v", err)
+	}
+
+	auth_key := string(ssh.MarshalAuthorizedKey(signer.PublicKey()))
+	// Strip off the trailing new line so we can add a comment.
+	auth_key = strings.TrimSpace(auth_key)
+	public = fmt.Sprintf("%s %s\n", auth_key, comment)
+
+	return string(identity), public, nil
+}

=== 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 @@
+// Copyright 2013 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package ssh_test
+
+import (
+	gc "launchpad.net/gocheck"
+
+	jc "launchpad.net/juju-core/testing/checkers"
+	"launchpad.net/juju-core/testing/testbase"
+	"launchpad.net/juju-core/utils/ssh"
+)
+
+type GenerateSuite struct {
+	testbase.LoggingSuite
+}
+
+var _ = gc.Suite(&GenerateSuite{})
+
+func (s *GenerateSuite) TestGenerate(c *gc.C) {
+	private, public, err := ssh.GenerateKey("some-comment")
+
+	c.Check(err, gc.IsNil)
+	c.Check(private, jc.HasPrefix, "-----BEGIN RSA PRIVATE KEY-----\n")
+	c.Check(private, jc.HasSuffix, "-----END RSA PRIVATE KEY-----\n")
+	c.Check(public, jc.HasPrefix, "ssh-rsa ")
+	c.Check(public, jc.HasSuffix, " some-comment\n")
+}