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

docs: invalid signing time fail to sign prompt improvement #829

Closed
fanndu opened this issue Nov 12, 2023 · 5 comments · Fixed by notaryproject/notation-core-go#173
Closed
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@fanndu
Copy link

fanndu commented Nov 12, 2023

Is your feature request related to a problem?

When the signing time is not within the validity period of the certificate, the signing failure message does not have a certificate expiration date, and the user needs to run other commands to check the validity period of the certificate, which is not a good experience.
./bin/notation sign localhost:5000/gateway@sha256:b992672d71a62c0a94cd8640f7c0db62ffb9de65317100742bf44892b179445f Error: certificate-chain is invalid, certificate with subject "CN=test,O=Notary,L=Seattle,ST=WA,C=US" was not valid at signing time of 2023-11-12 08:34:48 +0000 UTC

What solution do you propose?

It is a good experience to show the certificate validity period in the prompt.
./bin/notation sign localhost:5000/gateway@sha256:b992672d71a62c0a94cd8640f7c0db62ffb9de65317100742bf44892b179445f Error: certificate-chain is invalid, certificate with subject "CN=test,O=Notary,L=Seattle,ST=WA,C=US" was invalid at signing time of 2023-11-12 08:33:33 +0000 UTC. Valid from [2023-10-25 02:40:40 +0000 UTC] to [2023-10-26 02:40:40 +0000 UTC]

What alternatives have you considered?

N/A

Any additional context?

No response

@FeynmanZhou
Copy link
Member

FeynmanZhou commented Nov 13, 2023

Hi @fanndu ,

Thanks for the suggestion. You proposal sounds reasonable to me. It makes signers understand the validity of the certificate immediately when failed to sign images with an expired certificate.

Alternatively, signers can get the validity of a certificate by using notation certificate show --type <type> --store <name> <cert_fileName> as mentioned in the spec in this case. But I agree with showing validity in related error message as you mentioned above adds more clarity and convenience to signers.

/cc @notaryproject/notaryproject-notation-maintainers for inputs.

@yizha1
Copy link
Contributor

yizha1 commented Nov 13, 2023

Thanks @fanndu

As a side note, besides the error message for certificate expiry, we need to verify the signature expiry error message to make sure it is clear for users.

@fanndu
Copy link
Author

fanndu commented Nov 13, 2023

Hi @fanndu ,

Thanks for the suggestion. You proposal sounds reasonable to me. It makes signers understand the validity of the certificate immediately when failed to sign images with an expired certificate.

Alternatively, signers can get the validity of a certificate by using notation certificate show --type <type> --store <name> <cert_fileName> as mentioned in the spec in this case. But I agree with showing validity in related error message as you mentioned above adds more clarity and convenience to signers.

/cc @notaryproject/notaryproject-notation-maintainers for inputs.

Hi @FeynmanZhou,

Because of use default certificate, so user could't know which store type and store name, especilly when there are so many certs, as follow:
./bin/notation cert ls STORE TYPE STORE NAME CERTIFICATE ca expired-example expired-example.crt ca missing-example missing-example.crt ca test test.crt ca testgenerate testgenerate.crt ca valid-example valid-example.crt

And there is no cli command to show which cert is default. So i think we should show more cert info here and add new cli to show default cert info.

Signing key is define in /Users/xxx/Library/Application Support/notation/signingkeys.json. It's hard for beginners to find it.

@fanndu
Copy link
Author

fanndu commented Nov 14, 2023

@yizha1 I'm working on a PR to fix this.

@yizha1
Copy link
Contributor

yizha1 commented Nov 15, 2023

Thanks @fanndu, I will assign this issue to you. /cc @FeynmanZhou @shizhMSFT

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants