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

Add BIMI plugin (#8143) #8860

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add BIMI plugin (#8143) #8860

wants to merge 1 commit into from

Conversation

candrews
Copy link

@candrews candrews commented Jan 14, 2023

Brand Indicator Message Identification (BIMI) is an industry-wide standards effort to use brand logos as indicators to help email recipients recognize and avoid fraudulent messages.

See: https://datatracker.ietf.org/doc/draft-brand-indicators-for-message-identification/

Closes: #8143

Brand Indicator Message Identification (BIMI) is an industry-wide standards effort to use brand logos as indicators to help email recipients recognize and avoid fraudulent messages.

See: https://datatracker.ietf.org/doc/draft-brand-indicators-for-message-identification/

Signed-off-by: Craig Andrews <[email protected]>
@tomsommer
Copy link
Contributor

How do you prevent spam mails from impersonating/phishing others? I know the official RFC has some insane signing requirements, but would be nice to not have to force this. Maybe only show the images if they are not in the Junk folder? Or only show the BIMI if DKIM=pass?

@candrews
Copy link
Author

I know the official RFC has some insane signing requirements, but would be nice to not have to force this.

Indeed, I really don't want to have to implement that. Other providers aren't requiring certificates (aka VMCs) either; for example, Yahoo isn't requiring VMCs. As a user, I've added BIMI to my personal domain, and I'm certainly not going to pay $500+ for a VMC; I suspect there are many other people/organizations with that feeling too.

Maybe only show the images if they are not in the Junk folder?

Do you have any hints as to how the plugin can know if the current email is in the junk folder?

Or only show the BIMI if DKIM=pass?

That seems like a great idea, and I just noticed that the RFC suggests doing that: https://datatracker.ietf.org/doc/html/draft-brotman-ietf-bimi-guidance#name-bimi-processing-requirement And Apple does that, too.

I'm not sure how to read the current email's headers to tell if DKIM passed; I'll try to figure that out, but if you happen to know, I'd love a pointer to save me the trouble of figuring it out on my own :)

@tomsommer
Copy link
Contributor

tomsommer commented Jan 19, 2023

Apple seems to require the certificate still.

I meant dmarc=pass, of course - DKIM is not enough :)

You can look at https://github.com/pimlie/authres_status for inspiration, if you only look at DMARC for validation it shouldn't be that hard to check the Authentication-Results headers for dmarc=pass.

@cPanelBenjamin
Copy link

@candrews, reading the message headers is documented
https://github.com/roundcube/roundcubemail/blob/master/plugins/show_additional_headers/show_additional_headers.php
and spam headers discussed #6676

@ma3ki
Copy link

ma3ki commented Feb 6, 2023

I tried checking the Authentication-Results header, etc. FYI.
In my tests, the display location of the BIMI logo has been changed.

Below is what I am checking

  1. Authentication-Results header contains dmarc=pass
  2. spf record contains -all
  3. dmarc record contains p=reject or p=quarantine
  4. dmarc record does not contain sp=none
  5. dmarc record contains pct=100 or no pct=

bimi.php.txt
bimi_engine.php.txt
bimi_test

@tomsommer
Copy link
Contributor

It makes no sense to query DNS records live. Rely on the mail-headers.

@candrews
Copy link
Author

candrews commented Feb 6, 2023

Rely on the mail-headers.

Which mail header? I don't think there is one that specifies the necessary BIMI information.

@tomsommer
Copy link
Contributor

It was in reply to @ma3ki

@pep-un
Copy link

pep-un commented Aug 20, 2023

Hello,

It seams needed to query live DNS record for some information (as the policy) not present in the header.
is some teasting needet on this PR ? I will be happy to help for that part.

Regards,

@ghost
Copy link

ghost commented Sep 18, 2023

It would be great to support BIMI without requiring paid certification of course :)

I looked at the plugin code and its missing all the validation steps. Off the top of my head, the plugin should at least perform the following things:

