sigstore-rs
sigstore-rs copied to clipboard
JSON canonicalization uses multiple libraries.
Description
Different parts of code use different libraries for JSON canonicalization.
Examples:
https://github.com/sigstore/sigstore-rs/blob/d5ba303182318495a081d1c4ad50d5c27be015cc/src/sign.rs#L327-L332
https://github.com/sigstore/sigstore-rs/blob/d5ba303182318495a081d1c4ad50d5c27be015cc/src/cosign/bundle.rs#L81-L88
I was responsible for pulling in the second canonicalizer (json_syntax).
For context: the Rekor OpenAPI spec mentions that RFC 8785 style canonicalization should be used for SET verification. The cosign code uses OLPC style canonicalization. Investigating further, it seems that the major differences between these two canonicalization schemes are in floating-point numbers (OLPC doesn't allow them) and strings (RFC 8785 escapes control characters).
Suggestion: Can we standardize on json_syntax (or another RFC 8785 canonicalizer) across the codebase? We should never see these differences, but I figure that we are better safe than sorry :)
xref https://github.com/sigstore/sigstore-rs/pull/285#issuecomment-1947816580
I don't think it would be worth to drop the olpc-cjson dependency since it's a a transitive dependency of oci-distribution and tough.
json-syntax is a first level one. I don't think we can use olpc-cjson to validate Rekor's data. Maybe we could make this crate optional and require it only when the rekor feature is enabled
It seems this issue has been resolved already in https://github.com/sigstore/sigstore-rs/commit/68e640ba6d3f3fcf092788ce27da8afd8517ebb1
I saw that json-syntax has been made optional in depedencies. But it is not added as an optional dependency on rekor. Also, it was not removed from feature group sign and verify.
I think this above discussion is outdated and recent commit I mentioned has already audited the need of these dependencies. Still, if there is any change required. I would like to work on it.