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

Upgrade zlint from v3.6.0 to v3.6.2 #7594

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

pgporada
Copy link
Member

@pgporada pgporada commented Jul 12, 2024

Adds a few new lints (largely related to the Profiles ballot and recent CA incidents), two of which we need to disable:

  • e_cab_dv_subject_invalid_values fails with a Warning because we include a Common Name in most of our certificates. We already ignore w_subject_common_name_included, so this is a similar situation.

  • w_ext_subject_key_identifier_not_recommended_subscriber fails with a Warning because we include the SKID extension in all of our certificates. We intend to remove this extension in our upcoming "modernized" certificate profile.

DO NOT MERGE until IN-10466 is complete

@pgporada pgporada requested a review from a team as a code owner July 12, 2024 18:59
@pgporada pgporada requested a review from aarongable July 12, 2024 18:59
Copy link
Contributor

@pgporada, this PR appears to contain configuration and/or SQL schema changes. Please ensure that a corresponding deployment ticket has been filed with the new values.

@pgporada
Copy link
Member Author

SRE ticket filed, IN-10466

@aarongable
Copy link
Contributor

It's worth noting that e_cab_dv_subject_invalid_values contains multiple checks, only one of which we violate. Skipping this lint is somewhat risky, if zlint decides to remove other individual lints which check for things like the Country field.

This is also a good prompt to consider removing the SKID from our end-entity certificates. I don't believe anyone is relying on it, and would be some good bytes to shed. (See #7446.)

Finally, we may want to consider having different sets of lints for different issuance profiles, so that a "modern" profile which excludes the Common Name can be more strictly checked.

Copy link
Contributor

@aarongable aarongable left a comment

Choose a reason for hiding this comment

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

I have merged main to resolve the merge conflict in go.mod.

This PR contains an update of the PSL. We should arguably do that in a separate PR, but it's also okay for it to be here.

All other transitive dependency updates LGTM.

The vast majority of the 400 zlint file diffs are just changing the copyright date. All other zlint updates (most of which are for S/MIME lints) look reasonable to me.

@aarongable aarongable requested a review from jsha July 17, 2024 19:42
@beautifulentropy beautifulentropy self-requested a review July 18, 2024 19:33
@mathewhodson
Copy link

It's worth noting that e_cab_dv_subject_invalid_values contains multiple checks, only one of which we violate. Skipping this lint is somewhat risky, if zlint decides to remove other individual lints which check for things like the Country field.

The unhelpful warning was removed in zmap/zlint@068ae82
So you could also upgrade or 3.6.3 or 3.6.4 instead of skipping that lint.

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.

4 participants