Issue #1719: resolving hostnames for bind #1723

wanneut · 2018-03-14T10:56:36Z

Changes proposed there: #1719 .

gunicorn/config.py: using getaddrinfo gunicorn/util.py accept lists gunicorn/sock.py more general family handeling.

wanneut · 2018-03-14T17:45:43Z

I don't understand what this should do:

gunicorn/gunicorn/util.py

Line 247 in d1f5268

elif netloc == "":

netloc has should always have a value which is set Bind:

gunicorn/gunicorn/config.py

Line 536 in d1f5268

if 'PORT' in os.environ:

tilgovi · 2018-03-14T17:47:20Z

gunicorn/config.py

@@ -538,9 +538,9 @@ class Bind(Setting):
    validator = validate_list_string

    if 'PORT' in os.environ:
-        default = ['0.0.0.0:{0}'.format(os.environ.get('PORT'))]
+        default = ['[::]:{0}'.format(os.environ.get('PORT'))]


At least on my machine, this returns only IPv6 addresses from socket.getaddrinfo(). Since this is a list, maybe we want to give it two entries so Gunicorn binds to IPv4 and IPv6 by default?

If you bind to :: you will listen to any address (IPv4 and IPv6) as long the IPV6_V6ONLY flag is not set.
In fact :: could be written as ::0.0.0.0 which is already the IPv4-compatible address for 0.0.0.0. (Older version of ::FFFF:0.0.0.0)

tilgovi · 2018-03-14T17:51:43Z

I don't understand what this should do

I think it is just being safe and not relying on the config code to set it.

However, on my machine, an empty host does what I wanted from the default config where I commented:

>>> socket.getaddrinfo('', 80, proto=socket.IPPROTO_TCP)
[(<AddressFamily.AF_INET6: 30>, <SocketKind.SOCK_STREAM: 1>, 6, '', ('::1', 80, 0, 0)), (<AddressFamily.AF_INET: 2>, <SocketKind.SOCK_STREAM: 1>, 6, '', ('127.0.0.1', 80))]

So maybe the right default is the empty string and we don't need to handle it specially in parse_address?

tilgovi · 2018-03-14T17:52:54Z

Oh, I'm sorry, that's a good default when we don't automatically default to binding to every interface (PORT is undefined). I don't know what a good default is for the other case. Maybe it's to provide two addresses the way I described before.

berkerpeksag · 2018-03-14T21:54:18Z

gunicorn/sock.py

@@ -166,7 +159,8 @@ def create_sockets(conf, log, fds=None):
        for fd in fds:
            sock = socket.fromfd(fd, socket.AF_UNIX, socket.SOCK_STREAM)
            sock_name = sock.getsockname()
-            sock_type = _sock_type(sock_name)
+            sockinfo = (sock.family, sock.type, sock.proto, '', socket.getsockname())
+            sock_type = _sock_type(sockinfo)


Why did you pass a tuple here? _sock_type() only uses the first element of the tuple sockinfo. Can't we just pass sock.family?

It was done so before. But I can change that.

wanneut · 2018-03-15T09:43:36Z

I don't think that 0.0.0.0 is a good default. localhost is much better. I don't like programs witch listen to the world by default while it is very common to configure them insecure.
Think there are many gunicorn instances without security measures.
But in is set to that already in config.

wanneut · 2018-03-15T14:53:21Z

So now all test pass. (And I added a view.) I think all cases are handled properly now.

wanneut · 2018-03-15T17:33:35Z

OK, not on Ubuntu Trusty Tahr (from 2014).
The IPv6 entry in /etc/hosts for localhost is missing there.
On debian jessie (from 2015) and stretch all checks pass so it is on CentOS 7 and Ubuntu Bionic Beaver.

benoitc · 2018-03-20T11:31:18Z

_One question I have twith this PR is how it will work with OSes that doesn't support or have not been started with IPV6 support. I do think we still need to make sure ipv6 is supported on the OS somehow.

wanneut · 2018-03-20T12:50:39Z

I think it works perfect. It just binds to IPv4. Only some tests fail. Tried a view examples on a VM with disabled IPv6 in /proc/sys/net/.

benoitc · 2018-03-20T11:26:36Z

gunicorn/config.py

