axllent / mailpit

An email and SMTP testing tool with API for developers
https://mailpit.axllent.org
MIT License
5.55k stars 138 forks source link

POP3 end of file reached error #311

Closed antonionardella closed 3 months ago

antonionardella commented 4 months ago

Hello,

I am trying to set up mailpit to be used as mock email server for a staging installation of https://github.com/zammad/zammad according to their documentation here: https://admin-docs.zammad.org/en/latest/channels/email/index.html

This to see if the email are sent and received correctly and do further testing in a staging environment.

My issue is that once I set up mailpit and the pop3 connection parameters in zammad all I get is: end of file reached on Zammad's side

and ERRO[2024/06/12 13:20:15] [pop3] EOF on mailpits side.

I installed mkcert to create certificates and keys like so:

mkcert --install
mkcert 10.0.10.52

To generate certs for the IP of the test server

Then a pop3 auth file which contains nothing but user:pass

Then I start mailpit with

mailpit --pop3-auth-file ./mailpit.auth --pop3-tls-cert ./cert.pem --pop3-tls-key ./key.pem

What else could I try to make this work or debug it?

Thanks

antonionardella commented 4 months ago

I know that mailpit replies, because if I write the wrong password I get WARN[2024/06/12 14:39:24] [pop3] failed login: user

axllent commented 4 months ago

Sorry to hear you're having issues @antonionardella. I'm wondering if this was a result of a change we made in v1.18.3 (called dot-stuffing to prevent an email containing a single . on a line being interprited as the "end of the email"). Could you please try using v.1.18.2 (which does not have this fix) and let me know if it solves your issue? If it does, then it gives me some idea of where to look. If not (you still get the error), then I'm wondering if it has something to do with the TLS (you could then try without the TLS certs).

antonionardella commented 4 months ago

Hello @axllent!

Thank you very much for your response! image I installed the 1.18.2 version as suggested and the Zammad app is still giving the same error image

I tried also without TLS without success image image

SMTP works fine as expected image

But I am having issues with POP3 even with telnet image

This is uncharted territory for me, so please bear with my lack of knowledge.

axllent commented 4 months ago

That's all good. Port 1025 is for SMTP which isn't the problem here, it's POP3, so telnet localhost 1110 (followed by POP3 commands which you can see on sites like this, or this.

So at a guess it'll be something like:

telnet localhost 1110
...
USER user
...
PASS pass
...
LIST
...
RETR 1
...
QUIT

