April 2020 - tor-commits - lists.torproject.org

[snowflake/master] Move establishDataChannel to after exchangeSDP.
by dcf＠torproject.org 28 Apr '20

28 Apr '20

commit e8c41650ae44e300aabe57e361c445808635fb4f Author: David Fifield <david(a)bamsoftware.com> Date: Fri Apr 24 11:44:40 2020 -0600 Move establishDataChannel to after exchangeSDP. --- client/lib/webrtc.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/client/lib/webrtc.go b/client/lib/webrtc.go index 7ceeaff..b4c0aad 100644 --- a/client/lib/webrtc.go +++ b/client/lib/webrtc.go @@ -120,11 +120,6 @@ func (c *WebRTCPeer) connect(config *webrtc.Configuration, broker *BrokerChannel if err != nil { return err } - err = c.establishDataChannel() - if err != nil { - // nolint: golint - return errors.New("WebRTC: Could not establish DataChannel") - } answer := exchangeSDP(broker, c.pc.LocalDescription()) log.Printf("Received Answer.\n") err = c.pc.SetRemoteDescription(*answer) @@ -132,6 +127,11 @@ func (c *WebRTCPeer) connect(config *webrtc.Configuration, broker *BrokerChannel log.Println("WebRTC: Unable to SetRemoteDescription:", err) return err } + err = c.establishDataChannel() + if err != nil { + // nolint: golint + return errors.New("WebRTC: Could not establish DataChannel") + } go c.checkForStaleness() return nil }

1 0

[snowflake/master] Make exchangeSDP into a standalone function.
by dcf＠torproject.org 28 Apr '20

28 Apr '20

commit 85277274fd3020819fe5c556b8164e7671bb799b Author: David Fifield <david(a)bamsoftware.com> Date: Thu Apr 23 22:33:38 2020 -0600 Make exchangeSDP into a standalone function. --- client/lib/webrtc.go | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/client/lib/webrtc.go b/client/lib/webrtc.go index f47bb2e..7ceeaff 100644 --- a/client/lib/webrtc.go +++ b/client/lib/webrtc.go @@ -21,7 +21,6 @@ type WebRTCPeer struct { id string pc *webrtc.PeerConnection transport *webrtc.DataChannel - broker *BrokerChannel recvPipe *io.PipeReader writePipe *io.PipeWriter @@ -47,7 +46,6 @@ func NewWebRTCPeer(config *webrtc.Configuration, } connection.id = "snowflake-" + hex.EncodeToString(buf[:]) } - connection.broker = broker // Override with something that's not NullLogger to have real logging. connection.BytesLogger = &BytesNullLogger{} @@ -55,7 +53,7 @@ func NewWebRTCPeer(config *webrtc.Configuration, // Pipes remain the same even when DataChannel gets switched. connection.recvPipe, connection.writePipe = io.Pipe() - err := connection.connect(config) + err := connection.connect(config, broker) if err != nil { connection.Close() return nil, err @@ -113,7 +111,7 @@ func (c *WebRTCPeer) checkForStaleness() { } } -func (c *WebRTCPeer) connect(config *webrtc.Configuration) error { +func (c *WebRTCPeer) connect(config *webrtc.Configuration, broker *BrokerChannel) error { log.Println(c.id, " connecting...") // TODO: When go-webrtc is more stable, it's possible that a new // PeerConnection won't need to be re-prepared each time. @@ -127,8 +125,11 @@ func (c *WebRTCPeer) connect(config *webrtc.Configuration) error { // nolint: golint return errors.New("WebRTC: Could not establish DataChannel") } - err = c.exchangeSDP() - if err != nil { + answer := exchangeSDP(broker, c.pc.LocalDescription()) + log.Printf("Received Answer.\n") + err = c.pc.SetRemoteDescription(*answer) + if nil != err { + log.Println("WebRTC: Unable to SetRemoteDescription:", err) return err } go c.checkForStaleness() @@ -246,29 +247,20 @@ func (c *WebRTCPeer) establishDataChannel() error { return nil } -// exchangeSDP sends the local SDP offer to the Broker and awaits the SDP -// answer. -func (c *WebRTCPeer) exchangeSDP() error { +// exchangeSDP sends the local SDP offer to the Broker, awaits the SDP answer, +// and returns the answer. +func exchangeSDP(broker *BrokerChannel, offer *webrtc.SessionDescription) *webrtc.SessionDescription { // Keep trying the same offer until a valid answer arrives. - var answer *webrtc.SessionDescription for { - var err error // Send offer to broker (blocks). - answer, err = c.broker.Negotiate(c.pc.LocalDescription()) + answer, err := broker.Negotiate(offer) if err == nil { - break + return answer } log.Printf("BrokerChannel Error: %s", err) log.Printf("Failed to retrieve answer. Retrying in %v", ReconnectTimeout) <-time.After(ReconnectTimeout) } - log.Printf("Received Answer.\n") - err := c.pc.SetRemoteDescription(*answer) - if nil != err { - log.Println("WebRTC: Unable to SetRemoteDescription:", err) - return err - } - return nil } // Close all channels and transports

1 0

[snowflake/master] Make preparePeerConnection a standalone function.
by dcf＠torproject.org 28 Apr '20

28 Apr '20

commit 8295c87fbe4a3fb0d9e6c5fb04f6f0a52573faaf Author: David Fifield <david(a)bamsoftware.com> Date: Thu Apr 23 20:57:50 2020 -0600 Make preparePeerConnection a standalone function. --- client/lib/webrtc.go | 36 ++++++++++++++---------------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/client/lib/webrtc.go b/client/lib/webrtc.go index d489096..f47bb2e 100644 --- a/client/lib/webrtc.go +++ b/client/lib/webrtc.go @@ -19,7 +19,6 @@ import ( // one DataChannel. type WebRTCPeer struct { id string - config *webrtc.Configuration pc *webrtc.PeerConnection transport *webrtc.DataChannel broker *BrokerChannel @@ -48,7 +47,6 @@ func NewWebRTCPeer(config *webrtc.Configuration, } connection.id = "snowflake-" + hex.EncodeToString(buf[:]) } - connection.config = config connection.broker = broker // Override with something that's not NullLogger to have real logging. @@ -57,7 +55,7 @@ func NewWebRTCPeer(config *webrtc.Configuration, // Pipes remain the same even when DataChannel gets switched. connection.recvPipe, connection.writePipe = io.Pipe() - err := connection.connect() + err := connection.connect(config) if err != nil { connection.Close() return nil, err @@ -115,11 +113,12 @@ func (c *WebRTCPeer) checkForStaleness() { } } -func (c *WebRTCPeer) connect() error { +func (c *WebRTCPeer) connect(config *webrtc.Configuration) error { log.Println(c.id, " connecting...") // TODO: When go-webrtc is more stable, it's possible that a new // PeerConnection won't need to be re-prepared each time. - err := c.preparePeerConnection() + var err error + c.pc, err = preparePeerConnection(config) if err != nil { return err } @@ -136,19 +135,13 @@ func (c *WebRTCPeer) connect() error { return nil } -// Create and prepare callbacks on a new WebRTC PeerConnection. -func (c *WebRTCPeer) preparePeerConnection() error { - if nil != c.pc { - if err := c.pc.Close(); err != nil { - log.Printf("c.pc.Close returned error: %v", err) - } - c.pc = nil - } - - pc, err := webrtc.NewPeerConnection(*c.config) +// preparePeerConnection creates a new WebRTC PeerConnection and returns it +// after ICE candidate gathering is complete.. +func preparePeerConnection(config *webrtc.Configuration) (*webrtc.PeerConnection, error) { + pc, err := webrtc.NewPeerConnection(*config) if err != nil { log.Printf("NewPeerConnection ERROR: %s", err) - return err + return nil, err } // Prepare PeerConnection callbacks. offerChannel := make(chan struct{}) @@ -161,27 +154,26 @@ func (c *WebRTCPeer) preparePeerConnection() error { log.Printf("WebRTC: Got ICE candidate: %s", candidate.String()) } }) - c.pc = pc offer, err := pc.CreateOffer(nil) // TODO: Potentially timeout and retry if ICE isn't working. if err != nil { log.Println("Failed to prepare offer", err) - c.Close() - return err + pc.Close() + return nil, err } log.Println("WebRTC: Created offer") err = pc.SetLocalDescription(offer) if err != nil { log.Println("Failed to prepare offer", err) - c.Close() - return err + pc.Close() + return nil, err } log.Println("WebRTC: Set local description") <-offerChannel // Wait for ICE candidate gathering to complete. log.Println("WebRTC: PeerConnection created.") - return nil + return pc, nil } // Create a WebRTC DataChannel locally.

1 0

[snowflake/master] Wait for data channel OnOpen before returning from NewWebRTCPeer.
by dcf＠torproject.org 28 Apr '20

28 Apr '20

commit 047d3214bfb46de07e5d9f223e4fb1ba24584c8a Author: David Fifield <david(a)bamsoftware.com> Date: Fri Apr 24 13:30:13 2020 -0600 Wait for data channel OnOpen before returning from NewWebRTCPeer. Now callers cannot call Write without there being a DataChannel to write to. This lets us remove the internal buffer and checks for transport == nil. Don't set internal fields like writePipe, transport, and pc to nil when closing; just close them and let them return errors if further calls are made on them. There's now a constant DataChannelTimeout that's separate from SnowflakeTimeout (the latter is what checkForStaleness uses). Now we can set DataChannel timeout to a lower value, to quickly dispose of unconnectable proxies, while still keeping the threshold for detecting the failure of a once-working proxy at 30 seconds. https://bugs.torproject.org/33897 --- client/lib/snowflake.go | 2 ++ client/lib/webrtc.go | 96 +++++++++++++------------------------------------ 2 files changed, 26 insertions(+), 72 deletions(-) diff --git a/client/lib/snowflake.go b/client/lib/snowflake.go index 27991b2..0076b79 100644 --- a/client/lib/snowflake.go +++ b/client/lib/snowflake.go @@ -17,6 +17,8 @@ import ( const ( ReconnectTimeout = 10 * time.Second SnowflakeTimeout = 30 * time.Second + // How long to wait for the OnOpen callback on a DataChannel. + DataChannelTimeout = 30 * time.Second ) type dummyAddr struct{} diff --git a/client/lib/webrtc.go b/client/lib/webrtc.go index b4c0aad..edc8ab4 100644 --- a/client/lib/webrtc.go +++ b/client/lib/webrtc.go @@ -1,7 +1,6 @@ package lib import ( - "bytes" "crypto/rand" "encoding/hex" "errors" @@ -25,12 +24,10 @@ type WebRTCPeer struct { recvPipe *io.PipeReader writePipe *io.PipeWriter lastReceive time.Time - buffer bytes.Buffer closed bool - lock sync.Mutex // Synchronization for DataChannel destruction - once sync.Once // Synchronization for PeerConnection destruction + once sync.Once // Synchronization for PeerConnection destruction BytesLogger BytesLogger } @@ -70,16 +67,11 @@ func (c *WebRTCPeer) Read(b []byte) (int, error) { // Writes bytes out to remote WebRTC. // As part of |io.ReadWriter| func (c *WebRTCPeer) Write(b []byte) (int, error) { - c.lock.Lock() - defer c.lock.Unlock() - c.BytesLogger.AddOutbound(len(b)) - // TODO: Buffering could be improved / separated out of WebRTCPeer. - if nil == c.transport { - log.Printf("Buffered %d bytes --> WebRTC", len(b)) - c.buffer.Write(b) - } else { - c.transport.Send(b) + err := c.transport.Send(b) + if err != nil { + return 0, err } + c.BytesLogger.AddOutbound(len(b)) return len(b), nil } @@ -127,8 +119,9 @@ func (c *WebRTCPeer) connect(config *webrtc.Configuration, broker *BrokerChannel log.Println("WebRTC: Unable to SetRemoteDescription:", err) return err } - err = c.establishDataChannel() + c.transport, err = c.establishDataChannel() if err != nil { + log.Printf("establishDataChannel: %v", err) // nolint: golint return errors.New("WebRTC: Could not establish DataChannel") } @@ -177,13 +170,9 @@ func preparePeerConnection(config *webrtc.Configuration) (*webrtc.PeerConnection return pc, nil } -// Create a WebRTC DataChannel locally. -func (c *WebRTCPeer) establishDataChannel() error { - c.lock.Lock() - defer c.lock.Unlock() - if c.transport != nil { - panic("Unexpected datachannel already exists!") - } +// Create a WebRTC DataChannel locally. Blocks until the data channel is open, +// or a timeout or error occurs. +func (c *WebRTCPeer) establishDataChannel() (*webrtc.DataChannel, error) { ordered := true dataChannelOptions := &webrtc.DataChannelInit{ Ordered: &ordered, @@ -191,41 +180,15 @@ func (c *WebRTCPeer) establishDataChannel() error { dc, err := c.pc.CreateDataChannel(c.id, dataChannelOptions) if err != nil { log.Printf("CreateDataChannel ERROR: %s", err) - return err + return nil, err } + openChannel := make(chan struct{}) dc.OnOpen(func() { - c.lock.Lock() - defer c.lock.Unlock() log.Println("WebRTC: DataChannel.OnOpen") - if nil != c.transport { - panic("WebRTC: transport already exists.") - } - // Flush buffered outgoing SOCKS data if necessary. - if c.buffer.Len() > 0 { - dc.Send(c.buffer.Bytes()) - log.Println("Flushed", c.buffer.Len(), "bytes.") - c.buffer.Reset() - } - // Then enable the datachannel. - c.transport = dc + close(openChannel) }) dc.OnClose(func() { - c.lock.Lock() - // Future writes will go to the buffer until a new DataChannel is available. - if nil == c.transport { - // Closed locally, as part of a reset. - log.Println("WebRTC: DataChannel.OnClose [locally]") - c.lock.Unlock() - return - } - // Closed remotely, need to reset everything. - // Disable the DataChannel as a write destination. - log.Println("WebRTC: DataChannel.OnClose [remotely]") - c.transport = nil - dc.Close() - // Unlock before Close'ing, since it calls cleanup and asks for the - // lock to check if the transport needs to be be deleted. - c.lock.Unlock() + log.Println("WebRTC: DataChannel.OnClose") c.Close() }) dc.OnMessage(func(msg webrtc.DataChannelMessage) { @@ -244,7 +207,14 @@ func (c *WebRTCPeer) establishDataChannel() error { c.lastReceive = time.Now() }) log.Println("WebRTC: DataChannel created.") - return nil + + select { + case <-openChannel: + return dc, nil + case <-time.After(DataChannelTimeout): + dc.Close() + return nil, errors.New("timeout waiting for DataChannel.OnOpen") + } } // exchangeSDP sends the local SDP offer to the Broker, awaits the SDP answer, @@ -266,27 +236,10 @@ func exchangeSDP(broker *BrokerChannel, offer *webrtc.SessionDescription) *webrt // Close all channels and transports func (c *WebRTCPeer) cleanup() { // Close this side of the SOCKS pipe. - if nil != c.writePipe { - c.writePipe.Close() - c.writePipe = nil - } - c.lock.Lock() + c.writePipe.Close() if nil != c.transport { log.Printf("WebRTC: closing DataChannel") - dataChannel := c.transport - // Setting transport to nil *before* dc Close indicates to OnClose that - // this was locally triggered. - c.transport = nil - // Release the lock before calling DeleteDataChannel (which in turn - // calls Close on the dataChannel), but after nil'ing out the transport, - // since otherwise we'll end up in the onClose handler in a deadlock. - c.lock.Unlock() - if c.pc == nil { - panic("DataChannel w/o PeerConnection, not good.") - } - dataChannel.Close() - } else { - c.lock.Unlock() + c.transport.Close() } if nil != c.pc { log.Printf("WebRTC: closing PeerConnection") @@ -294,6 +247,5 @@ func (c *WebRTCPeer) cleanup() { if nil != err { log.Printf("Error closing peerconnection...") } - c.pc = nil } }

1 0

[snowflake/master] Remove `Snowflake` interface, use `*WebRTCPeer` directly.
by dcf＠torproject.org 27 Apr '20

27 Apr '20

commit 76732155e7d730573b3ced62209e4e1e4ead511c Author: David Fifield <david(a)bamsoftware.com> Date: Fri Apr 24 14:21:08 2020 -0600 Remove `Snowflake` interface, use `*WebRTCPeer` directly. The other interfaces in client/lib/interfaces.go exist for the purpose of running tests, but not Snowflake. Existing code would not have worked with other types anyway, because it does unchecked .(*WebRTCPeer) conversions. --- client/lib/interfaces.go | 13 +++---------- client/lib/lib_test.go | 8 ++++---- client/lib/peers.go | 17 +++++++---------- client/lib/rendezvous.go | 2 +- client/lib/webrtc.go | 3 --- 5 files changed, 15 insertions(+), 28 deletions(-) diff --git a/client/lib/interfaces.go b/client/lib/interfaces.go index 57171f8..e551c4d 100644 --- a/client/lib/interfaces.go +++ b/client/lib/interfaces.go @@ -9,16 +9,9 @@ type Connector interface { Connect() error } -// Interface for a single remote WebRTC peer. -// In the Client context, "Snowflake" refers to the remote browser proxy. -type Snowflake interface { - io.ReadWriteCloser - Connector -} - // Interface for catching Snowflakes. (aka the remote dialer) type Tongue interface { - Catch() (Snowflake, error) + Catch() (*WebRTCPeer, error) } // Interface for collecting some number of Snowflakes, for passing along @@ -26,10 +19,10 @@ type Tongue interface { type SnowflakeCollector interface { // Add a Snowflake to the collection. // Implementation should decide how to connect and maintain the webRTCConn. - Collect() (Snowflake, error) + Collect() (*WebRTCPeer, error) // Remove and return the most available Snowflake from the collection. - Pop() Snowflake + Pop() *WebRTCPeer // Signal when the collector has stopped collecting. Melted() <-chan struct{} diff --git a/client/lib/lib_test.go b/client/lib/lib_test.go index 1cdc2c6..50a211d 100644 --- a/client/lib/lib_test.go +++ b/client/lib/lib_test.go @@ -52,7 +52,7 @@ func (m *MockTransport) RoundTrip(req *http.Request) (*http.Response, error) { type FakeDialer struct{} -func (w FakeDialer) Catch() (Snowflake, error) { +func (w FakeDialer) Catch() (*WebRTCPeer, error) { fmt.Println("Caught a dummy snowflake.") return &WebRTCPeer{}, nil } @@ -70,9 +70,9 @@ func (f FakeSocksConn) Grant(addr *net.TCPAddr) error { return nil } type FakePeers struct{ toRelease *WebRTCPeer } -func (f FakePeers) Collect() (Snowflake, error) { return &WebRTCPeer{}, nil } -func (f FakePeers) Pop() Snowflake { return nil } -func (f FakePeers) Melted() <-chan struct{} { return nil } +func (f FakePeers) Collect() (*WebRTCPeer, error) { return &WebRTCPeer{}, nil } +func (f FakePeers) Pop() *WebRTCPeer { return nil } +func (f FakePeers) Melted() <-chan struct{} { return nil } const sampleSDP = `"v=0\r\no=- 4358805017720277108 2 IN IP4 8.8.8.8\r\ns=-\r\nt=0 0\r\na=group:BUNDLE data\r\na=msid-semantic: WMS\r\nm=application 56688 DTLS/SCTP 5000\r\nc=IN IP4 8.8.8.8\r\na=candidate:3769337065 1 udp 2122260223 8.8.8.8 56688 typ host generation 0 network-id 1 network-cost 50\r\na=candidate:2921887769 1 tcp 1518280447 8.8.8.8 35441 typ host tcptype passive generation 0 network-id 1 network-cost 50\r\na=ice-ufrag:aMAZ\r\na=ice-pwd:jcHb08Jjgrazp2dzjdrvPPvV\r\na=ice-options:trickle\r\na=fingerprint:sha-256 C8:88:EE:B9:E7:02:2E:21:37:ED:7A:D1:EB:2B:A3:15:A2:3B:5B:1C:3D:D4:D5:1F:06:CF:52:40:03:F8:DD:66\r\na=setup:actpass\r\na=mid:data\r\na=sctpmap:5000 webrtc-datachannel 1024\r\n"` diff --git a/client/lib/peers.go b/client/lib/peers.go index 2649638..f766a66 100644 --- a/client/lib/peers.go +++ b/client/lib/peers.go @@ -22,7 +22,7 @@ type Peers struct { Tongue BytesLogger BytesLogger - snowflakeChan chan Snowflake + snowflakeChan chan *WebRTCPeer activePeers *list.List capacity int @@ -33,14 +33,14 @@ type Peers struct { func NewPeers(max int) *Peers { p := &Peers{capacity: max} // Use buffered go channel to pass snowflakes onwards to the SOCKS handler. - p.snowflakeChan = make(chan Snowflake, max) + p.snowflakeChan = make(chan *WebRTCPeer, max) p.activePeers = list.New() p.melt = make(chan struct{}) return p } // As part of |SnowflakeCollector| interface. -func (p *Peers) Collect() (Snowflake, error) { +func (p *Peers) Collect() (*WebRTCPeer, error) { cnt := p.Count() s := fmt.Sprintf("Currently at [%d/%d]", cnt, p.capacity) if cnt >= p.capacity { @@ -64,21 +64,18 @@ func (p *Peers) Collect() (Snowflake, error) { // Pop blocks until an available, valid snowflake appears. Returns nil after End // has been called. -// -// Part of |SnowflakeCollector| interface. -func (p *Peers) Pop() Snowflake { +func (p *Peers) Pop() *WebRTCPeer { for { snowflake, ok := <-p.snowflakeChan if !ok { return nil } - conn := snowflake.(*WebRTCPeer) - if conn.closed { + if snowflake.closed { continue } // Set to use the same rate-limited traffic logger to keep consistency. - conn.BytesLogger = p.BytesLogger - return conn + snowflake.BytesLogger = p.BytesLogger + return snowflake } } diff --git a/client/lib/rendezvous.go b/client/lib/rendezvous.go index c82fc9e..f236c47 100644 --- a/client/lib/rendezvous.go +++ b/client/lib/rendezvous.go @@ -147,7 +147,7 @@ func NewWebRTCDialer(broker *BrokerChannel, iceServers []webrtc.ICEServer) *WebR } // Initialize a WebRTC Connection by signaling through the broker. -func (w WebRTCDialer) Catch() (Snowflake, error) { +func (w WebRTCDialer) Catch() (*WebRTCPeer, error) { // TODO: [#25591] Fetch ICE server information from Broker. // TODO: [#25596] Consider TURN servers here too. connection := NewWebRTCPeer(w.webrtcConfig, w.BrokerChannel) diff --git a/client/lib/webrtc.go b/client/lib/webrtc.go index 389e02b..19fd5c9 100644 --- a/client/lib/webrtc.go +++ b/client/lib/webrtc.go @@ -14,8 +14,6 @@ import ( ) // Remote WebRTC peer. -// Implements the |Snowflake| interface, which includes -// |io.ReadWriter| and |Connector|. // // Handles preparation of go-webrtc PeerConnection. Only ever has // one DataChannel. @@ -87,7 +85,6 @@ func (c *WebRTCPeer) Write(b []byte) (int, error) { return len(b), nil } -// As part of |Snowflake| func (c *WebRTCPeer) Close() error { c.once.Do(func() { c.closed = true

1 0

[snowflake/master] Don't do a separate check for a short write.
by dcf＠torproject.org 27 Apr '20

27 Apr '20

commit d9b076c32eed106a382a19bda973a9616cc44501 Author: David Fifield <david(a)bamsoftware.com> Date: Thu Apr 23 21:01:19 2020 -0600 Don't do a separate check for a short write. A short write will result in a non-nil error. It's an io.PipeWriter anyway, which blocks until all the data has been read or the read end is closed, in which case it returns io.ErrClosedPipe if not some other error. --- client/lib/webrtc.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/client/lib/webrtc.go b/client/lib/webrtc.go index 3fe8410..389e02b 100644 --- a/client/lib/webrtc.go +++ b/client/lib/webrtc.go @@ -239,8 +239,8 @@ func (c *WebRTCPeer) establishDataChannel() error { if len(msg.Data) <= 0 { log.Println("0 length message---") } - c.BytesLogger.AddInbound(len(msg.Data)) n, err := c.writePipe.Write(msg.Data) + c.BytesLogger.AddInbound(n) if err != nil { // TODO: Maybe shouldn't actually close. log.Println("Error writing to SOCKS pipe") @@ -248,10 +248,6 @@ func (c *WebRTCPeer) establishDataChannel() error { log.Printf("c.writePipe.CloseWithError returned error: %v", inerr) } } - if n != len(msg.Data) { - log.Println("Error: short write") - panic("short write") - } c.lastReceive = time.Now() }) log.Println("WebRTC: DataChannel created.")

1 0

[snowflake/master] Move pc.CreateOffer/pc.SetLocalDescription out of a goroutine.
by dcf＠torproject.org 27 Apr '20

27 Apr '20

commit 51bb49fa6f4ac7e01b19fd3136411a54a67a4ff6 Author: David Fifield <david(a)bamsoftware.com> Date: Thu Apr 23 18:52:32 2020 -0600 Move pc.CreateOffer/pc.SetLocalDescription out of a goroutine. This allows us to remove the internal errorChannel. --- client/lib/webrtc.go | 47 ++++++++++++++++++----------------------------- 1 file changed, 18 insertions(+), 29 deletions(-) diff --git a/client/lib/webrtc.go b/client/lib/webrtc.go index 719ea74..3fe8410 100644 --- a/client/lib/webrtc.go +++ b/client/lib/webrtc.go @@ -28,7 +28,6 @@ type WebRTCPeer struct { offerChannel chan *webrtc.SessionDescription answerChannel chan *webrtc.SessionDescription - errorChannel chan error recvPipe *io.PipeReader writePipe *io.PipeWriter lastReceive time.Time @@ -57,9 +56,6 @@ func NewWebRTCPeer(config *webrtc.Configuration, connection.broker = broker connection.offerChannel = make(chan *webrtc.SessionDescription, 1) connection.answerChannel = make(chan *webrtc.SessionDescription, 1) - // Error channel is mostly for reporting during the initial SDP offer - // creation & local description setting, which happens asynchronously. - connection.errorChannel = make(chan error, 1) // Override with something that's not NullLogger to have real logging. connection.BytesLogger = &BytesNullLogger{} @@ -167,21 +163,23 @@ func (c *WebRTCPeer) preparePeerConnection() error { } }) c.pc = pc - go func() { - offer, err := pc.CreateOffer(nil) - // TODO: Potentially timeout and retry if ICE isn't working. - if err != nil { - c.errorChannel <- err - return - } - log.Println("WebRTC: Created offer") - err = pc.SetLocalDescription(offer) - if err != nil { - c.errorChannel <- err - return - } - log.Println("WebRTC: Set local description") - }() + + offer, err := pc.CreateOffer(nil) + // TODO: Potentially timeout and retry if ICE isn't working. + if err != nil { + log.Println("Failed to prepare offer", err) + c.Close() + return err + } + log.Println("WebRTC: Created offer") + err = pc.SetLocalDescription(offer) + if err != nil { + log.Println("Failed to prepare offer", err) + c.Close() + return err + } + log.Println("WebRTC: Set local description") + log.Println("WebRTC: PeerConnection created.") return nil } @@ -276,13 +274,7 @@ func (c *WebRTCPeer) sendOfferToBroker() { // exchangeSDP blocks until an SDP offer is available, sends it to the Broker, // then awaits the SDP answer. func (c *WebRTCPeer) exchangeSDP() error { - select { - case <-c.offerChannel: - case err := <-c.errorChannel: - log.Println("Failed to prepare offer", err) - c.Close() - return err - } + <-c.offerChannel // Keep trying the same offer until a valid answer arrives. var ok bool var answer *webrtc.SessionDescription @@ -312,9 +304,6 @@ func (c *WebRTCPeer) cleanup() { if nil != c.answerChannel { close(c.answerChannel) } - if nil != c.errorChannel { - close(c.errorChannel) - } // Close this side of the SOCKS pipe. if nil != c.writePipe { c.writePipe.Close()

1 0

[tor-browser-build/master] Merge remote-tracking branch 'gktpo/bug_32027_v4'
by sysrqb＠torproject.org 27 Apr '20

27 Apr '20

commit 30bdc472d4d2b222b9435c4a1c728ab74ff96365 Merge: ea9ec4f 260a844 Author: Matthew Finkel <sysrqb(a)torproject.org> Date: Mon Apr 27 22:04:06 2020 +0000 Merge remote-tracking branch 'gktpo/bug_32027_v4' .../go/0001-Use-fixed-go-build-tmp-directory.patch | 22 ++++----- .../90a3ce02dc25adcf1598faf11a66b151ada3f637.patch | 57 ---------------------- projects/go/build | 2 - projects/go/config | 12 +++-- projects/obfs4/build | 7 +-- projects/snowflake/build | 4 -- 6 files changed, 23 insertions(+), 81 deletions(-)

1 0

[tor-browser-build/master] Bug 32027: Bump Go to 1.13.10
by sysrqb＠torproject.org 27 Apr '20

27 Apr '20

commit 260a8442b8b34ace8920c93b3e92cbe3e06a0c7f Author: Georg Koppen <gk(a)torproject.org> Date: Sat Apr 18 08:04:50 2020 +0000 Bug 32027: Bump Go to 1.13.10 Patch based on dcf's GO111MODULE idea as outlined in #32027. Thanks! --- .../go/0001-Use-fixed-go-build-tmp-directory.patch | 22 ++++----- .../90a3ce02dc25adcf1598faf11a66b151ada3f637.patch | 57 ---------------------- projects/go/build | 2 - projects/go/config | 12 +++-- projects/obfs4/build | 7 +-- projects/snowflake/build | 4 -- 6 files changed, 23 insertions(+), 81 deletions(-) diff --git a/projects/go/0001-Use-fixed-go-build-tmp-directory.patch b/projects/go/0001-Use-fixed-go-build-tmp-directory.patch index a10e059..022aa23 100644 --- a/projects/go/0001-Use-fixed-go-build-tmp-directory.patch +++ b/projects/go/0001-Use-fixed-go-build-tmp-directory.patch @@ -1,33 +1,33 @@ -From 44e3391a5a3d9a2c4b57f78e43414465ab84681e Mon Sep 17 00:00:00 2001 -From: Nicolas Vigier <boklm(a)torproject.org> -Date: Thu, 27 Jun 2019 19:21:05 +0200 +From 958f724b1b434702c3d44ceaca646791674d07b1 Mon Sep 17 00:00:00 2001 +From: Georg Koppen <gk(a)torproject.org> +Date: Fri, 17 Apr 2020 10:24:23 +0000 Subject: [PATCH] Use fixed go-build tmp directory Use fixed go-build tmp directory, when the directory does not exist, in order to avoid some reproducibility issues where build directory gets embedded in generated binaries. ---- - src/cmd/go/internal/work/action.go | 10 +++++++--- - 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/cmd/go/internal/work/action.go b/src/cmd/go/internal/work/action.go -index 1f91046eb1..ba74b26a38 100644 +index 33b7818fb2..5e369d0f53 100644 --- a/src/cmd/go/internal/work/action.go +++ b/src/cmd/go/internal/work/action.go -@@ -224,9 +224,13 @@ func (b *Builder) Init() { +@@ -249,9 +249,13 @@ func (b *Builder) Init() { if cfg.BuildN { b.WorkDir = "$WORK" } else { -- tmp, err := ioutil.TempDir(os.Getenv("GOTMPDIR"), "go-build") +- tmp, err := ioutil.TempDir(cfg.Getenv("GOTMPDIR"), "go-build") - if err != nil { - base.Fatalf("go: creating work dir: %v", err) -+ tmp := filepath.Join(os.Getenv("GOTMPDIR"), "go-build-workdir") ++ tmp := filepath.Join(cfg.Getenv("GOTMPDIR"), "go-build-workdir") + _, err := os.Stat(tmp) + if !os.IsNotExist(err) { -+ tmp, err = ioutil.TempDir(os.Getenv("GOTMPDIR"), "go-build") ++ tmp, err = ioutil.TempDir(cfg.Getenv("GOTMPDIR"), "go-build") + if err != nil { + base.Fatalf("go: creating work dir: %v", err) + } } if !filepath.IsAbs(tmp) { abs, err := filepath.Abs(tmp) +-- +2.26.1 + diff --git a/projects/go/90a3ce02dc25adcf1598faf11a66b151ada3f637.patch b/projects/go/90a3ce02dc25adcf1598faf11a66b151ada3f637.patch deleted file mode 100644 index dc66809..0000000 --- a/projects/go/90a3ce02dc25adcf1598faf11a66b151ada3f637.patch +++ /dev/null @@ -1,57 +0,0 @@ -From 90a3ce02dc25adcf1598faf11a66b151ada3f637 Mon Sep 17 00:00:00 2001 -From: Elias Naur <mail(a)eliasnaur.com> -Date: Wed, 27 Mar 2019 14:25:24 +0100 -Subject: [PATCH] cmd/link/internal/ld: skip TLS section on Android - -We don't use the TLS section on android, and dropping it avoids -complaints about underalignment from the Android Q linker. - -Updates #29674 - -Change-Id: I91dabf2a58e6eb1783872639a6a144858db09cef -Reviewed-on: https://go-review.googlesource.com/c/go/+/169618 -Reviewed-by: Ian Lance Taylor <iant(a)golang.org> ---- - src/cmd/link/internal/ld/lib.go | 29 +++++++++++++++++------------ - 1 file changed, 17 insertions(+), 12 deletions(-) - -diff --git a/src/cmd/link/internal/ld/lib.go b/src/cmd/link/internal/ld/lib.go -index 1d44c0eb18b..b331e39fe3e 100644 ---- a/src/cmd/link/internal/ld/lib.go -+++ b/src/cmd/link/internal/ld/lib.go -@@ -453,18 +453,23 @@ func (ctxt *Link) loadlib() { - } - } - -- tlsg := ctxt.Syms.Lookup("runtime.tlsg", 0) -- -- // runtime.tlsg is used for external linking on platforms that do not define -- // a variable to hold g in assembly (currently only intel). -- if tlsg.Type == 0 { -- tlsg.Type = sym.STLSBSS -- tlsg.Size = int64(ctxt.Arch.PtrSize) -- } else if tlsg.Type != sym.SDYNIMPORT { -- Errorf(nil, "runtime declared tlsg variable %v", tlsg.Type) -- } -- tlsg.Attr |= sym.AttrReachable -- ctxt.Tlsg = tlsg -+ // The Android Q linker started to complain about underalignment of the our TLS -+ // section. We don't actually use the section on android, so dont't -+ // generate it. -+ if objabi.GOOS != "android" { -+ tlsg := ctxt.Syms.Lookup("runtime.tlsg", 0) -+ -+ // runtime.tlsg is used for external linking on platforms that do not define -+ // a variable to hold g in assembly (currently only intel). -+ if tlsg.Type == 0 { -+ tlsg.Type = sym.STLSBSS -+ tlsg.Size = int64(ctxt.Arch.PtrSize) -+ } else if tlsg.Type != sym.SDYNIMPORT { -+ Errorf(nil, "runtime declared tlsg variable %v", tlsg.Type) -+ } -+ tlsg.Attr |= sym.AttrReachable -+ ctxt.Tlsg = tlsg -+ } - - var moduledata *sym.Symbol - if ctxt.BuildMode == BuildModePlugin { diff --git a/projects/go/build b/projects/go/build index 06af067..02fba2b 100644 --- a/projects/go/build +++ b/projects/go/build @@ -68,8 +68,6 @@ cd /var/tmp/dist/go/src CGO_ENABLED=1 CC_FOR_TARGET="$CC_FOR_TARGET" CC= CFLAGS= LDFLAGS= ./make.bash [% ELSIF c("var/android") -%] patch -p2 < $rootdir/0001-Use-fixed-go-build-tmp-directory.patch - # Obfs4 breaks on Android Q without this patch, see: #32303 - patch -p2 < $rootdir/90a3ce02dc25adcf1598faf11a66b151ada3f637.patch CGO_ENABLED=1 CC_FOR_TARGET=[% c("var/CC") %] CGO_CFLAGS='-D__ANDROID_API__=[% c("var/android_min_api") %]' CC= CFLAGS= LDFLAGS= ./make.bash [% END -%] diff --git a/projects/go/config b/projects/go/config index 371f8c2..509c6da 100644 --- a/projects/go/config +++ b/projects/go/config @@ -1,5 +1,5 @@ # vim: filetype=yaml sw=2 -version: 1.12.13 +version: 1.13.10 filename: '[% project %]-[% c("version") %]-[% c("var/build_id") %].tar.gz' var: @@ -17,6 +17,9 @@ var: export GOOS=[% c("var/GOOS") %] export GOARCH=[% c("var/GOARCH") %] [% END -%] + # Run in GOPATH mode: https://golang.org/cmd/go/#hdr-Module_support. (see: + # #32027) + export GO111MODULE=off export GOPATH=/var/tmp/dist/gopath export PATH=/var/tmp/dist/go/bin:/var/tmp/dist/gopath/bin:"$PATH" [% IF c("var/linux") -%] @@ -50,6 +53,9 @@ var: do patch -p1 < $p done + # Run in GOPATH mode: https://golang.org/cmd/go/#hdr-Module_support. (see: + # #32027) + export GO111MODULE=off [% IF c("var/build_go_lib_pre"); GET c("var/build_go_lib_pre"); END; -%] [% IF c("var/go_lib_install") -%] [% FOREACH inst IN c("var/go_lib_install") %] @@ -112,11 +118,9 @@ input_files: enable: '[% ! c("var/linux") %]' - URL: 'https://golang.org/dl/go[% c("version") %].src.tar.gz' name: go - sha256sum: 5383d3b8db4baa48284ffcb14606d9cad6f03e9db843fa6d835b94d63cccf5a7 + sha256sum: eb9ccc8bf59ed068e7eff73e154e4f5ee7eec0a47a610fb864e3332a2fdc8b8c - URL: 'https://golang.org/dl/go[% c("var/go14_version") %].src.tar.gz' name: go14 sha256sum: 9947fc705b0b841b5938c48b22dc33e9647ec0752bae66e50278df4f23f64959 - filename: 0001-Use-fixed-go-build-tmp-directory.patch enable: '[% c("var/android") %]' - - filename: 90a3ce02dc25adcf1598faf11a66b151ada3f637.patch - enable: '[% c("var/android") %]' diff --git a/projects/obfs4/build b/projects/obfs4/build index 53b83df..47c7228 100644 --- a/projects/obfs4/build +++ b/projects/obfs4/build @@ -7,6 +7,9 @@ mkdir -p $PTDIR [% IF c("var/android") -%] [% pc(c('var/compiler'), 'var/setup', { compiler_tarfile => c('input_files_by_name/' _ c('var/compiler')) }) %] + # We need to explicitly set CGO_ENABLED with Go 1.13.x as the Android build + # breaks otherwise. + export CGO_ENABLED=1 [% END -%] tar -C /var/tmp/dist -xf [% c('input_files_by_name/goptlib') %] @@ -22,11 +25,9 @@ mkdir -p /var/tmp/build tar -C /var/tmp/build -xf [% project %]-[% c('version') %].tar.gz cd /var/tmp/build/[% project %]-[% c('version') %] -# Remove go.mod and go.sum files until we can build using Go module -# versioning (see bug 28325). -rm -f go.mod go.sum mkdir -p "$GOPATH/src/gitlab.com/yawning" ln -sf "$PWD" "$GOPATH/src/gitlab.com/yawning/obfs4.git" + cd obfs4proxy go build -ldflags '-s' cp -a obfs4proxy[% IF c("var/windows") %].exe[% END %] $PTDIR diff --git a/projects/snowflake/build b/projects/snowflake/build index fe74e84..1aede45 100644 --- a/projects/snowflake/build +++ b/projects/snowflake/build @@ -15,10 +15,6 @@ cd /var/tmp/build/[% project %]-[% c('version') %] mkdir -p "$GOPATH/src/git.torproject.org/pluggable-transports" ln -sf "$PWD" "$GOPATH/src/git.torproject.org/pluggable-transports/snowflake.git" - -# Remove go.mod and go.sum files until we can build using Go module versioning -rm -f go.mod go.sum - # Fix gopath location of versioned dependencies ln -sf "$GOPATH/src/github.com/pion/webrtc" "$GOPATH/src/github.com/pion/webrtc/v2" ln -sf "$GOPATH/src/github.com/pion/sdp" "$GOPATH/src/github.com/pion/sdp/v2"

1 0

[gettor/master] Check SMTP error code and remove request from db
by cohosh＠torproject.org 27 Apr '20

27 Apr '20

commit 3a878213ba18178005238b0ded179fa84c5e1129 Author: Cecylia Bocovich <cohosh(a)torproject.org> Date: Mon Apr 27 14:36:58 2020 -0400 Check SMTP error code and remove request from db This adds an extra check on SMTP errors to see if it's due to a bad recipient address and then removes the request from the database. This prevents one bad request from hanging up the processing of other requests (see https://bugs.torproject.org/34027) --- gettor/services/email/sendmail.py | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/gettor/services/email/sendmail.py b/gettor/services/email/sendmail.py index 4d908b3..c4fdc95 100644 --- a/gettor/services/email/sendmail.py +++ b/gettor/services/email/sendmail.py @@ -18,19 +18,13 @@ import configparser from email.mime.text import MIMEText from twisted.internet import defer -from twisted.mail.smtp import sendmail +from twisted.mail import smtp from ...utils.db import SQLite3 as DB from ...utils.commons import log from ...utils import strings -class SMTPError(Exception): - """ - Error if we can't send emails. - """ - pass - from email.mime.text import MIMEText class Sendmail(object): """ @@ -70,7 +64,7 @@ class Sendmail(object): Errback if we don't/can't send the message. """ log.debug("Could not send email.") - raise SMTPError("{}".format(error)) + raise error def sendmail(self, email_addr, subject, body): """ @@ -93,7 +87,7 @@ class Sendmail(object): log.debug("Calling asynchronous sendmail.") - return sendmail( + return smtp.sendmail( self.settings.get("sendmail_host"), self.settings.get("sendmail_addr"), email_addr, message, requireTransportSecurity=True ).addCallback(self.sendmail_callback).addErrback(self.sendmail_errback) @@ -217,7 +211,14 @@ class Sendmail(object): id=id, service="email", date=date ) - except SMTPError as e: + except smtp.SMTPClientError as e: + if e.code == 501: # Bad recipient address syntax + yield self.conn.remove_request( + id=id, service="email", date=date + ) + log.info("Error sending email: {}.".format(e)) + + except Exception as e: log.info("Error sending email: {}.".format(e)) elif link_requests: @@ -270,7 +271,14 @@ class Sendmail(object): id=id, service="email", date=date ) - except SMTPError as e: + except smtp.SMTPClientError as e: + if e.code == 501: # Bad recipient address syntax + yield self.conn.remove_request( + id=id, service="email", date=date + ) + log.info("Error sending email: {}.".format(e)) + + except Exception as e: log.info("Error sending email: {}.".format(e)) else: log.debug("No pending email requests. Keep waiting.")

1 0