Merge branch...

Merge branch '60-stapled-keeps-sockets-open-preventing-clean-termination-of-haproxy-processes' into 'master'

Resolve "Stapled keeps sockets open preventing clean termination of haproxy processes"

Closes #60

See merge request !43

Dockerfile

 FROM debian:stretch
 RUN apt-get update -qq
-RUN apt-get upgrade
+RUN apt-get upgrade -y
 RUN apt-get install -q -y build-essential python3-cffi libffi-dev \
     python-all python3-all python3-dev python3-setuptools python3-pip \
     rpm tar gzip bzip2 git debhelper

config/stapled.conf

@@ -86,6 +86,19 @@ logdir=/var/log/stapled/
 ;; https://cbonte.github.io/haproxy-dconv/1.7/management.html#9.3-set%20ssl%20ocsp-response
 haproxy-sockets=[/var/run/haproxy/admin.sock]
 
+;; Use HAProxy config files as the source of cert-paths and socket mappings.
+;; Setting this will merge your `cert-paths` with paths found in the specified
+;; HAProxy config files. Sockets defined in `haproxy-sockets` will also be
+;; merged in the path to socket mapping.
+; haproxy-config=/etc/haproxy/haproxy.cfg
+
+;; Set a keep alive time in seconds after wich the connection to the HAProxy
+;; sockets is terminated. The minimum allowed value is 10 seconds, because
+;; stapled will take at least a bit of time to communicate with HAProxy, and
+;; either process could be "busy".
+; haproxy-socket-keepalive=3600
+
+
 ;; Don't output anything to stdout, can be used together with `logdir`
 ;; and/or `syslog` to prevent output on stdout while logging the set verbosity
 ;; level to a file or syslog. Uncomment to enable

debian/changelog

+stapled (1.1) stretch; urgency=low
+
+ * Change haproxy socket connection keep-alive (formerly "timeout") to new
+   default: 3600 seconds.
+ * Add --haproxy-socket-keepalive command line argument.
+
+ -- Chris <chris@greenhost.nl> Mon, 22 Oct 2018 18:34:15 +0200
+
 stapled (1.0) stretch; urgency=low
 
  * This version removes support for debian Jessie due to the shutdown of

docker/build-stretch/Dockerfile

 FROM debian:stretch
 RUN apt-get update -qq
-RUN apt-get upgrade
+RUN apt-get upgrade -y
 RUN apt-get install -q -y build-essential python3-cffi libffi-dev \
     python-all python3-all python3-dev python3-setuptools python3-pip \
     rpm tar gzip bzip2 git debhelper ca-certificates

docker/test-stretch/Dockerfile

 FROM debian:stretch
 RUN apt-get update -qq
-RUN apt-get upgrade
+RUN apt-get upgrade -y
 RUN apt-get install -y openssl ca-certificates python3-cffi \
     python3-configargparse python3-daemon
 COPY ./refresh_testdata.sh ./refresh_testdata.sh

stapled/__main__.py

 #!/usr/bin/env python3
 # -*- coding: utf-8 -*-
 """
-This is the module that parses your command line arguments and then starts
-the OCSP Staple daemon, which searches your certificate paths and
-requests staples for all certificates in them. They will then be saved as
-``certificatename.pem.ocsp`` in the same paths that are being indexed.
+Parse command line arguments and starts the OCSP Staple daemon.
+
+The daemon searches your certificate paths and requests staples for all
+certificates in them. They will then be saved as ``certificatename.pem.ocsp``
+in the same paths that are being indexed.
 
 Type ``stapled.py -h`` for all command line arguments.
 
@@ -29,15 +30,17 @@ user's process hierarchy node. In any case, it starts up the
 :mod:`stapled.core.daemon`
 module to bootstrap the application.
 """
-import configargparse
 import logging
 import logging.handlers
 import os
+import sys
+import configargparse
 import daemon
 import stapled
 import stapled.core.daemon
 import stapled.core.excepthandler
 from stapled.core.excepthandler import handle_file_error
+from stapled.core.exceptions import ArgumentError
 from stapled.util.haproxy import parse_haproxy_config
 from stapled.colourlog import ColourFormatter
 from stapled.version import __version__, __app_name__
@@ -222,6 +225,19 @@ def get_cli_arg_parser():
             "specified in the config file."
         )
     )
