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.

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

crypto: remove wildcard options for checkEmail #41599

Closed
wants to merge 1 commit into from
Closed

crypto: remove wildcard options for checkEmail #41599

wants to merge 1 commit into from

Conversation

tniessen
Copy link
Member

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
@tniessen tniessen added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 19, 2022
@nodejs-github-bot nodejs-github-bot added crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to the documentations. labels Jan 19, 2022
@tniessen tniessen requested a review from jasnell January 19, 2022 18:18
@github-actions github-actions bot 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

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

@tniessen tniessen 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

@bnb bnb 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-actions github-actions bot mentioned this pull request Jan 20, 2022
Open
17 tasks
@tniessen tniessen 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
Member Author

+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
Member Author

cc @nodejs/crypto

@github-actions github-actions bot mentioned this pull request Jan 21, 2022
Open
19 tasks
@tniessen tniessen mentioned this pull request Jan 21, 2022
Closed
@github-actions github-actions bot 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

@panva panva closed this Jan 22, 2022
@tniessen
Copy link
Member Author

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]>
@ruyadorno ruyadorno 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]>
@danielleadams danielleadams 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 free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bnb bnb bnb approved these changes

@panva panva panva approved these changes

@jasnell jasnell Awaiting requested review from jasnell

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to the documentations. tls Issues 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