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

Support DSSE in Metadata API #2246

Closed
wants to merge 11 commits into from
Closed

Conversation

lukpueh
Copy link
Member

@lukpueh lukpueh commented Dec 22, 2022

Notes to reviewer:

  • based on: #2165 and sslib#487
    (tests should pass locally, when installing tuf and securesystemslib from these branches)
  • relevant diff: lukpueh/tuf@no-key...dsse
  • leaving as draft until above PRs are merged and, in the case of securesystemslib, released
  • the design is smart (h/t @PradyumnaKrishna), but adds quite some complexity to metadata API
  • this needs better tests and documentation, but I wanted to get a first round of feedback before spending more time on it
  • the commit messages have many details

Description of the changes being introduced by the pull request:

Adds a TUF-specific DSSE (Envelope) implementation and defines an abstract interface (BaseMetadata) for common Envelope and Metadata operations. BaseMetadata can be used independently of the underlying metadata type to read or write from and to file or bytes, and to create or verify signatures.

caveat: Optional arguments may be different for common Envelope and Metadata methods. This is because they require de/serialization operations at different stages. Envelope must deserialize, when accessing the payload (get_signed), but Metadata not. Metadata, OTOH, must serialize to canonical json when signing or verifying (sign and verify_delegate), but Envelope not.

This PR also adopts the new securesystemslib de/serialization infrastructure, which, most notably,

  • separates basic JSON bytes deserialization from custom object instantiation,
  • adds a SerializationMixin, which provides convenience methods to read or write from and to file or bytes

caveat: users of the serialization mixin must implement methods that return default de/serializers that can be used by the mixin.

jku and others added 11 commits December 15, 2022 11:57
If the sslib release version matches, pip does not install the version from git
because the same version is already installed. Force the install.

Signed-off-by: Jussi Kukkonen <[email protected]>
tuf.exceptions should IMO be seen as the "default exception source".

Signed-off-by: Jussi Kukkonen <[email protected]>
Key has been moved to Securesystemslib: use it from there.

This still fails tests as Key API has chnaged a bit: issues are fixed
in followup commits.

Signed-off-by: Jussi Kukkonen <[email protected]>
New Securesystemslib Keys can now be instantiated in two ways:
* deserialize via Key.from_dict() as before
* generate new keys via implementation specific methods

Fix all cases where we call Key() or Key.from_securesystemslib_key()
and use SSlibKey methods instead. Fix related tests.

Signed-off-by: Jussi Kukkonen <[email protected]>
Key.verify_signature() API has changed:
* argument is bytes, not metadata
* raised error now comes from securesystemslib

Signed-off-by: Jussi Kukkonen <[email protected]>
verify_delegate() unfortunately needs an almost complete rewrite
as the Key.verify_signature() API change affects it quite a bit.

Refactoring the role and key lookup into a separate method makes the
code readable again.

Signed-off-by: Jussi Kukkonen <[email protected]>
Use generic json de/serializers and seralization mixin provided
by securesystemslib.

**De/serializers**
- tuf's metadata-specific `JSON[Des|S]erializer` now call into
  securesystemslib's generic `JSON[Des|S]erializer` for basic json
  de/serialization.

- securesystemslib's `Base[Des|S]erializer` is now used as
  abstract interface for methods that de/serialize, though
  tuf's `Metadata[Des|S]erializer` and `SignedSerializer` still
  exist to avoid API break.
  NOTE: this makes typing slightly weaker, as `Base[Des|S]erializer`
  returns/takes `Any` instead of `Metadata`

**SerializationMixin**
- replaces `Metadata.[to|from]_[bytes|file]` with
  equivalent methods inherited from `SerializationMixin`.

- `SerializationMixin` requires implementing
  `_default_serializer` and `_default_deserializer` helpers used in
  `[to|from]_[bytes|file]` methods.

Signed-off-by: Lukas Puehringer <[email protected]>
Add TUF-specific DSSE (`Envelope`) implementation and define
abstract interface (`BaseMetadata`) for common `Envelope` and
`Metadata` operations:
- get_payload() -> Signed
- sign() -> Signature
- verify_delegate() -> None

**Details**

- `Envelope` inherits and calls generic methods from base
  `Envelope` in securesystemslib to sign and verify using the
   DSSE protocol.

- `Envelope` overrides `sign` to add an `append` option, which
  is not available in the base `Envelope`.

