Re: [tor-bugs] #21304 [Obfuscation/Snowflake]: Sanitize snowflake.log

[Tor Bug Tracker & Wiki]

#21304: Sanitize snowflake.log
---+
 Reporter:  arlolra|  Owner:  cohosh
 Type:  defect | Status:  closed
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:  fixed
 Keywords:  starter|  Actual Points:
Parent ID: | Points:  1
 Reviewer: |Sponsor:
---+

Comment (by cohosh):

 Done (and appears to be working from the client side).

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21304 [Obfuscation/Snowflake]: Sanitize snowflake.log

[Tor Bug Tracker & Wiki]

#21304: Sanitize snowflake.log
---+
 Reporter:  arlolra|  Owner:  cohosh
 Type:  defect | Status:  closed
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:  fixed
 Keywords:  starter|  Actual Points:
Parent ID: | Points:  1
 Reviewer: |Sponsor:
---+

Comment (by dcf):

 I think the remaining task is to deploy the new code on the bridge.

 To upgrade the bridge:
 1. `service tor stop`
 2. `install --owner root ~/new-server /usr/local/bin/snowflake-server`
 3. `setcap 'cap_net_bind_service=+ep' /usr/local/bin/snowflake-server`
 4. `service tor start`

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21304 [Obfuscation/Snowflake]: Sanitize snowflake.log

[Tor Bug Tracker & Wiki]

#21304: Sanitize snowflake.log
---+
 Reporter:  arlolra|  Owner:  cohosh
 Type:  defect | Status:  closed
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:  fixed
 Keywords:  starter|  Actual Points:
Parent ID: | Points:  1
 Reviewer: |Sponsor:
---+
Changes (by cohosh):

 * status:  merge_ready => closed
 * resolution:   => fixed


Comment:

 Merged to master.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21304 [Obfuscation/Snowflake]: Sanitize snowflake.log

[Tor Bug Tracker & Wiki]

#21304: Sanitize snowflake.log
---+-
 Reporter:  arlolra|  Owner:  cohosh
 Type:  defect | Status:  merge_ready
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords:  starter|  Actual Points:
Parent ID: | Points:  1
 Reviewer: |Sponsor:
---+-

Comment (by dcf):

 Replying to [comment:17 cohosh]:
 > Replying to [comment:16 dcf]:
 > > Apart from the specific client and broker changes you made
 
[https://github.com/cohosh/snowflake/commit/3eb9064438ca6242f935173aed88ec29a0c16c7d
 here], do the client and broker benefit from using the same log scrubber?
 I.e., is it worth it to factor out the log scrubber into its own package
 and share it, after merging it to server?
 > >
 > I would say yes, as a precaution, just because the broker at least also
 uses the net.http package for connections that involve the client IP
 address. It would be a very small library... is that a good way to do it?

 I think the way to do it is create a new subdirectory containing the code,
 put `package safelog` (or whatever) at the top of the file, and then
 import it where needed using `import git.torproject.org/pluggable-
 transports/snowflake.git/safelog` (or whatever).

 Compare to how obfs4proxy factors out some of its common code:
 
https://gitlab.com/yawning/obfs4/blob/dba633c7dc0d7d919e8be1b61016cab7cfa8e60e/obfs4proxy/obfs4proxy.go#L45-48

 But in any case, the server code is fine to merge as-is now, IMO.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21304 [Obfuscation/Snowflake]: Sanitize snowflake.log

[Tor Bug Tracker & Wiki]

#21304: Sanitize snowflake.log
---+-
 Reporter:  arlolra|  Owner:  cohosh
 Type:  defect | Status:  merge_ready
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords:  starter|  Actual Points:
Parent ID: | Points:  1
 Reviewer: |Sponsor:
---+-

Comment (by cohosh):

 Replying to [comment:16 dcf]:
 > Apart from the specific client and broker changes you made
 
[https://github.com/cohosh/snowflake/commit/3eb9064438ca6242f935173aed88ec29a0c16c7d
 here], do the client and broker benefit from using the same log scrubber?
 I.e., is it worth it to factor out the log scrubber into its own package
 and share it, after merging it to server?
 >
 I would say yes, as a precaution, just because the broker at least also
 uses the net.http package for connections that involve the client IP
 address. It would be a very small library... is that a good way to do it?
 >
 > > I removed this from the client logs, but if there's now unused
 functionality in the client or proxy code perhaps we should remove that
 too?
 >
 > Yes; personally I think we can remove all the fifo copy-paste code.
 Let's do that in another ticket though.
 Created ticket #30008

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21304 [Obfuscation/Snowflake]: Sanitize snowflake.log

