feat(test_vector_handlers): TestVectors test with MPL constructs #650

lucasmcdonald3 · 2024-03-07T00:29:38Z

Issue #, if available:

Description of changes:

Modify test vector suite for all supported Python versions:
- Add test decrypting generated test vectors from the most recent successful run of ESDK-Dafny "Daily CI": current most recent
  - "most recent" will update daily, so the vectors used will update daily
- Add test decrypting required encryption context CMM manifest: link
  - For Python <3.10, MPL is not supported, and required encryption context CMM is not supported. However, this manifest also has "Default" CMM scenarios. As part of these changes, test vector runners without the MPL installed will filter out scenarios with the required encryption context CMM, and only test the "Default" CMM scenarios.
- Add test generating decrypt vectors with master key constructs, then decrypting them with Python master key constructs and the JS decrypt oracle.
- Add test encrypting vectors with master key constructs.
- Remove running awses_local; this test is now redundant by the additions above.
  - This consisted of 2 tests: 1) "test encrypt"; 2) "test generate decrypt vectors, then decrypt those vectors". These are now covered by the above.
Add test vectors if the MPL is installed:
- Add test generating decrypt vectors with keyring constructs, then decrypting them with Python keyrings, Python master keys, and the JS decrypt oracle.
- Add test decrypting the master key-generated vectors with keyrings.
- Add test encrypting vectors with keyring constructs.

Out of Scope

Hierarchy keyring test vectors. Tracked in feat(test_vector_handlers): Hierarchy keyring test vectors #674.
Decrypt with NET. Will be tracked separately. This requires publishing .NET MPL TestVectors and merging in test(test_vectors): Support reading manifests that specify a hierarchy keyring aws-encryption-sdk#649.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Check any applicable:

Were any files moved? Moving files changes their URL, which breaks all hyperlinks to the files.

src/aws_encryption_sdk/internal/formatting/deserialize.py

test_vector_handlers/src/awses_test_vectors/manifests/mpl_keyring.py

lucasmcdonald3 · 2024-03-21T20:10:03Z

test_vector_handlers/src/awses_test_vectors/manifests/mpl_keyring.py

+            )
+        )
+
+        # Weird hack #2:


Weird hack #2 is NOT present in Java.
Java test vectors use a modified keys.json file from other manifests: https://github.com/aws/aws-encryption-sdk-java/blob/master/src/test/resources/keys.json#L45
This is changed from "key-id": "rsa-4096-private" to "key-id": "rsa-4096"; similarly on the public key.
This allows the edk.keyProviderInfo == keyring.keyName check to pass, and the keyring "works".
But (afaik) all of these vector suites provide their own keys.json that still have key-ids that specify private/public, so Java would not work with those vector suites.

This code should probably be in the TestVectors Dafny. It's only here because I debugged this in Python, and want some feedback before changing the Dafny.

I'm not sure I'm following, I follow up.
This sounds like these hacks are fighting. The name of the key and the name of the key info are not the same thing.

I'd describe it as "coordinating" rather than fighting :)
The primary evidence for this is that the test vectors pass with this change

I think the plan of action here SHOULD be some combination of 1) editing key manifests going forward, so the key name of a public/private key pair is the same; 2) add a "legacy" check in TestVectors to align the key name of any keys with names "rsa-4096-private" and "rsa-4096-public"

Is this plan of action going to happen here or is this happening somewhere else?

seebees

So we generate with master key, and then decrypt with JS and master key.
How do we test keyring? Does the master key target do both?

src/aws_encryption_sdk/internal/formatting/deserialize.py

seebees · 2024-03-27T21:13:57Z

test_vector_handlers/src/awses_test_vectors/internal/mpl/tampering_mpl_materials.py

Should this be in the MPL Test vectors project?

If it isn't already, I think that's a good idea

Chatted with Jose, it's a good idea to put this in the test vector framework.

Can I resolve this? If this is done, just resolve the conversation plz :)

test_vector_handlers/src/awses_test_vectors/manifests/mpl_keyring.py

seebees · 2024-03-27T21:20:34Z

test_vector_handlers/src/awses_test_vectors/manifests/mpl_keyring.py

+            )
+        )
+
+        # Weird hack #2:


I'm not sure I'm following, I follow up.
This sounds like these hacks are fighting. The name of the key and the name of the key info are not the same thing.

lucasmcdonald3 · 2024-03-27T21:30:11Z

So we generate with master key, and then decrypt with JS and master key.
How do we test keyring? Does the master key target do both?

That is correct for Python <3.11.
For Python 3.11 and 3.12, the matrix is

Generate: [master key, keyring]
Decrypt: [master key, keyring, JS]

seebees

Looks good! So many commits!

seebees · 2024-05-07T23:27:53Z

setup.py

@@ -40,7 +40,8 @@ def get_requirements():
    license="Apache License 2.0",
    install_requires=get_requirements(),
    # pylint: disable=fixme
-    # TODO: Point at MPL main branch once Python MPL is merged into main.
+    # TODO-MPL: Point at PyPI once MPL is released.
+    # This blocks releasing ESDK-Python MPL integration.


Just wondering do we have a way to enforce this?

Yes! PyPI will not let you publish a package that uses GitHub directly. We MUST refer to the published MPL to publish the new ESDK-Python.

https://setuptools.pypa.io/en/latest/userguide/dependency_management.html#

seebees · 2024-05-07T23:32:01Z

test/unit/test_streaming_client_stream_decryptor.py

