From 12403bdfb098d8118df734275c302c8c5de20ee4 Mon Sep 17 00:00:00 2001 From: Gibheer Date: Tue, 16 Feb 2016 23:01:56 +0100 Subject: [PATCH 1/6] allow native and ssh-keygen public key check This commit adds the possibibility to use either the native golang libraries or ssh-keygen to check public keys. The check is adjusted depending on the settings, so that only supported keys are let through. This commit also brings back the blacklist feature, which was removed in 7ef9a055886574655d9f2be70c957bc16bf30500. This allows to blacklist algorythms or keys based on the key length. This works with the native and the ssh-keygen way. Because of #2179 it also includes a way to adjust the path to ssh-keygen and the working directory for ssh-keygen. With this, sysadmins should be able to adjust the settings in a way, that SELinux is okay with it. In the worst case, they can switch to the native implementation and only loose support for ed25519 keys at the moment. There are some other places which need adjustment to utilize the parameters and the native implementation, but this sets the ground work. --- conf/app.ini | 16 ++++ models/ssh_key.go | 145 ++++++++++++++++++++++++++++++++++--- models/ssh_key_test.go | 36 +++++++++ modules/setting/setting.go | 45 +++++++++++- 4 files changed, 227 insertions(+), 15 deletions(-) create mode 100644 models/ssh_key_test.go diff --git a/conf/app.ini b/conf/app.ini index 277f313d52..b444665b42 100644 --- a/conf/app.ini +++ b/conf/app.ini @@ -66,6 +66,13 @@ START_SSH_SERVER = false SSH_PORT = 22 ; Root path of SSH directory SSH_ROOT_PATH = +; override engine choice to check public keys (default: 'ssh-keygen' when +; DISABLE_SSH is set to false else 'native') +SSH_PUBLICKEY_CHECK = +; directory to create temporary files when using ssh-keygen (default: /tmp) +SSH_WORK_PATH = +; path to ssh-keygen (default: result of `which ssh-keygen`) +SSH_KEYGEN_PATH = ; Disable CDN even in "prod" mode OFFLINE_MODE = false DISABLE_ROUTER_LOG = false @@ -127,6 +134,15 @@ ENABLE_REVERSE_PROXY_AUTHENTICATION = false ENABLE_REVERSE_PROXY_AUTO_REGISTRATION = false ; Enable captcha validation for registration ENABLE_CAPTCHA = true +; Do not check minimum key size with corresponding type +ENABLE_MINIMUM_KEY_SIZE_CHECK = false + +; define allowed algorithms and their minimum key length (use -1 to disable a type) +[service.minimum_key_sizes] +ED25519 = 256 +ECDSA = 256 +RSA = 2048 +DSA = 1024 [webhook] ; Hook task queue length diff --git a/models/ssh_key.go b/models/ssh_key.go index 325a40a481..0e5226076b 100644 --- a/models/ssh_key.go +++ b/models/ssh_key.go @@ -12,9 +12,11 @@ import ( "fmt" "io" "io/ioutil" + "math/big" "os" "path" "path/filepath" + "strconv" "strings" "sync" "time" @@ -33,7 +35,10 @@ const ( _TPL_PUBLICK_KEY = `command="%s serv key-%d --config='%s'",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty %s` + "\n" ) -var sshOpLocker = sync.Mutex{} +var ( + sshOpLocker = sync.Mutex{} + SSH_UNKNOWN_KEY_TYPE = fmt.Errorf("unknown key type") +) type KeyType int @@ -153,7 +158,110 @@ func parseKeyString(content string) (string, error) { return keyType + " " + keyContent + " " + keyComment, nil } +// extract key type and length using ssh-keygen +func SSHKeyGenParsePublicKey(key string) (string, int, error) { + // The ssh-keygen in Windows does not print key type, so no need go further. + if setting.IsWindows { + return "", 0, nil + } + + tmpFile, err := ioutil.TempFile(setting.SSHWorkPath, "gogs_keytest") + if err != nil { + return "", 0, err + } + tmpName := tmpFile.Name() + defer os.Remove(tmpName) + + if ln, err := tmpFile.WriteString(key); err != nil { + tmpFile.Close() + return "", 0, err + } else if ln != len(key) { + tmpFile.Close() + return "", 0, fmt.Errorf("could not write complete public key (written: %d, should be: %d): %s", ln, len(key), key) + } + tmpFile.Close() + + stdout, stderr, err := process.Exec("CheckPublicKeyString", setting.SSHKeyGenPath, "-lf", tmpName) + if err != nil { + return "", 0, fmt.Errorf("public key check failed with error '%s': %s", err, stderr) + } + if strings.HasSuffix(stdout, "is not a public key file.") { + return "", 0, SSH_UNKNOWN_KEY_TYPE + } + fields := strings.Split(stdout, " ") + if len(fields) < 4 { + return "", 0, fmt.Errorf("invalid public key line: %s", stdout) + } + + length, err := strconv.Atoi(fields[0]) + if err != nil { + return "", 0, err + } + keyType := strings.Trim(fields[len(fields)-1], "()\r\n") + return strings.ToLower(keyType), length, nil +} + +// extract the key type and length using the golang ssh library +func SSHNativeParsePublicKey(keyLine string) (string, int, error) { + fields := strings.Fields(keyLine) + if len(fields) < 2 { + return "", 0, fmt.Errorf("not enough fields in public key line: %s", string(keyLine)) + } + + raw, err := base64.StdEncoding.DecodeString(fields[1]) + if err != nil { + return "", 0, err + } + + pkey, err := ssh.ParsePublicKey(raw) + if err != nil { + if strings.HasPrefix(err.Error(), "ssh: unknown key algorithm") { + return "", 0, SSH_UNKNOWN_KEY_TYPE + } + return "", 0, err + } + + // The ssh library can parse the key, so next we find out what key exactly we + // have. + switch pkey.Type() { + case ssh.KeyAlgoDSA: + rawPub := struct { + Name string + P, Q, G, Y *big.Int + }{} + if err := ssh.Unmarshal(pkey.Marshal(), &rawPub); err != nil { + return "", 0, err + } + // as per https://bugzilla.mindrot.org/show_bug.cgi?id=1647 we should never + // see dsa keys != 1024 bit, but as it seems to work, we will not check here + return "dsa", rawPub.P.BitLen(), nil // use P as per crypto/dsa/dsa.go (is L) + case ssh.KeyAlgoRSA: + rawPub := struct { + Name string + E *big.Int + N *big.Int + }{} + if err := ssh.Unmarshal(pkey.Marshal(), &rawPub); err != nil { + return "", 0, err + } + return "rsa", rawPub.N.BitLen(), nil // use N as per crypto/rsa/rsa.go (is bits) + case ssh.KeyAlgoECDSA256: + return "ecdsa", 256, nil + case ssh.KeyAlgoECDSA384: + return "ecdsa", 384, nil + case ssh.KeyAlgoECDSA521: + return "ecdsa", 521, nil + case "ssh-ed25519": // TODO replace with ssh constant when available + return "ed25519", 256, nil + default: + return "", 0, fmt.Errorf("no support for key length detection for type %s", pkey.Type()) + } + return "", 0, fmt.Errorf("SSHNativeParsePublicKey failed horribly, please investigate why") +} + // CheckPublicKeyString checks if the given public key string is recognized by SSH. +// +// The function returns the actual public key line on success. func CheckPublicKeyString(content string) (_ string, err error) { content, err = parseKeyString(content) if err != nil { @@ -168,22 +276,34 @@ func CheckPublicKeyString(content string) (_ string, err error) { // remove any unnecessary whitespace now content = strings.TrimSpace(content) - fields := strings.Fields(content) - if len(fields) < 2 { - return "", errors.New("too less fields") + var ( + keyType string + length int + ) + if setting.SSHPublicKeyCheck == setting.SSH_PUBLICKEY_CHECK_NATIVE { + keyType, length, err = SSHNativeParsePublicKey(content) + } else if setting.SSHPublicKeyCheck == setting.SSH_PUBLICKEY_CHECK_KEYGEN { + keyType, length, err = SSHKeyGenParsePublicKey(content) + } else { + log.Error(4, "invalid public key check type: %s", setting.SSHPublicKeyCheck) + return "", fmt.Errorf("invalid public key check type") } - key, err := base64.StdEncoding.DecodeString(fields[1]) - if err != nil { - return "", fmt.Errorf("StdEncoding.DecodeString: %v", err) - } - pkey, err := ssh.ParsePublicKey([]byte(key)) if err != nil { + log.Trace("invalid public key of type '%s' with length %d: %s", keyType, length, err) return "", fmt.Errorf("ParsePublicKey: %v", err) } - log.Trace("Key type: %s", pkey.Type()) + log.Trace("Key type: %s", keyType) - return content, nil + if !setting.Service.EnableMinimumKeySizeCheck { + return content, nil + } + if minLen, found := setting.Service.MinimumKeySizes[keyType]; found && length >= minLen { + return content, nil + } else if found && length < minLen { + return "", fmt.Errorf("key not large enough - got %d, needs %d", length, minLen) + } + return "", fmt.Errorf("key type '%s' is not allowed", keyType) } // saveAuthorizedKeyFile writes SSH key content to authorized_keys file. @@ -247,7 +367,7 @@ func addKey(e Engine, key *PublicKey) (err error) { } stdout, stderr, err := process.Exec("AddPublicKey", "ssh-keygen", "-lf", tmpPath) if err != nil { - return errors.New("ssh-keygen -lf: " + stderr) + return fmt.Errorf("'ssh-keygen -lf %s' failed with error '%s': %s", tmpPath, err, stderr) } else if len(stdout) < 2 { return errors.New("not enough output for calculating fingerprint: " + stdout) } @@ -267,6 +387,7 @@ func addKey(e Engine, key *PublicKey) (err error) { // AddPublicKey adds new public key to database and authorized_keys file. func AddPublicKey(ownerID int64, name, content string) (*PublicKey, error) { + log.Trace(content) if err := checkKeyContent(content); err != nil { return nil, err } diff --git a/models/ssh_key_test.go b/models/ssh_key_test.go new file mode 100644 index 0000000000..0dbf16ecb0 --- /dev/null +++ b/models/ssh_key_test.go @@ -0,0 +1,36 @@ +package models + +import ( + "testing" +) + +func TestSSHKeyVerification(t *testing.T) { + keys := map[string][]byte{ + "dsa-1024": []byte("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"), + "rsa-1024": []byte("ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDAu7tvIvX6ZHrRXuZNfkR3XLHSsuCK9Zn3X58lxBcQzuo5xZgB6vRwwm/QtJuF+zZPtY5hsQILBLmF+BZ5WpKZp1jBeSjH2G7lxet9kbcH+kIVj0tPFEoyKI9wvWqIwC4prx/WVk2wLTJjzBAhyNxfEq7C9CeiX9pQEbEqJfkKCQ== nocomment\n"), + "rsa-2048": []byte("ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDMZXh+1OBUwSH9D45wTaxErQIN9IoC9xl7MKJkqvTvv6O5RR9YW/IK9FbfjXgXsppYGhsCZo1hFOOsXHMnfOORqu/xMDx4yPuyvKpw4LePEcg4TDipaDFuxbWOqc/BUZRZcXu41QAWfDLrInwsltWZHSeG7hjhpacl4FrVv9V1pS6Oc5Q1NxxEzTzuNLS/8diZrTm/YAQQ/+B+mzWI3zEtF4miZjjAljWd1LTBPvU23d29DcBmmFahcZ441XZsTeAwGxG/Q6j8NgNXj9WxMeWwxXV2jeAX/EBSpZrCVlCQ1yJswT6xCp8TuBnTiGWYMBNTbOZvPC4e0WI2/yZW/s5F nocomment"), + "ecdsa-256": []byte("ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBFQacN3PrOll7PXmN5B/ZNVahiUIqI05nbBlZk1KXsO3d06ktAWqbNflv2vEmA38bTFTfJ2sbn2B5ksT52cDDbA= nocomment"), + "ecdsa-384": []byte("ecdsa-sha2-nistp384 AAAAE2VjZHNhLXNoYTItbmlzdHAzODQAAAAIbmlzdHAzODQAAABhBINmioV+XRX1Fm9Qk2ehHXJ2tfVxW30ypUWZw670Zyq5GQfBAH6xjygRsJ5wWsHXBsGYgFUXIHvMKVAG1tpw7s6ax9oA+dJOJ7tj+vhn8joFqT+sg3LYHgZkHrfqryRasQ== nocomment"), + "ecdsa-512": []byte("ecdsa-sha2-nistp521 AAAAE2VjZHNhLXNoYTItbmlzdHA1MjEAAAAIbmlzdHA1MjEAAACFBACGt3UG3EzRwNOI17QR84l6PgiAcvCE7v6aXPj/SC6UWKg4EL8vW9ZBcdYL9wzs4FZXh4MOV8jAzu3KRWNTwb4k2wFNUpGOt7l28MztFFEtH5BDDrtAJSPENPy8pvPLMfnPg5NhvWycqIBzNcHipem5wSJFN5PdpNOC2xMrPWKNqj+ZjQ== nocomment"), + "ed25519-256": []byte("ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIC+8wwPU2VCo6pA+s9eOSqtDdFYA83/w+fpuSJLHTahU nocomment"), + } + + for name, pubkey := range keys { + keyTypeN, lengthN, errN := SSHNativeParsePublicKey(pubkey) + if errN != nil { + if errN != SSH_UNKNOWN_KEY_TYPE { + t.Errorf("error parsing public key '%s': %s", name, errN) + continue + } + } + keyTypeK, lengthK, errK := SSHKeyGenParsePublicKey(pubkey) + if errK != nil { + t.Errorf("error parsing public key '%s': %s", name, errK) + continue + } + // we know that ed25519 is currently not supported by native and returns SSH_UNKNOWN_KEY_TYPE + if (keyTypeN != keyTypeK || lengthN != lengthK) && errN != SSH_UNKNOWN_KEY_TYPE { + t.Errorf("key mismatch for '%s': native: %s(%d), ssh-keygen: %s(%d)", name, keyTypeN, lengthN, keyTypeK, lengthK) + } + } +} diff --git a/modules/setting/setting.go b/modules/setting/setting.go index d82f16dbc2..e667ee7433 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -30,9 +30,11 @@ import ( type Scheme string const ( - HTTP Scheme = "http" - HTTPS Scheme = "https" - FCGI Scheme = "fcgi" + HTTP Scheme = "http" + HTTPS Scheme = "https" + FCGI Scheme = "fcgi" + SSH_PUBLICKEY_CHECK_NATIVE = "native" + SSH_PUBLICKEY_CHECK_KEYGEN = "ssh-keygen" ) type LandingPage string @@ -66,6 +68,9 @@ var ( SSHDomain string SSHPort int SSHRootPath string + SSHPublicKeyCheck string + SSHWorkPath string + SSHKeyGenPath string OfflineMode bool DisableRouterLog bool CertFile, KeyFile string @@ -328,6 +333,29 @@ func NewContext() { if err := os.MkdirAll(SSHRootPath, 0700); err != nil { log.Fatal(4, "Fail to create '%s': %v", SSHRootPath, err) } + checkDefault := SSH_PUBLICKEY_CHECK_KEYGEN + if DisableSSH { + checkDefault = SSH_PUBLICKEY_CHECK_NATIVE + } + SSHPublicKeyCheck = sec.Key("SSH_PUBLICKEY_CHECK").MustString(checkDefault) + if SSHPublicKeyCheck != SSH_PUBLICKEY_CHECK_NATIVE && + SSHPublicKeyCheck != SSH_PUBLICKEY_CHECK_KEYGEN { + log.Fatal(4, "SSH_PUBLICKEY_CHECK must be ssh-keygen or native") + } + SSHWorkPath = sec.Key("SSH_WORK_PATH").MustString(os.TempDir()) + if !DisableSSH && (!StartSSHServer || SSHPublicKeyCheck == SSH_PUBLICKEY_CHECK_KEYGEN) { + if tmpDirStat, err := os.Stat(SSHWorkPath); err != nil || !tmpDirStat.IsDir() { + log.Fatal(4, "directory '%s' set in SSHWorkPath is not a directory: %s", SSHWorkPath, err) + } + } + SSHKeyGenPath = sec.Key("SSH_KEYGEN_PATH").MustString("") + if !DisableSSH && !StartSSHServer && + SSHKeyGenPath == "" && SSHPublicKeyCheck == SSH_PUBLICKEY_CHECK_KEYGEN { + SSHKeyGenPath, err = exec.LookPath("ssh-keygen") + if err != nil { + log.Fatal(4, "could not find ssh-keygen, maybe set DISABLE_SSH to use the internal ssh server") + } + } OfflineMode = sec.Key("OFFLINE_MODE").MustBool() DisableRouterLog = sec.Key("DISABLE_ROUTER_LOG").MustBool() StaticRootPath = sec.Key("STATIC_ROOT_PATH").MustString(workDir) @@ -459,6 +487,8 @@ var Service struct { EnableReverseProxyAuth bool EnableReverseProxyAutoRegister bool EnableCaptcha bool + EnableMinimumKeySizeCheck bool + MinimumKeySizes map[string]int } func newService() { @@ -471,6 +501,15 @@ func newService() { Service.EnableReverseProxyAuth = sec.Key("ENABLE_REVERSE_PROXY_AUTHENTICATION").MustBool() Service.EnableReverseProxyAutoRegister = sec.Key("ENABLE_REVERSE_PROXY_AUTO_REGISTRATION").MustBool() Service.EnableCaptcha = sec.Key("ENABLE_CAPTCHA").MustBool() + Service.EnableMinimumKeySizeCheck = sec.Key("ENABLE_MINIMUM_KEY_SIZE_CHECK").MustBool() + Service.MinimumKeySizes = map[string]int{} + + minimumKeySizes := Cfg.Section("service.minimum_key_sizes").Keys() + for _, key := range minimumKeySizes { + if key.MustInt() != -1 { + Service.MinimumKeySizes[strings.ToLower(key.Name())] = key.MustInt() + } + } } var logLevels = map[string]string{ From 9eef2e706cbe26896e3d3d8ca6e65094392dc101 Mon Sep 17 00:00:00 2001 From: Gibheer Date: Wed, 17 Feb 2016 09:33:41 +0100 Subject: [PATCH 2/6] fix ssh public key tests The old API was using []byte, but was changed to string without running the tests again. It also sets the variables from the configuration to make them work. Maybe there is a better way to do this. --- models/ssh_key_test.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/models/ssh_key_test.go b/models/ssh_key_test.go index 0dbf16ecb0..0cd56c413f 100644 --- a/models/ssh_key_test.go +++ b/models/ssh_key_test.go @@ -1,18 +1,22 @@ package models import ( + "github.com/gogits/gogs/modules/setting" "testing" ) func TestSSHKeyVerification(t *testing.T) { - keys := map[string][]byte{ - "dsa-1024": []byte("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"), - "rsa-1024": []byte("ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDAu7tvIvX6ZHrRXuZNfkR3XLHSsuCK9Zn3X58lxBcQzuo5xZgB6vRwwm/QtJuF+zZPtY5hsQILBLmF+BZ5WpKZp1jBeSjH2G7lxet9kbcH+kIVj0tPFEoyKI9wvWqIwC4prx/WVk2wLTJjzBAhyNxfEq7C9CeiX9pQEbEqJfkKCQ== nocomment\n"), - "rsa-2048": []byte("ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDMZXh+1OBUwSH9D45wTaxErQIN9IoC9xl7MKJkqvTvv6O5RR9YW/IK9FbfjXgXsppYGhsCZo1hFOOsXHMnfOORqu/xMDx4yPuyvKpw4LePEcg4TDipaDFuxbWOqc/BUZRZcXu41QAWfDLrInwsltWZHSeG7hjhpacl4FrVv9V1pS6Oc5Q1NxxEzTzuNLS/8diZrTm/YAQQ/+B+mzWI3zEtF4miZjjAljWd1LTBPvU23d29DcBmmFahcZ441XZsTeAwGxG/Q6j8NgNXj9WxMeWwxXV2jeAX/EBSpZrCVlCQ1yJswT6xCp8TuBnTiGWYMBNTbOZvPC4e0WI2/yZW/s5F nocomment"), - "ecdsa-256": []byte("ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBFQacN3PrOll7PXmN5B/ZNVahiUIqI05nbBlZk1KXsO3d06ktAWqbNflv2vEmA38bTFTfJ2sbn2B5ksT52cDDbA= nocomment"), - "ecdsa-384": []byte("ecdsa-sha2-nistp384 AAAAE2VjZHNhLXNoYTItbmlzdHAzODQAAAAIbmlzdHAzODQAAABhBINmioV+XRX1Fm9Qk2ehHXJ2tfVxW30ypUWZw670Zyq5GQfBAH6xjygRsJ5wWsHXBsGYgFUXIHvMKVAG1tpw7s6ax9oA+dJOJ7tj+vhn8joFqT+sg3LYHgZkHrfqryRasQ== nocomment"), - "ecdsa-512": []byte("ecdsa-sha2-nistp521 AAAAE2VjZHNhLXNoYTItbmlzdHA1MjEAAAAIbmlzdHA1MjEAAACFBACGt3UG3EzRwNOI17QR84l6PgiAcvCE7v6aXPj/SC6UWKg4EL8vW9ZBcdYL9wzs4FZXh4MOV8jAzu3KRWNTwb4k2wFNUpGOt7l28MztFFEtH5BDDrtAJSPENPy8pvPLMfnPg5NhvWycqIBzNcHipem5wSJFN5PdpNOC2xMrPWKNqj+ZjQ== nocomment"), - "ed25519-256": []byte("ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIC+8wwPU2VCo6pA+s9eOSqtDdFYA83/w+fpuSJLHTahU nocomment"), + setting.SSHWorkPath = "/tmp" + setting.SSHKeyGenPath = "/usr/bin/ssh-keygen" + + keys := map[string]string{ + "dsa-1024": 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"), + "rsa-1024": string("ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDAu7tvIvX6ZHrRXuZNfkR3XLHSsuCK9Zn3X58lxBcQzuo5xZgB6vRwwm/QtJuF+zZPtY5hsQILBLmF+BZ5WpKZp1jBeSjH2G7lxet9kbcH+kIVj0tPFEoyKI9wvWqIwC4prx/WVk2wLTJjzBAhyNxfEq7C9CeiX9pQEbEqJfkKCQ== nocomment\n"), + "rsa-2048": string("ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDMZXh+1OBUwSH9D45wTaxErQIN9IoC9xl7MKJkqvTvv6O5RR9YW/IK9FbfjXgXsppYGhsCZo1hFOOsXHMnfOORqu/xMDx4yPuyvKpw4LePEcg4TDipaDFuxbWOqc/BUZRZcXu41QAWfDLrInwsltWZHSeG7hjhpacl4FrVv9V1pS6Oc5Q1NxxEzTzuNLS/8diZrTm/YAQQ/+B+mzWI3zEtF4miZjjAljWd1LTBPvU23d29DcBmmFahcZ441XZsTeAwGxG/Q6j8NgNXj9WxMeWwxXV2jeAX/EBSpZrCVlCQ1yJswT6xCp8TuBnTiGWYMBNTbOZvPC4e0WI2/yZW/s5F nocomment"), + "ecdsa-256": string("ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBFQacN3PrOll7PXmN5B/ZNVahiUIqI05nbBlZk1KXsO3d06ktAWqbNflv2vEmA38bTFTfJ2sbn2B5ksT52cDDbA= nocomment"), + "ecdsa-384": string("ecdsa-sha2-nistp384 AAAAE2VjZHNhLXNoYTItbmlzdHAzODQAAAAIbmlzdHAzODQAAABhBINmioV+XRX1Fm9Qk2ehHXJ2tfVxW30ypUWZw670Zyq5GQfBAH6xjygRsJ5wWsHXBsGYgFUXIHvMKVAG1tpw7s6ax9oA+dJOJ7tj+vhn8joFqT+sg3LYHgZkHrfqryRasQ== nocomment"), + "ecdsa-512": string("ecdsa-sha2-nistp521 AAAAE2VjZHNhLXNoYTItbmlzdHA1MjEAAAAIbmlzdHA1MjEAAACFBACGt3UG3EzRwNOI17QR84l6PgiAcvCE7v6aXPj/SC6UWKg4EL8vW9ZBcdYL9wzs4FZXh4MOV8jAzu3KRWNTwb4k2wFNUpGOt7l28MztFFEtH5BDDrtAJSPENPy8pvPLMfnPg5NhvWycqIBzNcHipem5wSJFN5PdpNOC2xMrPWKNqj+ZjQ== nocomment"), + "ed25519-256": string("ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIC+8wwPU2VCo6pA+s9eOSqtDdFYA83/w+fpuSJLHTahU nocomment"), } for name, pubkey := range keys { From dab74f21b7b7f9755a8056d17adbd74c185c4199 Mon Sep 17 00:00:00 2001 From: Gibheer Date: Wed, 17 Feb 2016 11:30:48 +0100 Subject: [PATCH 3/6] remove ed25519 test for now TravisCI is too old for ed25519, so it can't be tested correctly. --- models/ssh_key_test.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/models/ssh_key_test.go b/models/ssh_key_test.go index 0cd56c413f..697c8d46e2 100644 --- a/models/ssh_key_test.go +++ b/models/ssh_key_test.go @@ -10,13 +10,12 @@ func TestSSHKeyVerification(t *testing.T) { setting.SSHKeyGenPath = "/usr/bin/ssh-keygen" keys := map[string]string{ - "dsa-1024": 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"), - "rsa-1024": string("ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDAu7tvIvX6ZHrRXuZNfkR3XLHSsuCK9Zn3X58lxBcQzuo5xZgB6vRwwm/QtJuF+zZPtY5hsQILBLmF+BZ5WpKZp1jBeSjH2G7lxet9kbcH+kIVj0tPFEoyKI9wvWqIwC4prx/WVk2wLTJjzBAhyNxfEq7C9CeiX9pQEbEqJfkKCQ== nocomment\n"), - "rsa-2048": string("ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDMZXh+1OBUwSH9D45wTaxErQIN9IoC9xl7MKJkqvTvv6O5RR9YW/IK9FbfjXgXsppYGhsCZo1hFOOsXHMnfOORqu/xMDx4yPuyvKpw4LePEcg4TDipaDFuxbWOqc/BUZRZcXu41QAWfDLrInwsltWZHSeG7hjhpacl4FrVv9V1pS6Oc5Q1NxxEzTzuNLS/8diZrTm/YAQQ/+B+mzWI3zEtF4miZjjAljWd1LTBPvU23d29DcBmmFahcZ441XZsTeAwGxG/Q6j8NgNXj9WxMeWwxXV2jeAX/EBSpZrCVlCQ1yJswT6xCp8TuBnTiGWYMBNTbOZvPC4e0WI2/yZW/s5F nocomment"), - "ecdsa-256": string("ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBFQacN3PrOll7PXmN5B/ZNVahiUIqI05nbBlZk1KXsO3d06ktAWqbNflv2vEmA38bTFTfJ2sbn2B5ksT52cDDbA= nocomment"), - "ecdsa-384": string("ecdsa-sha2-nistp384 AAAAE2VjZHNhLXNoYTItbmlzdHAzODQAAAAIbmlzdHAzODQAAABhBINmioV+XRX1Fm9Qk2ehHXJ2tfVxW30ypUWZw670Zyq5GQfBAH6xjygRsJ5wWsHXBsGYgFUXIHvMKVAG1tpw7s6ax9oA+dJOJ7tj+vhn8joFqT+sg3LYHgZkHrfqryRasQ== nocomment"), - "ecdsa-512": string("ecdsa-sha2-nistp521 AAAAE2VjZHNhLXNoYTItbmlzdHA1MjEAAAAIbmlzdHA1MjEAAACFBACGt3UG3EzRwNOI17QR84l6PgiAcvCE7v6aXPj/SC6UWKg4EL8vW9ZBcdYL9wzs4FZXh4MOV8jAzu3KRWNTwb4k2wFNUpGOt7l28MztFFEtH5BDDrtAJSPENPy8pvPLMfnPg5NhvWycqIBzNcHipem5wSJFN5PdpNOC2xMrPWKNqj+ZjQ== nocomment"), - "ed25519-256": string("ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIC+8wwPU2VCo6pA+s9eOSqtDdFYA83/w+fpuSJLHTahU nocomment"), + "dsa-1024": 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"), + "rsa-1024": string("ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDAu7tvIvX6ZHrRXuZNfkR3XLHSsuCK9Zn3X58lxBcQzuo5xZgB6vRwwm/QtJuF+zZPtY5hsQILBLmF+BZ5WpKZp1jBeSjH2G7lxet9kbcH+kIVj0tPFEoyKI9wvWqIwC4prx/WVk2wLTJjzBAhyNxfEq7C9CeiX9pQEbEqJfkKCQ== nocomment\n"), + "rsa-2048": string("ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDMZXh+1OBUwSH9D45wTaxErQIN9IoC9xl7MKJkqvTvv6O5RR9YW/IK9FbfjXgXsppYGhsCZo1hFOOsXHMnfOORqu/xMDx4yPuyvKpw4LePEcg4TDipaDFuxbWOqc/BUZRZcXu41QAWfDLrInwsltWZHSeG7hjhpacl4FrVv9V1pS6Oc5Q1NxxEzTzuNLS/8diZrTm/YAQQ/+B+mzWI3zEtF4miZjjAljWd1LTBPvU23d29DcBmmFahcZ441XZsTeAwGxG/Q6j8NgNXj9WxMeWwxXV2jeAX/EBSpZrCVlCQ1yJswT6xCp8TuBnTiGWYMBNTbOZvPC4e0WI2/yZW/s5F nocomment"), + "ecdsa-256": string("ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBFQacN3PrOll7PXmN5B/ZNVahiUIqI05nbBlZk1KXsO3d06ktAWqbNflv2vEmA38bTFTfJ2sbn2B5ksT52cDDbA= nocomment"), + "ecdsa-384": string("ecdsa-sha2-nistp384 AAAAE2VjZHNhLXNoYTItbmlzdHAzODQAAAAIbmlzdHAzODQAAABhBINmioV+XRX1Fm9Qk2ehHXJ2tfVxW30ypUWZw670Zyq5GQfBAH6xjygRsJ5wWsHXBsGYgFUXIHvMKVAG1tpw7s6ax9oA+dJOJ7tj+vhn8joFqT+sg3LYHgZkHrfqryRasQ== nocomment"), + "ecdsa-512": string("ecdsa-sha2-nistp521 AAAAE2VjZHNhLXNoYTItbmlzdHA1MjEAAAAIbmlzdHA1MjEAAACFBACGt3UG3EzRwNOI17QR84l6PgiAcvCE7v6aXPj/SC6UWKg4EL8vW9ZBcdYL9wzs4FZXh4MOV8jAzu3KRWNTwb4k2wFNUpGOt7l28MztFFEtH5BDDrtAJSPENPy8pvPLMfnPg5NhvWycqIBzNcHipem5wSJFN5PdpNOC2xMrPWKNqj+ZjQ== nocomment"), } for name, pubkey := range keys { From 2f27ee2232ca23404baf31443a3cf661d4445232 Mon Sep 17 00:00:00 2001 From: Gibheer Date: Tue, 23 Feb 2016 15:39:05 +0100 Subject: [PATCH 4/6] variable should not use ALL_CAPS --- models/ssh_key.go | 8 ++++---- models/ssh_key_test.go | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/models/ssh_key.go b/models/ssh_key.go index 0e5226076b..c22b931c88 100644 --- a/models/ssh_key.go +++ b/models/ssh_key.go @@ -36,8 +36,8 @@ const ( ) var ( - sshOpLocker = sync.Mutex{} - SSH_UNKNOWN_KEY_TYPE = fmt.Errorf("unknown key type") + sshOpLocker = sync.Mutex{} + SSHUnknownKeyType = fmt.Errorf("unknown key type") ) type KeyType int @@ -186,7 +186,7 @@ func SSHKeyGenParsePublicKey(key string) (string, int, error) { return "", 0, fmt.Errorf("public key check failed with error '%s': %s", err, stderr) } if strings.HasSuffix(stdout, "is not a public key file.") { - return "", 0, SSH_UNKNOWN_KEY_TYPE + return "", 0, SSHUnknownKeyType } fields := strings.Split(stdout, " ") if len(fields) < 4 { @@ -216,7 +216,7 @@ func SSHNativeParsePublicKey(keyLine string) (string, int, error) { pkey, err := ssh.ParsePublicKey(raw) if err != nil { if strings.HasPrefix(err.Error(), "ssh: unknown key algorithm") { - return "", 0, SSH_UNKNOWN_KEY_TYPE + return "", 0, SSHUnknownKeyType } return "", 0, err } diff --git a/models/ssh_key_test.go b/models/ssh_key_test.go index 697c8d46e2..cfb8554ae2 100644 --- a/models/ssh_key_test.go +++ b/models/ssh_key_test.go @@ -21,7 +21,7 @@ func TestSSHKeyVerification(t *testing.T) { for name, pubkey := range keys { keyTypeN, lengthN, errN := SSHNativeParsePublicKey(pubkey) if errN != nil { - if errN != SSH_UNKNOWN_KEY_TYPE { + if errN != SSHUnknownKeyType { t.Errorf("error parsing public key '%s': %s", name, errN) continue } @@ -31,8 +31,8 @@ func TestSSHKeyVerification(t *testing.T) { t.Errorf("error parsing public key '%s': %s", name, errK) continue } - // we know that ed25519 is currently not supported by native and returns SSH_UNKNOWN_KEY_TYPE - if (keyTypeN != keyTypeK || lengthN != lengthK) && errN != SSH_UNKNOWN_KEY_TYPE { + // we know that ed25519 is currently not supported by native and returns SSHUnknownKeyType + if (keyTypeN != keyTypeK || lengthN != lengthK) && errN != SSHUnknownKeyType { t.Errorf("key mismatch for '%s': native: %s(%d), ssh-keygen: %s(%d)", name, keyTypeN, lengthN, keyTypeK, lengthK) } } From e3570ae45dc8e9f53ec5c0d3a6d5b29fb7574bdd Mon Sep 17 00:00:00 2001 From: Gibheer Date: Tue, 23 Feb 2016 15:41:44 +0100 Subject: [PATCH 5/6] seperate ssh constants from schema constants The contants were placed in the same section as the scheme ones, which may lead to confusion. --- modules/setting/setting.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/modules/setting/setting.go b/modules/setting/setting.go index e667ee7433..25b9d8590e 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -27,14 +27,17 @@ import ( "github.com/gogits/gogs/modules/user" ) +const ( + SSH_PUBLICKEY_CHECK_NATIVE = "native" + SSH_PUBLICKEY_CHECK_KEYGEN = "ssh-keygen" +) + type Scheme string const ( - HTTP Scheme = "http" - HTTPS Scheme = "https" - FCGI Scheme = "fcgi" - SSH_PUBLICKEY_CHECK_NATIVE = "native" - SSH_PUBLICKEY_CHECK_KEYGEN = "ssh-keygen" + HTTP Scheme = "http" + HTTPS Scheme = "https" + FCGI Scheme = "fcgi" ) type LandingPage string From e721c5cf86c4d693a84bcf48d3a8a531efd24aaf Mon Sep 17 00:00:00 2001 From: Gibheer Date: Tue, 23 Feb 2016 15:43:52 +0100 Subject: [PATCH 6/6] use StartSSHServer instead of DisableSSH DisableSSH doesn't check the kind of ssh server to use, so that was wrong. Use StartSSHServer instead. --- modules/setting/setting.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/setting/setting.go b/modules/setting/setting.go index 25b9d8590e..f3d4349be9 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -337,7 +337,7 @@ func NewContext() { log.Fatal(4, "Fail to create '%s': %v", SSHRootPath, err) } checkDefault := SSH_PUBLICKEY_CHECK_KEYGEN - if DisableSSH { + if StartSSHServer { checkDefault = SSH_PUBLICKEY_CHECK_NATIVE } SSHPublicKeyCheck = sec.Key("SSH_PUBLICKEY_CHECK").MustString(checkDefault)