Re: [PATCH v2 01/23] client: Add connmanctl D-Bus helper functions

2013-04-04 Thread Patrik Flykt 
On Tue, 2013-04-02 at 13:53 +0200, Daniel Wagner wrote:
 Hi Patrik
 
 On 04/02/2013 12:34 PM, Patrik Flykt wrote:
  +void __connmanctl_dbus_print(DBusMessageIter *iter, const char *pre,
  +   const char *dict, const char *sep);
  +
  +typedef void (*connmanctl_dbus_method_return_fn)(DBusMessageIter *iter,
  +   const char *error, void *user_data);
 
 Nitpicking: It seems we cannot agree on the postfix on a callback 
 typedef. So far we have:
 
 src/connman.h: typedef void (*connman_iptables_iterate_chains_cb_t)
 src/shared/util.h:typedef void (*util_debug_func_t)
 
 oFono and BlueZ prefer the _func_t prefix, IIRC. So I suggest we go 
 also in this direction.

Fixed according to the nitpicking above.

Cheers,

Patrik

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [PATCH v2 03/23] client: Add input handling

2013-04-04 Thread Patrik Flykt 
On Tue, 2013-04-02 at 13:56 +0200, Daniel Wagner wrote:
 Hi Patrik,
 
 again nitpicking :)
 
 On 04/02/2013 12:34 PM, Patrik Flykt wrote:
  +void __connmanctl_save_rl(void)
  +{
  +   if (interactive == false)
  +   return;
  +
  +   save_input = !RL_ISSTATE(RL_STATE_DONE);
  +
  +   if (save_input) {
  +   saved_point = rl_point;
  +   saved_line = rl_copy_text(0, rl_end);
  +   rl_save_prompt();
  +   rl_replace_line(, 0);
  +   rl_redisplay();
  +   }
  +}
 
 I understand we are going to adopt also oFono/BlueZ style for bools etc. 
 Should the first if read the more like
 
   if (!interactive)
   return;
 
 ?

Thanks for the head up on this, I decided to leave it as is.

Cheers,

Patrik
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [PATCH v2 00/23] Command line client, part 2

2013-04-04 Thread Patrik Flykt 
On Tue, 2013-04-02 at 13:34 +0300, Patrik Flykt wrote:
   Hi,
 
 Version 2 of the command line client part 2 makes connmanctl self-contained,
 with all code residing in client/ and a few helper functions used from
 gdbus/. This change eliminates the compile time mess when using functions
 from the main source directory.

There was yet another header file missing from Makefile.am, fixed that.
Also added a missing error printout on unknown command.

Applied,

Patrik
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [PATCH v2 03/23] client: Add input handling

2013-04-04 Thread Daniel Wagner 

On 04/04/2013 09:41 AM, Patrik Flykt wrote:

On Tue, 2013-04-02 at 13:56 +0200, Daniel Wagner wrote:

Hi Patrik,

again nitpicking :)

On 04/02/2013 12:34 PM, Patrik Flykt wrote:

+void __connmanctl_save_rl(void)
+{
+   if (interactive == false)
+   return;
+
+   save_input = !RL_ISSTATE(RL_STATE_DONE);
+
+   if (save_input) {
+   saved_point = rl_point;
+   saved_line = rl_copy_text(0, rl_end);
+   rl_save_prompt();
+   rl_replace_line(, 0);
+   rl_redisplay();
+   }
+}


I understand we are going to adopt also oFono/BlueZ style for bools etc.
Should the first if read the more like

if (!interactive)
return;

?


Thanks for the head up on this, I decided to leave it as is.


No problem. I think it is better to stream line the whole code base in 
one go anyway, if we want to do it :)


___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v0 2/5] session: Reorder shutdown sequence

2013-04-04 Thread Daniel Wagner 
From: Daniel Wagner daniel.wag...@bmw-carit.de

By calling __connman_session_cleanup() before __connman_plugin_cleanup()
we make sure all resources allocated can released in the correct order.
The code assumes after an successful allocation, free will always work
and therefore we double free allocated memory.
---
 src/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/main.c b/src/main.c
index 0f47943..a76ec3b 100644
--- a/src/main.c
+++ b/src/main.c
@@ -676,11 +676,11 @@ int main(int argc, char *argv[])
__connman_wpad_cleanup();
__connman_dhcpv6_cleanup();
__connman_dhcp_cleanup();
+   __connman_session_cleanup();
__connman_plugin_cleanup();
__connman_provider_cleanup();
__connman_connection_cleanup();
__connman_timeserver_cleanup();
-   __connman_session_cleanup();
__connman_detect_cleanup();
__connman_proxy_cleanup();
__connman_task_cleanup();
-- 
1.8.2.rc3.16.gce432ca

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v0 1/5] session: Empty policy list indicated no match all

2013-04-04 Thread Daniel Wagner 
From: Daniel Wagner daniel.wag...@bmw-carit.de

Match all is expressed via CONNMAN_SERVICE_TYPE_UNKNOWN. If the
policy list is empty, that we just do not match anything
---
 src/session.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/session.c b/src/session.c
index 6916458..39920af 100644
--- a/src/session.c
+++ b/src/session.c
@@ -514,7 +514,7 @@ static int filter_bearer(GSList *policy_bearers,
GSList *it;
 
if (policy_bearers == NULL)
-   goto clone;
+   return 0;
 
for (it = policy_bearers; it != NULL; it = it-next) {
policy = GPOINTER_TO_INT(it-data);
-- 
1.8.2.rc3.16.gce432ca

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v0 3/5] session: Do not access initialized hash table

2013-04-04 Thread Daniel Wagner 
From: Daniel Wagner daniel.wag...@bmw-carit.de

Make sure we never try to cleanup when the hash table has
been destroyed. In this case all resources have been
freed already.
---
 src/session.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/session.c b/src/session.c
index 39920af..d06edc8 100644
--- a/src/session.c
+++ b/src/session.c
@@ -359,6 +359,9 @@ static void remove_policy(struct connman_session_policy 
*policy)
gpointer key, value;
struct connman_session *session;
 
+   if (session_hash == NULL)
+   return;
+
DBG(policy %p name %s, policy, policy-name);
 
g_hash_table_iter_init(iter, session_hash);
-- 
1.8.2.rc3.16.gce432ca

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v0 4/5] session_policy_local: Do not free policy on load error

2013-04-04 Thread Daniel Wagner 
From: Daniel Wagner daniel.wag...@bmw-carit.de

When we open the policy file and try to parse we might run
into an error. Instead of freeing the policy we just need
to reset it to the defaults and then try to apply the new settings.
We should reallyt not unref the policy on the error case because
the policy lifetime is attached to the lifetime of the file not
on the result of the parsing.
---
 plugins/session_policy_local.c | 24 ++--
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/plugins/session_policy_local.c b/plugins/session_policy_local.c
index 5a8f6b8..aa734a1 100644
--- a/plugins/session_policy_local.c
+++ b/plugins/session_policy_local.c
@@ -294,6 +294,8 @@ static int load_policy(struct policy_data *policy)
char *str, **tokens;
int i, err = 0;
 
+   connman_session_set_default_config(config);
+
pathname = g_strdup_printf(%s/%s, POLICYDIR, policy-ident);
if(pathname == NULL)
return -ENOMEM;
@@ -318,8 +320,6 @@ static int load_policy(struct policy_data *policy)
if (str != NULL) {
config-roaming_policy = 
connman_session_parse_roaming_policy(str);
g_free(str);
-   } else {
-   config-roaming_policy = CONNMAN_SESSION_ROAMING_POLICY_DEFAULT;
}
 
