jstedfast / gmime

A C/C++ MIME creation and parser library with support for S/MIME, PGP, and Unix mbox spools.
GNU Lesser General Public License v2.1
114 stars 36 forks source link

GMime parsing bad-header behavior differs from some common implementations #52

Open russianfool opened 5 years ago

russianfool commented 5 years ago

GMime has sub-optimal behavior when faced with malformed headers. There's some logic in parser_step_headers to proceed past some bad lines, but as-implemented there are header combinations that will fool GMime into stopping header parsing early.

Specifically, I want to focus on sub-sections of the RFC 7103, because clients (e.g. Outlook?, Evolution/Camel, Thunderbird) seem to implement approach 1, whereas GMime implements approach 2:

  1. Header Anomalies This section covers common syntactic and semantic anomalies found in a message header and presents suggested methods of mitigation. ... 7.2. Non-Header Lines

    Some messages contain a line of text in the header that is not a valid message header field of any kind. For example:

    From: user@example.com {1} To: userpal@example.net {2} Subject: This is your reminder {3} about the football game tonight {4} Date: Wed, 20 Oct 2010 20:53:35 -0400 {5}

The suggested ways of dealing with this kind of stuff are:

  1. Some agents choose to separate the header of the message from the body only at the first empty line (that is, a CRLF immediately followed by another CRLF).

    1. Some agents assume this anomaly should be interpreted to mean the body starts at line {4}, as the end of the header is assumed by encountering something that is not a valid header field or folded portion thereof.

    2. Some agents assume this should be interpreted as an intended header folding as described above and thus simply append a single space character (ASCII 0x20) and the content of line {4} to that of line {3}.

    3. Some agents reject this outright as line {4} is neither a valid header field nor a folded continuation of a header field prior to an empty line.

GMime mainly does 2 (with the message/content headers heuristic), and doesn't implement 3 (headers are never treated as a mis-fold and "recovered").

--- Below here might not be interesting in light of Outlook not actually implementing 3 either.

I found the rspamd thread on this - I'd like to improve the behavior though, if possible, instead of abandoning GMime altogether. What kind of error recovery is in line with your goals for GMime?

Currently I've modified the logic in parser_step_headers to be a bit more tolerant, but...

  1. For the pasting of lines, manipulating the appended line (ends up in raw_value) means we're changing the email representation.
  2. It's very hard to reconcile the "guess we've hit a broken mailer" jump-to-content code already in GMime and this kind of RFC 7103 approach 3 behavior. From my limited testing, Outlook doesn't care to jump to content, even for multipart parts.
  3. If we do move it outside (... e.g. roll a priv_headers_to_object or whatever with enough options to support all the construct callers, and store broken lines as key-less lines in the array), then that information isn't available to decide whether to end header parsing (and you might have to dump a bunch of fake headers and re-parse as body).

