New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement AUTH command in smtpd. #66134
Comments
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: 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. |
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. |
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: Solution 2: Solution 3: I think solution 1 is the most explicit so I'll implement that so we have something to discuss :) |
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. |
Done. I added the keyarg 'enable_AUTH' and two abstract methods to the server: |
We could solve bpo-8503 at the same time by always calling 'accept_recipient(mailfrom, rcptto, user=None)' and providing a default implementation for backwards compatibility. |
RFC 4954 states Note: A server implementation MUST implement a configuration in which 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). |
My interpretation of this paragraph is the following (English is not my native language so please correct me if I'm wrong): 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? |
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. |
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 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? |
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. |
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 bpo-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. |
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. |
New changeset d13263ecf0c6 by R David Murray in branch '3.5': |
I used some of this code in writing tests for the auth_login failure in bpo-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'). |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: