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

Using timeout as the restricting mechanism fo signature verification #201

Open
priteshbandi opened this issue Nov 11, 2022 · 8 comments
Open
Milestone

Comments

@priteshbandi
Copy link
Contributor

    had a discussion with @gokarnm on this and he suggested using timeout as the restricting mechanism which sounds more appt here.

We should also modify the VerificationOutcome behavior to return only successful signature verification outcome.

cc: @shizhMSFT @rgnote

Originally posted by @priteshbandi in #191 (comment)

@priteshbandi priteshbandi added this to the RC-1 milestone Nov 11, 2022
@priteshbandi
Copy link
Contributor Author

The idea here is to have a configurable default timeout for verify command and once we have reached that limit and havent found successful verification we will fail verification command.

@priteshbandi
Copy link
Contributor Author

Also, we might want to pull in #197 to RC1 which would provide cushion for verification timeout

@shizhMSFT
Copy link
Contributor

The idea here is to have a configurable default timeout for verify command and once we have reached that limit and havent found successful verification we will fail verification command.

The default timeout is hard to decide as we also need to consider network and CPU constraints. Basically, clients with better network and CPU power can verify more signatures and thus can further process the artifacts. Therefore, the verification result of artifacts is not deterministic if two clients verify the same artifact at the same time even the remote server returns the same response. It impacts the availability.

It seems more reasonable to limit the number of signatures attempted. At least, we are able to verify one signature.

Besides, I have no concerns to return successful outcome only.

@patrickzheng200
Copy link
Contributor

@shizhMSFT @priteshbandi in terms of only returning successful outcome, the change is included in PR #186 along with the refactoring of the verifier package. PTAL.
As for issue #197, I think we could create a new PR for it after the refactoring work of notation-go is done.

@yizha1
Copy link
Contributor

yizha1 commented Nov 14, 2022

My two cents:

#186 (comment)

@yizha1
Copy link
Contributor

yizha1 commented Nov 15, 2022

Per community discussions:

  • notation-go supports both options: timeout for verification and Max Signature Attempts
  • No default values configured in notation-go library. Requiring notation CLI or other library user to pass the values into the library
  • Need further alignment on the notation CLI default behavior, like what is the default option, what is the default value, and how user can change it. (We could create an issue to track it)
    @priteshbandi @shizhMSFT @patrickzheng200 @toddysm any comments?

@vaninrao10
Copy link

Define the CLI behavior and defaults in RC1 and implementation can be moved to RC2. The scenarios to be addressed will be as shown below but this is not the exhaustive list, but can be thought through more about it.

  1. What will happen if user specify only timeout? Will they still be restricted by number of signatures?
  2. What will happen if user specify only max number of signature? Will they still be restricted by default timeout?
  3. What if user specified both max number of signature and timeout?
    After we have answered above questions and more, next question would be whats the default values ? Once the behavior is discussed, update the Spec as part of RC1.

@yizha1
Copy link
Contributor

yizha1 commented Nov 22, 2022

This issue will be split to three issues

@yizha1 yizha1 modified the milestones: RC-1, Discuss Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

No branches or pull requests

5 participants