Authentication requirements

  • If more than one RFC5322.From headers are present or if there is one RFC5322.From header but with multiple addresses, then BIMI processing must not be performed.

  • The DMARC result must be a pass or a quarantine with a percentage tag of 100% (pct=100) else BIMI processing must not be performed.

Assertion Record Discovery

  • Check for the BIMI selector header, use it if it exists otherwise use 'default' as the selector.

  • Records must start with a "v=" tag and current version else BIMI processing must not be performed.

Indicator Discovery Without Evidence

  • The TLS server certificate must be valid and trusted else BIMI processing must not be performed.

Indicator Validation

  • Check the size of the downloaded file and if found too big then BIMI processing must not be performed.

  • Check the file format is SVG or SVGZ and if not properly formatted then BIMI processing must not be performed.

@alecpl
Copy link
Member

alecpl commented May 17, 2024

I agree it needs more validation. Also maybe brandicon would be a better name for the plugin. Finally, the code stale needs to be fixed according to our new rules.

$res = $client->request('GET', $bimi_url);
if ( $res->getStatusCode() == 200 && $res->hasHeader('Content-Type') && strcasecmp($res->getHeader('Content-Type')[0], self::MIME_TYPE) == 0) {
$svg = $res->getBody()->getContents();
$svg = rcmail_attachment_handler::svg_filter($svg);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inlining arbitrary SVG into a page without even CSP and iframing equates installing a backdoor into every single message view. Removing a few specific XSS vectors that the filter knows about does not make that safe.

@Neustradamus
Copy link

@candrews: Can you update your PR with latest BIMI draft version?

@InputOutputZ
Copy link

I just had thought about this implementation and @ghost comment and recent draft, from user experience point of view which doesn't break the draft standards, I don't think it looks the best when loading multiple avatars for email account and displaying them, it just feels better if there is just one avatar, using following logic, if there is bimi present from the recipient message, current avatar should be replaced with bimi, and then alternative perhaps using gravatar or roundcube addressbook avatars.

Here are a relevant snapshots attached below from the bimi draft expiring on the 8th of May 2025.

It looks like the indicator can be displayed without evidence, as long as the uri with https transport call was made to retrieve the SVG/SVGZ, Also, size limit is not an issue, and receiver can proceed to display SVG as long as its valid even without evidence.

Screenshot 2025-02-27 at 20 55 14

Here it touches on personal avatars and BIMI, and it looks like if tag s=bimi is not present, then it should treated as the sender require BIMI to have preference while if was set to personal, then avatar should be used instead of BIMI, therefore its per this PR implementation I think its considered valid BIMI integration, since we still have choice to display personal avatar over BIMI, i.e. don't participate in BIMI whenever we want.

Screenshot 2025-02-27 at 20 56 14 Screenshot 2025-02-27 at 20 56 30

Lastly, BIMI processing is performed at MTA level and the only thing Roundcube needs to do either need to check message headers, which can be considered the secure way to fetch the SVG and decide whether to display it or not, when BIMI SVG is getting hashed and its hash truncation included in AR headers, otherwise retrieving SVG from DNS records with https call, its too considered safe perhaps safest since this bypasses the risk of message headers modification, and should be considered as the most valid BIMI source, and finally the SVG needs just to be validated and filtered from any other non-SVG tags and per comments, it looks like displaying the SVG via img tag src attribute is safer than displaying it via raw SVG element tag.
Screenshot 2025-02-27 at 21 39 56

Also, this bimi validator could be useful, I dont have experience writing code in perl and it would be great if anyone can convert it to PHP package.
https://github.com/marcbradshaw/bimi-validator
https://bimivalidator.authmilter.org

Also, this package, it might be helpful
https://github.com/biohzrdmx/bimini-php

I recognise that many of the MTAs have not integrated BIMI up until now and difficult to test, but I hope to see this PR closed, with fully rounded plugin ready to include as part Roundcube anytime soon.

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.

Display Brand Indicators for Message Identification (BIMI)
9 participants