Use relaxed parsing for DNS names

fabian-z commented 8 months ago

This allows DNS labels used for lookups to contain underscores, which may not be allowed as host names.

Prevents false TempError result, which masks underlying proto error: Label contains invalid characters: Err(Errors { invalid_mapping, disallowed_by_std3_ascii_rules })

A minimal reproducing test program, by example of a service by MxToolbox (Deliverability Check) which uses a SPF chain (mxtoolbox.com -> mxtoolbox.com.hosted.spf-report.com -> mxtoolbox_com_d82a85cc_0.flat.spf-report.com):

use mail_auth::Resolver;

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

    // Verify MAIL-FROM identity
    let result = resolver
        .verify_spf_sender("143.55.235.77".parse().unwrap(), "pc235-77.mailgun.net", "my-local-domain.org", "test@mxtoolbox.com")
        .await;
    println!("{}", result.result());
}

As of main / v0.3.6 this will incorrectly fail with "TempError" during SPF validation, which masks the actual DNS name parsing error. With the code changes in this PR the program will (correctly) print "Pass".

Corresponding mail-server PR: https://github.com/stalwartlabs/mail-server/pull/172

fabian-z commented 5 months ago

@mdecimus Any feedback on this? This issue prevents real-world, standards compliant email delivery. We run the patched version for several months without further issues regarding this patch.

mdecimus commented 5 months ago

No need to update the PR, I made the changes directly on the main branch. Could you confirm it works for you so I publish it to crates?

fabian-z commented 5 months ago

Thanks for your efforts! Going to test examples with your version of the fix tomorrow. However, the corresponding mail-server fix would need changes too, since I used the function as a helper there to avoid a new dependency on hickory_resolver in the code :smile:.

If you want, I would gladly offer to modify the other PR too to make it compatible :+1:

fabian-z commented 5 months ago

Your changes in https://github.com/stalwartlabs/mail-auth/commit/8be9cb49f5f3e808718a6a1e5b5fa55fd30eeeb3 look good to me and fix the underlying problem. Thank you very much! :tada: Since this is fixed, I would like to look at the integration of this version and fix in mail-server / smtp-server. Would you prefer introducing a helper function here or usage of from_str_relaxed in mail-server?

mdecimus commented 5 months ago

Thanks, a new version of mail-auth has been published and Stalwart has been updated as well. Version 0.7.0 is going to include these fixes.

stalwartlabs / mail-auth

Use relaxed parsing for DNS names #25