Opened 5 months ago

Closed 3 months ago

#28848 closed project (implemented)

Document Snowflake broker implementation

Reported by: ahf Owned by: ahf
Priority: Medium Milestone:
Component: Circumvention/Snowflake Version: Tor: unspecified
Severity: Normal Keywords: snowflake, tor-pt, network-team-roadmap-2019-Q1Q2
Cc: arma, nickm, dcf, arlolra, gaba Actual Points: 3
Parent ID: Points: 8
Reviewer: cohosh, arma Sponsor: Sponsor19

Description

We should document how the current Snowflake implementation distributes Snowflake proxies to clients that is in need of them.

Once we have specified how the current implementation works, we should figure out if there are things we would like to change before we can start shipping Snowflake in an alpha state to users.

1) Look at broker code in the Snowflake repository.
2) Document current behavior with long-polling clients waiting to get handed a proxy.
3) Let's discuss if this design have any weaknesses we would like to address.

One weakness here could be that currently the broker nor client DOES NOT pass information about which bridge/relay to connect to, but this bridge/relay host is hardcoded in both the Go and JS proxy code.

Child Tickets

Change History (15)

comment:1 Changed 5 months ago by gaba

Cc: gaba added

comment:2 Changed 5 months ago by gaba

Keywords: snowflake tor-pt added

comment:3 Changed 4 months ago by gaba

Points: 8

comment:4 Changed 4 months ago by ahf

Owner: set to ahf
Status: newassigned

comment:5 Changed 3 months ago by gaba

Milestone: Tor: unspecifiedNetwork Team 2019 Q1Q2

comment:6 Changed 3 months ago by gaba

Keywords: network-team-roadmap-2019-Q1Q2 added

comment:7 Changed 3 months ago by gaba

Milestone: Network Team 2019 Q1Q2

comment:8 Changed 3 months ago by ahf

Actual Points: 2

comment:9 Changed 3 months ago by ahf

Status: assignedneeds_review

Moving this part to needs_review. Don't close it when you think it's OK.

comment:10 Changed 3 months ago by ahf

Reviewer: cohosh, arma

comment:11 Changed 3 months ago by cohosh

Looking good! Here's some comments:

Dependencies

  • Let's keep track in the documentation of the current required version of Go needed to build the broker. This could be put in the Go Dependencies section.

Source code documentation

  • I think one of the goals of this documentation should also be for someone to be able to look at it and then implement their own client and proxies that can interface with the broker without having to look at the existing client and proxy code. As such, we can be more precise about how to use each of these handlers. More specifically, does the client/proxy make a GET or a POST request when interacting with the handlers, and are there any request headers that are necessary? For example: the /proxy handler expects the X-Session-ID header, and is implemented in proxy-go as a POST request with no body.
  • Maybe we should mention that the timeout is currently 10s?
  • We could have another section for command line arguments:
    Usage of ./broker:
      -acme-email string
        	optional contact email for Let's Encrypt notifications
      -acme-hostnames string
        	comma-separated hostnames for TLS certificate
      -addr string
        	address to listen on (default ":443")
      -disable-tls
        	don't use HTTPS
    
  • In the /answer handler section, it says If the client was too slow to send its offer but it should be if the *snowflake proxy* is too slow.
  • Also in /answer handler: We might want to expand on what "bogus answer" means here. I think it's simply if ioutil.ReadAll failed. I don't think there's any sort of mediation or sanitization of the actual Session Descriptor Answer.
  • Was the question about crawling ever answered? I can't think of a very good reason not to allow it. Even if censors were crawling the web for Snowflake brokers, they could get this information much more easily just from the source code. I think because of domain fronting we do not need to keep broker locations a secret. Perhaps if we are later doing something more sneaky with partitioning snowflakes and distributing them with something like Salmon or Hyphae we need to be careful about a crawler possibly gaining access to that information. In any case, my understanding of robots.txt is that is suggests to web crawlers not to crawl but doesn't have any actual security guarantees. So maybe we do not care one way or the other.

Metrics

  • Let's give the units for the round trip estimate. We might even want to add this to the debug output.

Other notes

I fixed some minor typos here: https://github.com/ahf/snowflake-notes/commit/fb4304a7df08c6ddeeb103f38fc9103721a20cd9

comment:12 in reply to:  11 Changed 3 months ago by dcf

Replying to cohosh:

  • Was the question about crawling ever answered? I can't think of a very good reason not to allow it. Even if censors were crawling the web for Snowflake brokers, they could get this information much more easily just from the source code.

The robots.txt is a bug, I believe. I opened #29565 for it.

comment:13 Changed 3 months ago by ahf

Actual Points: 23

I think I've fixed all the issues listed above with an exception of the following:

  • I've not specified that we require requests to be either POST/GET since it seems like the broker don't care, but in the client/proxy code we do specify it to use POST most of the time. I think if we begin to enforce this we should specify it.
  • I've left out command line options for now, since I think they are more of a implementation detail?

Let me know what you think.

comment:14 Changed 3 months ago by cohosh

Status: needs_reviewmerge_ready

It looks good, thanks!

I've not specified that we require requests to be either POST/GET since it seems like the broker don't >care, but in the client/proxy code we do specify it to use POST most of the time. I think if we begin >to enforce this we should specify it.

Sounds good.

I've left out command line options for now, since I think they are more of a implementation detail?

Good point, agreed.

comment:15 Changed 3 months ago by ahf

Resolution: implemented
Status: merge_readyclosed

Thanks! All done. Closing.

Note: See TracTickets for help on using tickets.