Opened 2 weeks ago

Last modified 11 days ago

#33157 new defect

Client generates SDP with "IN IP4 0.0.0.0", causing proxy to send "client_ip=0.0.0.0" and bridge to send "USERADDR 0.0.0.0:1"

Reported by: dcf Owned by:
Priority: Medium Milestone:
Component: Circumvention/Snowflake Version:
Severity: Normal Keywords:
Cc: cohosh, phw, arlolra, dcf Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

There is a pipeline of relaying the client IP address:

  • The proxy infers the client's IP address by grepping it out of the SDP during ICE negotiation (proxy, proxy-go) and attaches it to the WebSocket connection as a URL query parameter ?client_ip=A.B.C.D.
  • The bridge parses the client_ip query parameter and passes it on to tor with a USERADDR A.B.C.D:1 command on the ExtORPort.
  • tor does geoip lookups and aggregates statistics and ultimately sends them to Tor Metrics for country-specific graphs.

It looks like the pion SDP code puts 0.0.0.0 in the place where proxy and proxy-go look for the remote IP address. This causes the proxy to send ?client_ip=0.0.0.0 to the bridge, and the bridge to send USERADDR 0.0.0.0:1 to tor. I'm not sure that this happens every time; see below for bridge-extra-infos output.

I found it while testing proxy-go with a localhost client and a patch like:

  • proxy-go/snowflake.go

    a b import ( 
    2222       "git.torproject.org/pluggable-transports/snowflake.git/common/messages"
    23        "git.torproject.org/pluggable-transports/snowflake.git/common/safelog"
     23       _ "git.torproject.org/pluggable-transports/snowflake.git/common/safelog"
    2424       "git.torproject.org/pluggable-transports/snowflake.git/common/websocketconn"
    func (c *webRTCConn) Write(b []byte) (int, error) { 
    9393       // log.Printf("webrtc Write %d %+q", len(b), string(b))
    94        log.Printf("Write %d bytes --> WebRTC", len(b))
     94       // log.Printf("Write %d bytes --> WebRTC", len(b))
    9595       if c.dc != nil {
    func (c *webRTCConn) RemoteAddr() net.Addr { 
    114114       clientIP := remoteIPFromSDP(c.pc.RemoteDescription().SDP)
     115       log.Printf("RemoteAddr %+q", c.pc.RemoteDescription().SDP)
    115116       if clientIP == nil {
    func makePeerConnectionFromOffer(sdp *webrtc.SessionDescription, config webrtc.C 
    322323               dc.OnMessage(func(msg webrtc.DataChannelMessage) {
    323                        log.Printf("OnMessage <--- %d bytes", len(msg.Data))
     324                       // log.Printf("OnMessage <--- %d bytes", len(msg.Data))
    324325                       var n int
    func main() { 
    432433       //We want to send the log output through our scrubber first
    433        log.SetOutput(&safelog.LogScrubber{Output: logOutput})
     434       // log.SetOutput(&safelog.LogScrubber{Output: logOutput})
     435       log.SetOutput(logOutput)

For example, the beginning of an SDP string for me is

v=0
o=- 34318359 1580881353 IN IP4 0.0.0.0
s=-
t=0 0
a=fingerprint:sha-256 80:EE:E6:8D:55:07:CB:52:58:7A:CC:61:70:F9:F3:65:DB:4B:D3:69:CB:F9:68:C8:5F:E3:06:3D:D3:90:C1:E6
a=group:BUNDLE 0
m=application 9 DTLS/SCTP 5000
c=IN IP4 0.0.0.0

The client IP address inference, implemented in #18628, was always a bit of a hack, but it was effective enough, as evidenced by country counts in comment:4:ticket:29734. I just now looked at bridge-extra-infos-2020-02.tar.xz and it seems that we are still sometimes getting identified countries, but the largest count belongs to ??.

$ tar -O -xf bridge-extra-infos-2020-02.tar.xz | grep -A 24 '^extra-info flakey 5481936581E23D2D178105D44DB6915AB06BFB7F$' | grep -E '^dirreq-v3-reqs '
dirreq-v3-reqs ??=16,tr=16,cn=8,de=8,eg=8,in=8,ir=8,ph=8,us=8
dirreq-v3-reqs ??=16,tr=16,cn=8,de=8,eg=8,in=8,ir=8,ph=8,us=8
dirreq-v3-reqs ??=24,cn=8,es=8,fr=8,ir=8,tr=8
dirreq-v3-reqs ??=48,tr=16,ar=8,ca=8,eg=8,gb=8,ir=8,us=8
dirreq-v3-reqs ??=16,tr=16,cn=8,de=8,eg=8,in=8,ir=8,ph=8,us=8
dirreq-v3-reqs ??=24,cn=8,es=8,fr=8,ir=8,tr=8
dirreq-v3-reqs ??=48,tr=16,ar=8,ca=8,eg=8,gb=8,ir=8,us=8
dirreq-v3-reqs ??=24,cn=8,es=8,fr=8,ir=8,tr=8
dirreq-v3-reqs ??=24,cn=8,es=8,fr=8,ir=8,tr=8
dirreq-v3-reqs ??=24,cn=8,es=8,fr=8,ir=8,tr=8
dirreq-v3-reqs ??=48,tr=16,ar=8,ca=8,eg=8,gb=8,ir=8,us=8
dirreq-v3-reqs ??=16,tr=16,cn=8,de=8,eg=8,in=8,ir=8,ph=8,us=8
dirreq-v3-reqs ??=24,cn=8,es=8,fr=8,ir=8,tr=8
dirreq-v3-reqs ??=48,tr=16,ar=8,ca=8,eg=8,gb=8,ir=8,us=8
dirreq-v3-reqs ??=16,tr=16,cn=8,de=8,eg=8,in=8,ir=8,ph=8,us=8
dirreq-v3-reqs ??=16,tr=16,cn=8,de=8,eg=8,in=8,ir=8,ph=8,us=8
dirreq-v3-reqs ??=24,cn=8,es=8,fr=8,ir=8,tr=8
dirreq-v3-reqs ??=16,tr=16,cn=8,de=8,eg=8,in=8,ir=8,ph=8,us=8
dirreq-v3-reqs ??=24,cn=8,es=8,fr=8,ir=8,tr=8
dirreq-v3-reqs ??=24,tr=16,fr=8,om=8,ru=8,us=8
dirreq-v3-reqs ??=16,tr=16,cn=8,de=8,eg=8,in=8,ir=8,ph=8,us=8
dirreq-v3-reqs ??=48,tr=16,ar=8,ca=8,eg=8,gb=8,ir=8,us=8

Maybe it happens only intermittently. pion/sdp sets UnicastAddress: "0.0.0.0" unconditionally and I don't see where it is ever modified. Maybe the others are older non-pion clients?

A little searching indicates that IN IP4 0.0.0.0 has something to do with trickle ICE:

It seems that ultimately, we need a more reliable way for the proxy to infer the client's external IP address.

Child Tickets

Change History (4)

comment:1 Changed 2 weeks ago by cohosh

Possible related: #19026

comment:2 Changed 12 days ago by cohosh

You know, we probably shouldn't even be using trickle ICE. The intended use case as I understand it is to allow peers to send candidates to each other continually rather than all at once (which we definitely don't do). Our original reason for using it was because it allowed us to keep using the OnICEGatheringStateChange callback here (see #28942#comment:28).

I think it's worth revisiting that decision.

comment:3 Changed 11 days ago by dcf

I'll say that the bug here is not necessarily that the SDP contains 0.0.0.0; it's that the proxy uses 0.0.0.0 as the client address despite more meaningful information being available in the SDP. remoteIPFromSDP could choose one of the candidates from an a=candidate line rather than only looking at the c= line, for example. (Which one? It probably doesn't matter, as they are all likely to have the same geolocation, as long as you pick an external address candidate and not a LAN address candidate.) One way to fix the problem would be to ensure that clients generate SDP with a meaningful address in the c= line; another way would be to use a different method of extracting the address in the proxy.

comment:4 Changed 11 days ago by arlolra

I found it while testing proxy-go with a localhost client and a patch like:

Something like that patch was useful when working on #19026 so would you consider merging,
https://github.com/keroserene/snowflake/commit/dbd733e4b1430c046ec11e8052efdbac6010e58a

Note: See TracTickets for help on using tickets.