Bug 14990

Summary: getaddrinfo with servname=NULL returns duplicate addresses
Product: glibc Reporter: Pavel Šimerda <psimerda>
Component: networkAssignee: Pavel Šimerda <psimerda>
Status: NEW ---    
Severity: normal CC: me
Priority: P2 Flags: fweimer: security-
Version: 2.15   
Target Milestone: ---   
Host: Target:
Build: Last reconfirmed:

Description Pavel Šimerda 2012-12-30 12:53:41 UTC
Testing in python:

>>> print(*getaddrinfo ("localhost", None), sep='\n')

Actual result:

(10, 1, 6, '', ('::1', 0, 0, 0))
(10, 2, 17, '', ('::1', 0, 0, 0))
(10, 3, 0, '', ('::1', 0, 0, 0))
(2, 1, 6, '', ('127.0.0.1', 0))
(2, 2, 17, '', ('127.0.0.1', 0))
(2, 3, 0, '', ('127.0.0.1', 0))

Expected result:

(10, 0, 0, '', ('::1', 0, 0, 0))
(2, 0, 0, '', ('127.0.0.1', 0))

POSIX1-2008:

If servname is null, the call shall return network-level addresses for the specified nodename.

http://pubs.opengroup.org/onlinepubs/009604499/functions/getaddrinfo.html

Filling in the socktype and protocol is IMO not a good idea and returning duplicate addresses prevents applications from using getaddrinfo for host name resolution and violates POSIX.
Comment 1 Pavel Šimerda 2013-01-11 15:28:40 UTC
(In reply to comment #0)
> If servname is null, the call shall return network-level addresses for the
> specified nodename.

As a side effect, using socktype and/or protocol when servname=NULL has no chance to return useful information. These values are there only to supplement the service option.

But that means SOCK_RAW can be ignored entirely in getaddrinfo().

The only questions is, what should getaddrinfo return when servname is NULL but socktype and/or protocol is non-zero. There are a couple of options:

1) Fail. This might have impact on existing software.

2) Return zero socktype and zero protocol. This might have impact on existing software.

3) Return the supplied socktype and protocol.

   a) Without checking.

   b) With a validity check.
Comment 2 Pavel Šimerda 2013-01-11 15:58:28 UTC
The code for #3a could look something like this:

  if (!servname)
    {
      /* POSIX1-2008: If servname is null, the call shall return network-level
       * addresses for the specified nodename.
       *
       * Example:
       *
       * getaddrinfo ("localhost", NULL, ...) ->
       *   family=AF_INET6 socktype=0 protocol=0 address=::1 port=0
       *   family=AF_INET socktype=0 protocol=0 address=127.0.0.1 port=0
       */
      struct gaih_servtuple *st = malloc (sizeof (struct gaih_servtuple));
      st->next = NULL;
      st->port = 0;
      /* Copy socktype and protocol from the request. This maintains backwards
       * compatibility. Callers should not set socktype and protocol when
       * service is NULL.
       */
      st->socktype = socktype;
      st->protocol = protocol;

      *servtuple = st;
      return 0;
    }
Comment 3 Pavel Šimerda 2013-01-11 16:11:15 UTC
Interesting test results:

>>> getaddrinfo("localhost", None, AF_INET, SOCK_RAW, 0)
[(2, 3, 0, '', ('127.0.0.1', 0)), (2, 3, 0, '', ('127.0.0.1', 0))]
>>> getaddrinfo("www.fedora.cz", None, AF_INET, SOCK_RAW, 0)
[(2, 3, 0, '', ('217.170.99.20', 0))]
Comment 4 Ruslan N. Marchenko 2013-03-07 07:27:40 UTC
Hi Pavel,
You probably better look at it from perspective of C call, not python wrapper.
In C it is clear - you're asking for host:service information, providing the rest as hints.
As such, if service is empty, all hints which should make service more specific are useless and should be ignored. Hence getaddrinfo(something,null,whatever) should always return RAW. 
Similarly asking for RAW should ignore service since there's no service for raw socket.
This is akin to combination of AI_PASSIVE and host NULL - both having similar mining although using PASSIVE  with non-null host has no sense and PASSIVE flag is discarded. The difference is that we don't have documented behaviour of srv=null without RAW.

Anyway, here's what C call returns:

getaddrinfo(localhost,(null),{AF=0,ST=0,FLAGS=0})=0
socket(10, 1, 6)
socket: Success
getnameinfo: Success
 [::1:0] ---> [localhost:(null)]
