ssh: accept private key contents instead of path

We've been moving away from config fields expecting file paths that
Terraform will load, instead prefering fields that expect file contents,
leaning on `file()` to do loading from a path.

This helps with consistency and also flexibility - since this makes it
easier to shift sensitive files into environment variables.

Here we add a little helper package to manage the transitional period
for these fields where we support both behaviors.

Also included is the first of several fields being shifted over - SSH
private keys in provisioner connection config.

We're moving to new field names so the behavior is more intuitive, so
instead of `key_file` it's `private_key` now.

Additional field shifts will be included in follow up PRs so they can be
reviewed and discussed individually.

communicator/ssh/communicator.go

@@ -93,7 +93,7 @@ func (c *Communicator) Connect(o terraform.UIOutput) (err error) {
 				"  SSH Agent: %t",
 			c.connInfo.Host, c.connInfo.User,
 			c.connInfo.Password != "",
-			c.connInfo.KeyFile != "",
+			c.connInfo.PrivateKey != "",
 			c.connInfo.Agent,
 		))
 
@@ -107,7 +107,7 @@ func (c *Communicator) Connect(o terraform.UIOutput) (err error) {
 					"  SSH Agent: %t",
 				c.connInfo.BastionHost, c.connInfo.BastionUser,
 				c.connInfo.BastionPassword != "",
-				c.connInfo.BastionKeyFile != "",
+				c.connInfo.BastionPrivateKey != "",
 				c.connInfo.Agent,
 			))
 		}

communicator/ssh/provisioner.go

