Merge lp:~thumper/juju-core/system-ssh-key into lp:~go-bot/juju-core/trunk
- system-ssh-key
- Merge into trunk
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 |
Related bugs: |
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.
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.
Tim Penhey (thumper) wrote : | # |
Andrew Wilkins (axwalk) wrote : | # |
https:/
File environs/
https:/
environs/
ssh.SystemIdent
See comment on ssh.SystemIdent
obtained with path.Join, not filepath.Join. If you expose
ssh.SystemIdentity, you can just use cfg.dataFile(
here.
https:/
File provider/
https:/
provider/
environs.Environ) (privateKey string, err error) {
Doc comment please. Please mention that the function records the public
key in the environment config.
https:/
provider/
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:/
provider/
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:/
File utils/ssh/
https:/
utils/ssh/
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:/
File utils/ssh/
https:/
utils/ssh/
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:/
utils/ssh/
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.
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:/
File doc/system-
https:/
doc/system-
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:/
doc/system-
process. The public key part is
s/clout/cloud/
https:/
File utils/ssh/
https:/
utils/ssh/
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?
Tim Penhey (thumper) wrote : | # |
Please take a look.
https:/
File doc/system-
https:/
doc/system-
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:/
doc/system-
process. The public key part is
On 2013/12/19 11:59:17, rog wrote:
> s/clout/cloud/
Done.
https:/
File environs/
https:/
environs/
ssh.SystemIdent
On 2013/12/19 01:50:05, axw wrote:
> See comment on ssh.SystemIdent
obtained with
> path.Join, not filepath.Join. If you expose ssh.SystemIdentity, you
can just use
> cfg.dataFile(
Done.
https:/
File provider/
https:/
provider/
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:/
provider/
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:/
File utils/ssh/
https:/
utils/ssh/
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:/
File utils/ssh/
https:/
utils/ssh/
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) wrote : | # |
On 2013/12/19 22:16:10, thumper wrote:
> Please take a look.
> https:/
> File doc/system-
https:/
> doc/system-
> 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:/
> doc/system-
process.
> The public key part is
> On 2013/12/19 11:59:17, rog wrote:
> > s/clout/cloud/
> Done.
https:/
> File environs/
https:/
> environs/
> ssh.SystemIdent
> On 2013/12/19 01:50:05, axw wrote:
> > See comment on ssh.SystemIdent
obtained
> with
> > path.Join, not filepath.Join. If you expose ssh.SystemIdentity, you
can just
> use
> > cfg.dataFile(
> Done.
https:/
> File provider/
https:/
> provider/
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:/
> provider/
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:/
> File utils/ssh/
https:/
> utils/ssh/
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:/
> File utils/ssh/
https:/
> utils/ssh/
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.
utils/ssh/
Go Bot (go-bot) wrote : | # |
Attempt to merge into lp:juju-core failed due to conflicts:
text conflict in provider/
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.
utils/ssh/
Roger Peppe (rogpeppe) wrote : | # |
https:/
File provider/
https:/
provider/
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:/
File doc/system-
https:/
doc/system-
to the mongo database. It was an
Strictly speaking it creates a private key for *serving* the mongo
database and the API server.
https:/
doc/system-
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:/
File environs/
https:/
environs/
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 SystemIdentityF
and call WriteFile directly inside provider/local, the only caller.
https:/
environs/
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.
Tim Penhey (thumper) wrote : | # |
Please take a look.
https:/
File doc/system-
https:/
doc/system-
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:/
File environs/
https:/
environs/
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 SystemIdentityF
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.
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.
utils/ssh/
Preview Diff
1 | === modified file 'cloudinit/sshinit/configure_test.go' | |||
2 | --- cloudinit/sshinit/configure_test.go 2013-12-10 14:43:21 +0000 | |||
3 | +++ cloudinit/sshinit/configure_test.go 2014-01-05 22:29:00 +0000 | |||
4 | @@ -53,7 +53,7 @@ | |||
5 | 53 | func (s *configureSuite) getCloudConfig(c *gc.C, stateServer bool, vers version.Binary) *cloudinit.Config { | 53 | func (s *configureSuite) getCloudConfig(c *gc.C, stateServer bool, vers version.Binary) *cloudinit.Config { |
6 | 54 | var mcfg *envcloudinit.MachineConfig | 54 | var mcfg *envcloudinit.MachineConfig |
7 | 55 | if stateServer { | 55 | if stateServer { |
9 | 56 | mcfg = environs.NewBootstrapMachineConfig("http://whatever/dotcom") | 56 | mcfg = environs.NewBootstrapMachineConfig("http://whatever/dotcom", "private-key") |
10 | 57 | } else { | 57 | } else { |
11 | 58 | mcfg = environs.NewMachineConfig("0", "ya", nil, nil) | 58 | mcfg = environs.NewMachineConfig("0", "ya", nil, nil) |
12 | 59 | } | 59 | } |
13 | 60 | 60 | ||
14 | === modified file 'dependencies.tsv' | |||
15 | --- dependencies.tsv 2013-12-19 03:18:49 +0000 | |||
16 | +++ dependencies.tsv 2014-01-05 22:29:00 +0000 | |||
17 | @@ -1,4 +1,4 @@ | |||
19 | 1 | code.google.com/p/go.crypto hg 1747226a2f43c3f146f77dede1adf7073461b616 141 | 1 | code.google.com/p/go.crypto hg 6478cc9340cbbe6c04511280c5007722269108e9 184 |
20 | 2 | code.google.com/p/go.net hg 3591c18acabc99439c783463ef00e6dc277eee39 77 | 2 | code.google.com/p/go.net hg 3591c18acabc99439c783463ef00e6dc277eee39 77 |
21 | 3 | labix.org/v2/mgo bzr gustavo@niemeyer.net-20130830194015-zk51jnp704tqmp7n 240 | 3 | labix.org/v2/mgo bzr gustavo@niemeyer.net-20130830194015-zk51jnp704tqmp7n 240 |
22 | 4 | launchpad.net/gnuflag bzr roger.peppe@canonical.com-20121003093437-zcyyw0lpvj2nifpk 12 | 4 | launchpad.net/gnuflag bzr roger.peppe@canonical.com-20121003093437-zcyyw0lpvj2nifpk 12 |
23 | 5 | 5 | ||
24 | === added file 'doc/system-ssh-key.txt' | |||
25 | --- doc/system-ssh-key.txt 1970-01-01 00:00:00 +0000 | |||
26 | +++ doc/system-ssh-key.txt 2014-01-05 22:29:00 +0000 | |||
27 | @@ -0,0 +1,24 @@ | |||
28 | 1 | The idea of having a system SSH key is to support a number of real and | ||
29 | 2 | potential use-cases. | ||
30 | 3 | * The system ssh key could be used to monitor the bootstrap process, and this | ||
31 | 4 | would benefit the new users that don't have an existing SSH key | ||
32 | 5 | * Allows the api server machines to ssh to other machines in the environment | ||
33 | 6 | * could be used to set up ssh tunnels through a single public facing IP | ||
34 | 7 | address on the server | ||
35 | 8 | * allows juju-run commands to be run on remote machiens | ||
36 | 9 | |||
37 | 10 | Juju already creates a private key for serving the mongo database. It was an | ||
38 | 11 | option to also use this key, but in the end, having different keys for | ||
39 | 12 | different purposes just seems like a more robust idea. | ||
40 | 13 | |||
41 | 14 | A system key is generated when the environment is bootstrapped, and uploaded | ||
42 | 15 | as part of the cloud-init machine creation process. The public key part is | ||
43 | 16 | added to the authorized keys list. | ||
44 | 17 | |||
45 | 18 | This means that we need to generate an identity file and the authorized key | ||
46 | 19 | line prior to creating the new machine. | ||
47 | 20 | |||
48 | 21 | If subsequent state server machines are created, they also need to have the | ||
49 | 22 | system identity file on them. Actually, it is most likely the API server jobs | ||
50 | 23 | that we really care about. | ||
51 | 24 | |||
52 | 0 | 25 | ||
53 | === modified file 'environs/cloudinit.go' | |||
54 | --- environs/cloudinit.go 2013-12-03 06:04:43 +0000 | |||
55 | +++ environs/cloudinit.go 2014-01-05 22:29:00 +0000 | |||
56 | @@ -42,12 +42,13 @@ | |||
57 | 42 | // bootstrap node. You'll still need to supply more information, but this | 42 | // bootstrap node. You'll still need to supply more information, but this |
58 | 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. |
59 | 44 | // stateInfoURL is the storage URL for the environment's state file. | 44 | // stateInfoURL is the storage URL for the environment's state file. |
61 | 45 | func NewBootstrapMachineConfig(stateInfoURL string) *cloudinit.MachineConfig { | 45 | func NewBootstrapMachineConfig(stateInfoURL string, privateSystemSSHKey string) *cloudinit.MachineConfig { |
62 | 46 | // For a bootstrap instance, FinishMachineConfig will provide the | 46 | // For a bootstrap instance, FinishMachineConfig will provide the |
63 | 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". |
64 | 48 | mcfg := NewMachineConfig("0", state.BootstrapNonce, nil, nil) | 48 | mcfg := NewMachineConfig("0", state.BootstrapNonce, nil, nil) |
65 | 49 | mcfg.StateServer = true | 49 | mcfg.StateServer = true |
66 | 50 | mcfg.StateInfoURL = stateInfoURL | 50 | mcfg.StateInfoURL = stateInfoURL |
67 | 51 | mcfg.SystemPrivateSSHKey = privateSystemSSHKey | ||
68 | 51 | return mcfg | 52 | return mcfg |
69 | 52 | } | 53 | } |
70 | 53 | 54 | ||
71 | 54 | 55 | ||
72 | === modified file 'environs/cloudinit/cloudinit.go' | |||
73 | --- environs/cloudinit/cloudinit.go 2013-12-18 15:15:23 +0000 | |||
74 | +++ environs/cloudinit/cloudinit.go 2014-01-05 22:29:00 +0000 | |||
75 | @@ -33,6 +33,9 @@ | |||
76 | 33 | // is bootstrapping. | 33 | // is bootstrapping. |
77 | 34 | const BootstrapStateURLFile = "/tmp/provider-state-url" | 34 | const BootstrapStateURLFile = "/tmp/provider-state-url" |
78 | 35 | 35 | ||
79 | 36 | // SystemIdentity is the name of the file where the environment SSH key is kept. | ||
80 | 37 | const SystemIdentity = "system-identity" | ||
81 | 38 | |||
82 | 36 | // fileSchemePrefix is the prefix for file:// URLs. | 39 | // fileSchemePrefix is the prefix for file:// URLs. |
83 | 37 | const fileSchemePrefix = "file://" | 40 | const fileSchemePrefix = "file://" |
84 | 38 | 41 | ||
85 | @@ -118,6 +121,11 @@ | |||
86 | 118 | // DisableSSLHostnameVerification can be set to true to tell cloud-init | 121 | // DisableSSLHostnameVerification can be set to true to tell cloud-init |
87 | 119 | // that it shouldn't verify SSL certificates | 122 | // that it shouldn't verify SSL certificates |
88 | 120 | DisableSSLHostnameVerification bool | 123 | DisableSSLHostnameVerification bool |
89 | 124 | |||
90 | 125 | // SystemPrivateSSHKey is created at bootstrap time and recorded on every | ||
91 | 126 | // node that has an API server. At this stage, that is any machine where | ||
92 | 127 | // StateServer (member above) is set to true. | ||
93 | 128 | SystemPrivateSSHKey string | ||
94 | 121 | } | 129 | } |
95 | 122 | 130 | ||
96 | 123 | func base64yaml(m *config.Config) string { | 131 | func base64yaml(m *config.Config) string { |
97 | @@ -270,6 +278,8 @@ | |||
98 | 270 | cfg.MaybeAddCloudArchiveCloudTools(c) | 278 | cfg.MaybeAddCloudArchiveCloudTools(c) |
99 | 271 | 279 | ||
100 | 272 | if cfg.StateServer { | 280 | if cfg.StateServer { |
101 | 281 | identityFile := cfg.dataFile(SystemIdentity) | ||
102 | 282 | c.AddFile(identityFile, cfg.SystemPrivateSSHKey, 0600) | ||
103 | 273 | // Disable the default mongodb installed by the mongodb-server package. | 283 | // Disable the default mongodb installed by the mongodb-server package. |
104 | 274 | // Only do this if the file doesn't exist already, so users can run | 284 | // Only do this if the file doesn't exist already, so users can run |
105 | 275 | // their own mongodb server if they wish to. | 285 | // their own mongodb server if they wish to. |
106 | @@ -607,6 +617,9 @@ | |||
107 | 607 | if cfg.APIPort == 0 { | 617 | if cfg.APIPort == 0 { |
108 | 608 | return fmt.Errorf("missing API port") | 618 | return fmt.Errorf("missing API port") |
109 | 609 | } | 619 | } |
110 | 620 | if cfg.SystemPrivateSSHKey == "" { | ||
111 | 621 | return fmt.Errorf("missing system ssh identity") | ||
112 | 622 | } | ||
113 | 610 | } else { | 623 | } else { |
114 | 611 | if len(cfg.StateInfo.Addrs) == 0 { | 624 | if len(cfg.StateInfo.Addrs) == 0 { |
115 | 612 | return fmt.Errorf("missing state hosts") | 625 | return fmt.Errorf("missing state hosts") |
116 | 613 | 626 | ||
117 | === modified file 'environs/cloudinit/cloudinit_test.go' | |||
118 | --- environs/cloudinit/cloudinit_test.go 2013-12-17 12:13:50 +0000 | |||
119 | +++ environs/cloudinit/cloudinit_test.go 2014-01-05 22:29:00 +0000 | |||
120 | @@ -81,9 +81,10 @@ | |||
121 | 81 | Password: "bletch", | 81 | Password: "bletch", |
122 | 82 | CACert: []byte("CA CERT\n" + testing.CACert), | 82 | CACert: []byte("CA CERT\n" + testing.CACert), |
123 | 83 | }, | 83 | }, |
127 | 84 | Constraints: envConstraints, | 84 | Constraints: envConstraints, |
128 | 85 | DataDir: environs.DataDir, | 85 | DataDir: environs.DataDir, |
129 | 86 | StateInfoURL: "some-url", | 86 | StateInfoURL: "some-url", |
130 | 87 | SystemPrivateSSHKey: "private rsa key", | ||
131 | 87 | }, | 88 | }, |
132 | 88 | setEnvConfig: true, | 89 | setEnvConfig: true, |
133 | 89 | expectScripts: ` | 90 | expectScripts: ` |
134 | @@ -112,6 +113,8 @@ | |||
135 | 112 | install -m 600 /dev/null '/var/lib/juju/agents/machine-0/agent\.conf' | 113 | install -m 600 /dev/null '/var/lib/juju/agents/machine-0/agent\.conf' |
136 | 113 | printf '%s\\n' '.*' > '/var/lib/juju/agents/machine-0/agent\.conf' | 114 | printf '%s\\n' '.*' > '/var/lib/juju/agents/machine-0/agent\.conf' |
137 | 114 | ln -s /var/lib/juju/tools/machine-0/jujud /usr/local/bin/juju-run | 115 | ln -s /var/lib/juju/tools/machine-0/jujud /usr/local/bin/juju-run |
138 | 116 | install -D -m 600 /dev/null '/var/lib/juju/system-identity' | ||
139 | 117 | printf '%s\\n' '.*' > '/var/lib/juju/system-identity' | ||
140 | 115 | install -D -m 600 /dev/null '/var/lib/juju/server\.pem' | 118 | install -D -m 600 /dev/null '/var/lib/juju/server\.pem' |
141 | 116 | printf '%s\\n' 'SERVER CERT\\n[^']*SERVER KEY\\n[^']*' > '/var/lib/juju/server\.pem' | 119 | printf '%s\\n' 'SERVER CERT\\n[^']*SERVER KEY\\n[^']*' > '/var/lib/juju/server\.pem' |
142 | 117 | mkdir -p /var/lib/juju/db/journal | 120 | mkdir -p /var/lib/juju/db/journal |
143 | @@ -159,9 +162,10 @@ | |||
144 | 159 | Password: "bletch", | 162 | Password: "bletch", |
145 | 160 | CACert: []byte("CA CERT\n" + testing.CACert), | 163 | CACert: []byte("CA CERT\n" + testing.CACert), |
146 | 161 | }, | 164 | }, |
150 | 162 | Constraints: envConstraints, | 165 | Constraints: envConstraints, |
151 | 163 | DataDir: environs.DataDir, | 166 | DataDir: environs.DataDir, |
152 | 164 | StateInfoURL: "some-url", | 167 | StateInfoURL: "some-url", |
153 | 168 | SystemPrivateSSHKey: "private rsa key", | ||
154 | 165 | }, | 169 | }, |
155 | 166 | setEnvConfig: true, | 170 | setEnvConfig: true, |
156 | 167 | inexactMatch: true, | 171 | inexactMatch: true, |
157 | @@ -320,8 +324,9 @@ | |||
158 | 320 | Password: "bletch", | 324 | Password: "bletch", |
159 | 321 | CACert: []byte("CA CERT\n" + testing.CACert), | 325 | CACert: []byte("CA CERT\n" + testing.CACert), |
160 | 322 | }, | 326 | }, |
163 | 323 | DataDir: environs.DataDir, | 327 | DataDir: environs.DataDir, |
164 | 324 | StateInfoURL: "some-url", | 328 | StateInfoURL: "some-url", |
165 | 329 | SystemPrivateSSHKey: "private rsa key", | ||
166 | 325 | }, | 330 | }, |
167 | 326 | setEnvConfig: true, | 331 | setEnvConfig: true, |
168 | 327 | inexactMatch: true, | 332 | inexactMatch: true, |
169 | @@ -721,9 +726,10 @@ | |||
170 | 721 | Addrs: []string{"host:9999"}, | 726 | Addrs: []string{"host:9999"}, |
171 | 722 | CACert: []byte(testing.CACert), | 727 | CACert: []byte(testing.CACert), |
172 | 723 | }, | 728 | }, |
176 | 724 | Config: minimalConfig(c), | 729 | Config: minimalConfig(c), |
177 | 725 | DataDir: environs.DataDir, | 730 | DataDir: environs.DataDir, |
178 | 726 | MachineNonce: "FAKE_NONCE", | 731 | MachineNonce: "FAKE_NONCE", |
179 | 732 | SystemPrivateSSHKey: "private rsa key", | ||
180 | 727 | } | 733 | } |
181 | 728 | // check that the base configuration does not give an error | 734 | // check that the base configuration does not give an error |
182 | 729 | ci := coreCloudinit.New() | 735 | ci := coreCloudinit.New() |
183 | 730 | 736 | ||
184 | === modified file 'environs/manual/bootstrap.go' | |||
185 | --- environs/manual/bootstrap.go 2013-12-19 08:40:11 +0000 | |||
186 | +++ environs/manual/bootstrap.go 2014-01-05 22:29:00 +0000 | |||
187 | @@ -12,6 +12,7 @@ | |||
188 | 12 | "launchpad.net/juju-core/environs/bootstrap" | 12 | "launchpad.net/juju-core/environs/bootstrap" |
189 | 13 | envtools "launchpad.net/juju-core/environs/tools" | 13 | envtools "launchpad.net/juju-core/environs/tools" |
190 | 14 | "launchpad.net/juju-core/instance" | 14 | "launchpad.net/juju-core/instance" |
191 | 15 | "launchpad.net/juju-core/provider/common" | ||
192 | 15 | "launchpad.net/juju-core/tools" | 16 | "launchpad.net/juju-core/tools" |
193 | 16 | "launchpad.net/juju-core/worker/localstorage" | 17 | "launchpad.net/juju-core/worker/localstorage" |
194 | 17 | ) | 18 | ) |
195 | @@ -121,9 +122,14 @@ | |||
196 | 121 | return err | 122 | return err |
197 | 122 | } | 123 | } |
198 | 123 | 124 | ||
199 | 125 | privateKey, err := common.GenerateSystemSSHKey(args.Environ) | ||
200 | 126 | if err != nil { | ||
201 | 127 | return err | ||
202 | 128 | } | ||
203 | 129 | |||
204 | 124 | // Finally, provision the machine agent. | 130 | // Finally, provision the machine agent. |
205 | 125 | stateFileURL := fmt.Sprintf("file://%s/%s", storageDir, bootstrap.StateFile) | 131 | stateFileURL := fmt.Sprintf("file://%s/%s", storageDir, bootstrap.StateFile) |
207 | 126 | mcfg := environs.NewBootstrapMachineConfig(stateFileURL) | 132 | mcfg := environs.NewBootstrapMachineConfig(stateFileURL, privateKey) |
208 | 127 | if args.DataDir != "" { | 133 | if args.DataDir != "" { |
209 | 128 | mcfg.DataDir = args.DataDir | 134 | mcfg.DataDir = args.DataDir |
210 | 129 | } | 135 | } |
211 | 130 | 136 | ||
212 | === modified file 'provider/common/bootstrap.go' | |||
213 | --- provider/common/bootstrap.go 2013-12-20 02:38:56 +0000 | |||
214 | +++ provider/common/bootstrap.go 2014-01-05 22:29:00 +0000 | |||
215 | @@ -46,7 +46,12 @@ | |||
216 | 46 | if err != nil { | 46 | if err != nil { |
217 | 47 | return err | 47 | return err |
218 | 48 | } | 48 | } |
220 | 49 | machineConfig := environs.NewBootstrapMachineConfig(stateFileURL) | 49 | |
221 | 50 | privateKey, err := GenerateSystemSSHKey(env) | ||
222 | 51 | if err != nil { | ||
223 | 52 | return err | ||
224 | 53 | } | ||
225 | 54 | machineConfig := environs.NewBootstrapMachineConfig(stateFileURL, privateKey) | ||
226 | 50 | 55 | ||
227 | 51 | selectedTools, err := EnsureBootstrapTools(env, env.Config().DefaultSeries(), cons.Arch) | 56 | selectedTools, err := EnsureBootstrapTools(env, env.Config().DefaultSeries(), cons.Arch) |
228 | 52 | if err != nil { | 57 | if err != nil { |
229 | @@ -76,6 +81,29 @@ | |||
230 | 76 | return FinishBootstrap(ctx, inst, machineConfig) | 81 | return FinishBootstrap(ctx, inst, machineConfig) |
231 | 77 | } | 82 | } |
232 | 78 | 83 | ||
233 | 84 | // GenerateSystemSSHKey creates a new key for the system identity. The | ||
234 | 85 | // authorized_keys in the environment config is updated to include the public | ||
235 | 86 | // key for the generated key. | ||
236 | 87 | func GenerateSystemSSHKey(env environs.Environ) (privateKey string, err error) { | ||
237 | 88 | logger.Debugf("generate a system ssh key") | ||
238 | 89 | // Create a new system ssh key and add that to the authorized keys. | ||
239 | 90 | privateKey, publicKey, err := ssh.GenerateKey("juju-system-key") | ||
240 | 91 | if err != nil { | ||
241 | 92 | return "", fmt.Errorf("failed to create system key: %v", err) | ||
242 | 93 | } | ||
243 | 94 | authorized_keys := env.Config().AuthorizedKeys() + publicKey | ||
244 | 95 | newConfig, err := env.Config().Apply(map[string]interface{}{ | ||
245 | 96 | "authorized-keys": authorized_keys, | ||
246 | 97 | }) | ||
247 | 98 | if err != nil { | ||
248 | 99 | return "", fmt.Errorf("failed to create new config: %v", err) | ||
249 | 100 | } | ||
250 | 101 | if err = env.SetConfig(newConfig); err != nil { | ||
251 | 102 | return "", fmt.Errorf("failed to set new config: %v", err) | ||
252 | 103 | } | ||
253 | 104 | return privateKey, nil | ||
254 | 105 | } | ||
255 | 106 | |||
256 | 79 | // handelBootstrapError cleans up after a failed bootstrap. | 107 | // handelBootstrapError cleans up after a failed bootstrap. |
257 | 80 | func handleBootstrapError(err error, ctx environs.BootstrapContext, inst instance.Instance, env environs.Environ) { | 108 | func handleBootstrapError(err error, ctx environs.BootstrapContext, inst instance.Instance, env environs.Environ) { |
258 | 81 | if err == nil { | 109 | if err == nil { |
259 | 82 | 110 | ||
260 | === modified file 'provider/common/bootstrap_test.go' | |||
261 | --- provider/common/bootstrap_test.go 2013-12-20 09:25:57 +0000 | |||
262 | +++ provider/common/bootstrap_test.go 2014-01-05 22:29:00 +0000 | |||
263 | @@ -107,7 +107,7 @@ | |||
264 | 107 | instance.Instance, *instance.HardwareCharacteristics, error, | 107 | instance.Instance, *instance.HardwareCharacteristics, error, |
265 | 108 | ) { | 108 | ) { |
266 | 109 | c.Assert(cons, gc.DeepEquals, checkCons) | 109 | c.Assert(cons, gc.DeepEquals, checkCons) |
268 | 110 | c.Assert(mcfg, gc.DeepEquals, environs.NewBootstrapMachineConfig(checkURL)) | 110 | c.Assert(mcfg, gc.DeepEquals, environs.NewBootstrapMachineConfig(checkURL, mcfg.SystemPrivateSSHKey)) |
269 | 111 | return nil, nil, fmt.Errorf("meh, not started") | 111 | return nil, nil, fmt.Errorf("meh, not started") |
270 | 112 | } | 112 | } |
271 | 113 | 113 | ||
272 | @@ -209,11 +209,15 @@ | |||
273 | 209 | checkURL = mcfg.StateInfoURL | 209 | checkURL = mcfg.StateInfoURL |
274 | 210 | return &mockInstance{id: checkInstanceId}, &checkHardware, nil | 210 | return &mockInstance{id: checkInstanceId}, &checkHardware, nil |
275 | 211 | } | 211 | } |
277 | 212 | 212 | var mocksConfig = minimalConfig(c) | |
278 | 213 | var getConfigCalled int | 213 | var getConfigCalled int |
279 | 214 | getConfig := func() *config.Config { | 214 | getConfig := func() *config.Config { |
280 | 215 | getConfigCalled++ | 215 | getConfigCalled++ |
282 | 216 | return minimalConfig(c) | 216 | return mocksConfig |
283 | 217 | } | ||
284 | 218 | setConfig := func(c *config.Config) error { | ||
285 | 219 | mocksConfig = c | ||
286 | 220 | return nil | ||
287 | 217 | } | 221 | } |
288 | 218 | 222 | ||
289 | 219 | restore := envtesting.DisableFinishBootstrap() | 223 | restore := envtesting.DisableFinishBootstrap() |
290 | @@ -223,7 +227,9 @@ | |||
291 | 223 | storage: stor, | 227 | storage: stor, |
292 | 224 | startInstance: startInstance, | 228 | startInstance: startInstance, |
293 | 225 | config: getConfig, | 229 | config: getConfig, |
294 | 230 | setConfig: setConfig, | ||
295 | 226 | } | 231 | } |
296 | 232 | originalAuthKeys := env.Config().AuthorizedKeys() | ||
297 | 227 | ctx, _ := bootstrapContext(c) | 233 | ctx, _ := bootstrapContext(c) |
298 | 228 | err := common.Bootstrap(ctx, env, constraints.Value{}) | 234 | err := common.Bootstrap(ctx, env, constraints.Value{}) |
299 | 229 | c.Assert(err, gc.IsNil) | 235 | c.Assert(err, gc.IsNil) |
300 | @@ -234,6 +240,9 @@ | |||
301 | 234 | StateInstances: []instance.Id{instance.Id(checkInstanceId)}, | 240 | StateInstances: []instance.Id{instance.Id(checkInstanceId)}, |
302 | 235 | Characteristics: []instance.HardwareCharacteristics{checkHardware}, | 241 | Characteristics: []instance.HardwareCharacteristics{checkHardware}, |
303 | 236 | }) | 242 | }) |
304 | 243 | authKeys := env.Config().AuthorizedKeys() | ||
305 | 244 | c.Assert(authKeys, gc.Not(gc.Equals), originalAuthKeys) | ||
306 | 245 | c.Assert(authKeys, jc.HasSuffix, "juju-system-key\n") | ||
307 | 237 | } | 246 | } |
308 | 238 | 247 | ||
309 | 239 | type neverRefreshes struct { | 248 | type neverRefreshes struct { |
310 | 240 | 249 | ||
311 | === modified file 'provider/ec2/local_test.go' | |||
312 | --- provider/ec2/local_test.go 2013-12-20 02:38:56 +0000 | |||
313 | +++ provider/ec2/local_test.go 2014-01-05 22:29:00 +0000 | |||
314 | @@ -240,11 +240,12 @@ | |||
315 | 240 | var userDataMap map[interface{}]interface{} | 240 | var userDataMap map[interface{}]interface{} |
316 | 241 | err = goyaml.Unmarshal(userData, &userDataMap) | 241 | err = goyaml.Unmarshal(userData, &userDataMap) |
317 | 242 | c.Assert(err, gc.IsNil) | 242 | c.Assert(err, gc.IsNil) |
318 | 243 | expectedAuthKeys := strings.TrimSpace(env.Config().AuthorizedKeys()) | ||
319 | 243 | c.Assert(userDataMap, gc.DeepEquals, map[interface{}]interface{}{ | 244 | c.Assert(userDataMap, gc.DeepEquals, map[interface{}]interface{}{ |
320 | 244 | "output": map[interface{}]interface{}{ | 245 | "output": map[interface{}]interface{}{ |
321 | 245 | "all": "| tee -a /var/log/cloud-init-output.log", | 246 | "all": "| tee -a /var/log/cloud-init-output.log", |
322 | 246 | }, | 247 | }, |
324 | 247 | "ssh_authorized_keys": []interface{}{"my-keys"}, | 248 | "ssh_authorized_keys": []interface{}{expectedAuthKeys}, |
325 | 248 | "runcmd": []interface{}{ | 249 | "runcmd": []interface{}{ |
326 | 249 | "install -D -m 644 /dev/null '/var/lib/juju/nonce.txt'", | 250 | "install -D -m 644 /dev/null '/var/lib/juju/nonce.txt'", |
327 | 250 | "printf '%s\\n' 'user-admin:bootstrap' > '/var/lib/juju/nonce.txt'", | 251 | "printf '%s\\n' 'user-admin:bootstrap' > '/var/lib/juju/nonce.txt'", |
328 | 251 | 252 | ||
329 | === modified file 'provider/local/environ.go' | |||
330 | --- provider/local/environ.go 2013-12-19 08:40:11 +0000 | |||
331 | +++ provider/local/environ.go 2014-01-05 22:29:00 +0000 | |||
332 | @@ -114,6 +114,10 @@ | |||
333 | 114 | } | 114 | } |
334 | 115 | 115 | ||
335 | 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. |
336 | 117 | privateKey, err := common.GenerateSystemSSHKey(env) | ||
337 | 118 | if err != nil { | ||
338 | 119 | return err | ||
339 | 120 | } | ||
340 | 117 | 121 | ||
341 | 118 | cert, key, err := env.setupLocalMongoService() | 122 | cert, key, err := env.setupLocalMongoService() |
342 | 119 | if err != nil { | 123 | if err != nil { |
343 | @@ -151,7 +155,7 @@ | |||
344 | 151 | return err | 155 | return err |
345 | 152 | } | 156 | } |
346 | 153 | 157 | ||
348 | 154 | return env.setupLocalMachineAgent(cons, selectedTools) | 158 | return env.setupLocalMachineAgent(cons, selectedTools, privateKey) |
349 | 155 | } | 159 | } |
350 | 156 | 160 | ||
351 | 157 | // StateInfo is specified in the Environ interface. | 161 | // StateInfo is specified in the Environ interface. |
352 | @@ -459,11 +463,18 @@ | |||
353 | 459 | return cert, key, nil | 463 | return cert, key, nil |
354 | 460 | } | 464 | } |
355 | 461 | 465 | ||
357 | 462 | func (env *localEnviron) setupLocalMachineAgent(cons constraints.Value, possibleTools tools.List) error { | 466 | func (env *localEnviron) setupLocalMachineAgent(cons constraints.Value, possibleTools tools.List, privateKey string) error { |
358 | 463 | dataDir := env.config.rootDir() | 467 | dataDir := env.config.rootDir() |
359 | 464 | // unpack the first tools into the agent dir. | 468 | // unpack the first tools into the agent dir. |
360 | 465 | agentTools := possibleTools[0] | 469 | agentTools := possibleTools[0] |
361 | 466 | logger.Debugf("tools: %#v", agentTools) | 470 | logger.Debugf("tools: %#v", agentTools) |
362 | 471 | // save the system identity file | ||
363 | 472 | systemIdentityFilename := filepath.Join(dataDir, cloudinit.SystemIdentity) | ||
364 | 473 | logger.Debugf("writing system identity to %s", systemIdentityFilename) | ||
365 | 474 | if err := ioutil.WriteFile(systemIdentityFilename, []byte(privateKey), 0600); err != nil { | ||
366 | 475 | return fmt.Errorf("failed to write system identity: %v", err) | ||
367 | 476 | } | ||
368 | 477 | |||
369 | 467 | // brutally abuse our knowledge of storage to directly open the file | 478 | // brutally abuse our knowledge of storage to directly open the file |
370 | 468 | toolsUrl, err := url.Parse(agentTools.URL) | 479 | toolsUrl, err := url.Parse(agentTools.URL) |
371 | 469 | if err != nil { | 480 | if err != nil { |
372 | 470 | 481 | ||
373 | === added file 'utils/ssh/generate.go' | |||
374 | --- utils/ssh/generate.go 1970-01-01 00:00:00 +0000 | |||
375 | +++ utils/ssh/generate.go 2014-01-05 22:29:00 +0000 | |||
376 | @@ -0,0 +1,49 @@ | |||
377 | 1 | // Copyright 2013 Canonical Ltd. | ||
378 | 2 | // Licensed under the AGPLv3, see LICENCE file for details. | ||
379 | 3 | |||
380 | 4 | package ssh | ||
381 | 5 | |||
382 | 6 | import ( | ||
383 | 7 | "crypto/rand" | ||
384 | 8 | "crypto/rsa" | ||
385 | 9 | "crypto/x509" | ||
386 | 10 | "encoding/pem" | ||
387 | 11 | "fmt" | ||
388 | 12 | "strings" | ||
389 | 13 | |||
390 | 14 | "code.google.com/p/go.crypto/ssh" | ||
391 | 15 | ) | ||
392 | 16 | |||
393 | 17 | // KeyBits is used to determine the number of bits to use for the RSA keys | ||
394 | 18 | // created using the GenerateKey function. | ||
395 | 19 | var KeyBits = 2048 | ||
396 | 20 | |||
397 | 21 | // GenerateKey makes a 2048 bit RSA no-passphrase SSH capable key. The bit | ||
398 | 22 | // size is actually controlled by the KeyBits var. The private key returned is | ||
399 | 23 | // encoded to ASCII using the PKCS1 encoding. The public key is suitable to | ||
400 | 24 | // be added into an authorized_keys file, and has the comment passed in as the | ||
401 | 25 | // comment part of the key. | ||
402 | 26 | func GenerateKey(comment string) (private, public string, err error) { | ||
403 | 27 | key, err := rsa.GenerateKey(rand.Reader, KeyBits) | ||
404 | 28 | if err != nil { | ||
405 | 29 | return "", "", err | ||
406 | 30 | } | ||
407 | 31 | |||
408 | 32 | identity := pem.EncodeToMemory( | ||
409 | 33 | &pem.Block{ | ||
410 | 34 | Type: "RSA PRIVATE KEY", | ||
411 | 35 | Bytes: x509.MarshalPKCS1PrivateKey(key), | ||
412 | 36 | }) | ||
413 | 37 | |||
414 | 38 | signer, err := ssh.ParsePrivateKey(identity) | ||
415 | 39 | if err != nil { | ||
416 | 40 | return "", "", fmt.Errorf("failed to load key: %v", err) | ||
417 | 41 | } | ||
418 | 42 | |||
419 | 43 | auth_key := string(ssh.MarshalAuthorizedKey(signer.PublicKey())) | ||
420 | 44 | // Strip off the trailing new line so we can add a comment. | ||
421 | 45 | auth_key = strings.TrimSpace(auth_key) | ||
422 | 46 | public = fmt.Sprintf("%s %s\n", auth_key, comment) | ||
423 | 47 | |||
424 | 48 | return string(identity), public, nil | ||
425 | 49 | } | ||
426 | 0 | 50 | ||
427 | === added file 'utils/ssh/generate_test.go' | |||
428 | --- utils/ssh/generate_test.go 1970-01-01 00:00:00 +0000 | |||
429 | +++ utils/ssh/generate_test.go 2014-01-05 22:29:00 +0000 | |||
430 | @@ -0,0 +1,28 @@ | |||
431 | 1 | // Copyright 2013 Canonical Ltd. | ||
432 | 2 | // Licensed under the AGPLv3, see LICENCE file for details. | ||
433 | 3 | |||
434 | 4 | package ssh_test | ||
435 | 5 | |||
436 | 6 | import ( | ||
437 | 7 | gc "launchpad.net/gocheck" | ||
438 | 8 | |||
439 | 9 | jc "launchpad.net/juju-core/testing/checkers" | ||
440 | 10 | "launchpad.net/juju-core/testing/testbase" | ||
441 | 11 | "launchpad.net/juju-core/utils/ssh" | ||
442 | 12 | ) | ||
443 | 13 | |||
444 | 14 | type GenerateSuite struct { | ||
445 | 15 | testbase.LoggingSuite | ||
446 | 16 | } | ||
447 | 17 | |||
448 | 18 | var _ = gc.Suite(&GenerateSuite{}) | ||
449 | 19 | |||
450 | 20 | func (s *GenerateSuite) TestGenerate(c *gc.C) { | ||
451 | 21 | private, public, err := ssh.GenerateKey("some-comment") | ||
452 | 22 | |||
453 | 23 | c.Check(err, gc.IsNil) | ||
454 | 24 | c.Check(private, jc.HasPrefix, "-----BEGIN RSA PRIVATE KEY-----\n") | ||
455 | 25 | c.Check(private, jc.HasSuffix, "-----END RSA PRIVATE KEY-----\n") | ||
456 | 26 | c.Check(public, jc.HasPrefix, "ssh-rsa ") | ||
457 | 27 | c.Check(public, jc.HasSuffix, " some-comment\n") | ||
458 | 28 | } |
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): sshinit/ configure_ test.go ssh-key. txt cloudinit. go cloudinit/ cloudinit. go cloudinit/ cloudinit_ test.go manual/ bootstrap. go common/ bootstrap. go common/ bootstrap_ test.go ec2/local_ test.go local/environ. go generate. go generate_ test.go systemidentity. go systemidentity_ test.go
A [revision details]
M cloudinit/
A doc/system-
M environs/
M environs/
M environs/
M environs/
M provider/
M provider/
M provider/
M provider/
A utils/ssh/
A utils/ssh/
A utils/ssh/
A utils/ssh/