[Tor Bug Tracker & Wiki]

#21304: Sanitize snowflake.log
---+-
 Reporter:  arlolra|  Owner:  cohosh
 Type:  defect | Status:  merge_ready
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords:  starter|  Actual Points:
Parent ID: | Points:  1
 Reviewer: |Sponsor:
---+-
Changes (by dcf):

 * status:  needs_review => merge_ready


Old description:

> For starters, the timestamps are in the local timezone.  We can make that
> UTC
>
> See also #19026
>
> Known problems:
>  * When the websocket server panics (as in #29125), it writes the client
> IP address to the log:
>{{{
> 2019/01/18 18:56:29 http2: panic serving X.X.X.X:: interface
> conversion: *http2.responseWriter is not http.Hijacker: missing method
> Hijack
>}}}
>  * The broker logs the raw bytes of ICE answers:
>{{{
> 2019/03/22 06:28:32 Received answer:  [XXX XXX XXX XXX ... XXX]
>}}}

New description:

 For starters, the timestamps are in the local timezone.  We can make that
 UTC

 See also #19026

 Known problems:
  * The client logs full SDP stanzas, which include local IP addresses.
  * When the websocket server panics (as in #29125), it writes the client
 IP address to the log:
{{{
 2019/01/18 18:56:29 http2: panic serving X.X.X.X:: interface
 conversion: *http2.responseWriter is not http.Hijacker: missing method
 Hijack
}}}
  * The websocket server logs an IP address on certain connection errors.
 (These are probably automated scanners, but still...)
{{{
 2019/04/01 07:02:50 http: TLS handshake error from X.X.X.X::
 acme/autocert: missing server name
 2019/04/01 07:53:03 http: TLS handshake error from X.X.X.X:: tls:
 oversized record received with length 21536
}}}
  * The broker logs the raw bytes of ICE answers:
{{{
 2019/03/22 06:28:32 Received answer:  [XXX XXX XXX XXX ... XXX]
}}}

--

Comment:

 This looks good to me.

 I think this is ready to merge. It looks like we are not keeping permanent
 logs for snowflake-server; they are getting rotated in /var/log/tor. So we
 don't have to do anything special to purge the old logs.

 Apart from the specific client and broker changes you made
 
[https://github.com/cohosh/snowflake/commit/3eb9064438ca6242f935173aed88ec29a0c16c7d
 here], do the client and broker benefit from using the same log scrubber?
 I.e., is it worth it to factor out the log scrubber into its own package
 and share it, after merging it to server?


 Replying to [comment:15 cohosh]:
 > I think for now it's okay to just have the log scrubber. Before I
 started it, I went through and looked at all of the returned errors and I
 don't recall finding any net.OpErrors anyway.

 Agreed, I think the string-based scrubbing is the way to go.

 > {{{
 > const ipv4Address = `\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}`
 > const ipv6Address = `(([0-9a-fA-F]{0,4}:){2,7}([0-9a-fA-F]{0,4})?(` +
 ipv4Address + `))` +
 > `|(([0-9a-fA-F]{0,4}:){2,7}([0-9a-fA-F]{0,4})?)`
 > const optionalPort = `(:\d{1,5})?`
 > const addressPattern = `((` + ipv4Address + `)|(\[(` + ipv6Address +
 `)\])|(` + ipv6Address + `))` + optionalPort
 > }}}
 >
 > My reason for not going with the full one was that I found it
 unnecessary after expanding my tests with the ones you used. I also found
 that this massive regexp was still scrubbing the fingerprint, meaning
 additional changes still had to be made.

 This looks good to me.

 > Go does not yet support look-ahead or look-behind, which is what we'd
 want to pass this test case. Instead I implemented a work-around that will
 find regular expressions that check for characters surrounding the
 address, but only replaces the address portion of it:

 This is a good way to do it. I'm also fine if fingerprints get scrubbed, I
 don't think that necessarily has to be a test case. On the comment "go
 does ''not yet'' support look ahead or look behind," I think as a
 philosophical matter they plan never ever to implement those.

 > > That's what the "SEND" button at
 https://snowflake.torproject.org/snowflake.html, and the
 {{{NewCopyPasteDialer}}} in client, are for. I'm okay with that mode of
 operation going away.
 > I removed this from the client logs, but if there's now unused
 functionality in the client or proxy code perhaps we should remove that
 too?

 Yes; personally I think we can remove all the fifo copy-paste code. Let's
 do that in another ticket though.

 > Also please let me know if I should be doing something different for
 attributing the code you wrote for this.

 Nothing needed in this regard.

--
Ticket URL: 
Tor Bug Tracker & Wiki 

Re: [tor-bugs] #21304 [Obfuscation/Snowflake]: Sanitize snowflake.log

[Tor Bug Tracker & Wiki]

#21304: Sanitize snowflake.log
---+--
 Reporter:  arlolra|  Owner:  cohosh
 Type:  defect | Status:  needs_review
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords:  starter|  Actual Points:
Parent ID: | Points:  1
 Reviewer: |Sponsor:
---+--
Changes (by cohosh):

 * status:  needs_revision => needs_review


Comment:

 Here's a new candidate:
 https://github.com/cohosh/snowflake/compare/safe_log

 Replying to [comment:12 dcf]:
 Thanks for the helpful feedback! I've addressed each of the comments
 below:

 > Let me suggest a further simplification: ''only'' worry about the server
 log for now, and then we can perhaps factor the safe-logging code into a
 separate package for use by the other programs.

 This is what I've done so far, the log scrubber is only being used on the
 server log at the moment.

 >
 > * I think it's best to default to scrubbed in all cases.
 Agreed. Used the suggested logic to avoid missing default loggers in the
 future:
 
[https://github.com/cohosh/snowflake/blob/3eb9064438ca6242f935173aed88ec29a0c16c7d/server/server.go#L329
 server.go#L329]

 > * the default log output in the absence of a `--log` option should be
 >   `os.Stderr`, not `os.Stdout`.
 >   [https://golang.org/pkg/log/#pkg-overview Ref]: "That logger writes to
 standard error..."
 >   There's now a [https://golang.org/pkg/log/#Logger.Writer
 Logger.Writer] method that would allow
 >   us to ask for the default output, but as it was only
 [https://github.com/golang/go/pull/28399 added in 1.12],
 >   it's probably still too new to use.
 Oops, fixed. I agree that the Logger.Writer method is the best thing to
 use and it's probably a good idea to wait until we want to update the go
 version for other reasons as well.

 > * The `logScrubber.Write` method only looks at one buffer's worth of
 data at a time,
 >   See below re `RedactErrorWriter` for an idea on how to deal with this.
 Cool, I really like how it works. I implemented that for the log scrubber
 here:
 
[https://github.com/cohosh/snowflake/blob/3eb9064438ca6242f935173aed88ec29a0c16c7d/server/server.go#L91
 server.go#L91] and added your tests:
 
[https://github.com/cohosh/snowflake/blob/3eb9064438ca6242f935173aed88ec29a0c16c7d/server/server_test.go#L54
 server_test.go#L54]

 > * You can use `!bytes.Equal(a, b)` instead of `bytes.Compare(a, b) !=
 0`.
 Done.

 > * Should we scrub port numbers, too?
 Yeah, that's a good idea. I've updated the regular expressions to catch
 the port. See my comments below for what I settled on.

 > * Consider structuring repetitive tests like this:
 Done.
 > * Instead of `X.X.X.X`,
 
[https://gitweb.torproject.org/tor.git/tree/src/app/config/config.c?h=tor-0.3.5.8#n1102
 tor uses] `[scrubbed]`, as do
 >   [https://gitweb.torproject.org/pluggable-transports/meek.git/tree
 /meek-server/meek-server.go?h=0.31#n181 meek-server]
 >   and
 [https://gitlab.com/yawning/obfs4/blob/obfs4proxy-0.0.9/common/log/log.go#L42
 obfs4proxy].
 Nice, I like this better. It also leaks less information this way. Done.
 >
 > 

 > Is it worth doing both, structure-based object manipulation and string-
 based patterns? Perhaps not, since the correct functioning of one can mask
 errors in the other. You could think of it as defense in depth, but it
 will be hard to consistently use a `redactError` function everywhere, when
 forgetting it doesn't have any observable consequences. Surely having both
 is safer, but the marginal safety increase of `redactError` may not be
 worth the (albeit small) code complexity increase.

 I think for now it's okay to just have the log scrubber. Before I started
 it, I went through and looked at all of the returned errors and I don't
 recall finding any net.OpErrors anyway.

 > One other difference is this massive, yet conservative,
 [https://gitweb.torproject.org/user/dcf/snowflake.git/tree/server/error.go?h
 =redact-error#n58 IP address–detecting regexp]. I admit its appearance is
 comical, but it will detect IPv6 addresses not surrounded by brackets, and
 avoid false-positive matches like what you mentioned in comment:11. Also,
 it scrubs port numbers. I would say, test
 
[https://github.com/cohosh/snowflake/blob/4b0acda984a5ae4335d206fc534f51efb9303d5d/server/server_test.go#L53
 your] and
 
[https://gitweb.torproject.org/user/dcf/snowflake.git/tree/server/error_test.go?h
 =redact-error#n148 my] test cases against the pattern you already have;
 and if there are any misdetections, a more precise regexp may be in order.

 I stumbled across that as well when I wrote the original regexp >.< I did
 end up taking some elements from it and expanding the regexp that 

Re: [tor-bugs] #21304 [Obfuscation/Snowflake]: Sanitize snowflake.log

[Tor Bug Tracker & Wiki]

#21304: Sanitize snowflake.log
---+
 Reporter:  arlolra|  Owner:  cohosh
 Type:  defect | Status:  needs_revision
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords:  starter|  Actual Points:
Parent ID: | Points:  1
 Reviewer: |Sponsor:
---+
Description changed by dcf:

Old description:

> For starters, the timestamps are in the local timezone.  We can make that
> UTC
>
> See also #19026
>
> Known problems:
>  * When the websocket server panics (as in #29125), it writes the client
> IP address to the log:
>{{{
> 2019/01/18 18:56:29 http2: panic serving X.X.X.X:: interface
> conversion: *http2.responseWriter is not http.Hijacker: missing method
> Hijack
>}}}

New description:

 For starters, the timestamps are in the local timezone.  We can make that
 UTC

 See also #19026

 Known problems:
  * When the websocket server panics (as in #29125), it writes the client
 IP address to the log:
{{{
 2019/01/18 18:56:29 http2: panic serving X.X.X.X:: interface
 conversion: *http2.responseWriter is not http.Hijacker: missing method
 Hijack
}}}
  * The broker logs the raw bytes of ICE answers:
{{{
 2019/03/22 06:28:32 Received answer:  [XXX XXX XXX XXX ... XXX]
}}}

--

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21304 [Obfuscation/Snowflake]: Sanitize snowflake.log

[Tor Bug Tracker & Wiki]

#21304: Sanitize snowflake.log
---+
 Reporter:  arlolra|  Owner:  cohosh
 Type:  defect | Status:  needs_revision
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords:  starter|  Actual Points:
Parent ID: | Points:  1
 Reviewer: |Sponsor:
---+

Comment (by dcf):

 Replying to [comment:11 cohosh]:
 > What are our feelings about scrubbing fingerprints from logs as well? Is
 it necessary? The one in particular I'm looking at is the received answer
 in the client log:
 > {{{
 > a=ice-options:trickle
 > a=fingerprint:sha-256
 
8D:CE:FE:08:F1:AC:32:30:88:D1:B4:1A:34:84:19:C2:43:18:4A:57:A9:20:2F:DC:C2:32:01:38:F9:8B:E5:8C
 > a=setup:active
 > a=mid:data
 > }}}
 >
 > If we don't need the fingerprint, I'd prefer to "over scrub" instead of
 potentially leaking info if the IPv6 logging violates the above
 assumption.

 (Sidestepping the question slightly.) In the case of these full SDP
 stanzas, I think we should just not be logging them at all, not by default
 anyway.

 The historical reason why the client logs these, I believe, has to do with
 early development of the system, when we would do ICE signaling manually
 (i.e., copy-and-paste the information from the terminal and into the
 browser, and vice versa). That's what the "SEND" button at
 https://snowflake.torproject.org/snowflake.html, and the
 `NewCopyPasteDialer` in client, are for. I'm okay with that mode of
 operation going away.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21304 [Obfuscation/Snowflake]: Sanitize snowflake.log

[Tor Bug Tracker & Wiki]

#21304: Sanitize snowflake.log
---+
 Reporter:  arlolra|  Owner:  cohosh
 Type:  defect | Status:  needs_revision
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords:  starter|  Actual Points:
Parent ID: | Points:  1
 Reviewer: |Sponsor:
---+
Changes (by dcf):

 * status:  needs_review => needs_revision
 * cc: dcf (added)


Comment:

 Thanks, this is looking pretty good.

 We'll need the same functionality across multiple programs: broker,
 client, proxy-go, server. I think you've what you've done so far—putting
 the tests in server and copying the code where needed—is good for
 prototyping. Let me suggest a further simplification: ''only'' worry about
 the server log for now, and then we can perhaps factor the safe-logging
 code into a separate package for use by the other programs.

 * [https://github.com/cohosh/snowflake/compare/ticket21304#diff-
 91bbeda7eb98a7adc57b9e47e2cf5c2bL283 In server],
   logs are scrubbed whether the `--log` option is used or not.
   [https://github.com/cohosh/snowflake/compare/ticket21304#diff-
 ba4bd8a4477426567c409d66c2cf8a28L383 In proxy-go] though,
   logs are only scrubbed when `--log` is used, otherwise they are left
 unscrubbed.
   Is there a reason for the difference?
   I think it's best to default to scrubbed in all cases.
   Maybe even [https://github.com/cohosh/snowflake/compare/ticket21304
 #diff-0efd2b39fd0c8010b9dd51b4f07edf1bL94 in client],
   we should run logs through the scrubber before sending them to
 `ioutil.Discard`,
   both to remind us to use the scrubber after we can rely on #26360 being
 fixed and re-enable non-file logging,
   and to eliminate one difference in the logging code paths
   (imagine a bug in the log scrubber that only manifests when logging to a
 file).
   Maybe use logic like this:
   {{{
 var logOutput io.Writer = os.Stderr
 if logFilename != "" {
 f, err := os.OpenFile(logFilename,
 os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0600)
 if err != nil {
 log.Fatalf("can't open log file: %s", err)
 }
 defer f.Close()
 logOutput = f
 }
 //We want to send the log output through our scrubber first
 log.SetOutput({logOutput})
   }}}
   In client, the initial value for `logOutput` can be `ioutil.Discard`
 rather than `os.Stderr`.
 * Speaking of which, the default log output in the absence of a `--log`
 option should be
   `os.Stderr`, not `os.Stdout`.
   [https://golang.org/pkg/log/#pkg-overview Ref]: "That logger writes to
 standard error..."
   There's now a [https://golang.org/pkg/log/#Logger.Writer Logger.Writer]
 method that would allow
   us to ask for the default output, but as it was only
 [https://github.com/golang/go/pull/28399 added in 1.12],
   it's probably still too new to use.
 * The `logScrubber.Write` method only looks at one buffer's worth of data
 at a time,
   so it fails when an IP address is split across buffers. For example,
 this test fails:
   {{{
 type byteAtATimeWriter struct {
 io.Writer
 }
 func (w *byteAtATimeWriter) Write(p []byte) (int, error) {
 total := 0
 for i := range p {
 n, err := w.Writer.Write(p[i : i+1])
 total += n
 if err != nil {
 return total, err
 }
 }
 return total, nil
 }
 func TestLogScrubberSplit(t *testing.T) {
 log.SetFlags(0)
 var buff bytes.Buffer
 log.SetOutput({{}})
 log.Print("test 1.2.3.4 test")
 if !bytes.Equal(buff.Bytes(), []byte("test X.X.X.X test\n")) {
 t.Errorf("%q", buff.Bytes())
 }
 }
   }}}
   See below re `RedactErrorWriter` for an idea on how to deal with this.
 * You can use `!bytes.Equal(a, b)` instead of `bytes.Compare(a, b) != 0`.
 Try this:
   {{{
 gofmt -l -w -r 'bytes.Compare(a, b) != 0 -> !bytes.Equal(a, b)'
 server_test.go
   }}}
   Or, use `buff.String` and use string equality.
 * Should we scrub port numbers, too? A port number could be identifying
 within a short time span.
   See below for more ideas about this.
 * Consider structuring repetitive tests like this:
   {{{
 for _, test := range []struct {
 input, expected string
 }{
 {
 "http: TLS handshake error from 129.97.208.23:38310:",
 "http: TLS handshake error from X.X.X.X:38310:\n",
 },
 {
 "http2: panic serving
 [2620:101:f000:780:9097:75b1:519f:dbb8]:58344: interface conversion:
 *http2.responseWriter is not http.Hijacker: missing method Hijack",
 "http2: panic serving [X:X:X:X:X:X:X:X]:58344: 

Re: [tor-bugs] #21304 [Obfuscation/Snowflake]: Sanitize snowflake.log

[Tor Bug Tracker & Wiki]

#21304: Sanitize snowflake.log
---+--
 Reporter:  arlolra|  Owner:  cohosh
 Type:  defect | Status:  needs_review
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords:  starter|  Actual Points:
Parent ID: | Points:  1
 Reviewer: |Sponsor:
---+--
Changes (by cohosh):

 * status:  assigned => needs_review


Comment:

 Made some changes to the scrubber and added it to the other go components
 (proxy-go, broker, client). The candidate is here:
 https://github.com/cohosh/snowflake/compare/ticket21304

 One of the problems I can forsee with the current code is that I had to
 introduce a hack to make the scrubber leave fingerprints alone. This is
 based off the assumption that logged IPv6 addresses are enclosed in
 {{{[,]}}} brackets. As far as I can tell, this seems to be the case but it
 makes me a bit uncomfortable.

 What are our feelings about scrubbing fingerprints from logs as well? Is
 it necessary? The one in particular I'm looking at is the received answer
 in the client log:
 {{{
 a=ice-options:trickle
 a=fingerprint:sha-256
 
8D:CE:FE:08:F1:AC:32:30:88:D1:B4:1A:34:84:19:C2:43:18:4A:57:A9:20:2F:DC:C2:32:01:38:F9:8B:E5:8C
 a=setup:active
 a=mid:data
 }}}

 If we don't need the fingerprint, I'd prefer to "over scrub" instead of
 potentially leaking info if the IPv6 logging violates the above
 assumption.

 I did a cursory glance at the sanitized logs and there are no obvious
 privacy-related leaks at the moment.

 Putting this in needs_review because, while there are other logging
 changes that could be made, most are concerned with the verbosity of the
 output which is a separate issue.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21304 [Obfuscation/Snowflake]: Sanitize snowflake.log

[Tor Bug Tracker & Wiki]

#21304: Sanitize snowflake.log
---+--
 Reporter:  arlolra|  Owner:  cohosh
 Type:  defect | Status:  assigned
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords:  starter|  Actual Points:
Parent ID: | Points:  1
 Reviewer: |Sponsor:
---+--

Comment (by cohosh):

 I've implemented a log scrubber that sends all log output through a series
 of regular expressions to replace IPv4 and IPv6 addresses with safe
 strings ({{{X.X.X.X}}} and {{{X:X:X:X:X:X:X:X}}}, respectively). Here is
 the commit:
 
https://github.com/cohosh/snowflake/commit/e5dda8a3d526b692712baa4e8602b4248ea38376

 The http server panic mentioned above is solved in this way, since the
 server is set up to log messages to the default logger (set up in
 server.go to output to the provided log file). If the code is ever changed
 to give the http.Server structs created in server.go their own logger,
 we'll need to make sure we add a scrubber to the logger.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21304 [Obfuscation/Snowflake]: Sanitize snowflake.log

[Tor Bug Tracker & Wiki]

#21304: Sanitize snowflake.log
---+--
 Reporter:  arlolra|  Owner:  cohosh
 Type:  defect | Status:  assigned
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords:  starter|  Actual Points:
Parent ID: | Points:  1
 Reviewer: |Sponsor:
---+--
Changes (by cohosh):

 * owner:  (none) => cohosh
 * status:  new => assigned


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21304 [Obfuscation/Snowflake]: Sanitize snowflake.log

[Tor Bug Tracker & Wiki]

#21304: Sanitize snowflake.log
---+
 Reporter:  arlolra|  Owner:  (none)
 Type:  defect | Status:  new
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords:  starter|  Actual Points:
Parent ID: | Points:  1
 Reviewer: |Sponsor:
---+
Changes (by gaba):

 * points:   => 1


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21304 [Obfuscation/Snowflake]: Sanitize snowflake.log

[Tor Bug Tracker & Wiki]

#21304: Sanitize snowflake.log
---+
 Reporter:  arlolra|  Owner:  (none)
 Type:  defect | Status:  new
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords:  starter|  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+
Description changed by dcf:

Old description:

> For starters, the timestamps are in the local timezone.  We can make that
> UTC
>
> See also #19026

New description:

 For starters, the timestamps are in the local timezone.  We can make that
 UTC

 See also #19026

 Known problems:
  * When the websocket server panics (as in #29125), it writes the client
 IP address to the log:
{{{
 2019/01/18 18:56:29 http2: panic serving X.X.X.X:: interface
 conversion: *http2.responseWriter is not http.Hijacker: missing method
 Hijack
}}}

--

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21304 [Obfuscation/Snowflake]: Sanitize snowflake.log

[Tor Bug Tracker & Wiki]

#21304: Sanitize snowflake.log
---+
 Reporter:  arlolra|  Owner:  (none)
 Type:  defect | Status:  new
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords:  starter|  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+
Changes (by dcf):

 * keywords:   => starter
 * cc: Samdney (added)


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21304 [Obfuscation/Snowflake]: Sanitize snowflake.log

[Tor Bug Tracker & Wiki]

#21304: Sanitize snowflake.log
---+
 Reporter:  arlolra|  Owner:  (none)
 Type:  defect | Status:  new
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords: |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+
Changes (by dcf):

 * keywords:  starter =>
 * cc: Samdney (removed)


Comment:

 I think the highest priority for this ticket is to make sure that IP
 addresses don't end up in the logs. This can be a little tricky because IP
 addresses can show up as part of error messages. See
 [https://gitweb.torproject.org/pluggable-transports/meek.git/tree/meek-
 server/meek-server.go?id=502001aed9e40f79807b913b02ea82f7c381e47d#n178
 scrubError] in meek-server for a way of sanitizing certain kinds of
 errors. However for others, it may be necessary to do regex replacement or
 similar.

 Also there are multiple "snowflake.log"s made by different programs:
 client, server, broker, proxy-go.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21304 [Obfuscation/Snowflake]: Sanitize snowflake.log

[Tor Bug Tracker & Wiki]

#21304: Sanitize snowflake.log
---+
 Reporter:  arlolra|  Owner:  (none)
 Type:  defect | Status:  new
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords:  starter|  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+
Changes (by Samdney):

 * cc: Samdney (added)


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21304 [Obfuscation/Snowflake]: Sanitize snowflake.log

[Tor Bug Tracker & Wiki]

#21304: Sanitize snowflake.log
---+
 Reporter:  arlolra|  Owner:  (none)
 Type:  defect | Status:  new
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords:  starter|  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+
Changes (by Samdney):

 * keywords:   => starter


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21304 [Obfuscation/Snowflake]: Sanitize snowflake.log

[Tor Bug Tracker & Wiki]

#21304: Sanitize snowflake.log
---+-
 Reporter:  arlolra|  Owner:
 Type:  defect | Status:  new
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords: |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+-

Comment (by dcf):

 client logs in UTC since [https://gitweb.torproject.org/pluggable-
 transports/snowflake.git/commit/?id=86a244c39ea7937342239d9c2a003926685cb199
 86a244c39e].

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #21304 [Obfuscation/Snowflake]: Sanitize snowflake.log

[Tor Bug Tracker & Wiki]

#21304: Sanitize snowflake.log
---+-
 Reporter:  arlolra|  Owner:
 Type:  defect | Status:  new
 Priority:  Medium |  Milestone:
Component:  Obfuscation/Snowflake  |Version:
 Severity:  Normal | Resolution:
 Keywords: |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+-

Comment (by arma):

 Shouldn't Tor Browser be starting the snowflake stuff in UTC, so snowflake
 doesn't even get to *know* the local timezone?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs