validate: improve ssh public key validation

Use ParseAuthorizedKey from golang.org/x/crypto/ssh to validate that the provided ssh key can be parsed. https://jira.coreos.com/browse/CORS-850
openshift · Dec 15, 2018 · d2c528f · d2c528f
1 parent 1df7704
commit d2c528f
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 54 deletions.
diff --git a/pkg/validate/validate.go b/pkg/validate/validate.go
@@ -11,6 +11,7 @@ import (
 	"strings"
 	"unicode/utf8"
 
+	"golang.org/x/crypto/ssh"
 	k8serrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/validation"
 )
@@ -213,32 +214,9 @@ func lastIP(cidr *net.IPNet) net.IP {
 	return last
 }
 
-// SSHPublicKey checks if the given string is a valid OpenSSH public key
+// SSHPublicKey checks if the given string is a valid SSH public key
 // and returns an error if not.
 func SSHPublicKey(v string) error {
-	trimmed := strings.TrimSpace(v)
-
-	// Don't let users hang themselves
-	if isMatch(`-BEGIN [\w-]+ PRIVATE KEY-`, trimmed) {
-		return errors.New("invalid SSH public key (appears to be a private key)")
-	}
-
-	if strings.Contains(trimmed, "\n") {
-		return errors.New("invalid SSH public key (should not contain any newline characters)")
-	}
-
-	invalidError := errors.New("invalid SSH public key")
-
-	keyParts := regexp.MustCompile(`\s+`).Split(trimmed, -1)
-	if len(keyParts) < 2 {
-		return invalidError
-	}
-
-	keyType := keyParts[0]
-	keyBase64 := keyParts[1]
-	if !isMatch(`^[\w-]+$`, keyType) || !isMatch(`^[A-Za-z0-9+\/]+={0,2}$`, keyBase64) {
-		return invalidError
-	}
-
-	return nil
+	_, _, _, _, err := ssh.ParseAuthorizedKey([]byte(v))
+	return err
 }
diff --git a/pkg/validate/validate_test.go b/pkg/validate/validate_test.go
@@ -346,35 +346,41 @@ func TestImagePullSecret(t *testing.T) {
 	}
 }
 
