Re: [tor-bugs] #30206 [Obfuscation/Snowflake]: Segfault in proxy-go

[Tor Bug Tracker & Wiki]

#30206: Segfault in proxy-go
---+
 Reporter:  irl|  Owner:  cohosh
 Type:  defect | Status:  closed
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:  fixed
 Keywords: |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+
Changes (by cohosh):

 * status:  merge_ready => closed
 * resolution:   => fixed


Comment:

 Merged to master

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #30206 [Obfuscation/Snowflake]: Segfault in proxy-go

[Tor Bug Tracker & Wiki]

#30206: Segfault in proxy-go
---+-
 Reporter:  irl|  Owner:  cohosh
 Type:  defect | Status:  merge_ready
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords: |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+-
Changes (by dcf):

 * status:  needs_information => merge_ready


Comment:

 Okay.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #30206 [Obfuscation/Snowflake]: Segfault in proxy-go

[Tor Bug Tracker & Wiki]

#30206: Segfault in proxy-go
---+---
 Reporter:  irl|  Owner:  cohosh
 Type:  defect | Status:  needs_information
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords: |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+---

Comment (by cohosh):

 Ah, also there's a typo
 [https://github.com/cohosh/snowflake/compare/ticket30206#diff-
 ba4bd8a4477426567c409d66c2cf8a28R344 here] which I will fix

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #30206 [Obfuscation/Snowflake]: Segfault in proxy-go

[Tor Bug Tracker & Wiki]

#30206: Segfault in proxy-go
---+---
 Reporter:  irl|  Owner:  cohosh
 Type:  defect | Status:  needs_information
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords: |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+---

Comment (by cohosh):

 Replying to [comment:5 dcf]:
 Thanks for taking a look at this!
 > I ''think'' the chain of events is as follows:
 > * `dc.OnClose` calls `pw.Close`, where `pw` feeds into the read part of
 a `webRTCConn`.
 > * Closing `pw` causes `CopyLoopTimeout` to exit, which causes
 `datachannelHandler` to exit, which makes a deferred call to
 `webRTCConn.Close`.
 > * `webRTCConn.Close` calls `pc.Destroy`.
 > * `pc.Destroy` calls `CGO_DestroyPeer`.
 > * `CGO_DestroyPeer` removes the peer from `localPeers`, which drops the
 peer's reference count to 0, which sets `pc_ = nullptr`.
 > * Later, when the `dataChannelTimeout` expires, it calls
 `pc.ConnectionState`, where `pc` is a stale object reference that has
 already had its destructor called. In particular, its `pc_` member is
 null. `CGO_IceConnectionState` deferences the null pointer.
 That checks out with my understanding of how this works.
 >
 > 
 >
 > The fix looks okay to me. I presume there haven't been any more problems
 since you deployed it?
 Yup, no seg faults since I deployed the fix.
 >
 > Was [https://github.com/cohosh/snowflake/compare/ticket30206#diff-
 ba4bd8a4477426567c409d66c2cf8a28L33 this change] only to assist finding
 the bug, or is there another reason for it?
 > {{{#!diff
 > -const dataChannelTimeout = time.Minute
 > +const dataChannelTimeout = 20 * time.Second
 > }}}
 There were two reasons for this:
  1. An entire minute is a long time to wait in the event that there was an
 ICE error and the client needs to find a new proxy.
  2. The current proxy timeout is 30 seconds, which means that the earliest
 a client will attempt to close a data channel they believe is open is 30
 seconds later. So even without the seg fault fix, this would prevent a lot
 of failures. Obviously we don't need it with the fix, but it seems like a
 good idea to take into account our existing timeouts when creating this
 one.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #30206 [Obfuscation/Snowflake]: Segfault in proxy-go

[Tor Bug Tracker & Wiki]

#30206: Segfault in proxy-go
---+---
 Reporter:  irl|  Owner:  cohosh
 Type:  defect | Status:  needs_information
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords: |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+---
Changes (by dcf):

 * status:  needs_review => needs_information


