fix: allow multi-tenant queries with allow_plaintext #1240

ajewellamz · 2024-07-31T12:50:30Z

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

seebees · 2024-07-31T15:27:50Z

DynamoDbEncryption/dafny/DynamoDbEncryptionTransforms/src/QueryTransform.dfy

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


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

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.

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.

josecorella

a nice to have but we can always add later

josecorella · 2024-07-31T21:10:56Z

DynamoDbEncryption/dafny/DynamoDbEncryptionTransforms/src/ScanTransform.dfy

+      assert decrypted.parsedHeader.None? ==>
+          && EncOps.IsPlaintextItem(encryptedItems[x])
+          && !tableConfig.plaintextOverride.FORBID_PLAINTEXT_WRITE_FORBID_PLAINTEXT_READ?
+          && (
+               || tableConfig.plaintextOverride.FORBID_PLAINTEXT_WRITE_ALLOW_PLAINTEXT_READ?
+               || tableConfig.plaintextOverride.FORCE_PLAINTEXT_WRITE_ALLOW_PLAINTEXT_READ?
+             );


nit: since this assert is the same as the one in QueryTransform, it would be nice to split into a predicate and just call it.

## [3.6.1](v3.6.0...v3.6.1) (2024-08-12) ### Fixes * allow multi-tenant queries with allow_plaintext ([#1240](#1240)) ([1487d7e](1487d7e)) * **TestVectors:** define StartUpObject in csproj ([#1231](#1231)) ([2f97bf3](2f97bf3)) ### Maintenance * Add examples to examine contents of query error list ([#1251](#1251)) ([b5705ee](b5705ee)) * **CI:** add smithy diff checker GHA ([#1226](#1226)) ([86406f5](86406f5)) * **deps:** bump actions/setup-dotnet from 3 to 4 in /.github/workflows ([#1191](#1191)) ([c3b736e](c3b736e)) * **deps:** bump aws-actions/configure-aws-credentials ([#1190](#1190)) ([becbd0a](becbd0a)) * **deps:** bump com.amazonaws:aws-java-sdk-dynamodb ([#1230](#1230)) ([3aa25d0](3aa25d0)) * **deps:** bump dafny-lang/setup-dafny-action in /.github/workflows ([#1200](#1200)) ([5284f0b](5284f0b)) * **deps:** bump software.amazon.awssdk:bom ([#1227](#1227)) ([abd1727](abd1727)) * **deps:** bump software.amazon.awssdk:bom ([#1229](#1229)) ([bf3e1c3](bf3e1c3)) * **deps:** bump software.amazon.awssdk:core ([#1228](#1228)) ([9c67729](9c67729)) * do not add beacons when FORCE_PLAINTEXT_WRITE is used. ([#1232](#1232)) ([23c8a18](23c8a18)) * include bad item keys in query errors ([#1244](#1244)) ([07bba8b](07bba8b)) * update version to snapshot ([#1225](#1225)) ([c817b5b](c817b5b))

## [3.6.1](v3.6.0...v3.6.1) (2024-08-12) ### Fixes * allow multi-tenant queries with allow_plaintext ([#1240](#1240)) ([1487d7e](1487d7e)) * **TestVectors:** define StartUpObject in csproj ([#1231](#1231)) ([2f97bf3](2f97bf3)) ### Maintenance * Add examples to examine contents of query error list ([#1251](#1251)) ([b5705ee](b5705ee)) * **CI:** add smithy diff checker GHA ([#1226](#1226)) ([86406f5](86406f5)) * **deps:** bump actions/setup-dotnet from 3 to 4 in /.github/workflows ([#1191](#1191)) ([c3b736e](c3b736e)) * **deps:** bump aws-actions/configure-aws-credentials ([#1190](#1190)) ([becbd0a](becbd0a)) * **deps:** bump com.amazonaws:aws-java-sdk-dynamodb ([#1230](#1230)) ([3aa25d0](3aa25d0)) * **deps:** bump dafny-lang/setup-dafny-action in /.github/workflows ([#1200](#1200)) ([5284f0b](5284f0b)) * **deps:** bump software.amazon.awssdk:bom ([#1227](#1227)) ([abd1727](abd1727)) * **deps:** bump software.amazon.awssdk:bom ([#1229](#1229)) ([bf3e1c3](bf3e1c3)) * **deps:** bump software.amazon.awssdk:core ([#1228](#1228)) ([9c67729](9c67729)) * do not add beacons when FORCE_PLAINTEXT_WRITE is used. ([#1232](#1232)) ([23c8a18](23c8a18)) * include bad item keys in query errors ([#1244](#1244)) ([07bba8b](07bba8b)) * update version to snapshot ([#1225](#1225)) ([c817b5b](c817b5b)) --------- Co-authored-by: semantic-release-bot <[email protected]>

