Add dual-stack socket support for FTL #180

DL6ER · 2017-12-26T10:58:35Z

By submitting this pull request, I confirm the following (please check boxes, eg [X]) Failure to fill the template will close your PR:

Please submit all pull requests against the development branch. Failure to do so will delay or deny your request

I have read and understood the contributors guide.
I have checked that another pull request for this purpose does not exist.
I have considered, and confirmed that this submission will be valuable to others.
I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
I give this submission freely, and claim no ownership to its content.

How familiar are you with the codebase?:

10

This pull request adds full dual-stack support for FTL's telnet-like socket. Previously, we bound only to an IPv4 port, now we bind to both the IPv4 and IPv6 port spaces.

In case no IPv6 interface is available (e.g. in docker), FTL will bind only to the IPv4 port - just as it did before.

This template was created based on the work of udemy-dl.

Signed-off-by: DL6ER <[email protected]>

…ravis-ci/travis-ci#8361) Signed-off-by: DL6ER <[email protected]>

Signed-off-by: DL6ER <[email protected]>

DL6ER · 2017-12-26T13:27:35Z

Before this pull request:

$ echo ">quit" | nc -v 127.0.0.1 4711
Connection to 127.0.0.1 4711 port [tcp/*] succeeded!

$ echo ">quit" | nc -v ::1 4711
nc: connect to ::1 port 4711 (tcp) failed: Connection refused

With the proposed changes:

$ echo ">quit" | nc -v 127.0.0.1 4711
Connection to 127.0.0.1 4711 port [tcp/*] succeeded!

$ echo ">quit" | nc -v ::1 4711
Connection to ::1 4711 port [tcp/*] succeeded!

Signed-off-by: DL6ER <[email protected]> Conflicts: socket.c

DL6ER · 2018-01-09T18:46:29Z

Resolved merge conflicts, what you you think? @Mcat12 @dschaper

AzureMarker · 2018-01-09T18:54:11Z

socket.c

@@ -117,7 +151,7 @@ void bind_to_telnet_port(char type, int *socketdescriptor)
 		exit(EXIT_FAILURE);
 	}

-	logg("Listening on port %i for incoming connections", port);
+	logg("Listening on port %i for incoming %s connections", port, dualstack == true ? "IPv4 + IPv6" : "IPv4");


The == true can be removed since dualstack is a boolean already.

AzureMarker · 2018-01-09T18:55:49Z

socket.c

 		if(bind(*socketdescriptor, (struct sockaddr *) &serv_addr, sizeof(serv_addr)) < 0)
 		{
 			logg("Error on binding on port %i", port);
 		}
 		else
 		{
 			bound = true;
+			dualstack = true;
 			break;
 		}
 	}


On a non-IPv6 system this would produce 20 lines of errors saying it can't bind to a port. Is there a quicker way to figure out if we have IPv6 access?

I'm not sure how to detect if the system has a working ipv6 interface. If we cannot find out how to do it, we could also change the error reporting (don't report on the Individual failures, but only once at the end when we couldn't find at least one port).

Signed-off-by: DL6ER <[email protected]>

…accordingly (don't try to bind to a dual-stack port if the machine isn't capable to handling IPv6, e.g. in docker) Signed-off-by: DL6ER <[email protected]>

Signed-off-by: DL6ER <[email protected]>

DL6ER · 2018-01-09T23:50:20Z

Marking as [WIP]

While in6addr_any accepts IPv4 and IPv6 connections simultaneously, in6addr_loopback only accepts connections to ::1. Have to further investigate on this.

… IPv4-only and one for IPv6-only) to be able to bind to the corresponding loopback interfaces. We listen independently, but handle incoming requests using the same code. Signed-off-by: DL6ER <[email protected]>

DL6ER · 2018-01-10T19:52:53Z

Completely re-wrote this feature.

In case you are interested , here is the result of my research:
An IPv4 client can connect to INADDR_LOOPBACK (a.k.a. 127.0.0.1) on an dual-stack listening socket *if the listening socket is bound to IN6ADDR_ANY (a.k.a. ::). This is very convenient and allows to write more simple code that can live in both, the IPv4 and the IPv6 world at the same time.

However, an essential security feature of FTL (that can be disabled trough the config file) is that FTL does, by default, only bind to the lo interface such that data is only available locally and not from anywhere else in the network.
However, when binding a dual-stack socket to IN6ADDR_LOOPBACK (a.k.a. ::1), this will prevent IPv4 clients from being able to connect. Although he reason is simple it not very obvious: First, we have to understand that the socket lives in the IPv6 world, i.e. when an IPv4 client connects, it will be seen by the socket as connecting from the corresponding IPv4-mapped IPv6 address. Hence, when connecting to/from 127.0.0.1 the dual-stack socket will see the connection going to the IPv4-mapped IPv6 address ::FFFF:7F00:0001 (the equivalent of 127.0.0.1). However, as the dual-stack socket is explicitly bound to ::1 (and not ::) the IPv4 client cannot connect.

AzureMarker · 2018-01-10T19:49:00Z

socket.c

 {
+	// Try IPv4 only socket
+	// see the comments further up for details


This comment is no longer valid

AzureMarker · 2018-01-10T19:52:29Z

socket.c

-		logg("Reason: %s (%i)", strerror(errno), errno);
+	if(bind(*socketdescriptor, (struct sockaddr *) &address, sizeof (address)) != 0)
+	{
+		logg("Error on binding on unix socket %s: %s (%i)", FTLfiles.socketfile, strerror(errno), errno);


Unix is capitalized elsewhere.

AzureMarker · 2018-01-10T19:54:06Z

socket.c

@@ -335,16 +383,69 @@ void *telnet_listening_thread(void *args)
 	pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);

 	// Set thread name
-	prctl(PR_SET_NAME,"telnet listener",0,0,0);
+	prctl(PR_SET_NAME,"telnet-v4",0,0,0);


Should use IPv4 to distinguish this from appearing to be telnet version 4 API.

AzureMarker · 2018-01-10T19:58:52Z

socket.c

+	pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
+
+	// Set thread name
+	prctl(PR_SET_NAME,"telnet-v6",0,0,0);


Signed-off-by: DL6ER <[email protected]>

DL6ER added 4 commits December 26, 2017 11:31

Add full IPv6 support for FTL socket connections

b506aab

Signed-off-by: DL6ER <[email protected]>

Add IPv6 socket connection test

a29da60

Signed-off-by: DL6ER <[email protected]>

Disable IPv6 test for now due to broken IPv6 support on Travis (see t…

30fb2e7

…ravis-ci/travis-ci#8361) Signed-off-by: DL6ER <[email protected]>

Add IPv4-only fallback if dual-stack socket is not available

9d7617f

Signed-off-by: DL6ER <[email protected]>

DL6ER added the Enhancement label Dec 26, 2017

DL6ER added this to the v3.0 milestone Dec 26, 2017

DL6ER requested review from dschaper and AzureMarker December 26, 2017 10:58

DL6ER added 2 commits December 26, 2017 12:05

Adjust client name determination

22e3412

Signed-off-by: DL6ER <[email protected]>

Add client name determination for the fallback solution

80ec6b2

Signed-off-by: DL6ER <[email protected]>

DL6ER removed this from the v3.0 milestone Dec 26, 2017

DL6ER changed the title ~~Add dual-stack socket support for FTL~~ [WIP] Add dual-stack socket support for FTL Dec 26, 2017

DL6ER changed the title ~~[WIP] Add dual-stack socket support for FTL~~ Add dual-stack socket support for FTL Dec 26, 2017

DL6ER added 2 commits December 29, 2017 01:44

Merge branch 'development' into new/IPv6support

ddc305d

Merge branch 'development' into new/IPv6support

13b2fb3

Signed-off-by: DL6ER <[email protected]> Conflicts: socket.c

AzureMarker reviewed Jan 9, 2018

View reviewed changes

DL6ER added 3 commits January 9, 2018 19:59

dualstack will always bee boolean

1254b74

Signed-off-by: DL6ER <[email protected]>

Detect if the current machine has IPv6 capable interfaces and behave …

a95f683

…accordingly (don't try to bind to a dual-stack port if the machine isn't capable to handling IPv6, e.g. in docker) Signed-off-by: DL6ER <[email protected]>

Disable debug statement

1eebc23

Signed-off-by: DL6ER <[email protected]>

DL6ER changed the title ~~Add dual-stack socket support for FTL~~ [WIP] Add dual-stack socket support for FTL Jan 9, 2018

Complete re-write of this feature. We create now two sockets (one for…

c6b23ff

… IPv4-only and one for IPv6-only) to be able to bind to the corresponding loopback interfaces. We listen independently, but handle incoming requests using the same code. Signed-off-by: DL6ER <[email protected]>

DL6ER changed the title ~~[WIP] Add dual-stack socket support for FTL~~ Add dual-stack socket support for FTL Jan 10, 2018

AzureMarker reviewed Jan 10, 2018

View reviewed changes

DL6ER added 2 commits January 11, 2018 21:01

Incorporate review comments

51b645a

Signed-off-by: DL6ER <[email protected]>

Remove obsolete comment

5df4ea3

Signed-off-by: DL6ER <[email protected]>

AzureMarker approved these changes Jan 11, 2018

View reviewed changes

DL6ER merged commit bb7bf43 into development Jan 12, 2018

DL6ER deleted the new/IPv6support branch January 12, 2018 18:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add dual-stack socket support for FTL #180

Add dual-stack socket support for FTL #180

DL6ER commented Dec 26, 2017 •

edited

Loading

DL6ER commented Dec 26, 2017

DL6ER commented Jan 9, 2018

AzureMarker Jan 9, 2018

AzureMarker Jan 9, 2018

DL6ER Jan 9, 2018

DL6ER commented Jan 9, 2018

DL6ER commented Jan 10, 2018

AzureMarker Jan 10, 2018

AzureMarker Jan 10, 2018

AzureMarker Jan 10, 2018

AzureMarker Jan 10, 2018

Add dual-stack socket support for FTL #180

Add dual-stack socket support for FTL #180

Conversation

DL6ER commented Dec 26, 2017 • edited Loading

10

DL6ER commented Dec 26, 2017

DL6ER commented Jan 9, 2018

AzureMarker Jan 9, 2018

Choose a reason for hiding this comment

AzureMarker Jan 9, 2018

Choose a reason for hiding this comment

DL6ER Jan 9, 2018

Choose a reason for hiding this comment

DL6ER commented Jan 9, 2018

DL6ER commented Jan 10, 2018

AzureMarker Jan 10, 2018

Choose a reason for hiding this comment

AzureMarker Jan 10, 2018

Choose a reason for hiding this comment

AzureMarker Jan 10, 2018

Choose a reason for hiding this comment

AzureMarker Jan 10, 2018

Choose a reason for hiding this comment

DL6ER commented Dec 26, 2017 •

edited

Loading