rt icon indicating copy to clipboard operation
rt copied to clipboard

Update User.pm FriendlyFrom default

Open ciscoqid opened this issue 2 years ago • 6 comments

This PR sets a better default for FriendlyFrom as some mail filter services (like mimecast) frown on apparently-forged From headers. This makes sense as numerous MUA clients show only the comment as the sender. I think it makes sense here but a case could be made to perform the transformation in RT::Action::SendEmail::SetFrom instead.

ciscoqid avatar Aug 13 '21 21:08 ciscoqid

Hi ciscoqid, thanks for the PR. Could you add some more detail to explain how the current format causes issues with mimecast (with an example) and how your change fixes the issue (also with an example)? I would also look at the code around the $FriendlyFromLineFormat config option and consider updates there that would allow alternate configurations without changing the current default behavior.

cbrandtbuffalo avatar Aug 23 '21 12:08 cbrandtbuffalo

On 8/23/21 5:53 AM, Jim Brandt wrote:

Hi ciscoqid, thanks for the PR. Could you add some more detail to explain how the current format causes issues with mimecast (with an example) and how your change fixes the issue (also with an example)?

The problem we see is that mail received by Mimecast (and I assume others like Proofpoint, etc.) is often rejected as potentially forged when the From: line comment is an address in the recipient's domain.  The reason is simple -- phishers leverage as many MUAs hide the address in favor of the comment, thus tricking recipients into trusting them as legitimate.  So my choice is fix the format as I did (or another suitable method) or turn off the FriendlyFrom option entirely.  Prior to making this change locally, I was forced to manually edit each impacted user to add a real name to avoid the default email version.  I have tested this and it works perfectly, removing no useful information from email messages.

I would also look at the code around the $FriendlyFromLineFormat config option and consider updates there that would allow alternate configurations without changing the current default behavior.

I did look at this but it is not sophisticated enough to do what I needed.  The fields allowed include the name and email address, both of which are the same for auto-created users where there was no comment part to parse out.  I can't further post-process the email address the same way I did here via that variable.  If there were more options available for $FriendlyFromLineFormat to select fields and subfields, I would definitely consider it.

Thanks, Mark

ciscoqid avatar Aug 23 '21 15:08 ciscoqid

If there were more options available for $FriendlyFromLineFormat to select fields and subfields, I would definitely consider it.

Yep, agreed, that's why I am suggesting that a PR that makes that config option more configurable would be better for us than changing the default behavior. The goal would be to keep things working as-is for current users, and also provide a new way to fix the issue you found. Of course if your local change works for you, that's fine as well. Another option is to package it as an extension: https://docs.bestpractical.com/rt/5.0.1/writing_extensions.html

cbrandtbuffalo avatar Aug 24 '21 12:08 cbrandtbuffalo

Just adding a +1 to this - we've also seen mails blocked from mimecast for this reason - the propose patch looks like the ideal solution to keep as close as existing behavior.

I think making it configurable option is ideal, but I can imagine it's only a matter of time until this sort of anti-spoofing blocking becomes widespread as tricking people with the From name is a relevant phishing technique

danpoltawski avatar Jan 07 '22 10:01 danpoltawski

I have since run into even more crazy filters (e.g. Barracuda) and have adjusted to avoid including the raw domain in the default formatting. Not bothering to update the PR since it was made clear the change is not going to be accepted, but this might still help someone looking for a solution to this ever-growing problem. The adjusted logic is here:

my ($addr) = RT::EmailParser->ParseEmailAddress($name);
if (defined $addr and length $addr) {
    # avoid problems with mimecast and similar, which frown on apparent forged addresses
    my ($lhs,$domain) = split(/\@/, $addr);
    if (defined $lhs and defined $domain) {
        $domain =~ s/\./ (dot) /g;
        return "$lhs (at) $domain";
    }
}

ciscoqid avatar Jan 13 '22 17:01 ciscoqid

Thanks for the new example, @ciscoqid. As noted earlier, ideally we'd like to see a change that preserves backward compatibility and allows a flexible configuration to make it easy to handle new issues like this one. The more examples we have, the better we'll be able to confirm the new configuration option will handle them.

cbrandtbuffalo avatar Jan 14 '22 14:01 cbrandtbuffalo