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

[connman] Add VPN support to CLAT. Fixes JB#60276 #41

Merged
merged 28 commits into from Jun 15, 2023

Conversation

LaakkonenJussi
Copy link
Contributor

This is based on the PR #39 and needs to be rebased whenever it is changed or merged.

With these changes a VPN mode to disable default route on the CLAT device and to ignore setting of double nat when tethering is made possible.

@LaakkonenJussi LaakkonenJussi changed the title WIP: Add VPN support to CLAT. JB#60276 Add VPN support to CLAT. Fixes JB#60276 Mar 13, 2023
@LaakkonenJussi LaakkonenJussi force-pushed the jb60276 branch 2 times, most recently from c0ac01a to e8a89ea Compare May 10, 2023 14:39
@LaakkonenJussi LaakkonenJussi force-pushed the jb60276 branch 6 times, most recently from 73f53af to 0d88ca3 Compare May 22, 2023 13:15
Copy link

@spiiroin spiiroin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some nits/questions, check if there is something worthwhile in there.
But I did not spot any obvious problems, so fwiw lgtm.

connman/plugins/sailfish_ofono.c Show resolved Hide resolved
connman/src/connection.c Outdated Show resolved Hide resolved
connman/src/service.c Show resolved Hide resolved
connman/plugins/clat.c Show resolved Hide resolved
connman/plugins/clat.c Outdated Show resolved Hide resolved
connman/plugins/clat.c Show resolved Hide resolved
connman/unit/test-clat.c Outdated Show resolved Hide resolved
INADDR_ANY, i.e., default routes do not get removed with the RTF_HOST
flag set. The flag must be set only for host routes. Otherwise the
INADDR_ANY route is not removed and in such case the removal of the
route will return a ok reply indicating that the route was removed.
Check IPv4 and IPv6 indexes when determining if given index is of
default service. This is a part of dual index support to faciliate the
need for having two separate interfaces for different IP families.
This implements support for dual index mode on services in route
creation. Services can have two separete interfaces, one for each IP
family and this needs to be accounted for in setting up routes and
gateways.
The service can have two separate indexes one for each IP family and
relying on the IPv6-preferring service index is not reliable. Therefore,
try IPv4 first to get an index for VPN and then IPv6.
…0276

When enabling DNS support both indexes set on the service ipconfigs.
Check also presence of VPN on both, first IPv4 and then IPv6. By doing
this dual-mode services can be supported.
…60276

Add helper function to check if given ipconfig has an IP address set and
it the particular IP address is enabled by method. Returns automatically
false if IP address is omitted and when there is an address set the
method is used to determine if the address is being used.
Add connman_ipconfig_get_gateway_from_index() as public function usable
by plugins.

Add connman_ipconfig_get_index() available for plugins.
This splits the method based ipconfig setting from the
__connman_ipconfig_set_config() to allow a separate address based
setting of the configuration instead of passing a D-Bus array of
information to it.
To allow reset of ipconfig to new or empty address the existing
functionality for reset with D-Bus message is split to allow
re-usability of code. The new function can be used by plugins and it
sets the address similarly to D-Bus message use but accepts
address/netmask/gateway etc. input. The new ipconfig is enabled after a
succesful reset in order to trigger setting of gateway and routes.
…60276

Add ipconfig notify via service for plugins with a support for
toggling the change notify via PropertyChanged D-Bus signal. Some
plugins might need more control when the setup requires first resetting
the ipconfig of a service into certain state, which may not be a
connected state, thus the change notify would not be sent. Therefore, it
should be more controlled when the plugin reaches itself a connected
state to inform upper layers that the injection of IP family ipconfig
has been completed successfully.

In case the notify has been already done it is assumed that previously
the ipconfig info isn't notified. The plugin wrapper, therefore, only
amends the functionality of __connman_service_ipconfig_indicate_state()
that does not send the notify when called. This amended functionality is
controlled by a given boolean.

Also done some cleanup.
The commit added in a single slot manual IPv4 address to use system
entry. It does not seem to be used as such anywhere else like this but
instad used along with fixed method. This blocked sending of the proper
IPv4 data in D-Bus PropertyChanged signal in cases the method was set to
manual.

commit 79e3ee2
Author: Patrik Flykt <patrik.flykt@linux.intel.com>
Date:   Fri May 24 14:38:01 2013 +0300

    ipconfig: Use system set addresses for IPv4 property in 'manual' mode

    Addresses and gateway for the service IPv4 property being used in the
    system are available from ipconfig->system when the property is set to
    'manual'.
If the ipconfig is not the same what the DHCP was initialized with it
has been changed in between the DHCP request and return. It would then
cause removal of changed IP address set by an additional, plugin created
device for dual index support.
…enabled. JB#60276

When the protocol is either IPv4 or IPv6 also set the method to OFF to
enforce the status of being off on the not enabled IP family.

When the IP family is not being enabled on DUAL mode clear also the
existing address. This has an effect when changing from DUAL to IPv6.
In case the transport of a VPN does have only IPv6 it should not be
allowed to be disabled. This is a special case that can happen with the
presence of DNS64 where all traffic is tunneled over IPv6 using a
specific plugin and, thus, transport may not have IPv4 enabled. If
IPv6 would be disabled in this case the VPN naturally would not work.

