[WIP] Add support for Statements #570

chasen-bettinger · 2023-03-25T23:43:02Z

Fixes issue #:
#562

Description of the changes being introduced by the pull request:

Added the Statement model
Incorporated the Statement model into the Envelope.
Skipped a test because I'm awaiting feedback on whether this is the right direction to take this issue (Add support for ITE-6 Attestations #562). The test is built on the previously-used 'link' model as the payload.

Some notes:

This is largely a WIP - general feedback on data structures and architecture most appreciated!
I studied the following information throughout the process of writing this code:
I will add docs once the code is in a good place.

Some things I still don't understand:

What are valid predicate_types and where do they come from?
What is the expected output for the software after this change? I went through the demo and saw that .link files were being generated. I presumed based on the information on the SLSA website that Links are a subset of Statements and that once in-toto supported Statements (and Envelopes WITH Signatures and Statements), the output would be an Envelope? I don't really understand the bigger picture just yet.
I don't know what the values for a predicate hash should be nor where that comes from. This resource was most helpful in giving me breadcrumbs, but I'm still uncertain.

Please verify and check that the pull request fulfills the following
requirements:

The code follows the Code Style Guidelines
Tests have been added for the bug fix or new feature
Docs have been added for the bug fix or new feature

adityasaky

Great start, @chasen-bettinger! I've left some comments but I think this is looking quite good. I'd like to scope this PR to adding support for the statement layer and tackle predicate implementations separately.

Edit: I also recommend switching to a feature branch instead of using develop.

in_toto/models/statement.py

tests/models/test_statement.py

marcelamelara · 2023-03-27T22:43:23Z

Hello! I just came across this issue, thanks @chasen-bettinger . Have you seen the protobuf definition for statement? Instead of implementing a separate Python class for Statement, our recommendation (from the attestation side) would be to build applications based on the protos. This way, all ITE-6 compliant in-toto implementations actually use that same pre-defined format. If you're interested, we would very much welcome a contribution related to using the protos in the Python implementation.

adityasaky · 2023-04-26T14:44:53Z

@chasen-bettinger this should unblock you: https://pypi.org/project/in-toto-attestation/

chasen-bettinger · 2023-05-31T12:40:43Z

in_toto/models/link.py

