condemil commented 3 weeks ago

Issue

When sndmail command is called the additional TCP package with "\r\n.\r\n" contents is added after the DATA conversation is over.

TCP packages from WireShark

The message send during DATA

0000   1e 00 00 00 60 02 09 00 00 29 06 40 00 00 00 00   ....`....).@....
0010   00 00 00 00 00 00 00 00 00 00 00 01 00 00 00 00   ................
0020   00 00 00 00 00 00 00 00 00 00 00 01 f7 4a 04 01   .............J..
0030   19 ef c0 a4 1d cd e2 6d 80 18 18 e2 00 31 00 00   .......m.....1..
0040   01 01 08 0a fe 33 65 de f8 15 dc ed 74 65 73 74   .....3e.....test
0050   0d 0a 2e 0d 0a                                    .....

The unexpected "\r\n.\r\n" (0d 0a 2e 0d 0a)

0000   1e 00 00 00 60 02 09 00 00 25 06 40 00 00 00 00   ....`....%.@....
0010   00 00 00 00 00 00 00 00 00 00 00 01 00 00 00 00   ................
0020   00 00 00 00 00 00 00 00 00 00 00 01 f7 4a 04 01   .............J..
0030   19 ef c0 ad 1d cd e2 6d 80 18 18 e2 00 2d 00 00   .......m.....-..
0040   01 01 08 0a fe 33 65 de f8 15 dc ed 0d 0a 2e 0d   .....3e.........
0050   0a                                                .

Reason

This happens because of closeData trick that somehow sends dot twice during Close().

How to reproduce

Call sndmail

print -n "test" | sndmail test@example.com -f from@gmail.com

You can use this python script as a simple SMTP server to see the conversation with sndmail

import asyncio
import asyncio.selector_events

