Add IPv6 support to fwknop #285

khorben · 2018-08-20T09:01:57Z

This branch adds complete support for IPv6 on fwknop. It still has a few limitations though:

fwknop must be run with the -6 command-line parameter
when using IPTables, fwknop supports either IPv4 or IPv6 but not both at the same time
IPv6 is supported in UDP, TCP, and HTTP modes (both client and server)
This should close Add full IPv6 support to fwknop #1.

The remote code seems to be independent from the fwknop project though. Until it will be capable to return IPv6 addresses, in itself this will remain irrelevant for the purpose of adding IPv6 support to fwknop. On another hand, it does help us introduce definitions and update headers to actually support IPv6.

Again, this depends on the remote host to be actually supporting IPv6.

It is now called is_valid_ip_addr() and expects an additional parameter for the address family.

This greatly loosens the check for a valid IPv4/IPv6 there - but it is redundant anyway, since there is always a call to is_valid_ip_addr().

This alone should allow interacting with IPv4 firewalling rules over IPv6, for these two protocols.

This will allow porting the raw ICMP code to IPv6.

I believe it should be more portable this way, since AF_INET is required to be present in <sys/socket.h> in POSIX.

This should help with portability for the protocol family eventually.

This should eventually help with portability to IPv6.

The correct syntax is the comma (",") instead.

The problem looks real, fix looks good to me, but have not been able to reproduce the actual issue so far :(

mrash · 2018-08-22T01:52:23Z

Wow, tons of work. I have a local ipv6 branch to track this. I'm going to add some test suite support for IPv6 so I can work into what you've done, and then push this branch to github so we can collaborate. Let's target the next release of fwknop to merge your changes into master.

This should address #1.

Fixes #1.

securitygeneration · 2018-08-29T12:59:14Z

That's indeed some epic work. Well done @khorben! Now if only the world could finally move to IPv6 ;)

khorben · 2018-08-29T15:57:53Z

Thank you :)

khorben · 2018-08-29T15:59:51Z

This was sponsored by Asahi Net, Inc. by the way, https://asahi-net.co.jp/en/corporate/ (as mentioned in #1)

4mig4 · 2018-08-30T02:48:56Z

Great work Pierre thanks a lot , much appreciated.

mrash · 2018-09-02T02:04:37Z

Diving into this finally, sorry for the delay. I've opened a new issue based on a start on IPv6 test suite support.

mrash · 2018-10-15T02:41:13Z

Targeting this work for the next major release of fwknop.

bastien-roucaries

the size of ipv6 scoped litteral could be more than 40....

bastien-roucaries

Please instead of MAX_IP46_STR_LENGTH use INET6_ADDRSTRLEN

While you are right that longtest IPv6 address takes 39 bytes, with IPv4 tunneling, the longest form can be 45 bytes:

bastien-roucaries

You should change and use constant

bastien-roucaries · 2020-04-03T22:09:04Z

client/fwknop_common.h

@@ -87,8 +87,8 @@ typedef struct fko_cli_options
    int  no_save_args;
    int  use_hmac;
    char spa_server_str[MAX_SERVER_STR_LEN];  /* may be a hostname */
-    char allow_ip_str[MAX_IPV4_STR_LEN];
-    char spoof_ip_src_str[MAX_IPV4_STR_LEN];
+    char allow_ip_str[MAX_IPV46_STR_LEN];


use INET6_ADDRSTRLEN
While you are right that longtest IPv6 address takes 39 bytes, with IPv4 tunneling, the longest form can be 45 bytes:

bastien-roucaries · 2020-04-03T22:09:12Z

client/fwknop_common.h

-    char allow_ip_str[MAX_IPV4_STR_LEN];
-    char spoof_ip_src_str[MAX_IPV4_STR_LEN];
+    char allow_ip_str[MAX_IPV46_STR_LEN];
+    char spoof_ip_src_str[MAX_IPV46_STR_LEN];


use INET6_ADDRSTRLEN

bastien-roucaries · 2020-04-03T22:16:43Z

lib/fko_limits.h

@@ -59,6 +59,12 @@
 #define MAX_IPV4_STR_LEN             16


INET4_ADDRSTRLEN

bastien-roucaries · 2020-04-03T22:17:03Z

lib/fko_limits.h

@@ -59,6 +59,12 @@
 #define MAX_IPV4_STR_LEN             16
 #define MIN_IPV4_STR_LEN              7

+#define MAX_IPV46_STR_LEN            40


INET6_ADDRSTRLEN

bastien-roucaries · 2020-04-03T22:17:15Z

lib/fko_limits.h

+#define MAX_IPV46_STR_LEN            40
+#define MIN_IPV46_STR_LEN             3
+
+#define MAX_IPV6_STR_LEN             40


INET6_ADDRSTRLEN

bastien-roucaries

Ok with this one

bastien-roucaries · 2020-04-03T22:20:11Z

client/http_resolve_host.c

