Send connman mailing list submissions to
[email protected]
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.01.org/mailman/listinfo/connman
or, via email, send a message with subject or body 'help' to
[email protected]
You can reach the person managing the list at
[email protected]
When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."
Today's Topics:
1. Re: Session policy with multiple AllowedBearers (Daryl Nebrich)
2. Re: [RFC] Proposal for ConnMan session API extensions
(Daniel Wagner)
----------------------------------------------------------------------
Message: 1
Date: Sun, 22 Jan 2017 14:33:20 -0500
From: Daryl Nebrich <[email protected]>
To: Daniel Wagner <[email protected]>
Cc: [email protected]
Subject: Re: Session policy with multiple AllowedBearers
Message-ID:
<CANmGHYuAwWg=vr_4kpj=8kfqccjsjn2hvpvvovhhkxitp72...@mail.gmail.com>
Content-Type: text/plain; charset="utf-8"
Hi Daniel,
Thanks for response. First post on the list. I'll try to reply inline
using gmail so I'm not top posting, hopefully comes out ok.
On Sun, Jan 22, 2017 at 1:46 PM, Daniel Wagner <[email protected]> wrote:
> Hi Daryl
>
> On 01/20/2017 07:45 PM, Daryl Nebrich wrote:
> > Hi,
> >
> > I'm running version 1.33 with the local policy plugin enabled. I have 3
> > policies setup, first with AllowedBearers set to "wifi", second with
> > "cellular" and the third with "wifi cellular", each set to a unique
> > uid. The session-api.txt doc states that the order is important as it
> > sets priority. The issue I'm seeing is with the 3rd policy having both
> > wifi and cellular. I have a python script creating the sessions. The
> > nat and mangle tables are setup right and I've verified the routing
> > table using ip route show table 0x..
> >
> > Both wifi and cellular services are up. The issue is that the session
> > using policy 3 will sometimes initially create a default route for
> > wifi, sometimes cellular.
>
> The order of connection up event of the services is probably the source
> of the problem. That is if wifi gets up first that one will the default
> route. And if cellular is first, that's the default route. If that is
> the case there is bug in the session code.
>
>
>
> static void handle_service_state_online(struct connman_service *service,
> enum connman_service_state state,
> struct connman_service_info *info)
> {
> GHashTableIter iter;
> gpointer key, value;
>
> g_hash_table_iter_init(&iter, session_hash);
> while (g_hash_table_iter_next(&iter, &key, &value)) {
> struct connman_session *session = value;
> bool connected;
>
> connected = is_session_connected(session, state);
>
> if (session->service == service) {
> if (!connected) {
> DBG("session %p remove service %p",
> session, service);
> info->sessions =
> g_slist_remove(info->sessions,
> session);
> session->service = NULL;
> update_session_state(session);
> }
> } else if (connected && session_match_service(session,
> service)) {
> DBG("session %p add service %p", session, service);
>
> info->sessions = g_slist_prepend(info->sessions,
> session);
> session->service = service; //
> here is the problem I think
> update_session_state(session);
> }
> }
> }
>
> That should probably not just assigning the new service We need to check
> the AllowedBearers too.
>
> Can you try this here (only compile tested)
>
> diff --git a/src/session.c b/src/session.c
> index b1e9e1b..fd4be2b 100644
> --- a/src/session.c
> +++ b/src/session.c
> @@ -721,6 +721,25 @@ static void apply_policy_on_bearers(GSList
> *policy_bearers, GSList *bearers,
> }
> }
>
> +static unsigned int position_in_bearers(GSList *policy_bearers,
> + struct connman_service *service)
> +{
> + enum connman_service_type bearer;
> + GSList *it;
> + unsigned int pos = 0;
> +
> + for (it = policy_bearers; it; it = it->next) {
> + bearer = GPOINTER_TO_INT(it->data);
> +
> + if (bearer == connman_service_get_type(service))
> + return pos;
> +
> + pos++;
> + }
> +
> + return pos;
> +}
> +
> const char *connman_session_get_owner(struct connman_session *session)
> {
> return session->owner;
> @@ -1664,6 +1683,21 @@ static void handle_service_state_online(struct
> connman_service *service,
>
> info->sessions = g_slist_prepend(info->sessions,
> session);
> +
> + if (session->service) {
> + unsigned int pos_old;
> + unsigned int pos_new;
> +
> + pos_old = position_in_bearers(
> + session->info->config.allowed_
> bearers,
> + session->service);
> + pos_new = position_in_bearers(
> + session->info->config.allowed_
> bearers,
> + service);
> +
> + if (pos_old < pos_new)
> + return;
> + }
> session->service = service;
> update_session_state(session);
> }
>
>
I made a similar change and got it going earlier today. I ended up making
a change inside session_match_service() which is called from
handle_service_state_online().
@@ -1556,14 +1559,27 @@
enum connman_service_type bearer_type;
enum connman_service_type service_type;
GSList *list;
+ bool current_service_connected;
+ enum connman_service_type current_service_type;
if (policy && policy->allowed)
return policy->allowed(session, service);
+ // Handle case where session policy has multiple AllowedBearers.
+ // Check if current service in use is connected.
+ current_service_connected =
__connman_service_is_connected_state(session->service,
CONNMAN_IPCONFIG_TYPE_IPV4);
+ current_service_type = connman_service_get_type(session->service);
+
for (list = session->info->config.allowed_bearers; list; list =
list->next) {
bearer_type = GPOINTER_TO_INT(list->data);
service_type = connman_service_get_type(service);
+ // Keep using the current service if found first (higher priority)
+ if (current_service_connected && (bearer_type ==
current_service_type)) {
+ DBG("Keeping higher priority %s over %s",
__connman_service_type2string(current_service_type),
__connman_service_type2string(service_type));
+ return false;
+ }
+
if (bearer_type == service_type)
return true;
}
But for the above to work, I also had to change the order of looping in
apply_policy_on_bearers(). At least in 1.33, the func loops through
available bearers first, and then policy bearers. Which means the order of
bearers in the policy file may not be preserved. For me, if I put
"cellular wifi" in the policy file, I would get "wifi cellular" as the
order in allowed_bearers. I think that change is still needed with your
approach above.
> I would expect it to always be wifi.
>
> Me too.
>
> > If the
> > route is for cellular, and I manually bring down cellular, the session
> > will not revert to the wifi service (the route table stays empty).
>
> That is another bug I guess. Can you post the debug output part of this?
> This is supposed to work. BTW use the current HEAD for this. 1.33 is
> pretty old :)
>
>
I fixed this by adding the below to handle_service_state_offline(). Not
sure if this is the right way but it tested ok for me.
@@ -1695,6 +1711,9 @@
session->service = NULL;
update_session_state(session);
+
+ DBG("Try to find another service");
+ session_activate(session);
}
}
> > Shouldn't it revert? The logs show handle_service_state_offline()
> > setting the session->service to null when cellular goes away. And after
> > that I'll get a "type wifi busy" log from the auto_connect_service()
> > function in service.c. I see code in session.c to process the policy
> > bearers and setup allowed_bearers, but I'm not clear on where/how it's
> used.
>
> Sorry about that messy code. It was quite difficult to get it initially
> working
> but it is for sure not really great code. If you find a way to simplify
> I am more than happy to take those patches.
>
>
Sorry, I wasn't suggesting that. I just hadn't spent the time to review it
enough. Things are working well now with the changes above.
> I've reviewed the latest commits and see the change to nftables. But I
> > didn't see any code changes in session.c or session_local_policy.c that
> > might address this. Although I do see changes in service.c.
>
> Best thing for now is to use HEAD and port patches back if you need to
> stay at
> 1.33.
>
> > I'm
> > building for an ARM (i.MX6) target using Yocto. And the NXP repos are
> > setup to use connman 1.30. I've updated to 1.33 and ported the
> > patches. I can move to the tip if it would help.
>
> Sure. I'd like to see those kind of bugs gone. If you can provide some
> logs I can try to figure out what is going wrong.
>
> Thanks,
> Daniel
>
I'll back out my session_activate() change in
handle_service_state_offline() and reply later with a log. I'm a bit
hesitant to move to the tip now that I've got it going with iptables. Do
you know what the plans are for a 1.34 release? The bitbake recipe is
pulling down the code from kernel.org/pub/linux/network/connman.
Thanks,
Daryl
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.01.org/pipermail/connman/attachments/20170122/93c7d8bf/attachment-0001.html>
------------------------------
Message: 2
Date: Sun, 22 Jan 2017 20:47:43 +0100
From: Daniel Wagner <[email protected]>
To: "Nakamura, Yusuke (ADITJ/SWG)" <[email protected]>
Cc: "[email protected]" <[email protected]>, "Hoyer, Marko
(ADITG/SW2)" <[email protected]>, "[email protected]"
<[email protected]>, "EXTERNAL Thorwirth Bjoern (Brunel,
CM-CI1/ERN5-E)" <[email protected]>, "Ishikawa,
Tetsuri (ADITJ/SWG)" <[email protected]>, Lukasz Nowak
<[email protected]>
Subject: Re: [RFC] Proposal for ConnMan session API extensions
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252; format=flowed
Hi Yusuke,
On 01/16/2017 02:00 PM, Nakamura, Yusuke (ADITJ/SWG) wrote:
> Hello everybody
First, thanks a lot for the detailed proposal. This is really helping
understanding what you are trying to achieve.
> We would like to propose two extensions to the session API interface of
> ConnMan. The first extension enables a service differentiation for
> processes run by the same user.It allows ConnMan to differentiate
> between bearer usage permissions and the respective priorities based on
> the requested service type.
That should also be possible with the current code. You might need to
write your own session plugin for this though. It is quite hard to
support all types of application/user identification out of the box
because it depends heavily on the system integrator's choices.
Let me explain this in the section below.
> The second extension enables session-based
> routing based on source IPs. This mechanism is meant as an alternative
> means for differentiating between traffic of session API users. Source
> IP based routing may be used to route traffic from non-local sources,
> e.g. remote hosts or virtual machines/containers.
If I read this correctly than this could be part of Lukasz Nowak
patches:
[PATCH v2 0/7] session: add per-interface routing
At least it sets the foundation for it. Again, I'll comment in the
section below.
> 1 User service differentiating session API
>
>
>
> 1.1 State of art
>
> A minimal policy configuration in the session API takes the form
>
>
>
> [<policy_name>]
>
> uid=<system user ID>
>
> AllowedBearers=[<bearer>]*+**, *
>
>
>
> where a calling process that implements the session API is identified by
> the user ID it is run as. All processes of the same user share the same
> list of allowed bearers, and the same priority for choosing between
> available bearers is applied.
The session_policy_local.c plugin implements three types of identification.
uid
gid
lsm
The last one is of interest. It allows to identify your application
using the Linux Security Module. If you system used for example SELinux
you can identify your application using the SELinux label.
Unfortunately, as soon you use something else, let's say AppAmor or any
other LSM, you need to patch D-Bus for this.
> 1.2 Advantages of proposed extension
>
> Our extension allows processes to select a service context for which the
> routing decision is made. Several use cases are enabled through the
> extension:
>
> (1) An application that implements an interactive webservice, e.g. a
> Facebook agent, can be configured to use different bearers than a
> background process running a regular system update mechanism as the same
> user.
>
> (2) An application that runs different encapsulated services, e.g. a
> webbrowser executing a Facebook webservice, a device manufacturer?s
> portal site, and an automatic update mechanism, may be configured to use
> different bearers based on the currently active service.
Okay, those are the exact use cases for which the Session API was
initially added to ConnMan.
> 1.3 Implementation details
>
> The configuration for the extension is lightweight
>
>
>
> [<policy_name>]
>
> uid=<system user ID>**
>
> service=<service ID>**
>
> AllowedBearers=[<bearer>]*+**, *
>
>
>
>
>
> where the new parameter service defaults to * (any) if unset. If no
> default rule exists and the intended service is unknown or not set by
> the calling process, the session API denies routing.
>
> In order to indicate the intended service, the calling process leverages
> the option to set parameters during the /CreateSession(dict settings,
> object notifier) /method call in the ConnMan Manager API on session
> establishment, see Figure 1. We propose to introduce a new parameter
> service for this purpose.
>
>
>
> cid:[email protected]
>
> Optionally, this parameter may be changed by a /Update(dict settings)
> /request during the session lifetime.
When we were discussing the initial API design, the exact same proposal
for application identification was on the table. Marcel convinced me
that this is a rather bad idea. Mainly because you need to trust the
application to provide the correct ID. There is nothing which stops any
random application to set the magic ID to get full connectivity.
So the trick is to rely on a trustworthy 3rd party, in our case LSM.
What type of LSM are you going to use?
> 2 Source IP based routing
>
> 2.1 State of art
>
> The session API makes the decision which bearer shall be used by a
> requesting user. It then initiates the following steps
>
>
>
> 1. It generates a rule that unambiguously attaches a mark <MARK>
> to every packet originating from the user <UID> (resp. a GID) that
> passes through the kernel.
>
>
>
> iptables -t mangle -A OUTPUT -m owner --uid-owner <UID> -j MARK
> --set-mark <MARK>
>
>
>
> 2. It generates a new routing table by the name of <MARK> which is
> applied for all packets matching the mark value <MARK>
>
>
>
> ip rule add fwmark <MARK> table <MARK>
>
>
>
> 3. It adds a default route to the newly created routing table matching
> the intended bearer <BEARER>.
>
>
>
> ip route add default via XXX dev <BEARER> table <MARK>
>
>
>
> UID/GID based differentiation is only applicable for traffic generated
> at the local system.
>
> * *
>
> 2.2 Advantages of proposed extension
>
> Our extension allows the session API based to route alternatively based
> on the source IP of a packet. Several use cases are enabled through the
> extension:
>
>
>
> (1) The host implementing the ConnMan session API may act as a service
> aware router to an entire network. For example, different hosts in the
> network may use different outgoing network connections.
>
>
>
> (2) If the host implements virtual machines/containers that are
> connected through a Linux bridge, the session API can route their
> traffic according to the intended policies.
>
>
>
> In order to enable these scenarios, hosts/virtual machines/containers
> require authenticated access to the system DBus. Alternatively, using
> the user service differentiation proposed above, an application running
> on the host (e.g. a webportal), may enable routing on behalf of the
> remote machine.
That is basically lifting the limitation of application only being
handled by ConnMan. Sounds good to me :)
> 2.3 Implementation details
>
> The configuration for the extension is lightweight
>
>
>
> [<policy_name>]
>
> uid=<system user ID>**
>
> SourceIP=[<IP>]*+ *
>
> AllowedBearers=[<bearer>]*+**, *
>
>
>
> where the new parameter SourceIP carries one or more IP addresses. If
> the parameter is omitted, UID/GID-based routing is applied (backward
> compatible). If a single IP is given, routing for all bearers uses this
> IP address for differentiation, otherwise the number of IPs must match
> the number of bearers. If the nth bearer from the list of allowed
> bearers is selected, the nth IP is used.
I do not really follow the argumentation on the numbers of IP addresses
and bearers. Are you trying to match several 'virtual machines' with one
session? A Session is thought to be only used by one entity, until now
that would a single application.
> In the first step of the route establishment, the iptables rule is
> changed to
>
> iptables -A PREROUTING -t mangle -j MARK --set-mark <MARK>
>
>
> if source IP based routing is to be used. Note that the rule is added to
> a different chain than the original user-based routing.
>
>
> What do you think about our proposal?
Thanks again for providing this detailed description. I really
appreciate it.
For 1) we should have most of it in place, except bugs obviously.
Proposal 2) could be part of Lukasz' effort.
@Lukasz do you think proposal 2) could also be addressed by your
patches? It sounds like a generalization of your patch set.
Thanks,
Daniel
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 15, Issue 22
***************************************
