sftp: reject empty passwords in PasswordAuthenticator (#11938)

* sftp: reject empty passwords in PasswordAuthenticator

FileStore.ValidatePassword already refuses a zero-length stored or
supplied password, but the guard belongs at the authenticator too: an
empty password is never valid, and checking before the store call keeps
any user.Store implementation without its own empty-secret check from
matching on zero-length input.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* sftp: keep the brute-force delay on empty-password failures

The early rejection returned before the shared failure path, so an empty
password failed instantly while other failures slept 100-199 ms. Folding
the length check into the store call keeps the store untouched and every
failed attempt equally slow.

---------

Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
Chris LuandDevin authored and GitHub committed 2026-10-21 20:03:14 -07:00
1 parent c0152e6be1
commit ea859a0844
2 files changed
+46 -2

No files matched your search

+3 -2
View File
@@ -37,8 +37,9 @@ func (a *PasswordAuthenticator) Authenticate(conn ssh.ConnMetadata, password []b
return nil, fmt.Errorf("password authentication disabled")
}
// Validate password against user store
if a.userStore.ValidatePassword(username, password) {
// An empty password is never valid; skip the store so a store without its
// own empty-secret check cannot match on zero-length input.
if len(password) != 0 && a.userStore.ValidatePassword(username, password) {
return &ssh.Permissions{
Extensions: map[string]string{
"username": username,
+43
View File
@@ -0,0 +1,43 @@
package auth
import "testing"
// acceptAllStore records whether the store was consulted and accepts any password.
type acceptAllStore struct {
stubStore
called bool
}
func (s *acceptAllStore) ValidatePassword(string, []byte) bool {
s.called = true
return true
}
// An empty password must be rejected before the store is consulted, so a store
// without its own empty-secret check cannot authenticate on zero-length input.
func TestPasswordAuthenticatorRejectsEmptyPassword(t *testing.T) {
store := &acceptAllStore{}
a := NewPasswordAuthenticator(store, true)
for _, password := range [][]byte{nil, {}} {
if _, err := a.Authenticate(&fakeConnMetadata{user: "alice"}, password); err == nil {
t.Fatalf("expected empty password %v to be rejected", password)
}
}
if store.called {
t.Fatal("store must not be consulted for an empty password")
}
}
func TestPasswordAuthenticatorConsultsStore(t *testing.T) {
store := &acceptAllStore{}
a := NewPasswordAuthenticator(store, true)
perms, err := a.Authenticate(&fakeConnMetadata{user: "alice"}, []byte("s3cret"))
if err != nil {
t.Fatalf("expected non-empty password to reach the store: %v", err)
}
if perms.Extensions["username"] != "alice" {
t.Fatalf("expected username extension alice, got %q", perms.Extensions["username"])
}
}