[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #18628 [Obfuscation/Snowflake]: Devise some way for the browser proxy to forward metadata to the bridge before the OR data
#18628: Devise some way for the browser proxy to forward metadata to the bridge
before the OR data
-----------------------------------+--------------------------
Reporter: arlolra | Owner: cmm323
Type: defect | Status: assigned
Priority: High | Milestone:
Component: Obfuscation/Snowflake | Version:
Severity: Normal | Resolution:
Keywords: | Actual Points:
Parent ID: | Points:
Reviewer: | Sponsor:
-----------------------------------+--------------------------
Comment (by cmm323):
Replying to [comment:8 dcf]:
>
> In websocket:
>
> {{{
> + // Request
> + request http.Request
> }}}
> {{{
> + ws.request = *req
> }}}
> I'm thinking it would be better to make request a pointer; i.e. `request
*http.Request`, `ws.request = req`, in order to avoid copying the struct.
I'm not sure it's safe to shallow-copy a Request struct, which may contain
other recursive structures.
Done.
>
> ----
>
> In snowflake:
>
> I'm a little concerned about parsing the SDP in order to get the remote
address. Ideally, of course, we'd find another way to do it, or use a
proper library to parse the SDP. But in the meantime, I
[https://gitweb.torproject.org/user/dcf/snowflake.git/commit/?h=bug18628&id=485538bcf00bd4ddaeb5f81dd05e3caaa89ffd6d
pushed some tests] to cover some additional syntax options that I took
from RFC 4566. Can you pull those changes and update the code so that all
the tests pass with `go test`?
Let me know if you feel like this is still needed.
>
> {{{
> + if conn.RemoteAddr() != nil && conn.RemoteAddr().String() != ""
{
> }}}
> I don't think we need the `!= ""` comparison here. Unless there's a kind
of Addr you're thinking of that can return an empty string?
I removed it.
>
> {{{
> + if clientIP == nil {
> + // Set client addr to empty
> + log.Printf("failed to validate client_ip: %s", addr)
> + addr = ""
> +
> }}}
> Let's not log an IP address here. We can add it (behind an "unsafe
logging" option) if we need it later.
Done.
>
> > Note client IP address is now added to `opt.relayURL` before dialing
websocket.
>
> I think that doing it this way creates a race condition. You have a
bunch of goroutines reading a writing global state. Better to make a copy
of the global relay URL (e.g., via url.Parse) that you can mutate each
time. In fact, opt.relayURL should probably be removed completely
([https://gitweb.torproject.org/pluggable-
transports/snowflake.git/commit/?id=dbe1ef4fa55569e5d17c405df5707f6eb05bb56c
I just did that on the master branch]).
Done.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/18628#comment:12>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs