jvinet / knock

A port-knocking daemon
http://www.zeroflux.org/projects/knock
GNU General Public License v2.0
549 stars 113 forks source link

PortKnocker ignoring seq_timeout with latest update #71

Closed wardmundy closed 3 years ago

wardmundy commented 3 years ago

A user on CentOS 7 platform is reporting that, after executing a yum update on his server, PortKnocker no longer honors the 15 second seq_timeout value and instead keeps the interaction open for 10 hours which is the cmd_timeout value. Using random TCP ports, an intruder managed to break in on his second try. Any ideas??

[root@pbx log]# cat /etc/knockd.conf [options] logfile = /var/log/knockd.log

[opencloseALL] sequence = 9149:tcp,7209:tcp,6962:tcp seq_timeout = 15 tcpflags = syn start_command = /usr/sbin/iptables -I INPUT -s %IP% -j ACCEPT cmd_timeout = 3600 stop_command = /usr/sbin/iptables -D INPUT -s %IP% -j ACCEPT

[2020-09-28 10:58] shutting down [2020-09-28 10:58] starting up, listening on eth0 *Above is the reboot after running yum updates*** [2020-09-28 18:21] 89.248.168.39: opencloseALL: Stage 1 [2020-09-28 18:21] 89.248.168.39: opencloseALL: Stage 2 [2020-09-29 04:21] 89.248.168.39: opencloseALL: sequence timeout (stage 2)

[2020-09-29 04:21] 89.248.168.39: opencloseALL: Stage 1 [2020-09-29 04:21] 89.248.168.39: opencloseALL: Stage 2 [2020-09-29 04:21] 89.248.168.39: opencloseALL: Stage 3 [2020-09-29 04:21] 89.248.168.39: opencloseALL: OPEN SESAME [2020-09-29 04:21] opencloseALL: running command: /usr/sbin/iptables -I INPUT -s 89.248.168.39 -j ACCEPT

TDFKAOlli commented 3 years ago

Hi @wardmundy,

my understanding is that the code works as intended. There is no timer started which expires aside of the packet sniff. Instead when fetching a packet which matches one of the sequence, the "sniff()" function checks whether it matches the sequence and/or is outdated. If you check here (and the lines following) it states that it will "cleanup expired/completed/failed attempts": https://github.com/jvinet/knock/blob/c54d3cc68b22990f307a3dc2e873ad395951d81a/src/knockd.c#L1533 And it only happens, when "sniff()" is called which is only called when a packet is received from pcap matching the configured packets. And it is done as a the first action in that function which is fine in my understanding. And in your example it says "sequence timeout (stage 2)", as the next packet recieved matching the configured packets is out of the sequence timeout. Here it is irrelevant if it comes 16 seconds after this first packet or 10 hours.

As far as I can see, this was always the implementation and there is no change lately. So I would propose to close this issue.

BR, TDFKAOlli

wardmundy commented 3 years ago

Based upon your comments, it would be more secure to immediately close a sequence if the second or third port ping'd doesn't match the stored value. In other words, if the sequence is 1, 4, 5 and the pings received are 1, 7, 8, 9, 4, the sequence should have been closed upon receipt of the 7 rather than waiting for the attacker to attempt guessing the 4 with random numbers. With a high powered server such as Amazon EC2, the attacker could send thousands of pings within a window of even a couple of seconds, and the attacker gains access with the current design. Hope that clarifies the issue. --— Ward Mundy

On Tue, Oct 13, 2020 at 10:19 AM TDFKAOlli notifications@github.com wrote:

Hi @wardmundy https://github.com/wardmundy,

my understanding is that the code works as intended. There is no timer started which expires aside of the packet sniff. Instead when fetching a packet which matches one of the sequence, the "sniff()" function checks whether it matches the sequence and/or is outdated. If you check here (and the lines following) it states that it will "cleanup expired/completed/failed attempts":

https://github.com/jvinet/knock/blob/c54d3cc68b22990f307a3dc2e873ad395951d81a/src/knockd.c#L1533 And it only happens, when "sniff()" is called which is only called when a packet is received from pcap matching the configured packets. And it is done as a the first action in that function which is fine in my understanding. And in your example it says "sequence timeout (stage 2)", as the next packet recieved matching the configured packets is out of the sequence timeout. Here it is irrelevant if it comes 16 seconds after this first packet or 10 hours.

As far as I can see, this was always the implementation and there is no change lately. So I would propose to close this issue.

BR, TDFKAOlli

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jvinet/knock/issues/71#issuecomment-707771189, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANARI37FVREX5QBOA5CD7TSKROXJANCNFSM4R54GCCQ .

TDFKAOlli commented 3 years ago

Again this is not how the code works. The sequence is turned into a pcap string which matches specific packets, not all packets. E.g. example above: sequence = 9149:tcp,7209:tcp,6962:tcp So only packets to port 9149, 7209 and 6962 are given to the "sniff()" function by pcap and only if those are tcp packets. By this not every packet floods into the knockd but only specific ones. (Which is good, because otherwise you would have a user space application seeing all network traffic of the server, causing a very high CPU load.) The sniff() function checks that the sequence is kept and the timing. E.g. it receives 9149, then 6962, the sequence is not matched. https://github.com/jvinet/knock/blob/c54d3cc68b22990f307a3dc2e873ad395951d81a/src/knockd.c#L1593

I think here the attempt is marked for deletion, so this out of order breaks the sequences and the sequence is closed: https://github.com/jvinet/knock/blob/c54d3cc68b22990f307a3dc2e873ad395951d81a/src/knockd.c#L1619 Could be easily checked by sending an mixed sequence to an existing server and check the output.

Again, I propose to close this.