- `Envelope` provides a `from_signed` factory method, which
  serializes a `Signed` instance as payload.

- `Envelope.get_payload` takes a `SignedDeserializer` instance to
  deserialize the payload contents (default:
  `SignedJSONDeserializer`).
  `Metadata.get_payload` just returns the already deserialized
  `signed` attribute.

- `Metadata.[sign|verify_delegate]` methods take a
  `SignedSerializer` instance to serialize the payload prior to
  signing/verifying (default: `CanonicalJSONSerializer`).
  `Envelope.[sign|verify_delegate]` just signs/verifies the
  already serialized payload.

- `BaseMetadata` subclasses inherit `[to|from]_[bytes|file]`
  convenience methods from `SerializationMixin`. In turn they must
provide `_default_[de|s]erializer`s to be used by those methods.

- `BaseMetadata` provides default `JSON[Des|S]erializer` for
  both `Envelope` and `Metadata`.

- `JSONSerializer` requires a class to implement a `to_dict`
  method, which is defined by the `JSONSerializable` interface.
 `BaseMetadata` classes are `JSONSerializable`.

- `JSONDeserializer` can deserialize json bytes into both
  `Envelope` and `Metadata`. It case handles based on the presence
  of a certain field.

Signed-off-by: Lukas Puehringer <[email protected]>
Test Envelope (dsse) and common metadata abstraction for exemplary
root metadata: sign, verify, verify_delegate.

Signed-off-by: Lukas Puehringer <[email protected]>

from tuf.api.exceptions import RepositoryError

if TYPE_CHECKING:
# pylint: disable=cyclic-import
from tuf.api.metadata import Metadata, Signed

MetadataSerializer: TypeAlias = BaseSerializer
MetadataDeserializer: TypeAlias = BaseDeserializer
SignedSerializer: TypeAlias = BaseSerializer
Copy link
Member Author

@lukpueh lukpueh Dec 22, 2022

Choose a reason for hiding this comment

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

I created these aliases to not break the Metadata API.

Copy link
Member Author

@lukpueh lukpueh left a comment

Choose a reason for hiding this comment

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

@jku raised a valid concern about whether there is any merit in a common base class for Metadata and Envelope in the python-tuf metadata API.

Coming from the in-toto dsse integration work I had the following workflows in mind:

# metadata consumer
C1. load and verify metadata envelope
C2. extract payload and use it
# metadata producer
P1. create payload and wrap in metadata envelope
P2. sign and dump

A common base class is helpful for applications to reduce case handling for common operations (C1, P2). Users of the python-tuf metadata API, however, can be explicit about which type of metadata envelope they want to load, verify, sign, dump.

Also see inline comments for issues with common operations having different requirements...

delegated_role: str,
delegated_metadata: "BaseMetadata",
) -> None:
signed = self.get_signed(None)
Copy link
Member Author

Choose a reason for hiding this comment

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

The deserializer argument in get_signed should be configurable, e.g. via optional delegator_deserializer argument in verify_delegate.

Or maybe verify_delegate just should not be a method on the envelope, but rather on the already deserialized payload. At least for DSSE it does not seem like the right place to deserialize the payload here.

# pylint: disable=import-outside-toplevel
from tuf.api.serialization.json import JSONDeserializer

return JSONDeserializer()
Copy link
Member Author

Choose a reason for hiding this comment

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

Using the current implementation of JSONDeserializer as default deserializer for any BaseMetadata (i.e. Metadata and Envelope) can lead to unexpected behavior, because it can deserialize both traditional metadata and dsse:

if "payload" in json_dict:
return Envelope.from_dict(json_dict)
if "signed" in json_dict:
return Metadata.from_dict(json_dict)

This is the intended behavior for BaseMetadata, but a bug on either Metadata or Envelope:

#  Each `from_file` call below uses the default `JSONDeserializer` returned
#  from `_default_deserializer()`, and thus can return both `Metadata` or `Envelope`
BaseMetadata.from_file("path/to/any/metadata")  # returning `Metadata` or `Envelope` is expected
Metadata.from_file("path/to/any/metadata")  # should only return `Metadata`
Envelope.from_file("path/to/any/metadata")  # should only return `Envelope` 

@lukpueh
Copy link
Member Author

lukpueh commented May 8, 2023

Closing in favour of #2385

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