Title: Implement AUTH command in smtpd.
Type: enhancement Stage: resolved
Components: email Versions: Python 3.5
Status: closed Resolution: out of date
Dependencies: Superseder:
Assigned To: Nosy List: barry, jesstess, loewis, pitrou, python-dev, r.david.murray, zvyn
Priority: normal Keywords: patch

Created on 2014-07-07 21:00 by zvyn, last changed 2015-11-08 06:17 by r.david.murray. This issue is now closed.

File name Uploaded Description Edit
smtpd_AUTH.patch zvyn, 2014-07-07 21:00 Decryption and message processing for AUTH. (And nothing more!) review
smtpd_AUTH_full.patch zvyn, 2014-07-16 18:27 review
smtpd_AUTH_full2.patch zvyn, 2014-07-18 22:09 review
Messages (15)
msg222516 - (view) Author: Milan Oberkirch (zvyn) * Date: 2014-07-07 21:00
I implemented message processing for LOGIN and PLAIN authentication in smtpd. I also patched test_smtplib to make use of this functionality.

The goal for the API is to provide decryption and message processing in the smtpd library and call a externally provided or overwritten function(user, password) to verify credentials.

The patch provided is missing documentation and a proper API to use/activate this feature (and API specific tests of cause). Things which need to be discussed:
a) how shoud AUTH be activated? (when should MAIL etc. require AUTH? when should 'AUTH' be announced in the EHLO response?)
b) how should the programmer change the _verify_user_credentials method?

My idea to solve a and b at once would be to set the verification function by a keyword argument and require/activate AUTH if this kwarg is set.

I didn't implement CRAM-MD5 because it requires the correct password to be available in plain text.
msg222949 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-07-13 16:21
I think it would be a good idea to write the documentation.  It is much easier to get a feel for the API via docs than it is via code.  (That is, when you explain how to use the API, you sometimes find design bugs :)

for a/b: so you are thinking of an auth function passed in as opposed to a method overridden in the subclass?  That is consistent with how we enable other capabilities, so it has some attraction.

I haven't gone over the code in detail yet, but it looks like you need to add a reset of 'self.user' during the applicable state transitions.
msg223141 - (view) Author: Milan Oberkirch (zvyn) * Date: 2014-07-15 19:47
There is no real API in the current patch and authenticating has no effect (other then preventing you from authenticating again and storing the username). I am wondering how the user should turn AUTH on/off.

Solution 1:
add a keyword argument 'enable_AUTH' and require the programmer to override _verify_user_credentials. This function could
1.1 raise NotImplementedError
1.2 deny access
by default.

Solution 2:
add a keyword argument 'authentication_function' which turns AUTH support on when given and provides the function used to verify user credentials.

Solution 3:
enable AUTH if self has the _verify_user_credentials-function as attribute (and leave it undefined in the base class)

I think solution 1 is the most explicit so I'll implement that so we have something to discuss :)
msg223142 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-07-15 19:56
Describing how a programmer would implement authentication is exactly the API I was referring to, and that includes the signature and semantics of _verify_user_credentials.

I agree that (1) seems the cleanest.  I'd favor 1.1, NotImplemented, which lets the programmer see immediately what they did wrong.
msg223246 - (view) Author: Milan Oberkirch (zvyn) * Date: 2014-07-16 18:27
Done. I added the keyarg 'enable_AUTH' and two abstract methods to the server:
process_auth(user, password) for authentication and accept_recipient(user, mailfrom, rcptto) for authorization.
msg223283 - (view) Author: Milan Oberkirch (zvyn) * Date: 2014-07-16 21:25
We could solve issue 8503 at the same time by always calling 'accept_recipient(mailfrom, rcptto, user=None)' and providing a default implementation for backwards compatibility.
msg223322 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2014-07-17 08:31
RFC 4954 states

 Note: A server implementation MUST implement a configuration in which
   it does NOT permit any plaintext password mechanisms, unless either
   the STARTTLS [SMTP-TLS] command has been negotiated or some other
   mechanism that protects the session from password snooping has been
   provided.  Server sites SHOULD NOT use any configuration which
   permits a plaintext password mechanism without such a protection
   mechanism against password snooping.

