Page MenuHomePhabricator

Review default ferm INPUT policy
Open, MediumPublic

Description

Recently we have been looking at updating diffscan so that we can scan all TCP ports instead of the nmap top 2000 however the scan is taking some time (>24h) to complete largely because the scans now need to time out on 65k ports instead of 2k ports. We can update the nmap timings to try and bring this time down dramatically and indeed a test using nmaps T5 setting did bring the scan time down to about ~35 minutes however it also added a lot of false positives due to the various timers being too aggressive.

One way that we could speed up the nmap scans is to configure a default REJECT rule at the bottom of our ferm rules. This would mean that the nmap scan would receive a rst for closed ports and much more quickly determine that the port is closed (as appose to filtered when the packet times out). We could restrict such a rule to just the diffscan server however this runs the risk that we have a slightly different picture of reality compared to a truly external actor. As such i wanted to consider adding such a rule for all sources.

The advantage of this is that miss configured servers, our own or community services, would receive a rst packet and allow them to fail fast instead of waiting for the the connection to time out. This would also likely prevent some retry traffic (although likely unnoticeable compared to the larger background)

Regarding the down side @ayounsi already pointed out that this is not desirable on network gear as it means the packet has to first reach the control plane to construct the RST packet instead of responding from the edge. There will also be some additional resources required on hosts for the linux kernel to construct the RST packet. This would also mean that a malicious actor could use us to reflect RST packets however the 40b rst packet comes at a cost of a 60b syn packet. so it would be better to reflect syn/ack (60b) packets from a known open port e.g. 443.

The final point i would make is that although it is recommended via many standards e.g. PCI , ico27002, NIST etc to have a default drop policy i believe the aim is making it harder for a user to enumerate open ports. This is not a concern for us as our firewall rule base is already public via puppet.

Im sure I'm missing something so wanted to create this task to gather thoughts

(i also drafted a quick CR as a starting point)

Related Objects

StatusSubtypeAssignedTask
OpenNone
OpenNone

Event Timeline

Change 632543 had a related patch set uploaded (by Jbond; owner: John Bond):
[operations/puppet@production] (WIP) firewall: change to default reject instead of drop

https://gerrit.wikimedia.org/r/632543

I don't think the reflection is a concern; everything from NTP to memcache to OpenVPN to CLDAP is a better reflector by orders of magnitude.

Applying default-reject on hosts would greatly improve overall nmap times even if we don't do the same on network devices, right? Seems like a fine compromise to me.

I don't think the reflection is a concern; everything from NTP to memcache to OpenVPN to CLDAP is a better reflector by orders of magnitude.

Completely agree i just mentioning for completeness or incase there is something else round here im missing

Applying default-reject on hosts would greatly improve overall nmap times even if we don't do the same on network devices, right?

Yes, correct

As discussed over IRC, that LGTM.
Would it be easy to rollback if there are any issues or the result is not as fast as expected?

Would it be easy to rollback if there are any issues or the result is not as fast as expected?

Yes and although the current CR doesn't include this, To begin with i would propose we enable this via a hiera parameter to so we can try with a single host before rolling out more widely

Agreed, the service enumation/information disclosure angle is moot for us, so let's give this a shot. If we make it configurable via Hiera we can also test it beforehand on a few hosts.

@jbond FYI as a side note once this is deployed we can probably revisit a bit the firewall rules of the failoid hosts, that were designed already with reject-with icmp-port-unreachable to make the failure as quick as possible.

Overall, I am willing to test this out, couples of points though:

  • Since it's recommended by various standards to do the default DROP thing, we should assume most people expect that behavior and not REJECT. So I 'd say we need to document, communicate this and perhaps add it to onboarding ("Hey we differ by everyone else in this way")
  • There is going to be a change in behavior when debugging connections between servers. With DROP, you can just do a telnet <IP> <PORT> and if you don't get a response, you know that either
  1. the server is dead
  2. there is some network ACL somewhere, including the server itself.

If you get a Connection refused, you can discard both of the above with a rather high level of confidence and focus on why whatever you want to talk to that isn't listening on the server

With REJECT number Connection refused means that item number 2 above still applies. This is going to take some time to get used to.

Change 632897 had a related patch set uploaded (by Jbond; owner: John Bond):
[operations/puppet@production] sretest1001: Enable default reject rule for sretest1001

https://gerrit.wikimedia.org/r/632897

Overall, I am willing to test this out, couples of points though:

  • Since it's recommended by various standards to do the default DROP thing, we should assume most people expect that behavior and not REJECT. So I 'd say we need to document, communicate this and perhaps add it to onboarding ("Hey we differ by everyone else in this way")

Yes i think thats a good idea, Assuming all goes well and we do end up this as a permanent change. Ill send a mail to ops and work with @ayounsi to update the networking onboarding docs and wikitech pages

  • There is going to be a change in behavior when debugging connections between servers. With DROP, you can just do a telnet <IP> <PORT> and if you don't get a response, you know that either
  1. the server is dead
  2. there is some network ACL somewhere, including the server itself.

If you get a Connection refused, you can discard both of the above with a rather high level of confidence and focus on why whatever you want to talk to that isn't listening on the server

With REJECT number Connection refused means that item number 2 above still applies. This is going to take some time to get used to.

One saving grace is it should still be in the log, will look to see if we can include some advice around this when updating the aforementioned docs, thanks

This would also mean that a malicious actor could use us to reflect RST packets however the 40b rst packet comes at a cost of a 60b syn

This is not quite correct, the proposal is to reject with icmp-port-unreachable whih has a size of about 90b so there is some amplification (although still much less then other protocols.

I had hoped the use of icmp from iptables could help with the later point from alex but even nmap --reason shows just conn-refused for both rst and icmp-port-unreachable. Of course you see it in tcpdump but thats not the most usefull advice from something that should be a quick debugging clue.

FWIW, I am in general a fan of REJECT over DROP, especially when there's not even a great obscurity argument, as is the case here. It will be a change for us internally on the debugging woes mentioned, but it's more-correct in the overall, and letting things that will eventually fail do so faster seems like it's always a net win :)

Change 632933 had a related patch set uploaded (by Jbond; owner: John Bond):
[operations/puppet@production] diffscan: add defeat-rst-ratelimit

https://gerrit.wikimedia.org/r/632933

Change 632897 abandoned by Jbond:
[operations/puppet@production] sretest1001: Enable default reject rule for sretest1001

Reason:
enable on idp-test1001 instead as that has an external ip https://gerrit.wikimedia.org/r/c/operations/puppet/ /632937

https://gerrit.wikimedia.org/r/632897

Change 632933 merged by Jbond:
[operations/puppet@production] diffscan: add defeat-rst-ratelimit

https://gerrit.wikimedia.org/r/632933

Change 632543 merged by Jbond:
[operations/puppet@production] firewall: change to default reject instead of drop

https://gerrit.wikimedia.org/r/632543

Change 632948 had a related patch set uploaded (by Jbond; owner: John Bond):
[operations/puppet@production] base::firewall: use ferm::rule instead of ferm::conf

https://gerrit.wikimedia.org/r/632948

Change 632948 merged by Jbond:
[operations/puppet@production] base::firewall: use ferm::rule instead of ferm::conf

https://gerrit.wikimedia.org/r/632948

Thanks all quick update. I have deployed the firewall change to idp-test1001 and the scan time about 3x faster with the new rule (see below). however when looking at the current process (which has been running for about 48+ hours now) i noticed that there where a lot of LVS systems. so i started playing with them and trying various options and noticed that the --defeat-rst-ratelimit switch had a significant impacts. From 1262.84 seconds to 47.71 seconds im still not sure what is ratelimiting or droping RST;s so suggestions welcome. Either way i have applied a change to make this the default (please see change for more info on this specific switch).

Another thing i noticed is that the network devices which have interfaces in every VLAN are also quite slow to scan (taking 10mins even with nmaps aggressive T5 switch). So this is another area where we could optimise. possibly scanning them separately with different params or trying to find a happy medium.

I will try running nmap tonight to see how much difference speed up we have gained by just adding --defeat-rst-ratelimit which i suspect will be significant as a large amount of the addresses scanned are lvs addresses. Depending on the results it may be that changing our policy to a REJECT by default is not required. however i, like @BBlack, prefer reject to drop if possible. As such it would be nice to be good internet citizens and move ahead with the initial proposal even if we can get diffscan to work with a default drop. That said one thing we should consider is the standard requirements (likely PCI) that frack have to abide by which may well have instructions to insist on default DROP

Scan with DROP

root@diffscan:~#  nmap -vv -sS -PE -PS22,25,80,443,3306,8443,9100  -T4 --privileged --defeat-rst-ratelimit -p- 208.80.154.87
Nmap scan report for idp-test1001.wikimedia.org (208.80.154.87)
Host is up, received echo-reply ttl 62 (0.00075s latency).
Scanned at 2020-10-08 15:18:13 UTC for 440s
Not shown: 65534 filtered ports
Some closed ports may be reported as filtered due to --defeat-rst-ratelimit
Reason: 65534 no-responses
PORT    STATE SERVICE REASON
443/tcp open  https   syn-ack ttl 62

Read data files from: /usr/bin/../share/nmap
Nmap done: 1 IP address (1 host up) scanned in 440.08 seconds

Scan with Reject

root@diffscan:~# nmap -vv -sS -PE -PS22,25,80,443,3306,8443,9100  -T4 --privileged --defeat-rst-ratelimit -p- 208.80.154.87
Nmap scan report for idp-test1001.wikimedia.org (208.80.154.87)
Host is up, received echo-reply ttl 62 (0.00075s latency).
Scanned at 2020-10-08 15:18:13 UTC for 440s
Not shown: 65534 filtered ports
Some closed ports may be reported as filtered due to --defeat-rst-ratelimit
Reason: 65401 no-responses and 133 port-unreaches
PORT    STATE SERVICE REASON
443/tcp open  https   syn-ack ttl 62

Read data files from: /usr/bin/../share/nmap
Nmap done: 1 IP address (1 host up) scanned in 133.23 seconds

however i, like @BBlack, prefer reject to drop if possible. As such it would be nice to be good internet citizens and move ahead with the initial proposal even if we can get diffscan to work with a default drop.

+1 on that.

That said one thing we should consider is the standard requirements (likely PCI) that frack have to abide by which may well have instructions to insist on default DROP

Fortunately that's not a concern; the servers running the frack infrastructure use an unrelated Puppet repository from the main puppet.git, so they won't be affected.

Change 633156 had a related patch set uploaded (by Jbond; owner: John Bond):
[operations/puppet@production] Firewall: Change the default firewall rule fleet wide

https://gerrit.wikimedia.org/r/633156

Change 633157 had a related patch set uploaded (by Jbond; owner: John Bond):
[operations/puppet@production] Firewall: Change the default firewall rule cloud environment

https://gerrit.wikimedia.org/r/633157

no sure why but some host take a looong time

Completed SYN Stealth Scan against 208.80.153.45 in 50099.78s (63 hosts left)
Completed SYN Stealth Scan against 208.80.153.47 in 50373.67s (62 hosts left)
Completed SYN Stealth Scan against 208.80.153.46 in 50534.23s (61 hosts left)
Completed SYN Stealth Scan against 208.80.153.55 in 50840.30s (60 hosts left)
Completed SYN Stealth Scan against 208.80.153.52 in 51037.06s (59 hosts left)
Completed SYN Stealth Scan against 208.80.153.48 in 51106.43s (58 hosts left)
Completed SYN Stealth Scan against 208.80.153.56 in 51138.96s (57 hosts left)
Completed SYN Stealth Scan against 208.80.153.51 in 51142.13s (56 hosts left)
JMeybohm triaged this task as Medium priority.Oct 13 2020, 9:51 AM

no sure why but some host take a looong time

While debugging this i noticed that we where receiving a lot of messages like the following (avalible at -dd)

Ultrascan PING SENT to 208.80.154.26 [tcp to port 443; flags: S]
Ultrascan DROPPED PING probe packet to 208.80.154.26 detected

The ultrascan ping probe is used to ensure a host is still responding during a scan and to adjust the scan engine timing and scheduling values. roughly nmap picks a port it knows is open and sends a syn to it about every second and uses the syn/ack response or lack there off to determin if the host is still up and how lossy the network connection is. adjusting the send delay and other timeout values whenever a drop is determined. when testing the values i saw seemed to follow some sort of pattern. further when i looked at tcpdump i noticed that the ping probes where receiving a response and as such should not be reporting about dropped packets.

This all ultimately led me to a GH issue and commits[1][2]which seems to fix the problem. I compiled a version of nmap from master and tested this against idp1001 and managed to reduce the scan timeout from 438.79s to 87s. As such i will next backport a version of nmap from unstable and test it on diffscan to see what this looks like.

Further one more thing to add to the puzzle is that i also tried a scan against idp1001 with a default drop rule vs a default reject rule and to my surprise nmap is actually about 35% faster when scanning with the default *DROP* in place this may be caused by the overhead of processing all the rst packets and i have no idea if we will see the same behaviour when scanning multiple hosts.

scanning idp1001 with drop in place
PORT    STATE SERVICE REASON
443/tcp open  https   syn-ack ttl 63

Read data files from: /etc
Nmap done: 1 IP address (1 host up) scanned in 87.58 seconds
           Raw packets sent: 131139 (5.770MB) | Rcvd: 71 (3.108KB)
scanning idp1001 with reject in place
PORT    STATE SERVICE REASON
443/tcp open  https   syn-ack ttl 63

Read data files from: /etc
Nmap done: 1 IP address (1 host up) scanned in 134.76 seconds
           Raw packets sent: 131020 (5.765MB) | Rcvd: 144 (10.320KB)

[1]https://github.com/nmap/nmap/commit/7e644b391ea16f74dd57f5865be545d514de540d
[2]https://github.com/nmap/nmap/commit/e48361523b18596c023f565053e56d934fff70bd

I have created a diffscan using the scan file from 05-10-202 which used nmap --topports 2000 and a file from today which uses all ports. I will review manually but could be good for others who are familiar with this report to also give it an eyeball @MoritzMuehlenhoff @ayounsi

I have gone through the report and removed anything which is known or expected and made comments on the paste highlighting the diff and an explanation for the removal. I will raise tasks to address some of the issues observed

jbond closed subtask Restricted Task as Resolved.Oct 20 2020, 10:07 AM
ayounsi closed subtask Restricted Task as Resolved.Oct 27 2020, 10:22 AM
taavi closed subtask Restricted Task as Resolved.Feb 7 2023, 10:19 PM

Change 633157 abandoned by Jbond:

[operations/puppet@production] Firewall: Change the default firewall rule cloud environment

Reason:

we decided not to go this route

https://gerrit.wikimedia.org/r/633157

Change 633156 abandoned by Jbond:

[operations/puppet@production] Firewall: Change the default firewall rule fleet wide

Reason:

I thik we decided this wasn't needed

https://gerrit.wikimedia.org/r/633156