fix: timestamping

[Two-Hearts]

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]

Codecov Report

All modified and coverable lines are covered by tests ✅

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

Additional details and impacted files

@@            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              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[shizhMSFT]

LGTM

[JeyJeyGao]

LGTM

[priteshbandi]

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

[Two-Hearts]

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

[priteshbandi]

LGTM