Prefer native parser for SSH public key parsing (#23798)

Without this patch, the setting SSH.StartBuiltinServer decides whether
the native (Go) implementation is used rather than calling 'ssh-keygen'.
It's possible for 'using ssh-keygen' and 'using the built-in server' to
be independent.
In fact, the gitea rootless container doesn't ship ssh-keygen and can be
configured to use the host's SSH server - which will cause the public
key parsing mechanism to break.

This commit changes the decision to be based on SSH.KeygenPath instead.
Any existing configurations with a custom KeygenPath set will continue
to function. The new default value of '' selects the native version. The
downside of this approach is that anyone who has relying on plain
'ssh-keygen' to have special properties will now be using the native
version instead.
I assume the exec-variant is only there because /x/crypto/ssh didn't
support ssh-ed25519 until 2016. I don't see any other reason for using
it so it might be an acceptable risk.

Fixes #23363

EDIT: this message was garbled when I tried to get the commit
description back in.. Trying to reconstruct it:

## ⚠️ BREAKING ⚠️ Users who don't have SSH.KeygenPath
explicitly set and rely on the ssh-keygen binary need to set
SSH.KeygenPath to 'ssh-keygen' in order to be able to continue using it
for public key parsing.

There was something else but I can't remember at the moment.

EDIT2: It was about `make test` and `make lint`. Can't get them to run.
To reproduce the issue, I installed `golang` in `docker.io/node:16` and
got:
```
...
go: mvdan.cc/xurls/v2@v2.4.0: unknown revision mvdan.cc/xurls/v2.4.0
go: gotest.tools/v3@v3.4.0: unknown revision gotest.tools/v3.4.0
...
go: gotest.tools/v3@v3.0.3: unknown revision gotest.tools/v3.0.3
...
go: error loading module requirements
```

Signed-off-by: Leon M. Busch-George <leon@georgemail.eu>
This commit is contained in:
Leon Busch-George 2023-04-11 08:34:28 +02:00 committed by GitHub
parent ef7fd781f5
commit 7a8a4f5432
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 20 additions and 7 deletions

View file

@ -186,8 +186,8 @@ RUN_MODE = ; prod
;; default is the system temporary directory. ;; default is the system temporary directory.
;SSH_KEY_TEST_PATH = ;SSH_KEY_TEST_PATH =
;; ;;
;; Path to ssh-keygen, default is 'ssh-keygen' which means the shell is responsible for finding out which one to call. ;; Use `ssh-keygen` to parse public SSH keys. The value is passed to the shell. By default, Gitea does the parsing itself.
;SSH_KEYGEN_PATH = ssh-keygen ;SSH_KEYGEN_PATH =
;; ;;
;; Enable SSH Authorized Key Backup when rewriting all keys, default is true ;; Enable SSH Authorized Key Backup when rewriting all keys, default is true
;SSH_AUTHORIZED_KEYS_BACKUP = true ;SSH_AUTHORIZED_KEYS_BACKUP = true

View file

@ -345,7 +345,7 @@ The following configuration set `Content-Type: application/vnd.android.package-a
- `SSH_SERVER_MACS`: **hmac-sha2-256-etm@openssh.com, hmac-sha2-256, hmac-sha1**: For the built-in SSH server, choose the MACs to support for SSH connections, for system SSH this setting has no effect - `SSH_SERVER_MACS`: **hmac-sha2-256-etm@openssh.com, hmac-sha2-256, hmac-sha1**: For the built-in SSH server, choose the MACs to support for SSH connections, for system SSH this setting has no effect
- `SSH_SERVER_HOST_KEYS`: **ssh/gitea.rsa, ssh/gogs.rsa**: For the built-in SSH server, choose the keypairs to offer as the host key. The private key should be at `SSH_SERVER_HOST_KEY` and the public `SSH_SERVER_HOST_KEY.pub`. Relative paths are made absolute relative to the `APP_DATA_PATH`. If no key exists a 4096 bit RSA key will be created for you. - `SSH_SERVER_HOST_KEYS`: **ssh/gitea.rsa, ssh/gogs.rsa**: For the built-in SSH server, choose the keypairs to offer as the host key. The private key should be at `SSH_SERVER_HOST_KEY` and the public `SSH_SERVER_HOST_KEY.pub`. Relative paths are made absolute relative to the `APP_DATA_PATH`. If no key exists a 4096 bit RSA key will be created for you.
- `SSH_KEY_TEST_PATH`: **/tmp**: Directory to create temporary files in when testing public keys using ssh-keygen, default is the system temporary directory. - `SSH_KEY_TEST_PATH`: **/tmp**: Directory to create temporary files in when testing public keys using ssh-keygen, default is the system temporary directory.
- `SSH_KEYGEN_PATH`: **ssh-keygen**: Path to ssh-keygen, default is 'ssh-keygen' which means the shell is responsible for finding out which one to call. - `SSH_KEYGEN_PATH`: **\<empty\>**: Use `ssh-keygen` to parse public SSH keys. The value is passed to the shell. By default, Gitea does the parsing itself.
- `SSH_EXPOSE_ANONYMOUS`: **false**: Enable exposure of SSH clone URL to anonymous visitors, default is false. - `SSH_EXPOSE_ANONYMOUS`: **false**: Enable exposure of SSH clone URL to anonymous visitors, default is false.
- `SSH_PER_WRITE_TIMEOUT`: **30s**: Timeout for any write to the SSH connections. (Set to - `SSH_PER_WRITE_TIMEOUT`: **30s**: Timeout for any write to the SSH connections. (Set to
-1 to disable all timeouts.) -1 to disable all timeouts.)

View file

@ -179,7 +179,7 @@ func CheckPublicKeyString(content string) (_ string, err error) {
keyType string keyType string
length int length int
) )
if setting.SSH.StartBuiltinServer { if len(setting.SSH.KeygenPath) == 0 {
fnName = "SSHNativeParsePublicKey" fnName = "SSHNativeParsePublicKey"
keyType, length, err = SSHNativeParsePublicKey(content) keyType, length, err = SSHNativeParsePublicKey(content)
} else { } else {
@ -285,7 +285,12 @@ func SSHKeyGenParsePublicKey(key string) (string, int, error) {
} }
}() }()
stdout, stderr, err := process.GetManager().Exec("SSHKeyGenParsePublicKey", setting.SSH.KeygenPath, "-lf", tmpName) keygenPath := setting.SSH.KeygenPath
if len(keygenPath) == 0 {
keygenPath = "ssh-keygen"
}
stdout, stderr, err := process.GetManager().Exec("SSHKeyGenParsePublicKey", keygenPath, "-lf", tmpName)
if err != nil { if err != nil {
return "", 0, fmt.Errorf("fail to parse public key: %s - %s", err, stderr) return "", 0, fmt.Errorf("fail to parse public key: %s - %s", err, stderr)
} }

View file

@ -57,6 +57,14 @@ func Test_SSHParsePublicKey(t *testing.T) {
assert.Equal(t, tc.keyType, keyTypeK) assert.Equal(t, tc.keyType, keyTypeK)
assert.EqualValues(t, tc.length, lengthK) assert.EqualValues(t, tc.length, lengthK)
}) })
t.Run("SSHParseKeyNative", func(t *testing.T) {
keyTypeK, lengthK, err := SSHNativeParsePublicKey(tc.content)
if err != nil {
assert.Fail(t, "%v", err)
}
assert.Equal(t, tc.keyType, keyTypeK)
assert.EqualValues(t, tc.length, lengthK)
})
}) })
} }
} }

View file

@ -58,7 +58,7 @@ var SSH = struct {
ServerCiphers: []string{"chacha20-poly1305@openssh.com", "aes128-ctr", "aes192-ctr", "aes256-ctr", "aes128-gcm@openssh.com", "aes256-gcm@openssh.com"}, ServerCiphers: []string{"chacha20-poly1305@openssh.com", "aes128-ctr", "aes192-ctr", "aes256-ctr", "aes128-gcm@openssh.com", "aes256-gcm@openssh.com"},
ServerKeyExchanges: []string{"curve25519-sha256", "ecdh-sha2-nistp256", "ecdh-sha2-nistp384", "ecdh-sha2-nistp521", "diffie-hellman-group14-sha256", "diffie-hellman-group14-sha1"}, ServerKeyExchanges: []string{"curve25519-sha256", "ecdh-sha2-nistp256", "ecdh-sha2-nistp384", "ecdh-sha2-nistp521", "diffie-hellman-group14-sha256", "diffie-hellman-group14-sha1"},
ServerMACs: []string{"hmac-sha2-256-etm@openssh.com", "hmac-sha2-256", "hmac-sha1"}, ServerMACs: []string{"hmac-sha2-256-etm@openssh.com", "hmac-sha2-256", "hmac-sha1"},
KeygenPath: "ssh-keygen", KeygenPath: "",
MinimumKeySizeCheck: true, MinimumKeySizeCheck: true,
MinimumKeySizes: map[string]int{"ed25519": 256, "ed25519-sk": 256, "ecdsa": 256, "ecdsa-sk": 256, "rsa": 2047}, MinimumKeySizes: map[string]int{"ed25519": 256, "ed25519-sk": 256, "ecdsa": 256, "ecdsa-sk": 256, "rsa": 2047},
ServerHostKeys: []string{"ssh/gitea.rsa", "ssh/gogs.rsa"}, ServerHostKeys: []string{"ssh/gitea.rsa", "ssh/gogs.rsa"},
@ -134,7 +134,7 @@ func loadSSHFrom(rootCfg ConfigProvider) {
} }
} }
SSH.KeygenPath = sec.Key("SSH_KEYGEN_PATH").MustString("ssh-keygen") SSH.KeygenPath = sec.Key("SSH_KEYGEN_PATH").String()
SSH.Port = sec.Key("SSH_PORT").MustInt(22) SSH.Port = sec.Key("SSH_PORT").MustInt(22)
SSH.ListenPort = sec.Key("SSH_LISTEN_PORT").MustInt(SSH.Port) SSH.ListenPort = sec.Key("SSH_LISTEN_PORT").MustInt(SSH.Port)
SSH.UseProxyProtocol = sec.Key("SSH_SERVER_USE_PROXY_PROTOCOL").MustBool(false) SSH.UseProxyProtocol = sec.Key("SSH_SERVER_USE_PROXY_PROTOCOL").MustBool(false)