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

[Feature] Set final DNS to avoid issues when using templates.extend with templates.servers #37

Open
john-jane-doe opened this issue Dec 1, 2024 · 0 comments

Comments

@john-jane-doe
Copy link

john-jane-doe commented Dec 1, 2024

Commit: 474ab7e

According to the source code1, using templates.extend to extend a template, fields of type list in the derived template will be prepended to the field in the base template (which is a good feature, especially when overriding DNS/route rules). However, it raises an issue when extending servers (DNS servers) in a template.

For example, for the following configuration:

{
  "templates": [
    {
      "name": "base",
      "dns": "tls://8.8.8.8",
      "dns_local": "https://223.5.5.5/dns-query"
    },
    {
      "name": "derived",
      "servers": [
        {
          "tag": "internal",
          "address": "100.100.100.100"
        }
      ]
    }
  ]
}

the derived template produces the following DNS config:

{
  "dns": {
    "servers": [
      {
        "tag": "internal",
        "address": "100.100.100.100"
      },
      {
        "tag": "default",
        "address": "tls://8.8.8.8"
      },
      {
        "tag": "local",
        "address": "https://223.5.5.5/dns-query",
        "strategy": "prefer_ipv4",
        "detour": "direct"
      }
    ],
    "strategy": "ipv4_only"
  }
}

The newly added DNS server internal is the first in the list, therefore becomes the final DNS server2, which is not desired. We desire the original default DNS server to be the final one.

Therefore, we propose serenity to set dns.final to default (or user-specified value) to avoid this issue. It seems to require only one line of change:

--- a/template/render_dns.go
+++ b/template/render_dns.go
@@ -63,6 +63,7 @@ func (t *Template) renderDNS(metadata M.Metadata, options *option.Options) error
 		defaultDNSOptions.AddressResolver = DNSLocalTag
 	}
 	options.DNS.Servers = append(options.DNS.Servers, defaultDNSOptions)
+	options.DNS.Final = DNSDefaultTag  // or user-specified value
 	var (
 		localDNSOptions  option.DNSServerOptions
 		localDNSIsDomain bool

Footnotes

  1. https://github.com/SagerNet/sing/blob/9f69e7f9f7e2c4fc26828d98ec9f81ab72fdec98/common/json/badjson/merge.go#L111-L120

  2. https://github.com/SagerNet/sing-box/blob/77e4d86fa02d2f476e7a6767e8ea1de830a0b8b0/route/router.go#L268

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

No branches or pull requests

1 participant