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

File generation incorrect for: mobileconfig for DNS-over-TLS #2352

Closed
josephbu opened this issue Nov 23, 2020 · 7 comments
Closed

File generation incorrect for: mobileconfig for DNS-over-TLS #2352

josephbu opened this issue Nov 23, 2020 · 7 comments
Assignees
Milestone

Comments

@josephbu
Copy link

Issue Details

  • Version of AdGuard Home server:
    • 0.104.3
  • Operating system and version:
    • Ubuntu 20.04.1 LTS

Expected Behavior

File generation for .mobileconfig DNS-over-TLS to be successful.

Actual Behavior

In the web interface followed these steps: Setup Guide -> DNS Privacy -> Download .mobileconfig for DNS-over-TLS

The dot.mobileconfig file that was generated & downloaded is not a valid xml file or recognised by iOS for import.

Having a closer look, the top line of the generated file dot.mobileconfig it includes an error message getting host: address xxx: missing port in address, top 3 lines of my generated file are as follows:

getting host: address syd.adfilter.net: missing port in address
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">

Additionally under the array for section <key>PayloadContent</key> there is no <key>ServerName</key> or associated <string> as you would expect.

Additional Information

Based on the behaviour I have observed, the generation of the mobileconfig files tries to automatically detect hostname, IP, port based on the hostname/port that you use to access the web management interface.

For example, on my VM I have a "management" IP & http port that I mostly use to manage it as opposed to the public hostname over https, if I access the web interface via that "mangement" IP address I will get different contents in doh.mobileconfig.

To generate a correct doh.mobileconfig I needed to do this from the "public" hostname so that the details in the file are correct.

Back to this issue for DOT, I have tried to generate the file when accessing the web interfaces on both http and https ports, using the public hostname and IP, as well as the private management IP, in all cases I get the missing port in address error.

My guess is that it might be easier to move away from auto detection for the generating values for these config files, an option might be to use the details for server name and port from "Settings -> Encryption settings"?

If you need any further information please let me know!

@ameshkov ameshkov added this to the v0.105 milestone Nov 23, 2020
@ameshkov
Copy link
Member

My guess is that it might be easier to move away from auto detection for the generating values for these config files, an option might be to use the details for server name and port from "Settings -> Encryption settings"?

The information there may be ambiguous. For instance, you may use a wildcard certificate and keep the "server name" empty.

@josephbu
Copy link
Author

The information there may be ambiguous.

True, that is a fair point!

@ainar-g
Copy link
Contributor

ainar-g commented Nov 23, 2020

We do need to move away from this silly autodetection mechanism, but the bug in the original post can be fixed quicker. @EugeneOne1 should be able to do that this week.

As for a better mechanism, we need to think about it. For now we probably will fallback to the tls.server_name parameter from the configuration file. And respond with something like a 500 error.

@ameshkov
Copy link
Member

ameshkov commented Nov 23, 2020

And respond with something like a 500 error.

Respond like that if we can't find a way to detect the server name.
Response content should indicate what happened.

@ainar-g
Copy link
Contributor

ainar-g commented Nov 23, 2020

Yes, that's what I meant as well, thanks for clarifying.

@ainar-g
Copy link
Contributor

ainar-g commented Nov 24, 2020

We've decided to solve the issue in a more systematic fashion instead of yet another patch. @josephbu, can you please take a look at #2358 and tell us, if that would solve your issue?

@ainar-g
Copy link
Contributor

ainar-g commented Dec 4, 2020

I'll close this issue for now. Please feel free to open a new one if you think there are more issues.

@ainar-g ainar-g closed this as completed Dec 4, 2020
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