[PATCH] service: clear credentials on manual connect failure

2015-09-15 Thread Adam Moore
Poor RF environments can cause connect-failed regardless
of password correctness.  When performing a manual connection,
the correctness of the password has not been proved. Clear the
credentials in this case so they have an opportunity to be
corrected if necessary.
---
 src/service.c | 40 ++--
 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/src/service.c b/src/service.c
index 196f6b5..568300c 100644
--- a/src/service.c
+++ b/src/service.c
@@ -4048,6 +4048,24 @@ static DBusMessage *disconnect_service(DBusConnection 
*conn,
return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
 }

+static void clear_credentials(struct connman_service *service)
+{
+   g_free(service->passphrase);
+   service->passphrase = NULL;
+
+   g_free(service->identity);
+   service->identity = NULL;
+
+   g_free(service->anonymous_identity);
+   service->anonymous_identity = NULL;
+
+   g_free(service->agent_identity);
+   service->agent_identity = NULL;
+
+   g_free(service->eap);
+   service->eap = NULL;
+}
+
 bool __connman_service_remove(struct connman_service *service)
 {
if (service->type == CONNMAN_SERVICE_TYPE_ETHERNET ||
@@ -4064,20 +4082,7 @@ bool __connman_service_remove(struct connman_service 
*service)

__connman_service_disconnect(service);

-   g_free(service->passphrase);
-   service->passphrase = NULL;
-
-   g_free(service->identity);
-   service->identity = NULL;
-
-   g_free(service->anonymous_identity);
-   service->anonymous_identity = NULL;
-
-   g_free(service->agent_identity);
-   service->agent_identity = NULL;
-
-   g_free(service->eap);
-   service->eap = NULL;
+   clear_credentials(service);

service->error = CONNMAN_SERVICE_ERROR_UNKNOWN;

@@ -5500,6 +5505,13 @@ int __connman_service_indicate_error(struct 
connman_service *service,
__connman_service_ipconfig_indicate_state(service,
CONNMAN_SERVICE_STATE_FAILURE,
CONNMAN_IPCONFIG_TYPE_IPV6);
+
+   if (!service->favorite) {
+   if (error == CONNMAN_SERVICE_ERROR_CONNECT_FAILED) {
+   clear_credentials(service);
+   }
+   }
+
return 0;
 }

--
2.4.6


Statement of Confidentiality

The contents of this e-mail message and any attachments are confidential and 
are intended solely for the addressee. The information may also be legally 
privileged. This transmission is sent in trust, and the sole purpose of 
delivery to the intended recipient. If you have received this transmission in 
error, any use, reproduction or dissemination of this transmission is strictly 
prohibited. If you are not the intended recipient, please immediately notify 
the sender by reply e-mail or at 508.683.2500 and delete this message and its 
attachments, if any.

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman


Re: problem after suspend/resume using connman and switch

2015-09-15 Thread laurent vaudoit
Hi,

On Mon, Sep 14, 2015 at 2:20 PM, laurent vaudoit 
wrote:

> Hi Patrik,
>
> On Thu, Sep 10, 2015 at 12:24 PM, Patrik Flykt <
> patrik.fl...@linux.intel.com> wrote:
>
>> On Thu, 2015-09-10 at 11:41 +0200, laurent vaudoit wrote:
>>
>> > On Thu, Sep 10, 2015 at 11:35 AM, Patrik Flykt <
>> patrik.fl...@linux.intel.com
>> > > wrote:
>> > >
>> > > Hi,
>> > >
>> > > On Wed, 2015-09-09 at 16:52 +0200, laurent vaudoit wrote:
>> > > > So i've made a little modification in plugin/ethernet.c (inspired
>> from
>> > > > the vlan management).
>> > > > The evolution consist in checking if the interface use the dsa
>> driver,
>> > > > and in this case, add to service name the switch port number (as
>> > > > connman add the vlanid to the service name).
>> > >
>> > > What exactly is this dsa driver? Is it upstream or provided by a
>> vendor?
>> > >
>> > dsa is the "distributed switch architecture" driver, which is upstream.
>> > under this there is the chip driver for the switch, who communicate
>> trhough
>> > mdio interface with the chip.
>> > In my case, it is a new developped driver as the chip was not supported
>> by
>> > kernel, but there is allready some driver existing (marvell for example)
>> > source code is under net/dsa/
>>
>> Sounds like a plan. Send a patch after you have tested it.
>>
>
> We are running some test and most of our use case are ok.
> The only thing we do not understand, is that if the dhcp cleitn do not get
> an ip adress,
> we were expecting that the board get an ip in 169.254... through link
> local?
> Is there something we missed?
>
>
After some analysis, it seems that ipv4ll is launch only on one service if
dhcp timeout.
So when we have more than one services, only the first one get an ip from
link local.
Is there any reason for this behaviour?
It seems not to be linked to our "DSA patch", who is in attachment.

>
>> Cheers,
>>
>> Patrik
>>
>
> Regards
Laurent

> Regards
> Laurent
>
>>
>> ___
>> connman mailing list
>> connman@connman.net
>> https://lists.connman.net/mailman/listinfo/connman
>>
>
>
From 1b58bc565c3e9e1a005b3eac496e25acfd786f8e Mon Sep 17 00:00:00 2001
From: Laurent Vaudoit 
Date: Mon, 14 Sep 2015 15:49:10 +0200
Subject: [PATCH] add dsa interface support

---
 plugins/ethernet.c | 58 --
 1 file changed, 56 insertions(+), 2 deletions(-)

diff --git a/plugins/ethernet.c b/plugins/ethernet.c
index d176508..072dc7f 100644
--- a/plugins/ethernet.c
+++ b/plugins/ethernet.c
@@ -32,6 +32,7 @@
 
 #include 
 #include 
+#include 
 
 #ifndef IFF_LOWER_UP
 #define IFF_LOWER_UP	0x1
@@ -83,6 +84,56 @@ static int get_vlan_vid(const char *ifname)
 	return vid;
 }
 