@@ -3,14 +3,13 @@ package ssh
 import (
 	"encoding/pem"
 	"fmt"
-	"io/ioutil"
 	"log"
 	"net"
 	"os"
 	"time"
 
+	"github.com/hashicorp/terraform/helper/pathorcontents"
 	"github.com/hashicorp/terraform/terraform"
-	"github.com/mitchellh/go-homedir"
 	"github.com/mitchellh/mapstructure"
 	"golang.org/x/crypto/ssh"
 	"golang.org/x/crypto/ssh/agent"
@@ -37,7 +36,7 @@ const (
 type connectionInfo struct {
 	User       string
 	Password   string
-	KeyFile    string `mapstructure:"key_file"`
+	PrivateKey string `mapstructure:"private_key"`
 	Host       string
 	Port       int
 	Agent      bool
@@ -45,11 +44,15 @@ type connectionInfo struct {
 	ScriptPath string        `mapstructure:"script_path"`
 	TimeoutVal time.Duration `mapstructure:"-"`
 
-	BastionUser     string `mapstructure:"bastion_user"`
+	BastionUser       string `mapstructure:"bastion_user"`
-	BastionPassword string `mapstructure:"bastion_password"`
+	BastionPassword   string `mapstructure:"bastion_password"`
-	BastionKeyFile  string `mapstructure:"bastion_key_file"`
+	BastionPrivateKey string `mapstructure:"bastion_private_key"`
-	BastionHost     string `mapstructure:"bastion_host"`
+	BastionHost       string `mapstructure:"bastion_host"`
-	BastionPort     int    `mapstructure:"bastion_port"`
+	BastionPort       int    `mapstructure:"bastion_port"`
+
+	// Deprecated
+	KeyFile        string `mapstructure:"key_file"`
+	BastionKeyFile string `mapstructure:"bastion_key_file"`
 }
 
 // parseConnectionInfo is used to convert the ConnInfo of the InstanceState into
@@ -92,6 +95,15 @@ func parseConnectionInfo(s *terraform.InstanceState) (*connectionInfo, error) {
 		connInfo.TimeoutVal = DefaultTimeout
 	}
 
+	// Load deprecated fields; we can handle either path or contents in
+	// underlying implementation.
+	if connInfo.PrivateKey == "" && connInfo.KeyFile != "" {
+		connInfo.PrivateKey = conninfo.KeyFile
+	}
+	if connInfo.BastionPrivateKey == "" && connInfo.BastionKeyFile != "" {
+		connInfo.BastionPrivateKey = conninfo.BastionKeyFile
+	}
+
 	// Default all bastion config attrs to their non-bastion counterparts
 	if connInfo.BastionHost != "" {
 		if connInfo.BastionUser == "" {
@@ -100,8 +112,8 @@ func parseConnectionInfo(s *terraform.InstanceState) (*connectionInfo, error) {
 		if connInfo.BastionPassword == "" {
 			connInfo.BastionPassword = connInfo.Password
 		}
-		if connInfo.BastionKeyFile == "" {
+		if connInfo.BastionPrivateKey == "" {
-			connInfo.BastionKeyFile = connInfo.KeyFile
+			connInfo.BastionPrivateKey = connInfo.PrivateKey
 		}
 		if connInfo.BastionPort == 0 {
 			connInfo.BastionPort = connInfo.Port
@@ -130,10 +142,10 @@ func prepareSSHConfig(connInfo *connectionInfo) (*sshConfig, error) {
 	}
 
 	sshConf, err := buildSSHClientConfig(sshClientConfigOpts{
-		user:     connInfo.User,
+		user:       connInfo.User,
-		keyFile:  connInfo.KeyFile,
+		privateKey: connInfo.PrivateKey,
-		password: connInfo.Password,
+		password:   connInfo.Password,
-		sshAgent: sshAgent,
+		sshAgent:   sshAgent,
 	})
 	if err != nil {
 		return nil, err
@@ -142,10 +154,10 @@ func prepareSSHConfig(connInfo *connectionInfo) (*sshConfig, error) {
 	var bastionConf *ssh.ClientConfig
 	if connInfo.BastionHost != "" {
 		bastionConf, err = buildSSHClientConfig(sshClientConfigOpts{
-			user:     connInfo.BastionUser,
+			user:       connInfo.BastionUser,
-			keyFile:  connInfo.BastionKeyFile,
+			privateKey: connInfo.BastionPrivateKey,
-			password: connInfo.BastionPassword,
+			password:   connInfo.BastionPassword,
-			sshAgent: sshAgent,
+			sshAgent:   sshAgent,
 		})
 		if err != nil {
 			return nil, err
@@ -169,10 +181,10 @@ func prepareSSHConfig(connInfo *connectionInfo) (*sshConfig, error) {
 }
 
 type sshClientConfigOpts struct {
-	keyFile  string
+	privateKey string
-	password string
+	password   string
-	sshAgent *sshAgent
+	sshAgent   *sshAgent
-	user     string
+	user       string
 }
 
 func buildSSHClientConfig(opts sshClientConfigOpts) (*ssh.ClientConfig, error) {
@@ -181,7 +193,7 @@ func buildSSHClientConfig(opts sshClientConfigOpts) (*ssh.ClientConfig, error) {
 	}
 
 	if opts.keyFile != "" {
-		pubKeyAuth, err := readPublicKeyFromPath(opts.keyFile)
+		pubKeyAuth, err := readPrivateKey(opts.privateKey)
 		if err != nil {
 			return nil, err
 		}
@@ -201,31 +213,27 @@ func buildSSHClientConfig(opts sshClientConfigOpts) (*ssh.ClientConfig, error) {
 	return conf, nil
 }
 
-func readPublicKeyFromPath(path string) (ssh.AuthMethod, error) {
+func readPrivateKey(pk string) (ssh.AuthMethod, error) {
-	fullPath, err := homedir.Expand(path)
+	key, _, err := pathorcontents.Read(pk)
 	if err != nil {
-		return nil, fmt.Errorf("Failed to expand home directory: %s", err)
+		return nil, fmt.Errorf("Failed to read private key %q: %s", pk, err)
-	}
-	key, err := ioutil.ReadFile(fullPath)
-	if err != nil {
-		return nil, fmt.Errorf("Failed to read key file %q: %s", path, err)
 	}
 
 	// We parse the private key on our own first so that we can
 	// show a nicer error if the private key has a password.
-	block, _ := pem.Decode(key)
+	block, _ := pem.Decode([]byte(key))
 	if block == nil {
-		return nil, fmt.Errorf("Failed to read key %q: no key found", path)
+		return nil, fmt.Errorf("Failed to read key %q: no key found", pk)
 	}
 	if block.Headers["Proc-Type"] == "4,ENCRYPTED" {
 		return nil, fmt.Errorf(
 			"Failed to read key %q: password protected keys are\n"+
-				"not supported. Please decrypt the key prior to use.", path)
+				"not supported. Please decrypt the key prior to use.", pk)
 	}
 
-	signer, err := ssh.ParsePrivateKey(key)
+	signer, err := ssh.ParsePrivateKey([]byte(key))
 	if err != nil {
-		return nil, fmt.Errorf("Failed to parse key file %q: %s", path, err)
+		return nil, fmt.Errorf("Failed to parse key file %q: %s", pk, err)
 	}
 
 	return ssh.PublicKeys(signer), nil

communicator/ssh/provisioner_test.go

@@ -10,13 +10,13 @@ func TestProvisioner_connInfo(t *testing.T) {
 	r := &terraform.InstanceState{
 		Ephemeral: terraform.EphemeralState{
 			ConnInfo: map[string]string{
-				"type":     "ssh",
+				"type":        "ssh",
-				"user":     "root",
+				"user":        "root",
-				"password": "supersecret",
+				"password":    "supersecret",
-				"key_file": "/my/key/file.pem",
+				"private_key": "someprivatekeycontents",
-				"host":     "127.0.0.1",
+				"host":        "127.0.0.1",
-				"port":     "22",
+				"port":        "22",
-				"timeout":  "30s",
+				"timeout":     "30s",
 
 				"bastion_host": "127.0.1.1",
 			},
@@ -34,7 +34,7 @@ func TestProvisioner_connInfo(t *testing.T) {
 	if conf.Password != "supersecret" {
 		t.Fatalf("bad: %v", conf)
 	}
-	if conf.KeyFile != "/my/key/file.pem" {
+	if conf.PrivateKey != "someprivatekeycontents" {
 		t.Fatalf("bad: %v", conf)
 	}
 	if conf.Host != "127.0.0.1" {
@@ -61,7 +61,31 @@ func TestProvisioner_connInfo(t *testing.T) {
 	if conf.BastionPassword != "supersecret" {
 		t.Fatalf("bad: %v", conf)
 	}
-	if conf.BastionKeyFile != "/my/key/file.pem" {
+	if conf.BastionPrivateKey != "someprivatekeycontents" {
+		t.Fatalf("bad: %v", conf)
+	}
+}
+
+func TestProvisioner_connInfoLegacy(t *testing.T) {
+	r := &terraform.InstanceState{
+		Ephemeral: terraform.EphemeralState{
+			ConnInfo: map[string]string{
+				"type":         "ssh",
+				"key_file":     "/my/key/file.pem",
+				"bastion_host": "127.0.1.1",
+			},
+		},
+	}
+
+	conf, err := parseConnectionInfo(r)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	if conf.PrivateKey != "/my/key/file.pem" {
+		t.Fatalf("bad: %v", conf)
+	}
+	if conf.BastionPrivateKey != "/my/key/file.pem" {
 		t.Fatalf("bad: %v", conf)
 	}
 }

helper/pathorcontents/read.go

@@ -0,0 +1,40 @@
+// Helpers for dealing with file paths and their contents
+package pathorcontents
+
+import (
+	"io/ioutil"
+	"os"
+
+	"github.com/mitchellh/go-homedir"
+)
+
+// If the argument is a path, Read loads it and returns the contents,
+// otherwise the argument is assumed to be the desired contents and is simply
+// returned.
+//
+// The boolean second return value can be called `wasPath` - it indicates if a
+// path was detected and a file loaded.
+func Read(poc string) (string, bool, error) {
+	if len(poc) == 0 {
+		return poc, false, nil
+	}
+
+	path := poc
+	if path[0] == '~' {
+		var err error
+		path, err = homedir.Expand(path)
+		if err != nil {
+			return path, true, err
+		}
+	}
+
+	if _, err := os.Stat(path); err == nil {
+		contents, err := ioutil.ReadFile(path)
+		if err != nil {
+			return string(contents), true, err
+		}
+		return string(contents), true, nil
+	}
+
+	return poc, false, nil
+}

helper/pathorcontents/read_test.go

@@ -0,0 +1,140 @@
+package pathorcontents
+
+import (
+	"io"
+	"io/ioutil"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/mitchellh/go-homedir"
+)
+
+func TestRead_Path(t *testing.T) {
+	isPath := true
+	f, cleanup := testTempFile(t)
+	defer cleanup()
+
+	if _, err := io.WriteString(f, "foobar"); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	f.Close()
+
+	contents, wasPath, err := Read(f.Name())
+
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	if wasPath != isPath {
+		t.Fatalf("expected wasPath: %t, got %t", isPath, wasPath)
+	}
+	if contents != "foobar" {
+		t.Fatalf("expected contents %s, got %s", "foobar", contents)
+	}
+}
+
+func TestRead_TildePath(t *testing.T) {
+	isPath := true
+	home, err := homedir.Dir()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	f, cleanup := testTempFile(t, home)
+	defer cleanup()
+
+	if _, err := io.WriteString(f, "foobar"); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	f.Close()
+
+	r := strings.NewReplacer(home, "~")
+	homePath := r.Replace(f.Name())
+	contents, wasPath, err := Read(homePath)
+
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	if wasPath != isPath {
+		t.Fatalf("expected wasPath: %t, got %t", isPath, wasPath)
+	}
+	if contents != "foobar" {
+		t.Fatalf("expected contents %s, got %s", "foobar", contents)
+	}
+}
+
+func TestRead_PathNoPermission(t *testing.T) {
+	isPath := true
+	f, cleanup := testTempFile(t)
+	defer cleanup()
+
+	if _, err := io.WriteString(f, "foobar"); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	f.Close()
+
+	if err := os.Chmod(f.Name(), 0); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	contents, wasPath, err := Read(f.Name())
+
+	if err == nil {
+		t.Fatal("Expected error, got none!")
+	}
+	if wasPath != isPath {
+		t.Fatalf("expected wasPath: %t, got %t", isPath, wasPath)
+	}
+	if contents != "" {
+		t.Fatalf("expected contents %s, got %s", "", contents)
+	}
+}
+
+func TestRead_Contents(t *testing.T) {
+	isPath := false
+	input := "hello"
+
+	contents, wasPath, err := Read(input)
+
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	if wasPath != isPath {
+		t.Fatalf("expected wasPath: %t, got %t", isPath, wasPath)
+	}
+	if contents != input {
+		t.Fatalf("expected contents %s, got %s", input, contents)
+	}
+}
+
+func TestRead_TildeContents(t *testing.T) {
+	isPath := false
+	input := "~/hello/notafile"
+
+	contents, wasPath, err := Read(input)
+
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	if wasPath != isPath {
+		t.Fatalf("expected wasPath: %t, got %t", isPath, wasPath)
+	}
+	if contents != input {
+		t.Fatalf("expected contents %s, got %s", input, contents)
+	}
+}
+
+// Returns an open tempfile based at baseDir and a function to clean it up.
+func testTempFile(t *testing.T, baseDir ...string) (*os.File, func()) {
+	base := ""
+	if len(baseDir) == 1 {
+		base = baseDir[0]
+	}
+	f, err := ioutil.TempFile(base, "tf")
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	return f, func() {
+		os.Remove(f.Name())
+	}
+}

website/source/docs/provisioners/connection.html.markdown

@@ -68,8 +68,10 @@ provisioner "file" {
 
 **Additional arguments only supported by the "ssh" connection type:**
 
-* `key_file` - The SSH key to use for the connection. This takes preference over the
+* `private_key` - The contents of an SSH key to use for the connection. These can
-  password if provided.
+  be loaded from a file on disk using the [`file()` interpolation
+  function](/docs/configuration/interpolation.html#file_path_). This takes
+  preference over the password if provided.
 
 * `agent` - Set to false to disable using ssh-agent to authenticate.
 
@@ -99,5 +101,22 @@ The `ssh` connection additionally supports the following fields to facilitate a
 * `bastion_password` - The password we should use for the bastion host.
   Defaults to the value of `password`.
 
-* `bastion_key_file` - The SSH key to use for the bastion host. Defaults to the
+* `bastion_private_key` - The contents of an SSH key file to use for the bastion
-  value of `key_file`.
+  host. These can be loaded from a file on disk using the [`file()`
+  interpolation function](/docs/configuration/interpolation.html#file_path_).
+  Defaults to the value of `private_key`.
+
+## Deprecations
+
+These are supported for backwards compatibility and may be removed in a
+future version:
+
+* `key_file` - A path to or the contents of an SSH key to use for the
+  connection. These can be loaded from a file on disk using the [`file()`
+  interpolation function](/docs/configuration/interpolation.html#file_path_).
+  This takes preference over the password if provided.
+
+* `bastion_key_file` - The contents of an SSH key file to use for the bastion
+  host. These can be loaded from a file on disk using the [`file()`
+  interpolation function](/docs/configuration/interpolation.html#file_path_).
+  Defaults to the value of `key_file`.