theupdateframework
Tweaks to better interop with go-tuf
👋 I've been working on integrating the Vector project with Datadog's TUF/Uptane implementation, and this PR contains the handful of tweaks I've had to make to get everything interoperating happily:
- A
spec_versionof"1.0"is not actually valid according to SemVer. It should probably be"1.0.0"everywhere, but I've kept a check for the old value so that it continues to work with existing metadata. - The datetime parsing was overly strict relative to the spec, which only specifies ISO8601. Here it's adjusted to use a full RFC3339 parser that handles the slightly differing format that go-tuf can emit.
- Allowing
*in paths, which is legal according to the spec and used somewhat heavily when specifying delegations. - Adding a couple of
Sendbounds, which isn't related to interop but made the library easier to use.
The spec_version change does seem to break the interop tests, but I wanted to make sure we want to move forward with the change before doing the work of regenerating the test data.
If any these seem undesirable or warrant more discussion, I'd be happy to split them into separate PRs.
/cc @zenithar @cedricvanrompay-datadog
Thanks! I've updated the lib tests, so those should now be passing, but left the interop tests since I'm not sure of the best procedure for generating new fixtures.
As far as testing delegation support, I'll see if some of our TUF experts can have a look and give some feedback. I'm pretty sure we use them at Datadog, but my team is still pretty new to TUF.
First, nice work!
As far as testing delegation support, I'll see if some of our TUF experts can have a look and give some feedback. I'm pretty sure we use them at Datadog, but my team is still pretty new to TUF.
Second, yes, we do use delegations for full verification. I recommend taking a look at how go-tuf or python-tuf does it.
Interesting, it seems that go-tuf has it wrong as well for the spec_version format: https://github.com/theupdateframework/go-tuf/blob/ac7b5d7bce18cca5a84a28b021bd6372f450b35b/data/types.go#L112
Interesting, it seems that
go-tufhas it wrong as well for thespec_versionformat: https://github.com/theupdateframework/go-tuf/blob/ac7b5d7bce18cca5a84a28b021bd6372f450b35b/data/types.go#L112
If so, would you help us to create a new issue there? Thanks!
I've added one more commit here to add a catch-all additional_fields property to ~~TargetsMetadata~~ all of the top level metadata structs. It turns out that go-tuf has a custom field here that is not mentioned in the spec. I didn't necessarily want to duplicate that, but the spec does also say that implementations can use additional fields and they should be maintained, so that's what I did. If someone has a better solution for that I'd be happy to discuss it.
It looks like we also got some test failures. I'd also like the additional_fields to have some tests too. It might be better to factor that out into a separate PR so it doesn't conflate with the other changes.
FYI I did find a bug in rust-tuf's delegation https://github.com/theupdateframework/rust-tuf/issues/381.
I just realized that switching our metadata generation to building metadata with the spec_version of 1.0.0 would cause old clients to reject the now spec-conformant metadata, so we need to come up with a way to do a graceful migration.
As a stopgap, I factored out some of your fix in #386 to make sure new clients will at least be able to support the proper format once we start producing it.
@erickt Thanks so much for taking most of these across the line! Apologies for the radio silence, I had a couple weeks of travel there and some conflicting work priorities. It seems like the additional_fields piece, as well as maybe the Send bound tweak, are all that's left? This coming week I'm happy to open a more specific PR for just that.
Yeah, although I saw and landed a patch that removed a number of unnecessary trait bounds. It might be that inheriting Send might not be necessary anymore.