From ffcaffa32f8489cb320fc7afdca441dac2d42466 Mon Sep 17 00:00:00 2001 From: greatroar <61184462+greatroar@users.noreply.github.com> Date: Sat, 27 Feb 2021 08:57:12 +0100 Subject: [PATCH] lib/protocol: Optimize encrypted filename handling + make it more strict (#7408) --- lib/protocol/encryption.go | 21 +++++++++++------ lib/protocol/encryption_test.go | 40 ++++++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/lib/protocol/encryption.go b/lib/protocol/encryption.go index e4612cd04..71a853446 100644 --- a/lib/protocol/encryption.go +++ b/lib/protocol/encryption.go @@ -356,19 +356,23 @@ func DecryptFileInfo(fi FileInfo, folderKey *[keySize]byte) (FileInfo, error) { return decFI, nil } +var base32Hex = base32.HexEncoding.WithPadding(base32.NoPadding) + // encryptName encrypts the given string in a deterministic manner (the // result is always the same for any given string) and encodes it in a // filesystem-friendly manner. func encryptName(name string, key *[keySize]byte) string { enc := encryptDeterministic([]byte(name), key, nil) - b32enc := base32.HexEncoding.WithPadding(base32.NoPadding).EncodeToString(enc) - return slashify(b32enc) + return slashify(base32Hex.EncodeToString(enc)) } // decryptName decrypts a string from encryptName func decryptName(name string, key *[keySize]byte) (string, error) { - name = deslashify(name) - bs, err := base32.HexEncoding.WithPadding(base32.NoPadding).DecodeString(name) + name, err := deslashify(name) + if err != nil { + return "", err + } + bs, err := base32Hex.DecodeString(name) if err != nil { return "", err } @@ -524,9 +528,12 @@ func slashify(s string) string { // deslashify removes slashes and encrypted file extensions from the string. // This is the inverse of slashify(). -func deslashify(s string) string { - s = strings.ReplaceAll(s, encryptedDirExtension, "") - return strings.ReplaceAll(s, "/", "") +func deslashify(s string) (string, error) { + if len(s) == 0 || !strings.HasPrefix(s[1:], encryptedDirExtension) { + return "", fmt.Errorf("invalid encrypted path: %q", s) + } + s = s[:1] + s[1+len(encryptedDirExtension):] + return strings.ReplaceAll(s, "/", ""), nil } type rawResponse struct { diff --git a/lib/protocol/encryption_test.go b/lib/protocol/encryption_test.go index f9c9d94ea..2909f30e0 100644 --- a/lib/protocol/encryption_test.go +++ b/lib/protocol/encryption_test.go @@ -8,7 +8,9 @@ package protocol import ( "bytes" + "fmt" "reflect" + "regexp" "strings" "testing" @@ -16,11 +18,28 @@ import ( ) func TestEnDecryptName(t *testing.T) { + pattern := regexp.MustCompile( + fmt.Sprintf("^[0-9A-V]%s/[0-9A-V]{2}/([0-9A-V]{%d}/)*[0-9A-V]{1,%d}$", + regexp.QuoteMeta(encryptedDirExtension), + maxPathComponent, maxPathComponent-1)) + + makeName := func(n int) string { + b := make([]byte, n) + for i := range b { + b[i] = byte('a' + i%26) + } + return string(b) + } + var key [32]byte cases := []string{ "", "foo", "a longer name/with/slashes and spaces", + makeName(maxPathComponent), + makeName(1 + maxPathComponent), + makeName(2 * maxPathComponent), + makeName(1 + 2*maxPathComponent), } for _, tc := range cases { var prev string @@ -33,6 +52,11 @@ func TestEnDecryptName(t *testing.T) { if tc != "" && strings.Contains(enc, tc) { t.Error("shouldn't contain plaintext") } + if !pattern.MatchString(enc) { + t.Fatalf("encrypted name %s doesn't match %s", + enc, pattern) + } + dec, err := decryptName(enc, &key) if err != nil { t.Error(err) @@ -40,7 +64,21 @@ func TestEnDecryptName(t *testing.T) { if dec != tc { t.Error("mismatch after decryption") } - t.Log(enc) + t.Logf("%q encrypts as %q", tc, enc) + } + } +} + +func TestDecryptNameInvalid(t *testing.T) { + key := new([32]byte) + for _, c := range []string{ + "T.syncthing-enc/OD", + "T.syncthing-enc/OD/", + "T.wrong-extension/OD/PHVDD67S7FI2K5QQMPSOFSK", + "OD/PHVDD67S7FI2K5QQMPSOFSK", + } { + if _, err := decryptName(c, key); err == nil { + t.Errorf("no error for %q", c) } } }