Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve SSH key parser to handle newlines in keys #7522

Merged
merged 7 commits into from
Jul 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions models/ssh_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,17 +96,18 @@ func extractTypeFromBase64Key(key string) (string, error) {

// parseKeyString parses any key string in OpenSSH or SSH2 format to clean OpenSSH string (RFC4253).
func parseKeyString(content string) (string, error) {
// Transform all legal line endings to a single "\n".
content = strings.NewReplacer("\r\n", "\n", "\r", "\n").Replace(content)
// remove trailing newline (and beginning spaces too)
// remove whitespace at start and end
content = strings.TrimSpace(content)
lines := strings.Split(content, "\n")

var keyType, keyContent, keyComment string

if len(lines) == 1 {
if !strings.Contains(content, "-----BEGIN") {
// Parse OpenSSH format.
parts := strings.SplitN(lines[0], " ", 3)

// Remove all newlines
content = strings.NewReplacer("\r\n", "", "\n", "").Replace(content)

parts := strings.SplitN(content, " ", 3)
switch len(parts) {
case 0:
return "", errors.New("empty key")
Expand All @@ -133,6 +134,11 @@ func parseKeyString(content string) (string, error) {
}
} else {
// Parse SSH2 file format.

// Transform all legal line endings to a single "\n".
content = strings.NewReplacer("\r\n", "\n", "\r", "\n").Replace(content)

lines := strings.Split(content, "\n")
continuationLine := false

for _, line := range lines {
Expand Down
16 changes: 16 additions & 0 deletions models/ssh_key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,22 @@ func Test_SSHParsePublicKey(t *testing.T) {
}
}

func Test_CheckPublicKeyString(t *testing.T) {
silverwind marked this conversation as resolved.
Show resolved Hide resolved
for _, test := range []struct {
content string
}{
{"ssh-dss AAAAB3NzaC1kc3MAAACBAOChCC7lf6Uo9n7BmZ6M8St19PZf4Tn59NriyboW2x/DZuYAz3ibZ2OkQ3S0SqDIa0HXSEJ1zaExQdmbO+Ux/wsytWZmCczWOVsaszBZSl90q8UnWlSH6P+/YA+RWJm5SFtuV9PtGIhyZgoNuz5kBQ7K139wuQsecdKktISwTakzAAAAFQCzKsO2JhNKlL+wwwLGOcLffoAmkwAAAIBpK7/3xvduajLBD/9vASqBQIHrgK2J+wiQnIb/Wzy0UsVmvfn8A+udRbBo+csM8xrSnlnlJnjkJS3qiM5g+eTwsLIV1IdKPEwmwB+VcP53Cw6lSyWyJcvhFb0N6s08NZysLzvj0N+ZC/FnhKTLzIyMtkHf/IrPCwlM+pV/M/96YgAAAIEAqQcGn9CKgzgPaguIZooTAOQdvBLMI5y0bQjOW6734XOpqQGf/Kra90wpoasLKZjSYKNPjE+FRUOrStLrxcNs4BeVKhy2PYTRnybfYVk1/dmKgH6P1YSRONsGKvTsH6c5IyCRG0ncCgYeF8tXppyd642982daopE7zQ/NPAnJfag= nocomment"},
{"ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDAu7tvIvX6ZHrRXuZNfkR3XLHSsuCK9Zn3X58lxBcQzuo5xZgB6vRwwm/QtJuF+zZPtY5hsQILBLmF+BZ5WpKZp1jBeSjH2G7lxet9kbcH+kIVj0tPFEoyKI9wvWqIwC4prx/WVk2wLTJjzBAhyNxfEq7C9CeiX9pQEbEqJfkKCQ== nocomment\n"},
{"ssh-rsa AAAAB3NzaC1yc2EA\r\nAAADAQABAAAAgQDAu7tvIvX6ZHrRXuZNfkR3XLHSsuCK9Zn3X58lxBcQzuo5xZgB6vRwwm/QtJuF+zZPtY5hsQILBLmF+\r\nBZ5WpKZp1jBeSjH2G7lxet9kbcH+kIVj0tPFEoyKI9wvWqIwC4prx/WVk2wLTJjzBAhyNx\r\nfEq7C9CeiX9pQEbEqJfkKCQ== nocomment\r\n\r\n"},
{"ssh-rsa AAAAB3NzaC1yc2EA\r\nAAADAQABAAAAgQDAu7tvI\nvX6ZHrRXuZNfkR3XLHSsuCK9Zn3X58lxBcQzuo5xZgB6vRwwm/QtJuF+zZPtY5hsQILBLmF+\r\nBZ5WpKZp1jBeSjH2G7lxet9kbcH+kIVj0tPFEoyKI9wvW\nqIwC4prx/WVk2wLTJjzBAhyNx\r\nfEq7C9CeiX9pQEbEqJfkKCQ== nocomment\r\n\r\n"},
{"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAICV0MGX/W9IvLA4FXpIuUcdDcbj5KX4syHgsTy7soVgf"},
{"\r\nssh-ed25519 \r\nAAAAC3NzaC1lZDI1NTE5AAAAICV0MGX/W9IvLA4FXpIuUcdDcbj5KX4syHgsTy7soVgf\r\n\r\n"},
} {
_, err := CheckPublicKeyString(test.content)
assert.NoError(t, err)
}
}

func Test_calcFingerprint(t *testing.T) {
testCases := []struct {
name string
Expand Down