From 420e73fabd87ae54f8892b5343e5ff99498e6520 Mon Sep 17 00:00:00 2001 From: Romain LE DISEZ Date: Wed, 8 Mar 2017 18:33:21 +0100 Subject: [PATCH] Allow to configure the nameservers in cname_lookup For various reasons, an operator might want to use specifics nameservers instead of the systems ones to resolve CNAME in cname_lookup. This patch creates a new configuration variable nameservers which accepts a list of nameservers separated by commas. If not specified or empty, systems namservers are used as previously. Co-Authored-By: Tim Burke Change-Id: I34219e6ab7e45678c1a80ff76a1ac0730c64ddde --- doc/manpages/proxy-server.conf.5 | 4 + etc/proxy-server.conf-sample | 6 + swift/common/middleware/cname_lookup.py | 46 ++++---- .../common/middleware/test_cname_lookup.py | 107 ++++++++++++++++-- 4 files changed, 132 insertions(+), 31 deletions(-) diff --git a/doc/manpages/proxy-server.conf.5 b/doc/manpages/proxy-server.conf.5 index 4f4ac8be7f..5e1395026e 100644 --- a/doc/manpages/proxy-server.conf.5 +++ b/doc/manpages/proxy-server.conf.5 @@ -564,6 +564,10 @@ The domain to be used by the middleware. .IP \fBlookup_depth\fR How deep in the CNAME chain to look for something that matches the storage domain. The default is 1. +.IP \fBnameservers\fR +Specify the nameservers to use to do the CNAME resolution. If unset, the system +configuration is used. Multiple nameservers can be specified separated by a comma. +Default is unset. .RE .PD diff --git a/etc/proxy-server.conf-sample b/etc/proxy-server.conf-sample index ce4c656caf..f9ce6d4725 100644 --- a/etc/proxy-server.conf-sample +++ b/etc/proxy-server.conf-sample @@ -565,6 +565,12 @@ use = egg:swift#cname_lookup # storage_domain = example.com # # lookup_depth = 1 +# +# Specify the nameservers to use to do the CNAME resolution. If unset, the +# system configuration is used. Multiple nameservers can be specified +# separated by a comma. Default port 53 can be overriden. IPv6 is accepted. +# Example: 127.0.0.1, 127.0.0.2, 127.0.0.3:5353, [::1], [::1]:5353 +# nameservers = # Note: Put staticweb just after your auth filter(s) in the pipeline [filter:staticweb] diff --git a/swift/common/middleware/cname_lookup.py b/swift/common/middleware/cname_lookup.py index d5e7715605..1b5aa6df4b 100644 --- a/swift/common/middleware/cname_lookup.py +++ b/swift/common/middleware/cname_lookup.py @@ -29,7 +29,6 @@ rewritten and the request is passed further down the WSGI chain. from six.moves import range -import socket from swift import gettext_ as _ try: @@ -43,19 +42,21 @@ else: # executed if the try block finishes with no errors from swift.common.middleware import RewriteContext from swift.common.swob import Request, HTTPBadRequest -from swift.common.utils import cache_from_env, get_logger, list_from_csv, \ - register_swift_info +from swift.common.utils import cache_from_env, get_logger, is_valid_ip, \ + list_from_csv, parse_socket_string, register_swift_info -def lookup_cname(domain): # pragma: no cover +def lookup_cname(domain, resolver): # pragma: no cover """ Given a domain, returns its DNS CNAME mapping and DNS ttl. :param domain: domain to query on + :param resolver: dns.resolver.Resolver() instance used for executing DNS + queries :returns: (ttl, result) """ try: - answer = dns.resolver.query(domain, 'CNAME').rrset + answer = resolver.query(domain, 'CNAME').rrset ttl = answer.ttl result = answer.items[0].to_text() result = result.rstrip('.') @@ -69,18 +70,6 @@ def lookup_cname(domain): # pragma: no cover return 0, None -def is_ip(domain): - try: - socket.inet_pton(socket.AF_INET, domain) - return True - except socket.error: - try: - socket.inet_pton(socket.AF_INET6, domain) - return True - except socket.error: - return False - - class _CnameLookupContext(RewriteContext): base_re = r'^(https?://)%s(/.*)?$' @@ -108,6 +97,25 @@ class CNAMELookupMiddleware(object): self.storage_domain += [s for s in list_from_csv(storage_domain) if s.startswith('.')] self.lookup_depth = int(conf.get('lookup_depth', '1')) + nameservers = list_from_csv(conf.get('nameservers')) + try: + for i, server in enumerate(nameservers): + ip_or_host, maybe_port = nameservers[i] = \ + parse_socket_string(server, None) + if not is_valid_ip(ip_or_host): + raise ValueError + if maybe_port is not None: + int(maybe_port) + except ValueError: + raise ValueError('Invalid cname_lookup/nameservers configuration ' + 'found. All nameservers must be valid IPv4 or ' + 'IPv6, followed by an optional : port.') + self.resolver = dns.resolver.Resolver() + if nameservers: + self.resolver.nameservers = [ip for (ip, port) in nameservers] + self.resolver.nameserver_ports = { + ip: int(port) for (ip, port) in nameservers + if port is not None} self.memcache = None self.logger = get_logger(conf, log_route='cname-lookup') @@ -128,7 +136,7 @@ class CNAMELookupMiddleware(object): port = '' if ':' in given_domain: given_domain, port = given_domain.rsplit(':', 1) - if is_ip(given_domain): + if is_valid_ip(given_domain): return self.app(env, start_response) a_domain = given_domain if not self._domain_endswith_in_storage_domain(a_domain): @@ -141,7 +149,7 @@ class CNAMELookupMiddleware(object): memcache_key = ''.join(['cname-', a_domain]) found_domain = self.memcache.get(memcache_key) if found_domain is None: - ttl, found_domain = lookup_cname(a_domain) + ttl, found_domain = lookup_cname(a_domain, self.resolver) if self.memcache and ttl > 0: memcache_key = ''.join(['cname-', given_domain]) self.memcache.set(memcache_key, found_domain, diff --git a/test/unit/common/middleware/test_cname_lookup.py b/test/unit/common/middleware/test_cname_lookup.py index 9d1f85984a..ff40232c17 100644 --- a/test/unit/common/middleware/test_cname_lookup.py +++ b/test/unit/common/middleware/test_cname_lookup.py @@ -68,7 +68,7 @@ class TestCNAMELookup(unittest.TestCase): self.assertEqual(resp, ['FAKE APP']) @mock.patch('swift.common.middleware.cname_lookup.lookup_cname', - new=lambda d: (0, d)) + new=lambda d, r: (0, d)) def test_passthrough(self): req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': 'foo.example.com'}) @@ -85,7 +85,7 @@ class TestCNAMELookup(unittest.TestCase): self.assertEqual(resp, ['FAKE APP']) @mock.patch('swift.common.middleware.cname_lookup.lookup_cname', - new=lambda d: (0, '%s.example.com' % d)) + new=lambda d, r: (0, '%s.example.com' % d)) def test_good_lookup(self): req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': 'mysite.com'}) @@ -105,7 +105,7 @@ class TestCNAMELookup(unittest.TestCase): req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': 'mysite.com'}) - def my_lookup(d): + def my_lookup(d, r): if d == 'mysite.com': site = 'level1.foo.com' elif d == 'level1.foo.com': @@ -120,7 +120,7 @@ class TestCNAMELookup(unittest.TestCase): self.assertEqual(resp, ['CNAME lookup failed after 2 tries']) @mock.patch('swift.common.middleware.cname_lookup.lookup_cname', - new=lambda d: (0, 'some.invalid.site.com')) + new=lambda d, r: (0, 'some.invalid.site.com')) def test_lookup_chain_bad_target(self): req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': 'mysite.com'}) @@ -129,7 +129,7 @@ class TestCNAMELookup(unittest.TestCase): ['CNAME lookup failed to resolve to a valid domain']) @mock.patch('swift.common.middleware.cname_lookup.lookup_cname', - new=lambda d: (0, None)) + new=lambda d, r: (0, None)) def test_something_weird(self): req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': 'mysite.com'}) @@ -138,7 +138,7 @@ class TestCNAMELookup(unittest.TestCase): ['CNAME lookup failed to resolve to a valid domain']) @mock.patch('swift.common.middleware.cname_lookup.lookup_cname', - new=lambda d: (0, '%s.example.com' % d)) + new=lambda d, r: (0, '%s.example.com' % d)) def test_with_memcache(self): class memcache_stub(object): def __init__(self): @@ -175,7 +175,7 @@ class TestCNAMELookup(unittest.TestCase): self.cache[key] = value module = 'swift.common.middleware.cname_lookup.lookup_cname' - dns_module = 'dns.resolver.query' + dns_module = 'dns.resolver.Resolver.query' memcache = memcache_stub() with mock.patch(module) as m: @@ -236,7 +236,7 @@ class TestCNAMELookup(unittest.TestCase): self.assertFalse('cname-mysite5.com' in memcache.cache) @mock.patch('swift.common.middleware.cname_lookup.lookup_cname', - new=lambda d: (0, 'c.aexample.com')) + new=lambda d, r: (0, 'c.aexample.com')) def test_cname_matching_ending_not_domain(self): req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': 'foo.com'}) @@ -245,7 +245,7 @@ class TestCNAMELookup(unittest.TestCase): ['CNAME lookup failed to resolve to a valid domain']) @mock.patch('swift.common.middleware.cname_lookup.lookup_cname', - new=lambda d: (0, None)) + new=lambda d, r: (0, None)) def test_cname_configured_with_empty_storage_domain(self): app = cname_lookup.CNAMELookupMiddleware(FakeApp(), {'storage_domain': '', @@ -285,7 +285,7 @@ class TestCNAMELookup(unittest.TestCase): req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': 'c.a.example.com'}) module = 'swift.common.middleware.cname_lookup.lookup_cname' - with mock.patch(module, lambda x: (0, lookup_back)): + with mock.patch(module, lambda d, r: (0, lookup_back)): return app(req.environ, start_response) resp = do_test('c.storage1.com') @@ -323,7 +323,7 @@ class TestCNAMELookup(unittest.TestCase): req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': 'mysite.com'}) module = 'swift.common.middleware.cname_lookup.lookup_cname' - with mock.patch(module, lambda x: (0, 'example.com')): + with mock.patch(module, lambda d, r: (0, 'example.com')): resp = app(req.environ, start_response) self.assertEqual(resp, ['FAKE APP']) @@ -331,7 +331,7 @@ class TestCNAMELookup(unittest.TestCase): app = cname_lookup.CNAMELookupMiddleware(RedirectSlashApp(), {}) module = 'swift.common.middleware.cname_lookup.lookup_cname' - with mock.patch(module, lambda x: (0, 'cont.acct.example.com')): + with mock.patch(module, lambda d, r: (0, 'cont.acct.example.com')): req = Request.blank('/test', environ={'REQUEST_METHOD': 'GET'}, headers={'Host': 'mysite.com'}) resp = req.get_response(app) @@ -339,6 +339,89 @@ class TestCNAMELookup(unittest.TestCase): self.assertEqual(resp.headers.get('Location'), 'http://mysite.com/test/') + def test_configured_nameservers(self): + class MockedResolver(object): + def __init__(self): + self.nameservers = None + self.nameserver_ports = None + + def query(self, *args, **kwargs): + raise Exception('Stop processing') + + def reset(self): + self.nameservers = None + self.nameserver_ports = None + + mocked_resolver = MockedResolver() + dns_module = 'dns.resolver.Resolver' + + # If no nameservers provided in conf, resolver nameservers is unset + for conf in [{}, {'nameservers': ''}]: + mocked_resolver.reset() + with mock.patch(dns_module, return_value=mocked_resolver): + app = cname_lookup.CNAMELookupMiddleware(FakeApp(), conf) + self.assertIs(app.resolver, mocked_resolver) + self.assertIsNone(mocked_resolver.nameservers) + + # If invalid nameservers provided, resolver nameservers is unset + mocked_resolver.reset() + conf = {'nameservers': '127.0.0.1, 127.0.0.2, a.b.c.d'} + with mock.patch(dns_module, return_value=mocked_resolver): + with self.assertRaises(ValueError) as exc_mgr: + app = cname_lookup.CNAMELookupMiddleware(FakeApp(), conf) + self.assertIn('Invalid cname_lookup/nameservers configuration', + str(exc_mgr.exception)) + + # If nameservers provided in conf, resolver nameservers is set + mocked_resolver.reset() + conf = {'nameservers': '127.0.0.1'} + with mock.patch(dns_module, return_value=mocked_resolver): + app = cname_lookup.CNAMELookupMiddleware(FakeApp(), conf) + self.assertIs(app.resolver, mocked_resolver) + self.assertEqual(mocked_resolver.nameservers, ['127.0.0.1']) + self.assertEqual(mocked_resolver.nameserver_ports, {}) + + # IPv6 is OK + mocked_resolver.reset() + conf = {'nameservers': '[::1]'} + with mock.patch(dns_module, return_value=mocked_resolver): + app = cname_lookup.CNAMELookupMiddleware(FakeApp(), conf) + self.assertIs(app.resolver, mocked_resolver) + self.assertEqual(mocked_resolver.nameservers, ['::1']) + self.assertEqual(mocked_resolver.nameserver_ports, {}) + + # As are port overrides + mocked_resolver.reset() + conf = {'nameservers': '127.0.0.1:5354'} + with mock.patch(dns_module, return_value=mocked_resolver): + app = cname_lookup.CNAMELookupMiddleware(FakeApp(), conf) + self.assertIs(app.resolver, mocked_resolver) + self.assertEqual(mocked_resolver.nameservers, ['127.0.0.1']) + self.assertEqual(mocked_resolver.nameserver_ports, {'127.0.0.1': 5354}) + + # And IPv6 with port overrides + mocked_resolver.reset() + conf = {'nameservers': '[2001:db8::ff00:42:8329]:1234'} + with mock.patch(dns_module, return_value=mocked_resolver): + app = cname_lookup.CNAMELookupMiddleware(FakeApp(), conf) + self.assertIs(app.resolver, mocked_resolver) + self.assertEqual(mocked_resolver.nameservers, [ + '2001:db8::ff00:42:8329']) + self.assertEqual(mocked_resolver.nameserver_ports, { + '2001:db8::ff00:42:8329': 1234}) + + # Also accept lists, and bring it all together + mocked_resolver.reset() + conf = {'nameservers': '[::1], 127.0.0.1:5354, ' + '[2001:db8::ff00:42:8329]:1234'} + with mock.patch(dns_module, return_value=mocked_resolver): + app = cname_lookup.CNAMELookupMiddleware(FakeApp(), conf) + self.assertIs(app.resolver, mocked_resolver) + self.assertEqual(mocked_resolver.nameservers, [ + '::1', '127.0.0.1', '2001:db8::ff00:42:8329']) + self.assertEqual(mocked_resolver.nameserver_ports, { + '127.0.0.1': 5354, '2001:db8::ff00:42:8329': 1234}) + class TestSwiftInfo(unittest.TestCase): def setUp(self):