Skip to content

Commit 56ad2c8

Browse files
committed
CLOUDSTACK-6432: Prevent DNS reflection attacks
DNS on VR should not be publically accessible as it may be prone to DNS amplification/reflection attacks. This fixes the issue by only allowing VR DNS (port 53) to be accessible from guest network cidr, as per the fix in: https://issues.apache.org/jira/browse/CLOUDSTACK-6432 - Only allows guest network cidrs to query VR DNS on port 53. - Includes marvin smoke test that checks the VR DNS accessibility checks from guest and non-guest network. - Fixes Marvin sshClient to avoid using ssh agent when password is provided, previous some environments may have seen 'No existing session' exception without this fix. - Adds a new dnspython dependency that is used to perform dns resolutions in the tests. Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
1 parent b9801ef commit 56ad2c8

4 files changed

Lines changed: 282 additions & 14 deletions

File tree

systemvm/patches/debian/config/opt/cloud/bin/cs/CsAddress.py

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -394,12 +394,13 @@ def fw_router(self):
394394
self.fw.append(["filter", "", "-A INPUT -i lo -j ACCEPT"])
395395

396396
if self.get_type() in ["guest"]:
397+
guestNetworkCidr = self.address['network']
397398
self.fw.append(
398399
["filter", "", "-A INPUT -i %s -p udp -m udp --dport 67 -j ACCEPT" % self.dev])
399400
self.fw.append(
400-
["filter", "", "-A INPUT -i %s -p udp -m udp --dport 53 -j ACCEPT" % self.dev])
401+
["filter", "", "-A INPUT -i %s -p udp -m udp --dport 53 -s %s -j ACCEPT" % (self.dev, guestNetworkCidr)])
401402
self.fw.append(
402-
["filter", "", "-A INPUT -i %s -p tcp -m tcp --dport 53 -j ACCEPT" % self.dev])
403+
["filter", "", "-A INPUT -i %s -p tcp -m tcp --dport 53 -s %s -j ACCEPT" % (self.dev, guestNetworkCidr)])
403404
self.fw.append(
404405
["filter", "", "-A INPUT -i %s -p tcp -m tcp --dport 80 -m state --state NEW -j ACCEPT" % self.dev])
405406
self.fw.append(
@@ -436,8 +437,9 @@ def fw_vpcrouter(self):
436437
self.fw.append(["filter", "", "-A FORWARD -m state --state RELATED,ESTABLISHED -j ACCEPT"])
437438

438439
if self.get_type() in ["guest"]:
440+
guestNetworkCidr = self.address['network']
439441
self.fw.append(["filter", "", "-A FORWARD -d %s -o %s -j ACL_INBOUND_%s" %
440-
(self.address['network'], self.dev, self.dev)])
442+
(guestNetworkCidr, self.dev, self.dev)])
441443
self.fw.append(
442444
["filter", "front", "-A ACL_INBOUND_%s -d 224.0.0.18/32 -j ACCEPT" % self.dev])
443445
self.fw.append(
@@ -452,30 +454,26 @@ def fw_vpcrouter(self):
452454
self.fw.append(
453455
["filter", "", "-A INPUT -i %s -p udp -m udp --dport 67 -j ACCEPT" % self.dev])
454456
self.fw.append(
455-
["filter", "", "-A INPUT -i %s -p udp -m udp --dport 53 -j ACCEPT" % self.dev])
457+
["filter", "", "-A INPUT -i %s -p udp -m udp --dport 53 -s %s -j ACCEPT" % (self.dev, guestNetworkCidr)])
456458
self.fw.append(
457-
["filter", "", "-A INPUT -i %s -p tcp -m tcp --dport 53 -j ACCEPT" % self.dev])
459+
["filter", "", "-A INPUT -i %s -p tcp -m tcp --dport 53 -s %s -j ACCEPT" % (self.dev, guestNetworkCidr)])
458460

459461
self.fw.append(
460462
["filter", "", "-A INPUT -i %s -p tcp -m tcp --dport 80 -m state --state NEW -j ACCEPT" % self.dev])
461463
self.fw.append(
462464
["filter", "", "-A INPUT -i %s -p tcp -m tcp --dport 8080 -m state --state NEW -j ACCEPT" % self.dev])
463465
self.fw.append(["mangle", "",
464466
"-A PREROUTING -m state --state NEW -i %s -s %s ! -d %s/32 -j ACL_OUTBOUND_%s" %
465-
(self.dev, self.address[
466-
'network'], self.address['gateway'], self.dev)
467-
])
467+
(self.dev, guestNetworkCidr, self.address['gateway'], self.dev)])
468468

469469
self.fw.append(["", "front", "-A NETWORK_STATS_%s -i %s -d %s" %
470-
("eth1", "eth1", self.address['network'])])
470+
("eth1", "eth1", guestNetworkCidr)])
471471
self.fw.append(["", "front", "-A NETWORK_STATS_%s -o %s -s %s" %
472-
("eth1", "eth1", self.address['network'])])
472+
("eth1", "eth1", guestNetworkCidr)])
473473

