mirror of
https://github.com/axllent/mailpit.git
synced 2026-04-26 08:45:54 +03:00
[GH-ISSUE #56] [Feature] Always support authentication even without STARTTLS #38
Labels
No labels
awaiting feedback
bug
docker
documentation
enhancement
github_actions
invalid
pull-request
question
stale
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/mailpit#38
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @shizunge on GitHub (Feb 27, 2023).
Original GitHub issue: https://github.com/axllent/mailpit/issues/56
I am testing Vaultwarden which uses the library lettre sending emails. I set
SMTP_SECURITY=offand use empty username and password for testing. (I will refer vaultwarden as client below)Mailhog could receive the email. On the other hand, when connecting to mailpit, the client gives me
No compatible authentication mechanism was found.Digging into the code, it is due to mailpit does not send
250-AUTHin the EHLO response. We need the following condition to send250-AUTH.AuthHandleris set.So could we
smtpd.Server.AuthMechstomap[string]bool{"LOGIN": true, "PLAIN": true}Or set them to true via some CLI flags.AuthHandlereven if there is noconfig.SMTPAuthFileprovided. We can always return true in the new handler and log the username and password for debug purpose.The above settings may not make sense in a real SMTP server. But it is nice to have these options in mailpit.
@axllent commented on GitHub (Feb 27, 2023):
Hi there! If I understand you correctly, you are requesting an override to allow Mailpit to allow an insecure login? I don't believe that is a valid / allowed any more (authentication without TLS), it's even stated in that function's comment,
RFC 4954 specifies that plaintext authentication mechanisms such as LOGIN and PLAIN require a TLS connection.Can you please clarify what your reasoning is for not using TLS when authenticating, or why you are authenticating at all if insecure?
@shizunge commented on GitHub (Feb 27, 2023):
For me, everything is running locally, that is why I don't need TLS. And I don't actually need authentication.
The problem shows up that my client works with mailhog but not mailpit. And I guess is due to missing
250-AUTHinEHLO. It is comfirmed by building mailpit, always returning250-AUTH.I hope I can have an option to use a simply setup, minimizing the files and steps needed, like TLS keys. What I proposed is based on how to let the the stmpd library always return
250-AUTHinEHLOto keep my client happy.These setups do not make sence for real production stmp server. But for testing/verification, it could help us
@axllent commented on GitHub (Feb 28, 2023):
@shizunge Fair enough. I don't generally believe that "just because it worked in Mailhog it should work in Mailpit too" is a valid reason, especially when it's a non-RFC-compliant feature, however I can see the potential use-case for this in testing some mail client configurations.
From what I can see there are actually two separate features required here to get this working:
--allow-insecure-logins) - being that TLS is not set but authentication is required--allow-empty-logins) - to allow a blank username and password to be provided as a loginFrom what I understand, neither of these are standard (which surprises me that lettre would support this), so as you say, both of these features would need to be opt-in.
I will see how I get on and try add this at some point (I'm very busy at the moment so requests like this will have to wait unfortunately).
@shizunge commented on GitHub (Mar 3, 2023):
I drafted a pull request. If you had a time, please take a look.
@axllent commented on GitHub (Mar 3, 2023):
Thank you! I will definitely have a look at this in the next few days. I appreciate the effort.
@axllent commented on GitHub (Mar 7, 2023):
I am busy reviewing your PR, and trying to align it with my naming structure (and brain!). I believe there is some overlapping functionality between these two options, so I thought I would just check to see if I understand your intention correctly.
You introduce two new flags,
allow-logins-insecureandallow-logins-any.allow-logins-insecureThis should, if I understand correctly, allow username/password logins via a non-TLS connection? If I am I right, then I do not understand the logic of
srv.AuthMechs = map[string]bool{"LOGIN": true, "PLAIN": true}which sends the auth header (if not using TLS), but does not actually require auth. This logic seems horribly broken.Shouldn't this option simply (by default) require authentication (via
--smtp-auth-filehandled viaauthHandler()), but just not require TLS? The way you have implemented it is that it sends a authentication mechanisms header, but doesn't require a login (which sounds like the purpose ofallow-logins-any).allow-logins-anyI guess as the name implies, it just allows no authentication and authentication (accepts anything). This, when used in conjunction with
--allow-logins-insecurewould allow any non-encrypted SMTP request whether it contain a username/password or not.So what I think it should do here (and you will need to correct me if I am wrong) is this (and please don't change your code, I have imported your branch into a local testing branch and already made huge modifications, so there is no point):
--allow-logins-insecurewill require a--smtp-auth-file, and block using--smtp-ssl-certand--smtp-ssl-key~ running SMTP unencrypted--allow-logins-anywill block--smtp-auth-fileand require--smtp-ssl-certand--smtp-ssl-key~ running SMTP over TLS--allow-logins-insecure --allow-logins-anywill block--smtp-auth-file,--smtp-ssl-certand--smtp-ssl-key~ running SMTP unencrypted and allow any loginsWow, I hope I didn't confuse you there! Does this make sense, and is it what you are after? Based on what I understand, you require an unencrypted connection that accepts a blank username and password.
@shizunge commented on GitHub (Mar 7, 2023):
I don't think --allow-logins-insecure conflicts with --smtp-ssl-cert and
--smtp-ssl-key.
Setting all three --allow-logins-insecure --smtp-ssl-cert and --smtp-ssl-key
is same as just setting --smtp-ssl-cert and --smtp-ssl-key.
And Yes --allow-logins-insecure requires either --smtp-auth-file or
--allow-logins-any.
I let --smtp-auth-file take precedence. To me, "allow" defines a
less-restrictive
constraint, and --smtp-auth-file is more restrictive. When both
appear, the more
restrictive one wins. Same applies to --allow-logins-insecure v.s.
--smtp-ssl-cert and --smtp-ssl-key.
--
Shizun GE (蓋世尊)
On Mon, Mar 6, 2023 at 8:34 PM Ralph Slooten @.***>
wrote:
@axllent commented on GitHub (Mar 8, 2023):
Sorry, I believe I completely misunderstood the Authentication mechanisms & your question here, please bare with me here while I think aloud and ask for your opinion/confirmation here. I am just trying to ensure that I fully understand everything before I make any changes.
Re-reading
RFC 4954 specifies that plaintext authentication mechanisms such as LOGIN and PLAIN require a TLS connection, what this is actually saying is that both LOGIN and PLAIN are perfectly fine, provided it negotiates over TLS.So if I am to implement your request properly, as well as correct my existing misunderstanding of authentication requirements in SMTP, I would need to make the following changes to Mailpit:
--allow-logins-insecure. Currently Mailpit only allows authentication if TLS is used which I now understand is incorrect.If my understanding is correct then, in order for Mailpit to send the
250-AUTH ...header, then SMTP authentication should be required (shouldn't it?). I'm having great difficulty finding a clear explanation as to whether this should be sent when the server doesn't require authentication (all I know is mhale/smtpd doesn't send this when no auth is required).This brings me to your other feature (
--allow-logins-any). I believe that if used, this should require authentication (srv.AuthRequired = true), however will always return true (ie: any username/password), using one of the allowed authentication methods of course.Does it sound like I have a good understanding of the issue now? Please feel free to correct me, and I apologize for all the questions.
@shizunge commented on GitHub (Mar 10, 2023):
I think your understanding aligns with me.
I think this is already done.
@axllent commented on GitHub (Mar 12, 2023):
Thanks again for all your feedback & suggestions. In the end I must have created about 5 different variations while trying to integrate this in a manner I was happy with, but I think I got there in the end. I have just released v1.4.0 which contains both features (though different flags,
--smtp-auth-accept-any&--smtp-auth-allow-insecure). Although very different to your PR (code-wise) , it does follow the similar principals.When you get a chance, please have a play and let me know if this does what you expect it to? Thank you.
@shizunge commented on GitHub (Mar 15, 2023):
It works.
Thank you.
@axllent commented on GitHub (Mar 15, 2023):
Awesome, and thank you for all the feedback & testing! I'll close your PR now as it's no longer needed 👍