From be8f469a828ffd6ad8ca0baaef017c49e3bab713 Mon Sep 17 00:00:00 2001
From: Maksim Malchuk <maksim.malchuk@gmail.com>
Date: Tue, 12 Apr 2016 11:43:18 +0300
Subject: [PATCH] Correct error handling for external SSH client

In some cases, (e.g. private key not accessible or has incorrect
permissions) docker-machine failed with error "Something went wrong
running an SSH command!". This commit will add the correct debug
messages and show the correct errors for the bad private keys.
Also, due to incorrect handling POSIX file permissions in Windows
some checks should be ignored.

Signed-off-by: Maksim Malchuk <maksim.malchuk@gmail.com>
---
 libmachine/ssh/client.go      | 16 ++++++++++++++
 libmachine/ssh/client_test.go | 39 +++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)

diff --git a/libmachine/ssh/client.go b/libmachine/ssh/client.go
index 24963121d0..ad880fb292 100644
--- a/libmachine/ssh/client.go
+++ b/libmachine/ssh/client.go
@@ -6,6 +6,7 @@ import (
 	"io/ioutil"
 	"os"
 	"os/exec"
+	"runtime"
 	"strings"
 
 	"github.com/docker/docker/pkg/term"
@@ -322,6 +323,21 @@ func NewExternalClient(sshBinaryPath, user, host string, port int, auth *Auth) (
 	// Specify which private keys to use to authorize the SSH request.
 	for _, privateKeyPath := range auth.Keys {
 		if privateKeyPath != "" {
+			// Check each private key before use it
+			fi, err := os.Stat(privateKeyPath)
+			if err != nil {
+				// Abort if key not accessible
+				return nil, err
+			}
+			if runtime.GOOS != "windows" {
+				mode := fi.Mode()
+				log.Debugf("Using SSH private key: %s (%s)", privateKeyPath, mode)
+				// Private key file should have strict permissions
+				if mode != 0600 {
+					// Abort with correct message
+					return nil, fmt.Errorf("Permissions %#o for '%s' are too open.", mode, privateKeyPath)
+				}
+			}
 			args = append(args, "-i", privateKeyPath)
 		}
 	}
diff --git a/libmachine/ssh/client_test.go b/libmachine/ssh/client_test.go
index 65d35becd9..2ca3d41d3c 100644
--- a/libmachine/ssh/client_test.go
+++ b/libmachine/ssh/client_test.go
@@ -1,6 +1,7 @@
 package ssh
 
 import (
+	"runtime"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -43,3 +44,41 @@ func TestGetSSHCmdArgs(t *testing.T) {
 		assert.Equal(t, cmd.Args, c.expectedArgs)
 	}
 }
+
+func TestNewExternalClient(t *testing.T) {
+	cases := []struct {
+		sshBinaryPath string
+		user          string
+		host          string
+		port          int
+		auth          *Auth
+		expectedError string
+		skipOS        string
+	}{
+		{
+			sshBinaryPath: "/usr/local/bin/ssh",
+			user:          "docker",
+			host:          "localhost",
+			port:          22,
+			auth:          &Auth{Keys: []string{"/tmp/private-key-not-exist"}},
+			expectedError: "stat /tmp/private-key-not-exist: no such file or directory",
+			skipOS:        "none",
+		},
+		{
+			sshBinaryPath: "/usr/local/bin/ssh",
+			user:          "docker",
+			host:          "localhost",
+			port:          22,
+			auth:          &Auth{Keys: []string{"/dev/null"}},
+			expectedError: "Permissions 0410000666 for '/dev/null' are too open.",
+			skipOS:        "windows",
+		},
+	}
+
+	for _, c := range cases {
+		if runtime.GOOS != c.skipOS {
+			_, err := NewExternalClient(c.sshBinaryPath, c.user, c.host, c.port, c.auth)
+			assert.EqualError(t, err, c.expectedError)
+		}
+	}
+}