Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upstream NIC.CZ ODVR DoT throws error #2681

Closed
Xeevis opened this issue Feb 13, 2021 · 6 comments
Closed

Upstream NIC.CZ ODVR DoT throws error #2681

Xeevis opened this issue Feb 13, 2021 · 6 comments

Comments

@Xeevis
Copy link

Xeevis commented Feb 13, 2021

When I try to add upstream DNS tls://odvr.nic.cz according to docs, I'm met with error when I click Test upstreams in DNS settings.

Issue Details

  • Version of AdGuard Home server:
    • v0.105.0-SNAPSHOT-2bf2d5a1
  • How did you install AdGuard Home:
services: 
  adguard:
    image: adguard/adguardhome:edge
    container_name: adguard
    restart: unless-stopped
    volumes:
      - /volume1/docker/adguard/work:/opt/adguardhome/work
      - /volume1/docker/adguard/conf:/opt/adguardhome/conf
    ports:
      - 53:53/tcp
      - 53:53/udp
      - 3000:3000/tcp
  • CPU architecture:
    • AMD64
  • Operating system and version:
    • Synology DSM 7.0

Expected Behavior

No error when using tls://odvr.nic.cz as upstream DNS

Actual Behavior

[info] couldn't communicate with dns server tls://odvr.nic.cz: Failed to get a connection from TLSPool to tls://odvr.nic.cz:853, cause: Failed to connect to odvr.nic.cz, cause: remote error: tls: no application protocol

Screenshots

Screenshot:

image

Additional Information

According to this article CZ.NIC ODVR has in December disabled support for outdated TLS 1.0/1.1, could it be related?

@ameshkov ameshkov added this to the v0.105.1 milestone Feb 15, 2021
@ameshkov ameshkov added waiting for data Waiting for users to provide more data. and removed P3: Medium bug labels Feb 15, 2021
@ameshkov
Copy link
Member

Interesting. It seems it expects us to send some ALPN extension, but nowhere in the spec I see this requirement.

@ainar-g ainar-g modified the milestones: v0.105.1, v0.105.2 Feb 15, 2021
@ainar-g
Copy link
Contributor

ainar-g commented Feb 16, 2021

We should try to force our proxy to use at least TLS 1.2 and see if that helps. @ameshkov, what do you think?

@ainar-g ainar-g added enhancement P3: Medium and removed waiting for data Waiting for users to provide more data. labels Feb 16, 2021
@ameshkov
Copy link
Member

This shouldn't matter much, the server chooses the TLS version anyway.

@ameshkov
Copy link
Member

ameshkov commented Mar 3, 2021

@EugeneOne1 the problem is that we're speciying ALPN (tls.Config.NextProtos) for DNS-over-TLS connections.

In fact, we should only do this for DNS-over-QUIC and DNS-over-HTTPS:
https://github.com/AdguardTeam/dnsproxy/blob/master/upstream/bootstrap.go#L194

adguard pushed a commit to AdguardTeam/dnsproxy that referenced this issue Mar 3, 2021
Merge in DNS/dnsproxy from fix-alpn to master

Updates AdguardTeam/AdGuardHome#2681.

* commit '1beaef57054915a65636da4bda6157a1ec3d9acc':
  upstream: imp docs
  upstream: split host and port more carefully
  upstream: imp types, fix ALPN bug
adguard pushed a commit that referenced this issue Mar 3, 2021
Merge in DNS/adguard-home from 2681-fix-dot-bug to master

Updates #2681

Squashed commit of the following:

commit 8de0f4c
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Wed Mar 3 19:25:34 2021 +0300

    all: upd dnsproxy
@EugeneOne1
Copy link
Member

This should be fixed as of snapshot 400b76d. Could you please check, if our solution fixes the issue for you?

@Xeevis
Copy link
Author

Xeevis commented Mar 4, 2021

Works great now, many thanks!

@Xeevis Xeevis closed this as completed Mar 4, 2021
heyxkhoa pushed a commit to heyxkhoa/AdGuardHome that referenced this issue Mar 20, 2023
Merge in DNS/adguard-home from 2681-fix-dot-bug to master

Updates AdguardTeam#2681

Squashed commit of the following:

commit 8de0f4c
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Wed Mar 3 19:25:34 2021 +0300

    all: upd dnsproxy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants