Option: Don't fail on duplicate knocks#72
Option: Don't fail on duplicate knocks#72escitalopram wants to merge 5 commits intojvinet:masterfrom
Conversation
To be used when you can't control the amount of e.g. SYN packets sent, e.g. browser often send multiple SYN packets. Always on right now, configuration follows
via options config section and add docs
jvinet
left a comment
jvinet
left a comment
There was a problem hiding this comment.
Just a couple stylistic inconsistencies to address, and then we can merge. Thank you for the contribution!
src/knockd.c
Outdated
| * may be duplicated (e.g. if a browser always sends 2 SYN-packets) | ||
| */ | ||
| int already_matched(knocker_t * attempt, unsigned short dport, uint8_t proto) { | ||
| if (o_duplicate) { |
There was a problem hiding this comment.
In an effort to maintain consistency in code style, could you please adjust this function's style to match the rest of the file?
- Please remove the space between
iforfor, and its opening parenthesis, eg,if(o_duplicate) { - In the function signature, please remove the space between the asterisk and the type, eg,
int already_matched(knocker_t* attempt, ...
There was a problem hiding this comment.
I just noticed that there is a pcap filter that seems to filter out only ports that are mentioned in knock sequences. Doesn't this reduce the difficulty to guess the sequence significantly in general?
When combined with this new feature, though, you are basically bound to crack a port sequence just by repeatedly portscanning a few times, given the right circumstances. So this request can't be merged in its current form because it's a gaping security hole.
Do you think it is viable if I just disable the port filtering when this feature is active?
There was a problem hiding this comment.
You're speaking of the generate_pcap_filter() function, I presume? We ask pcap to only give us packets that we care about (ie, ports within one or more of our sequences), because it is infeasible to examine every TCP/UDP packet moving through the system. If we do that, your CPU will jump to 100% under realistic levels of network traffic, so we pretty much have to let the filter do its work.
Doesn't this reduce the difficulty to guess the sequence significantly in general?
Hmm, I don't see how, but I could be missing something. Does a portscan reveal something about the ports in the knock sequence? In my local tests with nmap, I cannot see anything notable, but maybe you do?
There was a problem hiding this comment.
Hmm, I don't see how, but I could be missing something. Does a portscan reveal something about the ports in the knock sequence
No, what I meant was that it could make brute-forcing the sequence way easier. Theoretically, as soon as an attacker has knocked the first port, there are 65534 ports that will reset the sequence, when knocked, excluding the port next in sequence and 0, if there is no filtering. With the filter, only (number of ports mentioned-1) will reset the sequence while the others are ignored.
Regarding the CPU problem: Maybe we could go the other way around and just exclude a lot of well-known ports that potentially have lots of traffic, and make that port list also configurable for people using the odd port? Then still almost any wrongly knocked port will reset the sequence and the cpu usage will be reasonably low. Then we could also automatically refuse knock sequences from the config, if they contain any of those potential high-traffic ports.
There was a problem hiding this comment.
Mhmm, I might be wrong, but only the ports you added as your own sequence are added to the pcap filter. As such knockd only sees the ports you set (lets say 3 ports) and only those can reset the sequence.
Anyhow from the view of an attacker:
First I need to know you use knockd. It is not visible from the outside in a port scan. (Or I could do one port scan and then a second to see if an additional port is now open).
Then even if I know, I don't know in which order to knock your ports or what kind of packets I have to use. Port scans usually run from port 1 to x (e.g. 65535). So increasing order.
Simple example:
You use port 1001, 1002 and 1003 and all three TCP with Syn.. well bad choice I think. According to my understanding this means the port scan in increasing order will touch all your ports and open the door.
But assume you use UDP, then nothing happens. And as an attacker I can't even see the difference between the two (only from doing a port scan again and seeing that another port has opened).
Just using a different order already make it very difficult. Lets say you use port 1001, 1003 and 1002. Now an increasing port scan would not work. 1-1000 is ignored because knockd doesn't see them. 1001 is stage 1 but then comes 1002 and this would be out-of-order and cancel the sequence. Next port 1003 is then also out-of-order because a new sequence is started and 1001 is expected. The rest up to 65535 is irrelevant.
Now of course I could port-scan so that 1001 is first touched, then 1003 and then 1002. But without knowledge of your sequence how should I come to this conclusion? I could only try all kind of combinations and orders of 3 (or more?) ports with all combination of UDP, TCP and syn, fin, etc. packets. And you always have to do another full scan to just see if your previous scan had any effect on another port to be open now. I guess that makes it save in the end (if you select a proper sequence).
Maybe there is a mistake in my understanding? If not, then in my understanding allowing knocking one port again doesn't weaken the security of it. How should that help an attacker in the given examples above?
There was a problem hiding this comment.
Ok, maybe I'm overreacting, because I've had a security contractor discover a (admittedly bad) knock sequence a few years ago. So if @jvinet has no problems with this merge request, I guess I'm okay with it too.
For my personal paranoia I can still add a few dozen ports to some NOOP-sequences in my config.
4 participants
This adds the AllowDuplicates configuration directive. If specified, knockd will ignore duplicates of packets that already matched the knock sequence. This allows for browser-based knocking.