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

POC: Use securesystemslib base+json de/serializer and mixin #2292

Closed

Conversation

lukpueh
Copy link
Member

@lukpueh lukpueh commented Feb 3, 2023

This PR is meant as a help for python-tuf maintainers to review the serialization-specific parts of secure-systems-lab/securesystemslib#487, which are more tailored towards in-toto and DSSE.

Please see commit message for details.

securesystemslib now provides a generic de/serialization module,
modelled after `tuf.api.serialization`:
secure-systems-lab/securesystemslib#487

This module provides abstract `BaseSerializer` and
`BaseDeserializer` classes, which define serialize and deserialize
methods respectively. They are similar to TUF's
`MetadataSerializer` and `MetadataDeserializer`, but without
restricting argument or return value type to `Metadata`.

The module further provides `JSONSerializer` and `JSONDeserializer`
classes, which implement serialize and deserialize methods
respectively. They are similar to TUF's existing `JSONSerializer`
and `JSONDeserializer`, but without `Metadata`-specific code. That
is, `JSONSerializer` takes any object that implements `to_dict`
(i.e. `JSONSerializable`), and `JSONDeserializer` just deserializes
json bytes to dict, and leaves object specific deserialization
(e.g. `Metadata.from_dict`) to a subclass implementation.

This patch uses securesystemslib's base de/serializer classes in
metadata method signatures (type-aliased for backwards
compatibility) and factors out the json-specific de/serialization
to securesystemslib.

**IMPORTANT NOTE:** This is a POC to demonstrate how the
`securesystemslib.serialization` can be used in general.  It may
not actually be that useful for python-tuf, given that python-tuf
has a working serialization subpackage and below caveat.  It can,
however, be re-used in a couple of other classes, which don't have
a proper de/serialization implementation yet.

Generic DSSE
- secureystemslib.Envelope.from_file
- secureystemslib.Envelope.get_payload
DSSE for in-toto payloads
- in_toto.Envelope.from_file
- in_toto.Envelope.get_payload
in-toto Metadata pendant
- in_toto.Metablock.from_file
in-toto Metablock and DSSE abstraction
- in_toto.AnyMetadata.from_file
- in_toto.AnyMetadata.get_payload

**CAVEAT:**
Using securesystemslib's `Base[Des|S]erializer` instead of
`Metadata[Des|S]erializer` (and `SignedSerializer`) weakens the
interface, because it is more generic about the argument/return
value type. Stricter typing and also raising the right TUF
de/serialization errors's needs to be implemented by a tuf-specific
de/serializer, see e.g. `tuf.api.serialization.json`.

Signed-off-by: Lukas Puehringer <[email protected]>
@coveralls
Copy link

coveralls commented Feb 3, 2023

Pull Request Test Coverage Report for Build 4083587150

  • 18 of 18 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 97.923%

Totals Coverage Status
Change from base Build 4074496200: 0.2%
Covered Lines: 1309
Relevant Lines: 1328

💛 - Coveralls

securesystemslib provides a mixin with generic methods to read and
write to and from bytes and files, which are reusable by TUF's
Metadata class, but also by the generic DSSE Envelope in
securesystemslib, and the in-toto DSSE Envelope and in-toto
Metablock.

This patch demonstrates how the mixin can be used instead of TUF's
own `Metadata.[to|from]_[bytes|file]` methods.

**CAVEAT:**

The default de/serializers can no longer be hardcoded into the
method (as they were in `Metadata.[to|from]_[bytes|file]`). To
still support defaults, users of the mixin must implement
`_default_serializer` and `_default_deserializer` helper methods.
This feels a bit intransparent and over-engineered.

Signed-off-by: Lukas Puehringer <[email protected]>
@lukpueh lukpueh force-pushed the use-sslib-serializers branch from d9c38ad to 2e1e2f1 Compare February 3, 2023 11:41
@lukpueh lukpueh changed the title POC: Use securesystemslib base+json de/serializer POC: Use securesystemslib base+json de/serializer and mixin Feb 3, 2023
@lukpueh
Copy link
Member Author

lukpueh commented Mar 3, 2023

The securesystemslib base+json de/serializer and mixin used here was rejected in securesystemslib for the reasons summarized in: secure-systems-lab/securesystemslib#487 (comment)

Closing

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