[PATCH] Fix UDP DNS queries in addns

[Jeremy Allison]

On Tue, Jun 12, 2018 at 08:09:14AM +0200, Andreas Schneider via samba-technical wrote:
> On Tuesday, 12 June 2018 02:56:38 CEST Timur I. Bakeyev via samba-technical 
> wrote:
> > Simple testing shows up that `net ads dns gethostbyname` is broken at least
> > on FreeBSD, although shouldn't work on Linux either:
> > 
> > # net ads dns gethostbyname 8.8.8.8 samba.org
> > do_gethostbyname returned ERROR_DNS_SOCKET_ERROR (10)
> > 
> > Strace shows for the AD server:
> > 
> > socket(PF_INET,SOCK_DGRAM,IPPROTO_UDP)         = 8 (0x8)
> > 
> > > connect(8,{ AF_INET 10.10.0.20:53 },16)         = 0 (0x0)
> > > getpeername(8,{ AF_INET 10.10.0.20:53 },0x7fffffffd34c) = 0 (0x0)
> > > read(5,":\\",2)                     = 2 (0x2)
> > > sendto(8,"\\:\0\0\0\^A\0\0\0\0\0\0\vcliffw"...,29,0,{ AF_INET
> > > 10.10.0.20:53 },128) ERR#56 'Socket is already connected'
> > > close(8)                     = 0 (0x0)
> > 
> > Which explains why this error occurs - in source3/utils/net_dns.c in
> > do_gethostbyname() we call  dns_open_connection(server, DNS_UDP, NULL,
> > &conn) and then, through other wrapping functions - dns_send_udp() from
> > lib/addns/dnssock.c.
> > 
> > dns_open_connection() for both UDP and TCP is calling dns_open_helper(),
> > which, among other staff calls connect() on the created socket.
> > 
> > For UDP to send a packet you can use two similar approaches, with slightly
> > different effects. One is to use connect()/send() sequence, another is just
> > to use sendto(). Both methods should work, but you shouldn't combine
> > connect() together with the sendto() - that would create the EISCONN error.
> > Even Linux manpage says:
> > 
> > If sendto() is used on a connection-mode (SOCK_STREAM, SOCK_SEQPACKET)
> > 
> > > socket, the arguments dest_addr and addrlen are ignored (and the  error
> > > EISCONN  may be returned  when they are not NULL and 0).
> > 
> > That doesn't say anything about SOCK_DGRAM, but it should be an error
> > also(and it's a bug in the implementation if it's not produced).
> 
> RB+

Reviewed-by: Jeremy Allison, and pushed. Thanks Timur, good catch !