tor-commits March 2018

tor-commits@lists.torproject.org

15 participants
1430 discussions

[tor-browser/tor-browser-52.7.2esr-8.0-1] Bug 23439: Exempt .onion domains from mixed content warnings
by gk＠torproject.org 21 Mar '18

21 Mar '18

commit 1316acb053d6191176e9ae4e4f502415b068525e Author: Georg Koppen <gk(a)torproject.org> Date: Wed Mar 21 08:21:01 2018 +0000 Bug 23439: Exempt .onion domains from mixed content warnings Tests --- browser/base/content/test/general/browser.ini | 4 +++ .../test/general/browser_no_mcb_for_onions.js | 39 ++++++++++++++++++++++ .../test/general/test_no_mcb_for_onions.html | 28 ++++++++++++++++ 3 files changed, 71 insertions(+) diff --git a/browser/base/content/test/general/browser.ini b/browser/base/content/test/general/browser.ini index 96e591ffea7b..321e444c2cde 100644 --- a/browser/base/content/test/general/browser.ini +++ b/browser/base/content/test/general/browser.ini @@ -492,3 +492,7 @@ tags = mcb [browser_newwindow_focus.js] skip-if = (os == "linux" && !e10s) # Bug 1263254 - Perma fails on Linux without e10s for some reason. [browser_bug1299667.js] +[browser_no_mcb_for_onions.js] +tags = mcb +support-files = + test_no_mcb_for_onions.html diff --git a/browser/base/content/test/general/browser_no_mcb_for_onions.js b/browser/base/content/test/general/browser_no_mcb_for_onions.js new file mode 100644 index 000000000000..8023b78276f3 --- /dev/null +++ b/browser/base/content/test/general/browser_no_mcb_for_onions.js @@ -0,0 +1,39 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +// The test loads a HTTPS web page with active content from HTTP .onion URLs +// and makes sure that the mixed content flags on the docshell are not set. +// +// Note that the URLs referenced within the test page intentionally use the +// unassigned port 8 because we don't want to actually load anything, we just +// want to check that the URLs are not blocked. + +const TEST_URL = getRootDirectory(gTestPath).replace("chrome://mochitests/content", "https://example.com") + "test_no_mcb_for_onions.html"; + +const PREF_BLOCK_DISPLAY = "security.mixed_content.block_display_content"; +const PREF_BLOCK_ACTIVE = "security.mixed_content.block_active_content"; +const PREF_ONION_WHITELIST = "dom.securecontext.whitelist_onions"; + +add_task(async function allowOnionMixedContent() { + registerCleanupFunction(function() { + gBrowser.removeCurrentTab(); + }); + + await SpecialPowers.pushPrefEnv({set: [[PREF_BLOCK_DISPLAY, true]]}); + await SpecialPowers.pushPrefEnv({set: [[PREF_BLOCK_ACTIVE, true]]}); + await SpecialPowers.pushPrefEnv({set: [[PREF_ONION_WHITELIST, true]]}); + + const tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL); + const browser = gBrowser.getBrowserForTab(tab); + + await ContentTask.spawn(browser, null, function() { + is(docShell.hasMixedDisplayContentBlocked, false, "hasMixedDisplayContentBlocked not set"); + is(docShell.hasMixedActiveContentBlocked, false, "hasMixedActiveContentBlocked not set"); + }); + + await assertMixedContentBlockingState(browser, { + activeBlocked: false, + activeLoaded: false, + passiveLoaded: false, + }); +}); diff --git a/browser/base/content/test/general/test_no_mcb_for_onions.html b/browser/base/content/test/general/test_no_mcb_for_onions.html new file mode 100644 index 000000000000..9715d526bf87 --- /dev/null +++ b/browser/base/content/test/general/test_no_mcb_for_onions.html @@ -0,0 +1,28 @@ + +<!DOCTYPE HTML> +<html> + <head> + <meta charset="utf8"> + <title>Bug 1382359</title> + </head> + + <style> + @font-face { + src: url("http://123456789abcdef.onion:8/test.ttf"); + } + </style> + + <body> + <img src="http://123456789abcdef.onion:8/test.png"> + + <iframe src="http://123456789abcdef.onion:8/test.html"></iframe> + </body> + + <script src="http://123456789abcdef.onion:8/test.js"></script> + + <link href="http://123456789abcdef.onion:8/test.css" rel="stylesheet"></link> + + <script> + fetch("http://123456789abcdef.onion:8"); + </script> +</html>