Also ignore IPv6 disable in case the interface indexes differ on transport.
This means that the IPv4 is depending on IPv6 via a plugin to transfer
traffic. In such case IPv6 should not be disabled.
…et yet. JB#60276

In some cases the gateway IP may be reported as untrusted_ip variable.
Use that if trusted_ip has not been set. This may help overcome some
setup issues.

In OpenVPN specification on untrusted_ip:
Actual IP address of connecting client or peer which has not been authenticated yet.
Sometimes used to nmap the connecting host in a --tls-verify script to ensure it is
firewalled properly.
…0276

Add a VPN mode to CLAT by injecting the IPv4 address into the tracked
service's ipconfig and ipaddress. This way the use of CLAT within
ConnMan is more transparent. Routing is mostly handled by ConnMan core
with this change and the host routes set for VPN is handled as in case
when VPN connects over service without the need for CLAT. The ipconfig
is being reset to the CLAT address on the tracked service via service.c's
address reset, which enables the ipconfig for connection.c to use for
routing and gateway setting.

When in VPN mode and in case tethering is or was previously enabled
double nat will not be set as it is not required. Thus, tethering is
done solely over VPN in such scenario.

Also when returning from VPN mode and default changes back to transport
the IPv4 address may have been set by CLAT. Check that and continue if
that is the case.

In VPN mode the VPN service is also tracked via the transport which is
the tracked service. VPN is disconnected by the plugin only when CLAT is
enabled and the VPN tracking is being unset (cleared).

Change to use ipconfig.c IP address check. Plugins should not need to do
this extra. Also do not react to ipconfig change set by CLAT as this is
the side effect of injecting IPv4 of CLAT to the tracked service. The
dual index mode allows to separate interfaces within a service and in
this case the ipconfig is checked to have the same index as the CLAT
device and to ignore the change in such case. Ipconfig changes are
checked always when the state is any of the run states.

Tracked service management is also improved by clearing the IP address
when CLAT is stopped. This makes it a smooth transition to a network
where IPv4 support is also enabled. Additionally The default service can
be sometimes NULL because of reasons. It is not necessarily requiring
to stop CLAT. Therefore, if the tracked service is still running and
CLAT is running ignore the NULL default service.

Use the proper nat.h path for including it. The header is now included
in the Makefile.am properly.
Make clat to change the ipconfig state according to CLAT internal state.
All of the states cannot be notified because of how ConnMan does combine
them. Association/configuration/disconnect will be dominant and that
would make cellular disconnect/change state as well. That is not
desired. State combiner would require more work in this case. As of now
the state changer is not checking the state changes but might require
work in the future.
When VPN is set to be the default service and it runs over CLAT (VPN
mode on) the periodic prefix query will fail because the DNS servers of
the VPN are only used. There is no DNS serving the request for CLAT
prefix, nor there is a route to the DNS server and, therefore, it is
better to simply drop the prefix query during the VPN is connected.

Enabling a DNS server of a transport would result in leaky DNS when VPN
is on because it would return, for example, an AAAA (IPv6) record for a
request and when VPN is IPv4 only the traffic would also bypass the VPN
as there is an active IPv6 connection, and, thus, the traffic would also
bypass CLAT and use the mobile data directly. That is reason enough for
using the approach in this commit because of simplicity and non-breaking
+ non-leaking.
… JB#60276

This change extends the VPN mode by dropping the default IPv6 gateway and
adding a CLAT prefix network route via the IPv6 gateway when VPN goes
over CLAT. This is to route only the CLAT prefixed traffic over IPv6 in
VPN mode.

The default IPv4 route for CLAT is managed by ConnMan core as the
ipconfig info is injected to the tracked service. Therefore, the adding
and removing of the IPv4 default route is disabled by default and can be
managed by adding a boolean to /etc/connman/clat.conf in group CLAT
(in square brackets, omitted here because of tag usage)

ClatDeviceSetDefaultRoute=true|false
This adds VPN as well as tethering tests for verifying that when
cellular/wlan/ethernet is used a transport the cases are handled
accordingly. Tethering is also tested with VPN.

The tracking of added routes is improved for the VPN and tethering
needs in the unit test stubs. As a result of injecting CLAT IPv4 into
tracked service the default route setting is disabled by default and is,
thus, disabled in the tests.

All services have IPv4 ipconfig as they are created. This is now
accounted for in the unit test as well. This required separation of the
ipaddress initialization in tests to be more obvious and flexible.

The default service may be NULL in some cases and as the plugin handles
it correctly now so are the relevant tests changed to do so.

The pausing of the prefix query when VPN is enabled is handled as well
by the test.
The index of the device may be changed when the connctx settings change
and, thus, it is imperative to update the network index in that case to
ensure consistency. It is known that ofono may change the interface for
each connection attempt.
The index of the tracked service may change between each mobile data
connect attempt. This change ensures that the index of the tracked
service will get updated in each possible scenario.
@LaakkonenJussi LaakkonenJussi changed the title Add VPN support to CLAT. Fixes JB#60276 [connman] Add VPN support to CLAT. Fixes JB#60276 Jun 15, 2023
@LaakkonenJussi LaakkonenJussi merged commit f0e59eb into sailfishos:master Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants