Add support of ipv6 on client and server (asked by issue #8)

svalat commented 6 years ago

Hi, I was looking for ipv6 support for one of my server so I implemented it. It should close #8 .

The patch contains changes to knock client and to knockd server to support v4/v6.

Client

About the client, there is a choice to be done for the default mode. I currently use the default IP returned by DNS resolving, which could be ipv6. Then you can choose to force v4/v6 by using option -4 / --ipv4 or -6 / --ipv6.

The user can give a host name or an ipv6 like with v4 mode.

To be seen if it is not better to keep v4 only by default, can be patched by changing the default value of o_ip line 55 in knock.c.

If we keep the default ip returned by the DNS maybe we should add a message to tell the user which ip we selected, otherwise someone might get stuck not knowing that he knock on ipv6 instead of v4.

Server

The server by default extract all the IPs from the selected interface (v6 and v4). Then it builds the pcap rules for all those ips. We can disable v6 support by using options -4 / --only-ip-v4.

Again to be seen if we do not make the invert and require enabling of ipv6 instead of enabled by default.

There is a trick for the pcap rules: tcp flags (SYN, FIN...) are not supported for ipv6 so I had to hardcode the extraction. Eg for FIN :

if (ipv6)
        bufsize = realloc_strcat(&buffer, " and ip6[13+40] & tcp-fin ", bufsize);//using directly mask as pcap didn't yet support flags for IPv6
else
        bufsize = realloc_strcat(&buffer, " and tcp[tcpflags] & tcp-fin ", bufsize);

Local address

IPv6 link often setup a local IP on every link, in the current patch I also handle the knocking on this local IP, if you want to disable this feature import my branch no-local there is a commit to get same behavior than the debian proposal from issue #8.

One thing on this, don't know why when requesting the list of IP of the selected interface the local link is returnes with an extra string : a0a6:sds4...:44%enp0s3. So to proceed I remove what is after %.

About IPv6 packet encapsulation

IPv6 can handle extended header for some usage. This is not handled by this patch, I considered the TCP/UDP header directly after the UDP packet. The other cases are detected and ignored.

Server rules

About the rules, we might want to call iptables to open access but ipv6 is handled by ip6tables instead of iptables. In order to proceed we provide two separate commands, one for ipv4 and one for ipv6 in the config file :

[options]
        logfile = /var/log/knockd.log

[openSSH]
        sequence    = 7000,8000,9000
        seq_timeout = 5
        command     = /bin/echo OPEN %IP% V4
        command_6   = /bin/echo OPEN %IP% V6
        tcpflags    = syn

[closeSSH]
        sequence    = 9000,8000,7000
        seq_timeout = 5
        command     = /bin/echo CLOSE %IP% V4
        command_6   = /bin/echo CLOSE %IP% V6
        tcpflags    = syn

Same for start_command_6 and stop_command_6.

New dependency

The code now depend on :

#include <netinet/ip6.h>

Doc

I also updated the documentation (--help and manpages) accordingly and add an ipv6 config file example for the server.

Bugfix

There was a double free corruption in knockd cleanup() function when the interface has multiple IPs.

Validation

I validated the full chain by makeing port knocking on ipv4 and v6 on one of my server using tcp-syn.

TDFKAOlli commented 5 years ago

Hi, I would like to propose a small additional change. I'm using the same command for IPv4 and IPv6, so I don't like to double the "command". So when command6 is not set I would like to default to command. Patch for this attached. (Note: Done on #53, #52 and #55, so line numbers might not match). --- a/src/knockd.c 2018-09-29 23:55:24.330999447 +0200 +++ b/src/knockd.c 2018-09-30 00:04:38.333019487 +0200 @@ -1469,6 +1469,11 @@ if (attempt->fromIpV6) { start_command = attempt->door->start_command6; + // Default to "command" if "command6" is not set + if(NULL == start_command) + { + start_command = attempt->door->start_command; + } stop_command = attempt->door->stop_command6; } else { start_command = attempt->door->start_command;

svalat commented 5 years ago

I made it for start & stop. Can you please test on your setup ? I do not currently have a IPV6 server under my hand.

As in your proposal I use NULL to detect not defined, please check it is enougth. It might keep a definition with empty value to be setup as "no command".

I also updated the manpage.

TDFKAOlli commented 5 years ago

Thanks a lot. I'll update to use your change. I have tested it with my proposal and that was working fine on my router. Before nothing was executed on IPv6 knock, with change the "command" line was exectued using same config. As such I'm very confident it is working. And I thought the same, just checking for NULL is best here, as it offers to disable the command by providing an empty string. I'll let you know when I tested it.

TDFKAOlli commented 5 years ago

Just noticed when compiling with new ipv6 .patch file from this pull request:

src/knockd.c: In function 'sniff': src/knockd.c:1654:12: warning: 'ip6' may be used uninitialized in this function [-Wmaybe-uninitialized] dport = ntohs(tcp->th_dport); ^~~~~~~~ I think compiler is right in that sense, that ip6 is not intiated in the alternative else-cases of "if(lltype == DLT_EN10MB)". So e.g. if lltype == DLT_LINUX_SLL or lltype == DLT_RAW, ip6 would be uninitialized and later in "else if (ip->ip_v == 6) {" we would try to use and dereference ip6...

Probably works in OpenWRT because all packets are of type DLT_EN10MB.

EDIT: Checked on OpenWRT and it is working. I still have problems to overwrite the default by an empty string, because of OpenWRT UCI config interpretation. But I did one manual change to check it at it is working.

EDIT2: Sketched the fix for the compiler warning: @@ -1598,9 +1598,11 @@ #ifdef __linux__ } else if(lltype == DLT_LINUX_SLL) { ip = (struct ip*)((u_char*)packet + 16); + ip6 = (struct ip6_hdr*)((u_char*)packet + 16); #endif } else if(lltype == DLT_RAW) { ip = (struct ip*)((u_char*)packet); + ip6 = (struct ip6_hdr*)((u_char*)packet); } else { dprint("link layer header type of packet not recognized, ignoring...\n"); return;