June 2021 - tor-commits - lists.torproject.org

[tor-browser-build/master] Merge remote-tracking branch 'cohoshgl/issue/40302'
by sysrqb＠torproject.org 24 Jun '21

24 Jun '21

commit 318a0923b373728fcdf3b2db84b0b61e1ccd8176 Merge: 2d69c71 4faacf8 Author: Matthew Finkel <sysrqb(a)torproject.org> Date: Thu Jun 24 16:07:51 2021 +0000 Merge remote-tracking branch 'cohoshgl/issue/40302' projects/snowflake/config | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

1 0

[snowflake/main] Add unit tests to check for webrtc peer data races
by cohosh＠torproject.org 24 Jun '21

24 Jun '21

commit 95cbe36565d65bf1512acd0faeab9a1e2580808f Author: Cecylia Bocovich <cohosh(a)torproject.org> Date: Thu Jun 17 17:43:40 2021 -0400 Add unit tests to check for webrtc peer data races --- client/lib/lib_test.go | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/client/lib/lib_test.go b/client/lib/lib_test.go index 55ea7b9..1eef0c0 100644 --- a/client/lib/lib_test.go +++ b/client/lib/lib_test.go @@ -7,6 +7,7 @@ import ( "net" "net/http" "testing" + "time" "git.torproject.org/pluggable-transports/snowflake.git/common/util" . "github.com/smartystreets/goconvey/convey" @@ -154,6 +155,25 @@ func TestSnowflakeClient(t *testing.T) { So(r, ShouldEqual, wc4) }) + Convey("Terminate Connect() loop", func() { + p, _ := NewPeers(FakeDialer{max: 4}) + go func() { + for { + p.Collect() + select { + case <-p.Melted(): + return + default: + } + } + }() + <-time.After(10 * time.Second) + + p.End() + <-p.Melted() + So(p.Count(), ShouldEqual, 0) + }) + }) Convey("Dialers", t, func() { @@ -245,6 +265,17 @@ func TestSnowflakeClient(t *testing.T) { } +func TestWebRTCPeer(t *testing.T) { + Convey("WebRTCPeer", t, func(c C) { + p := &WebRTCPeer{closed: make(chan struct{})} + Convey("checks for staleness", func() { + go p.checkForStaleness() + <-time.After(2 * SnowflakeTimeout) + So(p.Closed(), ShouldEqual, true) + }) + }) +} + func TestICEServerParser(t *testing.T) { Convey("Test parsing of ICE servers", t, func() { for _, test := range []struct {

1 0

[snowflake/main] Fix datarace for WebRTCPeer.closed
by cohosh＠torproject.org 24 Jun '21

24 Jun '21

commit ddcdfc4f0922e00c672797b0d6544371423f2989 Author: Cecylia Bocovich <cohosh(a)torproject.org> Date: Thu Jun 17 16:36:50 2021 -0400 Fix datarace for WebRTCPeer.closed The race condition occurs because concurrent goroutines are intermixing reads and writes of `WebRTCPeer.closed`. Spotted when integrating Snowflake inside OONI in https://github.com/ooni/probe-cli/pull/373. --- client/lib/lib_test.go | 6 +++--- client/lib/peers.go | 4 ++-- client/lib/webrtc.go | 24 ++++++++++++++++++------ 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/client/lib/lib_test.go b/client/lib/lib_test.go index e742e06..55ea7b9 100644 --- a/client/lib/lib_test.go +++ b/client/lib/lib_test.go @@ -33,7 +33,7 @@ type FakeDialer struct { func (w FakeDialer) Catch() (*WebRTCPeer, error) { fmt.Println("Caught a dummy snowflake.") - return &WebRTCPeer{}, nil + return &WebRTCPeer{closed: make(chan struct{})}, nil } func (w FakeDialer) GetMax() int { @@ -97,7 +97,7 @@ func TestSnowflakeClient(t *testing.T) { So(err, ShouldNotBeNil) So(p.Count(), ShouldEqual, c) - // But popping and closing allows it to continue. + // But popping allows it to continue. s := p.Pop() s.Close() So(s, ShouldNotBeNil) @@ -127,7 +127,7 @@ func TestSnowflakeClient(t *testing.T) { cnt := 5 p, _ := NewPeers(FakeDialer{max: cnt}) for i := 0; i < cnt; i++ { - p.activePeers.PushBack(&WebRTCPeer{}) + p.activePeers.PushBack(&WebRTCPeer{closed: make(chan struct{})}) } So(p.Count(), ShouldEqual, cnt) p.End() diff --git a/client/lib/peers.go b/client/lib/peers.go index d02eed3..6fa2d29 100644 --- a/client/lib/peers.go +++ b/client/lib/peers.go @@ -83,7 +83,7 @@ func (p *Peers) Pop() *WebRTCPeer { if !ok { return nil } - if snowflake.closed { + if snowflake.Closed() { continue } // Set to use the same rate-limited traffic logger to keep consistency. @@ -110,7 +110,7 @@ func (p *Peers) purgeClosedPeers() { next := e.Next() conn := e.Value.(*WebRTCPeer) // Purge those marked for deletion. - if conn.closed { + if conn.Closed() { p.activePeers.Remove(e) } e = next diff --git a/client/lib/webrtc.go b/client/lib/webrtc.go index 6a42ebd..234f53c 100644 --- a/client/lib/webrtc.go +++ b/client/lib/webrtc.go @@ -28,7 +28,7 @@ type WebRTCPeer struct { lastReceive time.Time open chan struct{} // Channel to notify when datachannel opens - closed bool + closed chan struct{} once sync.Once // Synchronization for PeerConnection destruction @@ -46,6 +46,7 @@ func NewWebRTCPeer(config *webrtc.Configuration, } connection.id = "snowflake-" + hex.EncodeToString(buf[:]) } + connection.closed = make(chan struct{}) // Override with something that's not NullLogger to have real logging. connection.BytesLogger = &BytesNullLogger{} @@ -78,9 +79,19 @@ func (c *WebRTCPeer) Write(b []byte) (int, error) { return len(b), nil } +//Returns a boolean indicated whether the peer is closed +func (c *WebRTCPeer) Closed() bool { + select { + case <-c.closed: + return true + default: + } + return false +} + func (c *WebRTCPeer) Close() error { c.once.Do(func() { - c.closed = true + close(c.closed) c.cleanup() log.Printf("WebRTC: Closing") }) @@ -95,9 +106,6 @@ func (c *WebRTCPeer) checkForStaleness() { c.lastReceive = time.Now() c.mu.Unlock() for { - if c.closed { - return - } c.mu.Lock() lastReceive := c.lastReceive c.mu.Unlock() @@ -107,7 +115,11 @@ func (c *WebRTCPeer) checkForStaleness() { c.Close() return } - <-time.After(time.Second) + select { + case <-c.closed: + return + case <-time.After(time.Second): + } } }

1 0

[snowflake/main] Fix datarace for Peers.melted
by cohosh＠torproject.org 24 Jun '21

24 Jun '21

commit bb7ff6180bf2cb9553db8cbb4600092107cf9303 Author: Cecylia Bocovich <cohosh(a)torproject.org> Date: Thu Jun 17 17:42:22 2021 -0400 Fix datarace for Peers.melted Using the boolean value was unnecessary since we already have a channel we can check for closure. --- client/lib/peers.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/client/lib/peers.go b/client/lib/peers.go index 6fa2d29..66373f1 100644 --- a/client/lib/peers.go +++ b/client/lib/peers.go @@ -26,8 +26,7 @@ type Peers struct { snowflakeChan chan *WebRTCPeer activePeers *list.List - melt chan struct{} - melted bool + melt chan struct{} collection sync.WaitGroup } @@ -51,8 +50,10 @@ func (p *Peers) Collect() (*WebRTCPeer, error) { // Engage the Snowflake Catching interface, which must be available. p.collection.Add(1) defer p.collection.Done() - if p.melted { + select { + case <-p.melt: return nil, fmt.Errorf("Snowflakes have melted") + default: } if nil == p.Tongue { return nil, errors.New("missing Tongue to catch Snowflakes with") @@ -120,7 +121,6 @@ func (p *Peers) purgeClosedPeers() { // Close all Peers contained here. func (p *Peers) End() { close(p.melt) - p.melted = true p.collection.Wait() close(p.snowflakeChan) cnt := p.Count()

1 0

[snowflake/main] Fix data race for Peers.collection
by cohosh＠torproject.org 24 Jun '21

24 Jun '21

commit e3351cb08abe6b806fa018bc8915f0f5ae045a40 Author: Cecylia Bocovich <cohosh(a)torproject.org> Date: Mon Jun 21 15:39:41 2021 -0400 Fix data race for Peers.collection We used a WaitGroup to prevent a call to Peers.End from melting snowflakes while a new one is being collected. However, calls to WaitGroup.Add are in a race with WaitGroup.Wait. To fix this, we use a Mutex instead. --- client/lib/peers.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/client/lib/peers.go b/client/lib/peers.go index 66373f1..7fba572 100644 --- a/client/lib/peers.go +++ b/client/lib/peers.go @@ -28,7 +28,7 @@ type Peers struct { melt chan struct{} - collection sync.WaitGroup + collectLock sync.Mutex } // Construct a fresh container of remote peers. @@ -48,8 +48,8 @@ func NewPeers(tongue Tongue) (*Peers, error) { // As part of |SnowflakeCollector| interface. func (p *Peers) Collect() (*WebRTCPeer, error) { // Engage the Snowflake Catching interface, which must be available. - p.collection.Add(1) - defer p.collection.Done() + p.collectLock.Lock() + defer p.collectLock.Unlock() select { case <-p.melt: return nil, fmt.Errorf("Snowflakes have melted") @@ -121,7 +121,8 @@ func (p *Peers) purgeClosedPeers() { // Close all Peers contained here. func (p *Peers) End() { close(p.melt) - p.collection.Wait() + p.collectLock.Lock() + defer p.collectLock.Unlock() close(p.snowflakeChan) cnt := p.Count() for e := p.activePeers.Front(); e != nil; {

1 0

[snowflake/main] Refactor checkForStaleness to take time.Duration
by cohosh＠torproject.org 24 Jun '21

24 Jun '21

commit 10b6075eaa90d65ebb4838b24ca8db4924e572ec Author: Cecylia Bocovich <cohosh(a)torproject.org> Date: Thu Jun 24 11:20:44 2021 -0400 Refactor checkForStaleness to take time.Duration --- client/lib/lib_test.go | 4 ++-- client/lib/webrtc.go | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/client/lib/lib_test.go b/client/lib/lib_test.go index 1eef0c0..e0856a5 100644 --- a/client/lib/lib_test.go +++ b/client/lib/lib_test.go @@ -269,8 +269,8 @@ func TestWebRTCPeer(t *testing.T) { Convey("WebRTCPeer", t, func(c C) { p := &WebRTCPeer{closed: make(chan struct{})} Convey("checks for staleness", func() { - go p.checkForStaleness() - <-time.After(2 * SnowflakeTimeout) + go p.checkForStaleness(time.Second) + <-time.After(2 * time.Second) So(p.Closed(), ShouldEqual, true) }) }) diff --git a/client/lib/webrtc.go b/client/lib/webrtc.go index 234f53c..72a3d64 100644 --- a/client/lib/webrtc.go +++ b/client/lib/webrtc.go @@ -101,7 +101,7 @@ func (c *WebRTCPeer) Close() error { // Prevent long-lived broken remotes. // Should also update the DataChannel in underlying go-webrtc's to make Closes // more immediate / responsive. -func (c *WebRTCPeer) checkForStaleness() { +func (c *WebRTCPeer) checkForStaleness(timeout time.Duration) { c.mu.Lock() c.lastReceive = time.Now() c.mu.Unlock() @@ -109,9 +109,9 @@ func (c *WebRTCPeer) checkForStaleness() { c.mu.Lock() lastReceive := c.lastReceive c.mu.Unlock() - if time.Since(lastReceive) > SnowflakeTimeout { + if time.Since(lastReceive) > timeout { log.Printf("WebRTC: No messages received for %v -- closing stale connection.", - SnowflakeTimeout) + timeout) c.Close() return } @@ -147,7 +147,7 @@ func (c *WebRTCPeer) connect(config *webrtc.Configuration, broker *BrokerChannel return errors.New("timeout waiting for DataChannel.OnOpen") } - go c.checkForStaleness() + go c.checkForStaleness(SnowflakeTimeout) return nil }

1 0

[snowflake/main] Fix datarace for WebRTCPeer.lastReceive
by cohosh＠torproject.org 24 Jun '21

24 Jun '21

commit ed2d5df87d0be7c86f21d19d42ec90b8d2616ae2 Author: Simone Basso <bassosimone(a)gmail.com> Date: Mon Jun 14 10:46:46 2021 +0200 Fix datarace for WebRTCPeer.lastReceive The race condition occurs because concurrent goroutines are intermixing reads and writes of `WebRTCPeer.lastReceive`. Spotted when integrating Snowflake inside OONI in https://github.com/ooni/probe-cli/pull/373. --- client/lib/webrtc.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/client/lib/webrtc.go b/client/lib/webrtc.go index af7ba6d..6a42ebd 100644 --- a/client/lib/webrtc.go +++ b/client/lib/webrtc.go @@ -21,8 +21,10 @@ type WebRTCPeer struct { pc *webrtc.PeerConnection transport *webrtc.DataChannel - recvPipe *io.PipeReader - writePipe *io.PipeWriter + recvPipe *io.PipeReader + writePipe *io.PipeWriter + + mu sync.Mutex // protects the following: lastReceive time.Time open chan struct{} // Channel to notify when datachannel opens @@ -89,12 +91,17 @@ func (c *WebRTCPeer) Close() error { // Should also update the DataChannel in underlying go-webrtc's to make Closes // more immediate / responsive. func (c *WebRTCPeer) checkForStaleness() { + c.mu.Lock() c.lastReceive = time.Now() + c.mu.Unlock() for { if c.closed { return } - if time.Since(c.lastReceive) > SnowflakeTimeout { + c.mu.Lock() + lastReceive := c.lastReceive + c.mu.Unlock() + if time.Since(lastReceive) > SnowflakeTimeout { log.Printf("WebRTC: No messages received for %v -- closing stale connection.", SnowflakeTimeout) c.Close() @@ -173,7 +180,9 @@ func (c *WebRTCPeer) preparePeerConnection(config *webrtc.Configuration) error { log.Printf("c.writePipe.CloseWithError returned error: %v", inerr) } } + c.mu.Lock() c.lastReceive = time.Now() + c.mu.Unlock() }) c.transport = dc c.open = make(chan struct{})

1 0

[tor-browser/tor-browser-78.11.0esr-10.5-1] fixup! Bug 27476: Implement about:torconnect captive portal within Tor Browser
by sysrqb＠torproject.org 24 Jun '21

24 Jun '21

commit ccef7b35e20bd47dec7717bac2524b86878b2bd3 Author: Richard Pospesel <richard(a)torproject.org> Date: Thu Jun 24 15:22:36 2021 +0200 fixup! Bug 27476: Implement about:torconnect captive portal within Tor Browser --- .../torconnect/content/aboutTorConnect.css | 24 ++++++++++++++++++++++ .../torconnect/content/aboutTorConnect.js | 1 + browser/themes/shared/onionPattern.inc.xhtml | 7 +++++-- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/browser/components/torconnect/content/aboutTorConnect.css b/browser/components/torconnect/content/aboutTorConnect.css index eff767205266..a70904ca0b28 100644 --- a/browser/components/torconnect/content/aboutTorConnect.css +++ b/browser/components/torconnect/content/aboutTorConnect.css @@ -10,6 +10,30 @@ --onion-radius: 50px; } +/* override firefox's default blue focus coloring */ +:focus { + outline: none!important; + box-shadow: 0 0 0 3px #CD8CED !important; + border: 1px #59316B solid !important; +} + +@media (prefers-color-scheme: dark) +{ + :focus { + box-shadow: 0 0 0 3px #8C689E !important; + } +} + +/* override firefox's default blue border on hover */ +input[type="checkbox"]:not(:disabled):hover { + border-color: #59316B; +} + +/* fix checkbox visibility when dark mode enabled */ +input[type="checkbox"]:checked { + fill: var(--in-content-page-color); +} + #connectButton { background-color: #7D4698; } diff --git a/browser/components/torconnect/content/aboutTorConnect.js b/browser/components/torconnect/content/aboutTorConnect.js index a88a887c7878..5fb5b2924b94 100644 --- a/browser/components/torconnect/content/aboutTorConnect.js +++ b/browser/components/torconnect/content/aboutTorConnect.js @@ -278,6 +278,7 @@ class AboutTorConnect { this.elemConnectButton.addEventListener("click", () => { this.connect(); }); + this.elemConnectButton.focus(); this.elemCancelButton.textContent = this.torStrings.torConnect.cancel; this.elemCancelButton.addEventListener("click", () => { diff --git a/browser/themes/shared/onionPattern.inc.xhtml b/browser/themes/shared/onionPattern.inc.xhtml index 95f073e673b4..6bbde93684a2 100644 --- a/browser/themes/shared/onionPattern.inc.xhtml +++ b/browser/themes/shared/onionPattern.inc.xhtml @@ -12,8 +12,11 @@ --> <div class="onion-pattern-container"> - <div class="onion-pattern-crop"> - <div class="onion-pattern-column"> +  + <div class="onion-pattern-crop" tabindex="-1"> + <div class="onion-pattern-column" tabindex="-1"> <div class="onion-pattern-row"> <div class="circle solid"></div>

1 0

[tor-browser/tor-browser-78.11.0esr-10.5-1] Merge remote-tracking branch 'richardgl/40480' into tor-browser-78.11.0esr-10.5-1
by sysrqb＠torproject.org 24 Jun '21

24 Jun '21

commit f6578c3e0085fbb401e7fa3d3e5e323b80962c01 Merge: 9cb771f3026c ccef7b35e20b Author: Matthew Finkel <sysrqb(a)torproject.org> Date: Thu Jun 24 14:06:01 2021 +0000 Merge remote-tracking branch 'richardgl/40480' into tor-browser-78.11.0esr-10.5-1 .../torconnect/content/aboutTorConnect.css | 24 ++++++++++++++++++++++ .../torconnect/content/aboutTorConnect.js | 1 + browser/themes/shared/onionPattern.inc.xhtml | 7 +++++-- 3 files changed, 30 insertions(+), 2 deletions(-)

1 0

[tor-browser/tor-browser-78.11.0esr-10.5-1] squash! Bug 28005: Implement .onion alias urlbar rewrites
by sysrqb＠torproject.org 24 Jun '21

24 Jun '21

commit 9cb771f3026c5303e2700e392826a9e028e9b2d5 Author: Matthew Finkel <sysrqb(a)torproject.org> Date: Mon Jun 21 20:17:21 2021 +0000 squash! Bug 28005: Implement .onion alias urlbar rewrites Bug 40456: Update the SecureDrop HTTPS-Everywhere update channel Bug 40478: Onion alias url rewrite is broken --- .../onionservices/HttpsEverywhereControl.jsm | 32 ++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/browser/components/onionservices/HttpsEverywhereControl.jsm b/browser/components/onionservices/HttpsEverywhereControl.jsm index c91db3e8de87..d673de4cd6e5 100644 --- a/browser/components/onionservices/HttpsEverywhereControl.jsm +++ b/browser/components/onionservices/HttpsEverywhereControl.jsm @@ -10,7 +10,7 @@ const { ExtensionMessaging } = ChromeUtils.import( const { setTimeout } = ChromeUtils.import("resource://gre/modules/Timer.jsm"); const EXTENSION_ID = "https-everywhere-eff(a)eff.org"; -const SECUREDROP_TOR_ONION_CHANNEL = { +const SECUREDROP_TOR_ONION_CHANNEL_2020 = { name: "SecureDropTorOnion", jwk: { kty: "RSA", @@ -24,6 +24,20 @@ const SECUREDROP_TOR_ONION_CHANNEL = { replaces_default_rulesets: false, }; +const SECUREDROP_TOR_ONION_CHANNEL = { + name: "SecureDropTorOnion2021", + jwk: { + kty: "RSA", + e: "AQAB", + n: + "vsC7BNafkRe8Uh1DUgCkv6RbPQMdJgAKKnWdSqQd7tQzU1mXfmo_k1Py_2MYMZXOWmqSZ9iwIYkykZYywJ2VyMGve4byj1sLn6YQoOkG8g5Z3V4y0S2RpEfmYumNjTzfq8nxtLnwjaYd4sCUd5wa0SzeLrpRQuXo2bF3QuUF2xcbLJloxX1MmlsMMCdBc-qGNonLJ7bpn_JuyXlDWy1Fkeyw1qgjiOdiRIbMC1x302zgzX6dSrBrNB8Cpsh-vCE0ZjUo8M9caEv06F6QbYmdGJHM0ZZY34OHMSNdf-_qUKIV_SuxuSuFE99tkAeWnbWpyI1V-xhVo1sc7NzChP8ci2TdPvI3_0JyAuCvL6zIFqJUJkZibEUghhg6F09-oNJKpy7rhUJq7zZyLXJsvuXnn0gnIxfjRvMcDfZAKUVMZKRdw7fwWzwQril4Ib0MQOVda9vb_4JMk7Gup-TUI4sfuS4NKwsnKoODIO-2U5QpJWdtp1F4AQ1pBv8ajFl1WTrVGvkRGK0woPWaO6pWyJ4kRnhnxrV2FyNNt3JSR-0JEjhFWws47kjBvpr0VRiVRFppKA-plKs4LPlaaCff39TleYmY3mETe3w1GIGc2Lliad32Jpbx496IgDe1K3FMBEoKFZfhmtlRSXft8NKgSzPt2zkatM9bFKfaCYRaSy7akbk", + }, + update_path_prefix: "https://securedrop.org/https-everywhere-2021/", + scope: + "^https?:\\/\\/[a-z0-9-]+(?:\\.[a-z0-9-]+)*\\.securedrop\\.tor\\.onion\\/", + replaces_default_rulesets: false, +}; + class HttpsEverywhereControl { constructor() { this._extensionMessaging = null; @@ -56,6 +70,20 @@ class HttpsEverywhereControl { // for that here. await HttpsEverywhereControl.wait(); + try { + // Delete the previous channel signing key, and add the new one below. + await this._sendMessage( + "delete_update_channel", + SECUREDROP_TOR_ONION_CHANNEL_2020.name + ); + } catch (e) { + if (retries <= 0) { + throw new Error("Could not uninstall SecureDropTorOnion update channel"); + } + await this.installTorOnionUpdateChannel(retries - 1); + return; + } + try { // TODO: we may want a way to "lock" this update channel, so that it cannot be modified // by the user via UI, but I think this is not possible at the time of writing via @@ -89,7 +117,7 @@ class HttpsEverywhereControl { * Returns the timestamp of the last .tor.onion update channel update. */ async getRulesetTimestamp() { - const rulesets = await this._sendMessage("get_ruleset_timestamps"); + const rulesets = await this._sendMessage("get_update_channel_timestamps"); const securedrop = rulesets && rulesets.find(([{ name }]) => name === SECUREDROP_TOR_ONION_CHANNEL.name);

1 0