Merge branch 'master' into authenticator_see_session

# Conflicts:
#	aiosmtpd/smtp.py
#	aiosmtpd/testing/helpers.py
#	aiosmtpd/tests/test_smtp.py

.gitignore

@@ -33,3 +33,4 @@ diffcov.html
 diffcov-*.html
 .python-version
 .pytype/
+~temp*

README.rst

@@ -48,8 +48,26 @@ protocols.
 Requirements
 ============
 
-You need **at least Python 3.6** to use this library.  Both Windows and \*nix are
-supported.
+You need **at least Python 3.6** to use this library.
+
+
+Supported Platforms
+-----------------------
+
+``aiosmtpd`` has been tested on the following platforms (in alphabetical order):
+
+* Cygwin (on Windows 10) [1]
+* FreeBSD 12 [2]
+* OpenSUSE Leap 15 [2]
+* Ubuntu 18.04
+* Ubuntu 20.04
+* Windows 10
+
+  | [1] Supported only with Cygwin-provided Python version
+  | [2] Supported only on the latest minor release
+
+``aiosmtpd`` *probably* can run on platforms not listed above,
+but we cannot provide support for unlisted platforms.
 
 
 License

aiosmtpd/docs/NEWS.rst

@@ -2,6 +2,20 @@
  NEWS for aiosmtpd
 ===================
 
+1.2.4 (2021-01-24)
+==================
+
+Added
+-----
+* Optional (default-disabled) logging of ``AUTH`` interaction -- with severe warnings
+
+Fixed/Improved
+--------------
+* ``AUTH`` command line now sanitized before logging (Closes #233)
+* Remove special handling for lone ``=`` during AUTH;
+  it is now treated as simple Base64-encoded ``b""``.
+  This is the correct, strict interpretation of :rfc:`4954` mentions about ``=``
+
 
 1.3.0 (aiosmtpd-next-next)
 ==========================

aiosmtpd/smtp.py

@@ -32,7 +32,8 @@
 # region #### Custom Data Types #######################################################
 
 class _Missing:
-    pass
+    def __repr__(self):
+        return "MISSING"
 
 
 class _AuthMechAttr(NamedTuple):
@@ -62,7 +63,7 @@ class _DataState(enum.Enum):
     "AuthMechanismType",
     "MISSING",
 ]  # Will be added to by @public
-__version__ = '1.3.0a1'
+__version__ = '1.3.0a2'
 __ident__ = 'Python SMTP {}'.format(__version__)
 log = logging.getLogger('mail.log')
 
@@ -79,6 +80,19 @@ class _DataState(enum.Enum):
 # https://tools.ietf.org/html/rfc3207.html#page-3
 ALLOWED_BEFORE_STARTTLS = {"NOOP", "EHLO", "STARTTLS", "QUIT"}
 
+# Auth hiding regexes
+CLIENT_AUTH_B = re.compile(
+    # Matches "AUTH" <mechanism> <whitespace_but_not_\r_nor_\n>
+    br"(?P<authm>\s*AUTH\s+\S+[^\S\r\n]+)"
+    # Param to AUTH <mechanism>. We only need to sanitize if param is given, which
+    # for some mechanisms contain sensitive info. If no param is given, then we
+    # can skip (match fails)
+    br"(\S+)"
+    # Optional bCRLF at end. Why optional? Because we also want to sanitize the
+    # stripped line. If no bCRLF, then this group will be b""
+    br"(?P<crlf>(?:\r\n)?)", re.IGNORECASE
+)
+
 # endregion
 
 
@@ -209,6 +223,23 @@ class TLSSetupException(Exception):
     pass
 
 
+@public
+def sanitize(text: bytes) -> bytes:
+    m = CLIENT_AUTH_B.match(text)
+    if m:
+        return m.group("authm") + b"********" + m.group("crlf")
+    return text
+
+
+@public
+def sanitized_log(func: Callable, msg: AnyStr, *args, **kwargs):
+    sanitized_args = [
+        sanitize(a) if isinstance(a, bytes) else a
+        for a in args
+    ]
+    func(msg, *sanitized_args, **kwargs)
+
+
 @public
 class SMTP(asyncio.StreamReaderProtocol):
     command_size_limit = 512
@@ -281,12 +312,14 @@ def __init__(self, handler,
                  "can lead to security vulnerabilities!")
             log.warning("auth_required == True but auth_require_tls == False")
         self._auth_require_tls = auth_require_tls
