serve ftp: use constant time comparison for password check GHSA-mfvx-7rcj-9m5g
The builtin authentication compared the configured username and password with ==, whose run time depends on how much of the value matches, giving a timing side-channel that could in principle help guess the password. serve sftp, serve s3 and the auth proxy already use subtle.ConstantTimeCompare so bring serve ftp in line with them. An empty configured password still accepts any password. Addresses GHSA-mfvx-7rcj-9m5g finding 4.
This commit is contained in:
@@ -5,6 +5,7 @@ package ftp
|
||||
|
||||
import (
|
||||
"context"
|
||||
"crypto/subtle"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
@@ -333,7 +334,13 @@ func (d *driver) CheckPasswd(sctx *ftp.Context, user, pass string) (ok bool, err
|
||||
d.userPass[user] = oPass
|
||||
d.userPassMu.Unlock()
|
||||
} else {
|
||||
ok = d.opt.User == user && (d.opt.Pass == "" || d.opt.Pass == pass)
|
||||
userOK := subtle.ConstantTimeCompare([]byte(d.opt.User), []byte(user))
|
||||
// No password configured means any password is accepted
|
||||
passOK := 1
|
||||
if d.opt.Pass != "" {
|
||||
passOK = subtle.ConstantTimeCompare([]byte(d.opt.Pass), []byte(pass))
|
||||
}
|
||||
ok = (userOK & passOK) == 1
|
||||
if !ok {
|
||||
fs.Infof(nil, "login failed: bad credentials")
|
||||
return false, nil
|
||||
|
||||
@@ -70,6 +70,36 @@ func TestFTP(t *testing.T) {
|
||||
servetest.Run(t, "ftp", start)
|
||||
}
|
||||
|
||||
// TestCheckPasswd checks the builtin authentication accepts only the
|
||||
// configured credentials, with an empty configured password accepting any
|
||||
// password.
|
||||
func TestCheckPasswd(t *testing.T) {
|
||||
for _, test := range []struct {
|
||||
name string
|
||||
optUser string
|
||||
optPass string
|
||||
user string
|
||||
pass string
|
||||
want bool
|
||||
}{
|
||||
{name: "good", optUser: "user", optPass: "pass", user: "user", pass: "pass", want: true},
|
||||
{name: "bad-pass", optUser: "user", optPass: "pass", user: "user", pass: "PASS", want: false},
|
||||
{name: "bad-user", optUser: "user", optPass: "pass", user: "USER", pass: "pass", want: false},
|
||||
{name: "wrong-length-pass", optUser: "user", optPass: "pass", user: "user", pass: "pass2", want: false},
|
||||
{name: "empty-configured-pass", optUser: "user", optPass: "", user: "user", pass: "anything", want: true},
|
||||
{name: "empty-configured-pass-bad-user", optUser: "user", optPass: "", user: "USER", pass: "anything", want: false},
|
||||
} {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
d := &driver{}
|
||||
d.opt.User = test.optUser
|
||||
d.opt.Pass = test.optPass
|
||||
ok, err := d.CheckPasswd(nil, test.user, test.pass)
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, test.want, ok)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestRc(t *testing.T) {
|
||||
if israce.Enabled {
|
||||
t.Skip("Skipping under race detector as underlying library is racy")
|
||||
|
||||
Reference in New Issue
Block a user