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
7ef9a05588. 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.
This commit is contained in:
Gibheer 2016-02-16 23:01:56 +01:00
parent 3af1d3c581
commit 12403bdfb0
4 changed files with 227 additions and 15 deletions

View file

@ -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

View file

@ -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
}

36
models/ssh_key_test.go Normal file
View file

@ -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)
}
}
}

View file

@ -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{