(I've added ... because that is where Mailpit should be answering you).

There are a lot more commands, but this should log you in as user user with the password pass, list your messages, retrieve the first message (print it on screen), and finally quit.

antonionardella commented 4 months ago

Alright, telnet works

image

At this point I suppose that there might be a different issue with Zammad

Thank you so much for your support. I'll come back if there is something else I can bring to the discussion

axllent commented 4 months ago

I suspect that may be the case (an issue with Zammad) - but I can' t be 100% sure as I don't know that software. I have tested with Thunderbird & telnet, but I'm assuming it should work with all POP3 clients. Zammad is open source, so maybe they can help?

antonionardella commented 4 months ago

Zammad is open source, yes. I posted to their forum https://community.zammad.org/t/pop3-issues-with-mailpit/14629 Let's see what happens next.

Again, thanks for your time.

antonionardella commented 4 months ago

Hello @axllent

I got this feedback from the zammad team:

It seems that Mailpit is not compatible to the way how the Net::POP3 Ruby library is fetching mails.

More technical information: net-pop/lib/net/pop.rb at e8d0afe2773b9eb6a23c39e9e437f6fc0fc7c733 · ruby/net-pop · GitHub

This is the line that is shown in your screenshot.

Not sure if there is time from your side to do anything about it, but at least we know more now.

m = /\A(\d+)[ \t]+(\d+)/.match(line) or
        raise POPBadResponse, "bad response: #{line}"

Each line is expected to contain a message number and a size, separated by whitespace. The regular expression \A(\d+)[ \t]+(\d+) is used to match lines that start with one or more digits (message number), followed by one or more spaces or tabs, followed by one or more digits (message size). If the line does not match this format, an exception POPBadResponse is raised with the message "bad response: #{line}".

Given this LIST response from mailpit when the LIST is called

LIST
+OK 5 messages (27300 octets)
1 %!d(float64=5461)
2 %!d(float64=5465)
3 %!d(float64=5350)
4 %!d(float64=5349)
5 %!d(float64=5675)

I see now where it might be incompatible.

According to the RFC 1939 https://www.rfc-editor.org/rfc/rfc1939#page-6

The list example looks like the following:

C: LIST
S: +OK 2 messages (320 octets)
S: 1 120
S: 2 200
S: .
....
C: LIST 2
S: +OK 2 200
...
C: LIST 3
S: -ERR no such message, only 2 messages in maildrop
antonionardella commented 4 months ago

Alright, the size part should be fixed with this PR: https://github.com/axllent/mailpit/pull/312

Now I have to understand why I am getting the end of file reached error

antonionardella commented 4 months ago

Further help from Zammad comes from

The end of file reached error comes from here: net-pop/lib/net/pop.rb at e8d0afe2773b9eb6a23c39e9e437f6fc0fc7c733 · ruby/net-pop · GitHub

But again, I don’t know what Mailpit is doing in this case, might be worth checking RFCs how POP3 should behave in such situations etc.

antonionardella commented 4 months ago

I was able to make it work with this commit https://github.com/antonionardella/mailpit/commit/e2f28f1dd846c55b828adf276c2b669d0476ec68

But it is also a huge refactoring of your code, so I am not pushing a PR unless you would like to do so.

This way Zammad was perfectly able to set up the POP3 mailbox without any issues so far

I was not able to compile the front-end on Windows (had no time to set up the node env), so please test it if possible

Have a great day!

axllent commented 4 months ago

@antonionardella Firstly thank you so much for looking into this, identifying the bug, and the PR. It's the start of another busy workday for me (plus I have another event this evening I have to attend) so I will look at this as soon as I'm able to (including your latest commit). If not today then tomorrow...

antonionardella commented 4 months ago

No hurry at all! Please.

Take your priorities first, I am only hoping that this is helping, more than making further mess (since I am not a professional developer).

Enjoy your day and weekend!

axllent commented 4 months ago

It is helping more than you know! The POP3 server implementation was originally a bit of a hack (and not something I personally use), so clearly some bugs have crept in over time, nor was it seemingly fully compliant. Compliance is something I strive for in Mailpit, so POP3 shouldn't be any different.

antonionardella commented 4 months ago

Readded also the websocket support https://github.com/antonionardella/mailpit/commit/5c33dd132a304be1b63f5a60e6ea9a33bd4cd43f

axllent commented 4 months ago

@antonionardella Firstly, I have merged the first PR - thank you, Mailpit was in fact printing garbage sizes, and the RFC 600-second timeout was a good catch too.

Secondly, I have been looking at your refactored branch which looks a lot nicer than my version, but which has some flaws in the logic (probably due to over-simplification). Before I get into that though, I'm curious as to what part exactly solved the issue with zammad? Was it just the fact that the sizes were incorrectly formatted (ie: LIST), or that you also needed to add sendResponse(conn, "LIST") to the CAPA list (I don't believe LIST is an expected CAPA response because it's just a standard POP3 command, not a "capability"), or was it something else?

Generally speaking with POP3:

In your refactored code however, you have greatly simplified the logic to just two states: UNAUTHORIZED & TRANSACTION. You set state = TRANSACTION (which in this context means "logged in") after USER, but you haven't checked the password yet. I realise that you don't load any messages until a valid password has been supplied, however you should not be allowed to run any of those other commands until being actually authorised.

In your refactor the following works when it shouldn't be allowed (note how there is no PASS):

telnet localhost 1110
Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.
+OK Mailpit POP3 server
USER user
+OK
LIST
+OK 0 messages (0 octets)
.

So to summarise - I like/prefer your refactored structure (it is much tidier), but there is a bit too much oversimplification of the logic which is resulting in non-compliant POP3. I think a few additional checks in your code would fix it though, but they are definitely needed. I'm also very curious to know what exactly fixed zammad.

antonionardella commented 4 months ago

Hello @axllent and thank you for your extensive feedback!

To add some context and pointers:

The first and merged PR (thank you!) fixed the size reporting issue described here: https://github.com/axllent/mailpit/issues/311#issuecomment-2165453243

After that, the ruby library, net-pop used by Zammad was only giving an end of file reached error as described here https://github.com/axllent/mailpit/issues/311#issuecomment-2164384970. This was blocking Zammad from adding the POP3 mailbox as email channel.

Once I applied my hack I was able to successfully add the mailpit POP3 mailbox to Zammad without errors, which also sends and retrieves a test email using the library. I did not have the time to test it through though.

I will look into it and add the checks to reach compliance and compatibility, thank you for taking the time to go through the code and add your valuable comments.

antonionardella commented 4 months ago

I took some time to refactor the code a little bit more: https://github.com/antonionardella/mailpit/commit/edf30413833c9efe64a6f2d8d77974233bb0116e

Now the code allows specific commands either in AUTHENTICATION or TRANSACTION state according to RFC2449

I also change the wording to make it easier to read the code according to the RFC (changed UNAUTHORIZED to AUTHENTICATION state).

Let me know if this works for you and is getting closer to the RFCs as expected.

I was able to compile it and test it with telnet and Zammad still no webUI tests though image

Best, Antonio

axllent commented 4 months ago

@antonionardella This code is looking much cleaner than my version, so thank you! I haven't tested it yet (I'm not able to today, but will try in about 24h) - sorry. From what I can see thought your implementation should work as expected. I'm still curious as to why Zammad is working with your version and not mine, do you have any idea? I promise to look into your version as soon as I'm able to, I'm just very busy right now and need to get to bed.

