ccding / go-stun

A go implementation of the STUN client (RFC 3489 and RFC 5389)
Apache License 2.0
662 stars 120 forks source link

Doesn't work with some servers #9

Open AlexKMDev opened 8 years ago

AlexKMDev commented 8 years ago

For example google ones:

stun.l.google.com:19302 stun1.l.google.com:19302 stun2.l.google.com:19302 stun3.l.google.com:19302 stun4.l.google.com:19302

Test1 fails with "No changed address." error. Any idea what could be wrong?

chrikoch commented 8 years ago

I'm still getting the error "Server error: no changed address." using the google servers.

EDIT: Google servers seem to have changed their port. But when using Port 19305 the returned *Host is nil.

xiang90 commented 8 years ago

@chrikoch Can you please provide a reproduce step?

chrikoch commented 8 years ago

@xiang90

My Code:

func minimalExample() { stunClient := stun.NewClient() stunClient.SetServerHost("stun1.l.google.com", 19305) _, host, err := stunClient.Discover()

if err != nil {
    log.Println(err)
    return
}

if host == nil {
    log.Println("no host")
    return
}

}

Output is: 2016/08/20 22:33:27 no host

ccding commented 8 years ago

I think there is a bug in the Google servers. In test2, we send a request to the server with flag of changing IP and port. But the received packet is from the original IP and port (not changed).

Let me know if you think it is not Google servers' bug and have any hints to me. Thanks.

ccding commented 8 years ago

Sorry I messed up this bug with another one. Please ignore the above comment. See below:

In the test1 response from Google servers, it doesn't provide a changedAddr attribute, which is required by RFC3489. I think this is a bug in Google servers. I think the previous fix is incorrect.

chrikoch commented 8 years ago

@ccding @xiang90 I tried to debug this a bit more. Now I think 19302 seems to be the correct port for googles stun servers. Requests to 19305 don't give any response in wireshark: 19305 So it's even stranger the lib doesn't return an error but a nil host.

Requests to 19302 do get a response from google: 19302 Output of my testing code is in that case: 2016/08/21 20:39:26 Server error: no changed address.

ccding commented 8 years ago

What I meant was that, the response doesn't include a CHANGED-ADDRESS field, which is required by RFC (see https://tools.ietf.org/html/rfc3489#section-11.2.3)

chrikoch commented 8 years ago

Actually I was looking for a stun library working with in-the-wild-servers, even if they don't respect the RFC ;-)

ccding commented 8 years ago

If a server doesn't follow RFC, how can I understand its response given the fact that the server itself doesn't provide any documentation? I'd suggest you to use this server stun.ekiga.net:3478

ccding commented 8 years ago

The reason RFC requires this filed is that we need this CHANGED-ADDRESS field to perform the next step test. If it doesn't exist, I cannot perform the next step. Therefore I have to report an error.

xiang90 commented 8 years ago

@ccding @chrikoch I think we want to accept the patch if it makes go-stun more adoptive to the wild. However, the patch itself needs to be well documented and isolated to make it clear that it exists for the purpose of one wild deployed service.

@chrikoch Do you want to help on this? Thank you!

ccding commented 8 years ago

It cannot be more adoptive to the wild.

1) There is another bug in google servers: in test2, when I request to change IP and port, the server doesn't change it. If I don't report error in CHANGED-ADDRESS, the error of not changing IP and port will be reported later.

2) If the NAT is not sym. UPD firewall or open Internet, by RFC, I have to perform another test: send packet to the CHANGED-ADDRESS specified in test1. Given the CHANGED-ADDRESS is not specified in test1, I don't know where to send the packet. The CHANGED-ADDRESS error will be reported anyway.

chrikoch commented 8 years ago

I don't think there is a bug in the google servers. Obviously they're using STUN from RFC 5389, which obsoletes RFC 3489 (which seems to be implemented in go-stun). The CHANGED-ADDRESS was removed from STUN:

"[...] CHANGED-ADDRESS, that have been removed from this specification"

See: https://tools.ietf.org/html/rfc5389#section-12

InsZVA commented 7 years ago

in RFC5780, CHANGED-ADDRESS has been replaced by OTHER-ADDRESS

ccding commented 7 years ago

see #20

yonderblue commented 5 years ago

Perhaps this is the wrong issue, but when I run the tool behind a mobile network I always hit the Server error: response IP/port errors. I've tried many servers, and ones that work behind normal home networks.

@ccding do you know where in the spec the fact that it returns from a diff ip/port that it invalidates the algo?

ccding commented 5 years ago

https://tools.ietf.org/html/rfc3489#section-11.2.3

robertsdotpm commented 3 months ago

I'm working on some stun code at the moment and what I've learned is that RFC 3489 was obsoleted by RFC 5389 which in turn was obsoleted by RFC 8489. What does this mean? Well, in RFC 3489 it had 'change requests'; You could send a request that made the server respond from a different IP and port. The corresponding 'change IP' and 'change port' fields in a response reflect what other addresses the server has (servers that support STUN fully have two addresses and run servers on at least two ports.) The recent RFC standards remove these change requests -- which yeah -- kind of completely screws up a lot of STUN code.

Additionally, there's a few other things I've found that are relevant now. There's a 'magic cookie'. It's an 8 byte fixed sequence that gets added to every packet. It takes up a portion of the transaction ID (which is now reduced from 16 bytes to 12.) Some servers also request a message integrity field. Probably many DIY STUN clients aren't going to work because they're written to run against old standards or servers that implement the standards so loosely that they mislead developers into thinking their implementations are sound. There's probably more that I'm missing but that's what I've learned so far.

ccding commented 3 months ago

@robertsdotpm are you available to help fixing the issues of this repo?

robertsdotpm commented 2 months ago

@robertsdotpm are you available to help fixing the issues of this repo?

I think it was a little different to how I expected. The repo seems to send the magic cookie. There are servers that are coded like: 'if you receive a magic cookie then operate in RFC 5389 >= mode' so it means that servers that could potentially support RFC 3489 wouldn't work because they're not operating in that mode. This isn't a theoretical concern. I believe the most popular STUN server software (Coturn) does this.

The way I've solved this issue with my code is I have two lists of servers. One I call 'change' servers and one for 'mapping.' Change servers correctly implement 3489 behaviour while mapping server only allow the regular bind requests. I also don't assume that the list a server ends up in corresponds to the 'mode' you have to speak to it which allows for more flexibility. I save the mode with the server which mostly controls whether the magic cookie is used (and some edge-cases with XOR encoded addressing that you could probably skip since there is also a mapped address field that is mostly included anyway.)

So anyway and counter-intuitively: what I suggest for this project is --not-- to send the magic cookie for better results. But ideally the behaviour above would be a little more flexible. Unfortunately, I don't have the time to put in a pull request.

Edit: setting the field to anything you like would be an appropriate fix. Even all 00s would work. Also, forgot to mention, those STUN servers in the OP don't support RFC 3489 so you won't see change fields in the reply. The only reason that Google's STUN servers are answering you, too, is because the magic cookie is being sent. But since it doesn't support what you want this isn't really desired.