diff --git a/fs/config/crypt.go b/fs/config/crypt.go index c25595900..158eaa220 100644 --- a/fs/config/crypt.go +++ b/fs/config/crypt.go @@ -15,6 +15,7 @@ import ( "strings" "golang.org/x/crypto/nacl/secretbox" + "golang.org/x/text/unicode/norm" "github.com/rclone/rclone/fs" "github.com/rclone/rclone/fs/config/obscure" @@ -274,6 +275,11 @@ func SetConfigPassword(password string) error { if err != nil { return err } + // Normalize the config encryption password to reduce weird + // variations so that the same password always derives the same + // key. This is safe for the master password as it is only ever + // used to derive the key, never sent anywhere verbatim. + password = norm.NFKC.String(password) // Create SHA256 has of the password sha := sha256.New() _, err = sha.Write([]byte("[" + password + "][rclone-config]")) diff --git a/fs/config/ui.go b/fs/config/ui.go index 1a897aefe..ccd247e03 100644 --- a/fs/config/ui.go +++ b/fs/config/ui.go @@ -24,7 +24,6 @@ import ( "github.com/rclone/rclone/fs/driveletter" "github.com/rclone/rclone/fs/fspath" "github.com/rclone/rclone/lib/terminal" - "golang.org/x/text/unicode/norm" ) var ( @@ -763,7 +762,11 @@ func suppressConfirm(ctx context.Context) context.Context { return newCtx } -// checkPassword normalises and validates the password +// checkPassword validates the password. +// +// It deliberately does not alter the password (e.g. by Unicode +// normalization) - the password is stored verbatim so that what the +// user typed is exactly what is obscured and later sent to the backend. func checkPassword(password string) (string, error) { if !utf8.ValidString(password) { return "", errors.New("password contains invalid utf8 characters") @@ -774,8 +777,6 @@ func checkPassword(password string) (string, error) { if len(password) != len(trimmedPassword) { _, _ = fmt.Fprintln(os.Stderr, "Your password contains leading/trailing whitespace - in previous versions of rclone this was stripped") } - // Normalize to reduce weird variations. - password = norm.NFKC.String(password) if len(password) == 0 || len(trimmedPassword) == 0 { return "", errors.New("no characters in password") } diff --git a/fs/config/ui_internal_test.go b/fs/config/ui_internal_test.go new file mode 100644 index 000000000..18b130c49 --- /dev/null +++ b/fs/config/ui_internal_test.go @@ -0,0 +1,31 @@ +package config + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCheckPassword(t *testing.T) { + // Empty password should give an error + _, err := checkPassword("") + require.Error(t, err) + + // Whitespace only should give an error + _, err = checkPassword(" \t ") + require.Error(t, err) + + // Invalid utf8 should give an error + _, err = checkPassword(string([]byte{0xff, 0xfe, 0xfd}) + "abc") + require.Error(t, err) + + // Passwords must be returned verbatim, in particular without + // Unicode normalization, so that obscured backend passwords reveal + // to exactly what the user typed - see #9507. NFKC normalization + // would turn ª (U+00AA) into a. + pw := "ËQÖ4];giª±;