From 801aaf1eec531fb744c91fac9abcec99045bf5e6 Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Thu, 2 Jul 2015 15:41:23 -0500 Subject: [PATCH] communicator/ssh: sort agent after static keyfile In the SSH client configuration, we had SSH Agent authentication listed before the static PrivateKey loaded from the `key_file` setting. Switching the default of the `agent` setting exposed the fact that the SSH agent overrides the `key_file` during the handshake. By listing the `key_file` first, we catch the provided key before any query goes out to the agent. Adds a key-based authentication SSH test to cover this new behavior. It fails without the reordering on any machine with an SSH agent running. Fixes #2614 --- communicator/ssh/communicator_test.go | 105 ++++++++++++++++++++++++-- communicator/ssh/provisioner.go | 8 +- 2 files changed, 103 insertions(+), 10 deletions(-) diff --git a/communicator/ssh/communicator_test.go b/communicator/ssh/communicator_test.go index 24571f0af53e..28755b6dbe3f 100644 --- a/communicator/ssh/communicator_test.go +++ b/communicator/ssh/communicator_test.go @@ -5,7 +5,9 @@ package ssh import ( "bytes" "fmt" + "io/ioutil" "net" + "os" "regexp" "strings" "testing" @@ -45,12 +47,8 @@ NsZoFj52ponUM6+99A2CmezFCN16c4mbA//luWF+k3VVqR6BpkrhKw== -----END RSA PRIVATE KEY-----` var serverConfig = &ssh.ServerConfig{ - PasswordCallback: func(c ssh.ConnMetadata, pass []byte) (*ssh.Permissions, error) { - if c.User() == "user" && string(pass) == "pass" { - return nil, nil - } - return nil, fmt.Errorf("password rejected for %q", c.User()) - }, + PasswordCallback: acceptUserPass("user", "pass"), + PublicKeyCallback: acceptPublicKey(testClientPublicKey), } func init() { @@ -169,6 +167,48 @@ func TestStart(t *testing.T) { } } +func TestStart_KeyFile(t *testing.T) { + address := newMockLineServer(t) + parts := strings.Split(address, ":") + + keyFile, err := ioutil.TempFile("", "tf") + if err != nil { + t.Fatalf("err: %s", err) + } + keyFilePath := keyFile.Name() + keyFile.Write([]byte(testClientPrivateKey)) + keyFile.Close() + defer os.Remove(keyFilePath) + + r := &terraform.InstanceState{ + Ephemeral: terraform.EphemeralState{ + ConnInfo: map[string]string{ + "type": "ssh", + "user": "user", + "key_file": keyFilePath, + "host": parts[0], + "port": parts[1], + "timeout": "30s", + }, + }, + } + + c, err := New(r) + if err != nil { + t.Fatalf("error creating communicator: %s", err) + } + + var cmd remote.Cmd + stdout := new(bytes.Buffer) + cmd.Command = "echo foo" + cmd.Stdout = stdout + + err = c.Start(&cmd) + if err != nil { + t.Fatalf("error executing remote command: %s", err) + } +} + func TestScriptPath(t *testing.T) { cases := []struct { Input string @@ -197,3 +237,56 @@ func TestScriptPath(t *testing.T) { } } } + +const testClientPrivateKey = `-----BEGIN RSA PRIVATE KEY----- +MIIEpQIBAAKCAQEAxOgNXOJ/jrRDxBZTSk2X9otNy9zcpUmJr5ifDi5sy7j2ZiQS +beBt1Wf+tLNWis8Cyq06ttEvjjRuM75yucyD6GrqDTXVCSm4PeOIQeDhPhw26wYZ +O0h/mFgrAiCwaEl8AFXVBInOhVn/0nqgUpkwckh76bjTsNeifkiugK3cfJOuBdrU +ZGbgugJjmYo4Cmv7ECo1gCFT5N+BAjOji3z3N5ClBH5HaWC77jH7kTH0k5cZ+ZRQ +tG9EqLyvWlnzTAR/Yly0JInkOa16Ms1Au5sIJwEoJfHKsNVK06IlLob53nblwYW0 +H5gv1Kb/rS+nUkpPtA5YFShB7iZnPLPPv6qXSwIDAQABAoIBAC0UY1rMkB9/rbQK +2G6+bPgI1HrDydAdkeQdsOxyPH43jlG8GGwHYZ3l/S4pkLqewijcmACay6Rm5IP8 +Kg/XfquLLqJvnKJIZuHkYaGTdn3dv8T21Hf6FRwvs0j9auW1TSpWfDpZwmpNPIBX +irTeVXUUmynbIrvt4km/IhRbuYrbbb964CLYD1DCl3XssXxoRNvPpc5EtOuyDorA +5g1hvZR1FqbOAmOuNQMYJociMuWB8mCaHb+o1Sg4A65OLXxoKs0cuwInJ/n/R4Z3 ++GrV+x5ypBMxXgjjQtKMLEOujkvxs1cp34hkbhKMHHXxbMu5jl74YtGGsLLk90rq +ieZGIgECgYEA49OM9mMCrDoFUTZdJaSARA/MOXkdQgrqVTv9kUHee7oeMZZ6lS0i +bPU7g+Bq+UAN0qcw9x992eAElKjBA71Q5UbZYWh29BDMZd8bRJmwz4P6aSMoYLWI +Sr31caJU9LdmPFatarNeehjSJtlTuoZD9+NElnnUwNaTeOOo5UdhTQsCgYEA3UGm +QWoDUttFwK9oL2KL8M54Bx6EzNhnyk03WrqBbR7PJcPKnsF0R/0soQ+y0FW0r8RJ +TqG6ze5fUJII72B4GlMTQdP+BIvaKQttwWQTNIjbbv4NksF445gdVOO1xi9SvQ7k +uvMVxOb+1jL3HAFa3furWu2tJRDs6dhuaILLxsECgYEAhnhlKUBDYZhVbxvhWsh/ +lKymY/3ikQqUSX7BKa1xPiIalDY3YDllql4MpMgfG8L85asdMZ96ztB0o7H/Ss/B +IbLxt5bLLz+DBVXsaE82lyVU9h10RbCgI01/w3SHJHHjfBXFAcehKfvgfmGkE+IP +2A5ie1aphrCgFqh5FetNuQUCgYEAibL42I804FUtFR1VduAa/dRRqQSaW6528dWa +lLGsKRBalUNEEAeP6dmr89UEUVp1qEo94V0QGGe5FDi+rNPaC3AWdQqNdaDgNlkx +hoFU3oYqIuqj4ejc5rBd2N4a2+vJz3W8bokozDGC+iYf2mMRfUPKwj1XW9Er0OFs +3UhBsEECgYEAto/iJB7ZlCM7EyV9JW0tsEt83rbKMQ/Ex0ShbBIejej0Xx7bwx60 +tVgay+bzJnNkXu6J4XVI98A/WsdI2kW4hL0STYdHV5HVA1l87V4ZbvTF2Bx8a8RJ +OF3UjpMTWKqOprw9nAu5VuwNRVzORF8ER8rgGeaR2/gsSvIYFy9VXq8= +-----END RSA PRIVATE KEY-----` + +var testClientPublicKey = `ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDE6A1c4n+OtEPEFlNKTZf2i03L3NylSYmvmJ8OLmzLuPZmJBJt4G3VZ/60s1aKzwLKrTq20S+ONG4zvnK5zIPoauoNNdUJKbg944hB4OE+HDbrBhk7SH+YWCsCILBoSXwAVdUEic6FWf/SeqBSmTBySHvpuNOw16J+SK6Ardx8k64F2tRkZuC6AmOZijgKa/sQKjWAIVPk34ECM6OLfPc3kKUEfkdpYLvuMfuRMfSTlxn5lFC0b0SovK9aWfNMBH9iXLQkieQ5rXoyzUC7mwgnASgl8cqw1UrToiUuhvneduXBhbQfmC/Upv+tL6dSSk+0DlgVKEHuJmc8s8+/qpdL` + +func acceptUserPass(goodUser, goodPass string) func(ssh.ConnMetadata, []byte) (*ssh.Permissions, error) { + return func(c ssh.ConnMetadata, pass []byte) (*ssh.Permissions, error) { + if c.User() == goodUser && string(pass) == goodPass { + return nil, nil + } + return nil, fmt.Errorf("password rejected for %q", c.User()) + } +} + +func acceptPublicKey(keystr string) func(ssh.ConnMetadata, ssh.PublicKey) (*ssh.Permissions, error) { + goodkey, _, _, _, err := ssh.ParseAuthorizedKey([]byte(keystr)) + if err != nil { + panic(fmt.Errorf("error parsing key: %s", err)) + } + return func(_ ssh.ConnMetadata, inkey ssh.PublicKey) (*ssh.Permissions, error) { + if bytes.Compare(inkey.Marshal(), goodkey.Marshal()) == 0 { + return nil, nil + } + + return nil, fmt.Errorf("public key rejected") + } +} diff --git a/communicator/ssh/provisioner.go b/communicator/ssh/provisioner.go index cda7fc2f3f67..813db57283ed 100644 --- a/communicator/ssh/provisioner.go +++ b/communicator/ssh/provisioner.go @@ -180,10 +180,6 @@ func buildSSHClientConfig(opts sshClientConfigOpts) (*ssh.ClientConfig, error) { User: opts.user, } - if opts.sshAgent != nil { - conf.Auth = append(conf.Auth, opts.sshAgent.Auth()) - } - if opts.keyFile != "" { pubKeyAuth, err := readPublicKeyFromPath(opts.keyFile) if err != nil { @@ -198,6 +194,10 @@ func buildSSHClientConfig(opts sshClientConfigOpts) (*ssh.ClientConfig, error) { PasswordKeyboardInteractive(opts.password))) } + if opts.sshAgent != nil { + conf.Auth = append(conf.Auth, opts.sshAgent.Auth()) + } + return conf, nil }