474474
self.fw.append(["nat", "front",
475475
"-A POSTROUTING -s %s -o %s -j SNAT --to-source %s" %
476-
(self.address['network'], self.dev,
477-
self.address['public_ip'])
478-
])
476+
(guestNetworkCidr, self.dev, self.address['public_ip'])])
479477

480478
if self.get_type() in ["public"]:
481479
self.fw.append(["", "front",
Lines changed: 268 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,268 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing,
12+
# software distributed under the License is distributed on an
13+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
# KIND, either express or implied. See the License for the
15+
# specific language governing permissions and limitations
16+
# under the License.
17+
18+
import logging
19+
import dns.resolver
20+
21+
from nose.plugins.attrib import attr
22+
from marvin.cloudstackTestCase import cloudstackTestCase
23+
from marvin.lib.utils import cleanup_resources
24+
from marvin.lib.base import (ServiceOffering,
25+
VirtualMachine,
26+
Account,
27+
NATRule,
28+
FireWallRule,
29+
NetworkOffering,
30+
Network)
31+
from marvin.lib.common import (get_zone,
32+
get_template,
33+
get_domain,
34+
list_routers,
35+
list_nat_rules,
36+
list_publicIP)
37+
38+
39+
class TestRouterDns(cloudstackTestCase):
40+
41+
@classmethod
42+
def setUpClass(cls):
43+
cls.logger = logging.getLogger('TestRouterDns')
44+
cls.stream_handler = logging.StreamHandler()
45+
cls.logger.setLevel(logging.DEBUG)
46+
cls.logger.addHandler(cls.stream_handler)
47+
48+
cls.testClient = super(TestRouterDns, cls).getClsTestClient()
49+
cls.api_client = cls.testClient.getApiClient()
50+
cls.services = cls.testClient.getParsedTestDataConfig()
51+
52+
cls.domain = get_domain(cls.api_client)
53+
cls.zone = get_zone(cls.api_client, cls.testClient.getZoneForTests())
54+
cls.services['mode'] = cls.zone.networktype
55+
cls.template = get_template(
56+
cls.api_client,
57+
cls.zone.id,
58+
cls.services["ostype"]
59+
)
60+
cls.services["virtual_machine"]["zoneid"] = cls.zone.id
61+
62+
cls.logger.debug("Creating Admin Account for domain %s on zone %s" % (cls.domain.id, cls.zone.id))
63+
cls.account = Account.create(
64+
cls.api_client,
65+
cls.services["account"],
66+
admin=True,
67+
domainid=cls.domain.id
68+
)
69+
70+
cls.logger.debug("Creating Service Offering on zone %s" % (cls.zone.id))
71+
cls.service_offering = ServiceOffering.create(
72+
cls.api_client,
73+
cls.services["service_offering"]
74+
)
75+
76+
cls.logger.debug("Creating Network Offering on zone %s" % (cls.zone.id))
77+
cls.services["isolated_network_offering"]["egress_policy"] = "true"
78+
cls.network_offering = NetworkOffering.create(cls.api_client,
79+
cls.services["isolated_network_offering"],
80+
conservemode=True)
81+
cls.network_offering.update(cls.api_client, state='Enabled')
82+
83+
cls.logger.debug("Creating Network for Account %s using offering %s" % (cls.account.name, cls.network_offering.id))
84+
cls.network = Network.create(cls.api_client,
85+
cls.services["network"],
86+
accountid=cls.account.name,
87+
domainid=cls.account.domainid,
88+
networkofferingid=cls.network_offering.id,
89+
zoneid=cls.zone.id)
90+
91+
cls.logger.debug("Creating guest VM for Account %s using offering %s" % (cls.account.name, cls.service_offering.id))
92+
cls.vm = VirtualMachine.create(cls.api_client,
93+
cls.services["virtual_machine"],
94+
templateid=cls.template.id,
95+
accountid=cls.account.name,
96+
domainid=cls.domain.id,
97+
serviceofferingid=cls.service_offering.id,
98+
networkids=[str(cls.network.id)])
99+
cls.vm.password = "password"
100+
101+
cls.services["natrule1"] = {
102+
"privateport": 22,
103+
"publicport": 22,
104+
"protocol": "TCP"
105+
}
106+
107+
cls.services["configurableData"] = {
108+
"host": {
109+
"password": "password",
110+
"username": "root",
111+
"port": 22
112+
},
113+
"input": "INPUT",
114+
"forward": "FORWARD"
115+
}
116+
117+
cls._cleanup = [
118+
cls.vm,
119+
cls.network,
120+
cls.network_offering,
121+
cls.service_offering,
122+
cls.account
123+
]
124+
125+
126+
@classmethod
127+
def tearDownClass(cls):
128+
try:
129+
cleanup_resources(cls.api_client, cls._cleanup)
130+
except Exception as e:
131+
raise Exception("Warning: Exception during cleanup : %s" % e)
132+
133+
134+
def setUp(self):
135+
self.apiclient = self.testClient.getApiClient()
136+
self.cleanup = []
137+
138+
139+
def tearDown(self):
140+
try:
141+
cleanup_resources(self.apiclient, self.cleanup)
142+
except Exception as e:
143+
raise Exception("Warning: Exception during cleanup : %s" % e)
144+
145+
146+
def test_router_common(self):
147+
"""Performs common router tests and returns router public_ips"""
148+
149+
routers = list_routers(
150+
self.apiclient,
151+
account=self.account.name,
152+
domainid=self.account.domainid
153+
)
154+
155+
self.assertEqual(
156+
isinstance(routers, list),
157+
True,
158+
"Check for list routers response return valid data"
159+
)
160+
161+
self.assertTrue(
162+
len(routers) >= 1,
163+
"Check list router response"
164+
)
165+
166+
router = routers[0]
167+
168+
self.assertEqual(
169+
router.state,
170+
'Running',
171+
"Check list router response for router state"
172+
)
173+
174+
public_ips = list_publicIP(
175+
self.apiclient,
176+
account=self.account.name,
177+
domainid=self.account.domainid,
178+
zoneid=self.zone.id
179+
)
180+
181+
self.assertEqual(
182+
isinstance(public_ips, list),
183+
True,
184+
"Check for list public IPs response return valid data"
185+
)
186+
187+
self.assertTrue(
188+
len(public_ips) >= 1,
189+
"Check public IP list has at least one IP"
190+
)
191+
192+
return public_ips
193+
194+
195+
@attr(tags=["advanced", "advancedns", "ssh"], required_hardware="true")
196+
def test_router_dns_externalipquery(self):
197+
"""Checks that non-guest network IPs cannot access VR DNS"""
198+
199+
self.logger.debug("Starting test_router_dns_externalips...")
200+
201+
public_ip = test_router_common()[0]
202+
203+
self.logger.debug("Querying VR DNS IP: " + public_ip.ipaddress)
204+
resolver = dns.resolver.Resolver()
205+
resolver.namerservers = [public_ip.ipaddress]
206+
try:
207+
resolver.query('google.com', 'A')
208+
self.fail("Non-guest network IPs are able to access VR DNS, failing.")
209+
except:
210+
self.logger.debug("VR DNS query failed from non-guest network IP as expected")
211+
212+
213+
@attr(tags=["advanced", "advancedns", "ssh"], required_hardware="true")
214+
def test_router_dns_guestipquery(self):
215+
"""Checks that guest VM can query VR DNS"""
216+
217+
self.logger.debug("Starting test_router_dns_guestipquery...")
218+
public_ip = test_router_common()[0]
219+
220+
self.logger.debug("Creating Firewall rule for VM ID: %s" % self.vm.id)
221+
FireWallRule.create(
222+
self.apiclient,
223+
ipaddressid=public_ip.id,
224+
protocol=self.services["natrule1"]["protocol"],
225+
cidrlist=['0.0.0.0/0'],
226+
startport=self.services["natrule1"]["publicport"],
227+
endport=self.services["natrule1"]["publicport"]
228+
)
229+
230+
self.logger.debug("Creating NAT rule for VM ID: %s" % self.vm.id)
231+
nat_rule1 = NATRule.create(
232+
self.apiclient,
233+
self.vm,
234+
self.services["natrule1"],
235+
public_ip.id
236+
)
237+
nat_rules = list_nat_rules(
238+
self.apiclient,
239+
id=nat_rule1.id
240+
)
241+
self.assertEqual(
242+
isinstance(nat_rules, list),
243+
True,
244+
"Check for list NAT rules response return valid data"
245+
)
246+
self.assertTrue(
247+
len(nat_rules) >= 1,
248+
"Check for list NAT rules to have at least one rule"
249+
)
250+
self.assertEqual(
251+
nat_rules[0].state,
252+
'Active',
253+
"Check list port forwarding rules"
254+
)
255+
256+
result = None
257+
try:
258+
self.logger.debug("SSH into guest VM with IP: %s" % nat_rule1.ipaddress)
259+
ssh = self.vm.get_ssh_client(ipaddress=nat_rule1.ipaddress, port=self.services['natrule1']["publicport"], retries=8)
260+
result = str(ssh.execute("nslookup google.com"))
261+
except Exception as e:
262+
self.fail("Failed to SSH into VM - %s due to exception: %s" % (nat_rule1.ipaddress, e))
263+
264+
if not result:
265+
self.fail("Did not to receive any response from the guest VM, failing.")
266+
267+
self.assertTrue("google.com" in result and "#53" in result,
268+
"VR DNS should serve requests from guest network, unable to get valid nslookup result from guest VM.")

tools/marvin/marvin/sshClient.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,8 @@ def createConnection(self):
118118
port=self.port,
119119
username=self.user,
120120
password=self.passwd,
121-
timeout=self.timeout)
121+
timeout=self.timeout,
122+
allow_agent=False)
122123
else:
123124
self.ssh.connect(hostname=self.host,
124125
port=self.port,

tools/marvin/setup.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
"ddt >= 0.4.0",
5454
"pyvmomi >= 5.5.0",
5555
"netaddr >= 0.7.14",
56+
"dnspython",
5657
"ipmisim >= 0.7"
5758
],
5859
py_modules=['marvin.marvinPlugin'],

0 commit comments

Comments
 (0)