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

consider localhost as invalid domain #255

Merged
merged 3 commits into from
Feb 3, 2025

Conversation

bigio
Copy link
Contributor

@bigio bigio commented Jan 28, 2025

fix issue #254

@msimerson msimerson changed the title consider localhost as a valid domain, fix issue #254 consider localhost as a valid domain Jan 28, 2025
@marcbradshaw
Copy link
Collaborator

question, should localhost be considered valid for DMARC?

It is called in 3 places

lib/Mail/DMARC.pm
48: croak "invalid envelope_to" if !$[0]->is_valid_domain( $[1] );
54: croak "invalid envelope_from" if !$[0]->is_valid_domain( $[1] );
60: croak "invalid header_from" if !$[0]->is_valid_domain( $[1] );

I would argue that foo@localhost in the From header is an invalid header_from

RFC7489 6.6.1 states

o Messages with an RFC5322.From field that contains no meaningful
domains, such as RFC 5322 [MAIL]'s "group" syntax, are typically
ignored.

I would argue that @localhost is not a meaningful domain in this context.

we should also lc $domain before checking, to account for @localhost etc.

It may also be worth covering .localdomain here too?

@bigio
Copy link
Contributor Author

bigio commented Jan 29, 2025

localhost case fixed, should @localhost.localdomain be a valid domain in RFC7489 context ?

@msimerson
Copy link
Owner

localhost case fixed, should @localhost.localdomain be a valid domain in RFC7489 context

Nope. I think a general rule might be, "if the domain doesn't resolve to public IPs, it's invalid."

@msimerson
Copy link
Owner

msimerson commented Jan 29, 2025

we should also lc $domain before checking, to account for @localhost etc.

It turns out, I already have a commit for lower casing in my local repo. Landing soon in #256

lib/Mail/DMARC/Base.pm Outdated Show resolved Hide resolved
@msimerson msimerson merged commit 7ffb97d into msimerson:master Feb 3, 2025
5 checks passed
@msimerson msimerson changed the title consider localhost as a valid domain consider localhost as invalid domain Feb 3, 2025
@msimerson msimerson mentioned this pull request Feb 3, 2025
msimerson added a commit that referenced this pull request Feb 3, 2025
- consider localhost as invalid domain #255
- add a stringify method to the Policy class #253
- lower case domains passed to is_valid_domain #252
- lower case match from and envelope-from domains #249
- Change validation result for RFC7489 6.6.3 step 6.2 #248
- point README links to search.cpan.org #240
- sender: set options when creating new sender object #239
- permit storage of UTF-8 chars in MySQL DB #238
- load report_store modules using Module::Load #237
- create an email message with a proper Message-ID #236
- imap: only use port 143 if requested #235
- find_psl_file: fix duplicate share in path #232
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.

3 participants