-func TestOpenSSHPublicKey(t *testing.T) {
-	const invalidMsg = "invalid SSH public key"
-	const multiLineMsg = "invalid SSH public key (should not contain any newline characters)"
-	const privateKeyMsg = "invalid SSH public key (appears to be a private key)"
-	tests := []struct {
-		in       string
-		expected string
+func TestSSHPublicKey(t *testing.T) {
+	cases := []struct {
+		name  string
+		key   string
+		valid bool
 	}{
-		{"a", invalidMsg},
-		{".", invalidMsg},
-		{"日本語", invalidMsg},
-		{"ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQDxL", ""},
-		{"ssh-rsa \t AAAAB3NzaC1yc2EAAAADAQABAAACAQDxL", ""},
-		{"ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQDxL you@example.com", ""},
-		{"\nssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQDxL you@example.com", ""},
-		{"ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQDxL you@example.com\n", ""},
-		{"ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQDxL\nssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQDxL", multiLineMsg},
-		{"ssh-rsa\nAAAAB3NzaC1yc2EAAAADAQABAAACAQDxL you@example.com", multiLineMsg},
-		{"ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQDxL\nyou@example.com", multiLineMsg},
-		{"ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQDxL", ""},
-		{"ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCt3BebCHqnSsgpLjo4kVvyfY/z2BS8t27r/7du+O2pb4xYkr7n+KFpbOz523vMTpQ+o1jY4u4TgexglyT9nqasWgLOvo1qjD1agHme8LlTPQSk07rXqOB85Uq5p7ig2zoOejF6qXhcc3n1c7+HkxHrgpBENjLVHOBpzPBIAHkAGaZcl07OCqbsG5yxqEmSGiAlh/IiUVOZgdDMaGjCRFy0wk0mQaGD66DmnFc1H5CzcPjsxr0qO65e7lTGsE930KkO1Vc+RHCVwvhdXs+c2NhJ2/3740Kpes9n1/YullaWZUzlCPDXtRuy6JRbFbvy39JUgHWGWzB3d+3f8oJ/N4qZ cardno:000603633110", ""},
-		{"-----BEGIN CERTIFICATE-----abcd-----END CERTIFICATE-----", invalidMsg},
-		{"-----BEGIN RSA PRIVATE KEY-----\nabc\n-----END RSA PRIVATE KEY-----", privateKeyMsg},
+		{
+			name:  "valid",
+			key:   "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAklOUpkDHrfHY17SbrmTIpNLTGK9Tjom/BWDSUGPl+nafzlHDTYW7hdI4yZ5ew18JH4JW9jbhUFrviQzM7xlELEVf4h9lFX5QVkbPppSwg0cda3Pbv7kOdJ/MTyBlWXFCR+HAo3FXRitBqxiX1nKhXpHAZsMciLq8V6RjsNAQwdsdMFvSlVK/7XAt3FaoJoAsncM1Q9x5+3V0Ww68/eIFmb1zuUFljQJKprrX88XypNDvjYNby6vw/Pb0rwert/EnmZ+AW4OZPnTPI89ZPmVMLuayrD2cE86Z/il8b+gw3r3+1nKatmIkjn2so1d01QraTlMqVSsbxNrRFi9wrf+M7Q==",
+			valid: true,
+		},
+		{
+			name:  "valid with email",
+			key:   "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAklOUpkDHrfHY17SbrmTIpNLTGK9Tjom/BWDSUGPl+nafzlHDTYW7hdI4yZ5ew18JH4JW9jbhUFrviQzM7xlELEVf4h9lFX5QVkbPppSwg0cda3Pbv7kOdJ/MTyBlWXFCR+HAo3FXRitBqxiX1nKhXpHAZsMciLq8V6RjsNAQwdsdMFvSlVK/7XAt3FaoJoAsncM1Q9x5+3V0Ww68/eIFmb1zuUFljQJKprrX88XypNDvjYNby6vw/Pb0rwert/EnmZ+AW4OZPnTPI89ZPmVMLuayrD2cE86Z/il8b+gw3r3+1nKatmIkjn2so1d01QraTlMqVSsbxNrRFi9wrf+M7Q== name@example.com",
+			valid: true,
+		},
+		{
+			name:  "invalid format",
+			key:   "bad-format AAAAB3NzaC1yc2EAAAABIwAAAQEAklOUpkDHrfHY17SbrmTIpNLTGK9Tjom/BWDSUGPl+nafzlHDTYW7hdI4yZ5ew18JH4JW9jbhUFrviQzM7xlELEVf4h9lFX5QVkbPppSwg0cda3Pbv7kOdJ/MTyBlWXFCR+HAo3FXRitBqxiX1nKhXpHAZsMciLq8V6RjsNAQwdsdMFvSlVK/7XAt3FaoJoAsncM1Q9x5+3V0Ww68/eIFmb1zuUFljQJKprrX88XypNDvjYNby6vw/Pb0rwert/EnmZ+AW4OZPnTPI89ZPmVMLuayrD2cE86Z/il8b+gw3r3+1nKatmIkjn2so1d01QraTlMqVSsbxNrRFi9wrf+M7Q==",
+			valid: true,
+		},
+		{
+			name:  "invalid key",
+			key:   "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQDxL",
+			valid: false,
+		},
 	}
-
-	for _, test := range tests {
-		err := SSHPublicKey(test.in)
-		if (err == nil && test.expected != "") || (err != nil && err.Error() != test.expected) {
-			t.Errorf("For %q, expected %q, got %q", test.in, test.expected, err)
-		}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := SSHPublicKey(tc.key)
+			if tc.valid {
+				assert.NoError(t, err)
+			} else {
+				assert.Error(t, err)
+			}
+		})
 	}
 }