Skip to content

Commit b10109f

Browse files
silverwindlafriks
authored andcommitted
Improve SSH key parser to handle newlines in keys (#7522)
* Strip newlines from SSH keys before adding them Fixes: #7500 * add test for CheckPublicKeyString * add one more test * simplify test * further simplify * make fmt
1 parent bcbc9f3 commit b10109f

File tree

2 files changed

+28
-6
lines changed

2 files changed

+28
-6
lines changed

models/ssh_key.go

+12-6
Original file line numberDiff line numberDiff line change
@@ -96,17 +96,18 @@ func extractTypeFromBase64Key(key string) (string, error) {
9696

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

105102
var keyType, keyContent, keyComment string
106103

107-
if len(lines) == 1 {
104+
if !strings.Contains(content, "-----BEGIN") {
108105
// Parse OpenSSH format.
109-
parts := strings.SplitN(lines[0], " ", 3)
106+
107+
// Remove all newlines
108+
content = strings.NewReplacer("\r\n", "", "\n", "").Replace(content)
109+
110+
parts := strings.SplitN(content, " ", 3)
110111
switch len(parts) {
111112
case 0:
112113
return "", errors.New("empty key")
@@ -133,6 +134,11 @@ func parseKeyString(content string) (string, error) {
133134
}
134135
} else {
135136
// Parse SSH2 file format.
137+
138+
// Transform all legal line endings to a single "\n".
139+
content = strings.NewReplacer("\r\n", "\n", "\r", "\n").Replace(content)
140+
141+
lines := strings.Split(content, "\n")
136142
continuationLine := false
137143

138144
for _, line := range lines {

models/ssh_key_test.go

+16
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,22 @@ func Test_SSHParsePublicKey(t *testing.T) {
5656
}
5757
}
5858

59+
func Test_CheckPublicKeyString(t *testing.T) {
60+
for _, test := range []struct {
61+
content string
62+
}{
63+
{"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"},
64+
{"ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDAu7tvIvX6ZHrRXuZNfkR3XLHSsuCK9Zn3X58lxBcQzuo5xZgB6vRwwm/QtJuF+zZPtY5hsQILBLmF+BZ5WpKZp1jBeSjH2G7lxet9kbcH+kIVj0tPFEoyKI9wvWqIwC4prx/WVk2wLTJjzBAhyNxfEq7C9CeiX9pQEbEqJfkKCQ== nocomment\n"},
65+
{"ssh-rsa AAAAB3NzaC1yc2EA\r\nAAADAQABAAAAgQDAu7tvIvX6ZHrRXuZNfkR3XLHSsuCK9Zn3X58lxBcQzuo5xZgB6vRwwm/QtJuF+zZPtY5hsQILBLmF+\r\nBZ5WpKZp1jBeSjH2G7lxet9kbcH+kIVj0tPFEoyKI9wvWqIwC4prx/WVk2wLTJjzBAhyNx\r\nfEq7C9CeiX9pQEbEqJfkKCQ== nocomment\r\n\r\n"},
66+
{"ssh-rsa AAAAB3NzaC1yc2EA\r\nAAADAQABAAAAgQDAu7tvI\nvX6ZHrRXuZNfkR3XLHSsuCK9Zn3X58lxBcQzuo5xZgB6vRwwm/QtJuF+zZPtY5hsQILBLmF+\r\nBZ5WpKZp1jBeSjH2G7lxet9kbcH+kIVj0tPFEoyKI9wvW\nqIwC4prx/WVk2wLTJjzBAhyNx\r\nfEq7C9CeiX9pQEbEqJfkKCQ== nocomment\r\n\r\n"},
67+
{"ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAICV0MGX/W9IvLA4FXpIuUcdDcbj5KX4syHgsTy7soVgf"},
68+
{"\r\nssh-ed25519 \r\nAAAAC3NzaC1lZDI1NTE5AAAAICV0MGX/W9IvLA4FXpIuUcdDcbj5KX4syHgsTy7soVgf\r\n\r\n"},
69+
} {
70+
_, err := CheckPublicKeyString(test.content)
71+
assert.NoError(t, err)
72+
}
73+
}
74+
5975
func Test_calcFingerprint(t *testing.T) {
6076
testCases := []struct {
6177
name string

0 commit comments

Comments
 (0)