Save the HAProxy state outside of its systemd unit

By default, SELinux prevents HAProxy context (haproxy_t) to execute
shell context (shell_exec_t) for security reasons.

This prevents HAProxy to actually reload properly, since SELinux will
deny its call to a shell to save its state to a file.

In order to avoid opening a potential security hole in the load-balancer
image, the best way is to generate the state file before the actual
reload.

There are more details about the SELinux denials in the associated Red
Hat Bugzilla.

 Conflicts:
	octavia/amphorae/backends/utils/haproxy_query.py

 Fixed in this backport:
   - A compatibility issue with Python 2.7
     (open() didn't have an encoding kwarg)
   - save_state() tried to use Exception().output, which in general
     causes an AttributeError

Resolves: rhbz#2073491
Change-Id: I6b9a5e1e3bafe77ad9f9506b8c0995d8c2a00081
(cherry picked from commit 21d74c373b)
(cherry picked from commit 0a062cd664)
(cherry picked from commit edcd6931fc)
(cherry picked from commit 97ac37bc6e)
(cherry picked from commit d4f2de8ba8)
(cherry picked from commit 86672909d4)

octavia/amphorae/backends/agent/api_server/loadbalancer.py

@@ -31,6 +31,7 @@ from werkzeug import exceptions
 from octavia.amphorae.backends.agent.api_server import haproxy_compatibility
 from octavia.amphorae.backends.agent.api_server import osutils
 from octavia.amphorae.backends.agent.api_server import util
+from octavia.amphorae.backends.utils import haproxy_query
 from octavia.common import constants as consts
 from octavia.common import utils as octavia_utils
 
@@ -248,6 +249,17 @@ class Loadbalancer(object):
         if action == consts.AMP_ACTION_RELOAD:
             if consts.OFFLINE == self._check_haproxy_status(lb_id):
                 action = consts.AMP_ACTION_START
+            else:
+                # We first have to save the state when we reload
+                haproxy_state_file = util.state_file_path(lb_id)
+                stat_sock_file = util.haproxy_sock_path(lb_id)
+
+                lb_query = haproxy_query.HAProxyQuery(stat_sock_file)
+                if not lb_query.save_state(haproxy_state_file):
+                    # We accept to reload haproxy even if the state_file is
+                    # not generated, but we probably want to know about that
+                    # failure!
+                    LOG.warning('Failed to save haproxy-%s state!', lb_id)
 
         cmd = ("/usr/sbin/service haproxy-{lb_id} {action}".format(
             lb_id=lb_id, action=action))

octavia/amphorae/backends/agent/api_server/templates/systemd.conf.j2

@@ -13,7 +13,6 @@ Environment="CONFIG={{ haproxy_cfg }}" "USERCONFIG={{ haproxy_user_group_cfg }}"
 
 ExecStartPre={{ haproxy_cmd }} -f $CONFIG -f $USERCONFIG -c -q -L {{ peer_name }}
 
-ExecReload=/bin/sh -c "echo 'show servers state' | socat stdio unix-connect:{{ haproxy_socket }} > {{ haproxy_state_file }}"
 ExecReload={{ haproxy_cmd }} -c -f $CONFIG -f $USERCONFIG -L {{ peer_name }}
 ExecReload=/bin/kill -USR2 $MAINPID
 

octavia/amphorae/backends/utils/haproxy_query.py

@@ -15,11 +15,14 @@
 import csv
 import socket
 
+from oslo_log import log as logging
 import six
 
 from octavia.common import constants as consts
 from octavia.i18n import _
 
+LOG = logging.getLogger(__name__)
+
 
 class HAProxyQuery(object):
     """Class used for querying the HAProxy statistics socket.
@@ -30,9 +33,9 @@ class HAProxyQuery(object):
     """
 
     def __init__(self, stats_socket):
-        """stats_socket
+        """Initialize the class
 
-            Path to the HAProxy statistics socket file.
+        :param stats_socket: Path to the HAProxy statistics socket file.
         """
 
         self.socket = stats_socket
@@ -134,3 +137,23 @@ class HAProxyQuery(object):
                 final_results[line['pxname']]['members'][line['svname']] = (
                     line['status'])
         return final_results
+
+    def save_state(self, state_file_path):
+        """Save haproxy connection state to a file.
+
+        :param state_file_path: Absolute path to the state file
+
+        :returns: bool (True if success, False otherwise)
+        """
+
+        try:
+            result = self._query('show servers state')
+            # No need for binary mode, the _query converts bytes to ascii.
+            with open(state_file_path, 'w') as fh:
+                fh.write(result)
+            return True
+        except Exception as e:
+            # Catch any exception - may be socket issue, or write permission
+            # issue as well.
+            LOG.warning("Unable to save state: %r", e)
+            return False

octavia/tests/functional/amphorae/backend/agent/api_server/test_server.py

@@ -352,8 +352,10 @@ class TestServerTestCase(base.TestCase):
     @mock.patch('octavia.amphorae.backends.agent.api_server.loadbalancer.'
                 'Loadbalancer._check_haproxy_status')
     @mock.patch('subprocess.check_output')
-    def _test_reload(self, distro, mock_subprocess, mock_haproxy_status,
+    @mock.patch('octavia.amphorae.backends.utils.haproxy_query.HAProxyQuery')
-                     mock_vrrp, mock_exists, mock_listdir):
+    def _test_reload(self, distro, mock_haproxy_query, mock_subprocess,
+                     mock_haproxy_status, mock_vrrp, mock_exists,
+                     mock_listdir):
 
         self.assertIn(distro, [consts.UBUNTU, consts.CENTOS])
 

octavia/tests/unit/amphorae/backends/agent/api_server/test_loadbalancer.py

@@ -65,8 +65,9 @@ class ListenerTestCase(base.TestCase):
     @mock.patch('octavia.amphorae.backends.agent.api_server.loadbalancer.'
                 'Loadbalancer._check_lb_exists')
     @mock.patch('subprocess.check_output')
-    def test_start_stop_lb(self, mock_check_output, mock_lb_exists,
+    @mock.patch('octavia.amphorae.backends.utils.haproxy_query.HAProxyQuery')
-                           mock_path_exists, mock_vrrp_update,
+    def test_start_stop_lb(self, mock_haproxy_query, mock_check_output,
+                           mock_lb_exists, mock_path_exists, mock_vrrp_update,
                            mock_check_status):
         listener_id = uuidutils.generate_uuid()
 

octavia/tests/unit/amphorae/backends/utils/test_haproxy_query.py

@@ -145,3 +145,32 @@ class QueryTestCase(base.TestCase):
              'description': '', 'Release_date': '2014/07/25'},
             self.q.show_info()
         )
+
+    def test_save_state(self):
+        filename = 'state_file'
+
+        query_mock = mock.Mock()
+        query_mock.return_value = 'DATA'
+
+        self.q._query = query_mock
+        builtin_open = '__builtin__.open' if six.PY2 else 'builtins.open'
+
+        with mock.patch(builtin_open) as mock_open:
+            mock_fh = mock.MagicMock()
+            mock_open().__enter__.return_value = mock_fh
+
+            self.q.save_state(filename)
+
+            mock_fh.write.assert_called_once_with('DATA')
+
+    def test_save_state_error(self):
+        """save_state() should swallow exceptions"""
+        filename = 'state_file'
+
+        query_mock = mock.Mock(side_effect=OSError())
+        self.q._query = query_mock
+
+        try:
+            self.q.save_state(filename)
+        except Exception as ex:
+            self.fail("save_state() raised %r unexpectedly!" % ex)

releasenotes/notes/ping-healthcheck-selinux-e3b7d360c8503527.yaml

@@ -0,0 +1,6 @@
+---
+issues:
+  - |
+    When using a distribution with a recent SELinux release such as CentOS 8
+    Stream, PING health-monitor does not work as shell_exec_t calls are denied
+    by SELinux.