albertito / chasquid / 7308830757

95%

main: 95%

Build Type

push

Committed by albertito

Commit Message

smtpsrv: Strict CRLF enforcement in DATA contents

The RFCs are very clear that in DATA contents:

> CR and LF MUST only occur together as CRLF; they MUST NOT appear
> independently in the body.

https://www.rfc-editor.org/rfc/rfc5322#section-2.3
https://www.rfc-editor.org/rfc/rfc5321#section-2.3.8

Allowing "independent" CR and LF can cause a number of problems.

In particular, there is a new "SMTP smuggling attack" published recently
that involves the server incorrectly parsing the end of DATA marker
`\r\n.\r\n`, which an attacker can exploit to impersonate a server when
email is transmitted server-to-server.

https://www.postfix.org/smtp-smuggling.html
https://sec-consult.com/blog/detail/smtp-smuggling-spoofing-e-mails-worldwide/

Currently, chasquid is vulnerable to this attack, because Go's standard
libraries net/textproto and net/mail do not enforce CRLF strictly.

This patch fixes the problem by introducing a new "dot reader" function
that strictly enforces CRLF when reading dot-terminated data, used in
the DATA input processing.

When an invalid newline terminator is found, the connection is aborted
immediately because we cannot safely recover from that state.

Note this also means the internal message representation will be using
\r\n, and thus the MDA and hook invocations will now see different (but
more compliant) line endings.

See https://github.com/albertito/chasquid/issues/47 for more details and
discussion.

Run Details

77 of 77 new or added lines in 2 files covered. (100.0%)

26 existing lines in 1 file now uncovered.

4391 of 4645 relevant lines covered (94.53%)

85474.31 hits per line