Skip to content

Commit

Permalink
ssh: accept private key contents instead of path
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
phinze committed Nov 12, 2015
1 parent 4252a3e commit 7ffa66d
Show file tree
Hide file tree
Showing 6 changed files with 281 additions and 50 deletions.
4 changes: 2 additions & 2 deletions communicator/ssh/communicator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
))

Expand All @@ -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,
))
}
Expand Down
78 changes: 43 additions & 35 deletions communicator/ssh/provisioner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -37,19 +36,23 @@ const (
type connectionInfo struct {
User string
Password string
KeyFile string `mapstructure:"key_file"`
PrivateKey string `mapstructure:"private_key"`
Host string
Port int
Agent bool
Timeout string
ScriptPath string `mapstructure:"script_path"`
TimeoutVal time.Duration `mapstructure:"-"`

BastionUser string `mapstructure:"bastion_user"`
BastionPassword string `mapstructure:"bastion_password"`
BastionKeyFile string `mapstructure:"bastion_key_file"`
BastionHost string `mapstructure:"bastion_host"`
BastionPort int `mapstructure:"bastion_port"`
BastionUser string `mapstructure:"bastion_user"`
BastionPassword string `mapstructure:"bastion_password"`
BastionPrivateKey string `mapstructure:"bastion_private_key"`
BastionHost string `mapstructure:"bastion_host"`
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
Expand Down Expand Up @@ -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 == "" {
Expand All @@ -100,8 +112,8 @@ func parseConnectionInfo(s *terraform.InstanceState) (*connectionInfo, error) {
if connInfo.BastionPassword == "" {
connInfo.BastionPassword = connInfo.Password
}
if connInfo.BastionKeyFile == "" {
connInfo.BastionKeyFile = connInfo.KeyFile
if connInfo.BastionPrivateKey == "" {
connInfo.BastionPrivateKey = connInfo.PrivateKey
}
if connInfo.BastionPort == 0 {
connInfo.BastionPort = connInfo.Port
Expand Down Expand Up @@ -130,10 +142,10 @@ func prepareSSHConfig(connInfo *connectionInfo) (*sshConfig, error) {
}

sshConf, err := buildSSHClientConfig(sshClientConfigOpts{
user: connInfo.User,
keyFile: connInfo.KeyFile,
password: connInfo.Password,
sshAgent: sshAgent,
user: connInfo.User,
privateKey: connInfo.PrivateKey,
password: connInfo.Password,
sshAgent: sshAgent,
})
if err != nil {
return nil, err
Expand All @@ -142,10 +154,10 @@ func prepareSSHConfig(connInfo *connectionInfo) (*sshConfig, error) {
var bastionConf *ssh.ClientConfig
if connInfo.BastionHost != "" {
bastionConf, err = buildSSHClientConfig(sshClientConfigOpts{
user: connInfo.BastionUser,
keyFile: connInfo.BastionKeyFile,
password: connInfo.BastionPassword,
sshAgent: sshAgent,
user: connInfo.BastionUser,
privateKey: connInfo.BastionPrivateKey,
password: connInfo.BastionPassword,
sshAgent: sshAgent,
})
if err != nil {
return nil, err
Expand All @@ -169,10 +181,10 @@ func prepareSSHConfig(connInfo *connectionInfo) (*sshConfig, error) {
}

type sshClientConfigOpts struct {
keyFile string
password string
sshAgent *sshAgent
user string
privateKey string
password string
sshAgent *sshAgent
user string
}

func buildSSHClientConfig(opts sshClientConfigOpts) (*ssh.ClientConfig, error) {
Expand All @@ -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
}
Expand All @@ -201,31 +213,27 @@ func buildSSHClientConfig(opts sshClientConfigOpts) (*ssh.ClientConfig, error) {
return conf, nil
}

func readPublicKeyFromPath(path string) (ssh.AuthMethod, error) {
fullPath, err := homedir.Expand(path)
if err != nil {
return nil, fmt.Errorf("Failed to expand home directory: %s", err)
}
key, err := ioutil.ReadFile(fullPath)
func readPrivateKey(pk string) (ssh.AuthMethod, error) {
key, _, err := pathorcontents.Read(pk)
if err != nil {
return nil, fmt.Errorf("Failed to read key file %q: %s", path, err)
return nil, fmt.Errorf("Failed to read private key %q: %s", pk, 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
Expand Down
42 changes: 33 additions & 9 deletions communicator/ssh/provisioner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ func TestProvisioner_connInfo(t *testing.T) {
r := &terraform.InstanceState{
Ephemeral: terraform.EphemeralState{
ConnInfo: map[string]string{
"type": "ssh",
"user": "root",
"password": "supersecret",
"key_file": "/my/key/file.pem",
"host": "127.0.0.1",
"port": "22",
"timeout": "30s",
"type": "ssh",
"user": "root",
"password": "supersecret",
"private_key": "someprivatekeycontents",
"host": "127.0.0.1",
"port": "22",
"timeout": "30s",

"bastion_host": "127.0.1.1",
},
Expand All @@ -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" {
Expand All @@ -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)
}
}
40 changes: 40 additions & 0 deletions helper/pathorcontents/read.go
Original file line number Diff line number Diff line change
@@ -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
}
Loading

0 comments on commit 7ffa66d

Please sign in to comment.