1 0

[snowflake/master] Don't log "SOCKS accepted" if there was an Accept error.
by dcf＠torproject.org 21 Mar '18

21 Mar '18

commit 6a9c2bf95123373d55798bcabcb439ef7c24f205 Author: David Fifield <david(a)bamsoftware.com> Date: Wed Mar 21 00:43:17 2018 -0700 Don't log "SOCKS accepted" if there was an Accept error. This actually caused a nil pointer dereference panic when Accept returned an error, because conn was nil. --- client/snowflake.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/snowflake.go b/client/snowflake.go index 3b8a248..105fd30 100644 --- a/client/snowflake.go +++ b/client/snowflake.go @@ -57,13 +57,13 @@ func socksAcceptLoop(ln *pt.SocksListener, snowflakes SnowflakeCollector) error for { log.Println("SOCKS listening...") conn, err := ln.AcceptSocks() - log.Println("SOCKS accepted: ", conn.Req) if err != nil { if e, ok := err.(net.Error); ok && e.Temporary() { continue } return err } + log.Println("SOCKS accepted: ", conn.Req) err = handler(conn, snowflakes) if err != nil { log.Printf("handler error: %s", err)

1 0

[snowflake/master] Exit immediately after SIGTERM is there are no signals running.
by dcf＠torproject.org 21 Mar '18

21 Mar '18

commit bdc1798adb83a34b5f0362c67dc5d997ed4840bd Author: David Fifield <david(a)bamsoftware.com> Date: Wed Mar 21 00:51:04 2018 -0700 Exit immediately after SIGTERM is there are no signals running. Cf. https://bugs.torproject.org/24875 --- client/snowflake.go | 7 ++----- server/server.go | 7 ++----- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/client/snowflake.go b/client/snowflake.go index 105fd30..e0f9746 100644 --- a/client/snowflake.go +++ b/client/snowflake.go @@ -241,11 +241,8 @@ func main() { ln.Close() } snowflakes.End() - for n := range handlerChan { - numHandlers += n - if numHandlers == 0 { - break - } + for numHandlers > 0 { + numHandlers += <-handlerChan } log.Println("snowflake is done.") } diff --git a/server/server.go b/server/server.go index 0136fc0..7c5a205 100644 --- a/server/server.go +++ b/server/server.go @@ -393,10 +393,7 @@ func main() { for _, server := range servers { server.Close() } - for n := range handlerChan { - numHandlers += n - if numHandlers == 0 { - break - } + for numHandlers > 0 { + numHandlers += <-handlerChan } }

1 0

[snowflake/master] Update server shutdown procedure.
by dcf＠torproject.org 21 Mar '18

21 Mar '18

