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 lint for checking that the 'critical' field is properly DER-encoded in extensions #839

Merged
merged 48 commits into from
May 11, 2024

Conversation

defacto64
Copy link
Contributor

It is generally considered a requirement that certificates be DER encoded (meaning not BER), although RFC 5280 is not crystal clear on this. In any case, the non-properly-DER encoding of some certificate fields has historically been considered a compliance error, as it can be seen on https://bugzilla.mozilla.org, therefore it's useful to be able to detect it.

Among other things, DER stipulates that "the encoding of a set value or sequence value shall not include an encoding for any component value which is equal to its default value" (see X.690). Therefore, when a certificate extension is not critical, its 'critical' field should be omitted. This lint checks just that.

defacto64 and others added 30 commits March 8, 2024 16:07
Added //nolint:all to comment block to avoid golangci-lint to complain about duplicate words in comment
Fine to me.

Co-authored-by: Christopher Henderson <[email protected]>
As per Chris Henderson's suggestion, to "improve readability".
As per Chris Henderson's suggestion.
Added CABFEV_Sec9_2_8_Date
Copy link
Member

@christopher-henderson christopher-henderson left a comment

Choose a reason for hiding this comment

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

Thank you! This one was indeed tricky as I was attempted to full deserialize the type in to a Go type, which had all of the complications and gotchas written up in #639

@christopher-henderson christopher-henderson merged commit 26ab5b0 into zmap:master May 11, 2024
4 checks 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