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

add SigningTimeNotValidError error #25

Merged
merged 2 commits into from
Jul 22, 2024

Conversation

jessepeterson
Copy link

Name of feature:

Add a specific typed error for an invalid signing time attribute invalid error.

Pain or issue this feature alleviates:

See discussion in #21.

Why is this important to the project (if not answered above):

Allows the capability specific handling of a signing time attribute by the caller of .Verify() via e.g. errors.Is(). This would expose the specific signing time time.Time object along with the NotAfter and NotBefore of the signer.

Is there documentation on how to use this feature? If so, where?

As an exported symbol the Go documentation should auto-generate once a release is cut.

In what environments or workflows is this feature supported?

Anybody using the P7 library.

In what environments or workflows is this feature explicitly NOT supported (if any)?

I suppose this error is never called if there's no signing time attribute (or it is valid).

Supporting links/other PRs/issues:

Again, see #21.

💔Thank you!

@jessepeterson jessepeterson changed the title add SigningTimeNotValidErr error add SigningTimeNotValidError error May 1, 2024
Comment on lines +61 to +62
NotBefore time.Time // NotBefore of signer
NotAfter time.Time // NotAfter of signer
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it makes sense to include these because the caller of p7.Verify() will also have access to the signer cert which can directly access the NotBefore/NotAfter. On the other hand it's nice and explicit to have the actual checked values right here in the error, too.

@hslatman hslatman enabled auto-merge July 22, 2024 11:08
@hslatman hslatman merged commit 08783c3 into smallstep:main Jul 22, 2024
1 check passed
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 this pull request may close these issues.

2 participants