opt-in flag --tcpmss to add MSS option to SYN packet; remediates (real-world) observed border policy drops

[mzpqnxow]

This patch is associated with the discussion #326

I was considering adding this for quite a while but it seemed like a solution in search of a problem. However, I recently encountered a network that was dropping the default masscan SYN probes. The network 157.185.128.0/18 (QUANTIL-NETWORKS, AS54994) has this behavior. You can reproduce it using:

masscan -p 80,443,8000 157.185.177.217 --rate 3

This will not return any open ports. However, manually completing a TCP connection (with curl, telnet, whatever) works on all three ports. I tried nmap, using nmap -p 80,443,8000 -sS -Pn 157.185.177.217 and saw that it worked as expected, returning all three ports as open

I quickly diffed the packets and the obvious difference was nmap was including a TCP MSS. I'm guessing this is some sort of border policy that drops SYN packets without any options set- quite possibly to filter masscan probes specifically

This patch adds --tcpmss (admittedly a pretty ugly name, maybe --mss would be better) which when specified, fixes up the TCP packet template at init time so that it contains an MSS option. It updates the IP header length and TCP header length, from 0x28 -> 0x2C and (0x5 << 2) -> (0x6 << 2) which is needed to make the packet parsing that takes place shortly after work correctly

I tested this against 157.185.177.217 using the same command as above (but with --tcpmss added) and received a SYN|ACK response for all three ports

I apologize for the ugliness of this code, but the code around it isn't exactly beautiful either, so I didn't spend much time trying to make it look nice. It's not focused on any sort of efficiency since it runs before the txloop even begins

[mzpqnxow]

If someone is willing to do some regression testing for the layer-7 features (e.g. --capture cert --capture html --banner) I would be very appreciative

[mzpqnxow]

I did some basic testing with --banner, --capture, etc. and didn't see any regressions

[p-l-]

FWIW the patch integrated to ivre's fork has been updated.

[mohemiv]

We have the same issue.

@robertdavidgraham Are there any chances you can merge this PR and make a release?

[mzpqnxow]

We have the same issue.

@robertdavidgraham Are there any chances you can merge this PR and make a release?

Highly recommend using the ivre fork, @p-l- does an excellent job of maintaining it (including accepting reasonable/necessary PRs) and there are quite a few bug-fixes and added features; the majority of the enhancements are useful for most users, not just those using it as part of IVRE

And it's where this PR ultimately ended up

[mohemiv]

I tested the fork and found that the --selftest flag doesn't work there and, more importantly, Issues page is disabled. I'm a bit afraid to use such a fork.

https://aur.archlinux.org/packages/masscan-ivre-git I've already made an AUR package for it, but as you can see the tests are disabled and there is nothing I can do with it.

[mzpqnxow]

I tested the fork and found that the --selftest flag doesn't work there and, more importantly, Issues page is disabled. I'm a bit afraid to use such a fork.

So you're stuck choosing between an unmaintained repository with a --selftest and an active one without it, I suppose 😞

I can't speak for @p-l- but I suspect he might accept a PR to fix the selftest issue if it could be shown that it ever did anything useful in the first place, and it was "fixed" correctly and provided some value

I may be misremembering but I thought the selftest was actually broken a long time ago. It was my impression that it was only actually useful during the earliest releases when there was still some development in critical areas of the code. May be wrong there

https://aur.archlinux.org/packages/masscan-ivre-git

I've already made an AUR package for it, but as you can see the tests are disabled and there is nothing I can do with it.

I'm not familiar with how arch packaging works but are you saying it's preventing you from being able to use a package that you created yourself? I thought Arch users lived on the edge?! 😝

Anyway, sorry I couldn't be more helpful. If I had time I would look into the selftest flag but I only have enough spare cycles to throw notes like this into issues right now, not fix (or break!) anything new

I was considering closing this PR soon but if it's helpful to you if it stays open, I can leave it as-is. Maybe one day it will be reviewed/merged

Perhaps if you send rob some $BTC... 😉

[mohemiv]

I just say that if you disable Issues page on GitHub it can't be a full-fledged fork. This is why I decided to ask @robertdavidgraham for a merge instead of doing PRs to ivre fork.

[p-l-]

@mohemiv that was not intended, that was default settings after a fork I guess. That's updated.

This fork was intended mostly for use with IVRE, but I'm clearly OK to integrate reasonable / useful fixes.

~~For now, the fork is regularly rebased against master from here, which makes it harder to use. I will probably change how this works but need to think a bit about it.~~ I have updated the README and starting from now will use pull requests and classical merges to keep the fork in sync with upstream.

If a need exists for this fork to be more widely usable, I'm happy to update the current practices if it may help.

[p-l-]

I'll add that tests have been partly fixed in ivre/masscan#6. Tests are OK on Ubuntu ~~but KO on MacOS: https://github.com/ivre/masscan/actions/runs/5840441507/~~ and MacOS (maybe a bit flaky).

[mohemiv]

Nice to hear it! I've updated the AUR package and enabled the tests. Also I have included your account to the maintainers list.

[mzpqnxow]

I’m going to close this now as I don’t think it’s going anywhere. Thanks @p-l- and @mohemiv for the discussion and collaboration