str = g_key_file_get_string(keyfile, Default, ConnectionType,
@@ -327,20 +327,18 @@ static int load_policy(struct policy_data *policy)
if (str != NULL) {
config-type = connman_session_parse_connection_type(str);
g_free(str);
-   } else {
-   config-type = CONNMAN_SESSION_TYPE_ANY;
}
 
config-ecall = g_key_file_get_boolean(keyfile, Default,
EmergencyCall, NULL);
 
-   g_slist_free(config-allowed_bearers);
-   config-allowed_bearers = NULL;
-
str = g_key_file_get_string(keyfile, Default, AllowedBearers,
NULL);
 
if (str != NULL) {
+   g_slist_free(config-allowed_bearers);
+   config-allowed_bearers = NULL;
+
tokens = g_strsplit(str,  , 0);
 
for (i = 0; tokens[i] != NULL; i++) {
@@ -352,11 +350,6 @@ static int load_policy(struct policy_data *policy)
 
g_free(str);
g_strfreev(tokens);
-   } else {
-   config-allowed_bearers = g_slist_append(NULL,
-   GINT_TO_POINTER(CONNMAN_SERVICE_TYPE_UNKNOWN));
-   if (config-allowed_bearers == NULL)
-   err = -ENOMEM;
}
 
g_key_file_free(keyfile);
@@ -397,6 +390,7 @@ static void notify_handler(struct inotify_event *event,
 const char *ident)
 {
struct policy_data *policy;
+   int err;
 
if (ident == NULL)
return;
@@ -418,8 +412,10 @@ static void notify_handler(struct inotify_event *event,
if (event-mask  IN_MODIFY) {
connman_info(Policy modifed for '%s', ident);
 
-   if (load_policy(policy)  0) {
-   remove_policy(policy);
+   err = load_policy(policy);
+   if (err  0) {
+   connman_warn(Loading policy file '%s' failed with %s,
+   ident, strerror(-err));
return;
}
}
-- 
1.8.2.rc3.16.gce432ca

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v0 5/5] session_policy_local: Load policy when a new file is added

2013-04-04 Thread Daniel Wagner 
From: Daniel Wagner daniel.wag...@bmw-carit.de

When a file is just added (not modified) we need also to parse it.
---
 plugins/session_policy_local.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/plugins/session_policy_local.c b/plugins/session_policy_local.c
index aa734a1..6a527cb 100644
--- a/plugins/session_policy_local.c
+++ b/plugins/session_policy_local.c
@@ -404,6 +404,13 @@ static void notify_handler(struct inotify_event *event,
policy_ref(policy);
else
policy = create_policy(ident);
+
+   err = load_policy(policy);
+   if (err  0) {
+   connman_warn(Loading policy file '%s' failed with %s,
+   ident, strerror(-err));
+   return;
+   }
}
 
if (policy == NULL)
-- 
1.8.2.rc3.16.gce432ca

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v0 0/5] Session and Policy Plugin Fixes

2013-04-04 Thread Daniel Wagner 
From: Daniel Wagner daniel.wag...@bmw-carit.de

Hi,

Let's have the next batch of patches from the 'per application routing
and staticics' series.

I have reordered a few patches so that we first have a bug fixes in
session.c and session_policy_local.c file only.

  session: Empty policy list indicated no match all

We need to express a 'no match' as well which is done by having no
bearer at all in the list. Obviously, when no bearer is in the policy
configuration than we should just return false in all cases.

  session: Reorder shutdown sequence

I saw ConnMan crash when I did a ctrl-c with an active session.
With this patch the crash is gone.

  session: Do not access initialized hash table

That one goes into the same catagory as the one above. Do not crash
when shuting down while a session is running.

  session_policy_local: Do not free policy on load error
  session_policy_local: Load policy when a new file is added

Both patches fixes corner cases how we handle/behave when the policy files
are modified/added/removed. These patches should make ConnMan more robust
when the files are invalid.

If you want to see where we are heading have a look at the
whole series here:

https://github.com/bmwcarit/connman/tree/secmark-v3

cheers,
daniel

Daniel Wagner (5):
  session: Empty policy list indicated no match all
  session: Reorder shutdown sequence
  session: Do not access initialized hash table
  session_policy_local: Do not free policy on load error
  session_policy_local: Load policy when a new file is added

 plugins/session_policy_local.c | 31 +--
 src/main.c |  2 +-
 src/session.c  |  5 -
 3 files changed, 22 insertions(+), 16 deletions(-)

-- 
1.8.2.rc3.16.gce432ca

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [PATCH v0 4/5] session_policy_local: Do not free policy on load error

2013-04-04 Thread Daniel Wagner 

@@ -397,6 +390,7 @@ static void notify_handler(struct inotify_event *event,
  const char *ident)
  {
struct policy_data *policy;
+   int err;

if (ident == NULL)
return;
@@ -418,8 +412,10 @@ static void notify_handler(struct inotify_event *event,
if (event-mask  IN_MODIFY) {
connman_info(Policy modifed for '%s', ident);

-   if (load_policy(policy)  0) {
-   remove_policy(policy);
+   err = load_policy(policy);
+   if (err  0) {
+   connman_warn(Loading policy file '%s' failed with %s,
+   ident, strerror(-err));
return;
}
}



I review my own patch: that junks doesn't belong to this patch ...
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [PATCH] Allow ethernet provision by interface name

2013-04-04 Thread Patrik Flykt 

Hi,

On Wed, 2013-04-03 at 13:38 +0200, Simon Busch wrote:
 Allowing provision by interface name can be helpful in some situations where
 interface names never change. In all other situations where the interface 
 name is very
 likely to change the MAC should still be used as identifier for provisioning.

Since we already have the MAC address identifying the interface, I'm
inclined not to take this patch. Recent versions of systemd will name
the interfaces very differently from what one might be used to, which
can become a not so nice source of confusion.

If the MAC address match is not set in the provisioning file, ConnMan
will provision the first or only ethernet adapter in the device. The
first adapter depends on the interface detection order in the Linux
kernel. And if plain DHCP configuration is desired, no provisioning file
needs to be written, it's enough to plug in the ethernet cable.


Cheers,

Patrik

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v2 1/8] inet: Get an address from a given interface and address family

2013-04-04 Thread Jukka Rissanen 
The returned address is used when we need to have a listening
socket tied to specific interface and address, and do not want to
bind to any address.
---
 src/connman.h |  1 +
 src/inet.c| 55 +++
 2 files changed, 56 insertions(+)

diff --git a/src/connman.h b/src/connman.h
index e9c774e..b63e658 100644
--- a/src/connman.h
+++ b/src/connman.h
@@ -133,6 +133,7 @@ int __connman_inet_modify_address(int cmd, int flags, int 
index, int family,
const char *peer,
unsigned char prefixlen,
const char *broadcast);
+int __connman_inet_get_interface_address(int index, int family, void *address);
 
 #include netinet/ip6.h
 #include netinet/icmp6.h
diff --git a/src/inet.c b/src/inet.c
index 0027fe6..5196576 100644
--- a/src/inet.c
+++ b/src/inet.c
@@ -45,6 +45,7 @@
 #include fcntl.h
 #include linux/if_tun.h
 #include ctype.h
+#include ifaddrs.h
 
 #include connman.h
 
@@ -2376,3 +2377,57 @@ connman_bool_t connman_inet_is_ipv6_supported()
close(sk);
return TRUE;
 }
+
+int __connman_inet_get_interface_address(int index, int family, void *address)
+{
+   struct ifaddrs *ifaddr, *ifa;
+   int err = -ENOENT;
+   char name[IF_NAMESIZE];
+
+   if (if_indextoname(index, name) == NULL)
+   return -EINVAL;
+
+   DBG(index %d interface %s, index, name);
+
+   if (getifaddrs(ifaddr)  0) {
+   err = -errno;
+   DBG(Cannot get addresses err %d/%s, err, strerror(-err));
+   return err;
+   }
+
+   for (ifa = ifaddr; ifa != NULL; ifa = ifa-ifa_next) {
+   if (ifa-ifa_addr == NULL)
+   continue;
+
+   if (strncmp(ifa-ifa_name, name, IF_NAMESIZE) == 0 
+   ifa-ifa_addr-sa_family == family) {
+   if (family == AF_INET) {
+   struct sockaddr_in *in4 = (struct sockaddr_in *)
+   ifa-ifa_addr;
+   if (in4-sin_addr.s_addr == INADDR_ANY)
+   continue;
+   memcpy(address, in4-sin_addr,
+   sizeof(struct in_addr));
+   } else if (family == AF_INET6) {
+   struct sockaddr_in6 *in6 =
+   (struct sockaddr_in6 *)ifa-ifa_addr;
+   if (memcmp(in6-sin6_addr, in6addr_any,
+   sizeof(struct in6_addr)) == 0)
+   continue;
+   memcpy(address, in6-sin6_addr,
+   sizeof(struct in6_addr));
+
+   } else {
+   err = -EINVAL;
+   goto out;
+   }
+
+   err = 0;
+   break;
+   }
+   }
+
+out:
+   freeifaddrs(ifaddr);
+   return err;
+}
-- 
1.7.11.7

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v2 0/8] DNS proxy should listen only needed addresses

2013-04-04 Thread Jukka Rissanen 
Hi,

v2:
- The removal of ::1 from resolv.conf was missing in various places,
  this is now fixed in patch 3
- Using ::1 instead of 127.0.0.1 when refreshing the dns because we
  are doing IPv6 address check (patch 4)
- Check for ::1 server was missing (patch 5)
- Null pointer check was incorrectly placed (patch 6)
- Memory leak if there is a request timeout from server (patch 7)
- Create cache if it is missing when hash lookup is done (patch 8)

v1:
Current DNS proxy implementation prevents any other DNS server
running in the system because we bind to port 53 in all interfaces.
This patchset changes this and connman only binds to those interfaces
that it needs meaning loopback and tethering interface if tethering
is enabled.
We are trying to bind both IPv4 and IPv6 addresses but if IPv6
is disabled, then only IPv4 listener is created.
The diff of patch 2 looks a bit complex to read but might be easier
to review if it is applied first.

Cheers,
Jukka


Jukka Rissanen (8):
  inet: Get an address from a given interface and address family
  dnsproxy: Listen only needed addresses
  dnsproxy: Add or remove ::1 to/from resolv.conf when necessary
  dnsproxy: Use ::1 when refreshing because of the address family
  dnsproxy: Do not add or remove ::1 server
  dnsproxy: Avoid null pointer access
  dnsproxy: Fix memory leak when request timeouts
  dnsproxy: Create cache if it is missing when doing lookup

 src/connman.h  |   1 +
 src/dnsproxy.c | 494 +++--
 src/inet.c |  55 +++
 3 files changed, 393 insertions(+), 157 deletions(-)

-- 
1.7.11.7

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v2 2/8] dnsproxy: Listen only needed addresses

2013-04-04 Thread Jukka Rissanen 
Do not bind to ANY address so that other DNS server applications
can be used in the same host. This means that we only create listener
to some specific IPv4 and/or IPv6 addresses.
---
 src/dnsproxy.c | 316 -
 1 file changed, 225 insertions(+), 91 deletions(-)

diff --git a/src/dnsproxy.c b/src/dnsproxy.c
index 7a9ca91..60b1512 100644
--- a/src/dnsproxy.c
+++ b/src/dnsproxy.c
@@ -105,6 +105,7 @@ struct request_data {
socklen_t sa_len;
int client_sk;
int protocol;
+   int family;
guint16 srcid;
guint16 dstid;
guint16 altid;
@@ -123,10 +124,16 @@ struct request_data {
 
 struct listener_data {
int index;
-   GIOChannel *udp_listener_channel;
-   guint udp_listener_watch;
-   GIOChannel *tcp_listener_channel;
-   guint tcp_listener_watch;
+
+   GIOChannel *udp4_listener_channel;
+   GIOChannel *tcp4_listener_channel;
+   guint udp4_listener_watch;
+   guint tcp4_listener_watch;
+
+   GIOChannel *udp6_listener_channel;
+   GIOChannel *tcp6_listener_channel;
+   guint udp6_listener_watch;
+   guint tcp6_listener_watch;
 };
 
 struct cache_data {
@@ -468,25 +475,39 @@ static void send_response(int sk, unsigned char *buf, int 
len,
}
 }
 
+static int get_req_udp_socket(struct request_data *req)
+{
+   GIOChannel *channel;
+
+   if (req-family == AF_INET)
+   channel = req-ifdata-udp4_listener_channel;
+   else
+   channel = req-ifdata-udp6_listener_channel;
+
+   if (channel == NULL)
+   return -1;
+
+   return g_io_channel_unix_get_fd(channel);
+}
+
 static gboolean request_timeout(gpointer user_data)
 {
struct request_data *req = user_data;
-   struct listener_data *ifdata;
 
DBG(id 0x%04x, req-srcid);
 
if (req == NULL)
return FALSE;
 
-   ifdata = req-ifdata;
-
request_list = g_slist_remove(request_list, req);
req-numserv--;
 
if (req-resplen  0  req-resp != NULL) {
int sk, err;
 
-   sk = g_io_channel_unix_get_fd(ifdata-udp_listener_channel);
+   sk = get_req_udp_socket(req);
+   if (sk  0)
+   return FALSE;
 
err = sendto(sk, req-resp, req-resplen, MSG_NOSIGNAL,
req-sa, req-sa_len);
@@ -506,10 +527,12 @@ static gboolean request_timeout(gpointer user_data)
 
hdr = (void *) (req-request);
hdr-id = req-srcid;
-   sk = g_io_channel_unix_get_fd(
-   ifdata-udp_listener_channel);
-   send_response(sk, req-request, req-request_len,
-   req-sa, req-sa_len, IPPROTO_UDP);
+
+   sk = get_req_udp_socket(req);
+   if (sk = 0)
+   send_response(sk, req-request,
+   req-request_len, req-sa,
+   req-sa_len, IPPROTO_UDP);
}
}
 
@@ -1494,8 +1517,7 @@ static int ns_resolv(struct server_data *server, struct 
request_data *req,
}
 
if (data != NULL  req-protocol == IPPROTO_UDP) {
-   int udp_sk = g_io_channel_unix_get_fd(
-   req-ifdata-udp_listener_channel);
+   int udp_sk = get_req_udp_socket(req);
 
send_cached_response(udp_sk, data-data,
data-data_len, req-sa, req-sa_len,
@@ -1600,7 +1622,6 @@ static int forward_dns_reply(unsigned char *reply, int 
reply_len, int protocol,
struct domain_hdr *hdr;
struct request_data *req;
int dns_id, sk, err, offset = protocol_offset(protocol);
-   struct listener_data *ifdata;
 
if (offset  0)
return offset;
@@ -1617,8 +1638,6 @@ static int forward_dns_reply(unsigned char *reply, int 
reply_len, int protocol,
DBG(req %p dstid 0x%04x altid 0x%04x rcode %d,
req, req-dstid, req-altid, hdr-rcode);
 
-   ifdata = req-ifdata;
-
reply[offset] = req-srcid  0xff;
reply[offset + 1] = req-srcid  8;
 
@@ -1694,7 +1713,7 @@ static int forward_dns_reply(unsigned char *reply, int 
reply_len, int protocol,
request_list = g_slist_remove(request_list, req);
 
if (protocol == IPPROTO_UDP) {
-   sk = g_io_channel_unix_get_fd(ifdata-udp_listener_channel);
+   sk = get_req_udp_socket(req);
err = sendto(sk, req-resp, req-resplen, 0,
 req-sa, req-sa_len);
} else {
@@ -2515,25 +2534,29 @@ static int parse_request(unsigned char *buf, int len,
 }
 
 static gboolean tcp_listener_event(GIOChannel *channel,

[PATCH v2 4/8] dnsproxy: Use ::1 when refreshing because of the address family

2013-04-04 Thread Jukka Rissanen 
---
 src/dnsproxy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/dnsproxy.c b/src/dnsproxy.c
index f1f7e4c..77a86f7 100644
--- a/src/dnsproxy.c
+++ b/src/dnsproxy.c
@@ -303,7 +303,7 @@ static void refresh_dns_entry(struct cache_entry *entry, 
char *name)
if (ipv6_resolve == NULL) {
ipv6_resolve = g_resolv_new(0);
g_resolv_set_address_family(ipv6_resolve, AF_INET6);
-   g_resolv_add_nameserver(ipv6_resolve, 127.0.0.1, 53, 0);
+   g_resolv_add_nameserver(ipv6_resolve, ::1, 53, 0);
}
 
if (entry-ipv4 == NULL) {
-- 
1.7.11.7

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v2 3/8] dnsproxy: Add or remove ::1 to/from resolv.conf when necessary

2013-04-04 Thread Jukka Rissanen 
We add IPv6 loopback address to resolv.conf if we have created
an IPv6 listening socket to port 53. Also remove ::1 from resolv.conf
when deleting the proxy.
---
 src/dnsproxy.c | 44 +++-
 1 file changed, 35 insertions(+), 9 deletions(-)

diff --git a/src/dnsproxy.c b/src/dnsproxy.c
index 60b1512..f1f7e4c 100644
--- a/src/dnsproxy.c
+++ b/src/dnsproxy.c
@@ -2924,8 +2924,19 @@ static GIOChannel *get_listener(int family, int 
protocol, int index)
return channel;
 }
 
+#define UDP_IPv4_FAILED 0x01
+#define TCP_IPv4_FAILED 0x02
+#define UDP_IPv6_FAILED 0x04
+#define TCP_IPv6_FAILED 0x08
+#define UDP_FAILED (UDP_IPv4_FAILED | UDP_IPv6_FAILED)
+#define TCP_FAILED (TCP_IPv4_FAILED | TCP_IPv6_FAILED)
+#define IPv6_FAILED (UDP_IPv6_FAILED | TCP_IPv6_FAILED)
+#define IPv4_FAILED (UDP_IPv4_FAILED | TCP_IPv4_FAILED)
+
 static int create_dns_listener(int protocol, struct listener_data *ifdata)
 {
+   int ret = 0;
+
if (protocol == IPPROTO_TCP) {
ifdata-tcp4_listener_channel = get_listener(AF_INET, protocol,
ifdata-index);
@@ -2934,6 +2945,8 @@ static int create_dns_listener(int protocol, struct 
listener_data *ifdata)
g_io_add_watch(ifdata-tcp4_listener_channel,
G_IO_IN, tcp4_listener_event,
(gpointer)ifdata);
+   else
+   ret |= TCP_IPv4_FAILED;
 
ifdata-tcp6_listener_channel = get_listener(AF_INET6, protocol,
ifdata-index);
@@ -2942,6 +2955,8 @@ static int create_dns_listener(int protocol, struct 
listener_data *ifdata)
g_io_add_watch(ifdata-tcp6_listener_channel,
G_IO_IN, tcp6_listener_event,
(gpointer)ifdata);
+   else
+   ret |= TCP_IPv6_FAILED;
} else {
ifdata-udp4_listener_channel = get_listener(AF_INET, protocol,
ifdata-index);
@@ -2950,6 +2965,8 @@ static int create_dns_listener(int protocol, struct 
listener_data *ifdata)
g_io_add_watch(ifdata-udp4_listener_channel,
G_IO_IN, udp4_listener_event,
(gpointer)ifdata);
+   else
+   ret |= UDP_IPv4_FAILED;
 
ifdata-udp6_listener_channel = get_listener(AF_INET6, protocol,
ifdata-index);
@@ -2958,9 +2975,11 @@ static int create_dns_listener(int protocol, struct 
listener_data *ifdata)
g_io_add_watch(ifdata-udp6_listener_channel,
G_IO_IN, udp6_listener_event,
(gpointer)ifdata);
+   else
+   ret |= UDP_IPv6_FAILED;
}
 
-   return 0;
+   return ret;
 }
 
 static void destroy_udp_listener(struct listener_data *ifdata)
@@ -2995,18 +3014,23 @@ static int create_listener(struct listener_data *ifdata)
int err, index;
 
err = create_dns_listener(IPPROTO_UDP, ifdata);
-   if (err  0)
-   return err;
+   if ((err  UDP_FAILED) == UDP_FAILED)
+   return -EIO;
 
-   err = create_dns_listener(IPPROTO_TCP, ifdata);
-   if (err  0) {
+   err |= create_dns_listener(IPPROTO_TCP, ifdata);
+   if ((err  TCP_FAILED) == TCP_FAILED) {
destroy_udp_listener(ifdata);
-   return err;
+   return -EIO;
}
 
index = connman_inet_ifindex(lo);
-   if (ifdata-index == index)
-   __connman_resolvfile_append(index, NULL, 127.0.0.1);
+   if (ifdata-index == index) {
+   if ((err  IPv6_FAILED) != IPv6_FAILED)
+   __connman_resolvfile_append(index, NULL, ::1);
+
+   if ((err  IPv4_FAILED) != IPv4_FAILED)
+   __connman_resolvfile_append(index, NULL, 127.0.0.1);
+   }
 
return 0;
 }
@@ -3017,8 +3041,10 @@ static void destroy_listener(struct listener_data 
*ifdata)
GSList *list;
 
index = connman_inet_ifindex(lo);
-   if (ifdata-index == index)
+   if (ifdata-index == index) {
__connman_resolvfile_remove(index, NULL, 127.0.0.1);
+   __connman_resolvfile_remove(index, NULL, ::1);
+   }
 
for (list = request_list; list; list = list-next) {
struct request_data *req = list-data;
-- 
1.7.11.7

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v2 5/8] dnsproxy: Do not add or remove ::1 server

2013-04-04 Thread Jukka Rissanen 
---
 src/dnsproxy.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/dnsproxy.c b/src/dnsproxy.c
index 77a86f7..d54ca70 100644
--- a/src/dnsproxy.c
+++ b/src/dnsproxy.c
@@ -2327,6 +2327,9 @@ int __connman_dnsproxy_append(int index, const char 
*domain,
if (g_str_equal(server, 127.0.0.1) == TRUE)
return -ENODEV;
 
+   if (g_str_equal(server, ::1) == TRUE)
+   return -ENODEV;
+
data = find_server(index, server, IPPROTO_UDP);
if (data != NULL) {
append_domain(index, domain);
@@ -2363,6 +2366,9 @@ int __connman_dnsproxy_remove(int index, const char 
*domain,
if (g_str_equal(server, 127.0.0.1) == TRUE)
return -ENODEV;
 
+   if (g_str_equal(server, ::1) == TRUE)
+   return -ENODEV;
+
remove_server(index, domain, server, IPPROTO_UDP);
remove_server(index, domain, server, IPPROTO_TCP);
 
-- 
1.7.11.7

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v2 7/8] dnsproxy: Fix memory leak when request timeouts

2013-04-04 Thread Jukka Rissanen 
---
 src/dnsproxy.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/dnsproxy.c b/src/dnsproxy.c
index 9bea3b1..04b4c79 100644
--- a/src/dnsproxy.c
+++ b/src/dnsproxy.c
@@ -490,6 +490,17 @@ static int get_req_udp_socket(struct request_data *req)
return g_io_channel_unix_get_fd(channel);
 }
 
+static void destroy_request_data(struct request_data *req)
+{
+   if (req-timeout  0)
+   g_source_remove(req-timeout);
+
+   g_free(req-resp);
+   g_free(req-request);
+   g_free(req-name);
+   g_free(req);
+}
+
 static gboolean request_timeout(gpointer user_data)
 {
struct request_data *req = user_data;
@@ -536,8 +547,8 @@ static gboolean request_timeout(gpointer user_data)
}
}
 
-   g_free(req-resp);
-   g_free(req);
+   req-timeout = 0;
+   destroy_request_data(req);
 
return FALSE;
 }
@@ -1605,17 +1616,6 @@ static int ns_resolv(struct server_data *server, struct 
request_data *req,
return 0;
 }
 
-static void destroy_request_data(struct request_data *req)
-{
-   if (req-timeout  0)
-   g_source_remove(req-timeout);
-
-   g_free(req-resp);
-   g_free(req-request);
-   g_free(req-name);
-   g_free(req);
-}
-
 static int forward_dns_reply(unsigned char *reply, int reply_len, int protocol,
struct server_data *data)
 {
-- 
1.7.11.7

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v2 6/8] dnsproxy: Avoid null pointer access

2013-04-04 Thread Jukka Rissanen 
---
 src/dnsproxy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/dnsproxy.c b/src/dnsproxy.c
index d54ca70..9bea3b1 100644
--- a/src/dnsproxy.c
+++ b/src/dnsproxy.c
@@ -494,11 +494,11 @@ static gboolean request_timeout(gpointer user_data)
 {
struct request_data *req = user_data;
 
-   DBG(id 0x%04x, req-srcid);
-
if (req == NULL)
return FALSE;
 
+   DBG(id 0x%04x, req-srcid);
+
request_list = g_slist_remove(request_list, req);
req-numserv--;
 
-- 
1.7.11.7

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v2 8/8] dnsproxy: Create cache if it is missing when doing lookup

2013-04-04 Thread Jukka Rissanen 
---
 src/dnsproxy.c | 98 +-
 1 file changed, 56 insertions(+), 42 deletions(-)

diff --git a/src/dnsproxy.c b/src/dnsproxy.c
index 04b4c79..8366fa4 100644
--- a/src/dnsproxy.c
+++ b/src/dnsproxy.c
@@ -701,6 +701,51 @@ static uint16_t cache_check_validity(char *question, 
uint16_t type,
return type;
 }
 
+static void cache_element_destroy(gpointer value)
+{
+   struct cache_entry *entry = value;
+
+   if (entry == NULL)
+   return;
+
+   if (entry-ipv4 != NULL) {
+   g_free(entry-ipv4-data);
+   g_free(entry-ipv4);
+   }
+
+   if (entry-ipv6 != NULL) {
+   g_free(entry-ipv6-data);
+   g_free(entry-ipv6);
+   }
+
+   g_free(entry-key);
+   g_free(entry);
+
+   if (--cache_size  0)
+   cache_size = 0;
+}
+
+static gboolean try_remove_cache(gpointer user_data)
+{
+   if (__sync_fetch_and_sub(cache_refcount, 1) == 1) {
+   DBG(No cache users, removing it.);
+
+   g_hash_table_destroy(cache);
+   cache = NULL;
+   }
+
+   return FALSE;
+}
+
+static void create_cache()
+{
+   if (__sync_fetch_and_add(cache_refcount, 1) == 0)
+   cache = g_hash_table_new_full(g_str_hash,
+   g_str_equal,
+   NULL,
+   cache_element_destroy);
+}
+
 static struct cache_entry *cache_check(gpointer request, int *qtype, int proto)
 {
char *question;
@@ -726,6 +771,11 @@ static struct cache_entry *cache_check(gpointer request, 
int *qtype, int proto)
if (type != 1  type != 28)
return NULL;
 
+   if (cache == NULL) {
+   create_cache();
+   return NULL;
+   }
+
entry = g_hash_table_lookup(cache, question);
if (entry == NULL)
return NULL;
@@ -1332,7 +1382,11 @@ static int cache_update(struct server_data *srv, 
unsigned char *msg,
if ((err == -ENOMSG || err == -ENOBUFS) 
reply_query_type(msg + offset,
msg_len - offset) == 28) {
-   entry = g_hash_table_lookup(cache, question);
+   if (cache == NULL) {
+   create_cache();
+   entry = NULL;
+   } else
+   entry = g_hash_table_lookup(cache, question);
if (entry  entry-ipv4  entry-ipv6 == NULL) {
int cache_offset = 0;
 
@@ -1733,42 +1787,6 @@ static int forward_dns_reply(unsigned char *reply, int 
reply_len, int protocol,
return err;
 }
 
-static void cache_element_destroy(gpointer value)
-{
-   struct cache_entry *entry = value;
-
-   if (entry == NULL)
-   return;
-
-   if (entry-ipv4 != NULL) {
-   g_free(entry-ipv4-data);
-   g_free(entry-ipv4);
-   }
-
-   if (entry-ipv6 != NULL) {
-   g_free(entry-ipv6-data);
-   g_free(entry-ipv6);
-   }
-
-   g_free(entry-key);
-   g_free(entry);
-
-   if (--cache_size  0)
-   cache_size = 0;
-}
-
-static gboolean try_remove_cache(gpointer user_data)
-{
-   if (__sync_fetch_and_sub(cache_refcount, 1) == 1) {
-   DBG(No cache users, removing it.);
-
-   g_hash_table_destroy(cache);
-   cache = NULL;
-   }
-
-   return FALSE;
-}
-
 static void server_destroy_socket(struct server_data *data)
 {
DBG(index %d server %s proto %d, data-index,
@@ -2138,11 +2156,7 @@ static int server_create_socket(struct server_data *data)
}
}
 
-   if (__sync_fetch_and_add(cache_refcount, 1) == 0)
-   cache = g_hash_table_new_full(g_str_hash,
-   g_str_equal,
-   NULL,
-   cache_element_destroy);
+   create_cache();
 
return 0;
 }
-- 
1.7.11.7

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH 0/2] DNS proxy must handle partial client request from TCP

2013-04-04 Thread Jukka Rissanen 
Hi,

as reported in the ml, ConnMan can get stuck if a client is not sending
full DNS request when using TCP.
I added also some unit tests for DNS proxy. These tests require that
ConnMan is running so they cannot be run automatically during distcheck.

These patches require the
PATCH v2 0/8] DNS proxy should listen only needed addresses
patchset as a base.

There could be bugs lurking in the code so everyone, please do some testing.

Cheers,
Jukka


Jukka Rissanen (2):
  dnsproxy: Handle partial TCP messages from client
  unit: Add unit test for dnsproxy

 Makefile.am  |   6 +-
 src/dnsproxy.c   | 475 ---
 unit/test-dnsproxy.c | 463 +
 3 files changed, 878 insertions(+), 66 deletions(-)
 create mode 100644 unit/test-dnsproxy.c

-- 
1.7.11.7

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH 2/2] unit: Add unit test for dnsproxy

2013-04-04 Thread Jukka Rissanen 
---
 Makefile.am  |   6 +-
 unit/test-dnsproxy.c | 463 +++
 2 files changed, 468 insertions(+), 1 deletion(-)
 create mode 100644 unit/test-dnsproxy.c

diff --git a/Makefile.am b/Makefile.am
index 78b1b33..c4c72bd 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -240,7 +240,8 @@ client_connmanctl_SOURCES =  $(gdbus_sources) src/connman.h 
\
 client_connmanctl_LDADD = @DBUS_LIBS@ @GLIB_LIBS@ -lreadline -ldl
 endif
 
-noinst_PROGRAMS += unit/test-pbkdf2-sha1 unit/test-prf-sha1 unit/test-ippool
+noinst_PROGRAMS += unit/test-pbkdf2-sha1 unit/test-prf-sha1 unit/test-ippool \
+   unit/test-dnsproxy
 
 unit_test_pbkdf2_sha1_SOURCES = unit/test-pbkdf2-sha1.c \
src/shared/sha1.h src/shared/sha1.c
@@ -254,6 +255,9 @@ unit_test_ippool_SOURCES = $(gdbus_sources) src/log.c 
src/dbus.c \
 src/ippool.c unit/test-ippool.c
 unit_test_ippool_LDADD = @GLIB_LIBS@ @DBUS_LIBS@ -ldl
 
+unit_test_dnsproxy_SOURCES = unit/test-dnsproxy.c
+unit_test_dnsproxy_LDADD = @GLIB_LIBS@
+
 TESTS = unit/test-pbkdf2-sha1 unit/test-prf-sha1 unit/test-ippool
 
 if WISPR
diff --git a/unit/test-dnsproxy.c b/unit/test-dnsproxy.c
new file mode 100644
index 000..1d0e66d
--- /dev/null
+++ b/unit/test-dnsproxy.c
@@ -0,0 +1,463 @@
+/*
+ *
+ *  Connection Manager
+ *
+ *  Copyright (C) 2013  Intel Corporation. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include config.h
+#endif
+
+#include errno.h
+#include stdlib.h
+#include string.h
+#include unistd.h
+#include arpa/inet.h
+#include netinet/in.h
+#include sys/types.h
+#include sys/socket.h
+#include netdb.h
+#include resolv.h
+#include glib.h
+#include fcntl.h
+
+#if 0
+#define DEBUG
+#endif
+#ifdef DEBUG
+#include stdio.h
+
+#define LOG(fmt, arg...) do { \
+   fprintf(stdout, %s:%s()  fmt \n,   \
+   __FILE__, __func__ , ## arg); \
+} while (0)
+#else
+#define LOG(fmt, arg...)
+#endif
+
+unsigned char msg[] = {
+   0x00, 0x1c, /* len 28 */
+   0x31, 0x82, /* tran id */
+   0x01, 0x00, /* flags (recursion required) */
+   0x00, 0x01, /* questions (1) */
+   0x00, 0x00, /* answer rr */
+   0x00, 0x00, /* authority rr */
+   0x00, 0x00, /* additional rr */
+   0x06, 0x6c, 0x6f, 0x6c, 0x67, 0x65, 0x30, /* lolge0, just some name */
+   0x03, 0x63, 0x6f, 0x6d, /* com */
+   0x00,   /* null terminator */
+   0x00, 0x01, /* type A */
+   0x00, 0x01, /* class IN */
+};
+
+unsigned char msg2[] = {
+   0x00, 0x1c, /* len 28 */
+   0x31, 0x83, /* tran id */
+   0x01, 0x00, /* flags (recursion required) */
+   0x00, 0x01, /* questions (1) */
+   0x00, 0x00, /* answer rr */
+   0x00, 0x00, /* authority rr */
+   0x00, 0x00, /* additional rr */
+   0x06, 0x6c, 0x6f, 0x67, 0x6c, 0x67, 0x65, /* loelge */
+   0x03, 0x63, 0x6f, 0x6d, /* com */
+   0x00,   /* null terminator */
+   0x00, 0x01, /* type A */
+   0x00, 0x01, /* class IN */
+
+   0x00, 0x1c, /* len 28 */
+   0x31, 0x84, /* tran id */
+   0x01, 0x00, /* flags (recursion required) */
+   0x00, 0x01, /* questions (1) */
+   0x00, 0x00, /* answer rr */
+   0x00, 0x00, /* authority rr */
+   0x00, 0x00, /* additional rr */
+   0x06, 0x6c, 0x6f, 0x67, 0x6c, 0x67, 0x65, /* loelge */
+   0x03, 0x6e, 0x65, 0x74, /* net */
+   0x00,   /* null terminator */
+   0x00, 0x01, /* type A */
+   0x00, 0x01, /* class IN */
+};
+
+unsigned char msg_invalid[] = {
+   0x00, 0x1c, /* len 28 */
+   0x31, 0xC0, /* tran id */
+};
+
+static int create_tcp_socket(int family)
+{
+   int sk, err;
+
+   sk = socket(family, SOCK_STREAM, IPPROTO_TCP);
+   if (sk  0) {
+   err = errno;
+   LOG(Failed to create TCP socket %d/%s, err, strerror(err));
+   return -err;
+   }
+
+   return sk;
+}
+
+static int create_udp_socket(int family)
+{
+   int sk, err;
+
+   sk = socket(family, SOCK_DGRAM, IPPROTO_UDP);
+   if (sk  0) {
+   err = errno;
+   LOG(Failed to create UDP socket %d/%s, err, strerror(err));
+   return -err;
+   }
+
+   return sk;
+}
+
+static int connect_tcp_socket(char *server)
+{
+   int sk;

[PATCH 1/2] dnsproxy: Handle partial TCP messages from client

2013-04-04 Thread Jukka Rissanen 
We were not handling client sent partial TCP messages correctly.
This meant that ConnMan could block if the client using TCP would
not send full DNS request.
---
 src/dnsproxy.c | 475 +
 1 file changed, 410 insertions(+), 65 deletions(-)

diff --git a/src/dnsproxy.c b/src/dnsproxy.c
index 8366fa4..388ea4b 100644
--- a/src/dnsproxy.c
+++ b/src/dnsproxy.c
@@ -32,6 +32,7 @@
 #include netinet/in.h
 #include sys/types.h
 #include sys/socket.h
+#include fcntl.h
 #include netdb.h
 #include resolv.h
 #include gweb/gresolv.h
@@ -136,6 +137,20 @@ struct listener_data {
guint tcp6_listener_watch;
 };
 
+/*
+ * The TCP client requires some extra handling as we need to
+ * be prepared to receive also partial DNS requests.
+ */
+struct tcp_partial_client_data {
+   int family;
+   struct listener_data *ifdata;
+   GIOChannel *channel;
+   guint watch;
+   unsigned char *buf;
+   unsigned int buf_end;
+   guint timeout;
+};
+
 struct cache_data {
time_t inserted;
time_t valid_until;
@@ -168,6 +183,11 @@ struct domain_rr {
 } __attribute__ ((packed));
 
 /*
+ * Max length of the DNS TCP packet.
+ */
+#define TCP_MAX_BUF_LEN 4096
+
+/*
  * We limit how long the cached DNS entry stays in the cache.
  * By default the TTL (time-to-live) of the DNS response is used
  * when setting the cache entry life time. The value is in seconds.
@@ -194,6 +214,7 @@ static GSList *server_list = NULL;
 static GSList *request_list = NULL;
 static GHashTable *listener_table = NULL;
 static time_t next_refresh;
+static GHashTable *partial_tcp_req_table;
 
 static guint16 get_id()
 {
@@ -516,12 +537,19 @@ static gboolean request_timeout(gpointer user_data)
if (req-resplen  0  req-resp != NULL) {
int sk, err;
 
-   sk = get_req_udp_socket(req);
-   if (sk  0)
-   return FALSE;
+   if (req-protocol == IPPROTO_UDP) {
+   sk = get_req_udp_socket(req);
+   if (sk  0)
+   return FALSE;
 
-   err = sendto(sk, req-resp, req-resplen, MSG_NOSIGNAL,
-   req-sa, req-sa_len);
+   err = sendto(sk, req-resp, req-resplen, MSG_NOSIGNAL,
+   req-sa, req-sa_len);
+   } else {
+   sk = req-client_sk;
+   err = send(sk, req-resp, req-resplen, MSG_NOSIGNAL);
+   if (err  0)
+   close(sk);
+   }
if (err  0)
return FALSE;
} else if (req-request  req-numserv == 0) {
@@ -547,6 +575,16 @@ static gboolean request_timeout(gpointer user_data)
}
}
 
+   /*
+* We cannot leave TCP client hanging so just kick it out
+* if we get a request timeout from server.
+*/
+   if (req-protocol == IPPROTO_TCP) {
+   DBG(client %d removed, req-client_sk);
+   g_hash_table_remove(partial_tcp_req_table,
+   GINT_TO_POINTER(req-client_sk));
+   }
+
req-timeout = 0;
destroy_request_data(req);
 
@@ -1773,7 +1811,6 @@ static int forward_dns_reply(unsigned char *reply, int 
reply_len, int protocol,
} else {
sk = req-client_sk;
err = send(sk, req-resp, req-resplen, MSG_NOSIGNAL);
-   close(sk);
}
 
if (err  0)
@@ -2553,64 +2590,95 @@ static int parse_request(unsigned char *buf, int len,
return 0;
 }
 
-static gboolean tcp_listener_event(GIOChannel *channel, GIOCondition condition,
-   struct listener_data *ifdata, int family,
-   guint *listener_watch)
+static void client_reset(struct tcp_partial_client_data *client)
 {
-   unsigned char buf[768];
-   char query[512];
+   if (client == NULL)
+   return;
+
+   if (client-channel != NULL) {
+   DBG(client %d closing,
+   g_io_channel_unix_get_fd(client-channel));
+
+   g_io_channel_unref(client-channel);
+   client-channel = NULL;
+   }
+
+   if (client-watch  0) {
+   g_source_remove(client-watch);
+   client-watch = 0;
+   }
+
+   if (client-timeout  0) {
+   g_source_remove(client-timeout);
+   client-timeout = 0;
+   }
+
+   g_free(client-buf);
+   client-buf = NULL;
+
+   client-buf_end = 0;
+}
+
+static unsigned int get_msg_len(unsigned char *buf)
+{
+   return buf[0]8 | buf[1];
+}
+
+static gboolean read_tcp_data(struct tcp_partial_client_data *client,
+   void *client_addr, socklen_t client_addr_len,
+   int read_len)
+{
+   char

Re: [PATCH 1/2] dnsproxy: Handle partial TCP messages from client

2013-04-04 Thread Daniel Wagner 

Hi Jukka,

On 04/04/2013 01:46 PM, Jukka Rissanen wrote:

We were not handling client sent partial TCP messages correctly.
This meant that ConnMan could block if the client using TCP would
not send full DNS request.


Could you describe how you solved it? This would make it simpler to 
review it.


cheers,
daniel
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [PATCH 2/2] unit: Add unit test for dnsproxy

2013-04-04 Thread Daniel Wagner 

Hi Jukka,

On 04/04/2013 01:46 PM, Jukka Rissanen wrote:

---
  Makefile.am  |   6 +-
  unit/test-dnsproxy.c | 463 +++
  2 files changed, 468 insertions(+), 1 deletion(-)
  create mode 100644 unit/test-dnsproxy.c

diff --git a/Makefile.am b/Makefile.am
index 78b1b33..c4c72bd 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -240,7 +240,8 @@ client_connmanctl_SOURCES =  $(gdbus_sources) src/connman.h 
\
  client_connmanctl_LDADD = @DBUS_LIBS@ @GLIB_LIBS@ -lreadline -ldl
  endif

-noinst_PROGRAMS += unit/test-pbkdf2-sha1 unit/test-prf-sha1 unit/test-ippool
+noinst_PROGRAMS += unit/test-pbkdf2-sha1 unit/test-prf-sha1 unit/test-ippool \
+   unit/test-dnsproxy


If I read this patch correct, test-dnsproxy expects that the ConnMan is 
running in the background. I think in this case it should be part of tools.


cheers,
daniel
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [PATCH 2/2] unit: Add unit test for dnsproxy

2013-04-04 Thread Jukka Rissanen 

Hi Daniel,

On 04.04.2013 15:05, Daniel Wagner wrote:

Hi Jukka,

On 04/04/2013 01:46 PM, Jukka Rissanen wrote:

---
  Makefile.am  |   6 +-
  unit/test-dnsproxy.c | 463
+++
  2 files changed, 468 insertions(+), 1 deletion(-)
  create mode 100644 unit/test-dnsproxy.c

diff --git a/Makefile.am b/Makefile.am
index 78b1b33..c4c72bd 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -240,7 +240,8 @@ client_connmanctl_SOURCES =  $(gdbus_sources)
src/connman.h \
  client_connmanctl_LDADD = @DBUS_LIBS@ @GLIB_LIBS@ -lreadline -ldl
  endif

-noinst_PROGRAMS += unit/test-pbkdf2-sha1 unit/test-prf-sha1
unit/test-ippool
+noinst_PROGRAMS += unit/test-pbkdf2-sha1 unit/test-prf-sha1
unit/test-ippool \
+unit/test-dnsproxy


If I read this patch correct, test-dnsproxy expects that the ConnMan is
running in the background. I think in this case it should be part of tools.


Hmm, ok. We are not running the tests in make distcheck as the 
test-dnsproxy is not added to TESTS. Anyway, I can certainly move the 
file to tools directory if needed.



Cheers,
Jukka

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [PATCH 1/2] dnsproxy: Handle partial TCP messages from client

2013-04-04 Thread Jukka Rissanen 

Hi Daniel,

On 04.04.2013 15:02, Daniel Wagner wrote:

Hi Jukka,

On 04/04/2013 01:46 PM, Jukka Rissanen wrote:

We were not handling client sent partial TCP messages correctly.
This meant that ConnMan could block if the client using TCP would
not send full DNS request.


Could you describe how you solved it? This would make it simpler to
review it.



Just making the client reading socket non blocking and reading bytes 
until we get a full dns request. There is also a timeout (2s) if client 
is not sending data fast enough.
The patch is actually very simple but looks complicated because 
dnsproxy.c is quite complex piece of code.



Cheers,
Jukka

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Re: [PATCH 2/2] unit: Add unit test for dnsproxy

2013-04-04 Thread Patrik Flykt 
On Thu, 2013-04-04 at 14:05 +0200, Daniel Wagner wrote:
 Hi Jukka,
 
 On 04/04/2013 01:46 PM, Jukka Rissanen wrote:
  ---
Makefile.am  |   6 +-
unit/test-dnsproxy.c | 463 
  +++
2 files changed, 468 insertions(+), 1 deletion(-)
create mode 100644 unit/test-dnsproxy.c
 
  diff --git a/Makefile.am b/Makefile.am
  index 78b1b33..c4c72bd 100644
  --- a/Makefile.am
  +++ b/Makefile.am
  @@ -240,7 +240,8 @@ client_connmanctl_SOURCES =  $(gdbus_sources) 
  src/connman.h \
client_connmanctl_LDADD = @DBUS_LIBS@ @GLIB_LIBS@ -lreadline -ldl
endif
 
  -noinst_PROGRAMS += unit/test-pbkdf2-sha1 unit/test-prf-sha1 
  unit/test-ippool
  +noinst_PROGRAMS += unit/test-pbkdf2-sha1 unit/test-prf-sha1 
  unit/test-ippool \
  +   unit/test-dnsproxy
 
 If I read this patch correct, test-dnsproxy expects that the ConnMan is 
 running in the background. I think in this case it should be part of tools.

Yep, this should go into /tools.

Cheers,

Patrik

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH] service: Drop unused argument in preferred_tech_list_get()

2013-04-04 Thread Daniel Wagner 
From: Daniel Wagner daniel.wag...@bmw-carit.de

---
 src/service.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/service.c b/src/service.c
index 469140a..f650402 100644
--- a/src/service.c
+++ b/src/service.c
@@ -3425,7 +3425,7 @@ static void preferred_tech_add_by_type(gpointer data, 
gpointer user_data)
}
 }
 
-static GSequence* preferred_tech_list_get(GSequence *list)
+static GSequence* preferred_tech_list_get(void)
 {
unsigned int *tech_array;
struct preferred_tech_data tech_data;
@@ -3520,7 +3520,7 @@ static gboolean run_auto_connect(gpointer data)
 
DBG();
 
-   preferred_tech = preferred_tech_list_get(service_list);
+   preferred_tech = preferred_tech_list_get();
if (preferred_tech != NULL)
iter = g_sequence_get_begin_iter(preferred_tech);
 
-- 
1.8.2.rc3.16.gce432ca

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v1 2/7] session: Do not access initialized hash table

2013-04-04 Thread Daniel Wagner 
From: Daniel Wagner daniel.wag...@bmw-carit.de

Make sure we never try to cleanup when the hash table has
been destroyed. In this case all resources have been
freed already.
---
 src/session.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/session.c b/src/session.c
index 6916458..5f5255e 100644
--- a/src/session.c
+++ b/src/session.c
@@ -359,6 +359,9 @@ static void remove_policy(struct connman_session_policy 
*policy)
gpointer key, value;
struct connman_session *session;
 
+   if (session_hash == NULL)
+   return;
+
DBG(policy %p name %s, policy, policy-name);
 
g_hash_table_iter_init(iter, session_hash);
-- 
1.8.2.rc3.16.gce432ca

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v1 1/7] session: Reorder shutdown sequence

2013-04-04 Thread Daniel Wagner 
From: Daniel Wagner daniel.wag...@bmw-carit.de

By calling __connman_session_cleanup() before __connman_plugin_cleanup()
we make sure all resources allocated can released in the correct order.
The code assumes after an successful allocation, free will always work
and therefore we double free allocated memory.
---
 src/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/main.c b/src/main.c
index 0f47943..a76ec3b 100644
--- a/src/main.c
+++ b/src/main.c
@@ -676,11 +676,11 @@ int main(int argc, char *argv[])
__connman_wpad_cleanup();
__connman_dhcpv6_cleanup();
__connman_dhcp_cleanup();
+   __connman_session_cleanup();
__connman_plugin_cleanup();
__connman_provider_cleanup();
__connman_connection_cleanup();
__connman_timeserver_cleanup();
-   __connman_session_cleanup();
__connman_detect_cleanup();
__connman_proxy_cleanup();
__connman_task_cleanup();
-- 
1.8.2.rc3.16.gce432ca

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v1 0/7] Session and Policy Plugin Fixes

2013-04-04 Thread Daniel Wagner 
From: Daniel Wagner daniel.wag...@bmw-carit.de

Hi,

Let's have the next batch of patches from the 'per application routing
and staticics' series.

This version of series changes slightly the order of the patches
and the previouly patch called

[PATCH v0 4/5] session_policy_local: Do not free policy on load error

is splitted into three patches


  session: Reorder shutdown sequence
  session: Do not access initialized hash table

These patches are fixing crashes I was able to trigger via
ctrl-c while a session was activated.

  session: Empty policy list indicated no match all
  session_policy_local: Set default policy using common code
  session_policy_local: Empty policy list indicated no match all

Fix the semantics of an empty AllowedBearers list. The D-Bus
AllowedBearers API does handles this correctly.

  session_policy_local: Do not free policy on load error
  session_policy_local: Load policy when a new file is added

These fix the parsing and freeing of the policy files.

If you want to see where we are heading have a look at the
whole series here:

https://github.com/bmwcarit/connman/tree/secmark-v3

cheers,
daniel

Daniel Wagner (7):
  session: Reorder shutdown sequence
  session: Do not access initialized hash table
  session: Empty policy list indicated no match all
  session_policy_local: Set default policy using common code
  session_policy_local: Empty policy list indicated no match all
  session_policy_local: Do not free policy on load error
  session_policy_local: Load policy when a new file is added

 plugins/session_policy_local.c | 31 +--
 src/main.c |  2 +-
 src/session.c  |  5 -
 3 files changed, 22 insertions(+), 16 deletions(-)

-- 
1.8.2.rc3.16.gce432ca

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v1 3/7] session: Empty policy list indicated no match all

2013-04-04 Thread Daniel Wagner 
From: Daniel Wagner daniel.wag...@bmw-carit.de

Match all is expressed via CONNMAN_SERVICE_TYPE_UNKNOWN. An empty
list means no match.
---
 src/session.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/session.c b/src/session.c
index 5f5255e..d06edc8 100644
--- a/src/session.c
+++ b/src/session.c
@@ -517,7 +517,7 @@ static int filter_bearer(GSList *policy_bearers,
GSList *it;
 
if (policy_bearers == NULL)
-   goto clone;
+   return 0;
 
for (it = policy_bearers; it != NULL; it = it-next) {
policy = GPOINTER_TO_INT(it-data);
-- 
1.8.2.rc3.16.gce432ca

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v1 4/7] session_policy_local: Set default policy using common code

2013-04-04 Thread Daniel Wagner 
From: Daniel Wagner daniel.wag...@bmw-carit.de

Use connman_sessoin_set_default_config() instead of open coded
version.

This prepars the next fix.
---
 plugins/session_policy_local.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/plugins/session_policy_local.c b/plugins/session_policy_local.c
index 5a8f6b8..ecaf15c 100644
--- a/plugins/session_policy_local.c
+++ b/plugins/session_policy_local.c
@@ -294,6 +294,8 @@ static int load_policy(struct policy_data *policy)
char *str, **tokens;
int i, err = 0;
 
+   connman_session_set_default_config(config);
+
pathname = g_strdup_printf(%s/%s, POLICYDIR, policy-ident);
if(pathname == NULL)
return -ENOMEM;
@@ -318,8 +320,6 @@ static int load_policy(struct policy_data *policy)
if (str != NULL) {
config-roaming_policy = 
connman_session_parse_roaming_policy(str);
g_free(str);
-   } else {
-   config-roaming_policy = CONNMAN_SESSION_ROAMING_POLICY_DEFAULT;
}
 
str = g_key_file_get_string(keyfile, Default, ConnectionType,
@@ -327,8 +327,6 @@ static int load_policy(struct policy_data *policy)
if (str != NULL) {
config-type = connman_session_parse_connection_type(str);
g_free(str);
-   } else {
-   config-type = CONNMAN_SESSION_TYPE_ANY;
}
 
config-ecall = g_key_file_get_boolean(keyfile, Default,
-- 
1.8.2.rc3.16.gce432ca

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v1 5/7] session_policy_local: Empty policy list indicated no match all

2013-04-04 Thread Daniel Wagner 
From: Daniel Wagner daniel.wag...@bmw-carit.de

Match all is expressed via CONNMAN_SERVICE_TYPE_UNKNOWN. An empty
list means no match.
---
 plugins/session_policy_local.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/plugins/session_policy_local.c b/plugins/session_policy_local.c
index ecaf15c..fa8349f 100644
--- a/plugins/session_policy_local.c
+++ b/plugins/session_policy_local.c
@@ -332,13 +332,13 @@ static int load_policy(struct policy_data *policy)
config-ecall = g_key_file_get_boolean(keyfile, Default,
EmergencyCall, NULL);
 
-   g_slist_free(config-allowed_bearers);
-   config-allowed_bearers = NULL;
-
str = g_key_file_get_string(keyfile, Default, AllowedBearers,
NULL);
 
if (str != NULL) {
+   g_slist_free(config-allowed_bearers);
+   config-allowed_bearers = NULL;
+
tokens = g_strsplit(str,  , 0);
 
for (i = 0; tokens[i] != NULL; i++) {
@@ -350,11 +350,6 @@ static int load_policy(struct policy_data *policy)
 
g_free(str);
g_strfreev(tokens);
-   } else {
-   config-allowed_bearers = g_slist_append(NULL,
-   GINT_TO_POINTER(CONNMAN_SERVICE_TYPE_UNKNOWN));
-   if (config-allowed_bearers == NULL)
-   err = -ENOMEM;
}
 
g_key_file_free(keyfile);
-- 
1.8.2.rc3.16.gce432ca

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v1 6/7] session_policy_local: Do not free policy on load error

2013-04-04 Thread Daniel Wagner 
From: Daniel Wagner daniel.wag...@bmw-carit.de

We should not unref the policy on parsing errors becase the policy
lifetime is attached to the lifetime of the file. When the file
is removed we remove it from the hash table.
---
 plugins/session_policy_local.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/plugins/session_policy_local.c b/plugins/session_policy_local.c
index fa8349f..aa734a1 100644
--- a/plugins/session_policy_local.c
+++ b/plugins/session_policy_local.c
@@ -390,6 +390,7 @@ static void notify_handler(struct inotify_event *event,
 const char *ident)
 {
struct policy_data *policy;
+   int err;
 
if (ident == NULL)
return;
@@ -411,8 +412,10 @@ static void notify_handler(struct inotify_event *event,
if (event-mask  IN_MODIFY) {
connman_info(Policy modifed for '%s', ident);
 
-   if (load_policy(policy)  0) {
-   remove_policy(policy);
+   err = load_policy(policy);
+   if (err  0) {
+   connman_warn(Loading policy file '%s' failed with %s,
+   ident, strerror(-err));
return;
}
}
-- 
1.8.2.rc3.16.gce432ca

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

[PATCH v1 7/7] session_policy_local: Load policy when a new file is added

2013-04-04 Thread Daniel Wagner 
From: Daniel Wagner daniel.wag...@bmw-carit.de

When a file is added (not modified) we need to parse it as well
---
 plugins/session_policy_local.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/plugins/session_policy_local.c b/plugins/session_policy_local.c
index aa734a1..6a527cb 100644
--- a/plugins/session_policy_local.c
+++ b/plugins/session_policy_local.c
@@ -404,6 +404,13 @@ static void notify_handler(struct inotify_event *event,
policy_ref(policy);
else
policy = create_policy(ident);
+
+   err = load_policy(policy);
+   if (err  0) {
+   connman_warn(Loading policy file '%s' failed with %s,
+   ident, strerror(-err));
+   return;
+   }
}
 
if (policy == NULL)
-- 
1.8.2.rc3.16.gce432ca

___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman