Merge pull request #3296 from mmalchuk/master

Correct error handling for external SSH client

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)
 		}
 	}

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)
+		}
+	}
+}