[PATCH] better constrain IP-literal validation in ngx_http_validate_host()

[Terence Honles]

Yes, thanks for the prompt response and pardon my delay in responding.

The reason I came across this code was I have NGINX handling HTTPS, and
proxying to Django via uWSGI. Django has the following RegEx [1]_. Which is is
compliant with the IPv6 literal notation, but causes Django to report an
error.

While I agree, there may not be an issue of security; Any down stream systems
may be confused and unable to handle a malformed hostname. Another alternative
would be to rewrite the hostname to not include the "[" and "]" if it is not a
valid IPv6 literal, but at that point you are validating the IPv6 literal and
to me it makes more sense to reject the connection and force the sending
system to fix the bug rather than have NGINX act in a non compliant manner.

I understand there may be a future effort to support something like:
``[unix:/path/to/unix.socket]``, but I would *NOT* want that to come over the
network via something akin to
``curl -H 'Host: [unix:/path/to/docker-etc.]' my-hostname``. To support other
workloads properly, I believe the validation may need to understand
differences between potentially trusted and untrusted input, and at that point
it may need to be rewritten. With that in mind, this change should not be seen
as a blocker to that potential future.

-Terence

.. [1] https://github.com/django/django/blob/194a4b526ca3f1e1b13cf27f6ed9aeec3a2e9f89/django/http/request.py#L22

On Mon, Dec 17, 2018 at 8:17 AM Maxim Dounin <mdounin at mdounin.ru> wrote:
>
> Hello!
>
> On Sun, Dec 16, 2018 at 07:18:19PM -0800, Terence Honles wrote:
>
> > # HG changeset patch
> > # User Terence Honles <terence at honles.com>
> > # Date 1542840079 28800
> > #      Wed Nov 21 14:41:19 2018 -0800
> > # Node ID 0763519f3dcce2c68ccd8894dcc02a4d6114b4c2
> > # Parent  be5cb9c67c05ccaf22dab7abba78aa4c1545a8ee
> > better constrain IP-literal validation in ngx_http_validate_host()
> >
> > The existing validation in ngx_http_validate_host() would allow a IP-literal
> > such as "[127.0.0.1]" which is invalid according to RFC 3986 (See Appendix A.
> > for the Collected ABNF). This format is intended for IPv6 and IPv-future not
> > IPv4.
>
> We've considered doing more strict checks when introducing IPv6
> literals in e7db97bfac25 (http://hg.nginx.org/nginx/rev/e7db97bfac25),
> yet decided that:
>
> - it doesn't add anything to security,
> - and may actually harm some future workloads, such as using
>   things like [unix:/path/to/unix.socket].
>
> In particular, it doesn't looks like permitting [127.0.0.1] can be
> a problem.
>
> Do you think that introducing more strict checks can be
> beneficial?  Could you please outline reasons?
>
> [...]
>
> --
> Maxim Dounin
> http://mdounin.ru/
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>