Skip to content

Commit c11a523

Browse files
tlimoncellitlimoncelli
authored
FEATURE: Fixing IDN support for domains (#3879)
# Issue The previous fix had backwards compatibility issues and treated uppercase Unicode incorrectly. # Resolution * Don't call strings.ToUpper() on Unicode strings. Only call it on the output of ToASCII. * Fix BIND's "filenameformat" to be more compatible (only breaks if you had uppercase unicode in a domain name... which you probably didn't) * Change IDN to ASCII in most places (Thanks for the suggestion, @KaiSchwarz-cnic!) * Update BIND documentation
1 parent e87f03a commit c11a523

File tree

9 files changed

+269
-135
lines changed

9 files changed

+269
-135
lines changed

‎documentation/provider/bind.md‎

Lines changed: 61 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,18 @@ Example:
2222
{
2323
"bind": {
2424
"TYPE": "BIND",
25-
"directory": "myzones",
26-
"filenameformat": "%U.zone"
25+
"directory": "myzones"
2726
}
2827
}
2928
```
3029
{% endcode %}
3130

31+
As of v4.2.0 `dnscontrol push` will create subdirectories along the path to
32+
the filename. This includes both the portion of the path created by the
33+
`directory` setting and the `filenameformat` setting. For security reasons, the
34+
automatic creation of subdirectories is disabled if `dnscontrol` is running as
35+
root. (Running DNSControl as root is not recommended in general.)
36+
3237
## Meta configuration
3338

3439
This provider accepts some optional metadata in the `NewDnsProvider()` call.
@@ -85,43 +90,59 @@ DNSControl does not handle special serial number math such as "looping through z
8590
# filenameformat
8691

8792
The `filenameformat` parameter specifies the file name to be used when
88-
writing the zone file. The default (`%U.zone`) is acceptable in most cases: the
93+
writing the zone file. The default (`%c.zone`) is acceptable in most cases: the
8994
file name is the name as specified in the `D()` function plus ".zone".
9095

9196
The filenameformat is a string with a few printf-like `%` verbs:
9297

93-
* The domain name without tag (the `example.com` part of `example.com!tag`):
94-
* `%D` as specified in `D()` (no IDN conversion, but downcased)
95-
* `%I` converted to IDN/Punycode (`xn--...`) and downcased.
96-
* `%N` converted to Unicode (downcased first)
97-
* `%T` the split horizon tag, or "" (the `tag` part of `example.com!tag`)
98-
* `%?x` this returns `x` if the split horizon tag is non-null, otherwise nothing. `x` can be any printable but is usually `!`.
99-
* `%U` short for "%I%?!%T". This is the universal, canonical, name for the domain used for comparisons within DNSControl. This is best for filenames which is why it is used in the default.
100-
* `%%` `%`
101-
* ordinary characters (not `%`) are copied unchanged to the output stream
102-
* FYI: format strings must not end with an incomplete `%` or `%?`
103-
104-
Typical values:
105-
106-
* `%U.zone` (The default)
107-
* `example.com.zone` or `example.com!tag.zone`
108-
* `%T%?_%I.zone` (optional tag and `_` + domain + `.zone`)
109-
* `tag_example.com.zone` or `example.com.zone`
110-
* `db_%T%?_%D`
111-
* `db_inside_example.com` or `db_example.com`
112-
113-
{% hint style="warning" %}
114-
**Warning** DNSControl will not warn you if two zones generate the same
115-
filename. Instead, each will write to the same place. The content would end up
116-
flapping back and forth between the two. The best way to prevent this is to
117-
always include the tag (`%T`) or use `%U` which includes the tag.
118-
{% endhint %}
119-
120-
(new in v4.2.0) `dnscontrol push` will create subdirectories along the path to
121-
the filename. This includes both the portion of the path created by the
122-
`directory` setting and the `filenameformat` setting. The automatic creation of
123-
subdirectories is disabled if `dnscontrol` is running as root for security
124-
reasons.
98+
| Verb | Description | `EXAMple.com` | `EXAMple.com!MyTag` | `рф.com!myTag` |
99+
| ------- | ------------------------------------------------- | ------------- | ------------------- | -------------------- |
100+
| `%T` | the tag | "" (null) | `myTag` | `myTag` |
101+
| `%c` | canonical name, globally unique and comparable | `example.com` | `example.com!myTag` | `xn--p1ai.com!myTag` |
102+
| `%a` | ASCII domain (Punycode, downcased) | `example.com` | `example.com` | `xn--p1ai.com` |
103+
| `%u` | Unicode domain (non-Unicode parts downcased) | `example.com` | `example.com` | `рф.com` |
104+
| `%r` | Raw (unmodified) Domain from `D()` (risky!) | `EXAMple.com` | `EXAMple.com` | `рф.com` |
105+
| `%f` | like `%c` but better for filenames (`%a%?_%T`) | `example.com` | `example.com_myTag` | `xn--p1ai.com_myTag` |
106+
| `%F` | like `%f` but reversed order (`%T%?_%a`) | `example.com` | `myTag_example.com` | `myTag_xn--p1ai.com` |
107+
| `%?x` | returns `x` if tag exists, otherwise "" | "" (null) | `x` | `x` |
108+
| `%%` | a literal percent sign | `%` | `%` | `%` |
109+
| `a-Z./` | other printable characters are copied exactly | `a-Z./` | `a-Z./` | `a-Z./` |
110+
| `%U` | (deprecated, use `%c`) Same as `%D%?!%T` (risky!) | `example.com` | `example.com!myTag` | `рф.com!myTag` |
111+
| `%D` | (deprecated, use `%r`) mangles Unicode (risky!) | `example.com` | `example.com` | `рф.com` |
112+
113+
* `%?x` is typically used to generate an optional `!` or `_` if there is a tag.
114+
* `%r` is considered "risky" because it can produce a domain name that is not
115+
canonical. For example, if you use `D("FOO.com")` and later change it to `D("foo.com")`, your file names will change.
116+
* Format strings must not end with an incomplete `%` or `%?`
117+
* Generating a filename without a tag is risky. For example, if the same
118+
`dnsconfig.js` has `D("example.com!inside", DSP_BIND)` and
119+
`D("example.com!outside", DSP_BIND)`, both will use the same filename.
120+
DNSControl will write both zone files to the same file, flapping between the
121+
two. No error or warning will be output.
122+
123+
Useful examples:
124+
125+
| Verb | Description | `EXAMple.com` | `EXAMple.com!MyTag` | `рф.com!myTag` |
126+
| ------------ | ----------------------------------- | ------------------ | ------------------------ | ------------------------- |
127+
| `%c.zone` | Default format (v4.28 and later) | `example.com.zone` | `example.com!myTag.zone` | `xn--p1ai.com!myTag.zone` |
128+
| `%U.zone` | Default format (pre-v4.28) (risky!) | `example.com.zone` | `example.com!myTag.zone` | `рф.com!myTag.zone` |
129+
| `db_%f` | Recommended in a popular DNS book | `db_example.com` | `db_example.com_myTag` | `db_xn--p1ai.com_myTag` |
130+
| `db_%a%?_%T` | same as above but using `%?_` | `db_example.com` | `db_example.com_myTag` | `db_xn--p1ai.com_myTag` |
131+
132+
Compatibility notes:
133+
134+
* `%D` should not be used. It downcases the string in a way that is probably
135+
incompatible with Unicode characters. It is retained for compatibility with
136+
pre-v4.28 releases. If your domain has capital Unicode characters, backwards
137+
compatibility is not guaranteed. Use `%r` instead.
138+
* `%U` relies on `%D` which is deprecated. Use `%c` instead.
139+
* As of v4.28 the default format string changed from `%U.zone` to `%c.zone`. This
140+
should only matter if your `D()` statements included non-ASCII (Unicode)
141+
runes that were capitalized.
142+
* If you are using pre-v4.28 releases the above table is slightly misleading
143+
because uppercase ASCII letters do not always work. If you are using
144+
pre-v4.28 releases, assume the above table lists `example.com` instead
145+
of `EXAMpl.com`.
125146

126147
# FYI: get-zones
127148

@@ -132,7 +153,8 @@ any files named `*.zone` and assumes they are zone files.
132153
dnscontrol get-zones --format=nameonly - BIND all
133154
```
134155

135-
If `filenameformat` is defined, `dnscontrol` makes a guess at which
136-
filenames are zones but doesn't try to hard to get it right, which is
137-
mathematically impossible in some cases. Feel free to file an issue if
138-
your format string doesn't work. I love a challenge!
156+
If `filenameformat` is defined, `dnscontrol` makes a guess at which filenames
157+
are zones by reversing the logic of the format string. It doesn't try very hard
158+
to get this right, as getting it right in all situations is mathematically
159+
impossible. Feel free to file an issue if find a situation where it doesn't
160+
work. I love a challenge!

‎integrationTest/integration_test.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2033,11 +2033,6 @@ func makeTests() []*TestGroup {
20332033
),
20342034
),
20352035

