golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.86k stars 17.65k forks source link

x/net/idna: context validation #30940

Open elmauromx opened 5 years ago

elmauromx commented 5 years ago

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 commented 5 years ago

/cc @mpvl, @vdobler

elmauromx commented 5 years ago

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 commented 5 years ago

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

elmauromx commented 5 years ago

@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 commented 5 years ago

Hi. Any update on this?

elmauromx commented 5 years ago

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

katiehockman commented 5 years ago

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

frickenate commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

neild commented 2 years ago

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

neild commented 2 years ago

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 commented 2 years ago

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