#18628: Devise some way for the browser proxy to forward metadata to the bridge before the OR data -----------------------------------+-------------------------------- Reporter: arlolra | Owner: Type: defect | Status: needs_revision Priority: High | Milestone: Component: Obfuscation/Snowflake | Version: Severity: Normal | Resolution: Keywords: | Actual Points: Parent ID: | Points: Reviewer: | Sponsor: -----------------------------------+--------------------------------
Comment (by dcf): Review of the patch from comment:4: {{{ @@ -50,6 +50,7 @@ type webRTCConn struct { dc *webrtc.DataChannel pc *webrtc.PeerConnection pr *io.PipeReader + client_ip string } ... @@ -234,7 +235,22 @@ func makePeerConnectionFromOffer(sdp *webrtc.SessionDescription, config *webrtc. panic("short write") } } - conn := &webRTCConn{pc: pc, dc: dc, pr: pr} + conn := &webRTCConn{pc: pc, dc: dc, pr: pr, client_ip: clientIP} }}} Rather than adding a new member to the webRTCConn struct, it's better to put the IP address extraction logic in the [https://golang.org/pkg/net/#Conn RemoteAddr] function. Not only is that what RemoteAddr is for, it will make it easier to write unit test code when the logic is isolated in its own function. It's better if the remote IP address is represented as a [https://golang.org/pkg/net/#IPAddr net.IPAddr], rather than a string. You can get a net.IPAddr from a string using [https://golang.org/pkg/net/#ParseIP net.ParseIP]. You can turn it back into a string (so you can add it to the URL query string) by calling [https://golang.org/pkg/net/#IPAddr.String String()] on it. ---- {{{ - wsConn, err := websocket.Dial(opt.relay, "", opt.relay) + wsConn, err := websocket.Dial(opt.relay + "?client_ip=" + conn.client_ip, "", opt.relay) }}} It's better to build the URL using [https://golang.org/pkg/net/url/ net/url] functions, rather than concatenating strings. That way, we know that escaping will be correct, and it's easier to extend if we want to set additional values in the query string. Something like this: {{{ u, err := url.Parse(opt.relay) if err != nil { panic(err) } u.Query().Set("client_ip", conn.client_ip) wsConn, err := websocket.Dial(u.String(), "", opt.relay) }}} ---- {{{ + //Parse Remote SDP to pass client IP to the server + var clientIP string = "" + remoteSDP := pc.RemoteDescription().Sdp + splitSDP := strings.Split(remoteSDP, "\r\n") + for i := range splitSDP { + if strings.HasPrefix(splitSDP[i], "c=") { + candidSplit := strings.Split(splitSDP[i], " ") + if len(candidSplit) >= 3 { + clientIP = candidSplit[2] + break + } + } + } }}} We need to think about what happens when this code fails to extract an IP address. Currently it will leave clientIP set to `""`, which will cause the websocket URL to be `wss://snowflake.bamsoftware.com/?client_ip=`. Maybe that's what we want, or maybe we want to omit the `client_ip` parameter completely in that case. What do you think? In any case, this code should be moved into webRTCConn.RemoteAddr, like I said in my first note. That function can return nil when there's no match found. -- Ticket URL: <https://trac.torproject.org/projects/tor/ticket/18628#comment:5> Tor Bug Tracker & Wiki <https://trac.torproject.org/> The Tor Project: anonymity online _______________________________________________ tor-bugs mailing list tor-bugs@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs