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

Check PTR against FQDN (including dot at the end) #28

Closed
wants to merge 1 commit into from

Conversation

fabian-z
Copy link

@fabian-z fabian-z commented Mar 16, 2024

PTR records are returned with a dot at the end (as FQDN) by hickory_resolver.
Without this fix, SPF checks for some mail providers (for example Yahoo and OVH) will fail without indicating errors.

Example program, showing the issue for Yahoo (easy testing due to free / open registration)

use mail_auth::Resolver;

// This is the main function.
#[tokio::main]
async fn main() {
    let resolver = Resolver::new_google().unwrap();

    let result = resolver
        .verify_spf_sender("87.248.110.31".parse().unwrap(), "sonic307-54.consmr.mail.ir2.yahoo.com", "my-local-domain.org", "testmail@yahoo.com")
        .await;
    println!("{}", result.result());

}

Expected result: Pass
Actual Result without this fix: Neutral (due to failure checking PTR records)

Thanks for your efforts!

This matches what is returned by hickory_resolver.
Without this, SPF checks e.g. for OVH and Yahoo will fail.
@fabian-z
Copy link
Author

@mdecimus Again thanks for your efforts! Would be great to get this in for Stalwart mail-server 0.7.0, too. Please let me know if I can provide further assistance.

Tests are passing after providing test records matching those returned by hickory_resolver:

running 33 tests
test common::headers::test::chained_header_iterator ... ok
test common::headers::test::header_iterator ... ok
test common::headers::test::header_parser ... ok
test common::resolver::test::reverse_lookup_addr ... ok
test common::base32::tests::base32_hash ... ok
test common::auth_results::test::authentication_results ... ok
test dkim::parse::test::dkim_report_record_parse ... ok
test dkim::canonicalize::test::dkim_canonicalize ... ok
test dkim::parse::test::dkim_record_parse ... ok
test dkim::verify::test::dkim_strip_signature ... ok
test dkim::parse::test::dkim_signature_parse ... ok
test dmarc::parse::test::parse_dmarc ... ok
test dmarc::verify::test::dmarc_verify ... ok
test mta_sts::parse::tests::mta_sts_record_parse ... ok
test mta_sts::parse::tests::tlsrpt_parse ... ok
test dkim::sign::test::dkim_sign ... ok
test report::arf::generate::test::arf_report_generate ... ok
test dmarc::verify::test::dmarc_verify_report_address ... ok
test report::arf::parse::test::arf_report_parse ... ok
test arc::verify::test::arc_verify ... ok
test report::tlsrpt::parse::tests::tlsrpt_parse ... ok
test report::dmarc::parse::test::dmarc_report_parse ... ok
test spf::parse::test::parse_ip4 ... ok
test spf::macros::test::expand_macro ... ok
test spf::parse::test::parse_ip6 ... ok
test report::dmarc::generate::test::dmarc_report_generate ... ok
test spf::parse::test::parse_spf ... ok
test report::tlsrpt::generate::test::tlsrpt_generate ... ok
test report::dmarc::parse::test::dmarc_report_eml_parse ... ok
test dkim::verify::test::dkim_verify ... ok
test dkim::sign::test::dkim_sign_verify ... ok
test spf::verify::test::spf_verify ... ok
test arc::seal::test::arc_seal ... ok

test result: ok. 33 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.65s

@fabian-z fabian-z changed the title Lookup PTR against FQDN (including dot at the end) Check PTR against FQDN (including dot at the end) Mar 16, 2024
@mdecimus mdecimus closed this in 3378f96 Apr 3, 2024
@mdecimus
Copy link
Member

mdecimus commented Apr 3, 2024

Hi @fabian-z , thanks for reporting this and also the PR! I have fixed this bug by stripping the dot from Hickory's response.

This fix will be included in version 0.7.0 of Stalwart Mail Server.

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.

None yet

2 participants