Compare commits

..
Author SHA1 Message Date
Chris Lu e3e087f18e fix(sftpd): harden bcrypt migration and auth path
- SetPassword now returns an error instead of silently truncating passwords
  >72 bytes (the prior fallback could also panic for short passwords when
  bcrypt returned any non-ErrPasswordTooLong error).
- CheckPassword is now pure and reports (ok, legacy); migration moves to
  FileStore.migrateLegacyPassword under a write lock to fix a data race on
  concurrent logins and a double-check avoids redundant re-hashing.
- ValidatePassword only rewrites the user store when a migration actually
  happened, eliminating a full file rewrite on every successful login.
- Legacy plaintext comparison uses subtle.ConstantTimeCompare again,
  restoring the pre-PR constant-time property.
- CreateUser propagates SetPassword errors and releases the write lock
  before calling saveUsers to avoid recursive RWMutex locking.
- Add unit tests covering new-user hashing, legacy migration, no-rewrite
  on normal login, wrong/empty passwords, oversize password rejection,
  and concurrent migration under -race.
2026-04-22 20:09:33 -07:00
Chris Lu 9b077d6f1f fix(sftpd): replace plaintext password storage with bcrypt hashing
SFTP user passwords were stored as plaintext in the JSON user store.
This replaces plaintext storage with bcrypt hashing:

- Add HashedPassword field to User struct for bcrypt hashes
- SetPassword() now hashes with bcrypt.DefaultCost
- CheckPassword() verifies against bcrypt hash with transparent
  legacy migration: plaintext passwords auto-upgrade on successful login
- CreateUser() uses SetPassword() instead of direct assignment
- ValidatePassword() delegates to CheckPassword() and persists
  migrated hashes

