Skip to content
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

Open
mrash opened this issue Jun 24, 2011 · 18 comments · May be fixed by #285
Open

Add full IPv6 support to fwknop #1

mrash opened this issue Jun 24, 2011 · 18 comments · May be fixed by #285
Milestone

Comments

@mrash
Copy link
Owner

mrash commented Jun 24, 2011

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

@skull-squadron
Copy link
Contributor

Very cool. Any chance of [ knock protected services , knock listening ] X [ ipv6, ipv4+6 ] as well as current ipv4 only (knock & ports)?

@fjoncourt
Copy link
Contributor

Hi,

Should the ipv6 or ipv4 support be choosen at build time ?

I mean, could the ip buffers be defined as following ?

#ifdef _IPV4_SUPPORT_
    dst_ip[INET_ADDRSTRLEN];
#else
    dst_ip[INET6_ADDRSTRLEN];
#endif

or maybe this is going to be defined in a variable in fwknopd.conf and then the ip buffers should be defined as :

dst_ip[INET6_ADDRSTRLEN];

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.

@mrash
Copy link
Owner Author

mrash commented Jul 4, 2013

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.

mrash added a commit that referenced this issue Aug 19, 2013
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
mrash added a commit that referenced this issue Jun 12, 2014
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
stubbsw added a commit to stubbsw/fwknop that referenced this issue Aug 17, 2014
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.
stubbsw added a commit to stubbsw/fwknop that referenced this issue Aug 17, 2014
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.
@fmarier
Copy link
Contributor

fmarier commented Sep 24, 2015

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:

fwknop -n server.example.com
ssh -4 server.example.com

That doesn't work because the DNS resolver gives me an IPv6 address for server.example.com. I have to hack /etc/hosts and put an IPv4 address in there for the server in order to make the fwknop call work. Is there a nicer work-around?

@mrash
Copy link
Owner Author

mrash commented Sep 26, 2015

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.

@fmarier
Copy link
Contributor

fmarier commented Sep 26, 2015

So, ssh -4 is able to resolve to an IPv4 address? I need to add the ability to force IPv4 then too.

It would be great if fwknop had a -4 option as well. In fact, until IPv6 support is in, it probably makes sense to default to whatever ssh does with -4.

In your case the resolver must be returning both v6 and v4 addresses.

Yes, my resolver returns both IPv4 and IPv6 addresses when I'm at work. Only IPv4 when I'm at home.

@mrash
Copy link
Owner Author

mrash commented Sep 26, 2015

Ok, looks like the client just needs to require the AF_INET family from getaddrinfo(). I'll send you a patch for testing.

@mrash
Copy link
Owner Author

mrash commented Sep 27, 2015

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.

@fmarier
Copy link
Contributor

fmarier commented Oct 1, 2015

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.

fmarier pushed a commit to fmarier/user-scripts that referenced this issue Oct 1, 2015
@sparrell
Copy link
Contributor

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?

@mrash
Copy link
Owner Author

mrash commented Feb 14, 2016

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.

@mstair
Copy link
Contributor

mstair commented Sep 7, 2016

Is this work currently in flight? I have a need for this functionality and am interested in contributing to the solution.

@jp-bennett
Copy link
Collaborator

ipv6 work has not yet started, though it's on my short list of things to look at.

@primeos
Copy link

primeos commented Mar 20, 2017

https://www.iab.org/2016/11/07/iab-statement-on-ipv6/

The IAB expects that the IETF will stop requiring IPv4 compatibility in new or extended protocols. Future IETF protocol work will then optimize for and depend on IPv6.

Preparation for this transition requires ensuring that many different environments are capable of operating completely on IPv6 without being dependent on IPv4 [see RFC 6540]. We recommend that all networking standards assume the use of IPv6, and be written so they do not require IPv4. We recommend that existing standards be reviewed to ensure they will work with IPv6, and use IPv6 examples.

In case you could need some additional motivation (but ofc it's ok if it'll take some time) 😉.

mrash pushed a commit that referenced this issue Mar 17, 2018
@khorben
Copy link
Contributor

khorben commented Apr 26, 2018

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!
My first concern is with the protocol itself and how to document/standardize it. I have to proceed and deliver a working implementation regardless of its approval upstream here, so the sooner this can be clarified, the better IMHO.

@mrash
Copy link
Owner Author

mrash commented Apr 28, 2018

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

@khorben
Copy link
Contributor

khorben commented Jun 14, 2018

I might well be able to implement most, if not all, of the support for IPv6 without even breaking compatibility.
The protocol is text-based and there is no ambiguity that I can think of between IPv4 and IPv6 addresses, so I expect to be able to do this successfully.

mrash added a commit that referenced this issue Aug 18, 2018
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
@khorben khorben linked a pull request Aug 20, 2018 that will close this issue
@bam80
Copy link

bam80 commented May 30, 2023

Any news?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants