postfix 3.5.X: Issue with reconstruction of long header lines (suspect: smtp)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

postfix 3.5.X: Issue with reconstruction of long header lines (suspect: smtp)

Andreas Weigel
Hi everyone,

I think I stumbled upon a problem with postfix 3.5.X and very long
header lines. To reproduce, I build a very simple setup with two VMs:
- "A" running postfix 3.3.0 (xubuntu 18.04 package)
- "B" running postfix 3.5.6 (xubuntu 20.10 package)

Both are relayhosts for each other and accept messages for their local
mailbox (A takes mail for intern01.local, B takes mail for
intern03.local). From my host, I transmit an email
(msg_longheader_3k.eml) for [hidden email] to B and and for
[hidden email] to A, which is then relayed to A and B, respectively.

Sending a long header line of 3000 bytes bytes to B and having it relay
to A, I end up with an incorrectly folded/chopped up header (see
attachment B2A) in the inbox. There is an additional line break after
the first line_length_limit (plus 4 for folding whitespace) bytes, which
marks the end of the header. The mail body then is filled up with the
remaining header bytes. Sending it in the other direction, using postfix
3.3.0 on A as relay, the message arrives intact and correctly folded
according to the setting of smtp_line_length_limit (see attachment A2B).

Additional observations:
- postfix 3.4.X behaves like postfix 3.3.X in this regard, the issue
does not occur there
- Sending the mail directly to a local mailbox, the message arrives
unchanged (neither chopped, nor folded).
- Increasing line_length_limit to some value larger than the length of
the header line prevents the described issue
- Increasing smtp_line_length_limit to a larger-than-header-line value
(but leave line_length_limit at 2048) does *not* prevent the issue;
instead, I get a single 2048-byte long line followed by two linebreaks
- It does not make a difference if TLS is used; I tried none/none
combinations for smtp(d)_tls_security_levels and
smtp_tls_security_level=encrypt on relay and
smtpd_tls_security_level=may on recipient.

Thus, I would suspect that there might be a problem in the code
responsible for reconstructing the chopped message within the smtp
module. Unfortunately, so far I haven't been able to put my finger on
the exact location.

Andreas


There is not much interesting stuff going on here.
Just some regular email plain text.

hhhhhhhhhhhhhiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiijjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbcccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffgggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggghhhhhhhhhhhhhhhhhhhhh
Date: 2020-11-03

There is not much interesting stuff going on here.
Just some regular email plain text.

There is not much interesting stuff going on here.
Just some regular email plain text.

A_postconf.txt (4K) Download Attachment
B_postconf.txt (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: (TENTATIVE patch): postfix 3.5.X: Issue with reconstruction of long header lines (suspect: smtp)

Viktor Dukhovni
On Wed, Nov 04, 2020 at 10:32:57AM -0500, Andreas Weigel wrote:

> Hi everyone,
>
> I think I stumbled upon a problem with postfix 3.5.X and very long
> header lines. To reproduce, I build a very simple setup with two VMs:
> - "A" running postfix 3.3.0 (xubuntu 18.04 package)
> - "B" running postfix 3.5.6 (xubuntu 20.10 package)
>

Yes, this looks like a bug introduced in Postfix 3.5 via:

    20190615

       Feature: SMTP/LMTP client support for 'D', 'O', 'R', 'X'
       flags similar to the pipe(8) daemon, to produce Delivered-To,
       X-Original-To, and Return-Path headers, and to indicate
       final delivery. Files: smtp/smtp.c, smtp/smtp.h, smtp/smtp_misc.c,
       smtp/smtp_proto.c, smtp/smtp_rcpt.c.

The new code that handles adding those headers was also reused to emit
all headers, incorrectly including logical headers that span multiple
physical lines:

    https://github.com/vdukhovni/postfix/commit/2ff7c91a461a093ff7a04a2ea9d1d3c0f1376243#diff-0e8480070aeb81a843eb195dc510a864f404ea03c5cdd597c6e00ab98d08de1cR2307-R2313

The value of "rec_type" needs to be passed to smtp_out_raw_or_mime() and
the output processed accordingly.  Untested patch below:

diff --git a/src/smtp/smtp_proto.c b/src/smtp/smtp_proto.c
index 7fd63486..2bb3d3ca 100644
--- a/src/smtp/smtp_proto.c
+++ b/src/smtp/smtp_proto.c
@@ -1389,17 +1389,17 @@ static void smtp_mime_fail(SMTP_STATE *state, int mime_errs)
 
 /* smtp_out_raw_or_mime - output buffer, raw output or MIME-aware */
 
-static int smtp_out_raw_or_mime(SMTP_STATE *state, VSTRING *buf)
+static int smtp_out_raw_or_mime(SMTP_STATE *state, int rec_type, VSTRING *buf)
 {
     SMTP_SESSION *session = state->session;
     int     mime_errs;
 
     if (session->mime_state == 0) {
- smtp_text_out((void *) state, REC_TYPE_NORM, vstring_str(buf),
+ smtp_text_out((void *) state, rec_type, vstring_str(buf),
       VSTRING_LEN(buf), (off_t) 0);
     } else {
  mime_errs =
-    mime_state_update(session->mime_state, REC_TYPE_NORM,
+    mime_state_update(session->mime_state, rec_type,
       vstring_str(buf), VSTRING_LEN(buf));
  if (mime_errs) {
     smtp_mime_fail(state, mime_errs);
@@ -1423,7 +1423,7 @@ static int smtp_out_add_header(SMTP_STATE *state, const char *label,
  vstring_str(session->scratch2),
  QUOTE_FLAG_DEFAULT | QUOTE_FLAG_APPEND);
     vstring_strcat(session->scratch, gt);
-    return (smtp_out_raw_or_mime(state, session->scratch));
+    return (smtp_out_raw_or_mime(state, REC_TYPE_NORM, session->scratch));
 }
 
 /* smtp_out_add_headers - output additional headers, uses session->scratch* */
@@ -2307,7 +2307,8 @@ static int smtp_loop(SMTP_STATE *state, NOCLOBBER int send_state,
  while ((rec_type = rec_get(state->src, session->scratch, 0)) > 0) {
     if (rec_type != REC_TYPE_NORM && rec_type != REC_TYPE_CONT)
  break;
-    if (smtp_out_raw_or_mime(state, session->scratch) < 0)
+    if (smtp_out_raw_or_mime(state, rec_type,
+     session->scratch) < 0)
  RETURN(0);
     prev_type = rec_type;
  }

--
    Viktor.
Reply | Threaded
Open this post in threaded view
|

Re: (TENTATIVE patch): postfix 3.5.X: Issue with reconstruction of long header lines (suspect: smtp)

Wietse Venema
Viktor Dukhovni:

> On Wed, Nov 04, 2020 at 10:32:57AM -0500, Andreas Weigel wrote:
>
> > Hi everyone,
> >
> > I think I stumbled upon a problem with postfix 3.5.X and very long
> > header lines. To reproduce, I build a very simple setup with two VMs:
> > - "A" running postfix 3.3.0 (xubuntu 18.04 package)
> > - "B" running postfix 3.5.6 (xubuntu 20.10 package)
> >
>
> Yes, this looks like a bug introduced in Postfix 3.5 via:
>
>     20190615
>
>        Feature: SMTP/LMTP client support for 'D', 'O', 'R', 'X'
>        flags similar to the pipe(8) daemon, to produce Delivered-To,
>        X-Original-To, and Return-Path headers, and to indicate
>        final delivery. Files: smtp/smtp.c, smtp/smtp.h, smtp/smtp_misc.c,
>        smtp/smtp_proto.c, smtp/smtp_rcpt.c.
>
> The new code that handles adding those headers was also reused to emit
> all headers, incorrectly including logical headers that span multiple
> physical lines:
>
>     https://github.com/vdukhovni/postfix/commit/2ff7c91a461a093ff7a04a2ea9d1d3c0f1376243#diff-0e8480070aeb81a843eb195dc510a864f404ea03c5cdd597c6e00ab98d08de1cR2307-R2313
>
> The value of "rec_type" needs to be passed to smtp_out_raw_or_mime() and
> the output processed accordingly.  Untested patch below:

Thanks, the untested patch passes the eyeball test. I introduced
an error that lost the queue file record type. Generating new headers
as REC_TYPE_NORM was OK, but losing the extsing record types was not.

Can the bug reporter confirm that this addresses the issue? Making
this automatically testatble will take some time.

        Wietse
Reply | Threaded
Open this post in threaded view
|

Re: (TENTATIVE patch): postfix 3.5.X: Issue with reconstruction of long header lines (suspect: smtp)

Andreas Weigel
I can confirm that the proposed patch fixes the issue. Just tested with
postfix 3.5.7 patched and unpatched.

On 04.11.20 12:18, Wietse Venema wrote:

> Viktor Dukhovni:
>> On Wed, Nov 04, 2020 at 10:32:57AM -0500, Andreas Weigel wrote:
>>
>>> Hi everyone,
>>>
>>> I think I stumbled upon a problem with postfix 3.5.X and very long
>>> header lines. To reproduce, I build a very simple setup with two VMs:
>>> - "A" running postfix 3.3.0 (xubuntu 18.04 package)
>>> - "B" running postfix 3.5.6 (xubuntu 20.10 package)
>>>
>> Yes, this looks like a bug introduced in Postfix 3.5 via:
>>
>>      20190615
>>
>>         Feature: SMTP/LMTP client support for 'D', 'O', 'R', 'X'
>>         flags similar to the pipe(8) daemon, to produce Delivered-To,
>>         X-Original-To, and Return-Path headers, and to indicate
>>         final delivery. Files: smtp/smtp.c, smtp/smtp.h, smtp/smtp_misc.c,
>>         smtp/smtp_proto.c, smtp/smtp_rcpt.c.
>>
>> The new code that handles adding those headers was also reused to emit
>> all headers, incorrectly including logical headers that span multiple
>> physical lines:
>>
>>      https://github.com/vdukhovni/postfix/commit/2ff7c91a461a093ff7a04a2ea9d1d3c0f1376243#diff-0e8480070aeb81a843eb195dc510a864f404ea03c5cdd597c6e00ab98d08de1cR2307-R2313
>>
>> The value of "rec_type" needs to be passed to smtp_out_raw_or_mime() and
>> the output processed accordingly.  Untested patch below:
> Thanks, the untested patch passes the eyeball test. I introduced
> an error that lost the queue file record type. Generating new headers
> as REC_TYPE_NORM was OK, but losing the extsing record types was not.
>
> Can the bug reporter confirm that this addresses the issue? Making
> this automatically testatble will take some time.
>
> Wietse