Merge pull request #3191 from joohoi/centos_listen

Don't add wildcard listen if user has more specific configuration
This commit is contained in:
Peter Eckersley 2016-06-23 11:26:05 -07:00 committed by GitHub
commit 85769844f2
2 changed files with 93 additions and 33 deletions

View file

@ -625,50 +625,93 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
"""
# If nonstandard port, add service definition for matching
if port != "443":
port_service = "%s %s" % (port, "https")
else:
port_service = port
self.prepare_https_modules(temp)
# Check for Listen <port>
# Note: This could be made to also look for ip:443 combo
listens = [self.parser.get_arg(x).split()[0] for
x in self.parser.find_dir("Listen")]
# In case no Listens are set (which really is a broken apache config)
if not listens:
listens = ["80"]
if port in listens:
# Listen already in place
if self._has_port_already(listens, port):
return
listen_dirs = set(listens)
for listen in listens:
# For any listen statement, check if the machine also listens on
# Port 443. If not, add such a listen statement.
if len(listen.split(":")) == 1:
# Its listening to all interfaces
if port not in listens:
if port == "443":
args = [port]
else:
# Non-standard ports should specify https protocol
args = [port, "https"]
self.parser.add_dir_to_ifmodssl(
parser.get_aug_path(
self.parser.loc["listen"]), "Listen", args)
self.save_notes += "Added Listen %s directive to %s\n" % (
port, self.parser.loc["listen"])
listens.append(port)
if port not in listen_dirs and port_service not in listen_dirs:
listen_dirs.add(port_service)
else:
# The Listen statement specifies an ip
_, ip = listen[::-1].split(":", 1)
ip = ip[::-1]
if "%s:%s" % (ip, port) not in listens:
if port == "443":
args = ["%s:%s" % (ip, port)]
else:
# Non-standard ports should specify https protocol
args = ["%s:%s" % (ip, port), "https"]
self.parser.add_dir_to_ifmodssl(
parser.get_aug_path(
self.parser.loc["listen"]), "Listen", args)
self.save_notes += ("Added Listen %s:%s directive to "
"%s\n") % (ip, port,
self.parser.loc["listen"])
listens.append("%s:%s" % (ip, port))
if "%s:%s" % (ip, port_service) not in listen_dirs and (
"%s:%s" % (ip, port_service) not in listen_dirs):
listen_dirs.add("%s:%s" % (ip, port_service))
self._add_listens(listen_dirs, listens, port)
def _add_listens(self, listens, listens_orig, port):
"""Helper method for prepare_server_https to figure out which new
listen statements need adding
:param set listens: Set of all needed Listen statements
:param list listens_orig: List of existing listen statements
:param string port: Port number we're adding
"""
# Add service definition for non-standard ports
if port != "443":
port_service = "%s %s" % (port, "https")
else:
port_service = port
new_listens = listens.difference(listens_orig)
if port in new_listens or port_service in new_listens:
# We have wildcard, skip the rest
self.parser.add_dir_to_ifmodssl(
parser.get_aug_path(self.parser.loc["listen"]),
"Listen", port_service.split(" "))
self.save_notes += "Added Listen %s directive to %s\n" % (
port_service, self.parser.loc["listen"])
else:
for listen in new_listens:
self.parser.add_dir_to_ifmodssl(
parser.get_aug_path(self.parser.loc["listen"]),
"Listen", listen.split(" "))
self.save_notes += ("Added Listen %s directive to "
"%s\n") % (listen,
self.parser.loc["listen"])
def _has_port_already(self, listens, port):
"""Helper method for prepare_server_https to find out if user
already has an active Listen statement for the port we need
:param list listens: List of listen variables
:param string port: Port in question
"""
if port in listens:
return True
# Check if Apache is already listening on a specific IP
for listen in listens:
if len(listen.split(":")) > 1:
# Ugly but takes care of protocol def, eg: 1.1.1.1:443 https
if listen.split(":")[-1].split(" ")[0] == port:
return True
def prepare_https_modules(self, temp):
"""Helper method for prepare_server_https, taking care of enabling

View file

@ -497,13 +497,8 @@ class MultipleVhostsTest(util.ApacheTest):
# Test Listen statements with specific ip listeed
self.config.prepare_server_https("443")
# Should only be 2 here, as the third interface
# already listens to the correct port
self.assertEqual(mock_add_dir.call_count, 2)
# Check argument to new Listen statements
self.assertEqual(mock_add_dir.call_args_list[0][0][2], ["1.2.3.4:443"])
self.assertEqual(mock_add_dir.call_args_list[1][0][2], ["[::1]:443"])
# Should be 0 as one interface already listens to 443
self.assertEqual(mock_add_dir.call_count, 0)
# Reset return lists and inputs
mock_add_dir.reset_mock()
@ -519,6 +514,28 @@ class MultipleVhostsTest(util.ApacheTest):
self.assertEqual(mock_add_dir.call_args_list[2][0][2],
["1.1.1.1:8080", "https"])
# mock_get.side_effect = ["1.2.3.4:80", "[::1]:80"]
# mock_find.return_value = ["test1", "test2", "test3"]
# self.config.parser.get_arg = mock_get
# self.config.prepare_server_https("8080", temp=True)
# self.assertEqual(self.listens, 0)
def test_prepare_server_https_needed_listen(self):
mock_find = mock.Mock()
mock_find.return_value = ["test1", "test2"]
mock_get = mock.Mock()
mock_get.side_effect = ["1.2.3.4:8080", "80"]
mock_add_dir = mock.Mock()
mock_enable = mock.Mock()
self.config.parser.find_dir = mock_find
self.config.parser.get_arg = mock_get
self.config.parser.add_dir_to_ifmodssl = mock_add_dir
self.config.enable_mod = mock_enable
self.config.prepare_server_https("443")
self.assertEqual(mock_add_dir.call_count, 1)
def test_prepare_server_https_mixed_listen(self):
mock_find = mock.Mock()