From ea859a0844274a1eaa820a496a91d7649ca1a2a0 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Wed, 21 Oct 2026 20:03:14 -0700 Subject: [PATCH] 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> --- weed/sftpd/auth/password.go | 5 ++-- weed/sftpd/auth/password_test.go | 43 ++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 weed/sftpd/auth/password_test.go diff --git a/weed/sftpd/auth/password.go b/weed/sftpd/auth/password.go index 21216d3ff8..7498adc7e0 100644 --- a/weed/sftpd/auth/password.go +++ b/weed/sftpd/auth/password.go @@ -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, diff --git a/weed/sftpd/auth/password_test.go b/weed/sftpd/auth/password_test.go new file mode 100644 index 0000000000..b54f6d29ce --- /dev/null +++ b/weed/sftpd/auth/password_test.go @@ -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"]) + } +}