## [3.6.1](v3.6.0...v3.6.1) (2024-08-12) ### Fixes * allow multi-tenant queries with allow_plaintext ([#1240](#1240)) ([1487d7e](1487d7e)) * **TestVectors:** define StartUpObject in csproj ([#1231](#1231)) ([2f97bf3](2f97bf3)) * update error message ([#1270](#1270)) ([7157e4d](7157e4d)) ### Maintenance * Add examples to examine contents of query error list ([#1251](#1251)) ([b5705ee](b5705ee)) * **CI:** add smithy diff checker GHA ([#1226](#1226)) ([86406f5](86406f5)) * **deps:** bump actions/setup-dotnet from 3 to 4 in /.github/workflows ([#1191](#1191)) ([c3b736e](c3b736e)) * **deps:** bump aws-actions/configure-aws-credentials ([#1190](#1190)) ([becbd0a](becbd0a)) * **deps:** bump com.amazonaws:aws-java-sdk-dynamodb ([#1230](#1230)) ([3aa25d0](3aa25d0)) * **deps:** bump dafny-lang/setup-dafny-action in /.github/workflows ([#1200](#1200)) ([5284f0b](5284f0b)) * **deps:** bump software.amazon.awssdk:bom ([#1227](#1227)) ([abd1727](abd1727)) * **deps:** bump software.amazon.awssdk:bom ([#1229](#1229)) ([bf3e1c3](bf3e1c3)) * **deps:** bump software.amazon.awssdk:core ([#1228](#1228)) ([9c67729](9c67729)) * do not add beacons when FORCE_PLAINTEXT_WRITE is used. ([#1232](#1232)) ([23c8a18](23c8a18)) * include bad item keys in query errors ([#1244](#1244)) ([07bba8b](07bba8b)) * **release:** 3.6.1 ([#1266](#1266)) ([64374e2](64374e2)), closes [#1240](#1240) [#1231](#1231) [#1251](#1251) [#1226](#1226) [#1191](#1191) [#1190](#1190) [#1230](#1230) [#1200](#1200) [#1227](#1227) [#1229](#1229) [#1228](#1228) [#1232](#1232) [#1244](#1244) [#1225](#1225) * update version to snapshot ([#1225](#1225)) ([c817b5b](c817b5b))

## [3.6.1](v3.6.0...v3.6.1) (2024-08-12) ### Fixes * allow multi-tenant queries with allow_plaintext ([#1240](#1240)) ([1487d7e](1487d7e)) * **TestVectors:** define StartUpObject in csproj ([#1231](#1231)) ([2f97bf3](2f97bf3)) * update error message ([#1270](#1270)) ([7157e4d](7157e4d)) ### Maintenance * Add examples to examine contents of query error list ([#1251](#1251)) ([b5705ee](b5705ee)) * **CI:** add smithy diff checker GHA ([#1226](#1226)) ([86406f5](86406f5)) * **deps:** bump actions/setup-dotnet from 3 to 4 in /.github/workflows ([#1191](#1191)) ([c3b736e](c3b736e)) * **deps:** bump aws-actions/configure-aws-credentials ([#1190](#1190)) ([becbd0a](becbd0a)) * **deps:** bump com.amazonaws:aws-java-sdk-dynamodb ([#1230](#1230)) ([3aa25d0](3aa25d0)) * **deps:** bump dafny-lang/setup-dafny-action in /.github/workflows ([#1200](#1200)) ([5284f0b](5284f0b)) * **deps:** bump software.amazon.awssdk:bom ([#1227](#1227)) ([abd1727](abd1727)) * **deps:** bump software.amazon.awssdk:bom ([#1229](#1229)) ([bf3e1c3](bf3e1c3)) * **deps:** bump software.amazon.awssdk:core ([#1228](#1228)) ([9c67729](9c67729)) * do not add beacons when FORCE_PLAINTEXT_WRITE is used. ([#1232](#1232)) ([23c8a18](23c8a18)) * include bad item keys in query errors ([#1244](#1244)) ([07bba8b](07bba8b)) * **release:** 3.6.1 ([#1266](#1266)) ([64374e2](64374e2)), closes [#1240](#1240) [#1231](#1231) [#1251](#1251) [#1226](#1226) [#1191](#1191) [#1190](#1190) [#1230](#1230) [#1200](#1200) [#1227](#1227) [#1229](#1229) [#1228](#1228) [#1232](#1232) [#1244](#1244) [#1225](#1225) * update version to snapshot ([#1225](#1225)) ([c817b5b](c817b5b)) * lint * remove recursive reference

fix: allow multi-tenant queries with allow_plaintext

a5a4aca

ajewellamz requested a review from a team as a code owner July 31, 2024 12:50

seebees reviewed Jul 31, 2024

View reviewed changes

ajewellamz and others added 2 commits July 31, 2024 14:07

m

46b0aaf

what about this?

99f9561

lucasmcdonald3 approved these changes Jul 31, 2024

View reviewed changes

ajewellamz merged commit 1487d7e into main Jul 31, 2024
34 checks passed

ajewellamz deleted the ajewell/multiplain branch July 31, 2024 21:10

josecorella approved these changes Jul 31, 2024

View reviewed changes

seebees mentioned this pull request Aug 12, 2024

chore(release): 3.6.1 #1266

Merged

seebees mentioned this pull request Aug 14, 2024

chore(release): 3.6.1 - update #1271

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: allow multi-tenant queries with allow_plaintext #1240

fix: allow multi-tenant queries with allow_plaintext #1240

ajewellamz commented Jul 31, 2024

seebees Jul 31, 2024

ajewellamz Jul 31, 2024

ajewellamz Jul 31, 2024 •

edited

Loading

josecorella left a comment

josecorella Jul 31, 2024

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

fix: allow multi-tenant queries with allow_plaintext #1240

fix: allow multi-tenant queries with allow_plaintext #1240

Conversation

ajewellamz commented Jul 31, 2024

seebees Jul 31, 2024

Choose a reason for hiding this comment

ajewellamz Jul 31, 2024

Choose a reason for hiding this comment

ajewellamz Jul 31, 2024 • edited Loading

Choose a reason for hiding this comment

josecorella left a comment

Choose a reason for hiding this comment

josecorella Jul 31, 2024

Choose a reason for hiding this comment

ajewellamz Jul 31, 2024 •

edited

Loading