[PATCH 0/2] DNS queries without domain part fail

2014-07-04 Thread Jukka Rissanen
Hi,

The commit fa7141ae55aad fixed the dns queries without
domain part. Unfortunately there was a bug in that commit
which is now fixed by patch 1. The labels in the dns query
were uncompressed which changes the label length bytes
into dots. These dots need to be converted back to
length bytes but that was not done. This is only seen
if the user is querying without domain name part.

Patch 2 fixes a nastier issue. The old code stripped
the domain part away from the dns question part but left
the FQDN into dns answers. This works ok for glibc users
of getaddrinfo() (like traceroute) but fails for
gethostbyname() users (like ping) which does some extra
checks and expects that answers contain only host part
without domain name.


Thanks to Pasi Sjöholm for reporting this.

Cheers,
Jukka


Jukka Rissanen (2):
  dnsproxy: Uncompressed label was not set to internal format
  dnsproxy: Remove domain part from answers

 src/dnsproxy.c | 85 +++---
 1 file changed, 69 insertions(+), 16 deletions(-)

-- 
1.8.3.1

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

[PATCH 1/2] dnsproxy: Uncompressed label was not set to internal format

2014-07-04 Thread Jukka Rissanen
The label was uncompressed by dn_expand() which changes label
lengths to dots but the dots were not set back to length bytes
later.
---
 src/dnsproxy.c | 13 +++--
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/src/dnsproxy.c b/src/dnsproxy.c
index 7232b98..77aabd7 100644
--- a/src/dnsproxy.c
+++ b/src/dnsproxy.c
@@ -1760,14 +1760,11 @@ static char *uncompress(int16_t field_count, char 
*start, char *end,
int pos;/* position in compressed string */
char name[NS_MAXLABEL]; /* tmp label */
uint16_t dns_type, dns_class;
+   int comp_pos;
 
-   pos = dn_expand((const u_char *)start, (u_char *)end,
-   (u_char *)ptr, name, NS_MAXLABEL);
-   if (pos  0) {
-   DBG(uncompress error [%d/%s], errno,
-   strerror(errno));
+   if (!convert_label(start, end, ptr, name, NS_MAXLABEL,
+   pos, comp_pos))
goto out;
-   }
 
/*
 * Copy the uncompressed resource record, type, class and \0 to
@@ -1775,7 +1772,6 @@ static char *uncompress(int16_t field_count, char *start, 
char *end,
 */
 
ulen = strlen(name);
-   *uptr++ = ulen;
strncpy(uptr, name, uncomp_len - (uptr - uncompressed));
 
DBG(pos %d ulen %d left %d name %s, pos, ulen,
@@ -1807,8 +1803,6 @@ static char *uncompress(int16_t field_count, char *start, 
char *end,
 * so we need to uncompress it also when necessary.
 */
if (dns_type == ns_t_cname) {
-   int comp_pos;
-
if (!convert_label(start, end, ptr, uptr,
uncomp_len - (uptr - uncompressed),
pos, comp_pos))
@@ -1833,7 +1827,6 @@ static char *uncompress(int16_t field_count, char *start, 
char *end,
ptr += dlen;
 
} else if (dns_type == ns_t_soa) {
-   int comp_pos;
int total_len = 0;
char *len_ptr;
 
-- 
1.8.3.1

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


[PATCH 2/2] dnsproxy: Remove domain part from answers

2014-07-04 Thread Jukka Rissanen
If user resolver tries to resolve a name without domain part,
then glibc gethostbyname() expects that returned answers do
not contain any domain name either. The getaddrinfo() works
differently and accepts FQDN in answers.
---
 src/dnsproxy.c | 72 +-
 1 file changed, 66 insertions(+), 6 deletions(-)

diff --git a/src/dnsproxy.c b/src/dnsproxy.c
index 77aabd7..2bf1690 100644
--- a/src/dnsproxy.c
+++ b/src/dnsproxy.c
@@ -1877,6 +1877,45 @@ out:
return NULL;
 }
 
+static int strip_domains(char *name, char *answers, int maxlen)
+{
+   uint16_t data_len;
+   int name_len = strlen(name);
+   char *ptr, *start = answers, *end = answers + maxlen;
+
+   while (maxlen  0) {
+   ptr = strstr(answers, name);
+   if (ptr) {
+   char *domain = ptr + name_len;
+
+   if (*domain) {
+   int domain_len = strlen(domain);
+
+   memmove(answers + name_len,
+   domain + domain_len,
+   end - (domain + domain_len));
+
+   end -= domain_len;
+   maxlen -= domain_len;
+   }
+   }
+
+   answers += strlen(answers) + 1;
+   answers += 2 + 2 + 4;  /* skip type, class and ttl fields */
+
+   data_len = answers[0]  8 | answers[1];
+   answers += 2; /* skip the length field */
+
+   if (answers + data_len  end)
+   return -EINVAL;
+
+   answers += data_len;
+   maxlen -= answers - ptr;
+   }
+
+   return end - start;
+}
+
 static int forward_dns_reply(unsigned char *reply, int reply_len, int protocol,
struct server_data *data)
 {
@@ -1972,6 +2011,8 @@ static int forward_dns_reply(unsigned char *reply, int 
reply_len, int protocol,
 */
if (domain_len  0) {
int len = host_len + 1;
+   int new_len, fixed_len;
+   char *answers;
 
/*
 * First copy host (without domain name) into
@@ -1994,6 +2035,8 @@ static int forward_dns_reply(unsigned char *reply, int 
reply_len, int protocol,
 */
ptr += NS_QFIXEDSZ;
uptr += NS_QFIXEDSZ;
+   answers = uptr;
+   fixed_len = answers - uncompressed;
 
/*
 * We then uncompress the result to buffer
@@ -2025,22 +2068,39 @@ static int forward_dns_reply(unsigned char *reply, int 
reply_len, int protocol,
goto out;
 
/*
+* The uncompressed buffer now contains almost
+* valid response. Final step is to get rid of
+* the domain name because at least glibc
+* gethostbyname() implementation does extra
+* checks and expects to find an answer without
+* domain name if we asked a query without
+* domain part. Note that glibc getaddrinfo()
+* works differently and accepts FQDN in answer
+*/
+   new_len = strip_domains(uncompressed, answers,
+   uptr - answers);
+   if (new_len  0) {
+   DBG(Corrupted packet);
+   return -EINVAL;
+   }
+
+   /*
 * Because we have now uncompressed the answers
-* we must create a bigger buffer to hold all
-* that data.
+* we might have to create a bigger buffer to
+* hold all that data.
 */
 
-   new_reply = g_try_malloc(header_len +
-   uptr - uncompressed);
+   reply_len = header_len + new_len + fixed_len;
+
+   new_reply = g_try_malloc(reply_len);
if (!new_reply)
return -ENOMEM;
 
memcpy(new_reply, reply, header_len);
memcpy(new_reply + 

[PATCH] dnsproxy: Use defined error codes instead of plain numbers for rcode

2014-07-04 Thread Jukka Rissanen
The DNS return codes (rcode) are defined in arpa/nameser.h so
use them instead of hard coded values.
---
 src/dnsproxy.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/dnsproxy.c b/src/dnsproxy.c
index 2bf1690..538f9bf 100644
--- a/src/dnsproxy.c
+++ b/src/dnsproxy.c
@@ -435,7 +435,7 @@ static void send_cached_response(int sk, unsigned char 
*buf, int len,
 
hdr-id = id;
hdr-qr = 1;
-   hdr-rcode = 0;
+   hdr-rcode = ns_r_noerror;
hdr-ancount = htons(answers);
hdr-nscount = 0;
hdr-arcount = 0;
@@ -482,7 +482,7 @@ static void send_response(int sk, unsigned char *buf, int 
len,
DBG(id 0x%04x qr %d opcode %d, hdr-id, hdr-qr, hdr-opcode);
 
hdr-qr = 1;
-   hdr-rcode = 2;
+   hdr-rcode = ns_r_servfail;
 
hdr-ancount = 0;
hdr-nscount = 0;
@@ -1401,7 +1401,7 @@ static int cache_update(struct server_data *srv, unsigned 
char *msg,
DBG(offset %d hdr %p msg %p rcode %d, offset, hdr, msg, hdr-rcode);
 
/* Continue only if response code is 0 (=ok) */
-   if (hdr-rcode != 0)
+   if (hdr-rcode != ns_r_noerror)
return 0;
 
if (!cache)
@@ -1943,7 +1943,7 @@ static int forward_dns_reply(unsigned char *reply, int 
reply_len, int protocol,
 
req-numresp++;
 
-   if (hdr-rcode == 0 || !req-resp) {
+   if (hdr-rcode == ns_r_noerror || !req-resp) {
unsigned char *new_reply = NULL;
 
/*
-- 
1.8.3.1

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


Re: [PATCH] dnsproxy: Prefer results with domain name while domain_append is true

2014-07-04 Thread Jukka Rissanen
Hi Pasi,

On to, 2014-07-03 at 15:14 +0300, pasi.sjoh...@jolla.com wrote:
 From: Pasi Sjöholm pasi.sjoh...@jollamobile.com
 
 If domain_append is set and forward_dns_reply() processes the response
 for query without the domain name earlier than the response for one
 with the domain name set we need to make sure that the response is
 not sent back to the client if rcode and ancount are zero until the
 last nameserver response is processed.
 ---
  src/dnsproxy.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
 
 diff --git a/src/dnsproxy.c b/src/dnsproxy.c
 index 7232b98..28e7cf7 100644
 --- a/src/dnsproxy.c
 +++ b/src/dnsproxy.c
 @@ -2068,7 +2068,8 @@ static int forward_dns_reply(unsigned char *reply, int 
 reply_len, int protocol,
   }
  
  out:
 - if (hdr-rcode  0  req-numresp  req-numserv)
 + if ((hdr-rcode  0 || (hdr-rcode == 0  hdr-ancount == 0 
 + req-append_domain))  req-numresp  req-numserv)
   return -EINVAL;
  
   request_list = g_slist_remove(request_list, req);

Looks good, couple of comments

- I sent a patch that changes how rcode values are compared, that patch
does not touch this function thou. So if you could replace rcode 0 value
with ns_r_noerror

- Try to set the indentation of the second line so that it does not
align with return statement, then it is easier to read the code, even
better if the complex if statement could be written more clearly, you
could probably even refactor it to two pieces

- Add clarification to commit message about the rcode == ns_r_noerror
 ancount == 0 case, it is not very clear from looking the code when
that situation happens. You had this explained partly in this mail
thread already

- Currently the commit message is just one big sentence, my head starts
to hurt when reading it. Perhaps you could refactor it more.


Cheers,
Jukka


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

[PATCH] doc: Add informations about the experimental P2P Peer services support

2014-07-04 Thread Tomasz Bursztyka

As soon as p2p technology is present, whether it is enabled or not, it
will be possible to register and un-register P2P Peer services. These
will be served once p2p technology gets enabled.

When scanning for Peers is possible, Peer objects will show the proposed
services of those if some are present.
---
 doc/manager-api.txt | 31 +++
 doc/peer-api.txt|  5 +
 2 files changed, 36 insertions(+)

diff --git a/doc/manager-api.txt b/doc/manager-api.txt
index 05d2701..3034c1e 100644
--- a/doc/manager-api.txt
+++ b/doc/manager-api.txt
@@ -156,6 +156,37 @@ Methodsdict GetProperties()
Possible Errors: [service].Error.InvalidArguments
 +		void RegisterPeerService(array{byte} specification, boolean master) 
[experimental]

+
+   Registers a local P2P Peer service
+
+   If p2p technology is not available, this will raise an
+   error on such support. This behavior does not apply if
+   such technology is just disabled.
+
+   A Peer service belongs to the process that registers
+   it, thus if that process dies, its Peer services will
+   be destroyed as well.
+
+   specification is the TLV formated byte array
+   describing the P2P service.
+
+   Connman will be able to determine in most cases
+   whether to be the P2P Group Owner or not. If the
+   service must belong to a group that this device
+   manages, the master property can be set. Do not set
+   the master property unless you are absolutely sure
+   you know what you are doing.
+
+   Possible Errors: [service].Error.InvalidArguments
+[service].Error.NotSupported
+
+   void UnregisterPeerService(array{byte} specification) 
[experimental]
+
+   Unregisters an existing local P2P Peer service
+
+   Possible Errors: [service].Error.InvalidArguments
+
 SignalsTechnologyAdded(object path, dict properties)
Signal that is sent when a new technology is added.
diff --git a/doc/peer-api.txt b/doc/peer-api.txt
index e38066f..59a3351 100644
--- a/doc/peer-api.txt
+++ b/doc/peer-api.txt
@@ -57,3 +57,8 @@ Propertiesstring State [readonly] [experimental]
string Netmask [readonly]
The current configured IPv4 netmask.
+
+   array{array{byte}} Services [readonly] [experimental]
+
+   Array of P2P service specifications, which are
+   themselves a TLV formated byte array.
--
1.8.5.5


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


Static IP on unconnected wired port?

2014-07-04 Thread Mike Purvis
Hey all,

I have a platform where I'd like an ethernet port to come up with a static
IP, much as it does with /etc/network/interfaces containing:

auto p5p1
iface p5p1 inet static
  address 192.168.54.1
  netmask 255.255.255.0

This is essential so that a user can set a static IP on their laptop, and
then SSH into the device in order to configure the wireless, etc.

However, it doesn't seem possible to do this under connman— without a wire
connected to the ethernet port, no wired services are shown, so I have no
opportunity to do:

connmanctl config service --ipv4 ...

Is there a way to do this which I'm not seeing? Alternatively, can connman
be set up to ignore my ethernet port and manage only the wifi?

I apologize for the elementary question— the best docs I've been able to
find are those on the Arch Linux wiki:
https://wiki.archlinux.org/index.php/Connman

I'm using connman v1.15, via the Ubuntu 14.04 deb package.

Thanks,

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