Send connman mailing list submissions to
connman@lists.01.org
To subscribe or unsubscribe via email, send a message with subject or
body 'help' to
connman-requ...@lists.01.org
You can reach the person managing the list at
connman-ow...@lists.01.org
When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."
Today's Topics:
1. RE: [PATCH] service: Prevent auto connection during passphrase request
(VAUTRIN Emmanuel (Canal Plus Prestataire))
2. Re: [RFC 0/8] Prevent IPv4 VPN data/DNS leak to IPv6 network(s)
(Jussi Laakkonen)
3. Re: [RFC 1/8] ipconfig: Add disabling of IPv6, support method restore and
refactor
(Jussi Laakkonen)
----------------------------------------------------------------------
Date: Mon, 29 Mar 2021 07:30:50 +0000
From: "VAUTRIN Emmanuel (Canal Plus Prestataire)"
<emmanuel.vaut...@cpexterne.org>
Subject: RE: [PATCH] service: Prevent auto connection during
passphrase request
To: Daniel Wagner <w...@monom.org>
Cc: "connman@lists.01.org" <connman@lists.01.org>
Message-ID: <pr1pr02mb47943b7ab41723c0f857e11a93...@pr1pr02mb4794.eur
prd02.prod.outlook.com>
Content-Type: text/plain; charset="Windows-1252"
Hi Daniel,
Good to read from you again, and thank you for the other patches integration.
> src/service.c: In function ‘__connman_service_connect’:
> src/service.c:6723:31: warning: passing argument 3 of ‘g_hash_table_replace’
> makes pointer from integer without a cast [-Wint-conversion]
> 6723 | GINT_TO_POINTER(index), true);
> | ^~~~
> | |
In fact, I have copied the similar code from wispr_portal_browser_reply_cb.
> I think you could just insert the service instead of the bool
> and use g_hash_table_remove() when done instead the bool. This would
> also release the hash table resources.
The aim is to avoid a new connection on the same interface, whichever
the service is, so the index, representative of the interface, shall be used.
Best regards,
Emmanuel
------------------------------
Date: Mon, 29 Mar 2021 12:29:50 +0300
From: Jussi Laakkonen <jussi.laakko...@jolla.com>
Subject: Re: [RFC 0/8] Prevent IPv4 VPN data/DNS leak to IPv6
network(s)
To: Daniel Wagner <w...@monom.org>
Cc: connman@lists.01.org
Message-ID: <bee43a21-f168-df01-2be0-ef9917923...@jolla.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Daniel,
Thanks for the comments!
On 3/27/21 4:00 PM, Daniel Wagner wrote:
> Hi Jussi,
>
> On Fri, Mar 26, 2021 at 05:58:17PM +0200, Jussi Laakkonen wrote:
>> Therefore, this simply does disable IPv6 on all connected services when the
>> IPv4 VPN is connected with the help of changes done to ipconfig.c. Setting
>> the
>> internal value to block IPv6 use within ConnMan is utilized only when there
>> can
>> be multiple connected technologies as otherwise the transport will get
>> replaced
>> and VPN is re-connected using the new transport.
>
> The current IPv6 support is pretty strange anyway. We rely on the kernel
> to do slaac & co and don't manage the IPv6 routing table. I started with
> an attempt to fix this but I got lost in the implementation. At this
> point I think it makes more sense to start over with IPv6. Disabling the
> kernel support for IPv6 on pure IPv4 connection makes sense.
>
Yeah we have been noticing some oddities here and there. What were your
plans exactly? We noticed some issue with DUID which we have as an WIP
solution that should be just tested more. In general it works but there
may be corner cases with different setups, though. I'm glad we're on the
same page here, that disabling IPv6 for the IPv4 VPN will not be an issue :)
>> If multiple connected technologies is enabled and a new service connects then
>> in this case it cannot first establish an IPv6 connection at all but if the
>> particular service then ranks over the current service the VPN will be
>> disconnected, IPv6 is re-enabled and the new service starts to establish an
>> IPv6 connection. Otherwise the new service remains with IPv4
>> connectivity only - but if it is IPv6 only the service cannot connect and
>> this
>> is expected (might it be good to have this feature as a configurable
>> option?).
>
> Good question. It sounds a bit like a corner case scenario. Do you think
> it's likely we need to support it? If not I'd say we document it and
> leave it away for the time being. No need to make things more complex
> than necessary.
Yes that IPv6 only is a corner case. I think it only concerns direct
connection request via D-Bus, with autoconnect that should not be an
issue. But currently I don't have such connection at my disposal (my ISP
does not even provide IPv6 on vDSL in 2021!) so it is hard to test what
the real implication on enabling such connection would even be.
But anyways, if IPv6 is to be revisited/fixed in general I guess this
might be good to add to TODO list as well? And to document as well of
course.
>
>> To be future-proof it may be better to have the new services to have their
>> IPv6
>> network not enabled but forcefully disabled in case at the time of connection
>> an IPv4 VPN is connected. Not only due to the fact that an IPv6 address is
>> not
>> attempted to be retrieved with the current approach for a newly connected
>> network. Alternative would be to call
>> __connman_provider_set_ipv6_for_connected() in case the default service is an
>> IPv4 VPN to get the service list traversed. But in that case IPv6
>> connectivity
>> may have been established and there might be a small timeframe for leaking
>> traffic.
>
> Hmm, it's a valid point but I have no idea if this is just another
> corner case which makes things very complex. As I said currently ConnMan
> doesn't do IPv6 correctly, this includes the routing. It relies heavily
> on the IPv6 stack in the kernel.
With my tests using our implementation of the ofono plugin this approach
allowed mobile data to connect in the background with IPv4 only even
though it had IPv6 enabled as a dual-stack connection. So IPv6 remains
in OFF state and as the old method is not saved at VPN disconnect that
connection will get set to AUTO and IPv6 is established. Yes, a bit
complex but improves functionality as the connection does not produce an
error in network.c making the connection to fail but just sets IPv6 to
be simply off.
Well to understand how this should work was a quite complex process
itself thus, I decided to send this as an RFC first despite our internal
reviewing and testing. I changed direction so many times with this that
I lost count on how many times I did that. I did have stateful and
stateless networks with and without DHCPv6 at my disposal but all of
them were dual-stack connections.
I think one issue I forgot to raise is that should ConnMan also manage
the autoconf value when disabling/enabling IPv6? As that tells kernel
whether to assign address for the interface, effectively controlling
whether IPv6 is usable or not. With different devices there were
different behavior on this if the autoconf was not set, using the system
wide all/disable_ipv6 worked on some but not on others and not being an
expert on IPv6 (yet) I just suspected that based on incoming traffic
kernel does its things as ConnMan, as you said, does not manage all IPv6
as of now.
Cheers,
Jussi
------------------------------
Date: Mon, 29 Mar 2021 12:32:53 +0300
From: Jussi Laakkonen <jussi.laakko...@jolla.com>
Subject: Re: [RFC 1/8] ipconfig: Add disabling of IPv6, support method
restore and refactor
To: Daniel Wagner <w...@monom.org>
Cc: connman@lists.01.org
Message-ID: <77bb59e3-d60b-5288-db7d-743d0365a...@jolla.com>
Content-Type: text/plain; charset=utf-8; format=flowed
On 3/27/21 4:02 PM, Daniel Wagner wrote:
> Hi Jussi,
>
> On Fri, Mar 26, 2021 at 05:58:18PM +0200, Jussi Laakkonen wrote:
>> -static bool get_ipv6_state(gchar *ifname)
>> +#define PROC_IPV4_CONF_PREFIX "/proc/sys/net/ipv4/conf"
>> +#define PROC_IPV6_CONF_PREFIX "/proc/sys/net/ipv6/conf"
>> +
>> +static int read_conf_value(const char *prefix, const char *ifname,
>> + const char *suffix, int *value)
>
> Could you split the patch and introduce the read/write_conf_value
> changes with the new feature? This is a nice cleanup!
>
Sure, after I sent these I started to think that this is too much
content for one patch. I think that at some point cleanup could continue
to separate the /proc value handling into a separate source file as
there are other places where it is needed as well.
> I have a similar patch for this for the IPv6 work I done. If you are
> interested to look at it I can share the patches.
>
Yes I can take a look on those. I guess the code explains itself but
please feel free to elaborate more :).
Cheers,
Jussi
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list -- connman@lists.01.org
To unsubscribe send an email to connman-le...@lists.01.org
------------------------------
End of connman Digest, Vol 65, Issue 12
***************************************