axllent commented 4 months ago

I should also add that the web UI is not important for this, in fact the two are not linked in any way. POP3 is only for POP3 clients, not the web UI 👍

antonionardella commented 4 months ago

I'm still curious as to why Zammad is working with your version and not mine, do you have any idea?

Good question, I added a handler specifically for EOF errors, which might be the key to the whole issue https://github.com/antonionardella/mailpit/blob/881ede5be4df3ef18d3e90defb6cd85bc756ccc4/server/pop3/pop3.go#L127C1-L132C10

axllent commented 3 months ago

Actually that's not really a handler, but rather just "hiding" (using the debug log) the logged EOF error - it shouldn't make any difference to the client connection. The only thing (I think) it tells me is that the Zammad client potentially doesn't send a QUIT when it is finished, but rather just breaks the connection instead when it is finished (if so then it's not RFC, and things like deleted messages won't actually get deleted). Please see below for more testing...

If Zammad isn't working after the first int64() fix, then I think it must actually be something else. Would you mind please confirming for me if the current develop branch of github.com/axllent/mailpit really doesn't work with Zammad? I understand that Mailpit will print an error, but my question is really whether Zammad can or can't add the POP3 server?

In addition to that, running Mailpit with the -v flag will print out exactly what the mail client is sending and receiving, for example:

DEBU[2024/06/18 21:44:11] [pop3] connection opened by 127.0.0.1:37918  
DEBU[2024/06/18 21:44:11] [pop3] response: +OK Mailpit POP3 server     
DEBU[2024/06/18 21:44:11] [pop3] received: CAPA (127.0.0.1:37918)      
DEBU[2024/06/18 21:44:11] [pop3] response: +OK Capability list follows 
DEBU[2024/06/18 21:44:11] [pop3] response: TOP                         
DEBU[2024/06/18 21:44:11] [pop3] response: USER                        
DEBU[2024/06/18 21:44:11] [pop3] response: UIDL                        
DEBU[2024/06/18 21:44:11] [pop3] response: IMPLEMENTATION Mailpit      
DEBU[2024/06/18 21:44:11] [pop3] response: .                           
DEBU[2024/06/18 21:44:11] [pop3] received: USER user (127.0.0.1:37918) 
DEBU[2024/06/18 21:44:11] [pop3] response: +OK                         
DEBU[2024/06/18 21:44:11] [pop3] received: PASS pass (127.0.0.1:37918) 
DEBU[2024/06/18 21:44:11] [pop3] response: +OK signed in               
DEBU[2024/06/18 21:44:11] [db] list INBOX in 274.908µs                 
DEBU[2024/06/18 21:44:11] [pop3] received: STAT (127.0.0.1:37918)      
DEBU[2024/06/18 21:44:11] [pop3] response: +OK 0 0                     
DEBU[2024/06/18 21:44:11] [pop3] received: QUIT (127.0.0.1:37918)      
DEBU[2024/06/18 21:44:11] [pop3] response: +OK Goodbye

