trailofbits
Case-insensitive version tags (#24)
Fix #24, allowing case insensitive v= / V= tag name.
I think that this could be generalized to all tag names per the spec, but I'm not 100% sure. I'd like to start with that, and if that's acceptable, we could generalize the approach? Sounds like a topic ripe for bike-shedding. 😅
Note that RFC 7489 does not seem to mention case-sensitiveness at any point, and uses lower-case v= in the formal definition. The only place I found a mention of it is on this 2016 blog post (look for: "Legal Constructs That Might Not Be Recognized"), where the author wrote:
The string “DMARC” is the only portion of the DMARC policy record that is case-sensitive, but it is not uncommon for people reading the specification to miss this fact. For best results, keep everything but “DMARC” lower case.
With that in mind, I think it's also good to be a little relaxed in what a parser accepts.
Thanks for the PR @oz!
Note that RFC 7489 does not seem to mention case-sensitiveness at any point, and uses lower-case
v=in the formal definition. The only place I found a mention of it is on this 2016 blog post (look for: "Legal Constructs That Might Not Be Recognized"), where the author wrote:
I don't think this is the correct interpretation of the RFC: RFCs tend to be explicit about expected states, and the absence of a mention of cases/sensitivity generally implies that values should be parsed exactly as they appear in the RFC. In the case of DMARC, that means v= and not V=. In particular, 7.1 in the RFC (via https://github.com/trailofbits/dmarc/issues/24#issuecomment-2418002920) strongly suggests that v= is the only accepted prefix for the first tag.
With that in mind, I think it's also good to be a little relaxed in what a parser accepts.
Sorry, but I don't agree! I think Postel's law (i.e. "be liberal in what you accept") has caused a lot of exploitable bugs and unnecessary parser differentials over the years. The same argument led to decades of bad DER/BER bugs in X.509 parsing, JSON/XML differentials, etc.
With all of the above said, I think there are two routes by which I'd be okay with supporting V= here:
- Per https://github.com/trailofbits/dmarc/issues/24#issuecomment-2418002920, if you could find 1-2 examples of other DMARC parsers being permissive here, then we can cite those and have this be permitted by default.
- If it's hard to find examples of other parsers being permissive here, then I'm okay with adding some kind of "permissive" mode, which the user has to explicitly opt into. That would allow us to handle the
V=case, as well as a few of the other cases in that <dmarc.org> page you've linked.
Let me know what you think of either of these.
@woodruffw I agree. Having a relaxed parser could make one problem "go away", but if the rest of the Internet disagrees, maybe we're shooting ourselves in the foot, which is not a service for people that rely on this gem either.
I briefly toured some other languages implementations.
- Python's dmarc package accepts
V=DMARC1. - I'm less sure what's the most popular in the JS land, but this one's also accepting uppercase
V=. NPM is a bit of a mess though. - It looks like opendmarc doesn't care (used in Postfix milters, apparently).
- Rust's mail-auth crate isn't strict about
V=.
Some implementations are stricter though:
- Rust's dmarc crate from Cloudflare is strict about
v=(but less popular thanmail-auth). - The most popular go parser AFAICT is also strict.
I didn't dig further: it's chaos. 🎲✨
It would be interesting to run actual tests against Yahoo!/Outlook/Gmail as they're some of the largest inbox providers, so what they enforce is probably important to consider.
Thanks, I appreciate the diligence!
I looked up some stats for the packages that are more lenient, and it looks like all (except OpenDMARC, which is hard to quantify) are less "popular" (by download counts, which is admittedly a very bad metric) than the dmarc gem itself. So I'm having a hard time determining whether the dog is wagging the tail or vice versa here 🙂
How would you feel about adding a "permissive" mode for this case? I think that could be done by adding an optional permissive: false keyword arg to DMARC::Record::parse, which would then allow users to opt into more permissive parsing here.
I'm having a hard time determining whether the dog is wagging the tail or vice versa here 🙂
😆
How would you feel about adding a "permissive" mode for this case? I think that could be done by adding an optional
permissive: falsekeyword arg toDMARC::Record::parse, which would then allow users to opt into more permissive parsing here.
I think it's a perfectly reasonable approach. I'll try to make a second PR, before closing this one. 👍🏻