commit 019e2cea23f7a78f8ef2132ce7d2ad64b365af24 Author: David Fifield <david(a)bamsoftware.com> Date: Wed Mar 21 00:53:31 2018 -0700 Update server shutdown procedure. Ignore SIGINT, honor TOR_PT_EXIT_ON_STDIN_CLOSE. --- server-webrtc/snowflake.go | 29 +++++++++++++++-------------- server/server.go | 11 +++++++++++ 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/server-webrtc/snowflake.go b/server-webrtc/snowflake.go index 0484d94..4d6641a 100644 --- a/server-webrtc/snowflake.go +++ b/server-webrtc/snowflake.go @@ -5,6 +5,7 @@ import ( "flag" "fmt" "io" + "io/ioutil" "log" "net" "os" @@ -291,9 +292,19 @@ func main() { var numHandlers int = 0 var sig os.Signal sigChan := make(chan os.Signal, 1) - signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM) + signal.Notify(sigChan, syscall.SIGTERM) - // wait for first signal + if os.Getenv("TOR_PT_EXIT_ON_STDIN_CLOSE") == "1" { + // This environment variable means we should treat EOF on stdin + // just like SIGTERM: https://bugs.torproject.org/15435. + go func() { + io.Copy(ioutil.Discard, os.Stdin) + log.Printf("synthesizing SIGTERM because of stdin close") + sigChan <- syscall.SIGTERM + }() + } + + // keep track of handlers and wait for a signal sig = nil for sig == nil { select { @@ -303,17 +314,7 @@ func main() { } } - if sig == syscall.SIGTERM { - return - } - - // wait for second signal or no more handlers - sig = nil - for sig == nil && numHandlers != 0 { - select { - case n := <-handlerChan: - numHandlers += n - case sig = <-sigChan: - } + for numHandlers > 0 { + numHandlers += <-handlerChan } } diff --git a/server/server.go b/server/server.go index 7c5a205..03095af 100644 --- a/server/server.go +++ b/server/server.go @@ -8,6 +8,7 @@ import ( "flag" "fmt" "io" + "io/ioutil" "log" "net" "net/http" @@ -378,6 +379,16 @@ func main() { sigChan := make(chan os.Signal, 1) signal.Notify(sigChan, syscall.SIGTERM) + if os.Getenv("TOR_PT_EXIT_ON_STDIN_CLOSE") == "1" { + // This environment variable means we should treat EOF on stdin + // just like SIGTERM: https://bugs.torproject.org/15435. + go func() { + io.Copy(ioutil.Discard, os.Stdin) + log.Printf("synthesizing SIGTERM because of stdin close") + sigChan <- syscall.SIGTERM + }() + } + // keep track of handlers and wait for a signal sig = nil for sig == nil {

1 0

[meek/master] Exit immediately after SIGTERM is there are no signals running.
by dcf＠torproject.org 21 Mar '18

21 Mar '18

commit 1a25ac4232675e6d5efa91ad062610efc771aaaf Author: David Fifield <david(a)bamsoftware.com> Date: Tue Mar 20 23:25:10 2018 -0700 Exit immediately after SIGTERM is there are no signals running. The logic erroneously assumed that there was at least one handler running at the time a signal was received. If there was not, it would wait forever for something handler-related to happen. https://bugs.torproject.org/24875 --- meek-client/meek-client.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/meek-client/meek-client.go b/meek-client/meek-client.go index c2ceaf9..bbacbb4 100644 --- a/meek-client/meek-client.go +++ b/meek-client/meek-client.go @@ -481,11 +481,8 @@ func main() { for _, ln := range listeners { ln.Close() } - for n := range handlerChan { - numHandlers += n - if numHandlers == 0 { - break - } + for numHandlers > 0 { + numHandlers += <-handlerChan } log.Printf("done")

1 0

[meek/master] Update server shutdown procedure.
by dcf＠torproject.org 21 Mar '18

21 Mar '18

commit 502001aed9e40f79807b913b02ea82f7c381e47d Author: David Fifield <david(a)bamsoftware.com> Date: Tue Mar 20 23:24:10 2018 -0700 Update server shutdown procedure. Ignore SIGINT, honor TOR_PT_EXIT_ON_STDIN_CLOSE. --- meek-server/meek-server.go | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/meek-server/meek-server.go b/meek-server/meek-server.go index 79dd80c..fe4c263 100644 --- a/meek-server/meek-server.go +++ b/meek-server/meek-server.go @@ -24,6 +24,7 @@ import ( "flag" "fmt" "io" + "io/ioutil" "log" "net" "net/http" @@ -496,9 +497,19 @@ func main() { var numHandlers int = 0 var sig os.Signal sigChan := make(chan os.Signal, 1) - signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM) + signal.Notify(sigChan, syscall.SIGTERM) + + if os.Getenv("TOR_PT_EXIT_ON_STDIN_CLOSE") == "1" { + // This environment variable means we should treat EOF on stdin + // just like SIGTERM: https://bugs.torproject.org/15435. + go func() { + io.Copy(ioutil.Discard, os.Stdin) + log.Printf("synthesizing SIGTERM because of stdin close") + sigChan <- syscall.SIGTERM + }() + } - // Wait for first signal. + // Keep track of handlers and wait for a signal. sig = nil for sig == nil { select { @@ -508,27 +519,15 @@ func main() { log.Printf("got signal %s", sig) } } + /* // Not supported until go1.8. for _, server := range servers { server.Close() } */ - - if sig == syscall.SIGTERM { - log.Printf("done") - return - } - - // Wait for second signal or no more handlers. - sig = nil - for sig == nil && numHandlers != 0 { - select { - case n := <-handlerChan: - numHandlers += n - case sig = <-sigChan: - log.Printf("got second signal %s", sig) - } + for numHandlers > 0 { + numHandlers += <-handlerChan } log.Printf("done")

1 0

[goptlib/master] Add TOR_PT_EXIT_ON_STDIN_CLOSE to dummy example programs.
by dcf＠torproject.org 21 Mar '18

21 Mar '18

commit c03acac8316d24ac99cf9bfcb72da1dc9a6ff3c3 Author: David Fifield <david(a)bamsoftware.com> Date: Tue Mar 20 23:00:40 2018 -0700 Add TOR_PT_EXIT_ON_STDIN_CLOSE to dummy example programs. --- examples/dummy-client/dummy-client.go | 10 ++++++++++ examples/dummy-server/dummy-server.go | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/examples/dummy-client/dummy-client.go b/examples/dummy-client/dummy-client.go index a15189c..de7f367 100644 --- a/examples/dummy-client/dummy-client.go +++ b/examples/dummy-client/dummy-client.go @@ -12,6 +12,7 @@ package main import ( "io" + "io/ioutil" "net" "os" "os/signal" @@ -116,6 +117,15 @@ func main() { sigChan := make(chan os.Signal, 1) signal.Notify(sigChan, syscall.SIGTERM) + if os.Getenv("TOR_PT_EXIT_ON_STDIN_CLOSE") == "1" { + // This environment variable means we should treat EOF on stdin + // just like SIGTERM: https://bugs.torproject.org/15435. + go func() { + io.Copy(ioutil.Discard, os.Stdin) + sigChan <- syscall.SIGTERM + }() + } + // keep track of handlers and wait for a signal sig = nil for sig == nil { diff --git a/examples/dummy-server/dummy-server.go b/examples/dummy-server/dummy-server.go index c31c63f..aea4df4 100644 --- a/examples/dummy-server/dummy-server.go +++ b/examples/dummy-server/dummy-server.go @@ -13,6 +13,7 @@ package main import ( "io" + "io/ioutil" "net" "os" "os/signal" @@ -108,6 +109,15 @@ func main() { sigChan := make(chan os.Signal, 1) signal.Notify(sigChan, syscall.SIGTERM) + if os.Getenv("TOR_PT_EXIT_ON_STDIN_CLOSE") == "1" { + // This environment variable means we should treat EOF on stdin + // just like SIGTERM: https://bugs.torproject.org/15435. + go func() { + io.Copy(ioutil.Discard, os.Stdin) + sigChan <- syscall.SIGTERM + }() + } + // keep track of handlers and wait for a signal sig = nil for sig == nil {

1 0

[goptlib/master] Exit immediately after SIGTERM even if no handlers are running.
by dcf＠torproject.org 21 Mar '18

21 Mar '18

commit 7d56ec4f381e8b1aedfda360594e35a5731b5337 Author: David Fifield <david(a)bamsoftware.com> Date: Tue Mar 20 23:14:16 2018 -0700 Exit immediately after SIGTERM even if no handlers are running. The loop logic assumed that there was at least one handler running when a signal was received; if there were none, it would wait forever for a handler event to happen. Cf. https://bugs.torproject.org/24875 --- examples/dummy-client/dummy-client.go | 7 ++----- examples/dummy-server/dummy-server.go | 7 ++----- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/examples/dummy-client/dummy-client.go b/examples/dummy-client/dummy-client.go index de7f367..4812a9b 100644 --- a/examples/dummy-client/dummy-client.go +++ b/examples/dummy-client/dummy-client.go @@ -140,10 +140,7 @@ func main() { for _, ln := range listeners { ln.Close() } - for n := range handlerChan { - numHandlers += n - if numHandlers == 0 { - break - } + for numHandlers > 0 { + numHandlers += <-handlerChan } } diff --git a/examples/dummy-server/dummy-server.go b/examples/dummy-server/dummy-server.go index aea4df4..db24fab 100644 --- a/examples/dummy-server/dummy-server.go +++ b/examples/dummy-server/dummy-server.go @@ -132,10 +132,7 @@ func main() { for _, ln := range listeners { ln.Close() } - for n := range handlerChan { - numHandlers += n - if numHandlers == 0 { - break - } + for numHandlers > 0 { + numHandlers += <-handlerChan } }

1 0

[snowflake/master] Add synchronization around destroying DataChannels and PeerConnections
by arlo＠torproject.org 21 Mar '18

21 Mar '18

commit 1114acbcb4acb82174b293983ced8afcaf9e2a93 Author: Arlo Breault <arlolra(a)gmail.com> Date: Wed Mar 14 13:35:39 2018 -0400 Add synchronization around destroying DataChannels and PeerConnections From https://trac.torproject.org/projects/tor/ticket/21312#comment:33 --- client/webrtc.go | 36 ++++++++++++++++++++++++++++-------- proxy-go/snowflake.go | 17 +++++++++++++---- server-webrtc/snowflake.go | 17 +++++++++++++---- 3 files changed, 54 insertions(+), 16 deletions(-) diff --git a/client/webrtc.go b/client/webrtc.go index e35c47d..8c7cb4c 100644 --- a/client/webrtc.go +++ b/client/webrtc.go @@ -5,6 +5,7 @@ import ( "errors" "io" "log" + "sync" "time" "github.com/dchest/uniuri" @@ -35,6 +36,9 @@ type WebRTCPeer struct { closed bool + lock sync.Mutex // Synchronization for DataChannel destruction + once sync.Once // Synchronization for PeerConnection destruction + BytesLogger } @@ -69,6 +73,8 @@ 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 { @@ -82,14 +88,12 @@ func (c *WebRTCPeer) Write(b []byte) (int, error) { // As part of |Snowflake| func (c *WebRTCPeer) Close() error { - if c.closed { // Skip if already closed. - return nil - } - // Mark for deletion. - c.closed = true - c.cleanup() - c.Reset() - log.Printf("WebRTC: Closing") + c.once.Do(func() { + c.closed = true + c.cleanup() + c.Reset() + log.Printf("WebRTC: Closing") + }) return nil } @@ -194,6 +198,8 @@ func (c *WebRTCPeer) preparePeerConnection() error { // 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!") } @@ -206,6 +212,8 @@ func (c *WebRTCPeer) establishDataChannel() error { return err } dc.OnOpen = func() { + c.lock.Lock() + defer c.lock.Unlock() log.Println("WebRTC: DataChannel.OnOpen") if nil != c.transport { panic("WebRTC: transport already exists.") @@ -220,10 +228,12 @@ func (c *WebRTCPeer) establishDataChannel() error { c.transport = dc } 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. @@ -231,6 +241,9 @@ func (c *WebRTCPeer) establishDataChannel() error { log.Println("WebRTC: DataChannel.OnClose [remotely]") c.transport = nil c.pc.DeleteDataChannel(dc) + // 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() c.Close() } dc.OnMessage = func(msg []byte) { @@ -321,16 +334,23 @@ func (c *WebRTCPeer) cleanup() { c.writePipe.Close() c.writePipe = nil } + c.lock.Lock() 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.") } c.pc.DeleteDataChannel(dataChannel.(*webrtc.DataChannel)) + } else { + c.lock.Unlock() } if nil != c.pc { log.Printf("WebRTC: closing PeerConnection") diff --git a/proxy-go/snowflake.go b/proxy-go/snowflake.go index a12cfb2..5a5925e 100644 --- a/proxy-go/snowflake.go +++ b/proxy-go/snowflake.go @@ -62,6 +62,9 @@ type webRTCConn struct { dc *webrtc.DataChannel pc *webrtc.PeerConnection pr *io.PipeReader + + lock sync.Mutex // Synchronization for DataChannel destruction + once sync.Once // Synchronization for PeerConnection destruction } func (c *webRTCConn) Read(b []byte) (int, error) { @@ -69,6 +72,8 @@ func (c *webRTCConn) Read(b []byte) (int, error) { } func (c *webRTCConn) Write(b []byte) (int, error) { + c.lock.Lock() + defer c.lock.Unlock() // log.Printf("webrtc Write %d %+q", len(b), string(b)) log.Printf("Write %d bytes --> WebRTC", len(b)) if c.dc != nil { @@ -77,8 +82,11 @@ func (c *webRTCConn) Write(b []byte) (int, error) { return len(b), nil } -func (c *webRTCConn) Close() error { - return c.pc.Destroy() +func (c *webRTCConn) Close() (err error) { + c.once.Do(func() { + err = c.pc.Destroy() + }) + return } func (c *webRTCConn) LocalAddr() net.Addr { @@ -255,17 +263,18 @@ func makePeerConnectionFromOffer(sdp *webrtc.SessionDescription, config *webrtc. log.Println("OnDataChannel") pr, pw := io.Pipe() - conn := &webRTCConn{pc: pc, dc: dc, pr: pr} dc.OnOpen = func() { log.Println("OnOpen channel") } dc.OnClose = func() { + conn.lock.Lock() + defer conn.lock.Unlock() log.Println("OnClose channel") - pw.Close() conn.dc = nil pc.DeleteDataChannel(dc) + pw.Close() } dc.OnMessage = func(msg []byte) { log.Printf("OnMessage <--- %d bytes", len(msg)) diff --git a/server-webrtc/snowflake.go b/server-webrtc/snowflake.go index 82b6afe..0484d94 100644 --- a/server-webrtc/snowflake.go +++ b/server-webrtc/snowflake.go @@ -43,6 +43,9 @@ type webRTCConn struct { dc *webrtc.DataChannel pc *webrtc.PeerConnection pr *io.PipeReader + + lock sync.Mutex // Synchronization for DataChannel destruction + once sync.Once // Synchronization for PeerConnection destruction } func (c *webRTCConn) Read(b []byte) (int, error) { @@ -50,6 +53,8 @@ func (c *webRTCConn) Read(b []byte) (int, error) { } func (c *webRTCConn) Write(b []byte) (int, error) { + c.lock.Lock() + defer c.lock.Unlock() // log.Printf("webrtc Write %d %+q", len(b), string(b)) log.Printf("Write %d bytes --> WebRTC", len(b)) if c.dc != nil { @@ -58,8 +63,11 @@ func (c *webRTCConn) Write(b []byte) (int, error) { return len(b), nil } -func (c *webRTCConn) Close() error { - return c.pc.Destroy() +func (c *webRTCConn) Close() (err error) { + c.once.Do(func() { + err = c.pc.Destroy() + }) + return } func (c *webRTCConn) LocalAddr() net.Addr { @@ -122,17 +130,18 @@ func makePeerConnectionFromOffer(sdp *webrtc.SessionDescription, config *webrtc. log.Println("OnDataChannel") pr, pw := io.Pipe() - conn := &webRTCConn{pc: pc, dc: dc, pr: pr} dc.OnOpen = func() { log.Println("OnOpen channel") } dc.OnClose = func() { + conn.lock.Lock() + defer conn.lock.Unlock() log.Println("OnClose channel") - pw.Close() conn.dc = nil pc.DeleteDataChannel(dc) + pw.Close() } dc.OnMessage = func(msg []byte) { log.Printf("OnMessage <--- %d bytes", len(msg))

1 0

[tor/maint-0.3.3] hs-v3: BUG() on missing descriptors during rotation
by nickm＠torproject.org 20 Mar '18

20 Mar '18

commit 5804ccc9070dc5443f1c6ce565dbf17572812764 Author: David Goulet <dgoulet(a)torproject.org> Date: Mon Feb 26 10:45:58 2018 -0500 hs-v3: BUG() on missing descriptors during rotation Because of #25306 for which we are unable to reproduce nor understand how it is possible, this commit removes the asserts() and BUG() on the missing descriptors instead when rotating them. This allows us to log more data on error but also to let tor recover gracefully instead of dying. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- changes/bug25306 | 6 ++++++ src/or/hs_service.c | 32 ++++++++++++++++++++++++++++---- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/changes/bug25306 b/changes/bug25306 new file mode 100644 index 000000000..a2e6306f4 --- /dev/null +++ b/changes/bug25306 @@ -0,0 +1,6 @@ + o Minor bugfixes (hidden service v3): + - Avoid asserting when building descriptors in the next rotation time is + out of sync with the consensus valid after time. Instead, log a bug + warning with extra information to hunt down the cause of this assert. + Fixes bug 25306; bugfix on 0.3.2.1-alpha. + diff --git a/src/or/hs_service.c b/src/or/hs_service.c index 45810c5c5..b6338ae6b 100644 --- a/src/or/hs_service.c +++ b/src/or/hs_service.c @@ -1508,7 +1508,9 @@ build_all_descriptors(time_t now) * empty, we'll try to build it for the next time period. This only * happens when we rotate meaning that we are guaranteed to have a new SRV * at that point for the next time period. */ - tor_assert(service->desc_current); + if (BUG(service->desc_current == NULL)) { + continue; + } if (service->desc_next == NULL) { build_service_descriptor(service, now, hs_get_next_time_period_num(0), @@ -1925,6 +1927,31 @@ should_rotate_descriptors(hs_service_t *service, time_t now) } if (ns->valid_after >= service->state.next_rotation_time) { + /* In theory, we should never get here with no descriptors. We can never + * have a NULL current descriptor except when tor starts up. The next + * descriptor can be NULL after a rotation but we build a new one right + * after. + * + * So, when tor starts, the next rotation time is set to the start of the + * next SRV period using the consensus valid after time so it should + * always be set to a future time value. This means that we should never + * reach this point at bootup that is this check safeguards tor in never + * allowing a rotation if the valid after time is smaller than the next + * rotation time. + * + * This is all good in theory but we've had a NULL descriptor issue here + * so this is why we BUG() on both with extra logging to try to understand + * how this can possibly happens. We'll simply ignore and tor should + * recover from this by skipping rotation and building the missing + * descriptors just after this. */ + if (BUG(service->desc_current == NULL || service->desc_next == NULL)) { + log_warn(LD_BUG, "Service descriptor is NULL (%p/%p). Next rotation " + "time is %ld (now: %ld). Valid after time from " + "consensus is %ld", + service->desc_current, service->desc_next, + service->state.next_rotation_time, now, ns->valid_after); + goto no_rotation; + } goto rotation; } @@ -1977,9 +2004,6 @@ rotate_all_descriptors(time_t now) continue; } - tor_assert(service->desc_current); - tor_assert(service->desc_next); - log_info(LD_REND, "Time to rotate our descriptors (%p / %p) for %s", service->desc_current, service->desc_next, safe_str_client(service->onion_address));

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

tor-commits March 2018