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: [PATCH 4/5] vpn: Run VPN plugin binaries with
configurable privileges (Daniel Wagner)
2. Re: [PATCH 5/5] man: Document VPN config file privilege
configuration in connman-vpn.conf (Daniel Wagner)
3. Re: Problem connecting to WISPr access point (Daniel Wagner)
4. Re: [PATCH 2/5] vpn: Add support for configurable user and
groups in settings (Jussi Laakkonen)
5. Re: [PATCH 3/5] task: Add support for a custom setup function
(Jussi Laakkonen)
6. Re: [PATCH 4/5] vpn: Run VPN plugin binaries with
configurable privileges (Jussi Laakkonen)
7. Re: [PATCH 5/5] man: Document VPN config file privilege
configuration in connman-vpn.conf (Jussi Laakkonen)
8. Re: [PATCH 2/5] vpn: Add support for configurable user and
groups in settings (Daniel Wagner)
----------------------------------------------------------------------
Message: 1
Date: Thu, 2 May 2019 08:31:42 +0200
From: Daniel Wagner <[email protected]>
To: Jussi Laakkonen <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH 4/5] vpn: Run VPN plugin binaries with
configurable privileges
Message-ID: <20190502063142.dh7zxz2lk7224dma@beryllium>
Content-Type: text/plain; charset=us-ascii
Hi Jussi,
On Mon, Apr 29, 2019 at 06:40:01PM +0300, Jussi Laakkonen wrote:
> This commit adds support for running VPN plugin binaries with
> configurable privileges. Function vpn_task_setup() is registered as a
> custom setup callback function into task.c and is called when the
> task is run and task_setup() is called.
>
> vpn_task_setup() retrieves user, group and supplementary groups for the
> plugin at vpn_register() in order to do it once and frees the plugin
> data at vpn_unregister(). The plugin data is stored by vpn-settings.c
> and is retrieved when VPN plugin is to be connected.
>
> Function vpn_task_setup() sets the permissions when VPN plugin's task is
> started. Use of setgid(), setgroups() and setuid() needs CAP_SETGID and
> CAP_SETUID capabilities.
>
> NOTE/TODO: It was observed that this worked correctly for external
> plugins but same configuration was not successful with builtin plugins.
That is odd. Any chance to look into this? I fear this will fire back
if not fixed in mainline soon after the commit.
Thanks,
Daniel
------------------------------
Message: 2
Date: Thu, 2 May 2019 08:41:59 +0200
From: Daniel Wagner <[email protected]>
To: Jussi Laakkonen <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH 5/5] man: Document VPN config file privilege
configuration in connman-vpn.conf
Message-ID: <20190502064159.5dxwmtscdmk2wtxk@beryllium>
Content-Type: text/plain; charset=us-ascii
Hi Jussi,
On Mon, Apr 29, 2019 at 06:40:02PM +0300, Jussi Laakkonen wrote:
> Added documentation of the privilege configuration to manual pages.
> Detailed both connman-vpn.conf and VPN plugin specific configuration.
> ---
> doc/connman-vpn.conf.5.in | 34 ++++++++++++++++++++++++++++++++--
> 1 file changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/doc/connman-vpn.conf.5.in b/doc/connman-vpn.conf.5.in
> index 20d30fcc..1b73d6b1 100644
> --- a/doc/connman-vpn.conf.5.in
> +++ b/doc/connman-vpn.conf.5.in
> @@ -14,6 +14,15 @@ is a configuration file for ConnMan-VPN. The configuration
> file is
> optional but it can be used to set up various aspects of ConnMan-VPN's
> behavior. The location of the file may be changed through use of
> the \fB\-\-config= \fRargument for \fBconnman-vpn\fP(8).
> +.P
> +This configuration can be also used to change the user, group and
> supplementary
> +groups to be used when running VPN connections. Configuration in
Not that I am a good documentation writer, but maybe 'Access
permission can be controlled by ...' would be better start into this
paragraph.
Apart of my nitpicking, the series looks good.
One thing I haven't understood so far, is why is the VPN API so
different to the ConnMan API that we need to handle the access control
directly. I must miss something. I though with polkit or even D-Bus
config files you can do this kind of things.
Thanks,
Daniel
------------------------------
Message: 3
Date: Thu, 2 May 2019 08:46:11 +0200
From: Daniel Wagner <[email protected]>
To: Thomas Green <[email protected]>
Cc: "[email protected]" <[email protected]>
Subject: Re: Problem connecting to WISPr access point
Message-ID: <20190502064611.jbyasplukambpvon@beryllium>
Content-Type: text/plain; charset=us-ascii
Hi Thomas,
> it never prompts for the wispr credentials. Turning on logging I
> see that the access point returns a 307 error when trying to attach.
> In wispr.c (line 735) it only checks for error 302. I added a case
> to that for 307 to handle the redirect, and tried it again. As you
> can see from the attached log, it now tries to handle the redirect,
> but fails to do so. As you can see in the log, it immediately
> returns a 400 (Bad Request) error. Trying to determine what is
> happening I then took a tcpdump of the connection process to see
> what happened then. When examining the pcap file that is attached,
> It doesn't look as if the attempt to actually perform the redirect
> actually happens. If you could help me determine what is happening,
> and fix this, I would surely appreciate it. Connecting to this
> access point works as expected on my android and apple devices, so
> I'm guessing that connman is doing something different.
>From your description I agree :)
I haven't had time to look at this a bit more closely. I try to find
some time to look into this. It's just a time management problem on my
side.
Thanks,
Daniel
------------------------------
Message: 4
Date: Thu, 2 May 2019 10:41:18 +0300
From: Jussi Laakkonen <[email protected]>
To: Daniel Wagner <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH 2/5] vpn: Add support for configurable user and
groups in settings
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Daniel,
On 5/2/19 9:28 AM, Daniel Wagner wrote:
> Hi Jussi,
>
> On Mon, Apr 29, 2019 at 06:39:59PM +0300, Jussi Laakkonen wrote:
>> This commit adds support for configurable user, group and supplementary
>> groups for VPNs. Common configuration is loaded from the main VPN config
>> file CONFIGDIR/connman-vpn.conf, which applies to all VPNs. This commit
>> also adds support for VPN specific configuration in
>> CONFIGDIR/vpn-plugin/ based on the plugin name + .conf suffix.
>>
>> Both configs follow the name format:
>> - User = user to use for running VPN binary (username or uid)
>> - Group = the main group to use for running VPN binary (groupname or
>> gid)
>> - SupplementaryGroups = supplementary groups used (separator: comma,
>> groupnames or gids)
>>
>> An example config is:
>> [VPNBinary]
>
> So if you want to do a group, it tell what it is about, maybe
> Permission/AccessControl or something along this. I wouldn't mind to
> have the User/Group/SupmentaryGroups in the general section
> either. VPNBinary seems not really fitting here.
>
The naming for the group was not perfect, I agree. I think these could
be in own group to have a clear separation of the configuration purposes.
I'll think renaming VPNBinary to Privileges as it would be the most
descriptive (or DACPrivileges, or DAC_Privileges), since this is solely
about changing the privileges of the binary ran by connman-vpnd when
connecting a VPN plugin. What do you say Daniel?
> Thanks,
> Daniel
>
Cheers,
Jussi
------------------------------
Message: 5
Date: Thu, 2 May 2019 10:41:22 +0300
From: Jussi Laakkonen <[email protected]>
To: Daniel Wagner <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH 3/5] task: Add support for a custom setup function
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Daniel,
On 5/2/19 9:29 AM, Daniel Wagner wrote:
> Hi Jussi,
>
>> index 953cc409..2583f484 100644
>> --- a/src/task.c
>> +++ b/src/task.c
>> @@ -45,8 +45,10 @@ struct connman_task {
>> GPtrArray *argv;
>> GPtrArray *envp;
>> connman_task_exit_t exit_func;
>> + connman_task_setup_t custom_setup_func;
>
> Nitpick of the day: just called it setup_func. That matches the exit_func.
>
That sounds like a good logical nitpick of the day. I will change this
and send updated patch(es) later today.
> Thanks,
> Daniel
>
Cheers,
Jussi
------------------------------
Message: 6
Date: Thu, 2 May 2019 10:41:27 +0300
From: Jussi Laakkonen <[email protected]>
To: Daniel Wagner <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH 4/5] vpn: Run VPN plugin binaries with
configurable privileges
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Daniel,
On 5/2/19 9:31 AM, Daniel Wagner wrote:
> Hi Jussi,
>
> On Mon, Apr 29, 2019 at 06:40:01PM +0300, Jussi Laakkonen wrote:
>> This commit adds support for running VPN plugin binaries with
>> configurable privileges. Function vpn_task_setup() is registered as a
>> custom setup callback function into task.c and is called when the
>> task is run and task_setup() is called.
>>
>> vpn_task_setup() retrieves user, group and supplementary groups for the
>> plugin at vpn_register() in order to do it once and frees the plugin
>> data at vpn_unregister(). The plugin data is stored by vpn-settings.c
>> and is retrieved when VPN plugin is to be connected.
>>
>> Function vpn_task_setup() sets the permissions when VPN plugin's task is
>> started. Use of setgid(), setgroups() and setuid() needs CAP_SETGID and
>> CAP_SETUID capabilities.
>>
>> NOTE/TODO: It was observed that this worked correctly for external
>> plugins but same configuration was not successful with builtin plugins.
>
> That is odd. Any chance to look into this? I fear this will fire back
> if not fixed in mainline soon after the commit.
>
I did some testing after sending these patches and it turned out to be
some recent change in our system that just required another
supplementary group for VPNs to function. So this comment is invalid, I
got it working with builtin and external plugins. Our permission system
is a bit different from the rest.
I'll update the commit message and send updated patch(es) later today.
> Thanks,
> Daniel
>
Cheers,
Jussi
------------------------------
Message: 7
Date: Thu, 2 May 2019 10:41:33 +0300
From: Jussi Laakkonen <[email protected]>
To: Daniel Wagner <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH 5/5] man: Document VPN config file privilege
configuration in connman-vpn.conf
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Daniel,
On 5/2/19 9:41 AM, Daniel Wagner wrote:
> Hi Jussi,
>
> On Mon, Apr 29, 2019 at 06:40:02PM +0300, Jussi Laakkonen wrote:
>> Added documentation of the privilege configuration to manual pages.
>> Detailed both connman-vpn.conf and VPN plugin specific configuration.
>> ---
>> doc/connman-vpn.conf.5.in | 34 ++++++++++++++++++++++++++++++++--
>> 1 file changed, 32 insertions(+), 2 deletions(-)
>>
>> diff --git a/doc/connman-vpn.conf.5.in b/doc/connman-vpn.conf.5.in
>> index 20d30fcc..1b73d6b1 100644
>> --- a/doc/connman-vpn.conf.5.in
>> +++ b/doc/connman-vpn.conf.5.in
>> @@ -14,6 +14,15 @@ is a configuration file for ConnMan-VPN. The
>> configuration file is
>> optional but it can be used to set up various aspects of ConnMan-VPN's
>> behavior. The location of the file may be changed through use of
>> the \fB\-\-config= \fRargument for \fBconnman-vpn\fP(8).
>> +.P
>> +This configuration can be also used to change the user, group and
>> supplementary
>> +groups to be used when running VPN connections. Configuration in
>
> Not that I am a good documentation writer, but maybe 'Access
> permission can be controlled by ...' would be better start into this
> paragraph.
>
I think your suggestion is good. I'll rephrase this part.
> Apart of my nitpicking, the series looks good.
>
Thanks!
> One thing I haven't understood so far, is why is the VPN API so
> different to the ConnMan API that we need to handle the access control
> directly. I must miss something. I though with polkit or even D-Bus
> config files you can do this kind of things.
>
Hm, maybe the other parts need a bit of clarifying then. This is not
about providing access to anything but enabling connman-vpnd to run the
VPN binaries with different DAC permissions.
For example, in our case, connmand and connman-vpnd are run as root, so
the VPN binaries (openvpn) are run with same privileges. This series
allows to change the privileges to something else, as there are cases
where it is desired to have the VPN running with less privileges as the
connman-vpnd. The openvpn run by OpenVPN plugin does not have to have
access to everything that connman-vpnd/root has.
I'll update the series + descriptions and send them later today.
> Thanks,
> Daniel
>
Cheers,
Jussi
------------------------------
Message: 8
Date: Thu, 2 May 2019 10:24:52 +0200
From: Daniel Wagner <[email protected]>
To: Jussi Laakkonen <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH 2/5] vpn: Add support for configurable user and
groups in settings
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8
Hi Jussi,
On 02.05.19 09:41, Jussi Laakkonen wrote:
>>> An example config is:
>>> [VPNBinary]
>>
>> So if you want to do a group, it tell what it is about, maybe
>> Permission/AccessControl or something along this. I wouldn't mind to
>> have the User/Group/SupmentaryGroups in the general section
>> either. VPNBinary seems not really fitting here.
>>
>
> The naming for the group was not perfect, I agree. I think these could
> be in own group to have a clear separation of the configuration purposes.
>
> I'll think renaming VPNBinary to Privileges as it would be the most
> descriptive (or DACPrivileges, or DAC_Privileges), since this is solely
> about changing the privileges of the binary ran by connman-vpnd when
> connecting a VPN plugin. What do you say Daniel?
I slowly understand why I didn't understand the purpose of the series
(last mail). Brains starts to work...
Indeed I think DACPrivileges or alike would be a better name. Just
wondering if we want to ponder on the idea if we want to support more
then one global configuration, e.g. two different configuration (not
active at the same time). Would that be overkill?
Thanks,
Daniel
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 43, Issue 3
**************************************
