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

Jump to bottom

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
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@elmauromx
Copy link

elmauromx commented Mar 19, 2019 •
edited by ALTree

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 commented Mar 20, 2019

/cc @mpvl, @vdobler

@elmauromx
Copy link
Author

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

@katiehockman
Copy link
Contributor

@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.

@katiehockman katiehockman added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 22, 2019
@elmauromx
Copy link
Author

@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 รืเ

@elmauromx
Copy link
Author

Hi. Any update on this?

@elmauromx
Copy link
Author

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

@katiehockman
Copy link
Contributor

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

@frickenate
Copy link

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.

@TimothyGu
Copy link
Contributor

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.

@gopherbot
Copy link

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

@TimothyGu TimothyGu mentioned this issue Oct 25, 2021
Closed
gopherbot pushed a commit to golang/text that referenced this issue Oct 29, 2021
@TimothyGu @neild
internal/export/idna: make Transitional an actual toggle
bb1c798 
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 commented Nov 4, 2021

Fixed by https://golang.org/cl/360381.

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

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.

@TimothyGu
Copy link
Contributor

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).

@seankhliao seankhliao added this to the Unplanned milestone Aug 20, 2022
xhit pushed a commit to xhit/text that referenced this issue Oct 10, 2022
@TimothyGu @xhit
internal/export/idna: make Transitional an actual toggle
8db1450 
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
8 participants
@neild @mikioh @frickenate @TimothyGu @gopherbot @seankhliao @katiehockman @elmauromx