+static int get_dsa_port(const char *ifname)
+{
+	int sk;
+	int dsaport=-1;
+	struct ifreq ifr;
+	struct ethtool_cmd cmd;
+	struct ethtool_drvinfo drvinfocmd;
+
+	sk = socket(AF_INET, SOCK_STREAM, 0);
+	if (sk < 0)
+		return -errno;
+
+	memset(, 0, sizeof(ifr));
+	strcpy(ifr.ifr_name, ifname);
+
+	/* get driver info */
+	drvinfocmd.cmd =  ETHTOOL_GDRVINFO;
+	ifr.ifr_data = (caddr_t)
+	
+	/*	ioctl failed:	*/
+	if (ioctl(sk, SIOCETHTOOL, ) != 0)
+	{
+		DBG("Cannot get driver infos\n");
+
+	}
+	else
+	{
+		
+		if(strcmp(drvinfocmd.driver,"dsa") == 0)
+		{
+			/* get dsa port*/
+			cmd.cmd =  ETHTOOL_GSET;
+			ifr.ifr_data = (caddr_t)
+			
+			/*	ioctl failed:	*/
+			if (ioctl(sk, SIOCETHTOOL, ) != 0)
+			{
+DBG("Cannot get device settings\n");
+
+			}
+			else
+dsaport = cmd.phy_address;
+		}
+	}
+	close(sk);
+
+	return dsaport;
+
+}
+
 static int eth_network_probe(struct connman_network *network)
 {
 	DBG("network %p", network);
@@ -126,7 +177,7 @@ static void add_network(struct connman_device *device,
 			struct ethernet_data *ethernet)
 {
 	struct connman_network *network;
-	int index, vid;
+	int index, vid,dsaport;
 	char *ifname;
 
 	network = connman_network_create("carrier",
@@ -140,6 +191,7 @@ static void add_network(struct connman_device *device,
 	if (!ifname)
 		return;
 	vid = get_vlan_vid(ifname);
+	dsaport = get_dsa_port(ifname);
 
 	connman_network_set_name(network, "Wired");
 
@@ -149,7 +201,7 @@ static void add_network(struct connman_device *device,
 	}
 
 	if (!eth_tethering) {
-		char group[10] = "cable";
+		char group[16] = "cable";
 		/*
 		 * Prevent service from starting the reconnect
 		 * procedure as we do not want the DHCP client
@@ -157,6 +209,8 @@ static void add_network(struct connman_device *device,
 		 */
 		if (vid >= 0)
 			snprintf(group, sizeof(group), "%03x_cable", vid);
+		if (dsaport >= 0)
+			snprintf(group, sizeof(group), "%03x_cable", dsaport);
 
 		connman_network_set_group(network, group);
 	}
-- 
1.9.1

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: problem after suspend/resume using connman and switch

2015-09-15 Thread Patrik Flykt
On Tue, 2015-09-15 at 08:25 +0200, laurent vaudoit wrote:
> After some analysis, it seems that ipv4ll is launch only on one
> service if dhcp timeout.
> So when we have more than one services, only the first one get an ip
> from link local.

The current implementation starts IPv4 link local after a DHCPv4
timeout.

As IPv4 link local addressing is reusing subnet addressing per
interface, it's not a reliable means of identifying hosts by routing and
is therefore implemented only for one interface at a time in ConnMan.
IPv4 link local addressing is no substitute for networks lacking DHCP or
static configuration.

> Is there any reason for this behaviour?
> It seems not to be linked to our "DSA patch", who is in attachment.

Please send patches as inline mails. Use git send-email for example.

The numerical dsa port id you proposed in the patch will get mixed with
VLAN ids, so you will have to add something more to identify the network
being a dsa one and not a VLAN. According to the coding style, else is
on the same line as the end of the last code block, i.e. '} else {'.

Cheers,

Patrik


___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman


Re: problem after suspend/resume using connman and switch

2015-09-15 Thread laurent vaudoit
On Tue, Sep 15, 2015 at 9:10 AM, Patrik Flykt 
wrote:

> On Tue, 2015-09-15 at 08:25 +0200, laurent vaudoit wrote:
> > After some analysis, it seems that ipv4ll is launch only on one
> > service if dhcp timeout.
> > So when we have more than one services, only the first one get an ip
> > from link local.
>
> The current implementation starts IPv4 link local after a DHCPv4
> timeout.
>
> As IPv4 link local addressing is reusing subnet addressing per
> interface, it's not a reliable means of identifying hosts by routing and
> is therefore implemented only for one interface at a time in ConnMan.
> IPv4 link local addressing is no substitute for networks lacking DHCP or
> static configuration.
>
> > Is there any reason for this behaviour?
> > It seems not to be linked to our "DSA patch", who is in attachment.
>
> Please send patches as inline mails. Use git send-email for example.
>
> The numerical dsa port id you proposed in the patch will get mixed with
> VLAN ids, so you will have to add something more to identify the network
> being a dsa one and not a VLAN. According to the coding style, else is
> on the same line as the end of the last code block, i.e. '} else {'.
>
>
thank you for your remark, i will check the coding style.
in order to identify the network, is there some rules to apply?
Is it possible to use the format:
ethernet_MACADRESS_switchPORTNB_cable with PORTNB the dsa port number on
the switch?

I will send patch through git the next time.

Cheers,
>
> Patrik
>
> Regards
Laurent

>
> ___
> connman mailing list
> connman@connman.net
> https://lists.connman.net/mailman/listinfo/connman
>
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman


Re: [PATCH] resolver: allow writing to /etc/resolv.conf to be disabled

2015-09-15 Thread Sam Nazarko
Hi Patrik,

I am happy with this solution to write to /var/run/connman/resolv.conf. I am 
happy to submit a patch for this as well as a revised systemd service unit with 
an ExecStartPre= entry to create the symlink before starting ConnMan. Please 
let me know if you will accept this. 

Our current implementation actually calls a script before launching ConnMan to 
run some sanity checks and evaluate whether we want ConnMan's resolv.conf or 
not, but revising the systemd unit is probably the best method to maintain 
immediate compatibility and provide an entry point for other distributions to 
change this behaviour. 

Sam

From: connman  on behalf of Patrik Flykt 

Sent: 11 September 2015 07:17
To: connman@connman.net
Subject: Re: [PATCH] resolver: allow writing to /etc/resolv.conf to be disabled

Hi,

On Fri, 2015-09-11 at 01:07 +, Sam Nazarko wrote:

> The primary use case is to keep ConnMan running so that in OSMC users
> can still configure Bluetooth connections or WiFi adapters with a
> unified interface (and we can support it with a single API).

Thanks for the info.

The above means that in the OSMC configuration the DNS servers from the
kernel command line, kernel variables or other static entries written to
resolv.conf also satisfy the name lookups done while using Bluetooth or
WiFi.

I was thinking that this leads to a solution like:
- always write ConnMan's resolv.conf entries to the hereafter
  "well-known" location at /var/run/connman/resolv.conf
- by default replace /etc/resolv conf with a symlink to ConnMan's own
  resolv.conf file
- prevent the creation of the symlink by defining a command line
  option and main.conf variable.

By not making ConnMan's resolv.conf location configurable keeps
resolv.conf handling distribution agnostic and ConnMan specific while
being in line with what systemd-resolved offers. The two latter points
above are there only to keep the current status quo.

Cheers,

Patrik


___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman


Re: Wi-Fi access point use case - tether DNS and IP address

2015-09-15 Thread Patrik Flykt

Hi,

On Tue, 2015-09-15 at 15:40 +1000, Craig McQueen wrote:
> I'd like to use ConnMan in a device that connects to an Ethernet
> network, and also (optionally) can operate Wi-Fi in access point mode
> to provide technician type access to itself.
> 
> So it looks like "tethering" is the way to do a Wi-Fi access point.
> But in this use case, I don't want Wi-Fi tethering to permit access to
> upstream connections (the wired Ethernet). I can easily achieve that
> by adding a firewall with FORWARD rule set to DROP.

By default ConnMan does not touch /proc/sys/net/ipv4/conf/*/forwarding,
so whatever is the system default forwarding/routing policy will be
followed. Whether this is a good or bad idea depends on the used case,
but happens to be the current status quo.

> However, I also want to lock down DNS. Even with forwarding stopped, a
> Wi-Fi client can still do DNS look-ups through the upstream
> connection, thus providing a back channel of communication.

The sane default is to provide the tethered networks with DNS services,
so blocking the DNS port in your case sounds like applying an extra
iptables rule.

> Secondly, I'd also like Wi-Fi clients to be able to access the device
> via a DNS name, such as my-serial-number.lan. I'm not sure how to
> configure a DNS server for the tether interface.

That is currently not implemented. Patches for reading e.g. /etc/hosts
or some other (symlinked) file like /var/lib/connman/hosts can be
considered.

> Related to this: maybe I could run a DNS server, getting the tether
> interface's IP address updates through D-Bus. But as I've seen with
> connmanctl monitor, tether IP address doesn't seem to be notified on
> D-Bus.

As a hackish solution one can always disable dns proxying in ConnMan,
but that usually creates more problem than it solves. So a patch for
reading a file of host names looks like a more attractive option here.

> In summary, these questions:
> 
> 1) How could the tether interface's DNS look-ups through upstream be
> restricted?
> 2) How could a DNS server be provided for the tether interface, which
>responds to my-serial-number.lan with the tether interface's current IP
>address?

It won't be easy to do selective DNS lookups in ConnMan, but some file
could be read to provide local DNS services. But before doing thta, it
should be investigated how or if systemd-resolved can be exploited to
handle name lookups in the future, especially if it gains a D-Bus API.

> 3) How can notifications of tether interface's IP address changes be
> obtained via D-Bus?

As tethering is considered to happen automatically without needing any
user intervention, there hasn't been any real need to expose tethered IP
address information either. Although ConnMan is light-weight, it's still
not a generic embedded routing platform.

Cheers,

Patrik

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman


Re: Wi-Fi access point use case - tether DNS and IP address

2015-09-15 Thread David Lechner

On 09/15/2015 02:37 AM, Patrik Flykt wrote:

3) How can notifications of tether interface's IP address changes be
obtained via D-Bus?

As tethering is considered to happen automatically without needing any
user intervention, there hasn't been any real need to expose tethered IP
address information either. Although ConnMan is light-weight, it's still
not a generic embedded routing platform.


I have a use case for this as well (not trying to make a router, but 
just displaying the IP address of the tether interface for informational 
purposes). The work around that I came up with is to use udev to listen 
to add, change and remove events. The device is always named "tether" so 
it is easy to match. If the device is added or changed, I look up the 
address by using getifaddrs and finding it in the list.

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman


Re: [PATCH] resolver: allow writing to /etc/resolv.conf to be disabled

2015-09-15 Thread Patrik Flykt

Hi,

On Tue, 2015-09-15 at 11:29 +, Sam Nazarko wrote:
> >But also scripts/connman.in should be modified to create the
> >needed symlink.
> 
> Are you saying that you would like to create the symlink as part of
> the packaging? I'm not sure this is necessarily a good idea,
> particularly when packaging with Debian. This means that connman would
> take 'ownership' of /etc/resolv.conf which is not necessarily a good
> idea.

That connman.in file is a generic init script. Need it work for Debian,
it needs LSB fields added in a patch or otherwise anyway. The init
script in the upstream tar ball properly sources /etc/default/connman,
so one can always hide updating of resolv.conf behind a variable.

Having ConnMan write directly into /etc/resolv.conf is probably not what
Debian would like to happen either, but for better or worse it's the
current behavior. The idea here is that from one version to another
there should be a very high probability of things working exactly as
before, also when using init scripts.

Cheers,

Patrik


___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman


Re: [PATCH] resolver: allow writing to /etc/resolv.conf to be disabled

2015-09-15 Thread Patrik Flykt
On Tue, 2015-09-15 at 07:15 +, Sam Nazarko wrote:
> I am happy with this solution to write
> to /var/run/connman/resolv.conf. I am happy to submit a patch for this
> as well as a revised systemd service unit with an ExecStartPre= entry
> to create the symlink before starting ConnMan. Please let me know if
> you will accept this. 

systemd-tmpfiles looks like being the correct tool for this task. With
this solution no additional variables are needed main.conf, which is a
good thing. But also scripts/connman.in should be modified to create the
needed symlink.

The above scheme can fail if the system provides its own init scripts,
so now would be the time to take notice, speak up and/or fix such init
scripts.

> Our current implementation actually calls a script before launching
> ConnMan to run some sanity checks and evaluate whether we want
> ConnMan's resolv.conf or not, but revising the systemd unit is
> probably the best method to maintain immediate compatibility and
> provide an entry point for other distributions to change this
> behaviour. 

Here I suppose the modifications are done via a connman.service.d/*.conf
systemd.unit files in order to eliminate source code patches for ConnMan
systemd service startup.

More comments, anyone?

Patrik

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman


Re: problem after suspend/resume using connman and switch

2015-09-15 Thread laurent vaudoit
On Tue, Sep 15, 2015 at 1:04 PM, Patrik Flykt 
wrote:

>
> Hi,
>
> On Tue, 2015-09-15 at 09:30 +0200, laurent vaudoit wrote:
>
> > in order to identify the network, is there some rules to apply?
> > Is it possible to use the format:
> > ethernet_MACADRESS_switchPORTNB_cable with PORTNB the dsa port number on
> > the switch?
>
> If this behavior is generic enough, maybe a '_p_cable' should be
> used. Also check the port range, i.e. is %03x a suitable value or not. I
> suppose it is possible to create VLANs for this interface also, which
> means that the identifier needs to be built step by step taking also the
> VLAN information into account. Instead of an ioctl, is this information
> available in a newlink message? If so, that one should be used to avoid
> the extra ioctl.
>

ok, in case a vlan is created on this interface,
service name will be _p__cable
and if it is not a vlan, service name will be _p>id>_cable.
For the port range, i don't know many component, but  a %02x should be
enough.
for newlink message, i'm not a specilaist of this but i did not found the
information in it, that's the reason i used ioctl.

>
> While we are at it, you have looked at systemd-networkd also, but
> noticed that you actually do need ConnMan because it also supports
> Bluetooth and WiFi?
>

Yes we need wifi, bluetooth, ethernet, ofono, and thats why we choose
connman

>
> Cheers,
>
> Patrik
>

Regards
   Laurent


>
> ___
> connman mailing list
> connman@connman.net
> https://lists.connman.net/mailman/listinfo/connman
>
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman


Re: problem after suspend/resume using connman and switch

2015-09-15 Thread Patrik Flykt

Hi,

On Tue, 2015-09-15 at 09:30 +0200, laurent vaudoit wrote:

> in order to identify the network, is there some rules to apply?
> Is it possible to use the format:
> ethernet_MACADRESS_switchPORTNB_cable with PORTNB the dsa port number on
> the switch?

If this behavior is generic enough, maybe a '_p_cable' should be
used. Also check the port range, i.e. is %03x a suitable value or not. I
suppose it is possible to create VLANs for this interface also, which
means that the identifier needs to be built step by step taking also the
VLAN information into account. Instead of an ioctl, is this information
available in a newlink message? If so, that one should be used to avoid
the extra ioctl.

While we are at it, you have looked at systemd-networkd also, but
noticed that you actually do need ConnMan because it also supports
Bluetooth and WiFi?

Cheers,

Patrik

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman


Re: [PATCH] resolver: allow writing to /etc/resolv.conf to be disabled

2015-09-15 Thread Sam Nazarko
Hi,

>But also scripts/connman.in should be modified to create the
>needed symlink.

Are you saying that you would like to create the symlink as part of the 
packaging? I'm not sure this is necessarily a good idea, particularly when 
packaging with Debian. This means that connman would take 'ownership' of 
/etc/resolv.conf which is not necessarily a good idea.

>Here I suppose the modifications are done via a connman.service.d/*.conf
>systemd.unit files in order to eliminate source code patches for ConnMan
>systemd service startup.

We do not use a systemd dropin, we instead distribute our own systemd 
configuration. Part of this stems from the need for customisation at the 
moment, and also because at the time ConnMan had incorrect service 
dependencies. This was raised in #CM-683 by Simon Byrnand (OSMC) and was fixed 
in 1.30. 

Sam

From: connman  on behalf of Patrik Flykt 

Sent: 15 September 2015 11:46
To: connman@connman.net
Subject: Re: [PATCH] resolver: allow writing to /etc/resolv.conf to be disabled

On Tue, 2015-09-15 at 07:15 +, Sam Nazarko wrote:
> I am happy with this solution to write
> to /var/run/connman/resolv.conf. I am happy to submit a patch for this
> as well as a revised systemd service unit with an ExecStartPre= entry
> to create the symlink before starting ConnMan. Please let me know if
> you will accept this.

systemd-tmpfiles looks like being the correct tool for this task. With
this solution no additional variables are needed main.conf, which is a
good thing. But also scripts/connman.in should be modified to create the
needed symlink.

The above scheme can fail if the system provides its own init scripts,
so now would be the time to take notice, speak up and/or fix such init
scripts.

> Our current implementation actually calls a script before launching
> ConnMan to run some sanity checks and evaluate whether we want
> ConnMan's resolv.conf or not, but revising the systemd unit is
> probably the best method to maintain immediate compatibility and
> provide an entry point for other distributions to change this
> behaviour.

Here I suppose the modifications are done via a connman.service.d/*.conf
systemd.unit files in order to eliminate source code patches for ConnMan
systemd service startup.

More comments, anyone?

Patrik

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman