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

Allow ICANN TLDs as registration email domains #7212

Closed
wants to merge 4 commits into from

Conversation

aarongable
Copy link
Contributor

@aarongable aarongable commented Dec 13, 2023

We currently use the same logic to ensure that domain names are valid for certificate issuance and are valid for being the domain component of a subscriber's contact email address. However, we do not need to be as strict for email addresses: namely, it is possible for a subscriber to have an email whose domain component is exactly an ICANN TLD, while it is forbidden for us to issue a certificate to a name that is exactly an ICANN TLD.

Move the TLD logic out of ValidDomain and into its two callers: willingToIssue and ValidEmail. Slightly modify the logic in ValidEmail to not reject ICANN TLDs.

Fixes #5372
Fixes #4736

We currently use the same logic to ensure that domain names are valid for certificate issuance and are valid for being the domain component of a subscriber's contact email address. However, we do not need to be as strict for email addresses: namely, it is possible for a subscriber to have an email whose domain component is exactly an ICANN TLD, while it is forbidden for us to issue a certificate to a name that is exactly an ICANN TLD.

Move the TLD logic out of ValidDomain and into its two callers: willingToIssue and ValidEmail. Slightly modify the logic in ValidEmail to not reject ICANN TLDs.

Fixes #5372
Fixes #4814
@aarongable aarongable requested a review from a team as a code owner December 13, 2023 01:45
@aarongable
Copy link
Contributor Author

Note to reviewers: I don't love this solution, because ValidDomain is exported and could be called by some other code which is expecting it to enforce this ICANN TLD restriction. Currently the only other caller is the new ratelimits code:

boulder/ratelimits/names.go

Lines 145 to 148 in 260bbab

// validateDomain validates that the provided string is formatted 'domain',
// where domain is a domain name.
func validateDomain(id string) error {
err := policy.ValidDomain(id)

Maybe we can find some other solution for that?

@jsha
Copy link
Contributor

jsha commented Dec 14, 2023

Yes it is technically possible to have an email domain that is exactly an ICANN TLD (notable I've heard of email addresses @ai). But I think this is exceedingly rare and not worth supporting. It increases the likelihood of accepting garbage email addresses without meaningfully improving the service. I vote to close.

@jprenken
Copy link
Contributor

I agree it's not worth supporting newly added emails @ exact TLDs, but LE does have some legacy ones that made it into the DB before the check existed (#5372).

@aarongable
Copy link
Contributor Author

Honestly I'm not concerned about us not sending emails to a tiny subset of folks that signed up long ago; they haven't been receiving emails for a long time and haven't complained about it. The only reason to do this imo is to prevent the "wait, but this is a real email address!" case during new registration.

I'm totally happy to not land this, I don't feel strongly either way. But then I think we should close both attached bugs as WontFix to make our stance clear.

@aarongable
Copy link
Contributor Author

Closing in favor of #7220

@aarongable aarongable closed this Dec 15, 2023
@aarongable aarongable deleted the allow-tld-emails branch December 15, 2023 22:52
@kansal15
Copy link

Yes it is technically possible to have an email domain that is exactly an ICANN TLD (notable I've heard of email addresses @ai). But I think this is exceedingly rare and not worth supporting. It increases the likelihood of accepting garbage email addresses without meaningfully improving the service. I vote to close.

We have nic.in and gov.in added in PSL but also have more than 3 Million email ids over nic.in/gov.in
without the support of email ids available over PSL , all these users will not be able to register for LE

@kansal15
Copy link

I agree it's not worth supporting newly added emails @ exact TLDs, but LE does have some legacy ones that made it into the DB before the check existed (#5372).

nic.in and gov.in are added in PSL but also have more than 3 million email ids over nic.in/gov.in

@kansal15
Copy link

Honestly I'm not concerned about us not sending emails to a tiny subset of folks that signed up long ago; they haven't been receiving emails for a long time and haven't complained about it. The only reason to do this imo is to prevent the "wait, but this is a real email address!" case during new registration.

I'm totally happy to not land this, I don't feel strongly either way. But then I think we should close both attached bugs as WontFix to make our stance clear.

kindly consider to re-open this issue as all domains who are added in PSL but are not TLDs, will not be able to use their email ids for LE registration including myself and all other GOV.IN and NIC.IN email ids.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants