From 251946e32f1889283b3407cbe7dda528f8eb6e6a Mon Sep 17 00:00:00 2001 From: Gabriel Keller Date: Tue, 9 Jun 2026 12:06:50 -0400 Subject: [PATCH 1/3] fix(sync): use current team uid for team-delivered shared folder keys In `sync_down` `to_team`, each team-delivered shared-folder key is stored with `encrypter_uid = team_uid`, but `team_uid` is a stale closure variable from the preceding `for team in response.teams` loop, so it points at the LAST team in the response for every team processed. The key bytes are encrypted with the correct current team's key (`encrypt_aes_v2(sfkd, team_key)`), so the stored link is mislabeled: at vault rebuild `VaultData._decrypt_shared_folder_key` looks up the wrong team and AES-GCM raises InvalidTag, dropping the folder and making its records invisible. Use the current team being processed, matching the existing correct usage a few lines below (`s_team.team_uid = utils.base64_url_encode(sync_down_team.teamUid)`). Only affects users who belong to more than one team; single-team users happen to have the last team equal to their only team, which is why this went unnoticed. Co-authored-by: Cursor --- keepersdk-package/src/keepersdk/vault/sync_down.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keepersdk-package/src/keepersdk/vault/sync_down.py b/keepersdk-package/src/keepersdk/vault/sync_down.py index 9b7bec93..b41eff5a 100644 --- a/keepersdk-package/src/keepersdk/vault/sync_down.py +++ b/keepersdk-package/src/keepersdk/vault/sync_down.py @@ -264,7 +264,7 @@ def to_team(sync_down_team: SyncDown_pb2.Team) -> Optional[StorageTeam]: sshk = StorageSharedFolderKey() sshk.shared_folder_uid = sf_uid - sshk.encrypter_uid = team_uid + sshk.encrypter_uid = utils.base64_url_encode(sync_down_team.teamUid) sshk.key_type = StorageKeyType.TeamKey_AES_GCM sshk.shared_folder_key = crypto.encrypt_aes_v2(sfkd, team_key) sf_keys.append(sshk) From c1533ff964af750bd58cddff55ef06bb4cdfb8ca Mon Sep 17 00:00:00 2001 From: Gabriel Keller Date: Tue, 9 Jun 2026 12:06:50 -0400 Subject: [PATCH 2/3] fix(crypto): left-pad any short RSA ciphertext, not only key_size==2047 `decrypt_rsa` only prepends zero bytes when `rsa_key.key_size == 2047`. When Keeper delivers a ciphertext whose leading zero byte was stripped against a 2048-bit modulus, decryption fails because the input is shorter than the modulus. Pad any ciphertext shorter than the modulus length to the modulus size, matching keepercommander's `decrypt_rsa` behavior. Co-authored-by: Cursor --- keepersdk-package/src/keepersdk/crypto.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/keepersdk-package/src/keepersdk/crypto.py b/keepersdk-package/src/keepersdk/crypto.py index 5d3623b3..7b34faf2 100644 --- a/keepersdk-package/src/keepersdk/crypto.py +++ b/keepersdk-package/src/keepersdk/crypto.py @@ -129,10 +129,9 @@ def encrypt_rsa(data: bytes, rsa_key: rsa.RSAPublicKey) -> bytes: def decrypt_rsa(data: bytes, rsa_key: rsa.RSAPrivateKey) -> bytes: - if rsa_key.key_size == 2047: - remainder = len(data) % 256 - if remainder > 0: - data = b'\x00' * (256-remainder) + data + modulus_size = (rsa_key.key_size + 7) // 8 + if len(data) < modulus_size: + data = b'\x00' * (modulus_size - len(data)) + data return rsa_key.decrypt(data, PKCS1v15()) From 58530f95ae168809b4d558b773db6630bbc4c076 Mon Sep 17 00:00:00 2001 From: Gabriel Keller Date: Tue, 9 Jun 2026 12:23:13 -0400 Subject: [PATCH 3/3] test: cover team shared-folder-key uid fix and RSA short-ciphertext padding - test_sync_down: two teams each owning a distinct team-shared folder; asserts both folder keys resolve after sync. Reproduces the stale-team_uid bug (the folder owned by the non-last team decrypts with the wrong team key -> InvalidTag) and verifies the fix. - test_crypto: a leading-zero-stripped RSA ciphertext still decrypts, covering the generalized padding (previously only handled key_size == 2047). Co-authored-by: Cursor --- keepersdk-package/unit_tests/test_crypto.py | 18 +++++ .../unit_tests/test_sync_down.py | 67 ++++++++++++++++++- 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/keepersdk-package/unit_tests/test_crypto.py b/keepersdk-package/unit_tests/test_crypto.py index 56be70d0..a1ecbcd3 100644 --- a/keepersdk-package/unit_tests/test_crypto.py +++ b/keepersdk-package/unit_tests/test_crypto.py @@ -70,6 +70,24 @@ def test_encrypt_rsa(self): dec_data = crypto.decrypt_rsa(enc_data, prk) self.assertEqual(data, dec_data) + def test_decrypt_rsa_pads_leading_zero_stripped_ciphertext(self): + # Keeper can deliver an RSA ciphertext whose leading zero byte(s) were + # stripped; decrypt_rsa must left-pad back to the modulus length before + # decrypting (previously only handled when key_size == 2047). + puk = crypto.load_rsa_public_key(utils.base64_url_decode(_test_public_key)) + prk = crypto.load_rsa_private_key(utils.base64_url_decode(_test_private_key)) + data = b'nominal rsa padding regression' + + stripped = None + for _ in range(5000): + candidate = crypto.encrypt_rsa(data, puk) + if candidate[:1] == b'\x00': + stripped = candidate.lstrip(b'\x00') + break + self.assertIsNotNone(stripped, 'could not produce a leading-zero ciphertext') + self.assertLess(len(stripped), (puk.key_size + 7) // 8) + self.assertEqual(crypto.decrypt_rsa(stripped, prk), data) + def test_derive_key_hash_v1(self): password = 'q2rXmNBFeLwAEX55hVVTfg' salt = utils.base64_url_decode('Ozv5_XSBgw-XSrDosp8Y1A') diff --git a/keepersdk-package/unit_tests/test_sync_down.py b/keepersdk-package/unit_tests/test_sync_down.py index 4ac8a20d..b0427218 100644 --- a/keepersdk-package/unit_tests/test_sync_down.py +++ b/keepersdk-package/unit_tests/test_sync_down.py @@ -5,7 +5,7 @@ import data_vault from keepersdk import crypto, utils from keepersdk.proto import record_pb2 -from keepersdk.vault import vault_online, sync_down, memory_storage, record_type_management +from keepersdk.vault import vault_online, sync_down, memory_storage, record_type_management, vault_types from keepersdk.proto import SyncDown_pb2 @@ -108,6 +108,71 @@ def test_delete_team(self): self.assertIsNone(vault.vault_data.get_team(team_uid)) self.assertTrue(vault.vault_data.shared_folder_count < orig_sf_count) + def test_team_shared_folder_keys_resolve_per_team_not_last_team(self): + # Regression: to_team stored every team-delivered shared-folder key under + # the LAST team's uid (a stale closure variable) while encrypting the + # bytes with the CURRENT team's key, so a folder owned by a non-last team + # failed to decrypt at rebuild. With two teams each owning a distinct + # folder, both keys must resolve correctly. + sf1 = vault_types.SharedFolder() + sf1.shared_folder_uid = utils.generate_uid() + sf1.name = 'Team1 Folder' + sf2 = vault_types.SharedFolder() + sf2.shared_folder_uid = utils.generate_uid() + sf2.name = 'Team2 Folder' + + sfd1, sfk1 = data_vault.generate_shared_folder(sf1, False) + sfd2, sfk2 = data_vault.generate_shared_folder(sf2, False) + + team1 = vault_types.Team() + team1.team_uid = utils.generate_uid() + team1.name = 'Team1' + team1.rsa_private_key = crypto.load_rsa_private_key(utils.base64_url_decode(data_vault.TeamPrivateKey)) + team2 = vault_types.Team() + team2.team_uid = utils.generate_uid() + team2.name = 'Team2' + team2.rsa_private_key = crypto.load_rsa_private_key(utils.base64_url_decode(data_vault.TeamPrivateKey)) + + t1, sft1, _ = data_vault.generate_team(team1, [(sfd1, sfk1)]) + t2, sft2, _ = data_vault.generate_team(team2, [(sfd2, sfk2)]) + + rs = SyncDown_pb2.SyncDownResponse() + rs.continuationToken = crypto.get_random_bytes(64) + rs.hasMore = False + rs.cacheStatus = SyncDown_pb2.CLEAR + rs.sharedFolders.extend([sfd1, sfd2]) + rs.sharedFolderTeams.extend([*sft1, *sft2]) + rs.teams.extend([t1, t2]) + user = SyncDown_pb2.User() + user.username = data_vault.UserName + user.accountUid = data_vault.AccountUid + rs.users.append(user) + + def execute_auth_rest(endpoint, request, response_type): + if endpoint == 'vault/sync_down': + return rs + if endpoint == 'vault/get_record_types': + rts = record_pb2.RecordTypesResponse() + rts.standardCounter = 1 + rt = record_pb2.RecordType() + rt.scope = record_pb2.RecordTypeScope.RT_STANDARD + rt.recordTypeId = 1 + rt.content = data_vault.RecordTypes + rts.recordTypes.append(rt) + return rts + raise Exception(f'Endpoint "{endpoint}" not supported') + + auth = data_vault.get_connected_auth() + mock = MagicMock() + mock.side_effect = execute_auth_rest + auth.execute_auth_rest = mock + + vault = vault_online.VaultOnline(auth, memory_storage.InMemoryVaultStorage()) + vault.sync_down() + + self.assertEqual(vault.vault_data.get_shared_folder_key(sf1.shared_folder_uid), sfk1) + self.assertEqual(vault.vault_data.get_shared_folder_key(sf2.shared_folder_uid), sfk2) + def test_delete_shared_folder(self): vault = get_populated_vault() orig_sf_count = vault.vault_data.shared_folder_count