2036-
// This MUST be the last test.
2037-
testgroup("final",
2038-
tc("final", txt("final", `TestDNSProviders was successful!`)),
2039-
),
2040-
20412036
testgroup("SMIMEA",
20422037
requires(providers.CanUseSMIMEA),
20432038
tc("SMIMEA record", smimea("_443._tcp", 3, 1, 1, sha256hash)),
@@ -2060,6 +2055,12 @@ func makeTests() []*TestGroup {
20602055
// every quarter. There may be library updates, API changes,
20612056
// etc.
20622057

2058+
// This SHOULD be the last test. We do this so that we always
2059+
// leave zones with a single TXT record exclaming our success.
2060+
// Nothing depends on this record existing or should depend on it.
2061+
testgroup("final",
2062+
tc("final", txt("final", `TestDNSProviders was successful!`)),
2063+
),
20632064
}
20642065

20652066
return tests

‎models/domain.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ const (
1313
DomainTag = "dnscontrol_tag" // A copy of DomainConfig.Tag
1414
DomainUniqueName = "dnscontrol_uniquename" // A copy of DomainConfig.UniqueName
1515
DomainNameRaw = "dnscontrol_nameraw" // A copy of DomainConfig.NameRaw
16-
DomainNameIDN = "dnscontrol_nameidn" // A copy of DomainConfig.NameIDN
16+
DomainNameASCII = "dnscontrol_nameascii" // A copy of DomainConfig.NameASCII
1717
DomainNameUnicode = "dnscontrol_nameunicode" // A copy of DomainConfig.NameUnicode
1818
)
1919

@@ -74,15 +74,15 @@ func (dc *DomainConfig) PostProcess() {
7474

7575
// Turn the user-supplied name into the fixed forms.
7676
ff := domaintags.MakeDomainFixForms(dc.Name)
77-
dc.Tag, dc.NameRaw, dc.Name, dc.NameUnicode, dc.UniqueName = ff.Tag, ff.NameRaw, ff.NameIDN, ff.NameUnicode, ff.UniqueName
77+
dc.Tag, dc.NameRaw, dc.Name, dc.NameUnicode, dc.UniqueName = ff.Tag, ff.NameRaw, ff.NameASCII, ff.NameUnicode, ff.UniqueName
7878

7979
// Store the FixForms is Metadata so we don't have to change the signature of every function that might need them.
8080
// This is a bit ugly but avoids a huge refactor. Please avoid using these to make the future refactor easier.
8181
if dc.Tag != "" {
8282
dc.Metadata[DomainTag] = dc.Tag
8383
}
8484
//dc.Metadata[DomainNameRaw] = dc.NameRaw
85-
//dc.Metadata[DomainNameIDN] = dc.Name
85+
//dc.Metadata[DomainNameASCII] = dc.Name
8686
//dc.Metadata[DomainNameUnicode] = dc.NameUnicode
8787
dc.Metadata[DomainUniqueName] = dc.UniqueName
8888
}

‎pkg/domaintags/domaintags.go‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import (
99
// DomainFixedForms stores the various fixed forms of a domain name and tag.
1010
type DomainFixedForms struct {
1111
NameRaw string // "originalinput.com" (name as input by the user, lowercased (no tag))
12-
NameIDN string // "punycode.com"
12+
NameASCII string // "punycode.com"
1313
NameUnicode string // "unicode.com" (converted to downcase BEFORE unicode conversion)
1414
UniqueName string // "punycode.com!tag"
1515

@@ -25,7 +25,7 @@ type DomainFixedForms struct {
2525
// * .UniqueName: "example.com!tag" unique across the entire config.
2626
func MakeDomainFixForms(n string) DomainFixedForms {
2727
var err error
28-
var tag, nameRaw, nameIDN, nameUnicode, uniqueName string
28+
var tag, nameRaw, nameASCII, nameUnicode, uniqueName string
2929
var hasBang bool
3030

3131
// Split tag from name.
@@ -38,42 +38,45 @@ func MakeDomainFixForms(n string) DomainFixedForms {
3838
hasBang = false
3939
}
4040

41-
nameRaw = strings.ToLower(p[0])
41+
nameRaw = p[0]
4242
if strings.HasPrefix(n, nameRaw) {
4343
// Avoid pointless duplication.
4444
nameRaw = n[0:len(nameRaw)]
4545
}
4646

47-
nameIDN, err = idna.ToASCII(nameRaw)
47+
nameASCII, err = idna.ToASCII(nameRaw)
4848
if err != nil {
49-
nameIDN = nameRaw // Fallback to raw name on error.
49+
nameASCII = nameRaw // Fallback to raw name on error.
5050
} else {
51+
nameASCII = strings.ToLower(nameASCII)
5152
// Avoid pointless duplication.
52-
if nameIDN == nameRaw {
53-
nameIDN = nameRaw
53+
if strings.HasPrefix(n, nameASCII) {
54+
// Avoid pointless duplication.
55+
nameASCII = n[0:len(nameASCII)]
5456
}
5557
}
5658

57-
nameUnicode, err = idna.ToUnicode(nameRaw)
59+
nameUnicode, err = idna.ToUnicode(nameASCII) // We use nameASCII since it is already lowercased.
5860
if err != nil {
5961
nameUnicode = nameRaw // Fallback to raw name on error.
6062
} else {
6163
// Avoid pointless duplication.
62-
if nameUnicode == nameRaw {
63-
nameUnicode = nameRaw
64+
if strings.HasPrefix(n, nameUnicode) {
65+
// Avoid pointless duplication.
66+
nameUnicode = n[0:len(nameUnicode)]
6467
}
6568
}
6669

6770
if hasBang {
68-
uniqueName = nameIDN + "!" + tag
71+
uniqueName = nameASCII + "!" + tag
6972
} else {
70-
uniqueName = nameIDN
73+
uniqueName = nameASCII
7174
}
7275

7376
return DomainFixedForms{
7477
Tag: tag,
7578
NameRaw: nameRaw,
76-
NameIDN: nameIDN,
79+
NameASCII: nameASCII,
7780
NameUnicode: nameUnicode,
7881
UniqueName: uniqueName,
7982
HasBang: hasBang,

‎pkg/domaintags/domaintags_test.go‎

Lines changed: 48 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ func Test_MakeDomainFixForms(t *testing.T) {
1010
input string
1111
wantTag string
1212
wantNameRaw string
13-
wantNameIDN string
13+
wantNameASCII string
1414
wantNameUnicode string
1515
wantUniqueName string
1616
wantHasBang bool
@@ -20,7 +20,7 @@ func Test_MakeDomainFixForms(t *testing.T) {
2020
input: "example.com",
2121
wantTag: "",
2222
wantNameRaw: "example.com",
23-
wantNameIDN: "example.com",
23+
wantNameASCII: "example.com",
2424
wantNameUnicode: "example.com",
2525
wantUniqueName: "example.com",
2626
wantHasBang: false,
@@ -30,7 +30,7 @@ func Test_MakeDomainFixForms(t *testing.T) {
3030
input: "example.com!mytag",
3131
wantTag: "mytag",
3232
wantNameRaw: "example.com",
33-
wantNameIDN: "example.com",
33+
wantNameASCII: "example.com",
3434
wantNameUnicode: "example.com",
3535
wantUniqueName: "example.com!mytag",
3636
wantHasBang: true,
@@ -40,7 +40,7 @@ func Test_MakeDomainFixForms(t *testing.T) {
4040
input: "example.com!",
4141
wantTag: "",
4242
wantNameRaw: "example.com",
43-
wantNameIDN: "example.com",
43+
wantNameASCII: "example.com",
4444
wantNameUnicode: "example.com",
4545
wantUniqueName: "example.com!",
4646
wantHasBang: true,
@@ -50,7 +50,7 @@ func Test_MakeDomainFixForms(t *testing.T) {
5050
input: "उदाहरण.com",
5151
wantTag: "",
5252
wantNameRaw: "उदाहरण.com",
53-
wantNameIDN: "xn--p1b6ci4b4b3a.com",
53+
wantNameASCII: "xn--p1b6ci4b4b3a.com",
5454
wantNameUnicode: "उदाहरण.com",
5555
wantUniqueName: "xn--p1b6ci4b4b3a.com",
5656
wantHasBang: false,
@@ -60,7 +60,7 @@ func Test_MakeDomainFixForms(t *testing.T) {
6060
input: "उदाहरण.com!mytag",
6161
wantTag: "mytag",
6262
wantNameRaw: "उदाहरण.com",
63-
wantNameIDN: "xn--p1b6ci4b4b3a.com",
63+
wantNameASCII: "xn--p1b6ci4b4b3a.com",
6464
wantNameUnicode: "उदाहरण.com",
6565
wantUniqueName: "xn--p1b6ci4b4b3a.com!mytag",
6666
wantHasBang: true,
@@ -70,17 +70,29 @@ func Test_MakeDomainFixForms(t *testing.T) {
7070
input: "xn--p1b6ci4b4b3a.com",
7171
wantTag: "",
7272
wantNameRaw: "xn--p1b6ci4b4b3a.com",
73-
wantNameIDN: "xn--p1b6ci4b4b3a.com",
73+
wantNameASCII: "xn--p1b6ci4b4b3a.com",
7474
wantNameUnicode: "उदाहरण.com",
7575
wantUniqueName: "xn--p1b6ci4b4b3a.com",
7676
wantHasBang: false,
7777
},
78+
{
79+
// Unicode chars should be left alone (as far as case folding goes)
80+
// Here are some Armenian characters https://tools.lgm.cl/lettercase.html
81+
name: "mixed case unicode",
82+
input: "fooԷէԸըԹ.com!myTag",
83+
wantTag: "myTag",
84+
wantNameRaw: "fooԷէԸըԹ.com",
85+
wantNameASCII: "xn--foo-b7dfg43aja.com",
86+
wantNameUnicode: "fooԷէԸըԹ.com",
87+
wantUniqueName: "xn--foo-b7dfg43aja.com!myTag",
88+
wantHasBang: true,
89+
},
7890
{
7991
name: "punycode domain with tag",
8092
input: "xn--p1b6ci4b4b3a.com!mytag",
8193
wantTag: "mytag",
8294
wantNameRaw: "xn--p1b6ci4b4b3a.com",
83-
wantNameIDN: "xn--p1b6ci4b4b3a.com",
95+
wantNameASCII: "xn--p1b6ci4b4b3a.com",
8496
wantNameUnicode: "उदाहरण.com",
8597
wantUniqueName: "xn--p1b6ci4b4b3a.com!mytag",
8698
wantHasBang: true,
@@ -89,8 +101,8 @@ func Test_MakeDomainFixForms(t *testing.T) {
89101
name: "mixed case domain",
90102
input: "Example.COM",
91103
wantTag: "",
92-
wantNameRaw: "example.com",
93-
wantNameIDN: "example.com",
104+
wantNameRaw: "Example.COM",
105+
wantNameASCII: "example.com",
94106
wantNameUnicode: "example.com",
95107
wantUniqueName: "example.com",
96108
wantHasBang: false,
@@ -99,12 +111,34 @@ func Test_MakeDomainFixForms(t *testing.T) {
99111
name: "mixed case domain with tag",
100112
input: "Example.COM!MyTag",
101113
wantTag: "MyTag",
102-
wantNameRaw: "example.com",
103-
wantNameIDN: "example.com",
114+
wantNameRaw: "Example.COM",
115+
wantNameASCII: "example.com",
104116
wantNameUnicode: "example.com",
105117
wantUniqueName: "example.com!MyTag",
106118
wantHasBang: true,
107119
},
120+
// This is used in the documentation for the BIND provider, thus we test
121+
// it to make sure we got it right.
122+
{
123+
name: "BIND example 1",
124+
input: "рф.com!myTag",
125+
wantTag: "myTag",
126+
wantNameRaw: "рф.com",
127+
wantNameASCII: "xn--p1ai.com",
128+
wantNameUnicode: "рф.com",
129+
wantUniqueName: "xn--p1ai.com!myTag",
130+
wantHasBang: true,
131+
},
132+
{
133+
name: "BIND example 2",
134+
input: "рф.com",
135+
wantTag: "",
136+
wantNameRaw: "рф.com",
137+
wantNameASCII: "xn--p1ai.com",
138+
wantNameUnicode: "рф.com",
139+
wantUniqueName: "xn--p1ai.com",
140+
wantHasBang: false,
141+
},
108142
}
109143

110144
for _, tt := range tests {
@@ -116,8 +150,8 @@ func Test_MakeDomainFixForms(t *testing.T) {
116150
if got.NameRaw != tt.wantNameRaw {
117151
t.Errorf("MakeDomainFixForms() gotNameRaw = %v, want %v", got.NameRaw, tt.wantNameRaw)
118152
}
119-
if got.NameIDN != tt.wantNameIDN {
120-
t.Errorf("MakeDomainFixForms() gotNameIDN = %v, want %v", got.NameIDN, tt.wantNameIDN)
153+
if got.NameASCII != tt.wantNameASCII {
154+
t.Errorf("MakeDomainFixForms() gotNameASCII = %v, want %v", got.NameASCII, tt.wantNameASCII)
121155
}
122156
if got.NameUnicode != tt.wantNameUnicode {
123157
t.Errorf("MakeDomainFixForms() gotNameUnicode = %v, want %v", got.NameUnicode, tt.wantNameUnicode)

0 commit comments

Comments
 (0)