Merge pull request #7278 from axw/ssh-stricthostkeychecking

Put the secure into secure shell

## Description of change

Up until now, Juju bootstrap has had an insecure
initial SSH connection. The client had no way of
knowing the server's host key, and so strict
host key checking was disabled.

This branch does two main things:
 - updates to the new juju/utils version which
   defaults strict host key checking to "ask"
   (unless overridden in OpenSSH client config)
   for both the golang.org/x/crypto and openssh
   implementations
 - generates and injects an SSH host key into
   the created server via cloud-init, then uses
   the public key to perform strict host key
   checking. The key is regenerated server-side
   as the first thing the initial SSH connection
   does, so that user code deployed to controllers
   cannot sniff the keys from metadata services.

## QA steps

1. juju bootstrap localhost
2. lxc launch ubuntu:xenial x
3. lxc file push ~/.ssh/id_rsa.pub x/home/ubuntu/.ssh/authorized_keys
4. juju add-machine ssh:<ip-of-x>
5. juju ssh -m controller 0 true
6. juju ssh 0 true

Repeat with "ssh" removed from $PATH. There should be no warnings about SSH host keys changing, nor warnings about host keys being added to known_hosts.

## Documentation changes

There is one change for users: on Windows, manual provisioning will now prompt the user to verify host SSH keys.

## Bug reference

Fixes https://bugs.launchpad.net/juju/+bug/1683099
Fixes https://bugs.launchpad.net/juju/+bug/1579593

cloudconfig/cloudinit/cloudinit.go

@@ -325,6 +325,18 @@ func (cfg *cloudConfig) SetSSHAuthorizedKeys(rawKeys string) {
 	}
 }
 
