Skip to content

crypto: remove wildcard options for checkEmail#41599

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.

Sign up for GitHub

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

Jump to bottom
Closed
tniessen wants to merge 1 commit into from
Closed

crypto: remove wildcard options for checkEmail #41599

tniessen wants to merge 1 commit into from

Conversation

@tniessen
Copy link
Member

@tniessentniessen commented Jan 19, 2022

Wildcard options do not affect X509_check_email.

Refs: openssl/openssl#17536
Refs: #41571

@tniessen
crypto: remove wildcard options for checkEmail
971e41f
Wildcard options do not affect X509_check_email. Refs: openssl/openssl#17536 Refs: nodejs#41571
@tniessentniessen added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 19, 2022
@nodejs-github-botnodejs-github-bot added crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to the documentations. labels Jan 19, 2022
@tniessentniessen requested a review from jasnellJanuary 19, 2022 18:18
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 19, 2022
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jan 19, 2022

CI: https://ci.nodejs.org/job/node-test-pull-request/42034/

@tniessentniessen added the tls Issues and PRs related to the tls subsystem. label Jan 19, 2022
bnb
bnb approved these changes Jan 19, 2022
View reviewed changes
Copy link
Contributor

@bnbbnb left a comment

Choose a reason for hiding this comment

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

+1 if they truly have no effect

@github-actionsgithub-actionsbot mentioned this pull request Jan 20, 2022
Open
17 tasks
@tniessentniessen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 20, 2022
@tniessen
Copy link
MemberAuthor

tniessen commented Jan 20, 2022

+1 if they truly have no effect

To elaborate, these four options only affect how OpenSSL's X509_check_host handles *.example.com, a*.example.com, *b.example.com, etc. For email addresses, X.509 uses the RFC 822 mailbox syntax, which does not permit any of these wildcard patterns, and OpenSSL's X509_check_email will ignore wildcards regardless of whether these options are set or not :)

@tniessen
Copy link
MemberAuthor

tniessen commented Jan 20, 2022

cc @nodejs/crypto

@github-actionsgithub-actionsbot mentioned this pull request Jan 21, 2022
Open
19 tasks
@tniessentniessen mentioned this pull request Jan 21, 2022
Closed
@github-actionsgithub-actionsbot mentioned this pull request Jan 22, 2022
Open
17 tasks
panva pushed a commit that referenced this pull request Jan 22, 2022
@tniessen@panva
crypto: remove wildcard options for checkEmail
5aa4010
Wildcard options do not affect X509_check_email. Refs: openssl/openssl#17536 Refs: #41571 PR-URL: #41599 Reviewed-By: Tierney Cyren <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
@panva
Copy link
Member

panva commented Jan 22, 2022

Landed in 5aa4010

@panvapanva closed this Jan 22, 2022
@tniessen
Copy link
MemberAuthor

tniessen commented Jan 22, 2022

Thanks for reviewing, @bnb and @panva!

This was referenced Jan 23, 2022
Open
Open
Open
BethGriggs pushed a commit that referenced this pull request Jan 25, 2022
@tniessen@BethGriggs
crypto: remove wildcard options for checkEmail
9e39e62
Wildcard options do not affect X509_check_email. Refs: openssl/openssl#17536 Refs: #41571 PR-URL: #41599 Reviewed-By: Tierney Cyren <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
Linkgoron pushed a commit to Linkgoron/node that referenced this pull request Jan 31, 2022
@tniessen@Linkgoron
crypto: remove wildcard options for checkEmail
900128f
Wildcard options do not affect X509_check_email. Refs: openssl/openssl#17536 Refs: nodejs#41571 PR-URL: nodejs#41599 Reviewed-By: Tierney Cyren <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
@ruyadornoruyadorno mentioned this pull request Feb 8, 2022
Merged
danielleadams pushed a commit that referenced this pull request Feb 28, 2022
@tniessen@danielleadams
crypto: remove wildcard options for checkEmail
2a46a67
Wildcard options do not affect X509_check_email. Refs: openssl/openssl#17536 Refs: #41571 PR-URL: #41599 Reviewed-By: Tierney Cyren <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
danielleadams pushed a commit that referenced this pull request Mar 2, 2022
@tniessen@danielleadams
crypto: remove wildcard options for checkEmail
c3e20a4
Wildcard options do not affect X509_check_email. Refs: openssl/openssl#17536 Refs: #41571 PR-URL: #41599 Reviewed-By: Tierney Cyren <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
@danielleadamsdanielleadams mentioned this pull request Mar 3, 2022
Merged
danielleadams pushed a commit that referenced this pull request Mar 3, 2022
@tniessen@danielleadams
crypto: remove wildcard options for checkEmail
26cd8b3
Wildcard options do not affect X509_check_email. Refs: openssl/openssl#17536 Refs: #41571 PR-URL: #41599 Reviewed-By: Tierney Cyren <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
danielleadams pushed a commit that referenced this pull request Mar 14, 2022
@tniessen@danielleadams
crypto: remove wildcard options for checkEmail
fe0f9dc
Wildcard options do not affect X509_check_email. Refs: openssl/openssl#17536 Refs: #41571 PR-URL: #41599 Reviewed-By: Tierney Cyren <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@panvapanvapanva approved these changes

@bnbbnbbnb approved these changes

@jasnelljasnellAwaiting requested review from jasnell

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.cryptoIssues and PRs related to the crypto subsystem.docIssues and PRs related to the documentations.tlsIssues and PRs related to the tls subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@tniessen@nodejs-github-bot@panva@bnb