search

theupdateframework / specification

The Update Framework specification
https://theupdateframework.github.io/specification/
Other
368 stars 54 forks source link

Clarify current state of key formats in the spec #272

Closed joshuagl closed 1 year ago

joshuagl commented 1 year ago

Key formats are a recurring source of confusion (see https://github.com/secure-systems-lab/securesystemslib/issues/513, https://github.com/theupdateframework/specification/issues/201, and https://github.com/secure-systems-lab/securesystemslib/issues/308). Do we specify key formats, or simply record those used by the reference implementation?

The current status is unclear, so this PR attempts to:

  1. clarify that the spec only documents the key formats used by the reference implementation
  2. recommend against redefining the formats documented in the spec
  3. renames the "ecdsa-sha2-nistp256" key format to "ecdsa", per the reference implementation (https://github.com/secure-systems-lab/securesystemslib/pull/267)

The above changes are included in a single commit ac9fc63234c92483a1cda81bf3c148553bf6edcf.

There are additional changes in this PR to address a bikeshed error in newer versions of the generator (2de414906d634d2d24df30e51c0bd593791da0ff) and to add additional navigation headers to make it easier to find and link to details of the recommended file formats (5fef3750d9749bfd944fd8e513f928f2732c994b). I included these changes in one PR because the workflow is awkward for small changes.

I encourage reviewers to use the commits tab and review each patch independently.

(Note: deeper discussions around key formats and specification vs. recommendation and interoperability of implementations is necessary, and has been added to the agenda

joshuagl commented 1 year ago

I just rebased on main and pushed an additional commit which: adds a header for the object format and makes that section, the key format section and the date-time section all subsections (one level of heading lower) of the "File formats: general principles" section.

@lukpueh PTAL at https://github.com/theupdateframework/specification/pull/272/commits/9fe0b0154ff1a3cd8ba87a400c054486693f3e09 and re-approve if you agree.

Joshua, did you coordinate with @trishankatdatadog that he syncs your branch with upstream?

We did not coordinate.

trishankatdatadog commented 1 year ago

Sorry, what branch upstream?

lukpueh commented 1 year ago

Sorry, what branch upstream?

You seemed to have pushed a merge commit (https://github.com/theupdateframework/specification/commit/e83ed236af1c73ea47f581dbe0fa0306f2f649f4) onto Joshua's branch.

joshuagl commented 1 year ago

Rebased on master and force pushed. That didn't dismiss @lukpueh's approval 😌 has GitHub evolved?

@trishankatdatadog @mnm678 any chance of a review/2nd approval?