pepipost / BIMI-official

✨ Brand Indicators for Message Identification or BIMI ( 📢 pronounced: Bih-mee) is an emerging email 📧 specification that enables the use of brand-controlled logos within supporting email clients 🔮.
https://bimigroup.org/
MIT License
8 stars 5 forks source link

DMARC checks #5

Closed marcbradshaw closed 3 years ago

marcbradshaw commented 3 years ago
        else:
            dmarcRecord['status'] = dmarc['valid']
            searchpct = re.search(regex_pct, dmarc['record'])
            if dmarc['record'].find('p=quarantine')==-1 and dmarc['record'].find('p=reject')==-1:
                dmarcRecord['status'] = False
                dmarcRecord['errors'] = ["dmarc policy should be set to p=quarantine or p=reject for BIMI to work"]
            if searchpct and int(searchpct.group(0).split("=")[1]) != 100:
                dmarcRecord['status'] = False
            dmarcRecord['record'] = dmarc['record']

This section in CheckRecords is subtly wrong, the DMARC record must be enforcing, which means either a p=quarantine with an effective pct=100, or p=reject with any pct= value.

Additionally, I believe there is both a bug and an omission here

if dmarc['record'].find('p=quarantine')==-1 and dmarc['record'].find('p=reject')==-1:

We are not checking subdomain policy as required by the spec, and the find method call will match on those policies, such that a sp=reject policy will be matched by the .find('p=reject') search.

We also assume that records are properly formatted as lowercase, there are some in the wild which are not.

For example, "v=DMARC1; P=none; sp=reject;"

Personally, I dislike using regex to parse records with structure, matches can happen in unexpected places.

I don't see any code to check DMARC at the org domain level, do we do that?

Hiteshpandey commented 3 years ago

Thanks for the feedback. Have applied the suggested rules in the latest update. You can check that.