close 53

[andrijapanicsb]

close 53 (udp port 53 that is, and tcp ;)

[DaanHoogland]

@andrija, what is this with port 67?

[DaanHoogland]

I think we should delete the commented code. if need be replace with a regular comment and submit to security.

now cause of the motherload of the change

[DaanHoogland]

@nvazquez @andrijapanicsb @rhtyd @PaulAngus I just removed the entire class from the code as it was not doing anything anymore. But to be sure: BOOTP 67 isn't used ?

[DaanHoogland]

@blueorangutan package

[PaulAngus]

thb , i don't know, but i'd hope that it was there for a reason..

[DaanHoogland]

bootp when you have dhcp, does not seem to have a valid use-case? the only thing i can think of is some older component needing support that doesn't speak dhcp.

[PaulAngus]

What worries more, is the possibility that there are VPC configurations which would no longer be able to use their DNS because they are not instanciated 'properly' but worked because DNS was 'wide-open'

[DaanHoogland]

Agree, extensive testing needed. Should we share on sec@ yet?

[PaulAngus]

My comment really relates to this possible fix, and is really only valid if we propose simply removing this section of code. I don't know/understand the detailed model being followed to suggest alternatives, I am simply wary that;
a. 'we' didn't realise that this code was there, which made our community fix ineffective, and that we could misunderstand the reason for the port being made open here (this piece of code) the second time around as well.
b. others' work may have appeared to properly allow DNS ingress, but through luck rather than judgement.

removing this section of code may well be the correct solution, but 'once bitten, twice shy'

[andrijapanicsb]

Need to check why I didn't see any of those comments - ah it went into the subfolder next to other 8500+ emails... right.

Whatever ports are needed for the VM on the network to work - those have to be opened in that other file CsAddress.py, next to other ports like 53 and 8080

We aleady briefly tested 10d ago: 10.2.2.216
UDP port number 67 is the destination port of a DHCP server (and UDP port number 68 is used by the client)
In that test env above, VMs were getting IPs just fine, since (what a coincidence) there were also rules to allow port 67 there, next to 53, and 8080 in that second file CsAddress!

So yes, testing is required - but considering the wording "self.config.has_dns()" - I'm pretty sure was written by someone "unsecure by default" in mind - Nicolas has traced this to date back to pre-4.6 release and some more ago... - so it's really ancient code.

[andrijapanicsb]

i.e.:

cloudstack/systemvm/debian/opt/cloud/bin/cs/CsAddress.py

Lines 406 to 423 in 08b1267

	if self.get_type() in ["guest"]:
	guestNetworkCidr = self.address['network']
	self.fw.append(
	["filter", "", "-A INPUT -i %s -p udp -m udp --dport 67 -j ACCEPT" % self.dev])
	self.fw.append(
	["filter", "", "-A INPUT -i %s -p udp -m udp --dport 53 -s %s -j ACCEPT" % (self.dev, guestNetworkCidr)])
	self.fw.append(
	["filter", "", "-A INPUT -i %s -p tcp -m tcp --dport 53 -s %s -j ACCEPT" % (self.dev, guestNetworkCidr)])
	self.fw.append(
	["filter", "", "-A INPUT -i %s -p tcp -m tcp --dport 80 -m state --state NEW -j ACCEPT" % self.dev])
	self.fw.append(
	["filter", "", "-A INPUT -i %s -p tcp -m tcp --dport 8080 -m state --state NEW -j ACCEPT" % self.dev])
	self.fw.append(
	["filter", "", "-A FORWARD -i %s -o eth1 -m state --state RELATED,ESTABLISHED -j ACCEPT" % self.dev])
	self.fw.append(
	["filter", "", "-A FORWARD -i %s -o %s -m state --state NEW -j ACCEPT" % (self.dev, self.dev)])
	self.fw.append(
	["filter", "", "-A FORWARD -i eth0 -o eth0 -m state --state RELATED,ESTABLISHED -j ACCEPT"])

[DaanHoogland]

@andrija, we will need to test shared networks as well (and maybe even basic zones)

[github-actions]

Hi @dependabot[bot], your pull request has merge conflicts. Can you fix the conflicts and sync your branch with the base branch?