Comment:

 I agree with your diagnosis. I can make the segfault happen by applying
 this change, closing the datachannel immediately after it is opened:
 {{{#!diff
 --- a/proxy-go/snowflake.go
 +++ b/proxy-go/snowflake.go
 @@ -280,6 +280,7 @@ func makePeerConnectionFromOffer(sdp
 *webrtc.SessionDescription, config *webrtc.

 dc.OnOpen = func() {
 log.Println("OnOpen channel")
 +   dc.Close()
 }
 dc.OnClose = func() {
 conn.lock.Lock()
 }}}
 I ''think'' the chain of events is as follows:
 * `dc.OnClose` calls `pw.Close`, where `pw` feeds into the read part of a
 `webRTCConn`.
 * Closing `pw` causes `CopyLoopTimeout` to exit, which causes
 `datachannelHandler` to exit, which makes a deferred call to
 `webRTCConn.Close`.
 * `webRTCConn.Close` calls `pc.Destroy`.
 * `pc.Destroy` calls `CGO_DestroyPeer`.
 * `CGO_DestroyPeer` removes the peer from `localPeers`, which drops the
 peer's reference count to 0, which sets `pc_ = nullptr`.
 * Later, when the `dataChannelTimeout` expires, it calls
 `pc.ConnectionState`, where `pc` is a stale object reference that has
 already had its destructor called. In particular, its `pc_` member is
 null. `CGO_IceConnectionState` deferences the null pointer.

 

 The fix looks okay to me. I presume there haven't been any more problems
 since you deployed it?

 Was [https://github.com/cohosh/snowflake/compare/ticket30206#diff-
 ba4bd8a4477426567c409d66c2cf8a28L33 this change] only to assist finding
 the bug, or is there another reason for it?
 {{{#!diff
 -const dataChannelTimeout = time.Minute
 +const dataChannelTimeout = 20 * time.Second
 }}}

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #30206 [Obfuscation/Snowflake]: Segfault in proxy-go

[Tor Bug Tracker & Wiki]

#30206: Segfault in proxy-go
---+--
 Reporter:  irl|  Owner:  cohosh
 Type:  defect | Status:  needs_review
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords: |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+--
Changes (by irl):

 * status:  needs_revision => needs_review


Comment:

 I think you meant this.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #30206 [Obfuscation/Snowflake]: Segfault in proxy-go

[Tor Bug Tracker & Wiki]

#30206: Segfault in proxy-go
---+
 Reporter:  irl|  Owner:  cohosh
 Type:  defect | Status:  needs_revision
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords: |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+
Changes (by cohosh):

 * status:  accepted => needs_revision


Comment:

 Here's a fix: https://github.com/cohosh/snowflake/compare/ticket30206

 The proxy-go instances were seg faulting occasionally in cases where the
 client successfully made a data channel connection but the data channel
 was closed before the timeout occurred.

 I added a second channel that will trigger when the data channel has been
 successfully opened (and therefore pc.Destroy() is handled by
 dataChannelHandler), and only destroy the connection if this channel has
 not been closed before the Timeout (which I've shortened to a more
 realistic time).

 I'm going to patch our deployed proxy-go instances immediately, but wait
 to merge until this has been reviewed.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #30206 [Obfuscation/Snowflake]: Segfault in proxy-go

[Tor Bug Tracker & Wiki]

#30206: Segfault in proxy-go
---+--
 Reporter:  irl|  Owner:  cohosh
 Type:  defect | Status:  accepted
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords: |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+--
Changes (by cohosh):

 * owner:  (none) => cohosh
 * status:  new => accepted


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #30206 [Obfuscation/Snowflake]: Segfault in proxy-go

[Tor Bug Tracker & Wiki]

#30206: Segfault in proxy-go
---+
 Reporter:  irl|  Owner:  (none)
 Type:  defect | Status:  new
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords: |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+

Comment (by cohosh):

 This has been occurring as a result of the proxy-go deadlock patch from
 #25688.

 The problem part of the code is here:
 {{{
 // Set a timeout on peerconnection. If the connection state has
 not
 // advanced to PeerConnectionStateConnected in this time,
 // destroy the peer connection and return the token.
 go func() {
 <-time.After(dataChannelTimeout)
 if pc.ConnectionState() !=
 webrtc.PeerConnectionStateConnected {
 }}}
 where pc (or some members of it) are nil and cause an invalid memory
 access

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

[tor-bugs] #30206 [Obfuscation/Snowflake]: Segfault in proxy-go

[Tor Bug Tracker & Wiki]

#30206: Segfault in proxy-go
---+
 Reporter:  irl|  Owner:  (none)
 Type:  defect | Status:  new
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal |   Keywords:
Actual Points: |  Parent ID:
   Points: |   Reviewer:
  Sponsor: |
---+
 It happened quite quickly, so here's the full output.

 {{{
 2019/04/16 [scrubbed] starting
 INFO: configuration.go:174: Created Configuration at
 &{[{[stun:stun.l.google.com:19302]  }] All Balanced }
 INFO: configuration.go:256: Converting Config:
 &{[{[stun:stun.l.google.com:19302]  }] All Balanced }
 INFO: peerconnection.go:168: Created PeerConnection:  &{  false
 
 {[{[stun:stun.l.google.com:19302]  }] 3 0 } 0x7f65bc000c60 1}
 0x7f65bc000c60
 INFO: peerconnection.go:436: fired OnSignalingStateChange:  1
 HaveRemoteOffer
 2019/04/16 [scrubbed] sdp offer successfully received.
 2019/04/16 [scrubbed] Generating answer...
 INFO: peerconnection.go:436: fired OnSignalingStateChange:  1 Stable
 INFO: peerconnection.go:504: fired OnIceConnectionStateChange:  1
 INFO: peerconnection.go:495: fired OnConnectionStateChange:  1
 INFO: peerconnection.go:513: fired OnIceGatheringStateChange: 1
 INFO: peerconnection.go:459: fired OnIceCandidate:  1 candidate:1020330603
 1 udp 2122262783 2001:470:1d58:1337:2066:3507:9c2e:9cc8 39908 typ host
 generation 0 ufrag gUW/ network-id 2 network-cost 50
 INFO: peerconnection.go:459: fired OnIceCandidate:  1 candidate:498995695
 1 udp 2122194687 172.22.152.130 59852 typ host generation 0 ufrag gUW/
 network-id 1 network-cost 50
 INFO: peerconnection.go:459: fired OnIceCandidate:  1 candidate:3509792380
 1 udp 1685987071 84.92.41.201 59913 typ srflx raddr 172.22.152.130 rport
 59852 generation 0 ufrag gUW/ network-id 1 network-cost 50
 INFO: peerconnection.go:459: fired OnIceCandidate:  1 candidate:1918041755
 1 tcp 1518283007 2001:470:1d58:1337:2066:3507:9c2e:9cc8 34045 typ host
 tcptype passive generation 0 ufrag gUW/ network-id 2 network-cost 50
 INFO: peerconnection.go:459: fired OnIceCandidate:  1 candidate:1396567327
 1 tcp 1518214911 172.22.152.130 54255 typ host tcptype passive generation
 0 ufrag gUW/ network-id 1 network-cost 50
 INFO: peerconnection.go:513: fired OnIceGatheringStateChange: 1
 INFO: peerconnection.go:504: fired OnIceConnectionStateChange:  1
 INFO: peerconnection.go:495: fired OnConnectionStateChange:  1
 INFO: peerconnection.go:527: fired OnDataChannel:  1 0x7f6584008960
 2019/04/16 [scrubbed] OnDataChannel
 2019/04/16 [scrubbed] OnOpen channel
 2019/04/16 [scrubbed] connected to relay
 INFO: configuration.go:256: Converting Config:
 &{[{[stun:stun.l.google.com:19302]  }] All Balanced }
 INFO: peerconnection.go:168: Created PeerConnection:  &{  false
 
 {[{[stun:stun.l.google.com:19302]  }] 3 0 } 0x7f658b20 2}
 0x7f658b20
 INFO: peerconnection.go:436: fired OnSignalingStateChange:  2
 HaveRemoteOffer
 2019/04/16 [scrubbed] sdp offer successfully received.
 2019/04/16 [scrubbed] Generating answer...
 INFO: peerconnection.go:436: fired OnSignalingStateChange:  2 Stable
 INFO: peerconnection.go:504: fired OnIceConnectionStateChange:  2
 INFO: peerconnection.go:495: fired OnConnectionStateChange:  2
 INFO: peerconnection.go:513: fired OnIceGatheringStateChange: 2
 INFO: peerconnection.go:459: fired OnIceCandidate:  2 candidate:1020330603
 1 udp 2122262783 2001:470:1d58:1337:2066:3507:9c2e:9cc8 59602 typ host
 generation 0 ufrag ZK+V network-id 2 network-cost 50
 INFO: peerconnection.go:459: fired OnIceCandidate:  2 candidate:498995695
 1 udp 2122194687 172.22.152.130 58452 typ host generation 0 ufrag ZK+V
 network-id 1 network-cost 50
 INFO: peerconnection.go:459: fired OnIceCandidate:  2 candidate:3509792380
 1 udp 1685987071 84.92.41.201 59538 typ srflx raddr 172.22.152.130 rport
 58452 generation 0 ufrag ZK+V network-id 1 network-cost 50
 INFO: peerconnection.go:459: fired OnIceCandidate:  2 candidate:1918041755
 1 tcp 1518283007 2001:470:1d58:1337:2066:3507:9c2e:9cc8 5 typ host
 tcptype passive generation 0 ufrag ZK+V network-id 2 network-cost 50
 INFO: peerconnection.go:459: fired OnIceCandidate:  2 candidate:1396567327
 1 tcp 1518214911 172.22.152.130 55661 typ host tcptype passive generation
 0 ufrag ZK+V network-id 1 network-cost 50
 INFO: peerconnection.go:513: fired OnIceGatheringStateChange: 2
 INFO: peerconnection.go:504: fired OnIceConnectionStateChange:  2
 INFO: peerconnection.go:495: fired OnConnectionStateChange:  2
 INFO: peerconnection.go:527: fired OnDataChannel:  2 0x7f656800a310
 2019/04/16 [scrubbed] OnDataChannel
 2019/04/16 [scrubbed] OnOpen channel
 2019/04/16 [scrubbed] connected to relay
 INFO: configuration.go:256: Converting Config: