DNSDist 1.9.0 DoH picks alpn http/1.1 over h2 when http/1.1 is listed first

tykling commented 8 months ago

[X] This is not a support question, I have read about opensource and will send support questions to the IRC channel, Github Discussions or the mailing list.
[X] I have read and understood the 'out in the open' support policy

Program: dnsdist 1.9.0
Issue type: Bug report

Short description

dnsdist 1.9.0 picks http/1.1 over h2 when both are offered in alpn, where 1.8.3 picks h2.

Environment

Operating system: FreeBSD 13.2
Software version: dnsdist 1.9.0
Software source: DoH Client is dnspython==2.6.1 which uses httpx==0.26.0 which in turn uses httpcore==1.0.2 which always adds http/1.1 to alpn. The same client works with http/2 on dnsdist 1.8.3.

Steps to reproduce

[tykling@irc2 ~]$ python3.9 -m venv venv
[tykling@irc2 ~]$ source venv/bin/activate
(venv) [tykling@irc2 ~]$ pip install dnspython[doh]
<snip>
Successfully installed anyio-4.3.0 certifi-2024.2.2 exceptiongroup-1.2.0 h11-0.14.0 h2-4.1.0 hpack-4.0.0 httpcore-1.0.4 httpx-0.27.0 hyperframe-6.0.1 idna-3.6 sniffio-1.3.1 typing-extensions-4.10.0
(venv) [tykling@irc2 ~]$ python
Python 3.9.16 (main, Dec 19 2022, 23:38:01) 
[Clang 13.0.0 (git@github.com:llvm/llvm-project.git llvmorg-13.0.0-0-gd7b669b3a on freebsd13
Type "help", "copyright", "credits" or "license" for more information.
>>> import dns.message, dns.name, dns.query
>>> q = dns.message.make_query(dns.name.from_text("example.com"), "A")
>>> dns.query.https(q, "https://deic-lgb.anycast.uncensoreddns.org/dns-query") # this server runs dnsdist 1.9.0
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/home/tykling/venv/lib/python3.9/site-packages/dns/query.py", line 489, in https
    raise ValueError(
ValueError: https://deic-lgb.anycast.uncensoreddns.org/dns-query responded with status code 400
Response body: b'<html><body>This server implements RFC 8484 - DNS Queries over HTTP, and requires HTTP/2 in accordance with section 5.2 of the RFC.</body></html>\r\n'
>>> dns.query.https(q, "https://deic-ore.anycast.uncensoreddns.org/dns-query") # this server runs dnsdist 1.8.3
<DNS message, ID 46966>
>>>

Expected behaviour

I expected h2 to be used

Actual behaviour

dnsdist picks alpn http/1.1 and since that is not supported in 1.9.0 it returns an error

Other information

These pcaps illustrate the problem but because the ServerHello is partially encrypted the actual application_layer_protocol_negotiation extension is not readable: dnsdist 1.8.3 working dnsdist 1.9.0 failing

also:

2024-03-03 08:55:58 < dwfreed> ottom: getNextProtocol doesn't check the first protocol listed, it just gets the *selected* protocol; the protocol selection picks http/1.1 because the client sends that one first
2024-03-03 08:56:38 < dwfreed> https://github.com/PowerDNS/pdns/blob/master/pdns/tcpiohandler.cc#L874
2024-03-03 08:57:20 < dwfreed> that code could break with tradition and do server preference, which would avoid the issue
2024-03-03 08:57:34 < dwfreed> httpcore could also be fixed to put h2 first

And also section 3.2 of rfc7301 says:

In that case, the server SHOULD select the most highly preferred protocol that it supports and that is also advertised by the client.

dwfreed commented 8 months ago

This is why dnspython sends http/1.1 first:

https://github.com/encode/httpcore/blob/7b39e6abd256474d753b4241a67aa10b6bd70d18/httpcore/_sync/connection.py#L144

Here's the ALPN selection code (referenced above):

https://github.com/PowerDNS/pdns/blob/524ce4f93e96c3f9e41f00a68ba99a71118447f6/pdns/tcpiohandler.cc#L874-L901

This is a client preference algorithm, but as noted in the issue description, it should be a server preference algorithm.

rgacogne commented 7 months ago

Quick summary to be sure this issue is correctly understood: when the nghttp2 DNS over HTTPS provider is used (default), if the client advertises support for HTTP/1.1 and HTTP/2 AND offers HTTP/1.1 before offering HTTP/2, dnsdist wrongly selects HTTP/1.1. This is particularly bad because the nghttp2 provider doesn't actually support HTTP/1.1, just enough to tell the client it should upgrade to HTTP/2. The good news is that most DoH clients should in theory not offer HTTP/1.1, or at the very least not before HTTP/2.

PowerDNS / pdns