Moving it outside seems like a better approach (just because you have more flexibility in seeking across the different lines and don't have to worry about the 4k limit), but nothing about this screams elegant; what are your thoughts? I guess header_cb should be called on the re-folded version as well?

The two most-interesting cases RE actual samples:

  1. Stopping header parsing too early because of badly-folded headers. Outlook 2016, Thunderbird 60, etc. chew through these bad boys just fine. This sometimes causes GMime to miss important headers (boundaries, content types, etc.).

From: a.a@gmail.com To: b.b@gmail.com Message-ID: 12345@google.com Subject: Confirmed email X-IBM-SpamModules-Scores: A=1; B=2; C=3; D=4; E=5; X-IBM-SpamModules-Versions: A=1; B=2; C=3; D=4; E=5; F=6; G=7; H=8; Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: quoted-printable

...
  1. Malformed header lines that could benefit from re-folding, e.g. as described here:

    Content-Type: application/x-zip-compressed; x-unix-mode=0600; name="7DDA4_foo_9E5D72.zip"`

Although it does seem like the re-folding needs to be line-parsing-aware, since I have cases like:

Content-Type: application/x-zip-compressed; x-unix-mode=0600; name=" 7DDA4_foo_9E5D72.zip"`

Maybe that's overkill though, haven't tested whether common clients end up recovering the attachment name properly here.

jstedfast commented 5 years ago

The title isn't quite right since, as you said, GMime implements option 2 ;-)

It's late so I'm tired and won't be able to look at this tonight, but could you point me to the rspamd discussion? I have not seen this...

It seems like no matter what we do, someone isn't going to be happy :-)

The reason I chose option 2 is because the broken messages that I had users complain to me about GMime not handling in the past were clearly meant to be handled as option 2 - e.g. there was no blank line between the end of the headers and the start of the message body.

Obviously you must be encountering broken messages where this behavior should clearly be treated as option 3.

Unfortunately, this is only "obvious" to us humans and not easy to determine with a parser.

I also don't see it as particularly useful to have a "config option" to decide which approach to use since every message is going to be different and you can't possibly know ahead of time.

jstedfast commented 5 years ago

Looks like rspamd already switched away from GMime? I forgot I had a discussion almost 2 years ago with the author on why they switched (and I don't recall this particular issue coming up).

https://github.com/rspamd/rspamd/issues/1225

russianfool commented 5 years ago

The rspamd thread I meant is here. Right :)

jstedfast commented 5 years ago

I don't have a particular preference for GMime's behavior in this scenario since, for me, any message with this level of brokenness always tends to be spam anyway...

That said, perhaps @albrechtd, @gauteh, @pjstevns, and @dkg should be part of this discussion since they are all actively working on mail software using GMime and may have some preferences.

(I'm hoping @dkg can poke the relevant notmuch devs on this).

jstedfast commented 5 years ago

@russianfool it might also be worth sending a message to the https://mail.gnome.org/mailman/listinfo/gmime-devel-list mailing-list so anyone else who may not be monitoring the GitHub repo can chime in as well.

These types of changes always scare me :-(

In any case, if Outlook and Thunderbird implement option 3, perhaps we should match them.

What does Evolution do?

russianfool commented 5 years ago

@jstedfast : Fully agree in terms of being scared of these kinds of changes; the existing testing doesn't really cover this and it has pretty large ripple effects. Definitely also don't feel like you need to reply ASAP to these messages!

RE: Evolution Haven't tested it, or dug through their code (or Thunderbird's code). That's not a bad next step, they're just large projects and it's a lot to search through.

From a quick (well, couple of hour) dig through their code (do take this with a grain of salt)... Evolution: They're running Camel, from their evolution-data-server. Honestly the code looks almost identical to the GMime one, without the early-terminate and in-line ':' search additions (who came first?). See folder_scan_header. They do add a boundary check though.

Thunderbird is running some pretty simple "break at first blank line" in mimemsg.cpp (from the looks of it); they don't break early, but they also don't "recover" any values in mimehdrs.cpp. Their JSmime implementation seems almost identical in terms of behavior to their cpp implementation (just more regex).

russianfool commented 5 years ago

@jstedfast : Also, and this is my fault (I'm overworked and oversubscribed :P), I'd like to amend the "implement the more robust solutions" statement in the initial message. Outlook certainly does not seem to implement 3 (not for misfolded To:/Subject: lines, not for in-the-middle-of-param-value-misfold like name="hello.txt").

Maybe a more appropriate title for the ticket here is "GMime handles misfolded headers differently from common implementations"? Implementing 3 is then outside the scope of GMime and left as an "exercise for the reader", and then the discussion is just between 1 vs. 2 (or some hybrid, like with the heuristics).

jstedfast commented 5 years ago

I was one of the 2 main authors that wrote Camel back in the early 2000's - GMime was, at the time, my test-bed for new ideas that other GNOME mail & news clients were using because they didn't want to take a dependency on Evolution (obviously) :p

I didn't intend necessarily for you to go digging thru that code, lol, I just meant it more as a reminder to myself to see if anything had changed since I worked on it :)

Thanks for looking into these implementations, though, it's useful to know what the big players do since they likely set the "standard" for what is expected behavior in these scenarios.

albrechtd commented 5 years ago

Disclaimer # 1: I'm working in IT security, so I guess I'm somewhat biased here. Disclaimer # 2: Speaking for myself only, not for the entire group of Balsa developers.

From my experience, I must admit that I never saw any legitimate message which is broken as described above. There are still some half-broken MUA's around (mostly web mailers), but not to this degree, and all MTA's I know stick to the standards (even M$, more or less); at least they don't tamper with messages as described above. So I fully agree with @jstedfast that such broken messages are most likely spam, typically produced by inferior scripts…

I would also like to draw your attention to Steffen Ulrich's research into evading security appliances by misusing MIME, i.e. cases where the standards are not absolutely clear. And here we actually face a clear violation of the standards. IMO, the only sensible approach for a MUA (or even a MTA/milter) from the security POV is to reject broken messages completely (i.e. approach 4 of RFC 7103). Interpreting a broken message somehow by guessing what the spammer might want (as in approach 3 of RFC 7103) is basically a security risk. The fact that different MUA's (and security appliances) have different approaches (if I understand the above comments correctly) is a nice feature for an attacker: a message may look benign in, say, Apple Mail, but exposes malware in Outlook…

Obviously, the scope of a library like GMime slightly differs from that of a MUA. The latter should always reject/discard broken messages, and maybe notify the user. To this end, use g_mime_parser_options_set_warning_callback() to catch any issues. Regarding the library/parser itself, approach 2 from RFC 7103 as implemented in GMime is just fine for me. Please don't implement approaches which hide errors! Thus, I vote for keeping everything “as is”.

russianfool commented 5 years ago

@albrechtd / @jstedfast : Thank you for the thoughtful responses. Both this conversation and looking through the other implementations have been super helpful, even if it results in no changes to GMime proper.

I've read the noxxi.de HTTP evasions list, but didn't see that there was a similar list for mime (I know there are lots of evasions, but I haven't seen the base64 '=' padding before).

From my experience, I must admit that I never saw any legitimate message which is broken as described above. ... the only sensible approach for a MUA (or even a MTA/milter) from the security POV is to reject broken messages completely ... guessing what the spammer might want ... is basically a security risk.

Not all MTA's reject this kind of email unfortunately, meaning that it does end up in the hands of people from time to time. You could probably argue that choosing option 2 in GMime (over option 4) is participating in "guessing what the spammer might want" (on par with 1, but not nearly as much as 3). That's what heuristics like has_message_headers and similar are for, right?

On the topic of security risks, RFC 7103 says this:

  1. Security Considerations

    The discussions of the anomalies above and their prescribed solutions are themselves security considerations. The practices enumerated in this document are generally perceived as attempts to resolve security considerations that already exist rather than introducing new ones.

My own thoughts are somewhere in the middle. "True" security is pretty much an unsolvable problem, but there's plenty of interest in "good-enough" solutions and a lot of interest in being "better". Enough, at least, to keep a whole industry afloat (which, late Disclaimer # 1, I participate in). So if we can do better, why not? Had computing been standards-strict right out of the gate, maybe we'd be in a better spot now.

To this end, use g_mime_parser_options_set_warning_callback() to catch any issues. Regarding the library/parser itself, approach 2 from RFC 7103 as implemented in GMime is just fine for me. Please don't implement approaches which hide errors!

No, of course not! It's possible to still issue the warning and set an externally-visible parsing status, for instance, when something like this happens. But that's why I'm here, wanted to find out what kind of tolerance and recovery is "in scope" for GMime-proper (and for sage advice).

Otherwise - users can split parsing and recovery (I don't have a single original thought, that's from here). On failure, run the email through a dedicated recovery tool and try parsing again. Generate multiple versions as needed. ... of course, the recovery tool would have to also parse mime, but it's probably easier to do some operations outside of the parser_step_headers constraints.

albrechtd commented 5 years ago

I've read the noxxi.de HTTP evasions list, but didn't see that there was a similar list for mime (I know there are lots of evasions, but I haven't seen the base64 '=' padding before).

Please note that the GMime parser can detect all evasions Steffen describes (didn't check against the RFC, but at first glance they are all covered, too), with the exception of the padded etc. base64. GMime decodes it properly, so if you pass the decoded content to an antivirus product, malware might be detected. And detecting obfuscated base64 parts is trivial.

Not all MTA's reject this kind of email unfortunately, meaning that it does end up in the hands of people from time to time. You could probably argue that choosing option 2 in GMime (over option 4) is participating in "guessing what the spammer might want" (on par with 1, but not nearly as much as 3).

I think we should distinguish two use cases for the GMime library at this point:

  1. In a security product (or Milter). In this case the feedback about RFC errors and possible evasion attempts is required. It doesn't make much difference whether option 1 or 2 (or maybe even 3) is implemented, as long as an alert can be raised. If the message is interesting enough (whatever the definition may be), it might be inspected manually by a forensics expert, maybe using a tool also built on GMime.
  2. In a MUA. IMO, here option 4 is the only sensible one. Never display anything to the user which may be harmful. Experience has shown that people click on everything and open every attachment, even in the worst crafted campaigns.

Thus, it might be a good idea if the GMime parser could be run in either mode 2 (or 1 or 3) for use case 1, or in mode 4 for use case 2.

[snipped RFC 7103 Security Considerations]

My own thoughts are somewhere in the middle. "True" security is pretty much an unsolvable problem, but there's plenty of interest in "good-enough" solutions and a lot of interest in being "better" Enough, at least, to keep a whole industry afloat (which, late Disclaimer # 1, I participate in). So if we can do better, why not? Had computing been standards-strict right out of the gate, maybe we'd be in a better spot now.

I think we agree that the vast majority of messages containing the anomalies described in the RFC are spam or malware, right? Rejecting them is most likely what the user actually wants, which additionally improves security (you won't catch well-crafted Fancy Bear attacks, but that's a completely different topic).

And if a legitimate MUA with legitimate content creates messages with serious issues (i.e. not unencoded 8-bit chars etc., see the GMimeParserWarning WARN vs. CRIT items) – let the market solve the problem! If enough users cannot receive the broken messages, they will either fix their product, or it will disappear. So, basically, we might need to re-think the “be liberal what you accept” policy as it opens doors for attackers.

Otherwise - users can split parsing and recovery (I don't have a single original thought, that's from here). On failure, run the email through a dedicated recovery tool and try parsing again.

That's actually the work of the forensics expert, i.e. a tool for use case 1 above. Don't let the user do it. They will click on everything a recovery tool (“recovery means it's safe now, isn't it?”) spits out.

gauteh commented 5 years ago

My impression, from skimming through this, is that option 2 (current behaviour?) with a warning flag is safest approach for our use case.

russianfool commented 5 years ago

@gauteh : option 2 (with the extra "have we seen the message/part headers" checks to run option 3 - lite on a line-by-line basis) is GMime's current behavior, yes. When "option 3 - lite" is active, GMime will eat "key-less" headers and not add them to the header array (on write they will be missing), but will raise warnings.

@albrechtd & all: Thank you so much for all your thoughtful responses. I don't have much to give back for it at the moment; feel free to close this "issue" or otherwise mark it as a discussion in the meantime.

jstedfast commented 5 years ago

I think it's been a useful discussion either way, so thanks for bringing it up.