Post work for #2637

Improve test cases, config settings, also show SSH config settings on admin config panel.

README.md

@@ -3,7 +3,7 @@ Gogs - Go Git Service [![Build Status](https://travis-ci.org/gogits/gogs.svg?bra
 
 ![](https://github.com/gogits/gogs/blob/master/public/img/gogs-large-resize.png?raw=true)
 
-##### Current version: 0.8.46
+##### Current version: 0.8.47
 
 | Web | UI  | Preview  |
 |:-------------:|:-------:|:-------:|

cmd/serve.go

@@ -136,7 +136,7 @@ func runServ(c *cli.Context) {
 
 	setup("serv.log")
 
-	if setting.DisableSSH {
+	if setting.SSH.Disabled {
 		println("Gogs: SSH has been disabled")
 		return
 	}

conf/app.ini

@@ -71,13 +71,13 @@ SSH_PORT = 22
 SSH_LISTEN_PORT = %(SSH_PORT)s
 ; Root path of SSH directory, default is '~/.ssh', but you have to use '/home/git/.ssh'.
 SSH_ROOT_PATH = 
-; override engine choice to check public keys (default: 'ssh-keygen' when
+; Directory to create temporary files when test publick key using ssh-keygen,
-; DISABLE_SSH is set to false else 'native')
+; default is system temporary directory.
-SSH_PUBLICKEY_CHECK =
+SSH_KEY_TEST_PATH =
-; directory to create temporary files when using ssh-keygen (default: /tmp)
+; Path to ssh-keygen, default is 'ssh-keygen' and let shells find out which one to call.
-SSH_WORK_PATH =
+SSH_KEYGEN_PATH = ssh-keygen
-; path to ssh-keygen (default: result of `which ssh-keygen`)
+; Indicate whether to check minimum key size with corresponding type
-SSH_KEYGEN_PATH =
+MINIMUM_KEY_SIZE_CHECK = false
 ; Disable CDN even in "prod" mode
 OFFLINE_MODE = false
 DISABLE_ROUTER_LOG = false
@@ -98,6 +98,13 @@ ENABLE_GZIP = false
 ; Landing page for non-logged users, can be "home" or "explore"
 LANDING_PAGE = home
 
+; Define allowed algorithms and their minimum key length (use -1 to disable a type)
+[ssh.minimum_key_sizes]
+ED25519 = 256
+ECDSA   = 256
+RSA     = 2048
+DSA     = 1024
+
 [database]
 ; Either "mysql", "postgres" or "sqlite3", it's your choice
 DB_TYPE = mysql
@@ -139,15 +146,6 @@ 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

conf/locale/locale_en-US.ini

@@ -960,6 +960,19 @@ config.static_file_root_path = Static File Root Path
 config.log_file_root_path = Log File Root Path
 config.script_type = Script Type
 config.reverse_auth_user = Reverse Authentication User
+
+config.ssh_config = SSH Configuration
+config.ssh_enabled = Enabled
+config.ssh_start_builtin_server = Start Builtin Server
+config.ssh_domain = Domain
+config.ssh_port = Port
+config.ssh_listen_port = Listen Port
+config.ssh_root_path = Root Path
+config.ssh_key_test_path = Key Test Path
+config.ssh_keygen_path = Keygen ('ssh-keygen') Path
+config.ssh_minimum_key_size_check = Minimum Key Size Check
+config.ssh_minimum_key_sizes = Minimum Key Sizes
+
 config.db_config = Database Configuration
 config.db_type = Type
 config.db_host = Host

gogs.go

@@ -17,7 +17,7 @@ import (
 	"github.com/gogits/gogs/modules/setting"
 )
 
-const APP_VER = "0.8.46.0227"
+const APP_VER = "0.8.47.0227"
 
 func init() {
 	runtime.GOMAXPROCS(runtime.NumCPU())

models/repo.go

@@ -457,10 +457,10 @@ func (repo *Repository) cloneLink(isWiki bool) *CloneLink {
 
 	repo.Owner = repo.MustOwner()
 	cl := new(CloneLink)
-	if setting.SSHPort != 22 {
+	if setting.SSH.Port != 22 {
-		cl.SSH = fmt.Sprintf("ssh://%s@%s:%d/%s/%s.git", setting.RunUser, setting.SSHDomain, setting.SSHPort, repo.Owner.Name, repoName)
+		cl.SSH = fmt.Sprintf("ssh://%s@%s:%d/%s/%s.git", setting.RunUser, setting.SSH.Domain, setting.SSH.Port, repo.Owner.Name, repoName)
 	} else {
-		cl.SSH = fmt.Sprintf("%s@%s:%s/%s.git", setting.RunUser, setting.SSHDomain, repo.Owner.Name, repoName)
+		cl.SSH = fmt.Sprintf("%s@%s:%s/%s.git", setting.RunUser, setting.SSH.Domain, repo.Owner.Name, repoName)
 	}
 	cl.HTTPS = fmt.Sprintf("%s%s/%s.git", setting.AppUrl, repo.Owner.Name, repoName)
 	return cl

models/ssh_key.go

@@ -16,7 +16,6 @@ import (
 	"os"
 	"path"
 	"path/filepath"
-	"strconv"
 	"strings"
 	"sync"
 	"time"
@@ -35,10 +34,7 @@ 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 (
+var sshOpLocker = sync.Mutex{}
-	sshOpLocker       = sync.Mutex{}
-	SSHUnknownKeyType = fmt.Errorf("unknown key type")
-)
 
 type KeyType int
 
@@ -83,19 +79,18 @@ func (key *PublicKey) GetAuthorizedString() string {
 func extractTypeFromBase64Key(key string) (string, error) {
 	b, err := base64.StdEncoding.DecodeString(key)
 	if err != nil || len(b) < 4 {
-		return "", errors.New("Invalid key format")
+		return "", fmt.Errorf("Invalid key format: %v", err)
 	}
 
 	keyLength := int(binary.BigEndian.Uint32(b))
-
 	if len(b) < 4+keyLength {
-		return "", errors.New("Invalid key format")
+		return "", fmt.Errorf("Invalid key format: not enough length")
 	}
 
 	return string(b[4 : 4+keyLength]), nil
 }
 
-// parseKeyString parses any key string in openssh or ssh2 format to clean openssh string (rfc4253)
+// 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"
 	s := strings.Replace(strings.Replace(strings.TrimSpace(content), "\r\n", "\n", -1), "\r", "\n", -1)
@@ -158,50 +153,53 @@ func parseKeyString(content string) (string, error) {
 	return keyType + " " + keyContent + " " + keyComment, nil
 }
 
-// extract key type and length using ssh-keygen
+// writeTmpKeyFile writes key content to a temporary file
+// and returns the name of that file, along with any possible errors.
+func writeTmpKeyFile(content string) (string, error) {
+	tmpFile, err := ioutil.TempFile(setting.SSH.KeyTestPath, "gogs_keytest")
+	if err != nil {
+		return "", fmt.Errorf("TempFile: %v", err)
+	}
+	defer tmpFile.Close()
+
+	if _, err = tmpFile.WriteString(content); err != nil {
+		return "", fmt.Errorf("tmpFile.WriteString: %v", err)
+	}
+	return tmpFile.Name(), nil
+}
+
+// SSHKeyGenParsePublicKey extracts 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")
+	tmpName, err := writeTmpKeyFile(key)
 	if err != nil {
-		return "", 0, err
+		return "", 0, fmt.Errorf("writeTmpKeyFile: %v", err)
 	}
-	tmpName := tmpFile.Name()
 	defer os.Remove(tmpName)
 
-	if ln, err := tmpFile.WriteString(key); err != nil {
+	stdout, stderr, err := process.Exec("SSHKeyGenParsePublicKey", setting.SSH.KeygenPath, "-lf", tmpName)
-		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)
+		return "", 0, fmt.Errorf("Fail to parse public key: %s - %s", err, stderr)
 	}
-	if strings.HasSuffix(stdout, "is not a public key file.") {
+	if strings.Contains(stdout, "is not a public key file") {
-		return "", 0, SSHUnknownKeyType
+		return "", 0, ErrKeyUnableVerify{stdout}
 	}
+
 	fields := strings.Split(stdout, " ")
 	if len(fields) < 4 {
-		return "", 0, fmt.Errorf("invalid public key line: %s", stdout)
+		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
+	return strings.ToLower(keyType), com.StrTo(fields[0]).MustInt(), nil
 }
 
-// extract the key type and length using the golang ssh library
+// SSHNativeParsePublicKey extracts the key type and length using the golang SSH library.
+// NOTE: ed25519 is not supported.
 func SSHNativeParsePublicKey(keyLine string) (string, int, error) {
 	fields := strings.Fields(keyLine)
 	if len(fields) < 2 {
@@ -215,14 +213,13 @@ func SSHNativeParsePublicKey(keyLine string) (string, int, error) {
 
 	pkey, err := ssh.ParsePublicKey(raw)
 	if err != nil {
-		if strings.HasPrefix(err.Error(), "ssh: unknown key algorithm") {
+		if strings.Contains(err.Error(), "ssh: unknown key algorithm") {
-			return "", 0, SSHUnknownKeyType
+			return "", 0, ErrKeyUnableVerify{err.Error()}
 		}
-		return "", 0, err
+		return "", 0, fmt.Errorf("ssh.ParsePublicKey: %v", err)
 	}
 
-	// The ssh library can parse the key, so next we find out what key exactly we
+	// The ssh library can parse the key, so next we find out what key exactly we have.
-	// have.
 	switch pkey.Type() {
 	case ssh.KeyAlgoDSA:
 		rawPub := struct {
@@ -253,16 +250,18 @@ func SSHNativeParsePublicKey(keyLine string) (string, int, error) {
 		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")
+	return "", 0, fmt.Errorf("Unsupported key length detection for type: %s", pkey.Type())
 }
 
 // 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) {
+	if setting.SSH.Disabled {
+		return "", errors.New("SSH is disabled")
+	}
+
 	content, err = parseKeyString(content)
 	if err != nil {
 		return "", err
@@ -280,30 +279,25 @@ func CheckPublicKeyString(content string) (_ string, err error) {
 		keyType string
 		length  int
 	)
-	if setting.SSHPublicKeyCheck == setting.SSH_PUBLICKEY_CHECK_NATIVE {
+	if setting.SSH.StartBuiltinServer {
 		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)
+		keyType, length, err = SSHKeyGenParsePublicKey(content)
-		return "", fmt.Errorf("invalid public key check type")
 	}
-
 	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", keyType)
+	log.Trace("Key info [native: %v]: %s-%d", setting.SSH.StartBuiltinServer, keyType, length)
 
-	if !setting.Service.EnableMinimumKeySizeCheck {
+	if !setting.SSH.MinimumKeySizeCheck {
 		return content, nil
 	}
-	if minLen, found := setting.Service.MinimumKeySizes[keyType]; found && length >= minLen {
+	if minLen, found := setting.SSH.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 length is not enough: got %d, needs %d", length, minLen)
 	}
-	return "", fmt.Errorf("key type '%s' is not allowed", keyType)
+	return "", fmt.Errorf("Key type is not allowed: %s", keyType)
 }
 
 // saveAuthorizedKeyFile writes SSH key content to authorized_keys file.
@@ -311,7 +305,7 @@ func saveAuthorizedKeyFile(keys ...*PublicKey) error {
 	sshOpLocker.Lock()
 	defer sshOpLocker.Unlock()
 
-	fpath := filepath.Join(setting.SSHRootPath, "authorized_keys")
+	fpath := filepath.Join(setting.SSH.RootPath, "authorized_keys")
 	f, err := os.OpenFile(fpath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600)
 	if err != nil {
 		return err
@@ -379,7 +373,7 @@ func addKey(e Engine, key *PublicKey) (err error) {
 	}
 
 	// Don't need to rewrite this file if builtin SSH server is enabled.
-	if setting.StartSSHServer {
+	if setting.SSH.StartBuiltinServer {
 		return nil
 	}
 	return saveAuthorizedKeyFile(key)
@@ -529,12 +523,12 @@ func deletePublicKey(e *xorm.Session, keyID int64) error {
 	}
 
 	// Don't need to rewrite this file if builtin SSH server is enabled.
-	if setting.StartSSHServer {
+	if setting.SSH.StartBuiltinServer {
 		return nil
 	}
 
-	fpath := filepath.Join(setting.SSHRootPath, "authorized_keys")
+	fpath := filepath.Join(setting.SSH.RootPath, "authorized_keys")
-	tmpPath := filepath.Join(setting.SSHRootPath, "authorized_keys.tmp")
+	tmpPath := fpath + ".tmp"
 	if err = rewriteAuthorizedKeys(key, fpath, tmpPath); err != nil {
 		return err
 	} else if err = os.Remove(fpath); err != nil {
@@ -576,7 +570,8 @@ func RewriteAllPublicKeys() error {
 	sshOpLocker.Lock()
 	defer sshOpLocker.Unlock()
 
-	tmpPath := filepath.Join(setting.SSHRootPath, "authorized_keys.tmp")
+	fpath := filepath.Join(setting.SSH.RootPath, "authorized_keys")
+	tmpPath := fpath + ".tmp"
 	f, err := os.OpenFile(tmpPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600)
 	if err != nil {
 		return err
@@ -592,7 +587,6 @@ func RewriteAllPublicKeys() error {
 		return err
 	}
 
-	fpath := filepath.Join(setting.SSHRootPath, "authorized_keys")
 	if com.IsExist(fpath) {
 		if err = os.Remove(fpath); err != nil {
 			return err

models/ssh_key_test.go

@@ -1,39 +1,45 @@
 package models
 
 import (
-	"github.com/gogits/gogs/modules/setting"
+	"fmt"
 	"testing"
+
+	. "github.com/smartystreets/goconvey/convey"
+
+	"github.com/gogits/gogs/modules/setting"
 )
 
-func TestSSHKeyVerification(t *testing.T) {
+func init() {
-	setting.SSHWorkPath = "/tmp"
+	setting.NewContext()
-	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"),
 }
 
-	for name, pubkey := range keys {
+func Test_SSHParsePublicKey(t *testing.T) {
-		keyTypeN, lengthN, errN := SSHNativeParsePublicKey(pubkey)
+	testKeys := map[string]struct {
-		if errN != nil {
+		typeName string
-			if errN != SSHUnknownKeyType {
+		length   int
-				t.Errorf("error parsing public key '%s': %s", name, errN)
+		content  string
-				continue
+	}{
-			}
+		"dsa-1024":  {"dsa", 1024, "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":  {"rsa", 1024, "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDAu7tvIvX6ZHrRXuZNfkR3XLHSsuCK9Zn3X58lxBcQzuo5xZgB6vRwwm/QtJuF+zZPtY5hsQILBLmF+BZ5WpKZp1jBeSjH2G7lxet9kbcH+kIVj0tPFEoyKI9wvWqIwC4prx/WVk2wLTJjzBAhyNxfEq7C9CeiX9pQEbEqJfkKCQ== nocomment\n"},
-		keyTypeK, lengthK, errK := SSHKeyGenParsePublicKey(pubkey)
+		"rsa-2048":  {"rsa", 2048, "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDMZXh+1OBUwSH9D45wTaxErQIN9IoC9xl7MKJkqvTvv6O5RR9YW/IK9FbfjXgXsppYGhsCZo1hFOOsXHMnfOORqu/xMDx4yPuyvKpw4LePEcg4TDipaDFuxbWOqc/BUZRZcXu41QAWfDLrInwsltWZHSeG7hjhpacl4FrVv9V1pS6Oc5Q1NxxEzTzuNLS/8diZrTm/YAQQ/+B+mzWI3zEtF4miZjjAljWd1LTBPvU23d29DcBmmFahcZ441XZsTeAwGxG/Q6j8NgNXj9WxMeWwxXV2jeAX/EBSpZrCVlCQ1yJswT6xCp8TuBnTiGWYMBNTbOZvPC4e0WI2/yZW/s5F nocomment"},
-		if errK != nil {
+		"ecdsa-256": {"ecdsa", 256, "ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBFQacN3PrOll7PXmN5B/ZNVahiUIqI05nbBlZk1KXsO3d06ktAWqbNflv2vEmA38bTFTfJ2sbn2B5ksT52cDDbA= nocomment"},
-			t.Errorf("error parsing public key '%s': %s", name, errK)
+		"ecdsa-384": {"ecdsa", 384, "ecdsa-sha2-nistp384 AAAAE2VjZHNhLXNoYTItbmlzdHAzODQAAAAIbmlzdHAzODQAAABhBINmioV+XRX1Fm9Qk2ehHXJ2tfVxW30ypUWZw670Zyq5GQfBAH6xjygRsJ5wWsHXBsGYgFUXIHvMKVAG1tpw7s6ax9oA+dJOJ7tj+vhn8joFqT+sg3LYHgZkHrfqryRasQ== nocomment"},
-			continue
+		"ecdsa-521": {"ecdsa", 521, "ecdsa-sha2-nistp521 AAAAE2VjZHNhLXNoYTItbmlzdHA1MjEAAAAIbmlzdHA1MjEAAACFBACGt3UG3EzRwNOI17QR84l6PgiAcvCE7v6aXPj/SC6UWKg4EL8vW9ZBcdYL9wzs4FZXh4MOV8jAzu3KRWNTwb4k2wFNUpGOt7l28MztFFEtH5BDDrtAJSPENPy8pvPLMfnPg5NhvWycqIBzNcHipem5wSJFN5PdpNOC2xMrPWKNqj+ZjQ== nocomment"},
-		}
-		// 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)
 	}
+
+	Convey("Parse public keys in both native and ssh-keygen", t, func() {
+		for name, key := range testKeys {
+			fmt.Println("\nTesting key:", name)
+
+			keyTypeN, lengthN, errN := SSHNativeParsePublicKey(key.content)
+			So(errN, ShouldBeNil)
+			So(keyTypeN, ShouldEqual, key.typeName)
+			So(lengthN, ShouldEqual, key.length)
+
+			keyTypeK, lengthK, errK := SSHKeyGenParsePublicKey(key.content)
+			So(errK, ShouldBeNil)
+			So(keyTypeK, ShouldEqual, key.typeName)
+			So(lengthK, ShouldEqual, key.length)
 		}
+	})
 }

modules/middleware/repo.go

@@ -169,7 +169,7 @@ func RepoAssignment(args ...bool) macaron.Handler {
 		ctx.Data["IsRepositoryPusher"] = ctx.Repo.IsPusher()
 		ctx.Data["CanPullRequest"] = ctx.Repo.IsAdmin() && repo.BaseRepo != nil && repo.BaseRepo.AllowsPulls()
 
-		ctx.Data["DisableSSH"] = setting.DisableSSH
+		ctx.Data["DisableSSH"] = setting.SSH.Disabled
 		ctx.Data["CloneLink"] = repo.CloneLink()
 		ctx.Data["WikiCloneLink"] = repo.WikiCloneLink()
 

modules/setting/setting.go

@@ -27,11 +27,6 @@ import (
 	"github.com/gogits/gogs/modules/user"
 )
 
-const (
-	SSH_PUBLICKEY_CHECK_NATIVE = "native"
-	SSH_PUBLICKEY_CHECK_KEYGEN = "ssh-keygen"
-)
-
 type Scheme string
 
 const (
@@ -66,15 +61,6 @@ var (
 	Domain             string
 	HttpAddr, HttpPort string
 	LocalURL           string
-	DisableSSH         bool
-	StartSSHServer     bool
-	SSHDomain          string
-	SSHPort            int
-	SSHListenPort      int
-	SSHRootPath        string
-	SSHPublicKeyCheck  string
-	SSHWorkPath        string
-	SSHKeyGenPath      string
 	OfflineMode        bool
 	DisableRouterLog   bool
 	CertFile, KeyFile  string
@@ -82,6 +68,19 @@ var (
 	EnableGzip         bool
 	LandingPageUrl     LandingPage
 
+	SSH struct {
+		Disabled            bool           `ini:"DISABLE_SSH"`
+		StartBuiltinServer  bool           `ini:"START_SSH_SERVER"`
+		Domain              string         `ini:"SSH_DOMAIN"`
+		Port                int            `ini:"SSH_PORT"`
+		ListenPort          int            `ini:"SSH_LISTEN_PORT"`
+		RootPath            string         `ini:"SSH_ROOT_PATH"`
+		KeyTestPath         string         `ini:"SSH_KEY_TEST_PATH"`
+		KeygenPath          string         `ini:"SSH_KEYGEN_PATH"`
+		MinimumKeySizeCheck bool           `ini:"-"`
+		MinimumKeySizes     map[string]int `ini:"-"`
+	}
+
 	// Security settings
 	InstallLock          bool
 	SecretKey            string
@@ -327,40 +326,6 @@ func NewContext() {
 	HttpAddr = sec.Key("HTTP_ADDR").MustString("0.0.0.0")
 	HttpPort = sec.Key("HTTP_PORT").MustString("3000")
 	LocalURL = sec.Key("LOCAL_ROOT_URL").MustString("http://localhost:" + HttpPort + "/")
-	DisableSSH = sec.Key("DISABLE_SSH").MustBool()
-	if !DisableSSH {
-		StartSSHServer = sec.Key("START_SSH_SERVER").MustBool()
-	}
-	SSHDomain = sec.Key("SSH_DOMAIN").MustString(Domain)
-	SSHPort = sec.Key("SSH_PORT").MustInt(22)
-	SSHListenPort = sec.Key("SSH_LISTEN_PORT").MustInt(SSHPort)
-	SSHRootPath = sec.Key("SSH_ROOT_PATH").MustString(path.Join(homeDir, ".ssh"))
-	if err := os.MkdirAll(SSHRootPath, 0700); err != nil {
-		log.Fatal(4, "Fail to create '%s': %v", SSHRootPath, err)
-	}
-	checkDefault := SSH_PUBLICKEY_CHECK_KEYGEN
-	if StartSSHServer {
-		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)
@@ -373,6 +338,39 @@ func NewContext() {
 		LandingPageUrl = LANDING_PAGE_HOME
 	}
 
+	SSH.RootPath = path.Join(homeDir, ".ssh")
+	SSH.KeyTestPath = os.TempDir()
+	if err = Cfg.Section("server").MapTo(&SSH); err != nil {
+		log.Fatal(4, "Fail to map SSH settings: %v", err)
+	}
+	// When disable SSH, start builtin server value is ignored.
+	if SSH.Disabled {
+		SSH.StartBuiltinServer = false
+	}
+
+	if !SSH.Disabled && !SSH.StartBuiltinServer {
+		if err := os.MkdirAll(SSH.RootPath, 0700); err != nil {
+			log.Fatal(4, "Fail to create '%s': %v", SSH.RootPath, err)
+		} else if err = os.MkdirAll(SSH.KeyTestPath, 0644); err != nil {
+			log.Fatal(4, "Fail to create '%s': %v", SSH.KeyTestPath, err)
+		}
+
+		if !filepath.IsAbs(SSH.KeygenPath) {
+			if _, err := exec.LookPath(SSH.KeygenPath); err != nil {
+				log.Fatal(4, "Fail to test '%s' command: %v (forgotten install?)", SSH.KeygenPath, err)
+			}
+		}
+	}
+
+	SSH.MinimumKeySizeCheck = sec.Key("MINIMUM_KEY_SIZE_CHECK").MustBool()
+	SSH.MinimumKeySizes = map[string]int{}
+	minimumKeySizes := Cfg.Section("ssh.minimum_key_sizes").Keys()
+	for _, key := range minimumKeySizes {
+		if key.MustInt() != -1 {
+			SSH.MinimumKeySizes[strings.ToLower(key.Name())] = key.MustInt()
+		}
+	}
+
 	sec = Cfg.Section("security")
 	InstallLock = sec.Key("INSTALL_LOCK").MustBool()
 	SecretKey = sec.Key("SECRET_KEY").String()
@@ -492,8 +490,6 @@ var Service struct {
 	EnableReverseProxyAuth         bool
 	EnableReverseProxyAutoRegister bool
 	EnableCaptcha                  bool
-	EnableMinimumKeySizeCheck      bool
-	MinimumKeySizes                map[string]int
 }
 
 func newService() {
@@ -506,15 +502,6 @@ 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{

routers/admin/admin.go

@@ -204,6 +204,8 @@ func Config(ctx *middleware.Context) {
 	ctx.Data["ScriptType"] = setting.ScriptType
 	ctx.Data["ReverseProxyAuthUser"] = setting.ReverseProxyAuthUser
 
+	ctx.Data["SSH"] = setting.SSH
+
 	ctx.Data["Service"] = setting.Service
 	ctx.Data["DbCfg"] = models.DbCfg
 	ctx.Data["Webhook"] = setting.Webhook

routers/install.go

@@ -88,9 +88,9 @@ func GlobalInit() {
 	}
 	checkRunMode()
 
-	if setting.StartSSHServer {
+	if setting.SSH.StartBuiltinServer {
-		ssh.Listen(setting.SSHListenPort)
+		ssh.Listen(setting.SSH.ListenPort)
-		log.Info("SSH server started on :%v", setting.SSHListenPort)
+		log.Info("SSH server started on :%v", setting.SSH.ListenPort)
 	}
 
 	// Build Sanitizer
@@ -152,7 +152,7 @@ func Install(ctx *middleware.Context) {
 	}
 
 	form.Domain = setting.Domain
-	form.SSHPort = setting.SSHPort
+	form.SSHPort = setting.SSH.Port
 	form.HTTPPort = setting.HttpPort
 	form.AppUrl = setting.AppUrl
 	form.LogRootPath = setting.LogRootPath

templates/.VERSION

@@ -1 +1 @@
-0.8.46.0227
+0.8.47.0227

templates/admin/config.tmpl

@@ -22,12 +22,16 @@
 						<dd><i class="fa fa{{if .OfflineMode}}-check{{end}}-square-o"></i></dd>
 						<dt>{{.i18n.Tr "admin.config.disable_router_log"}}</dt>
 						<dd><i class="fa fa{{if .DisableRouterLog}}-check{{end}}-square-o"></i></dd>
+
 						<div class="ui divider"></div>
+
 						<dt>{{.i18n.Tr "admin.config.run_user"}}</dt>
 						<dd>{{.RunUser}}</dd>
 						<dt>{{.i18n.Tr "admin.config.run_mode"}}</dt>
 						<dd>{{.RunMode}}</dd>
+
 						<div class="ui divider"></div>
+
 						<dt>{{.i18n.Tr "admin.config.repo_root_path"}}</dt>
 						<dd>{{.RepoRootPath}}</dd>
 						<dt>{{.i18n.Tr "admin.config.static_file_root_path"}}</dt>
@@ -41,6 +45,41 @@
 					</dl>
 				</div>
 
+				<h4 class="ui top attached header">
+					{{.i18n.Tr "admin.config.ssh_config"}}
+				</h4>
+				<div class="ui attached table segment">
+					<dl class="dl-horizontal admin-dl-horizontal">
+						<dt>{{.i18n.Tr "admin.config.ssh_enabled"}}</dt>
+						<dd><i class="fa fa{{if not .SSH.Disabled}}-check{{end}}-square-o"></i></dd>
+						{{if not .SSH.Disabled}}
+							<dt>{{.i18n.Tr "admin.config.ssh_start_builtin_server"}}</dt>
+							<dd><i class="fa fa{{if .SSH.StartBuiltinServer}}-check{{end}}-square-o"></i></dd>
+							<dt>{{.i18n.Tr "admin.config.ssh_domain"}}</dt>
+							<dd>{{.SSH.Domain}}</dd>
+							<dt>{{.i18n.Tr "admin.config.ssh_port"}}</dt>
+							<dd>{{.SSH.Port}}</dd>
+							<dt>{{.i18n.Tr "admin.config.ssh_listen_port"}}</dt>
+							<dd>{{.SSH.ListenPort}}</dd>
+
+							{{if not .SSH.StartBuiltinServer}}
+								<dt>{{.i18n.Tr "admin.config.ssh_root_path"}}</dt>
+								<dd>{{.SSH.RootPath}}</dd>
+								<dt>{{.i18n.Tr "admin.config.ssh_key_test_path"}}</dt>
+								<dd>{{.SSH.KeyTestPath}}</dd>
+								<dt>{{.i18n.Tr "admin.config.ssh_keygen_path"}}</dt>
+								<dd>{{.SSH.KeygenPath}}</dd>
+								<dt>{{.i18n.Tr "admin.config.ssh_minimum_key_size_check"}}</dt>
+								<dd><i class="fa fa{{if .SSH.MinimumKeySizeCheck}}-check{{end}}-square-o"></i></dd>
+								{{if .SSH.MinimumKeySizeCheck}}
+								<dt>{{.i18n.Tr "admin.config.ssh_minimum_key_sizes"}}</dt>
+								<dd>{{.SSH.MinimumKeySizes}}</dd>
+								{{end}}
+							{{end}}
+						{{end}}
+					</dl>
+				</div>
+
 				<h4 class="ui top attached header">
 					{{.i18n.Tr "admin.config.db_config"}}
 				</h4>
@@ -109,7 +148,8 @@
 					<dl class="dl-horizontal admin-dl-horizontal">
 						<dt>{{.i18n.Tr "admin.config.mailer_enabled"}}</dt>
 						<dd><i class="fa fa{{if .MailerEnabled}}-check{{end}}-square-o"></i></dd>
-						{{if .MailerEnabled}}<dt>{{.i18n.Tr "admin.config.mailer_name"}}</dt>
+						{{if .MailerEnabled}}
+							<dt>{{.i18n.Tr "admin.config.mailer_name"}}</dt>
 							<dd>{{.Mailer.Name}}</dd>
 							<dt>{{.i18n.Tr "admin.config.mailer_disable_helo"}}</dt>
 							<dd><i class="fa fa{{if .Mailer.DisableHelo}}-check{{end}}-square-o"></i></dd>