-    @patch("aws_encryption_sdk.streaming_client.derive_data_encryption_key")
-    @patch("aws_encryption_sdk.streaming_client.DecryptionMaterialsRequest")
-    @patch("aws_encryption_sdk.streaming_client.Verifier")
-    # Given: no MPL
-    @pytest.mark.skipif(HAS_MPL, reason="Test should only be executed without MPL in installation")
-    def test_GIVEN_decrypt_config_has_ec_WHEN_read_header_THEN_calls_decrypt_materials_with_reproduced_ec(
-        self,


Just wondering why we are deleting these?

This might be a diff issue... let me double check

Ah! This was a merge conflict issue. I committed this test when I shouldn't have.
The behavior from this test is no longer valid.
This test would assert that encryption context on decrypt works for native (ESDK-Python) CMMs.
We decided that encryption context on decrypt would not be supported in this thread.
The new behavior validating encryption context on decrypt raises an exception is present here.

lucasmcdonald3 added 28 commits February 23, 2024 18:00

fix

d413b65

copyright

c4ca658

more unit tests

d99b666

more unit tests

49cb7c8

more unit tests

705113a

more unit tests

f76d7f9

more unit tests

0da2a4f

cleanup

0040b2c

cleanup

9131433

poc impl

e6826eb

passing

a9fa1a5

cleanup

4eeb858

protect

21a8c93

ex

de870b8

changes

eedf1a3

changes

1db73eb

cleanup

8415c2c

cleanup

20bdaff

cleanup

6bf6094

cleanup

febe6db

cleanup

dc8abca

cleanup

8ff46f4

cleanup

aba7ccc

all message format versions

40fecc0

sync upstream

52043b9

working

d256bf5

cleanup

488bdda

cleanup:

cea9dab

lucasmcdonald3 changed the title ~~Lucmcdon/mpl testvectors~~ feat: MPL TestVectors Mar 7, 2024

lucasmcdonald3 changed the title ~~feat: MPL TestVectors~~ feat: TestVectors test against MPL constructs Mar 7, 2024

lucasmcdonald3 added 7 commits March 20, 2024 15:59

cleanup

67f0179

cleanup

7d8a515

debug

48974b0

cleanup

1bb5527

cleanup

72de35b

cleanup

f2792bc

cleanup

df45d5d

lucasmcdonald3 changed the title ~~feat(test_vector_handlers): TestVectors test against MPL constructs~~ feat(test_vector_handlers): TestVectors test with MPL constructs Mar 20, 2024

cleanup

c66938f

lucasmcdonald3 marked this pull request as ready for review March 20, 2024 23:50

lucasmcdonald3 requested a review from a team as a code owner March 20, 2024 23:50

lucasmcdonald3 commented Mar 21, 2024

View reviewed changes

src/aws_encryption_sdk/internal/formatting/deserialize.py Show resolved Hide resolved

lucasmcdonald3 commented Mar 21, 2024

View reviewed changes

test_vector_handlers/src/awses_test_vectors/manifests/mpl_keyring.py Show resolved Hide resolved

lucasmcdonald3 commented Mar 21, 2024

View reviewed changes

seebees reviewed Mar 27, 2024

View reviewed changes

Base automatically changed from lucmcdon/mpl-requiredec to mpl-reviewed April 26, 2024 17:47

lucasmcdonald3 added 10 commits May 3, 2024 14:16

fix: Try all master key providers when decrypting raw RSA data key

30ed6fa

resovle merge

f577371

resovle merge

a30bceb

cleanup

5393825

cleanup

709fb3a

cleanup

d64dc81

fix

c953b21

fix

c7c6a56

rv vectors

990e2b8

fix

6979419

seebees approved these changes May 7, 2024

View reviewed changes

lucasmcdonald3 merged commit 986f54c into mpl-reviewed May 8, 2024
151 checks passed

lucasmcdonald3 deleted the lucmcdon/mpl-testvectors branch May 9, 2024 21:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(test_vector_handlers): TestVectors test with MPL constructs #650

feat(test_vector_handlers): TestVectors test with MPL constructs #650

lucasmcdonald3 commented Mar 7, 2024 •

edited

Loading

lucasmcdonald3 Mar 21, 2024

seebees Mar 27, 2024

lucasmcdonald3 Mar 27, 2024 •

edited

Loading

seebees May 7, 2024

seebees left a comment

seebees Mar 27, 2024

lucasmcdonald3 Mar 27, 2024

lucasmcdonald3 Apr 10, 2024

seebees May 7, 2024

seebees Mar 27, 2024

lucasmcdonald3 commented Mar 27, 2024 •

edited

Loading

seebees left a comment

seebees May 7, 2024

lucasmcdonald3 May 7, 2024

lucasmcdonald3 May 7, 2024

seebees May 7, 2024

lucasmcdonald3 May 7, 2024

lucasmcdonald3 May 8, 2024

feat(test_vector_handlers): TestVectors test with MPL constructs #650

feat(test_vector_handlers): TestVectors test with MPL constructs #650

Conversation

lucasmcdonald3 commented Mar 7, 2024 • edited Loading

Check any applicable:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucasmcdonald3 Mar 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seebees left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucasmcdonald3 commented Mar 27, 2024 • edited Loading

seebees left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucasmcdonald3 commented Mar 7, 2024 •

edited

Loading

lucasmcdonald3 Mar 27, 2024 •

edited

Loading

lucasmcdonald3 commented Mar 27, 2024 •

edited

Loading