+// SetSSHKeys is defined on the SSHKeysConfig interface.
+func (cfg *cloudConfig) SetSSHKeys(keys SSHKeys) {
+	if keys.RSA != nil {
+		cfg.SetAttr("ssh_keys", map[string]interface{}{
+			string(RSAPrivate): keys.RSA.Private,
+			string(RSAPublic):  keys.RSA.Public,
+		})
+	} else {
+		cfg.UnsetAttr("ssh_keys")
+	}
+}
+
 // SetDisableRoot is defined on the RootUserConfig interface.
 func (cfg *cloudConfig) SetDisableRoot(disable bool) {
 	cfg.SetAttr("disable_root", disable)

cloudconfig/cloudinit/cloudinit_test.go

@@ -366,6 +366,32 @@ var ctests = []struct {
 	func(cfg cloudinit.CloudConfig) {
 		cfg.ManageEtcHosts(true)
 	},
+}, {
+	"SetSSHKeys",
+	map[string]interface{}{"ssh_keys": map[string]interface{}{
+		"rsa_private": "private",
+		"rsa_public":  "public",
+	}},
+	func(cfg cloudinit.CloudConfig) {
+		cfg.SetSSHKeys(cloudinit.SSHKeys{
+			RSA: &cloudinit.SSHKey{
+				Private: "private",
+				Public:  "public",
+			},
+		})
+	},
+}, {
+	"SetSSHKeys unsets keys",
+	map[string]interface{}{},
+	func(cfg cloudinit.CloudConfig) {
+		cfg.SetSSHKeys(cloudinit.SSHKeys{
+			RSA: &cloudinit.SSHKey{
+				Private: "private",
+				Public:  "public",
+			},
+		})
+		cfg.SetSSHKeys(cloudinit.SSHKeys{})
+	},
 }}
 
 func (S) TestOutput(c *gc.C) {

cloudconfig/cloudinit/interface.go

@@ -49,6 +49,7 @@ type CloudConfig interface {
 	DeviceMountConfig
 	OutputConfig
 	SSHAuthorizedKeysConfig
+	SSHKeysConfig
 	RootUserConfig
 	WrittenFilesConfig
 	RenderConfig
@@ -271,6 +272,12 @@ type SSHAuthorizedKeysConfig interface {
 	SetSSHAuthorizedKeys(string)
 }
 
+// SSHKeysConfig is the interface for setting ssh host keys.
+type SSHKeysConfig interface {
+	// SetSSHKeys sets the SSH host keys for the machine.
+	SetSSHKeys(SSHKeys)
+}
+
 // RootUserConfig is the interface for all root user-related settings.
 type RootUserConfig interface {
 	// SetDisableRoot sets whether ssh login to the root account of the new server
@@ -432,6 +439,20 @@ func New(ser string) (CloudConfig, error) {
 	}
 }
 
+// SSHKeys contains SSH host keys to configure on a machine.
+type SSHKeys struct {
+	RSA *SSHKey
+}
+
+// SSHKey is an SSH key pair.
+type SSHKey struct {
+	// Private is the SSH private key.
+	Private string
+
+	// Public is the SSH public key.
+	Public string
+}
+
 // SSHKeyType is the type of the four used key types passed to cloudinit
 // through the cloudconfig
 type SSHKeyType string

cloudconfig/instancecfg/instancecfg.go

@@ -184,13 +184,41 @@ type BootstrapConfig struct {
 	// Timeout is the amount of time to wait for bootstrap to complete.
 	Timeout time.Duration
 
+	// InitialSSHHostKeys contains the initial SSH host keys to configure
+	// on the bootstrap machine, indexed by algorithm. These will only be
+	// valid for the initial SSH connection. The first thing we do upon
+	// making the initial SSH connection is to replace each of these host
+	// keys, to avoid the host keys being extracted from the metadata
+	// service by a bad actor post-bootstrap.
+	//
+	// Any existing host keys on the machine with algorithms not specified
+	// in the map will be left alone. This is important so that we do not
+	// trample on the host keys of manually provisioned machines.
+	InitialSSHHostKeys SSHHostKeys
+
 	// StateServingInfo holds the information for serving the state.
 	// This is only specified for bootstrap; controllers started
 	// subsequently will acquire their serving info from another
 	// server.
 	StateServingInfo params.StateServingInfo
 }
 
+// SSHHostKeys contains the SSH host keys to configure for a bootstrap host.
+type SSHHostKeys struct {
+	// RSA, if non-nil, contains the RSA key to configure as the initial
+	// SSH host key.
+	RSA *SSHKeyPair
+}
+
+// SSHKeyPair is an SSH host key pair.
+type SSHKeyPair struct {
+	// Private contains the private key, PEM-encoded.
+	Private string
+
+	// Public contains the public key in authorized_keys format.
+	Public string
+}
+
 // StateInitializationParams contains parameters for initializing the
 // state database.
 //

cloudconfig/sshinit/configure.go

@@ -27,6 +27,9 @@ type ConfigureParams struct {
 	// If Client is nil, ssh.DefaultClient will be used.
 	Client ssh.Client
 
+	// SSHOptions contains options for running the SSH command.
+	SSHOptions *ssh.Options
+
 	// Config is the cloudinit config to carry out.
 	Config cloudinit.CloudConfig
 
@@ -52,12 +55,17 @@ cat > $tmpfile
 /bin/bash $tmpfile
 `))
 
+	client := params.Client
+	if client == nil {
+		client = ssh.DefaultClient
+	}
+
 	// bash will read a byte at a time when consuming commands
 	// from stdin. We avoid sending the entire script -- which
 	// will be very large when uploading tools -- directly to
 	// bash for this reason. Instead, run cat which will write
 	// the script to disk, and then execute it from there.
-	cmd := ssh.Command(params.Host, []string{
+	cmd := client.Command(params.Host, []string{
 		"sudo", "/bin/bash", "-c",
 		// The outer bash interprets the $(...), and executes
 		// the decoded script in the nested bash. This avoids
@@ -67,7 +75,7 @@ cat > $tmpfile
 			`/bin/bash -c "$(echo %s | base64 -d)"`,
 			utils.ShQuote(encoded),
 		),
-	}, nil)
+	}, params.SSHOptions)
 
 	cmd.Stdin = strings.NewReader(script)
 	cmd.Stderr = params.ProgressWriter

cloudconfig/userdatacfg_test.go

@@ -1360,3 +1360,39 @@ func (*cloudinitSuite) TestSetUbuntuUserCentOS(c *gc.C) {
 	expected := expectedUbuntuUser(cloudconfig.CentOSGroups, keys)
 	c.Assert(string(data), jc.YAMLEquals, expected)
 }
+
+func (*cloudinitSuite) TestCloudInitBootstrapInitialSSHKeys(c *gc.C) {
+	instConfig := makeBootstrapConfig("quantal").maybeSetModelConfig(
+		minimalModelConfig(c),
+	).render()
+	instConfig.Bootstrap.InitialSSHHostKeys.RSA = &instancecfg.SSHKeyPair{
+		Private: "private",
+		Public:  "public",
+	}
+	cloudcfg, err := cloudinit.New(instConfig.Series)
+	c.Assert(err, jc.ErrorIsNil)
+
+	udata, err := cloudconfig.NewUserdataConfig(&instConfig, cloudcfg)
+	c.Assert(err, jc.ErrorIsNil)
+	err = udata.Configure()
+	c.Assert(err, jc.ErrorIsNil)
+	data, err := cloudcfg.RenderYAML()
+	c.Assert(err, jc.ErrorIsNil)
+
+	configKeyValues := make(map[interface{}]interface{})
+	err = goyaml.Unmarshal(data, &configKeyValues)
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(configKeyValues["ssh_keys"], jc.DeepEquals, map[interface{}]interface{}{
+		"rsa_private": "private",
+		"rsa_public":  "public",
+	})
+
+	cmds := cloudcfg.BootCmds()
+	c.Assert(cmds, jc.DeepEquals, []string{
+		`echo 'Regenerating SSH RSA host key' >&$JUJU_PROGRESS_FD`,
+		`rm /etc/ssh/ssh_host_rsa_key*`,
+		`ssh-keygen -t rsa -N "" -f /etc/ssh/ssh_host_rsa_key`,
+		`ssh-keygen -t dsa -N "" -f /etc/ssh/ssh_host_dsa_key`,
+		`ssh-keygen -t ecdsa -N "" -f /etc/ssh/ssh_host_ecdsa_key`,
+	})
+}

cloudconfig/userdatacfg_unix.go

@@ -124,6 +124,21 @@ func (w *unixConfigure) ConfigureBasic() error {
 		w.addCleanShutdownJob(service.InitSystemSystemd)
 	}
 	SetUbuntuUser(w.conf, w.icfg.AuthorizedKeys)
+
+	if w.icfg.Bootstrap != nil {
+		// For the bootstrap machine only, we set the host keys
+		// except when manually provisioning.
+		icfgKeys := w.icfg.Bootstrap.InitialSSHHostKeys
+		var keys cloudinit.SSHKeys
+		if icfgKeys.RSA != nil {
+			keys.RSA = &cloudinit.SSHKey{
+				Private: icfgKeys.RSA.Private,
+				Public:  icfgKeys.RSA.Public,
+			}
+		}
+		w.conf.SetSSHKeys(keys)
+	}
+
 	w.conf.SetOutput(cloudinit.OutAll, "| tee -a "+w.icfg.CloudInitOutputLog, "")
 	// Create a file in a well-defined location containing the machine's
 	// nonce. The presence and contents of this file will be verified
@@ -183,6 +198,25 @@ func (w *unixConfigure) ConfigureJuju() error {
 		w.conf.AddBootCmd(cloudinit.LogProgressCmd("Logging to %s on the bootstrap machine", w.icfg.CloudInitOutputLog))
 	}
 
+	if w.icfg.Bootstrap != nil {
+		// Before anything else, we must regenerate the SSH host keys.
+		var any bool
+		keys := w.icfg.Bootstrap.InitialSSHHostKeys
+		if keys.RSA != nil {
+			any = true
+			w.conf.AddBootCmd(cloudinit.LogProgressCmd("Regenerating SSH RSA host key"))
+			w.conf.AddBootCmd(`rm /etc/ssh/ssh_host_rsa_key*`)
+			w.conf.AddBootCmd(`ssh-keygen -t rsa -N "" -f /etc/ssh/ssh_host_rsa_key`)
+		}
+		if any {
+			// ssh_keys was specified in cloud-config, which will
+			// disable all key generation. Generate the other keys
+			// that we did not generate previously.
+			w.conf.AddBootCmd(`ssh-keygen -t dsa -N "" -f /etc/ssh/ssh_host_dsa_key`)
+			w.conf.AddBootCmd(`ssh-keygen -t ecdsa -N "" -f /etc/ssh/ssh_host_ecdsa_key`)
+		}
+	}
+
 	w.conf.AddPackageCommands(
 		w.icfg.AptProxySettings,
 		w.icfg.AptMirror,

cmd/juju/commands/ssh_common.go

@@ -172,7 +172,7 @@ func (c *SSHCommon) getSSHOptions(enablePty bool, targets ...*resolvedTarget) (*
 
 	if c.noHostKeyChecks {
 		options.SetStrictHostKeyChecking(ssh.StrictHostChecksNo)
-		options.SetKnownHostsFile("/dev/null")
+		options.SetKnownHostsFile(os.DevNull)
 	} else {
 		knownHostsPath, err := c.generateKnownHosts(targets)
 		if err != nil {
@@ -189,10 +189,6 @@ func (c *SSHCommon) getSSHOptions(enablePty bool, targets ...*resolvedTarget) (*
 			// strict host key checking.
 			options.SetStrictHostKeyChecking(ssh.StrictHostChecksYes)
 			options.SetKnownHostsFile(knownHostsPath)
-		} else {
-			// If the user's personal known_hosts is used, also use
-			// the user's personal StrictHostKeyChecking preferences.
-			options.SetStrictHostKeyChecking(ssh.StrictHostChecksUnset)
 		}
 	}
 

dependencies.tsv

@@ -48,7 +48,7 @@ github.com/juju/terms-client	git	9b925afd677234e4146dde3cb1a11e187cbed64e	2016-0
 github.com/juju/testing	git	06d21ddace802a83d08c82f513e30d84010ce31f	2017-05-01T02:35:42Z
 github.com/juju/txn	git	5c6f1c49ecbd4a916ed7b5a8f81ee67203e86043	2017-04-21T05:33:50Z
 github.com/juju/usso	git	68a59c96c178fbbad65926e7f93db50a2cd14f33	2016-04-01T10:44:24Z
-github.com/juju/utils	git	51660d020f8c823ce1d0b7904c26e6b907163050	2017-04-21T12:10:45Z
+github.com/juju/utils	git	6622634c402775ec773bb05427bccf976e6917e7	2017-04-27T05:01:38Z
 github.com/juju/version	git	1f41e27e54f21acccf9b2dddae063a782a8a7ceb	2016-10-31T05:19:06Z
 github.com/juju/webbrowser	git	54b8c57083b4afb7dc75da7f13e2967b2606a507	2016-03-09T14:36:29Z
 github.com/juju/xml	git	eb759a627588d35166bc505fceb51b88500e291e	2015-04-13T13:11:21Z
@@ -78,7 +78,7 @@ github.com/prometheus/common	git	dd586c1c5abb0be59e60f942c22af711a2008cb4	2016-0
 github.com/prometheus/procfs	git	abf152e5f3e97f2fafac028d2cc06c1feb87ffa5	2016-04-11T19:08:41Z
 github.com/rogpeppe/fastuuid	git	6724a57986aff9bff1a1770e9347036def7c89f6	2015-01-06T09:32:20Z
 github.com/vmware/govmomi	git	c0c7ce63df7edd78e713257b924c89d9a2dac119	2016-06-30T15:37:42Z
-golang.org/x/crypto	git	8e06e8ddd9629eb88639aba897641bff8031f1d3	2016-09-22T17:06:29Z
+golang.org/x/crypto	git	96846453c37f0876340a66a47f3f75b1f3a6cd2d	2017-04-21T04:31:20Z
 golang.org/x/net	git	ea47fc708ee3e20177f3ca3716217c4ab75942cb	2015-08-29T23:03:18Z
 golang.org/x/oauth2	git	11c60b6f71a6ad48ed6f93c65fa4c6f9b1b5b46a	2015-03-25T02:00:22Z
 golang.org/x/sys	git	7a6e5648d140666db5d920909e082ca00a87ba2c	2017-02-01T05:12:45Z

juju/home.go

@@ -4,6 +4,8 @@
 package juju
 
 import (
+	"path/filepath"
+
 	"github.com/juju/errors"
 	"github.com/juju/utils/ssh"
 	"gopkg.in/juju/charmrepo.v2-unstable"
@@ -20,9 +22,12 @@ func InitJujuXDGDataHome() error {
 		return errors.New("cannot determine juju data home, required environment variables are not set")
 	}
 	charmrepo.CacheDir = osenv.JujuXDGDataHomePath("charmcache")
-	if err := ssh.LoadClientKeys(osenv.JujuXDGDataHomePath("ssh")); err != nil {
+
+	sshDir := osenv.JujuXDGDataHomePath("ssh")
+	if err := ssh.LoadClientKeys(sshDir); err != nil {
 		return errors.Annotate(err, "cannot load ssh client keys")
 	}
+	ssh.SetGoCryptoKnownHostsFile(filepath.Join(sshDir, "gocrypto_known_hosts"))
 
 	return nil
 }