Skip to content

fix: allow multi-tenant queries with allow_plaintext #1240

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 31, 2024
Merged

fix: allow multi-tenant queries with allow_plaintext #1240

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a5a4aca
fix: allow multi-tenant queries with allow_plaintext
ajewellamz Jul 31, 2024
46b0aaf
m
ajewellamz Jul 31, 2024
99f9561
what about this?
seebees Jul 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions DynamoDbEncryption/dafny/DynamoDbEncryptionTransforms/src/QueryTransform.dfy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,10 @@ module QueryTransform {
//# with the resulting decrypted [DynamoDB Item](./decrypt-item.md#dynamodb-item-1).
var decryptInput := EncTypes.DecryptItemInput(encryptedItem := encryptedItems[x]);
var decryptRes := tableConfig.itemEncryptor.DecryptItem(decryptInput);

var decrypted :- MapError(decryptRes);
if keyId.KeyId? {
:- Need(decrypted.parsedHeader.Some?, E("Decrypted query result has no parsed header."));

// No parsed header is ok, because it means ALLOW_PLAINTEXT_READ and a plain text item
if keyId.KeyId? && decrypted.parsedHeader.Some? {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is not clear to me is how decrypted.parsedHeader.None? will fail if ALLOW_PLAINTEXT_READ is false.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the opposite.
If ALLOW_PLAINTEXT_READ is true, and the item read is plain text, then parsedHeader will be None.
In that case, we DO want to return the item, whereas the existing code will return an error.

Copy link
Contributor Author

@ajewellamz ajewellamz Jul 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or if you're asking something else:
decrypted.parsedHeader.None? <==> (ALLOW_PLAINTEXT_READ && plain text item)

If ALLOW_PLAINTEXT_READ is false and it's a plain text item, then it throws an error.
If ALLOW_PLAINTEXT_READ is false and it succeeds, then it was an encrypted item and parsedHeader is Some.

:- Need(|decrypted.parsedHeader.value.encryptedDataKeys| == 1, E("Query result has more than one Encrypted Data Key"));
if decrypted.parsedHeader.value.encryptedDataKeys[0].keyProviderInfo == keyIdUtf8 {
decryptedItems := decryptedItems + [decrypted.plaintextItem];
Expand Down
5 changes: 3 additions & 2 deletions DynamoDbEncryption/dafny/DynamoDbEncryptionTransforms/src/ScanTransform.dfy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,10 @@ module ScanTransform {

var decryptInput := EncTypes.DecryptItemInput(encryptedItem := encryptedItems[x]);
var decryptRes := tableConfig.itemEncryptor.DecryptItem(decryptInput);

var decrypted :- MapError(decryptRes);
if keyId.KeyId? {

// No parsed header is ok, because it means ALLOW_PLAINTEXT_READ and a plain text item
if keyId.KeyId? && decrypted.parsedHeader.Some? {
:- Need(decrypted.parsedHeader.Some?, E("Decrypted scan result has no parsed header."));
:- Need(|decrypted.parsedHeader.value.encryptedDataKeys| == 1, E("Scan result has more than one Encrypted Data Key"));
if decrypted.parsedHeader.value.encryptedDataKeys[0].keyProviderInfo == keyIdUtf8 {
Expand Down
Loading