Add SSL support (SMTPS/STARTTLS) to CLI (#252)

* Rename internal "args" variable in parseargs
* Add SSL parameters and combination validation
* Add certfile & keyfile validation (check they exist)
* Invert "--requiretls" to "--no-requiretls"
* Assert default values
* Implement SSL Context injection for STARTTLS and SMTPS
* Implement test for "TLS Required"
* Update CLI docs
* Properly generate man page
* Version Up + NEWS
* Run Bandit as part of testing
* Silence Bandit complaints
* Update README.rst
* Build manpage as well in GA (detect errors)
* Tidying up
* Reduce housekeep.py superclean clutter

.github/workflows/unit-testing-and-coverage.yml

@@ -47,8 +47,9 @@ jobs:
         run: |
           pip install colorama pytest pytest-mock sphinx sphinx-autofixture sphinx_rtd_theme
           pytest -v aiosmtpd/docs
-          python setup.py build_sphinx
-      - name: "Static Type Checking"
+          sphinx-build --color -b html -d build/.doctree aiosmtpd/docs build/html
+          sphinx-build --color -b man  -d build/.doctree aiosmtpd/docs build/man
+      - name: "Static Code Checking"
         # language=bash
         run: |
           # Required by examples
@@ -93,6 +94,11 @@ jobs:
         pip install colorama coverage[toml] coverage-conditional-plugin packaging pytest pytest-cov pytest-mock
         # Package deps
         python setup.py develop
+    - name: "Security checking"
+      # language=bash
+      run: |
+        pip install bandit
+        bandit -c bandit.yml -r aiosmtpd
     - name: "Execute testing"
       shell: bash
       # language=bash

README.rst

@@ -195,25 +195,41 @@ have been configured and tested:
   is pre-prepared, such as Travis CI or |GitHub Actions|_; this will definitely
   save some time during tox's testenv prepping.
 
+  For all testenv combinations except diffcov,
+  |bandit|_ security check will also be run prior to running pytest.
+
+.. _bandit: https://github.com/PyCQA/bandit
+.. |bandit| replace:: ``bandit``
+
+
 * ``qa``
 
-  Perform ``flake8`` code style checking
+  Performs |flake8|_ code style checking,
+  and |flake8-bugbear|_ design checking.
+
+  In addition, some tests to help ensure that ``aiosmtpd`` is *releasable* to PyPI are also run.
+
+.. _flake8: https://flake8.pycqa.org/en/latest/
+.. |flake8| replace:: ``flake8``
+.. _flake8-bugbear: https://github.com/PyCQA/flake8-bugbear
+.. |flake8-bugbear| replace:: ``flake8-bugbear``
 
 * ``docs``
 
-  Builds **HTML documentation** using Sphinx.
+  Builds **HTML documentation** and **manpage** using Sphinx.
   A `pytest doctest`_ will run prior to actual building of the documentation.
 
 * ``static``
 
   Performs a **static type checking** using ``pytype``.
-  Please ensure that `all its dependencies`_ have been installed before
+
+  **Note 1:** Please ensure that `all pytype dependencies`_ have been installed before
   executing this testenv.
 
-  **Note:** Because ``pytype`` does not run on Windows,
+  **Note 2:** Because ``pytype`` does not run on Windows,
   This testenv must be invoked explicitly; it will not automatically run.
 
-.. _`all its dependencies`: https://github.com/google/pytype/blob/2021.02.09/CONTRIBUTING.md#pytype-dependencies
+.. _`all pytype dependencies`: https://github.com/google/pytype/blob/2021.02.09/CONTRIBUTING.md#pytype-dependencies
 
 
 Environment Variables

aiosmtpd/__init__.py

@@ -1,4 +1,4 @@
 # Copyright 2014-2021 The aiosmtpd Developers
 # SPDX-License-Identifier: Apache-2.0
 
-__version__ = "1.4.0a2"
+__version__ = "1.4.0a3"

aiosmtpd/docs/NEWS.rst

@@ -10,6 +10,7 @@ Added
 -----
 * Support for |PROXY Protocol|_ (Closes #174)
 * Example for authentication
+* SSL Support for CLI. See :ref:`the man page <manpage>` for more info. (Closes #172)
 
 .. _`PROXY Protocol`: https://www.haproxy.com/blog/using-haproxy-with-the-proxy-protocol-to-better-secure-your-database/
 .. |PROXY Protocol| replace:: **PROXY Protocol**

aiosmtpd/docs/_exts/autoprogramm.py

@@ -151,7 +151,7 @@ def import_object(import_name):
                 with open(f[0]) as fobj:
                     codestring = fobj.read()
                 foo = imp.new_module("foo")
-                exec(codestring, foo.__dict__)
+                exec(codestring, foo.__dict__)  # nosec
 
                 sys.modules["foo"] = foo
                 mod = __import__("foo")
@@ -163,7 +163,7 @@ def import_object(import_name):
     globals_ = builtins
     if not isinstance(globals_, dict):
         globals_ = globals_.__dict__
-    return eval(expr, globals_, mod.__dict__)
+    return eval(expr, globals_, mod.__dict__)  # nosec
 
 
 class AutoprogrammDirective(Directive):

aiosmtpd/docs/conf.py

@@ -101,6 +101,7 @@ def syspath_insert(pth: Path):
 # today = ''
 # Else, today_fmt is used as the format for a strftime call.
 # today_fmt = '%B %d, %Y'
+today_fmt = "%Y-%m-%d"
 
 # List of patterns, relative to source directory, that match files and
 # directories to ignore when looking for source files.
@@ -282,7 +283,7 @@ def syspath_insert(pth: Path):
 # One entry per manual page. List of tuples
 # (source start file, name, description, authors, manual section).
 man_pages = [
-    ("manpage", "aiosmtpd", "aiosmtpd Documentation", ["aiosmtpd hackers"], 1),
+    ("manpage", "aiosmtpd", "asyncio based SMTP server", [author], 1),
 ]
 
 # If true, show URL addresses after external links.

aiosmtpd/docs/manpage.rst

@@ -8,7 +8,7 @@ Provides an asynchronous, RFC 5321 compliant Simple Mail Transfer Protocol (SMTP
 extensions.
 
 :Author: |author|
-:Date: 2020-11-20
+:Date: |today|
 :Copyright: |copyright|
 :Version: |version|
 :Manual section: 1
@@ -30,6 +30,5 @@ ENVIRONMENT
 
 .. envvar:: AIOSMTPD_CONTROLLER_TIMEOUT
 
-    How long the main thread will wait (in seconds) until the SMTP thread is ready.
-
-    Default: ``1.0``
+    | How long the main thread will wait (in seconds) until the SMTP thread is ready.
+    | Default: ``1.0``

aiosmtpd/main.py

@@ -5,11 +5,13 @@
 import logging
 import os
 import signal
+import ssl
 import sys
 from argparse import ArgumentParser
 from contextlib import suppress
 from functools import partial
 from importlib import import_module
+from pathlib import Path
 
 from public import public
 
@@ -87,7 +89,9 @@ def _parser() -> ArgumentParser:
         "--debug",
         default=0,
         action="count",
-        help="""Increase debugging output.""",
+        help=(
+            "Increase debugging output. Every ``-d`` increases debugging level by one."
+        )
     )
     parser.add_argument(
         "-l",
@@ -102,6 +106,57 @@ def _parser() -> ArgumentParser:
             "``{host}:{port}`` is used.".format(host=DEFAULT_HOST, port=DEFAULT_PORT)
         ),
     )
+    parser.add_argument(
+        "--smtpscert",
+        metavar="CERTFILE",
+        type=Path,
+        default=None,
+        help=(
+            "The certificate file for implementing **SMTPS**. If given, the parameter "
+            "``--smtpskey`` must also be specified."
+        ),
+    )
+    parser.add_argument(
+        "--smtpskey",
+        metavar="KEYFILE",
+        type=Path,
+        default=None,
+        help=(
+            "The key file for implementing **SMTPS**. If given, the parameter "
+            "``--smtpscert`` must also be specified."
+        ),
+    )
+    parser.add_argument(
+        "--tlscert",
+        metavar="CERTFILE",
+        type=Path,
+        default=None,
+        help=(
+            "The certificate file for implementing **STARTTLS**. If given, the "
+            "parameter ``--tlskey`` must also be specified."
+        ),
+    )
+    parser.add_argument(
+        "--tlskey",
+        metavar="KEYFILE",
+        type=Path,
+        default=None,
+        help=(
+            "The key file for implementing **STARTTLS**. If given, the parameter "
+            "``--tlscert`` must also be specified."
+        ),
+    )
+    parser.add_argument(
+        "--no-requiretls",
+        dest="requiretls",
+        default=True,
+        action="store_false",
+        help=(
+            "If specified, disables ``require_starttls`` of the SMTP class. "
+            "(By default, ``require_starttls`` is True.) "
+            "Has no effect if ``--tlscert`` and ``--tlskey`` are not specified."
+        ),
+    )
     parser.add_argument(
         "classargs",
         metavar="CLASSARGS",
@@ -114,33 +169,48 @@ def _parser() -> ArgumentParser:
 
 def parseargs(args=None):
     parser = _parser()
-    args = parser.parse_args(args)
+    parsed = parser.parse_args(args)
     # Find the handler class.
-    path, dot, name = args.classpath.rpartition(".")
+    path, dot, name = parsed.classpath.rpartition(".")
     module = import_module(path)
     handler_class = getattr(module, name)
     if hasattr(handler_class, "from_cli"):
-        args.handler = handler_class.from_cli(parser, *args.classargs)
+        parsed.handler = handler_class.from_cli(parser, *parsed.classargs)
     else:
-        if len(args.classargs) > 0:
+        if len(parsed.classargs) > 0:
             parser.error(f"Handler class {path} takes no arguments")
-        args.handler = handler_class()
+        parsed.handler = handler_class()
     # Parse the host:port argument.
-    if args.listen is None:
-        args.host = DEFAULT_HOST
-        args.port = DEFAULT_PORT
+    if parsed.listen is None:
+        parsed.host = DEFAULT_HOST
+        parsed.port = DEFAULT_PORT
     else:
-        host, colon, port = args.listen.rpartition(":")
+        host, colon, port = parsed.listen.rpartition(":")
         if len(colon) == 0:
-            args.host = port
-            args.port = DEFAULT_PORT
+            parsed.host = port
+            parsed.port = DEFAULT_PORT
         else:
-            args.host = DEFAULT_HOST if len(host) == 0 else host
+            parsed.host = DEFAULT_HOST if len(host) == 0 else host
             try:
-                args.port = int(DEFAULT_PORT if len(port) == 0 else port)
+                parsed.port = int(DEFAULT_PORT if len(port) == 0 else port)
             except ValueError:
                 parser.error("Invalid port number: {}".format(port))
-    return parser, args
+
+    if bool(parsed.smtpscert) ^ bool(parsed.smtpskey):
+        parser.error("--smtpscert and --smtpskey must be specified together")
+    if parsed.smtpscert and not parsed.smtpscert.exists():
+        parser.error(f"Cert file {parsed.smtpscert} not found")
+    if parsed.smtpskey and not parsed.smtpskey.exists():
+        parser.error(f"Key file {parsed.smtpskey} not found")
+
+    if bool(parsed.tlscert) ^ bool(parsed.tlskey):
+        parser.error("--tlscert and --tlskey must be specified together")
+    if parsed.tlscert and not parsed.tlscert.exists():
+        parser.error(f"Cert file {parsed.tlscert} not found")
+    if parsed.tlskey and not parsed.tlskey.exists():
+        parser.error(f"Key file {parsed.tlskey} not found")
+
+    return parser, parsed
 
 
 @public
@@ -163,8 +233,20 @@ def main(args=None):
             )
             sys.exit(1)
 
+    if args.tlscert and args.tlskey:
+        tls_context = ssl.create_default_context(ssl.Purpose.CLIENT_AUTH)
+        tls_context.check_hostname = False
+        tls_context.load_cert_chain(str(args.tlscert), str(args.tlskey))
+    else:
+        tls_context = None
+
     factory = partial(
-        SMTP, args.handler, data_size_limit=args.size, enable_SMTPUTF8=args.smtputf8
+        SMTP,
+        args.handler,
+        data_size_limit=args.size,
+        enable_SMTPUTF8=args.smtputf8,
+        tls_context=tls_context,
+        require_starttls=args.requiretls,
     )
 
     logging.basicConfig(level=logging.ERROR)
@@ -178,10 +260,19 @@ def main(args=None):
     if args.debug > 2:
         loop.set_debug(enabled=True)
 
+    if args.smtpscert and args.smtpskey:
+        smtps_context = ssl.create_default_context(ssl.Purpose.CLIENT_AUTH)
+        smtps_context.check_hostname = False
+        smtps_context.load_cert_chain(str(args.smtpscert), str(args.smtpskey))
+    else:
+        smtps_context = None
+
     log.debug("Attempting to start server on %s:%s", args.host, args.port)
     server = server_loop = None
     try:
-        server = loop.create_server(factory, host=args.host, port=args.port)
+        server = loop.create_server(
+            factory, host=args.host, port=args.port, ssl=smtps_context
+        )
         server_loop = loop.run_until_complete(server)
     except RuntimeError:  # pragma: nocover
         raise

aiosmtpd/qa/test_0packaging.py

@@ -36,7 +36,7 @@ def test_ge_master(self, aiosmtpd_version, capsys):
         cmd = f"git show {reference}".split()
         try:
             with capsys.disabled():
-                master_smtp = subprocess.check_output(cmd).decode()
+                master_smtp = subprocess.check_output(cmd).decode()  # nosec
         except subprocess.CalledProcessError:
             pytest.skip("Skipping due to git error")
             return

aiosmtpd/tests/conftest.py

@@ -25,6 +25,15 @@
     HAS_PROACTOR = False
 
 
+__all__ = [
+    "controller_data",
+    "handler_data",
+    "Global",
+    "SERVER_CRT",
+    "SERVER_KEY",
+]
+
+
 # region #### Aliases #################################################################
 
 controller_data = pytest.mark.controller_data
@@ -55,6 +64,9 @@ def set_addr_from(cls, contr: Controller):
         cls.SrvAddr = HostPort(contr.hostname, contr.port)
 
 
+SERVER_CRT = resource_filename("aiosmtpd.tests.certs", "server.crt")
+SERVER_KEY = resource_filename("aiosmtpd.tests.certs", "server.key")
+
 # endregion
 
 
@@ -281,10 +293,7 @@ def ssl_context_server() -> Generator[ssl.SSLContext, None, None]:
     """
     context = ssl.create_default_context(ssl.Purpose.CLIENT_AUTH)
     context.check_hostname = False
-    context.load_cert_chain(
-        resource_filename("aiosmtpd.tests.certs", "server.crt"),
-        resource_filename("aiosmtpd.tests.certs", "server.key"),
-    )
+    context.load_cert_chain(SERVER_CRT, SERVER_KEY)
     #
     yield context
 
@@ -296,9 +305,7 @@ def ssl_context_client() -> Generator[ssl.SSLContext, None, None]:
     """
     context = ssl.create_default_context(ssl.Purpose.SERVER_AUTH)
     context.check_hostname = False
-    context.load_verify_locations(
-        resource_filename("aiosmtpd.tests.certs", "server.crt")
-    )
+    context.load_verify_locations(SERVER_CRT)
     #
     yield context