Do you mind providing me with the output displayed (using-v` in Mailpit) when you try connect Zammad?

I've tested your version using Thunderbird and it works fine for me, so I'll be happy for another MR once we can find out exactly what was broken (if it was actually broken) after the previous int64() fix. Thanks!

antonionardella commented 3 months ago

What I did on the Zammad staging server:

Mailpit

Zammad

Here the Mailpit Debug log:

DEBU[2024/06/18 12:28:44] [pop3] connection opened by 127.0.0.1:46126
DEBU[2024/06/18 12:28:44] [pop3] response: +OK Mailpit POP3 server
DEBU[2024/06/18 12:28:44] [pop3] received: USER user1 (127.0.0.1:46126)
DEBU[2024/06/18 12:28:44] [pop3] response: +OK
DEBU[2024/06/18 12:28:44] [pop3] received: PASS password1 (127.0.0.1:46126)
DEBU[2024/06/18 12:28:44] [pop3] response: +OK signed in
DEBU[2024/06/18 12:28:44] [db] list INBOX in 358.598µs
DEBU[2024/06/18 12:28:44] [pop3] received: STAT (127.0.0.1:46126)
DEBU[2024/06/18 12:28:44] [pop3] response: +OK 0 0
DEBU[2024/06/18 12:28:44] [pop3] received: QUIT (127.0.0.1:46126)

image

Here the error in Zammad image

This with the latest version built from my fork:

Mailpit

Zammad Add account with same parameters

DEBU[2024/06/18 12:32:52] [pop3] connection opened by 127.0.0.1:35700
DEBU[2024/06/18 12:32:52] [pop3] response: +OK Mailpit POP3 server
DEBU[2024/06/18 12:32:52] [pop3] received: USER user1 (127.0.0.1:35700)
DEBU[2024/06/18 12:32:52] [pop3] response: +OK
DEBU[2024/06/18 12:32:52] [pop3] received: PASS password1 (127.0.0.1:35700)
DEBU[2024/06/18 12:32:52] [pop3] response: +OK signed in
DEBU[2024/06/18 12:32:52] [db] list INBOX in 348.689µs
DEBU[2024/06/18 12:32:52] [pop3] received: STAT (127.0.0.1:35700)
DEBU[2024/06/18 12:32:52] [pop3] response: +OK 0 0
DEBU[2024/06/18 12:32:52] [pop3] received: QUIT (127.0.0.1:35700)
DEBU[2024/06/18 12:32:52] [pop3] response: +OK Goodbye

image

No error in Zammad and asks for the next step, which is the SMTP setup image

At this point, the difference I see, is that the updated fork sends an +OK Goodbye message after the QUIT command

I hope that helps

axllent commented 3 months ago

Yes, I believe you are right! Good catch - Mailpit was just closing the connection without returning a response to the QUIT. Thanks, I missed that...

Would you mind opening another PR (you closed the original one) with your current code and I will get that merged in (and released as soon as possible)? Thanks @antonionardella!

antonionardella commented 3 months ago

PR is open

https://github.com/axllent/mailpit/pull/315

Thanks a lot for your questions, feedback, time and awesome project!

axllent commented 3 months ago

You're very welcome, and thank you for all your hard work finding & fixing the bug(s), plus refactoring the POP3 code (it is so much better now)!

I also made a few more additional changes:

Your changes, my changes, and some dependency updates to the Go & JS libraries have been released in v.1.18.6 :tada:

Thanks again! :heart: