Skip to content
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.

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

Add match_payload method to Envelope #11

Closed

Conversation

PradyumnaKrishna
Copy link

Description of the changes being introduced by the pull request:

match_payload method checks for the payoad if deserializes into
the provided class type. This method is essential in order to
add DSSE compatibility in in_toto.

This method will provide interface similar to current in_toto's
Metablock property _type. Which is used at various places in
in_toto project to identify type of the payload.

Please verify and check that the pull request fulfils 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

match_payload method checks for the payoad if deserializes into
the provided class type. This method is essential in order to
add DSSE compatibility in in_toto.

This method will provide interface similar to current in_toto's
Metablock property `_type`. Which is used at various places in
in_toto project to identify type of the payload.

Note: This type is not payload_type of DSSE envelope.

Signed-off-by: Pradyumna Krishna <git@onpy.in>
except exceptions.DeserializationError:
return False
else:
return True
Copy link
Member

Choose a reason for hiding this comment

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

Why not look at self.payload_type to know which class to deserialize into?

Copy link
Author

Choose a reason for hiding this comment

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

ITE-5 suggests that in-toto will have metadata with payload_type=application/vnd.in-toto+json and in-toto has payloads that can further classified into two types, Link and Layout.

Also, This method is common with in-toto Metablock which has only two types either link or layout.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, right, thanks for the reminder. Still, it seems unnecessary to fully deserialize the payload twice, given that we know exactly what we expect.

Copy link
Author

Choose a reason for hiding this comment

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

Oh, right, thanks for the reminder. Still, it seems unnecessary to fully deserialize the payload twice, given that we know exactly what we expect.

I am open to other solution to this problem. There is only one case where payload was deserialized twice in a row.

Copy link
Member

Choose a reason for hiding this comment

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

There is only one case where payload was deserialized twice in a row.

We still deserialize the full object to extract a single field, and then throw away the object even if we still need it.

I am open to other solution to this problem.

I'd like to use a similar pattern as in Metablock.from_dict. It also doesn't need to be passed a Layout or Link class in order to deserialize the passed data into its signed field, because it knows that the passed data must have a _type field, whose value must be either link or layout, so it can during deserialization just call Link.from_dict or Layout.from_dict.

And the exact same thing applies to an Envelope when in the in-toto context. The problem is that Envelope does not only exist in the in-toto context. So we need to find a way to register an in-toto specific payload parsing for a generic Envelope class. I have to think about how to best do this.

@lukpueh
Copy link
Member

lukpueh commented Jan 10, 2023

match_payload will not be upstreamed, see corresponding note in secure-systems-lab#370 (comment). Thanks for the efforts, @PradyumnaKrishna, this work was helpful for path finding!

@lukpueh lukpueh closed this Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants