diff --git a/cmd/serve/ftp/ftp.go b/cmd/serve/ftp/ftp.go index e5f574ae1..6d15d4cfc 100644 --- a/cmd/serve/ftp/ftp.go +++ b/cmd/serve/ftp/ftp.go @@ -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 diff --git a/cmd/serve/ftp/ftp_test.go b/cmd/serve/ftp/ftp_test.go index 83255fc77..df6655ac5 100644 --- a/cmd/serve/ftp/ftp_test.go +++ b/cmd/serve/ftp/ftp_test.go @@ -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")