March 2018 - tor-commits - lists.torproject.org

[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 680dece41e71d30afd4616aa19001c60e55dc852 Author: Kathy Brade <brade(a)pearlcrescent.com> Date: Tue Mar 20 15:02:32 2018 -0400 Bug 23439: Exempt .onion domains from mixed content warnings --- dom/html/HTMLFormElement.cpp | 5 +++++ dom/security/nsContentSecurityManager.cpp | 7 +++--- dom/security/nsMixedContentBlocker.cpp | 37 ++++++++++++++++++++++++++++--- dom/security/nsMixedContentBlocker.h | 2 ++ 4 files changed, 44 insertions(+), 7 deletions(-) diff --git a/dom/html/HTMLFormElement.cpp b/dom/html/HTMLFormElement.cpp index 5164391f8d2b..71b6248d5b06 100644 --- a/dom/html/HTMLFormElement.cpp +++ b/dom/html/HTMLFormElement.cpp @@ -14,6 +14,7 @@ #include "mozilla/dom/AutocompleteErrorEvent.h" #include "mozilla/dom/nsCSPUtils.h" #include "mozilla/dom/nsCSPContext.h" +#include "mozilla/dom/nsMixedContentBlocker.h" #include "mozilla/dom/HTMLFormControlsCollection.h" #include "mozilla/dom/HTMLFormElementBinding.h" #include "mozilla/Move.h" @@ -907,6 +908,10 @@ HTMLFormElement::DoSecureToInsecureSubmitCheck(nsIURI* aActionURL, return NS_OK; } + if (nsMixedContentBlocker::IsPotentiallyTrustworthyOnion(aActionURL)) { + return NS_OK; + } + nsCOMPtr<nsPIDOMWindowOuter> window = OwnerDoc()->GetWindow(); if (!window) { return NS_ERROR_FAILURE; diff --git a/dom/security/nsContentSecurityManager.cpp b/dom/security/nsContentSecurityManager.cpp index c95226b56e91..aa4f735a5c4f 100644 --- a/dom/security/nsContentSecurityManager.cpp +++ b/dom/security/nsContentSecurityManager.cpp @@ -10,6 +10,7 @@ #include "nsMixedContentBlocker.h" #include "mozilla/dom/Element.h" +#include "mozilla/dom/nsMixedContentBlocker.h" NS_IMPL_ISUPPORTS(nsContentSecurityManager, nsIContentSecurityManager, @@ -689,11 +690,9 @@ nsContentSecurityManager::IsOriginPotentiallyTrustworthy(nsIPrincipal* aPrincipa } } } - // Maybe we have a .onion URL. Treat it as whitelisted as well when + // Maybe we have a .onion URL. Treat it as whitelisted as well if // `dom.securecontext.whitelist_onions` is `true`. - bool whitelistOnions = - Preferences::GetBool("dom.securecontext.whitelist_onions", false); - if (whitelistOnions && StringEndsWith(host, NS_LITERAL_CSTRING(".onion"))) { + if (nsMixedContentBlocker::IsPotentiallyTrustworthyOnion(uri)) { *aIsTrustWorthy = true; return NS_OK; } diff --git a/dom/security/nsMixedContentBlocker.cpp b/dom/security/nsMixedContentBlocker.cpp index a9aca5333491..98e262f0bcf4 100644 --- a/dom/security/nsMixedContentBlocker.cpp +++ b/dom/security/nsMixedContentBlocker.cpp @@ -394,6 +394,29 @@ nsMixedContentBlocker::ShouldLoad(uint32_t aContentType, return rv; } +/* Maybe we have a .onion URL. Treat it as whitelisted as well if + * `dom.securecontext.whitelist_onions` is `true`. + */ +bool +nsMixedContentBlocker::IsPotentiallyTrustworthyOnion(nsIURI* aURL) { + static bool sInited = false; + static bool sWhiteListOnions = false; + if (!sInited) { + Preferences::AddBoolVarCache(&sWhiteListOnions, + "dom.securecontext.whitelist_onions"); + sInited = true; + } + if (!sWhiteListOnions) { + return false; + } + + nsAutoCString host; + nsresult rv = aURL->GetHost(host); + NS_ENSURE_SUCCESS(rv, false); + return StringEndsWith(host, NS_LITERAL_CSTRING(".onion")); +} + + /* Static version of ShouldLoad() that contains all the Mixed Content Blocker * logic. Called from non-static ShouldLoad(). */ @@ -696,6 +719,17 @@ nsMixedContentBlocker::ShouldLoad(bool aHadInsecureImageRedirect, return NS_OK; } + bool isHttpScheme = false; + rv = innerContentLocation->SchemeIs("http", &isHttpScheme); + NS_ENSURE_SUCCESS(rv, rv); + + // .onion URLs are encrypted and authenticated. Don't treat them as mixed + // content if potentially trustworthy (i.e. whitelisted). + if (isHttpScheme && IsPotentiallyTrustworthyOnion(innerContentLocation)) { + *aDecision = ACCEPT; + return NS_OK; + } + // The page might have set the CSP directive 'upgrade-insecure-requests'. In such // a case allow the http: load to succeed with the promise that the channel will // get upgraded to https before fetching any data from the netwerk. @@ -707,9 +741,6 @@ nsMixedContentBlocker::ShouldLoad(bool aHadInsecureImageRedirect, // we only have to check against http: here. Skip mixed content blocking if the // subresource load uses http: and the CSP directive 'upgrade-insecure-requests' // is present on the page. - bool isHttpScheme = false; - rv = innerContentLocation->SchemeIs("http", &isHttpScheme); - NS_ENSURE_SUCCESS(rv, rv); nsIDocument* document = docShell->GetDocument(); MOZ_ASSERT(document, "Expected a document"); if (isHttpScheme && document->GetUpgradeInsecureRequests(isPreload)) { diff --git a/dom/security/nsMixedContentBlocker.h b/dom/security/nsMixedContentBlocker.h index 539c3ebbb7f0..24fbac171dac 100644 --- a/dom/security/nsMixedContentBlocker.h +++ b/dom/security/nsMixedContentBlocker.h @@ -43,6 +43,8 @@ public: nsMixedContentBlocker(); + static bool IsPotentiallyTrustworthyOnion(nsIURI* aURL); + /* Static version of ShouldLoad() that contains all the Mixed Content Blocker * logic. Called from non-static ShouldLoad(). * Called directly from imageLib when an insecure redirect exists in a cached

1 0

[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