Existing users with plaintext passwords are transparently migrated
to bcrypt on their next successful authentication.
2026-04-01 21:28:24 -07:00
Chris LuandGitHub 4287b7b12a Add manual trigger to Rust volume server release build workflow (#8873)
* Add manual trigger to Rust volume server release build workflow

When triggered manually via workflow_dispatch, binaries are uploaded as
downloadable workflow artifacts instead of release assets. On tag push
the existing release upload behavior is unchanged.

* Vendor OpenSSL for cross-compilation of Rust volume server

The aarch64-unknown-linux-gnu build fails because openssl-sys cannot
find OpenSSL via pkg-config when cross-compiling. Adding openssl with
the vendored feature builds OpenSSL from source, fixing the issue.

* Fix aarch64 cross-compilation: install libssl-dev:arm64 instead of vendoring OpenSSL

The vendored OpenSSL feature breaks the S3 tier unit test by altering
the TLS stack behavior. Instead, install the aarch64 OpenSSL dev
libraries and point the build at them via OPENSSL_DIR/LIB_DIR/INCLUDE_DIR.
2026-04-01 20:31:35 -07:00
4 changed files with 316 additions and 29 deletions
+39 -1
View File
@@ -39,8 +39,16 @@ jobs:
- name: Install cross-compilation tools
if: matrix.cross
run: |
sudo apt-get install -y gcc-aarch64-linux-gnu
sudo dpkg --add-architecture arm64
sudo sed -i 's/^deb /deb [arch=amd64] /' /etc/apt/sources.list
echo "deb [arch=arm64] http://ports.ubuntu.com/ jammy main restricted universe multiverse" | sudo tee /etc/apt/sources.list.d/arm64.list
echo "deb [arch=arm64] http://ports.ubuntu.com/ jammy-updates main restricted universe multiverse" | sudo tee -a /etc/apt/sources.list.d/arm64.list
sudo apt-get update
sudo apt-get install -y gcc-aarch64-linux-gnu libssl-dev:arm64
echo "CARGO_TARGET_AARCH64_UNKNOWN_LINUX_GNU_LINKER=aarch64-linux-gnu-gcc" >> "$GITHUB_ENV"
echo "OPENSSL_DIR=/usr" >> "$GITHUB_ENV"
echo "OPENSSL_INCLUDE_DIR=/usr/include" >> "$GITHUB_ENV"
echo "OPENSSL_LIB_DIR=/usr/lib/aarch64-linux-gnu" >> "$GITHUB_ENV"
- name: Cache cargo registry and target
uses: actions/cache@v5
@@ -80,6 +88,7 @@ jobs:
rm weed-volume-normal
- name: Upload release assets
if: startsWith(github.ref, 'refs/tags/')
uses: softprops/action-gh-release@v2
with:
files: |
@@ -88,6 +97,15 @@ jobs:
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- name: Upload artifacts
if: ${{ !startsWith(github.ref, 'refs/tags/') }}
uses: actions/upload-artifact@v4
with:
name: rust-volume-${{ matrix.asset_suffix }}
path: |
weed-volume_large_disk_${{ matrix.asset_suffix }}.tar.gz
weed-volume_${{ matrix.asset_suffix }}.tar.gz
build-rust-volume-darwin:
permissions:
contents: write
@@ -147,6 +165,7 @@ jobs:
rm weed-volume-normal
- name: Upload release assets
if: startsWith(github.ref, 'refs/tags/')
uses: softprops/action-gh-release@v2
with:
files: |
@@ -155,6 +174,15 @@ jobs:
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- name: Upload artifacts
if: ${{ !startsWith(github.ref, 'refs/tags/') }}
uses: actions/upload-artifact@v4
with:
name: rust-volume-${{ matrix.asset_suffix }}
path: |
weed-volume_large_disk_${{ matrix.asset_suffix }}.tar.gz
weed-volume_${{ matrix.asset_suffix }}.tar.gz
build-rust-volume-windows:
permissions:
contents: write
@@ -206,6 +234,7 @@ jobs:
rm weed-volume-normal.exe
- name: Upload release assets
if: startsWith(github.ref, 'refs/tags/')
uses: softprops/action-gh-release@v2
with:
files: |
@@ -213,3 +242,12 @@ jobs:
weed-volume_windows_amd64.zip
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- name: Upload artifacts
if: ${{ !startsWith(github.ref, 'refs/tags/') }}
uses: actions/upload-artifact@v4
with:
name: rust-volume-windows_amd64
path: |
weed-volume_large_disk_windows_amd64.zip
weed-volume_windows_amd64.zip
+45 -18
View File
@@ -8,6 +8,7 @@ import (
"path"
"sync"
"github.com/seaweedfs/seaweedfs/weed/glog"
"golang.org/x/crypto/ssh"
)
@@ -147,15 +148,45 @@ func (s *FileStore) GetUser(username string) (*User, error) {
return user, nil
}
// ValidatePassword checks if the password is valid for the user
// ValidatePassword checks if the password is valid for the user. Legacy
// plaintext entries are transparently upgraded to bcrypt on successful match.
func (s *FileStore) ValidatePassword(username string, password []byte) bool {
user, err := s.GetUser(username)
if err != nil {
return false
}
// Compare plaintext password using constant time comparison for security
return subtle.ConstantTimeCompare([]byte(user.Password), password) == 1
ok, legacy := user.CheckPassword(string(password))
if !ok {
return false
}
if legacy {
s.migrateLegacyPassword(user, string(password))
}
return true
}
// migrateLegacyPassword re-hashes a legacy plaintext password under a write
// lock and persists the user store. The FileStore mutex guards both the
// re-hash (to avoid a data race with concurrent logins) and the double-check
// that another goroutine hasn't already migrated. Failures are logged but
// not surfaced — auth has already succeeded and the next login will retry.
func (s *FileStore) migrateLegacyPassword(user *User, password string) {
s.mu.Lock()
if user.HashedPassword != "" {
s.mu.Unlock()
return
}
if err := user.SetPassword(password); err != nil {
s.mu.Unlock()
glog.V(0).Infof("sftpd: failed to upgrade legacy password for %s: %v", user.Username, err)
return
}
s.mu.Unlock()
if err := s.saveUsers(); err != nil {
glog.V(0).Infof("sftpd: failed to persist upgraded password for %s: %v", user.Username, err)
}
}
// ValidatePublicKey checks if the public key is valid for the user
@@ -239,25 +270,21 @@ func (s *FileStore) ListUsers() ([]string, error) {
// CreateUser creates a new user with the given username and password
func (s *FileStore) CreateUser(username, password string) (*User, error) {
s.mu.Lock()
defer s.mu.Unlock()
// Check if user already exists
if _, exists := s.users[username]; exists {
return nil, fmt.Errorf("user already exists: %s", username)
}
// Create new user
user := NewUser(username)
// Store plaintext password
user.Password = password
// Add default permissions
if err := user.SetPassword(password); err != nil {
return nil, err
}
user.Permissions[user.HomeDir] = []string{"all"}
// Save the user
s.mu.Lock()
if _, exists := s.users[username]; exists {
s.mu.Unlock()
return nil, fmt.Errorf("user already exists: %s", username)
}
s.users[username] = user
s.mu.Unlock()
// saveUsers acquires RLock, so we must release the write lock above first.
if err := s.saveUsers(); err != nil {
return nil, err
}
+195
View File
@@ -0,0 +1,195 @@
package user
import (
"bytes"
"encoding/json"
"os"
"path/filepath"
"strings"
"sync"
"testing"
"golang.org/x/crypto/bcrypt"
)
func newTempStore(t *testing.T) *FileStore {
t.Helper()
dir := t.TempDir()
s, err := NewFileStore(filepath.Join(dir, "users.json"))
if err != nil {
t.Fatalf("NewFileStore: %v", err)
}
return s
}
func readUsers(t *testing.T, s *FileStore) []*User {
t.Helper()
data, err := os.ReadFile(s.filePath)
if err != nil {
t.Fatalf("read store file: %v", err)
}
var users []*User
if err := json.Unmarshal(data, &users); err != nil {
t.Fatalf("unmarshal store file: %v", err)
}
return users
}
func TestCreateUserHashesPassword(t *testing.T) {
s := newTempStore(t)
u, err := s.CreateUser("alice", "s3cret")
if err != nil {
t.Fatalf("CreateUser: %v", err)
}
if u.Password != "" {
t.Errorf("plaintext Password leaked: %q", u.Password)
}
if u.HashedPassword == "" {
t.Fatal("HashedPassword empty")
}
if err := bcrypt.CompareHashAndPassword([]byte(u.HashedPassword), []byte("s3cret")); err != nil {
t.Fatalf("hash does not match password: %v", err)
}
stored := readUsers(t, s)
if len(stored) != 1 || stored[0].Password != "" {
t.Errorf("plaintext persisted: %+v", stored)
}
}
func TestValidatePasswordMigratesLegacyPlaintext(t *testing.T) {
s := newTempStore(t)
legacy := NewUser("bob")
legacy.Password = "hunter2"
if err := s.SaveUser(legacy); err != nil {
t.Fatalf("SaveUser: %v", err)
}
if !s.ValidatePassword("bob", []byte("hunter2")) {
t.Fatal("legacy login rejected")
}
u, err := s.GetUser("bob")
if err != nil {
t.Fatalf("GetUser: %v", err)
}
if u.Password != "" {
t.Errorf("legacy Password not cleared: %q", u.Password)
}
if u.HashedPassword == "" {
t.Fatal("HashedPassword not populated after migration")
}
stored := readUsers(t, s)
if len(stored) != 1 || stored[0].Password != "" || stored[0].HashedPassword == "" {
t.Errorf("migration not persisted: %+v", stored)
}
}
// Successful logins that don't migrate must not rewrite the store file.
// We detect a rewrite by replacing the file contents with a sentinel after
// CreateUser and verifying the sentinel is untouched after a valid login.
func TestValidatePasswordNoRewriteWithoutMigration(t *testing.T) {
s := newTempStore(t)
if _, err := s.CreateUser("carol", "pw"); err != nil {
t.Fatalf("CreateUser: %v", err)
}
sentinel := []byte("SENTINEL_NOT_REAL_JSON")
if err := os.WriteFile(s.filePath, sentinel, 0600); err != nil {
t.Fatalf("write sentinel: %v", err)
}
if !s.ValidatePassword("carol", []byte("pw")) {
t.Fatal("login rejected")
}
got, err := os.ReadFile(s.filePath)
if err != nil {
t.Fatalf("read: %v", err)
}
if !bytes.Equal(got, sentinel) {
t.Errorf("store file was rewritten on non-migrating login: got %q", got)
}
}
func TestValidatePasswordWrongPassword(t *testing.T) {
s := newTempStore(t)
if _, err := s.CreateUser("dave", "correct"); err != nil {
t.Fatalf("CreateUser: %v", err)
}
if s.ValidatePassword("dave", []byte("wrong")) {
t.Error("wrong password accepted for bcrypt user")
}
legacy := NewUser("eve")
legacy.Password = "plain"
if err := s.SaveUser(legacy); err != nil {
t.Fatal(err)
}
if s.ValidatePassword("eve", []byte("wrong")) {
t.Error("wrong password accepted for legacy user")
}
if s.ValidatePassword("eve", []byte("")) {
t.Error("empty password accepted for legacy user")
}
}
func TestValidatePasswordUnknownUser(t *testing.T) {
s := newTempStore(t)
if s.ValidatePassword("nobody", []byte("whatever")) {
t.Error("unknown user accepted")
}
}
func TestCheckPasswordEmptyCredentials(t *testing.T) {
u := NewUser("frank")
if ok, _ := u.CheckPassword(""); ok {
t.Error("empty password accepted on user with no credentials")
}
if ok, _ := u.CheckPassword("x"); ok {
t.Error("password accepted on user with no credentials")
}
}
func TestSetPasswordTooLong(t *testing.T) {
u := NewUser("grace")
if err := u.SetPassword(strings.Repeat("a", 73)); err == nil {
t.Error("expected error for >72 byte password")
}
if u.HashedPassword != "" {
t.Errorf("HashedPassword set despite error: %q", u.HashedPassword)
}
}
// Concurrent ValidatePassword calls on a legacy user must not race on the
// User fields and must converge to a single persisted bcrypt hash.
func TestValidatePasswordConcurrentMigration(t *testing.T) {
s := newTempStore(t)
legacy := NewUser("heidi")
legacy.Password = "p"
if err := s.SaveUser(legacy); err != nil {
t.Fatal(err)
}
const N = 16
var wg sync.WaitGroup
wg.Add(N)
for i := 0; i < N; i++ {
go func() {
defer wg.Done()
if !s.ValidatePassword("heidi", []byte("p")) {
t.Error("login rejected")
}
}()
}
wg.Wait()
u, err := s.GetUser("heidi")
if err != nil {
t.Fatalf("GetUser: %v", err)
}
if u.Password != "" {
t.Errorf("plaintext not cleared after concurrent migration: %q", u.Password)
}
if err := bcrypt.CompareHashAndPassword([]byte(u.HashedPassword), []byte("p")); err != nil {
t.Errorf("final hash invalid: %v", err)
}
}
+37 -10
View File
@@ -2,19 +2,24 @@
package user
import (
"crypto/subtle"
"fmt"
"math/rand/v2"
"path/filepath"
"golang.org/x/crypto/bcrypt"
)
// User represents an SFTP user with authentication and permission details
type User struct {
Username string // Username for authentication
Password string // Plaintext password
PublicKeys []string // Authorized public keys
HomeDir string // User's home directory
Permissions map[string][]string // path -> permissions (read, write, list, etc.)
Uid uint32 // User ID for file ownership
Gid uint32 // Group ID for file ownership
Username string `json:"Username"`
HashedPassword string `json:"HashedPassword"` // bcrypt hash
Password string `json:"Password,omitempty"` // deprecated: plaintext, migrated to HashedPassword on first successful login
PublicKeys []string `json:"PublicKeys,omitempty"`
HomeDir string `json:"HomeDir"`
Permissions map[string][]string `json:"Permissions,omitempty"`
Uid uint32 `json:"Uid"`
Gid uint32 `json:"Gid"`
}
// NewUser creates a new user with default settings
@@ -33,9 +38,31 @@ func NewUser(username string) *User {
}
}
// SetPassword sets a plaintext password for the user
func (u *User) SetPassword(password string) {
u.Password = password
// SetPassword hashes and stores the password using bcrypt. It clears any
// legacy plaintext value. Returns an error if the password cannot be hashed
// (e.g. exceeds bcrypt's 72-byte limit).
func (u *User) SetPassword(password string) error {
hash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost)
if err != nil {
return fmt.Errorf("hash password: %w", err)
}
u.HashedPassword = string(hash)
u.Password = ""
return nil
}
// CheckPassword verifies a password against the stored credentials without
// mutating the receiver. The legacy return is true when the match succeeded
// via the deprecated plaintext field; the caller is expected to re-hash and
// persist the password under an appropriate lock.
func (u *User) CheckPassword(password string) (ok bool, legacy bool) {
if u.HashedPassword != "" {
return bcrypt.CompareHashAndPassword([]byte(u.HashedPassword), []byte(password)) == nil, false
}
if u.Password != "" && subtle.ConstantTimeCompare([]byte(u.Password), []byte(password)) == 1 {
return true, true
}
return false, false
}
// AddPublicKey adds a public key to the user