So I'm -1 on this patch, and also on the feature until STARTTLS is implemented (and then this patch needs to be updated to conform to this requirement).
msg223327 - (view) Author: Milan Oberkirch (zvyn) * Date: 2014-07-17 10:31
My interpretation of this paragraph is the following (English is not my native language so please correct me if I'm wrong):
The requirement is to provide a configuration where plain auth is disabled if password snooping would be possible otherwise not to forbid such configurations generally. An admin SHOULD provide security measures to prevent password snooping. 

Setting enable_AUTH=False is a configuration where plain authentication is not permitted. The admin should provide a STARTTLS (or any other encrypted) tunnel if enabling AUTH (stunnel would be a common solution on Linux).

Maybe we should explicitly mention that in the docs?
msg223336 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-07-17 14:19
Providing starttls support would be the preferred solution, but that is a Hard Problem.  We probably need to rewrite smtpd using asyncio in order to provide starttls.
msg223352 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2014-07-17 18:09
Milan: Your interpretation of the MUST requirement is correct.

However, we still cannot support the SHOULD NOT requirement: A server operator SHOULD NOT accept unencrypted passwords. RFC 2119 explains

   This phrase, or the phrase "NOT RECOMMENDED" mean that
   there may exist valid reasons in particular circumstances when the
   particular behavior is acceptable or even useful, but the full
   implications should be understood and the case carefully weighed
   before implementing any behavior described with this label.

I cannot see any particular circumstances where unencrypted passwords for smtpd would be acceptable, given that there are perfectly established technologies. So I remain -1 on this patch.

A (not recommended) STARTTLS alternative is SMTPS (port 465). I would be -0 if there was an SMTPS implementation in smtpd, and the documentation would discuss that AUTH is best used with SMTPS until STARTTLS is implemented.

I don't understand why STARTTLS would require asyncio. Wouldn't wrap_socket solve the problem?
msg223354 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-07-17 18:55
I haven't looked at the problem myself.  Someone (Giampaolo?) told me that wrap_socket wouldn't work because of the fact that smtpd uses asynchat.

As for the 'particular circumstances' clause, I would suggest that one of the primary use cases for smtpd is in testing infrastructure (and that is in fact the motivation for this issue, for stdlib testing), and in that situation unencrypted passwords would not be an issue.

However, if wrap_socket/SMTPDS would work, it would make the feature useful beyond the testing arena.
msg223418 - (view) Author: Milan Oberkirch (zvyn) * Date: 2014-07-18 15:54
After trying to implement SMTPS with asyncore and wrap_socket I agree with David that it is at least hard: somehow the handshake fails (ssl.SSLWantReadError) and I did not really figure out why. Looking at the debugging output of openssl indicates that the connection drops immediately after setting up the session on the client side.

Anyway: I think we should apply a better version of my patch (will submit one soon) to be able to test smtplib (and also fix issue 8503). I'm going to make it clear that the AUTH functionality should only be used for testing or in combination with an encrypted tunnel.
msg245656 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2015-06-22 22:10
Martin says: "I cannot see any particular circumstances where unencrypted passwords for smtpd would be acceptable, given that there are perfectly established technologies. So I remain -1 on this patch."

Here's a use case: a testing SMTP server, such as lazr.smtptest which is built on top of smtpd.  In some testing scenarios, you really don't care about STARTTLS complications, and everything's connecting over localhost anyway.
msg254326 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-11-08 06:06
New changeset d13263ecf0c6 by R David Murray in branch '3.5':
#25446: Fix regression in smtplib's AUTH LOGIN support.
msg254328 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-11-08 06:17
I used some of this code in writing tests for the auth_login failure in issue 25446 (thanks, Milan, you saved me a bunch of work :)

Now that we have asyncio and asynchat is deprecated, we've decided that the only real purpose of smtpd going forward is the smtplib tests.  Any other smtpd use should be converted to aiosmtpd instead.  Perhaps that library will eventually make its way into the stdlib and test_smtplib can use it as well.

So, I'm going to close this issue, but we may want to consider if and how auth fits into aiosmtpd (maybe as part of a 'testing server').
Date User Action Args
2015-11-08 06:17:29r.david.murraysetstatus: open -> closed
resolution: out of date
messages: + msg254328

stage: resolved
2015-11-08 06:06:04python-devsetnosy: + python-dev
messages: + msg254326
2015-06-22 22:10:38barrysetmessages: + msg245656
2014-07-18 22:09:28zvynsetfiles: + smtpd_AUTH_full2.patch
2014-07-18 15:54:19zvynsetmessages: + msg223418
2014-07-17 18:55:32r.david.murraysetmessages: + msg223354
2014-07-17 18:09:14loewissetmessages: + msg223352
2014-07-17 14:19:10r.david.murraysetmessages: + msg223336
2014-07-17 10:31:03zvynsetmessages: + msg223327
2014-07-17 08:31:39loewissetnosy: + loewis
messages: + msg223322
2014-07-16 21:25:01zvynsetmessages: + msg223283
2014-07-16 18:28:02zvynsetfiles: + smtpd_AUTH_full.patch

messages: + msg223246
2014-07-15 19:56:10r.david.murraysetmessages: + msg223142
2014-07-15 19:47:43zvynsetmessages: + msg223141
2014-07-13 16:21:49r.david.murraysetmessages: + msg222949
2014-07-07 21:00:23zvyncreate