class SMTPServerProtocol(asyncio.Protocol):
    def __init__(self):
        self.data_mode = False
        self.data = ""
        self.peer = None

    def connection_made(self, transport: asyncio.Transport):
        self.transport = transport
        self.write(b"220 ESMTP\r\n")

    def data_received(self, data):
        print(f"< {str(data)[1:].strip("'")}")

        data = data.removesuffix(b"\r\n")
        lines = data.split(b"\r\n")

        for line in lines:
            self.process_line(line.decode())

    def process_line(self, message: str):
        if message.startswith("HELO") or message.startswith("EHLO"):
            self.write(b"250 Hello\r\n")
        elif message.startswith("MAIL FROM"):
            self.write(b"250 OK\r\n")
        elif message.startswith("RCPT TO"):
            self.write(b"250 OK\r\n")
        elif message.startswith("DATA"):
            self.write(b"354 End data with <CR><LF>.<CR><LF>\n")
            self.data_mode = True
        elif self.data_mode and message == ".":
            self.write(b"250 OK\r\n")
            self.data_mode = False
        elif message.startswith("QUIT"):
            self.write(b"221 Bye\r\n")
            self.transport.close()
            print()

    def write(self, msg: bytes):
        print(f"> {str(msg)[1:].strip("'")}")
        self.transport.write(msg)

async def main():
    port = 1025
    loop = asyncio.get_running_loop()
    server = await loop.create_server(lambda: SMTPServerProtocol(), "localhost", port)

    print(f"SMTP server running on port {port}...")
    async with server:
        await server.serve_forever()

if __name__ == "__main__":
    try:
        asyncio.run(main())
    except KeyboardInterrupt:
        print()

axllent commented 3 weeks ago

You are right, I did not realise that! Hmmm, trying to find a solution but failing so far. I'll keep looking. Thanks for reporting @condemil.

axllent commented 3 weeks ago

I have spent a few hours looking into this, and to be honest, I came up completely empty regarding a working (RFC compliant) solution. The only reason this code is structured like this is for logging purposes (ie: to log the status & return message). The default net/smtp package only returns an error (if there is an error), but not the server response code or message from the SMTP server.

In order to archive this, one has to provide a custom WriteCloser to both close the connection and return the status code & server message - but this is where I get stuck. That linked solution (where I originally got the solution, and where I see you've commented on) is actually the only "working" solution I could find ~ except the client.Text.DotWriter() appends the additional \r\n.\r\n which is what is causing the issue.

There must be another solution somewhere, but I just can't find it. Do you have any ideas?

condemil commented 3 weeks ago

I think custom data command with exposing response to log will do the job:

func sendMessage(c *smtp.Client, msg []byte) (int, string, error) {
    code, message, err := dataCmd(c)
    if err != nil {
        return code, message, err
    }

    err = writeMessage(c, msg)
    if err != nil {
        return 0, "", err
    }

    code, message, err = c.Text.ReadResponse(250)
    return code, message, err
}

func dataCmd(c *smtp.Client) (int, string, error) {
    id, err := c.Text.Cmd("DATA")
    if err != nil {
        return 0, "", err
    }

    c.Text.StartResponse(id)
    defer c.Text.EndResponse(id)
    code, message, err := c.Text.ReadResponse(354)

    return code, message, err
}

func writeMessage(c *smtp.Client, msg []byte) (error) {
    w := c.Text.DotWriter()
    defer w.Close()
    _, err := w.Write(msg);

    return err
}

axllent commented 2 weeks ago

@condemil - I can't say how grateful I am - no matter what I tried I was not able to solve that, but your solution works perfectly! Thank you so much. I have released v.0.0.5 with this fix, as well as another feature to add a Message-Id and Date header if those are missing (I noticed other sendmail implementations also do this).

Please confirm this works as expected for you now?

condemil commented 2 weeks ago

Looks like new version is not working anymore:

$ print -n "test" | ./sndmail test@example.com -f from@gmail.com
malformed header line: test

axllent commented 2 weeks ago

This would be because "test" is not a valid formatted email (structure). Whilst this would have worked previously for testing, it would have been rejected by a valid SMTP server due to the lack of headers. Sendmail is a MTA, it should not generate a message structure like mailx etc. With the other recent change I added (adding of Message-Id and Date) it now also tests to ensure the message has headers (else it fails ~ what you are getting).

Try something like: echo "Subject: Test\r\n\r\nTest"| ./sndmail test@example.com

condemil commented 2 weeks ago

sendmail accepts it

$ print -n "test" | sendmail test@example.com

axllent commented 2 weeks ago

Which sendmail would that be exactly - there are about 50 different implementations of it?

condemil commented 2 weeks ago

Postfix sendmail, the one pre-installed in macOS

axllent commented 2 weeks ago

Thanks - I'm investigating, but I see that:

sendmail from the ssmtp package works with just echo -n "test" | ..., and adds a "From" & Date header on a new line directly before the "test" meaning the headers are messed up because "test" is joined to the headers (not separated by a blank line) - so the message would be broken because the body and header is not separated by a blank line - so this would not work with a valid smtp server
sendmail from busybox works and adds a To header directly before the "test" meaning the headers are messed up (same as above) - so the message would be broken too
I can't seem to configure sendmail from postfix to send to any other server or port - it seems hardcoded to deliver to the local postfix server but I want to test it to the python script

Please can you tell me how you are configuring your sendmail implementation to send to a SMTP (I'm using the python script you provided earlier - very handy) server and not deliver to postfix? What I'm really interested in is what postfix's sendmail generates in your python script for echo -n "test" | sendmail test@example.com (the DATA part).

I guess my point is still that sendmail is just a MTA - it assumes that the message being passed to it is valid and already correctly formatted (ie: already has a header and body). The first two tests above also inject different headers, however that would break the email by the time it gets to the SMTP because the body and header are not separated by a new line. I also know that busybox's sendmail has a bug whereby it also converts existing \r\n line breaks into \r\r\n ~ which is one of the reasons I wrote sndmail in the first place.

I don't know about postfix's implementation because I can't test it. If you could provide the DATA displayed in your python script that would be helpful.

I can definitely make a change to sndmail so that it produces a valid email with header and body if it is provided with something that isn't valid (eg "test") - but the question is whether this is actually a good idea as it technically is not correct and not something a sendmail implementation should do in the first place.

condemil commented 2 weeks ago

I added this to /etc/postfix/main.cf on macOS:

relayhost = 127.0.0.1:1025

condemil commented 2 weeks ago

Here is the output for print -n "test" | sendmail test@example.com

> 220 ESMTP\r\n
< EHLO my-hostname.localdomain\r\n
> 250 Hello\r\n
< MAIL FROM:<my-username@my-hostname.localdomain>\r\n
> 250 OK\r\n
< RCPT TO:<test@example.com>\r\n
> 250 OK\r\n
< DATA\r\n
> 354 End data with <CR><LF>.<CR><LF>\n
< Received: by my-hostname.localdomain (Postfix, from userid 501)\r\n\tid DBBD8829BE5; Wed, 10 Jul 2024 12:39:13 +0700 (+07)\r\nMessage-Id: <20240710053913.DBBD8829BE5@my-hostname.localdomain>\r\nDate: Wed, 10 Jul 2024 12:39:13 +0700 (+07)\r\nFrom: my-username@my-hostname.localdomain (my-username)\r\n\r\ntest\r\n.\r\n
> 250 OK\r\n
< QUIT\r\n
> 221 Bye\r\n

And for print -n "test" | sendmail -f from@example.com to@example.com

> 220 ESMTP\r\n
< EHLO my-hostname.localdomain\r\n
> 250 Hello\r\n
< MAIL FROM:<from@example.com>\r\n
> 250 OK\r\n
< RCPT TO:<to@example.com>\r\n
> 250 OK\r\n
< DATA\r\n
> 354 End data with <CR><LF>.<CR><LF>\n
< Received: by my-hostname.localdomain (Postfix, from userid 501)\r\n\tid ED3D7829D7D; Wed, 10 Jul 2024 12:41:14 +0700 (+07)\r\nMessage-Id: <20240710054114.ED3D7829D7D@my-hostname.localdomain>\r\nDate: Wed, 10 Jul 2024 12:41:14 +0700 (+07)\r\nFrom: from@example.com (my-username)\r\n\r\ntest\r\n.\r\n
> 250 OK\r\n
< QUIT\r\n
> 221 Bye\r\n

axllent commented 2 weeks ago

Thanks. Yeah, in all fairness sendmail is not sending directly to your python script - it's postfix. Sendmail is passing the message to postfix, and postfix is relaying the message to python.

Unlike the other two sendmail implementations though, the final result has ...\r\n\r\ntest... - so there is an empty line between the headers and body which is correct.

It just goes to show how three different implementations here all handle things very differently.

I will (probably tomorrow) make a change to allow for this kind of behaviour, but I repeat, it is not sendmail's job to format emails, It is a MTA and not an email client. Some of these sendmail implementations "auto-correct" the input to a certain degree (I know that postfix accepts almost anything via SMTP which isn't technically RFC correct), but that is for convenience and half the time they break the message as I've demonstrated above. To use sendmail in this manner is dangerous and bad practice - you should format your email correctly before passing it to sendmail.

axllent commented 2 weeks ago

@condemil This should be working now for you in v0.0.6 - please confirm?

condemil commented 2 weeks ago

It works, thank you for fixing it

axllent commented 2 weeks ago

Great, and thank you again for your help fixing the original \r\n.\r\n bug!

axllent / sndmail

sndmail sends "\r\n.\r\n" after DATA is over #10

Issue

TCP packages from WireShark

Reason

How to reproduce