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

Part 1 of SC-62 related updates to zlint #739

Merged
merged 8 commits into from
Sep 17, 2023

Conversation

cardonator
Copy link
Contributor

@cardonator cardonator commented Sep 12, 2023

These are two of the more critical changes required by the SC-62 ballot that goes into effect on Sept 15th. The following changes were made in this PR:

  • Updated lint for common name handling. The definition for the CN field has switched from deprecated to NOT RECOMMENDED (essentially SHOULD NOT). An IneffectiveDate was added to the original lint. [This was based on nomenclature adopted from https://datatracker.ietf.org/doc/html/rfc2119#section-4]
  • Added a new lint for subscriber cert basic constraints checking. Post-SC62, basicConstraints MAY be included but MUST be critical if present.
  • Added a date for SC62 Effective

I did want to get eyes on the approach here. We don't usually use Ballot numbers in the effective dates, however these changes present a unique challenge since we have one lint that is no longer effective while a new lint becomes effective; additionally SC-62 makes changes to both the TLS BRs and the EVGs. Perhaps I could change that to the version of the TLS BRs going into effect on that date anyway? paging @christopher-henderson for feedback.

…d has switched from deprecated to NOT RECOMMENDED (essentially SHOULD NOT). An IneffectiveDate was added to the original lint.

Added a new lint for subscriber cert basic constraints checking. Post-SC62, basicConstraint MAY be included but MUST be critical if present.

Added a date for SC62 Effective
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.

@cardonator

We don't usually use Ballot numbers in the effective dates...

I believe that the issue that we are seeing here is that we have an efficacy date for a requirement that does not line up with the efficacy date of its containing BR.

While it's not what is usually done (indeed, most efficacy dates are tied to BR dates) there is precedence in utils/time.go of efficacy dates being declared against very specific requirements.

So, so long as the name of the efficacy date is at least moderately descriptive of where the date came from (and it is), then I believe that what you've done here is perfectly fine.

Restated, I think it's most important that it's clear where the date came from for tracing purposes. That the given date does not lineup with a release, and is referenced in multiple documents, should be fine so long as it is traceable to this SC.


Aside from that, I had two tiny code nits. It otherwise looks great.

cardonator and others added 2 commits September 17, 2023 13:56
Co-authored-by: Christopher Henderson <chris@chenderson.org>
Co-authored-by: Christopher Henderson <chris@chenderson.org>
@cardonator
Copy link
Contributor Author

Thanks, I accepted both of those changes and agree with your reasoning. Appreciate the review!

@christopher-henderson christopher-henderson merged commit 1fd1c0d into zmap:master Sep 17, 2023
4 checks passed
@cardonator cardonator deleted the SC62_lint_updates_part1 branch September 18, 2023 03:40
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.

None yet

2 participants