+            self.environment = {}
+
+        if hasattr(self, 'byproducts') == True:
+            self.byproducts["return-value"] = int(


@adityasaky In order to successfully serialize this class into a canonical JSON, I've had to write in this hack. When the class is parsed from a protobuf to a JSON, the value becomes a float, which breaks downstream processes because that is against the rules of canonical JSON. In the current design, we cannot get around this limitation because it is a central part of the Signable class and its signable_bytes property.

As you may have already witnessed, either due to the architecture or my poor Python skills (most likely the latter), this class (and the Statement class) became quite ugly when trying to integrate the protobufs into the existing structure. As the code is written now, I believe there's not a way around using the Signable class as it is integral in generating and validating signatures. I fear a slight refactor might be in the near future, but I wanted your thoughts first.

Any thoughts on the best way to proceed?

cc: @marcelamelara

If I'm not mistaken, the link predicate shouldn't be Signable at all. The predicate will be embedded in a Statement which is what we'd actually be signing.

I missed that the link was extending Signable. Agree with Aditya that shouldn't be the case. The link proto should be converted into a struct, inserted into the Statement predicate field, and the Statement will then be signed.

I also believe that Statement might not have to extend Signable anymore either. The validation of the structure is now implemented in the Statement wrapper class provided by the in_toto_attestation package, and we can similarly use protobuf APIs directly to expose the signable bytes for a Statement.

adityasaky

I have more comments about how we'd architect this that I'll submit separately.

in_toto/models/link.py

adityasaky · 2023-06-01T14:10:36Z

in_toto/models/link.py

+            self.environment = {}
+
+        if hasattr(self, 'byproducts') == True:
+            self.byproducts["return-value"] = int(


If I'm not mistaken, the link predicate shouldn't be Signable at all. The predicate will be embedded in a Statement which is what we'd actually be signing.

.gitignore

- Added the Statement model - Incorporated the Statement model into the Envelope. - Skipped a test because I'm awaiting feedback on whether this is the right direction to take this issue (in-toto#562). The test is built on the previously-used 'link' model as the payload. Signed-off-by: Chasen Bettinger <bettingerchasen@gmail.com>

Signed-off-by: Chasen Bettinger <bettingerchasen@gmail.com>

chasen-bettinger · 2023-08-25T02:38:38Z

in_toto/models/statement.py

+STATEMENT_TYPE = "https://in-toto.io/Statement/v1"
+
+
+# NOTE: 08/24/23 - This function is an intentional


@adityasaky @marcelamelara Review please

I fear there is not much else you can do here, if you want Statement to be a Signable.

FYI: I had a very similar issue in in-toto-golang a while ago and tried the same fix you proposed in secure-systems-lab/securesystemslib#625 but did not use it for the reasons pointed out by @jku. The fix I did end up using there (case-handling json.Number) won't work in Python. But maybe that's okay, because the fix is actually quite a hack.

Because of all these problems with canonical json, I think the best way going forward would be to just not support a canonical json representation for Statements, and use it with DSSE Envelopes only.

Seems like others have said this before: #570 (comment)

Yea, understood. I'm curious to learn the ramifications of adding a new method to the Envelope class within this library that doesn't require a Signable instance to instantiate.

chasen-bettinger · 2023-08-25T02:39:35Z

tests/test_in_toto_run.py

I'll correct these tests once the structure of the implementation details have settled. Was using this as my test bed for validating changes.

adityasaky · 2023-08-28T16:49:20Z

.gitignore

@@ -60,4 +60,4 @@ target/

 #Ipython Notebook
 .ipynb_checkpoints
-.travis-solo/
+.travis-solo/


Can you add this new line back in?

adityasaky · 2023-08-28T16:49:51Z

in_toto/common_args.py

@@ -196,6 +196,13 @@ def parse_password_and_prompt_args(args):
    "help": ("generate metadata using dsse (experimental)."),
 }

+STATEMENT_ARGS = ["--use-statement"]


I'm worried statement is ambiguous, could you update this to be attestation instead?

Ideally, --generate-attestation.

adityasaky · 2023-08-28T16:53:46Z

in_toto/runlib.py

@@ -588,7 +593,33 @@ def in_toto_run(

    if use_dsse:
        LOG.info("Generating link metadata using DSSE...")
-        link_metadata = Envelope.from_signable(link)
+
+        if use_statement == False:


Potential footgun: if I set use_statement (or use_attestation) to True, I shouldn't be expected to set use_dsse to True as well. I see the CLI handles this but this is an issue with direct API use. I suggest we move use_attestation out of the use_dsse block and only support DSSE there. The use_dsse conditional block should only be for legacy links, and something we phase out in a future version as we deprecate legacy links in favour of link predicates.

adityasaky · 2023-09-25T15:21:55Z

in_toto/models/metadata.py

@@ -223,7 +226,8 @@ def __repr__(self):
        separators = (",", ":") if self.compact_json else (",", ": ")

        return json.dumps(
-            {"signatures": self.signatures, "signed": attr.asdict(self.signed)},
+            {"signatures": self.signatures,


editor shenanigans?

adityasaky · 2023-09-25T15:22:09Z

in_toto/models/metadata.py

@@ -438,7 +442,8 @@ def _validate_signatures(self):
            )

        for signature in self.signatures:
-            securesystemslib.formats.ANY_SIGNATURE_SCHEMA.check_match(signature)
+            securesystemslib.formats.ANY_SIGNATURE_SCHEMA.check_match(
+                signature)


and here too?

chasen-bettinger marked this pull request as draft March 25, 2023 23:43

chasen-bettinger force-pushed the develop branch from 142d15c to adc4a74 Compare March 25, 2023 23:46

adityasaky self-requested a review March 26, 2023 16:00

adityasaky requested changes Mar 26, 2023

View reviewed changes

chasen-bettinger mentioned this pull request Mar 28, 2023

Include generated python bindings for protobufs in-toto/attestation#171

Closed

chasen-bettinger force-pushed the develop branch from adc4a74 to 8f82989 Compare May 29, 2023 22:56

chasen-bettinger commented May 31, 2023

View reviewed changes

adityasaky reviewed Jun 1, 2023

View reviewed changes

marcelamelara mentioned this pull request Jun 1, 2023

Add Python validation APIs and unit tests in-toto/attestation#227

Merged

chasen-bettinger force-pushed the develop branch from 70b881e to f059705 Compare August 17, 2023 20:39

chasen-bettinger added 4 commits August 17, 2023 16:45

WIP: Building statement with protobuf

7382d33

Signed-off-by: Chasen Bettinger <bettingerchasen@gmail.com>

Working version of Statements

6145861

Signed-off-by: Chasen Bettinger <bettingerchasen@gmail.com>

Move Link to use attestation protobufs

b15ddc2

Signed-off-by: Chasen Bettinger <bettingerchasen@gmail.com>

chasen-bettinger force-pushed the develop branch 2 times, most recently from e644a07 to b15ddc2 Compare August 17, 2023 20:49

chasen-bettinger added 5 commits August 17, 2023 16:59

Remove .vscode from .gitignore

46c67c9

Signed-off-by: Chasen Bettinger <bettingerchasen@gmail.com>

Revert back to previous runlib code

2a59a3c

Signed-off-by: Chasen Bettinger <bettingerchasen@gmail.com>

Return to previous iteration of link

67607a7

Signed-off-by: Chasen Bettinger <bettingerchasen@gmail.com>

Fix spacing in .gitignore

e2e93a1

Signed-off-by: Chasen Bettinger <bettingerchasen@gmail.com>

Progress on Statement

d2d71a8

Signed-off-by: Chasen Bettinger <bettingerchasen@gmail.com>

chasen-bettinger requested review from adityasaky and marcelamelara August 25, 2023 02:37

chasen-bettinger commented Aug 25, 2023

View reviewed changes

chasen-bettinger mentioned this pull request Aug 25, 2023

support floats within _encode_canonical secure-systems-lab/securesystemslib#625

Closed

lukpueh mentioned this pull request Aug 25, 2023

Envelope.from_signable gives a wrong impression about Signable #621

Open

adityasaky requested changes Aug 28, 2023

View reviewed changes

adityasaky reviewed Sep 25, 2023

View reviewed changes

adityasaky mentioned this pull request Feb 22, 2024

Add support for ITE-6 Attestations #562

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Add support for Statements #570

[WIP] Add support for Statements #570

chasen-bettinger commented Mar 25, 2023

adityasaky left a comment •

edited

marcelamelara commented Mar 27, 2023

adityasaky commented Apr 26, 2023

chasen-bettinger May 31, 2023

adityasaky Jun 1, 2023

marcelamelara Jun 1, 2023

adityasaky left a comment

adityasaky Jun 1, 2023

chasen-bettinger Aug 25, 2023

lukpueh Aug 25, 2023 •

edited

lukpueh Aug 25, 2023

chasen-bettinger Aug 25, 2023

chasen-bettinger Aug 25, 2023

adityasaky Aug 28, 2023

adityasaky Aug 28, 2023

adityasaky Sep 25, 2023

adityasaky Aug 28, 2023

adityasaky Sep 25, 2023

adityasaky Sep 25, 2023

		STATEMENT_TYPE = "https://in-toto.io/Statement/v1"


		# NOTE: 08/24/23 - This function is an intentional

[WIP] Add support for Statements #570

Are you sure you want to change the base?

[WIP] Add support for Statements #570

Conversation

chasen-bettinger commented Mar 25, 2023

adityasaky left a comment • edited

Choose a reason for hiding this comment

marcelamelara commented Mar 27, 2023

adityasaky commented Apr 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adityasaky left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukpueh Aug 25, 2023 • edited

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adityasaky left a comment •

edited

lukpueh Aug 25, 2023 •

edited