search

notaryproject / notation-go

A collection of libraries for supporting sign and verify OCI artifacts. Based on Notary Project specifications.
Apache License 2.0
39 stars 42 forks source link

fix: timestamping #478

Closed Two-Hearts closed 1 week ago

Two-Hearts commented 1 week ago

This PR adds a sanity check on timstamp value against the signing time: timestamp value should always be bounded after the signing time. This is to say, one cannot timestamp a signature before the signature itself been created. If it happens, fail the verification.

codecov[bot] commented 1 week ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 80.34%. Comparing base (7b9636e) to head (9c53750). Report is 1 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #478 +/- ## ========================================== + Coverage 80.33% 80.34% +0.01% ========================================== Files 34 34 Lines 3320 3323 +3 ========================================== + Hits 2667 2670 +3 Misses 508 508 Partials 145 145 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

priteshbandi commented 1 week ago

Do we need to account for clock skew such as when the local system clock runs faster than the timestamp server??

Two-Hearts commented 1 week ago

Do we need to account for clock skew such as when the local system clock runs faster than the timestamp server??

This change has been tested with several public tsa servers, such as digicert and globalsign. Since we cannot tell the difference between a clock skew and an intentionally manipulated local system clock, we should fail the verification on both cases following the secure by default practice.

In addition, this check also covers the scenario where the signer generates a Notary Project compliant signature using another tool and send it to tsa for timestamping later on, then attach the signature themselves. When verifying such signature envelopes in Notation, we expect the tsa timestamp to be later than the signing time as well. (given the timestamp being an unsigned attribute)

My suggestion is to follow the secure by default practice for now. If users do meet this issue, we could find a way to relax this validation in the future. @priteshbandi