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

[BUG] verifier_domain is used as HELO argument and has much stricter requirements #18

Closed
AlexeyDemidov opened this issue Mar 26, 2019 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@AlexeyDemidov
Copy link

As verifier_domain is used as HELO/EHLO argument during SMTP session it has much stricter requirements than the documentation implies and default value derived from verifier_domain can cause failure to establish SMTP session. By RFC 5321 HELO/EHLO argument should contain FQDN of the server which initiated SMTP session, and many SMTP servers implement strict HELO checks where they check that IP which initiated connection has PTR record which matches FQDN presented in HELO. So, if this check is run on a server with IP 1.2.3.4 which has PTR record 1-2-3-4.hosterdomain.com then HELO argument should be 1-2-3-4.hosterdomain.com and also you need to make sure that 1-2-3-4.hosterdomain.com resolves into 1.2.3.4

@bestwebua bestwebua self-assigned this Apr 1, 2019
@bestwebua bestwebua added the enhancement New feature or request label Apr 1, 2019
bestwebua added a commit that referenced this issue Apr 14, 2019
bestwebua added a commit that referenced this issue Apr 17, 2019
Update linters config

Update linter config

Fix typo

Refactor, add raise_unless method

Refactor

Update gem version

Add base worker class, implement inheritance pattern

Refactor namespaces

Typo fix

Add Truemail::Auditor, Truemail::Auditor::Result tests

Update Truemail defined constants

Add tests

Complete Audit::Ptr tests

Add host_audit tests

Update readme

Update test
bestwebua added a commit that referenced this issue Apr 17, 2019
* Implement auditor feature, #18
@bestwebua
Copy link
Member

Hi, @AlexeyDemidov! Checkout PTR check feature in last gem release. What you think about SPF, DKIM and DMARC implementation for next auditor feature?

@AlexeyDemidov
Copy link
Author

I would suggest passing an IP to test as a parameter to host_audit (and I would call this function something different, like sender_verify). Currently, using Socket.gethostname and attempting to resolve resulting hostname into an IP is very unreliable way to get host IP - a host name can be non-FQDN (without full domain name) so it won't exactly match PTR, a hostname doesn't guarantee to resolve into any IP at all, it also won't resolve into actual IP used to connect to external servers (on many cloud VMs and/or servers behind NAT you'll get a private IP for a given hostname).

There is a missing step in PTR verification - it checks that PTR matches verifier_domain but it doesn't check that verifier_domain itself resolves into the given IP. For example, it possible to have an IP 1.2.3.4 to resolve into example.com but example.com itself can resolve into a different IP, like 4.3.2.1. In this case of mismatch many SMTP servers won't trust that email is coming from example.com and will mark it as suspicious.

Regarding SPF, DKIM and DMARC - it could be useful to implement DMARC and SPF checks as a part of sender verification functionality (DKIM is impossible to check on SMTP level) but parsing these DNS records is quite complex, so it will be necessary to use 3rd party gems to implement it (https://github.com/trailofbits/dmarc and https://github.com/trailofbits/spf-query). In long term it would be probably better to split sender verification functions into a separate gem.

@bestwebua bestwebua reopened this Apr 26, 2019
@bestwebua bestwebua added bug Something isn't working and removed enhancement New feature or request labels Apr 26, 2019
@bestwebua bestwebua mentioned this issue Apr 29, 2019
3 tasks
bestwebua added a commit that referenced this issue Apr 29, 2019
* Fix #current_host_address behavior, implement reverse trace
bestwebua added a commit that referenced this issue Apr 29, 2019
* Fix #current_host_address behavior, implement reverse trace

Update codeclimate config

Update linters config
@bestwebua
Copy link
Member

Alexey, thank for your details! I have fixed it. Cheers!

@bestwebua bestwebua changed the title verifier_domain is used as HELO argument and has much stricter requirements [BUG] verifier_domain is used as HELO argument and has much stricter requirements Nov 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

2 participants