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

x/net/idna: context validation #30940

Open
elmauromx opened this issue Mar 19, 2019 · 13 comments
Open

x/net/idna: context validation #30940

elmauromx opened this issue Mar 19, 2019 · 13 comments

Comments

@elmauromx
Copy link

@elmauromx elmauromx commented Mar 19, 2019

What version of Go are you using (go version)?

$ go version
go version go1.11.5 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env

GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"

What did you do?

I wrote an example to convert a-label to u-label an viceversa and also perform IDNA2008 validation based on the provided a-label.

I checked the results with other IDNA tools and found that with net/idna I am not getting context errors that other libraries are reporting.

Here is the main code of the example:

func main() {
	p = idna.New(
		idna.BidiRule(),
		idna.MapForLookup(),
		idna.StrictDomainName(true),
		idna.Transitional(false),
		idna.ValidateForRegistration(),
		idna.ValidateLabels(true),
		idna.VerifyDNSLength(true))
	alabel := os.Args[1]

	ulabel, error := p.ToUnicode(alabel)
	if error != nil {
		fmt.Printf("Error converting string: %v\n", error)
		os.Exit(1)
	}
	fmt.Println(alabel,ulabel)

	convertedAlabel, error := p.ToASCII(ulabel)
	if error != nil {
		fmt.Printf("Error converting string: %v\n", error)
		os.Exit(1)
	}

	if convertedAlabel != alabel {
		fmt.Printf("Provide a-label doesn't match converted a-label: %v\n", convertedAlabel)
		fmt.Println(convertedAlabel)
		os.Exit(1)
	}

}

Here are some label examples:

xn--diethealth-zp5i
xn--diy-ps4bb
xn--pfs-ps4bb

Can you please confirm if this is missing validation of IDNA2008?

Could it be possible to include one new method/flag to get full IDNA2008 validation with the function?

What did you expect to see?

Here are the errors reported for 2 other idna utilities:

 xn--diethealth-zp5i,string contains a forbidden context-o character,Codepoint U+30FB not allowed at position 5 in 'diet・health'
  xn--diy-ps4bb,string contains a forbidden context-o character,Codepoint U+30FB not allowed at position 2 in 'd・i・y'
  xn--pfs-ps4bb,string contains a forbidden context-o character,Codepoint U+30FB not allowed at position 2 in 'p・f・s'

What did you see instead?

xn--diethealth-zp5i diet・health
xn--diy-ps4bb d・i・y
xn--pfs-ps4bb p・f・s
@mikioh mikioh changed the title net/idna context validation x/net/idna: context validation Mar 20, 2019
@mikioh
Copy link
Contributor

@mikioh mikioh commented Mar 20, 2019

Loading

@elmauromx
Copy link
Author

@elmauromx elmauromx commented Mar 21, 2019

I would like to add another case of what appears to be a false negative (xn--03c4b1a). The program is reporting:

Error converting string: idna: invalid label "ิรืเ"

Other IDNA utilities successfully convert a-label to u-label an viceversa

Loading

@katiehockman
Copy link
Member

@katiehockman katiehockman commented Mar 22, 2019

@elmauromx can you confirm that this same issue happens on the latest version of Go? It looks like you are running on go1.11.5.

Loading

@elmauromx
Copy link
Author

@elmauromx elmauromx commented Mar 22, 2019

@katiehockman . It is the same behavior with go1.12.1 for the first case reported:

$HOME/go/bin/go version
go version go1.12.1 linux/amd64

xn--diethealth-zp5i diet・health
xn--diy-ps4bb d・i・y
xn--pfs-ps4bb p・f・s

For xn--03c4b1a it is working good on the new version:

xn--03c4b1a รืเ

Loading

@elmauromx
Copy link
Author

@elmauromx elmauromx commented Apr 18, 2019

Hi. Any update on this?

Loading

@elmauromx
Copy link
Author

@elmauromx elmauromx commented Jul 26, 2019

Is there any update on this? Any plans to fix it?

Loading

@katiehockman
Copy link
Member

@katiehockman katiehockman commented Jul 30, 2019

@mpvl have you had a chance to look into this? Or is there another person that can investigate?

Loading

@frickenate
Copy link

@frickenate frickenate commented Apr 25, 2021

FYI, calling idna.Transitional(false) is bugged; internally the flag is set to true regardless of which boolean value you pass. The only way to get false is to not call Transitional() at all.

Permalink to the method in current master; the method takes transitional bool, but the body of the function is o.transitional = true instead of o.transitional = transitional.

Loading

@TimothyGu
Copy link
Contributor

@TimothyGu TimothyGu commented May 6, 2021

I looked into this. For now, the idna package implements UTS 46, which has no CONTEXTO requirement – though CONTEXTJ checks are widely deployed. We could certainly add a flag for CONTEXTO support though, for registration profiles. Here's what icu4j's documentation says about CONTEXTO:

This is for use by registries for IDNA2008 conformance. UTS #46 does not require the CONTEXTO check.

Loading

@gopherbot
Copy link

@gopherbot gopherbot commented May 6, 2021

Change https://golang.org/cl/317729 mentions this issue: internal/export/idna: fix Transitional

Loading

gopherbot pushed a commit to golang/text that referenced this issue Oct 29, 2021
Previously, it always enabled transitional processing instead of
toggling, despite the fact that it took a boolean argument.

For golang/go#30940.

Change-Id: I00ad51ec55abfb2de28deb8c98f949989ece1099
Reviewed-on: https://go-review.googlesource.com/c/text/+/317729
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Trust: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
@neild
Copy link
Contributor

@neild neild commented Nov 4, 2021

Loading

@neild neild closed this Nov 4, 2021
@neild neild reopened this Nov 4, 2021
@neild
Copy link
Contributor

@neild neild commented Nov 4, 2021

Maybe fixed? Is the issue here just that idna.Transitional(false) didn't work (now fixed), or is there something in addition to that? I thought the former, but rereading the comments I'm no longer certain.

Loading

@TimothyGu
Copy link
Contributor

@TimothyGu TimothyGu commented Nov 4, 2021

https://golang.org/cl/360381 only fixes part of the bug. The original bug here is the lack of CONTEXTO support, which is not yet added, and which should only be used for the Registration profile. Adding CONTEXTO support essentially means implementing the restrictions in Appendices A.3–9 in RFC 5892.

Notice that we already support CONTEXTJ through CheckJoiners, which is enabled by both ValidateForRegistration and MapForLookup (through ValidateLabels).

Loading

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

Successfully merging a pull request may close this issue.

None yet
7 participants