socket(10, 2, 17)
socket: Success
getnameinfo: Success
 [::1:0] ---> [localhost:(null)]
socket(10, 3, 0)
socket: Protocol not supported
socket(2, 1, 6)
socket: Protocol not supported
getnameinfo: Protocol not supported
 [127.0.0.1:0] ---> [localhost:(null)]
socket(2, 2, 17)
socket: Protocol not supported
getnameinfo: Protocol not supported
 [127.0.0.1:0] ---> [localhost:(null)]
socket(2, 3, 0)
socket: Protocol not supported


I.e. similar results.

This list is returned by gaih_inet() (where all nasty things happen) 
line 375
          /* Neither socket type nor protocol is set.  Return all socket types
             we know about.  */

and successfully passed through deprecation validation and sorting, and not a piece of code bothered to strip off unneeded results. Will submit patch later.
Comment 5 Pavel Šimerda 2013-03-07 12:00:27 UTC
(In reply to comment #4)
> Hi Pavel,
> You probably better look at it from perspective of C call, not python wrapper.

Please look at it from the perspective of both the POSIX standard and the fact that getaddrinfo() was designed to replace both gethostbyname().

I don't know why you bring Python into the discussion when the Python interface is exactly 1:1 to the C interface except the hints are expanded. Python is only used because it is more convenient for testing.

> you're asking for host:service information, providing the rest as hints.

Correct, unless some of the hints have special meaning on their own.

> As such, if service is empty, all hints which should make service
> more specific are useless and should be ignored.

Correct.

Hence getaddrinfo(something,null,whatever)
> should always return RAW.

Incorrect, in my opinion. I am curious about your explenation taking into account POSIX and/or real-world use cases. There's IMO no reason to return RAW unless the caller wants it.

> Similarly asking for RAW should ignore service since there's no service
> for raw socket.

I don't say you are not right here but you contradict yourself, as asking for RAW is only possible using the hints. Above you asked hints to be always ignored when service is NULL.

> This is akin to combination of AI_PASSIVE and host NULL - both having similar
> mining although using PASSIVE with non-null host has no sense and PASSIVE flag
> is discarded.

This is wrong. Using AI_PASSIVE with non-null host makes perfect sense even though AI_PASSIVE is ignored then. You and many others are forgetting that the flags and the values usually don't come from the same source.

The programmer uses AI_PASSIVE without the knowledge whether the host will be NULL or non-NULL, which is determined by the user configuration. This is a convenience tool so that allows the programmer to use the same getaddrinfo() interface regardless the user input.

> The difference is that we don't have documented behaviour of
> srv=null without RAW.

> Anyway, here's what C call returns:

The Python function is a convenience wrapper over the C call, there's no need to compare them.

The patch is already available as part of my patchset:

http://git.pavlix.net/gitweb/?p=glibc.git;a=shortlog;h=refs/heads/pavlix/getaddrinfo
Comment 6 Ruslan N. Marchenko 2013-03-07 19:29:01 UTC
> Please look at it from the perspective of both the POSIX standard and the fact
> that getaddrinfo() was designed to replace both gethostbyname().

This is the only point I'm looking at it from

>> Hence getaddrinfo(something,null,whatever) should always return RAW.
>Incorrect, in my opinion.

>> Similarly asking for RAW should ignore service since there's no service
>> for raw socket.
> Above you asked hints to be always ignored when service is NULL.
Yes, this is not explicitly written by standard so my common sense is saying that it's useless to parse socktype when service s null. although one can always return error for unclear by standard cases.

> Using AI_PASSIVE with non-null host makes perfect sense even 
> though AI_PASSIVE is ignored then.

I think we need to be careful with common sense when dealing with explicitly stated standard:
http://pubs.opengroup.org/onlinepubs/009695399/functions/getaddrinfo.html
>> qoute >>
The AI_PASSIVE flag shall be ignored if the nodename argument is not null.
<< end quote <<

> You and many others are forgetting that the flags and the values usually don't come from the same source.

I don't, and that the background for my reasoning.

>The Python function is a convenience wrapper over the C call, there's no need
to compare them.

right, my c snippet initially was returning different result so i started this C part but then i found i was setting socktype at the end. You can disregard it.

> The patch is already available as part of my patchset:
Glad you care about that. And since getaddrinfo is to be completely revamped anyway I'm going to propose another solution for passive flag.
Comment 7 Ruslan N. Marchenko 2013-03-07 19:55:03 UTC
After second read it seems we arguing about the same topic with different words. I'd better postpone this discussion till weekend, hard to concentrate after work.