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

log.ID should be derived from origin in addition to public key #14

Closed
mhutchinson opened this issue Jan 23, 2023 · 0 comments · Fixed by #15
Closed

log.ID should be derived from origin in addition to public key #14

mhutchinson opened this issue Jan 23, 2023 · 0 comments · Fixed by #15
Assignees

Comments

@mhutchinson
Copy link
Contributor

mhutchinson commented Jan 23, 2023

log.ID was designed to take both origin and public key, but only uses public key: https://github.com/transparency-dev/formats/blob/main/log/identifier.go#L27. This is somewhat reasonable as there is a strong push to make the key for each log unique. On the other hand, we already have an example of a pair of logs that use the same key material but have different origin strings: https://github.com/google/trillian-examples/blob/master/witness/golang/omniwitness/witness_configs/witness.yaml#L10,L19

It seems likely that other logs could do this in the future, so let's bite the bullet now and change the ID function to also salt in the origin. This will be a breaking change for the distributors and witnesses, but as there are a limited number of these at the moment and we can contact all of the operators, there isn't going to be a better time than now.

Edit: A quick note on how the current situation is already broken. Anyone running a witness with the configuration linked above will be unable to witness both of the rekor logs. One of the logs will always fail, with log spam such as feeder.go:143] failed to parse CP from witness: got Origin "rekor.sigstore.dev - 2605736670972794746" but expected "rekor.sigstore.dev - 3904496407287907110"

@mhutchinson mhutchinson self-assigned this Jan 23, 2023
mhutchinson added a commit to mhutchinson/trillian-examples that referenced this issue Jan 23, 2023
This currently fails because of the colliding Rekor configs. This PR should not be merged until transparency-dev/formats#14 is addressed and the dependency here is updated.
mhutchinson added a commit to mhutchinson/formats that referenced this issue Jan 23, 2023
mhutchinson added a commit that referenced this issue Jan 24, 2023
…n string (#15)

Previously the ID function only used the public key. For operators that reuse key material between logs (not recommended, but not the end of the world), this causes different logs to be mapped to the same ID. This defeats the primary reason for having an ID.

We now use both the origin string and the key material to compute the ID. This fixes #14. Any log operator using the same origin string and key for different logs is really doing things very wrong, and this is very much behaviour we don't want to support.
mhutchinson added a commit to mhutchinson/trillian-examples that referenced this issue Jan 24, 2023
This currently fails because of the colliding Rekor configs. This PR should not be merged until transparency-dev/formats#14 is addressed and the dependency here is updated.
mhutchinson added a commit to mhutchinson/trillian-examples that referenced this issue Jan 24, 2023
This currently fails because of the colliding Rekor configs. This PR should not be merged until transparency-dev/formats#14 is addressed and the dependency here is updated.
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 a pull request may close this issue.

1 participant