@@ -122,7 +122,11 @@ def worker_class(self):
    @property
    def address(self):
        s = self.settings['bind'].get()
-        return [util.parse_address(_compat.bytes_to_str(bind)) for bind in s]
+


why replacing that code?

It was described at the beginning. It can handle only one address.

benoitc · 2018-03-20T11:27:38Z

gunicorn/config.py

@@ -534,9 +538,9 @@ class Bind(Setting):
    validator = validate_list_string

    if 'PORT' in os.environ:
-        default = ['0.0.0.0:{0}'.format(os.environ.get('PORT'))]
+        default = ['[::]:{0}'.format(os.environ.get('PORT'))]


what about the machine that doesn't support or has not been installed with ipv6 support?

The older version dosn't work on IPv6 installations at all.
This version, works everywhere (IPv4 like IPv6) exempt of some obscure Linux-configurations (I think there are not many other OSes where something can be done.), where someone explicitly removed IPv6 support. I think such users should know how to specify another bind address.
If someone shoots himself into his foot he should not wonder if it hurts.
Gunicorn requires Python 2 (which is newer than IPv6.) multithreading and several other things which are much more uncommon than IPv6.
At the end you have to decide what your defaults are.
I mean what about installations with disabled IPv4? It is deprecated for 20 years now. Time to turn it off? IPv6 is compatible to it. So it shouldn't be needed any more. The new code could handle that.

benoitc · 2018-04-01T08:14:28Z

mm if ipv6 is not enabled at all on the machine at the kernel level it shouldn’t work normally. i will test anyway

wanneut · 2018-04-03T09:38:25Z

It works not if you set the PORT environment variable but no bind-address. The defaults work. If you set a IPv4 bind-address it woks also.

wanneut · 2018-04-18T13:41:01Z

I think the TOXENV=lint check is broken.

tilgovi · 2018-04-26T20:18:48Z

@wanneut why are some of the test cases commented out?

wanneut · 2018-04-27T08:06:09Z

On almost all OSes (even older ones) localhost resolves to ::1 and 127.0.0.1. So I used this to test an address that is known to be dual stack. But this seems not to be true for Ubuntu Trusty Tahr. Since this is the environment where the tests are executed I commented them out.
Yesterday there was a release of an new Ubuntu LTS version. So may the testing environment is updated and the tests can be used again.
Another option would be to replace localhost by a "real" domain but this would require a working Internet connection for testing. I don't know if that is wanted.
So I just commented them out and hoped that someone else, who reads it decides what to do.
Btw.: I thought about testing the protocol specific localhost addresses but this is even worse for testing on different OSes. On some they are named ip4-loopback and ip6-loopback on some localhost4 on some localhost-v4 on some there are only the v6 specific and the v4 specific missing...

tilgovi · 2018-04-27T22:25:43Z

If these tests passed before, they should pass now, unless behavior has changed. Where the behavior has changed deliberately, can we update the tests to show what the new expectation is?

I would love to get this merged.

wanneut · 2018-05-03T09:51:04Z

The tests passed on my debian PC with. They did never pass in the testing environment.

wanneut · 2018-06-12T12:48:36Z

Is anybody still following this?

tilgovi · 2018-06-14T23:41:04Z

@wanneut thanks for the nudge. I'll try to pick this up.

tilgovi · 2019-01-22T09:04:23Z

I'll pick this up again for the next cycle after R20. Sorry for the delay and thank you for your patience and efforts.

rivimey · 2020-09-28T21:50:05Z

bump - is this still relevant? I'm interested in getting gunicorn listening on ipv6 for a dual-family host.

wanneut added 8 commits March 12, 2018 18:36

Using getaddrinfo to resolve hostnames properly

f3ceae2

gunicorn/config.py: using getaddrinfo gunicorn/util.py accept lists gunicorn/sock.py more general family handeling.

better style

b1779b5

fixed socket.close

61ea13e

spcaes

f0f2928

adopted some tests

06bd5d4

reseted requirements

3f633ec

more reasonable defaults: [::] and localhost will lsiten on IPv6+IPv4

0c45c4a

127.0.0.1 -> localhost

25935b7

tilgovi reviewed Mar 14, 2018

View reviewed changes

tilgovi self-requested a review March 14, 2018 17:47

berkerpeksag reviewed Mar 14, 2018

View reviewed changes

wanneut and others added 3 commits March 15, 2018 10:52

_sock_type gets only family

4874c1b

adopted last test

2bdd721

IPv6 schme handling, test coverage

c94048e

test for tcp://

d3568c8

benoitc reviewed Apr 1, 2018

View reviewed changes

wanneut added 2 commits April 18, 2018 10:34

Removed test depending on modern /etc/hosts

7904f3f

accept IPv4 only environment

0ebf15c

rerun checks

6f91687

wanneut added 3 commits April 25, 2018 09:43

making code unreadable

51466dc

removed trailing whitespace

c597560

-

4c20ec1

tilgovi self-assigned this Jun 14, 2018

benoitc force-pushed the master branch from 5a43f72 to fe7632f Compare January 24, 2019 22:19

benoitc added the Feature/Core label May 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue #1719: resolving hostnames for bind #1723

Issue #1719: resolving hostnames for bind #1723

wanneut commented Mar 14, 2018

wanneut commented Mar 14, 2018

tilgovi Mar 14, 2018

wanneut Mar 15, 2018 •

edited

Loading

tilgovi commented Mar 14, 2018

tilgovi commented Mar 14, 2018 •

edited

Loading

berkerpeksag Mar 14, 2018

wanneut Mar 15, 2018

wanneut commented Mar 15, 2018 •

edited

Loading

wanneut commented Mar 15, 2018

wanneut commented Mar 15, 2018 •

edited

Loading

benoitc commented Mar 20, 2018

wanneut commented Mar 20, 2018

benoitc Mar 20, 2018

wanneut Apr 3, 2018

benoitc Mar 20, 2018

wanneut Apr 3, 2018 •

edited

Loading

benoitc commented Apr 1, 2018

wanneut commented Apr 3, 2018

wanneut commented Apr 18, 2018

tilgovi commented Apr 26, 2018

wanneut commented Apr 27, 2018 •

edited

Loading

tilgovi commented Apr 27, 2018

wanneut commented May 3, 2018

wanneut commented Jun 12, 2018

tilgovi commented Jun 14, 2018

tilgovi commented Jan 22, 2019

rivimey commented Sep 28, 2020

Issue #1719: resolving hostnames for bind #1723

Are you sure you want to change the base?

Issue #1719: resolving hostnames for bind #1723

Conversation

wanneut commented Mar 14, 2018

wanneut commented Mar 14, 2018

tilgovi Mar 14, 2018

Choose a reason for hiding this comment

wanneut Mar 15, 2018 • edited Loading

Choose a reason for hiding this comment

tilgovi commented Mar 14, 2018

tilgovi commented Mar 14, 2018 • edited Loading

berkerpeksag Mar 14, 2018

Choose a reason for hiding this comment

wanneut Mar 15, 2018

Choose a reason for hiding this comment

wanneut commented Mar 15, 2018 • edited Loading

wanneut commented Mar 15, 2018

wanneut commented Mar 15, 2018 • edited Loading

benoitc commented Mar 20, 2018

wanneut commented Mar 20, 2018

benoitc Mar 20, 2018

Choose a reason for hiding this comment

wanneut Apr 3, 2018

Choose a reason for hiding this comment

benoitc Mar 20, 2018

Choose a reason for hiding this comment

wanneut Apr 3, 2018 • edited Loading

Choose a reason for hiding this comment

benoitc commented Apr 1, 2018

wanneut commented Apr 3, 2018

wanneut commented Apr 18, 2018

tilgovi commented Apr 26, 2018

wanneut commented Apr 27, 2018 • edited Loading

tilgovi commented Apr 27, 2018

wanneut commented May 3, 2018

wanneut commented Jun 12, 2018

tilgovi commented Jun 14, 2018

tilgovi commented Jan 22, 2019

rivimey commented Sep 28, 2020

wanneut Mar 15, 2018 •

edited

Loading

tilgovi commented Mar 14, 2018 •

edited

Loading

wanneut commented Mar 15, 2018 •

edited

Loading

wanneut commented Mar 15, 2018 •

edited

Loading

wanneut Apr 3, 2018 •

edited

Loading

wanneut commented Apr 27, 2018 •

edited

Loading