raw sockets don't accept output of getaddrinfo (at least under linux)

[smason]

I've just been playing with creating an async ICMP ping utility and noticed most socket methods seem broken with type=SOCK_RAW sockets raising:

Traceback (most recent call last):
  File "test_raw_socket.py", line 9, in <module>
    trio.run(main)
  File "/home/smason/work/trio/trio/_core/_run.py", line 1783, in run
    raise runner.main_task_outcome.error
  File "test_raw_socket.py", line 7, in main
    await sock.sendto(b'x' * 56, ('127.0.0.1', 0))
  File "/home/smason/work/trio/trio/_socket.py", line 743, in sendto
    args[-1] = await self._resolve_remote_address(args[-1])
  File "/home/smason/work/trio/trio/_socket.py", line 554, in _resolve_remote_address
    return await self._resolve_address(address, 0)
  File "/home/smason/work/trio/trio/_socket.py", line 529, in _resolve_address
    host, port, self._sock.family, self.type, self._sock.proto, flags
  File "/home/smason/work/trio/trio/_socket.py", line 159, in getaddrinfo
    host, port, family, type, proto, flags | _NUMERIC_ONLY
  File "/usr/lib64/python3.7/socket.py", line 748, in getaddrinfo
    for res in _socket.getaddrinfo(host, port, family, type, proto, flags):
socket.gaierror: [Errno -8] Servname not supported for ai_socktype

when run with the following code:

import trio
from trio import socket

async def main():
    with socket.socket(socket.AF_INET, socket.SOCK_RAW, socket.IPPROTO_ICMP) as sock:
        await sock.sendto(b'x' * 56, ('127.0.0.1', 0))

trio.run(main)

note that this needs to be run with super-user privileges (RAW sockets are generally sensitive).
this is obviously an invalid ICMP packet, but it doesn't get far enough for this to actually matter.

the above is the code I wanted to write, but it also raises a similar exception for sock.connect(('127.0.0.1', 0)) due to both using socket._resolve_address. the address comes is this form from getaddrinfo, e.g:

socket.getaddrinfo('localhost', '', family=socket.AF_INET, proto=socket.IPPROTO_ICMP)

gives me [(<AddressFamily.AF_INET: 2>, <SocketKind.SOCK_RAW: 3>, 1, '', ('127.0.0.1', 0))] and is accepted by the native/non-async socket.sendto, or can be worked around with:

        await sock._nonblocking_helper(
            socket._stdlib_socket.socket.sendto,
            (b'x' * 56, ('127.0.0.1', 0)), {},
            trio.hazmat.wait_writable
        )

note that using await sock.sendto(b'x' * 56, ('127.0.0.1', '')) works, i.e. an empty string for port.

it seems reasonable for _resolve_address to special case SOCK_RAW and always pass '' for the getaddrinfo service, maybe raising ValueError if it's truthy?

I can submit a patch doing this, but am not sure if my understanding of raw sockets is off/there are other preferences. also not sure what to do about tests, OS level privilege checks make this awkward.

some version numbers for reference: am using trio's current master branch, under Python 3.7.3 and Linux 5.1.15

[njsmith]

Oh interesting. So getaddrinfo returns the service as 0, but when you call getaddrinfo it insists that the service be passed as ""?

I agree that some kind of special case workaround is appropriate. Do you happen to know how the stdlib handles this case? (The _resolve_address code generally tries to match the way the stdlib implicitly calls getaddrinfo, but obviously we missed some detail.)

Re: testing: well, it depends somewhat on what workaround we end up with, but one option is to use a custom socket factory to mock out the parts that need root.

[smason]

the standard library only calls getaddrinfo to resolve hostnames, port numbers have to be integers as they are just passed through via calls like bind, connect and sendto. doing the same would fix this issue as strings would start failing with TypeError as the stdlib does

trio's API allowing port/service names to be specified as strings certainly doesn't match the stdlib, but changing that now will probably affect users of your API. your docs do say that they "are identical to their equivalents in socket.socket" so they maybe should be changed. there's also mention of "pre-resolved addresses" which I don't understand

if you're looking through Python's source code, they all end up callingsetipaddr() in Modules/socketmodule.c, which only calls getaddrinfo as a last resort