+    parser.add(
+        '--haproxy-socket-keepalive',
+        type=int,
+        default=3600,
+        metavar="KEEP-ALIVE <seconds, minimum: 10>",
+        help=(
+            "HAProxy sockets are kept open for performance reasons, you can "
+            "set the amount of seconds sockets should remain open "
+            "(default=3600). Note that a short amount of time is required to "
+            "to pass messages to HAProxy, so 10 seconds if the minimum "
+            "accepted value."
+        )
+    )
     parser.add(
         '--haproxy-config',
         type=str,
@@ -311,39 +327,13 @@ def init():
     :func:`stapled.core.daemon.run()` either in daemonised mode if the ``-d``
     argument was supplied, or in the current context if ``-d`` wasn't supplied.
     """
-    parser = get_cli_arg_parser()
-    args = parser.parse_args()
-    log_file_handles = __init_logging(args)
-
-    # Parse the cert_paths argument
-    arg_cert_paths = __get_arg_cert_paths(args)
-    # Parse haproxy_sockets argument.
-    arg_haproxy_sockets = __get_arg_haproxy_sockets(args)
-    # Make a mapping from certificate paths to sockets in a dict.
-    haproxy_socket_mapping = dict(zip(arg_cert_paths, arg_haproxy_sockets))
-
-    # Parse HAProxy config files.
-    try:
-        conf_cert_paths, conf_haproxy_sockets = parse_haproxy_config(
-            args.haproxy_config
-        )
-    except (OSError, IOError) as exc:
-        logger.critical(handle_file_error(exc))
-        exit(1)
+    args = __get_validated_args()
 
-    # Combine the socket and certificate paths of the arguments and config
-    # files in the sockets dictionary.
-    for i, paths in enumerate(conf_cert_paths):
-        for path in paths:
-            if path in haproxy_socket_mapping:
-                haproxy_socket_mapping[path] = unique(
-                    haproxy_socket_mapping[path] + conf_haproxy_sockets[i],
-                    preserve_order=False
-                )
-            else:
-                haproxy_socket_mapping[path] = conf_haproxy_sockets[i]
+    log_file_handles = __init_logging(args)
 
-    logger.debug("Paths to socket mapping: %s", str(haproxy_socket_mapping))
+    # Get a mapping of configured sockets and certificate directories from:
+    # haproxy config, stapled config and command line arguments
+    haproxy_socket_mapping = __get_haproxy_socket_mapping(args)
 
     # Now sockets' keys are the merged cert paths from arguments and haproxy
     # config files, de-duplicated.
@@ -356,6 +346,7 @@ def init():
     daemon_kwargs = dict(
         cert_paths=cert_paths,
         haproxy_socket_mapping=haproxy_socket_mapping,
+        haproxy_socket_keepalive=args.haproxy_socket_keepalive,
         file_extensions=args.file_extensions,
         renewal_threads=args.renewal_threads,
         refresh_interval=args.refresh_interval,
@@ -461,6 +452,69 @@ def __init_logging(args):
     return log_file_handles
 
 
+def __get_haproxy_socket_mapping(args):
+    """
+    Get mapping of configured sockets and certificate directories.
+
+    From: haproxy config, stapled config and command line arguments.
+
+    :param Namespace args: Argparser argument list.
+    :return dict Of cert-paths and sockets for inform of changes.
+    """
+    # Parse the cert_paths argument
+    arg_cert_paths = __get_arg_cert_paths(args)
+    # Parse haproxy_sockets argument.
+    arg_haproxy_sockets = __get_arg_haproxy_sockets(args)
+    # Make a mapping from certificate paths to sockets in a dict.
+    mapping = dict(zip(arg_cert_paths, arg_haproxy_sockets))
+
+    # Parse HAProxy config files.
+    try:
+        conf_cert_paths, conf_haproxy_sockets = parse_haproxy_config(
+            args.haproxy_config
+        )
+    except (OSError, IOError) as exc:
+        logger.critical(handle_file_error(exc))
+        exit(1)
+
+    # Combine the socket and certificate paths of the arguments and config
+    # files in the sockets dictionary.
+    for i, paths in enumerate(conf_cert_paths):
+        for path in paths:
+            if path in mapping:
+                mapping[path] = unique(
+                    mapping[path] + conf_haproxy_sockets[i],
+                    preserve_order=False
+                )
+            else:
+                mapping[path] = conf_haproxy_sockets[i]
+
+    logger.debug("Paths to socket mapping: %s", str(mapping))
+    return mapping
+
+
+def __get_validated_args():
+    """
+    Parse and validate CLI arguments and configuration.
+
+    Checks should match the restrictions in the usage help messages.
+
+    :returns Namespace: Validated argparser argument list.
+    """
+    parser = get_cli_arg_parser()
+    args = parser.parse_args()
+    try:
+        if args.haproxy_socket_keepalive < 10:
+            raise ArgumentError(
+                "`--haproxy-socket-keepalive` should be higher than 10."
+            )
+    except ArgumentError as exc:
+        parser.print_usage(sys.stderr)
+        logger.critical("Invalid command line argument or value: %s", exc)
+        exit(1)
+    return args
+
+
 if __name__ == '__main__':
     try:
         init()

stapled/core/daemon.py

@@ -92,6 +92,7 @@ class Stapledaemon(object):
         self.haproxy_socket_mapping = kwargs.pop(
             'haproxy_socket_mapping', None
         )
+        self.haproxy_socket_keepalive = kwargs.pop('haproxy_socket_keepalive')
         self.file_extensions = kwargs.pop('file_extensions')
         self.file_extensions = self.file_extensions.replace(" ", "").split(",")
         self.renewal_threads = kwargs.pop('renewal_threads')
@@ -170,6 +171,7 @@ class Stapledaemon(object):
             name="proxy-adder",
             thread_object=StapleAdder,
             haproxy_socket_mapping=self.haproxy_socket_mapping,
+            haproxy_socket_keepalive=self.haproxy_socket_keepalive,
             scheduler=self.scheduler
         )
 

stapled/core/exceptions.py

@@ -60,3 +60,9 @@ class CertValidationError(Exception):
     """
 
     pass
+
+class ArgumentError(Exception):
+    """
+    Raised when a command line argument has an invalid value.
+    """
+    pass

stapled/core/stapleadder.py

@@ -13,7 +13,6 @@ import stapled.core.exceptions
 
 LOG = logging.getLogger(__name__)
 SOCKET_BUFFER_SIZE = 1024
-SOCKET_TIMEOUT = 86400
 
 
 class StapleAdder(threading.Thread):
@@ -41,12 +40,6 @@ class StapleAdder(threading.Thread):
     #: the base64 encoded OCSP staple
     OCSP_ADD = 'set ssl ocsp-response {}'
 
-    #: Predefines commands to send to sockets just after opening them.
-    CONNECT_COMMANDS = [
-        "prompt",
-        "set timeout cli {}".format(SOCKET_TIMEOUT)
-    ]
-
     def __init__(self, *args, **kwargs):
         """
         Initialise the thread and its parent :class:`threading.Thread`.
@@ -65,11 +58,21 @@ class StapleAdder(threading.Thread):
         self.haproxy_socket_mapping = kwargs.pop(
             'haproxy_socket_mapping', None
         )
-
+        self.haproxy_socket_keepalive = kwargs.pop(
+            'haproxy_socket_keepalive', None
+        )
         assert self.scheduler is not None, \
             "Please pass a scheduler to get and add proxy-add tasks."
         assert self.haproxy_socket_mapping is not None, \
             "The StapleAdder needs a haproxy_socket_mapping dict"
+        assert self.haproxy_socket_keepalive is not None, \
+            "No keep-alive defined for haproxy socket connection."
+
+        # Predefines commands to send to sockets just after opening them.
+        self.connect_commands = [
+            "prompt",
+            "set timeout cli {}".format(self.haproxy_socket_keepalive)
+        ]
 
         self.socks = {}
         for paths in self.haproxy_socket_mapping.values():
@@ -119,7 +122,7 @@ class StapleAdder(threading.Thread):
         try:
             sock.connect(path)
             result = []
-            for command in self.CONNECT_COMMANDS:
+            for command in self.connect_commands:
                 result.extend(self._send(sock, command))
             # Results (index 1) come per path (index 0), we need only results
             result = [res[1] for res in result]

stapled/version.py

-__version__ = '1.0'
+__version__ = '1.1'
 __app_name__ = 'stapled'
 __debian_version__ = 'stretch'

version

@@ -286,7 +286,7 @@ class GitVersion(object):
             # Take index 7: to strip off 'commit '
             commit_hash = str(
                 subprocess.check_output(
-                    ['git', 'log', '-n', '1', self.file],
+                    ['git', 'log', '--decorate=', '-n', '1', self.file],
                     universal_newlines=True
                 )
             ).split('\n')[0][len('commit '):]
@@ -453,7 +453,7 @@ def main():
         default='stapled/version.py',
         const='stapled/version.py',
         help=(
-            'Save the new number to the a file named .version, optionally pass'
+            'Save the new number to the version file, optionally pass'
             'a different filename to the argument.'
         )
     )