+
         if authenticator is not None:
             self._authenticator: AuthenticatorType = authenticator
             self._auth_callback = None
         else:
             self._auth_callback = auth_callback or login_always_fail
             self._authenticator = None
+
         self._auth_required = auth_required
         # Get hooks & methods to significantly speedup getattr's
         self._auth_methods: Dict[str, _AuthMechAttr] = {
@@ -489,10 +522,10 @@ async def _handle_client(self):
                     # send error response and read the next command line.
                     await self.push('500 Command line too long')
                     continue
-                log.debug('_handle_client readline: %r', line)
+                sanitized_log(log.debug, '_handle_client readline: %r', line)
                 # XXX this rstrip may not completely preserve old behavior.
                 line = line.rstrip(b'\r\n')
-                log.info('%r >> %r', self.session.peer, line)
+                sanitized_log(log.info, '%r >> %r', self.session.peer, line)
                 if not line:
                     await self.push('500 Error: bad syntax')
                     continue
@@ -807,13 +840,17 @@ async def challenge_auth(
             self,
             challenge: AnyStr,
             encode_to_b64: bool = True,
+            log_client_response: bool = False,
     ) -> Union[_Missing, bytes]:
         """
         Send challenge during authentication. "334 " will be prefixed, so do NOT
         put "334 " at start of server_message.
 
         :param challenge: Challenge to send to client. If str, will be utf8-encoded.
         :param encode_to_b64: If true, then perform Base64 encoding on challenge
+        :param log_client_response: Perform logging of client's response.
+            WARNING: Might cause leak of sensitive information! Do not turn on
+            unless _absolutely_ necessary!
         :return: Response from client, or MISSING
         """
         challenge = (
@@ -825,9 +862,13 @@ async def challenge_auth(
         #   - https://tools.ietf.org/html/rfc4954#page-4 ¶ 5
         #   - https://tools.ietf.org/html/rfc4954#page-13 "continue-req"
         challenge = b"334 " + (b64encode(challenge) if encode_to_b64 else challenge)
-        log.debug("Send challenge to %r: %r", self.session.peer, challenge)
+        log.debug("%r << challenge: %r", self.session.peer, challenge)
         await self.push(challenge)
         line = await self._reader.readline()
+        if log_client_response:
+            warn("AUTH interaction logging is enabled!")
+            warn("Sensitive information might be leaked!")
+            log.debug("%r >> %r", self.session.peer, line)
         blob: bytes = line.strip()
         # '*' handling in accordance with RFC4954
         if blob == b"*":

aiosmtpd/testing/helpers.py

@@ -66,24 +66,15 @@ def start(plugin):
 
 
 def assert_auth_success(testcase: TestCase, *response):
-    testcase.assertEqual(
-        (235, b"2.7.0 Authentication successful"),
-        response
-    )
+    assert response == (235, b"2.7.0 Authentication successful")
 
 
 def assert_auth_invalid(testcase: TestCase, *response):
-    testcase.assertEqual(
-        (535, b"5.7.8 Authentication credentials invalid"),
-        response
-    )
+    assert response == (535, b"5.7.8 Authentication credentials invalid")
 
 
 def assert_auth_required(testcase: TestCase, *response):
-    testcase.assertEqual(
-        (530, b"5.7.0 Authentication required"),
-        response
-    )
+    assert response == (530, b"5.7.0 Authentication required")
 
 
 SUPPORTED_COMMANDS_TLS: bytes = (

aiosmtpd/tests/test_smtp.py

@@ -1,8 +1,10 @@
 """Test the SMTP protocol."""
 
+import os
 import time
 import socket
 import asyncio
+import logging
 import unittest
 import warnings
 
@@ -45,13 +47,32 @@
 
 
 ModuleResources = ExitStack()
+mail_logger = logging.getLogger('mail.log')
 
 
 def setUpModule():
     # Needed especially on FreeBSD because socket.getfqdn() is slow on that OS,
     # and oftentimes (not always, though) leads to Error
     ModuleResources.enter_context(patch("socket.getfqdn", return_value="localhost"))
 
+    loglevel = int(os.environ.get("AIOSMTPD_TESTLOGLEVEL", logging.INFO))
+    mail_logger.setLevel(loglevel)
+
+    if "AIOSMTPD_TESTLOGFILE" in os.environ:
+        fhandler = logging.FileHandler(
+            os.environ["AIOSMTPD_TESTLOGFILE"],
+            mode="a",
+            encoding="latin1",
+        )
+        fhandler.setLevel(1)
+        fhandler.setFormatter(
+            logging.Formatter(
+                u"{asctime} - {name} - {levelname} - {message}",
+                style="{",
+            )
+        )
+        mail_logger.addHandler(fhandler)
+
 
 def tearDownModule():
     ModuleResources.close()
@@ -99,7 +120,14 @@ async def auth_DONT(
     ):
         return MISSING
 
-    async def auth_WITH_UNDERSCORE(self, server, args):
+    async def auth_WITH_UNDERSCORE(self, server: Server, args):
+        """
+        Be careful when using this AUTH mechanism; log_client_response is set to
+        True, and this will raise some severe warnings.
+        """
+        await server.challenge_auth(
+            "challenge", encode_to_b64=False, log_client_response=True
+        )
         return "250 OK"
 
     @auth_mechanism("with-dash")
@@ -298,6 +326,7 @@ class TestProtocol(unittest.TestCase):
     def setUp(self):
         self.transport = Mock()
         self.transport.write = self._write
+        self.transport.get_extra_info.return_value = "MockedPeer"
         self.responses = []
         self._old_loop = asyncio.get_event_loop()
         self.loop = asyncio.new_event_loop()
@@ -1016,6 +1045,26 @@ def test_auth_login_good_credentials(self):
             code, response = client.docmd('Z29vZHBhc3N3ZA==')  # "goodpassword"
             assert_auth_success(self, code, response)
 
+    def test_authplain_goodcreds_sanitized_log(self):
+        oldlevel = mail_logger.getEffectiveLevel()
+        mail_logger.setLevel(logging.DEBUG)
+        with SMTP(*self.address) as client:
+            client.ehlo('example.com')
+            with self.assertLogs(mail_logger, level="DEBUG") as cm:
+                code, response = client.docmd(
+                    'AUTH PLAIN ' +
+                    b64encode(b'\0goodlogin\0goodpasswd').decode()
+                )
+        mail_logger.setLevel(oldlevel)
+        interestings = [
+            msg for msg in cm.output if "AUTH PLAIN" in msg
+        ]
+        assert len(interestings) == 2
+        assert interestings[0].startswith("DEBUG:")
+        assert interestings[0].endswith("b'AUTH PLAIN ********\\r\\n'")
+        assert interestings[1].startswith("INFO:")
+        assert interestings[1].endswith("b'AUTH PLAIN ********'")
+
     def test_auth_plain_null(self):
         with SMTP(*self.address) as client:
             client.ehlo('example.com')
@@ -1229,6 +1278,19 @@ def test_auth_individually(self):
                 code, mesg = client.docmd("AAA=")
                 assert_auth_success(self, code, mesg)
 
+    def test_auth_loginteract_warning(self):
+        """AUTH state of different clients must be independent"""
+        with SMTP(*self.address) as client:
+            client.ehlo("example.com")
+            resp = client.docmd("AUTH WITH_UNDERSCORE")
+            assert resp == (334, b"challenge")
+            with warnings.catch_warnings(record=True) as w:
+                code, mesg = client.docmd('=')
+            assert_auth_success(self, code, mesg)
+            assert len(w) > 0
+            assert str(w[0].message) == "AUTH interaction logging is enabled!"
+            assert str(w[1].message) == "Sensitive information might be leaked!"
+
     def test_auth_NONE(self):
         with SMTP(*self.address) as client:
             client.ehlo("example.com")

setup.cfg

@@ -6,10 +6,15 @@ long_description =
     This is a server for SMTP and related protocols, similar in utility to the
     standard library's smtpd.py module, but rewritten to be based on asyncio for
     Python 3.
+
+    Please visit the Project Homepage for more information.
 long_description_content_type = text/x-rst
-url = http://aiosmtpd.readthedocs.io/
-keywords =
-    email
+url = https://aiosmtpd.readthedocs.io/
+project_urls =
+    Bug Tracker = https://github.com/aio-libs/aiosmtpd/issues
+    Documentation = https://aiosmtpd.readthedocs.io/
+    Source Code = https://github.com/aio-libs/aiosmtpd
+keywords = email, smtpd, smtp
 license = Apache-2.0
 classifiers =
     License :: OSI Approved