-    int     o1, o2, o3, o4, got_resp=0, i=0;
-    char   *ndx, resp[MAX_IPV4_STR_LEN+1] = {0};
+    int     got_resp=0, error;
+    char    resp[MAX_IPV4_STR_LEN+1] = {0};


MAX_IPV46_STR_LEN ?

bastien-roucaries · 2020-04-03T22:21:08Z

client/http_resolve_host.c

+    for (rp = result; rp != NULL; rp = rp->ai_next) {
+	/* the canonical value is in the first structure returned */
+	strlcpy(options->allow_ip_str,
+	        rp->ai_canonname, sizeof(options->allow_ip_str));


Why getting last entry ?

bam80 · 2023-05-31T15:49:45Z

Why it wasn't merged still?

khorben · 2023-05-31T15:56:40Z

I do not know. The reviewer (@bastien-roucaries) seems to have approved the changes 3 years ago; on my side I haven't had the opportunity to comment on the proposed changes.
Maybe the remaining limitations are considered unsuitable for merge into master; @mrash?

bam80 · 2023-06-06T10:12:58Z

@mrash Could you comment please?
Without IPv6 support, it looks mostly as abandon-ware these days, so it makes sense to clarify the project status..

@damienstuart @bastien-roucaries
#344 #309

mrash · 2023-06-06T12:06:25Z

Hi all, Apologies for the long delays. Generally I need to get a maintenance release out for fwknop to get grounded in the latest Linux distributions and account for drift over the past few years. This should happen before any large feature additions like IPv6 support. On the IPv6 support itself, there are a couple of observations. First, if we can get nft support into fwknop, then v6 support becomes a lot easier. Either way though, how many combinations of v4/v6 should be supported? If a v6 request is made, should an ACCEPT rule go into both the iptables and ip6tables rules? If a v6 request is made, should the assumption be that the access will come over v6? Or v4? How about the other way, so using v4 to gain access to a service that is only advertised via v6? Maybe we start with some advertised assumptions that @khorben makes at the start - such as the fact that v4+v6 are not supported simultaneously and work up from there if user demand asks for additions. This would allow for iterative releases of this feature. Also, for a feature like this, we definitely need to add comprehensive test suite support, and that is not there yet. Let me get a maintenance release out of fwknop in the next week or so, and then look at this for a major release. Thanks,

…

--Mike

On Tue, Jun 6, 2023 at 6:13 AM Andrey Butirsky ***@***.***> wrote: @mrash <https://github.com/mrash> Could you comment please? Without IPv6 support, it looks mostly as abandon-ware these days, so it makes sense to clarify the project status.. — Reply to this email directly, view it on GitHub <#285 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAC42RENIJ6RP3II2WLUPGLXJ37DLANCNFSM4FQONLOQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

-- Michael Rash | Founder http://www.cipherdyne.org/ Key fingerprint = 53EA 13EA 472E 3771 894F AC69 95D8 5D6B A742 839F

bam80 · 2023-06-06T12:20:26Z

Thank you for the update Michael.

First, if we can get nft support into fwknop, then v6 support becomes a lot easier.

Yeah, that is the other blocker: #107.
Without it we can't even run the daemon on recent popular router firmware:
#352
https://forum.openwrt.org/t/fwknop-on-x86-64-22-03-2/143855
So should be probably addressed first indeed.

There seems to be a lot of work ahead but I hope we will get there eventually!

khorben added 30 commits August 20, 2018 10:59

Let IPs resolve to IPv6 addresses over SSL

b8252db

Again, this depends on the remote host to be actually supporting IPv6.

Be more consistent when creating ~/.fwknoprc

460bd8c

Also catch the IPv6 version of INADDR_ANY

a0dda67

Migrate is_valid_ipv4_addr() to a more generic function

b3494dc

It is now called is_valid_ip_addr() and expects an additional parameter for the address family.

Implement is_valid_ip_addr() with getaddrinfo()

aea56f5

Give have_allow_ip() a chance to allow IPv6

f7b18d6

This greatly loosens the check for a valid IPv4/IPv6 there - but it is redundant anyway, since there is always a call to is_valid_ip_addr().

Allow connecting to remote IPv6 hosts for TCP or UDP

2f6ea52

This alone should allow interacting with IPv4 firewalling rules over IPv6, for these two protocols.

Use uppercase to log "IP"

72a50b9

Also check for <netinet/ip6.h> and <netinet/icmp6.h>

2367bc2

This will allow porting the raw ICMP code to IPv6.

Rework the network listening routine

ce9b5fb

Rework the network listening routine some more

3dc6116

Code cleanup

1a813bb

Also include <netinet/ip6.h> where relevant

a525734

Protect some more headers for inclusion

37a8000

Add support for receiving SPA messages over IPv6

6bcaf4f

Interpret incoming addresses according to their family

8ecd10b

Let access list stanzas be defined in IPv6

89c7d6f

Typo

bd1c488

Typo

a2462c6

Constify

82a5eec

Use a more appropriate type for sd_len

17549b9

Allow access control "ANY" with any protocol family

d1c1373

Use AF_INET instead of PF_INET

baed23c

I believe it should be more portable this way, since AF_INET is required to be present in <sys/socket.h> in POSIX.

Use sizeof() instead of re-using hard-coded values

f61a308

This should help with portability for the protocol family eventually.

Use sizeof() instead of re-using hard-coded values

b070e80

This should help with portability for the protocol family eventually.

Use a constant for AF_INET

da1ab05

This should eventually help with portability to IPv6.

Use /usr/bin/env to locate perl

76d609b

Rework IPv6 support when comparing addresses

587a4fb

Fix processing command-line arguments with whitespace

11e9b29

khorben added 5 commits August 20, 2018 11:02

Use "::" for matching any address for IPv6 rules

73d7b79

No longer accept colon (":") as port separator

460774c

The correct syntax is the comma (",") instead.

Fix DNAT with IPv6

f3895bb

Use "::/0" for IPT_ANY_IPV6

46d4d77

Port src_dst_check() to IPv6

3f3b304

The problem looks real, fix looks good to me, but have not been able to reproduce the actual issue so far :(

This was referenced Aug 20, 2018

Support for IPv6 (1/x) #275

Closed

Allow connecting to remote IPv6 hosts for TCP or UDP #272

Closed

mrash self-assigned this Aug 22, 2018

khorben added 2 commits August 22, 2018 16:41

Also output the value obtained for ip6tables

a2edaec

This should address #1.

Complete the check and search for ip6tables

91f3842

Fixes #1.

bastien-roucaries suggested changes Apr 3, 2020

View reviewed changes

bastien-roucaries approved these changes Apr 3, 2020

View reviewed changes

bastien-roucaries suggested changes Apr 3, 2020

View reviewed changes

bastien-roucaries approved these changes Apr 3, 2020

View reviewed changes

davidandreoletti mentioned this pull request Jun 14, 2023

Linux NFTables Support in fwknop #107

Open

Seb35 mentioned this pull request Oct 17, 2023

nftables firewall #353

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add IPv6 support to fwknop #285

Add IPv6 support to fwknop #285

khorben commented Aug 20, 2018 •

edited

mrash commented Aug 22, 2018

securitygeneration commented Aug 29, 2018

khorben commented Aug 29, 2018

khorben commented Aug 29, 2018

4mig4 commented Aug 30, 2018

mrash commented Sep 2, 2018

mrash commented Oct 15, 2018

bastien-roucaries left a comment

bastien-roucaries left a comment

bastien-roucaries left a comment

bastien-roucaries Apr 3, 2020

bastien-roucaries Apr 3, 2020

bastien-roucaries Apr 3, 2020

bastien-roucaries Apr 3, 2020

bastien-roucaries Apr 3, 2020

bastien-roucaries left a comment

bastien-roucaries Apr 3, 2020

bastien-roucaries Apr 3, 2020

bam80 commented May 31, 2023

khorben commented May 31, 2023

bam80 commented Jun 6, 2023 •

edited

mrash commented Jun 6, 2023 via email

bam80 commented Jun 6, 2023 •

edited

Add IPv6 support to fwknop #285

Are you sure you want to change the base?

Add IPv6 support to fwknop #285

Conversation

khorben commented Aug 20, 2018 • edited

mrash commented Aug 22, 2018

securitygeneration commented Aug 29, 2018

khorben commented Aug 29, 2018

khorben commented Aug 29, 2018

4mig4 commented Aug 30, 2018

mrash commented Sep 2, 2018

mrash commented Oct 15, 2018

bastien-roucaries left a comment

Choose a reason for hiding this comment

bastien-roucaries left a comment

Choose a reason for hiding this comment

bastien-roucaries left a comment

Choose a reason for hiding this comment

bastien-roucaries Apr 3, 2020

Choose a reason for hiding this comment

bastien-roucaries Apr 3, 2020

Choose a reason for hiding this comment

bastien-roucaries Apr 3, 2020

Choose a reason for hiding this comment

bastien-roucaries Apr 3, 2020

Choose a reason for hiding this comment

bastien-roucaries Apr 3, 2020

Choose a reason for hiding this comment

bastien-roucaries left a comment

Choose a reason for hiding this comment

bastien-roucaries Apr 3, 2020

Choose a reason for hiding this comment

bastien-roucaries Apr 3, 2020

Choose a reason for hiding this comment

bam80 commented May 31, 2023

khorben commented May 31, 2023

bam80 commented Jun 6, 2023 • edited

mrash commented Jun 6, 2023 via email

bam80 commented Jun 6, 2023 • edited

khorben commented Aug 20, 2018 •

edited

bam80 commented Jun 6, 2023 •

edited

bam80 commented Jun 6, 2023 •

edited