config: fix normalization when obscuring passwords - fixes #9507

Interactively-entered passwords were run through NFKC Unicode
normalization before being obscured, which silently rewrote characters
such as ª (U+00AA) to a. The obscured password then revealed to
something different from what the user typed confusing everyone.

Normalization is only needed for the config encryption master
password, so apply it there (in SetConfigPassword) rather than in the
shared checkPassword used for backend password options.

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]"))

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")
 	}

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ª±;<H'õPVzú6j¢ÔBí}×_-D%}"
+	got, err := checkPassword(pw)
+	require.NoError(t, err)
+	assert.Equal(t, pw, got)
+}