Bump snowflake/go-webrtc for trac 21312

Please see the attached.

Incidentally, this should also fix, https://trac.torproject.org/projects/tor/ticket/24203#comment:11

However, note that this now means there won't be a snowflake.log anymore, unless we provide a path to -log in the torrc-defaults-appendix

added TorBrowserTeam201803R component::applications/tor browser owner::tbb-team priority::medium resolution::fixed severity::normal snowflake status::closed type::defect labels

Trac:
0001-Bump-snowflake-go-webrtc-for-trac-21312.patch

Replying to arlolra:

However, note that this now means there won't be a snowflake.log anymore, unless we provide a path to -log in the torrc-defaults-appendix

I think that's fine or do you think we should make it easier for debugging for now and add that flag?

Given #21304 (moved) is still open, it's probably prudent to leave it off for the time being.

I'm fine with no logging by default. If someone has a problem, hopefully we can ask them to turn on logging.

My removal of default logging in 12922a232b was arguably rash: while obviously eventually we would have to stop logging by default, requiring a -log option with a path prevents writing to a log file that's inside the pt_state directory, which, in sandbox situations, may be the only place the transport plugin is allowed to write. obfs4proxy just has a boolean option -enableLogging that doesn't take a path, unconditionally writing to pt_state/obfs4proxy.log. Perhaps something like that would be better, even if it's less convenient for development.

Perhaps something like that would be better, even if it's less convenient for development.

Maybe just add another flag, -logToStateDir or the like

--- a/client/snowflake.go
+++ b/client/snowflake.go
@@ -10,6 +10,7 @@ import (
        "net"
        "os"
        "os/signal"
+       "path"
        "strings"
        "sync"
        "syscall"
@@ -126,12 +127,22 @@ func main() {
        brokerURL := flag.String("url", "", "URL of signaling broker")
        frontDomain := flag.String("front", "", "front domain")
        logFilename := flag.String("log", "", "name of log file")
+       logToStateDir := flag.Bool("logToStateDir", false, "supersedes log file")
        max := flag.Int("max", DefaultSnowflakeCapacity,
                "capacity for number of multiplexed WebRTC peers")
        flag.Parse()
 
        webrtc.SetLoggingVerbosity(1)
        log.SetFlags(log.LstdFlags | log.LUTC)
+
+       if *logToStateDir {
+               stateDir, err := pt.MakeStateDir()
+               if err != nil {
+                       log.Fatal(err)
+               }
+               *logFilename = path.Join(stateDir, "snowflake.log")
+       }
+
        if *logFilename != "" {
                logFile, err := os.OpenFile(*logFilename,
                        os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0600)

Replying to arlolra:

Perhaps something like that would be better, even if it's less convenient for development.

Maybe just add another flag, -logToStateDir or the like

I opened #25471 (moved) to talk about the logging issue separately.

Per comment:2, I think we're fine to go ahead with disabled logging for now. 0001-Bump-snowflake-go-webrtc-for-trac-21312.patch looks good to me.

People are going to like this change. I'm using it right now and it's light years better than it was.

Trac:
Status: new to merge_ready

Trac:
Keywords: N/A deleted, TorBrowserTeam201803R added
Status: merge_ready to needs_review

Applied to master with commit 42720c512520ecef5437d23e405d8ce011764661, thanks!

Trac:
Resolution: N/A to fixed
Status: needs_review to closed

closed

mentioned in issue #25471 (moved)

mentioned in issue #25579 (moved)

mentioned in issue tpo/anti-censorship/pluggable-transports/snowflake#25471 (closed)

moved to tpo/applications/tor-browser#25449 (closed)

mentioned in issue tpo/applications/tor-browser#25579 (closed)