-
Notifications
You must be signed in to change notification settings - Fork 232
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add full IPv6 support to fwknop #1
Comments
Very cool. Any chance of [ knock protected services , knock listening ] X [ ipv6, ipv4+6 ] as well as current ipv4 only (knock & ports)? |
Hi, Should the ipv6 or ipv4 support be choosen at build time ? I mean, could the ip buffers be defined as following ?
or maybe this is going to be defined in a variable in fwknopd.conf and then the ip buffers should be defined as :
but I am not sure about the side effects :) I have started to replace all of the occurence of MAX_IPV4_STR by INET_ADDRSTRLEN for consistency purposes since this macro is already defined out of the fwknop source code. |
Ideally I think fwknop be able to support both ipv4 and ipv6 at run time without having to recompile (which would make it harder for users whenever there are mixed v4/v6 environments). The ipv6 compatibility work should also probably go into a dedicated branch which can be merged in when it's ready since it is going to be a large set of changes. |
This commit fixes a crash if the replay digest init() routine fails - fwknopd attempted to make use of replay tracking anyway. The crash was discovered during testing fwknopd with an AppArmor enforce policy deployed. The following stack trace shows the crash (taken before the previous static function commit): Program received signal SIGSEGV, Segmentation fault. __strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:31 31 ../sysdeps/x86_64/multiarch/../strlen.S: No such file or directory. (gdb) where #0 __strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:31 #1 0x00007f59cabd8b26 in add_replay_file_cache (opts=opts@entry=0x7fff3eaa0bb0, digest=digest@entry=0x0) at replay_cache.c:516 #2 0x00007f59cabd8cf5 in add_replay (opts=opts@entry=0x7fff3eaa0bb0, digest=digest@entry=0x0) at replay_cache.c:472 #3 0x00007f59cabd62eb in incoming_spa (opts=0x7fff3eaa0bb0) at incoming_spa.c:536 #4 0x00007f59ca56164e in ?? () from /usr/lib/x86_64-linux-gnu/libpcap.so.0.8 #5 0x00007f59cabd7175 in pcap_capture (opts=opts@entry=0x7fff3eaa0bb0) at pcap_capture.c:269 #6 0x00007f59cabd3d4d in main (argc=5, argv=0x7fff3eaa1458) at fwknopd.c:314
Using the 'fiu-run' fault injection binary, a couple of cases were turned up with libfko does not properly check the strdup() return value. This commit fixes these issues, and here is an illustration of the stack trace for one such issue: Core was generated by `../client/.libs/fwknop -A tcp/22 -a 127.0.0.2 -D 127.0.0.1 --get-key local_spa.'. Program terminated with signal 11, Segmentation fault. #0 __strnlen_sse2 () at ../sysdeps/x86_64/multiarch/../strnlen.S:34 34 ../sysdeps/x86_64/multiarch/../strnlen.S: No such file or directory. (gdb) where #0 __strnlen_sse2 () at ../sysdeps/x86_64/multiarch/../strnlen.S:34 #1 0x00007effa38189bc in _rijndael_encrypt (enc_key_len=<optimized out>, enc_key=<optimized out>, ctx=0x7effa5945750) at fko_encryption.c:141 #2 fko_encrypt_spa_data (ctx=0x7effa5945750, enc_key=<optimized out>, enc_key_len=<optimized out>) at fko_encryption.c:605 #3 0x00007effa381a2d6 in fko_spa_data_final (ctx=0x7effa5945750, enc_key=enc_key@entry=0x7fff3ff4aa10 "fwknoptest", enc_key_len=<optimized out>, hmac_key=hmac_key@entry=0x7fff3ff4aaa0 "", hmac_key_len=0) at fko_funcs.c:489 #4 0x00007effa405f2fb in main (argc=<optimized out>, argv=<optimized out>) at fwknop.c:449
Workaround for libpcap returning a length that is 4 bytes longer than the packet on the wire. Observed on: Linux beaglebone 3.8.13-bone50 mrash#1 SMP Tue May 13 13:24:52 UTC 2014 armv7l GNU/Linux ldd fwknopd libfko.so.2 => /usr/local/lib/libfko.so.2 (0xb6f62000) libpcap.so.0.8 => /usr/lib/arm-linux-gnueabihf/libpcap.so.0.8 (0xb6f20000) libc.so.6 => /lib/arm-linux-gnueabihf/libc.so.6 (0xb6e3b000) /lib/ld-linux-armhf.so.3 (0xb6f94000) libgcc_s.so.1 => /lib/arm-linux-gnueabihf/libgcc_s.so.1 (0xb6e17000) Calculate the new pkt_end from the length in the ip header.
Workaround for libpcap returning a length that is 4 bytes longer than the packet on the wire. Observed on: Linux beaglebone 3.8.13-bone50 mrash#1 SMP Tue May 13 13:24:52 UTC 2014 armv7l GNU/Linux ldd fwknopd libfko.so.2 => /usr/local/lib/libfko.so.2 (0xb6f62000) libpcap.so.0.8 => /usr/lib/arm-linux-gnueabihf/libpcap.so.0.8 (0xb6f20000) libc.so.6 => /lib/arm-linux-gnueabihf/libc.so.6 (0xb6e3b000) /lib/ld-linux-armhf.so.3 (0xb6f94000) libgcc_s.so.1 => /lib/arm-linux-gnueabihf/libgcc_s.so.1 (0xb6e17000) Calculate the new pkt_end from the length in the ip header.
In the meantime, what's the current way of forcing IPv4 with fwknop? I'm on an IPv6-enabled network, trying to connect to an IPv6 server and I do this:
That doesn't work because the DNS resolver gives me an IPv6 address for |
So, ssh -4 is able to resolve to an IPv4 address? I need to add the ability to force IPv4 then too. In your case the resolver must be returning both v6 and v4 addresses. |
It would be great if fwknop had a
Yes, my resolver returns both IPv4 and IPv6 addresses when I'm at work. Only IPv4 when I'm at home. |
Ok, looks like the client just needs to require the AF_INET family from getaddrinfo(). I'll send you a patch for testing. |
I've pushed a commit to master (b03c007) to add a new --server-resolve-ipv4 option. If you pull from git and recompile then the client should function in your environment I think. Please let me know if there are any issues with this code. Once I start on the full IPv6 support there will be additional options along these lines, but hopefully this will work in the meantime. |
That patch works for me, thanks! Any reason what that shouldn't be the default until IPv6 is supported? There's really no point in using IPv6 addresses right now when we know they're not going to work. |
Is this still an open issue? Did fwknop 'add full ipv6 support'? if yes, close this issue and maybe make it more obvious in documentation. If no, are there plans to do so? |
IPv6 support is coming likely in fwknop-3.0. So, fwknop does not yet support IPv6 today. There was a switch to getaddrinfo() in the client which will make supporting IPv6 easier (and this is already in current fwknop releases), but the main effort will be in getting the server to work with it. Actually it would be interesting to see how the new command open/close cycle stuff might help with this. But, the SPA packet format will also have to be updated to handle it. |
Is this work currently in flight? I have a need for this functionality and am interested in contributing to the solution. |
ipv6 work has not yet started, though it's on my short list of things to look at. |
https://www.iab.org/2016/11/07/iab-statement-on-ipv6/
In case you could need some additional motivation (but ofc it's ok if it'll take some time) 😉. |
Hi there, I just got contracted by Asahi Net, Inc. from Tokyo, Japan to contribute IPv6 support to the fwknop project. I will try to achieve this until the beginning of July this year. Any hint or procedure I should follow to achieve this the best way possible for this work to be easily integrated to the project is more than welcome! |
Hello @khorben. Adding IPv6 support to fwknop would indeed be an amazing contribution. I would start with a lightweight approach to this. On the server side, the easiest path would be to use the CMD_CYCLE feature to interface with a local firewall that is applying IPv6 policy (such as ip6tables). This way you wouldn't have to write something analogous to the 'server/fw_util_iptables.c' code, and the CMD_CYCLE feature is generally a lot more powerful anyway. The bulk of the work as you said would need to be on the SPA protocol itself. We had in the past the idea of a binary protocol, but I would not try to implement this for IPv6 support because it represents a total rewrite essentially. If you keep the current ascii protocol, but extend the current implementation to support an IPv6 address at every place that we can have an IPv4 address, then this would work. The result would not be backwards compatible, but IPv6 support is important enough to warrant this I think. Fortunately, the access request field is the main thing to concentrate on, and perhaps dealing with NAT too (although this is less important in IPv6 except that gateways between v4 and v6 would still be relevant here). Here is a good overview of all SPA packet formats and associated data: https://github.com/mrash/fwknop/blob/master/test/spa_fuzzing.py#L35 |
I might well be able to implement most, if not all, of the support for IPv6 without even breaking compatibility. |
Replace strlcpy() with memcpy() since the source buffer is not a string. strlcpy() caught this anyway, but memcpy() usage is probably more valid. ==29766==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x562bc2e50420 in strlcpy /home/mbr/git/fwknop.git/common/strlcpy.c:61:3 #1 0x562bc2e25362 in process_packet /home/mbr/git/fwknop.git/server/process_packet.c:225:5 #2 0x7fa6173c9d57 (/lib64/libpcap.so.1+0x1fd57) #3 0x562bc2e2456a in pcap_capture /home/mbr/git/fwknop.git/server/pcap_capture.c:227:15 #4 0x562bc2e13ef0 in main /home/mbr/git/fwknop.git/server/fwknopd.c:296:13 #5 0x7fa61643724a in __libc_start_main /usr/src/debug/glibc-2.27-74-g68c1bf8097/csu/../csu/libc-start.c:308:16 #6 0x562bc2d9dec9 in _start (/home/mbr/git/fwknop.git/server/.libs/fwknopd+0x1dec9) Uninitialized value was created by a heap allocation #0 0x562bc2da6c84 in malloc (/home/mbr/git/fwknop.git/server/.libs/fwknopd+0x26c84) #1 0x7fa6173ca996 (/lib64/libpcap.so.1+0x20996) SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/mbr/git/fwknop.git/common/strlcpy.c:61:3 in strlcpy
This should address mrash#1.
Any news? |
Looking forward to this! |
Sometime potentially for the fwknop 2.1 release we should add full IPv6 support to both the fwknop client and fwknop server. A start was made on this for the client with this commit: http://www.cipherdyne.org/cgi-bin/gitweb.cgi?p=fwknop.git;a=commitdiff;h=6f79b6fb04090c53bca9abe53fc15e13786587da;hp=31ef94024cea1edb3024c9f78efa30794aa81264
The text was updated successfully, but these errors were encountered: