commit 638ec6c222327ca02950338b2919ce8b22e1f900 Author: Cecylia Bocovich <cohosh@torproject.org> Date: Thu Sep 9 16:01:38 2021 -0400 Update Snowflake client library documentation Follow best practices for documenting the exported pieces of the Snowflake client library. --- client/lib/interfaces.go | 17 +++++++------- client/lib/peers.go | 18 +++++++------- client/lib/rendezvous.go | 23 +++++++----------- client/lib/rendezvous_ampcache.go | 4 ++-- client/lib/rendezvous_http.go | 2 +- client/lib/rendezvous_test.go | 4 ++-- client/lib/snowflake.go | 49 ++++++++++++++++++++++++++++++++------- client/lib/webrtc.go | 16 ++++++++----- 8 files changed, 83 insertions(+), 50 deletions(-) diff --git a/client/lib/interfaces.go b/client/lib/interfaces.go index 0a871b8..66b9d56 100644 --- a/client/lib/interfaces.go +++ b/client/lib/interfaces.go @@ -1,23 +1,24 @@ package lib -// Interface for catching Snowflakes. (aka the remote dialer) +// Tongue is an interface for catching Snowflakes. (aka the remote dialer) type Tongue interface { + // Catch makes a connection to a new snowflake. Catch() (*WebRTCPeer, error) - // Get the maximum number of snowflakes + // GetMax returns the maximum number of snowflakes a client can have. GetMax() int } -// Interface for collecting some number of Snowflakes, for passing along -// ultimately to the SOCKS handler. +// SnowflakeCollector is an interface for managing a client's collection of snowflakes. type SnowflakeCollector interface { - // Add a Snowflake to the collection. - // Implementation should decide how to connect and maintain the webRTCConn. + // Collect adds a snowflake to the collection. + // The implementation of Collect should decide how to connect to and maintain + // the connection to the WebRTCPeer. Collect() (*WebRTCPeer, error) - // Remove and return the most available Snowflake from the collection. + // Pop removes and returns the most available snowflake from the collection. Pop() *WebRTCPeer - // Signal when the collector has stopped collecting. + // Melted returns a channel that will signal when the collector has stopped. Melted() <-chan struct{} } diff --git a/client/lib/peers.go b/client/lib/peers.go index 1abcd95..6bddbf2 100644 --- a/client/lib/peers.go +++ b/client/lib/peers.go @@ -8,7 +8,7 @@ import ( "sync" ) -// Container which keeps track of multiple WebRTC remote peers. +// Peers is a container that keeps track of multiple WebRTC remote peers. // Implements |SnowflakeCollector|. // // Maintaining a set of pre-connected Peers with fresh but inactive datachannels @@ -31,7 +31,7 @@ type Peers struct { collectLock sync.Mutex } -// Construct a fresh container of remote peers. +// NewPeers constructs a fresh container of remote peers. func NewPeers(tongue Tongue) (*Peers, error) { p := &Peers{} // Use buffered go channel to pass snowflakes onwards to the SOCKS handler. @@ -45,7 +45,7 @@ func NewPeers(tongue Tongue) (*Peers, error) { return p, nil } -// As part of |SnowflakeCollector| interface. +// Collect connects to and adds a new remote peer as part of |SnowflakeCollector| interface. func (p *Peers) Collect() (*WebRTCPeer, error) { // Engage the Snowflake Catching interface, which must be available. p.collectLock.Lock() @@ -76,8 +76,8 @@ func (p *Peers) Collect() (*WebRTCPeer, error) { return connection, nil } -// Pop blocks until an available, valid snowflake appears. Returns nil after End -// has been called. +// Pop blocks until an available, valid snowflake appears. +// Pop will return nil after End has been called. func (p *Peers) Pop() *WebRTCPeer { for { snowflake, ok := <-p.snowflakeChan @@ -93,12 +93,13 @@ func (p *Peers) Pop() *WebRTCPeer { } } -// As part of |SnowflakeCollector| interface. +// Melted returns a channel that will close when peers stop being collected. +// Melted is a necessary part of |SnowflakeCollector| interface. func (p *Peers) Melted() <-chan struct{} { return p.melt } -// Returns total available Snowflakes (including the active one) +// Count returns the total available Snowflakes (including the active ones) // The count only reduces when connections themselves close, rather than when // they are popped. func (p *Peers) Count() int { @@ -118,7 +119,8 @@ func (p *Peers) purgeClosedPeers() { } } -// Close all Peers contained here. +// End closes all active connections to Peers contained here, and stops the +// collection of future Peers. func (p *Peers) End() { close(p.melt) p.collectLock.Lock() diff --git a/client/lib/rendezvous.go b/client/lib/rendezvous.go index cf67e09..689e51c 100644 --- a/client/lib/rendezvous.go +++ b/client/lib/rendezvous.go @@ -1,10 +1,5 @@ // WebRTC rendezvous requires the exchange of SessionDescriptions between // peers in order to establish a PeerConnection. -// -// This file contains the one method currently available to Snowflake: -// -// - Domain-fronted HTTP signaling. The Broker automatically exchange offers -// and answers between this client and some remote WebRTC proxy. package lib @@ -22,7 +17,7 @@ import ( ) const ( - BrokerErrorUnexpected string = "Unexpected error, no answer." + brokerErrorUnexpected string = "Unexpected error, no answer." readLimit = 100000 //Maximum number of bytes to be read from an HTTP response ) @@ -55,7 +50,7 @@ func createBrokerTransport() http.RoundTripper { return transport } -// Construct a new BrokerChannel, where: +// NewBrokerChannel construct a new BrokerChannel, where: // |broker| is the full URL of the facilitating program which assigns proxies // to clients, and |front| is the option fronting domain. func NewBrokerChannel(broker, ampCache, front string, keepLocalAddresses bool) (*BrokerChannel, error) { @@ -85,10 +80,8 @@ func NewBrokerChannel(broker, ampCache, front string, keepLocalAddresses bool) ( }, nil } -// Roundtrip HTTP POST using WebRTC SessionDescriptions. -// -// Send an SDP offer to the broker, which assigns a proxy and responds -// with an SDP answer from a designated remote WebRTC peer. +// Negotiate uses a RendezvousMethod to send the client's WebRTC SDP offer +// and receive a snowflake proxy WebRTC SDP answer in return. func (bc *BrokerChannel) Negotiate(offer *webrtc.SessionDescription) ( *webrtc.SessionDescription, error) { // Ideally, we could specify an `RTCIceTransportPolicy` that would handle @@ -135,6 +128,7 @@ func (bc *BrokerChannel) Negotiate(offer *webrtc.SessionDescription) ( return util.DeserializeSessionDescription(resp.Answer) } +// SetNATType sets the NAT type of the client so we can send it to the WebRTC broker. func (bc *BrokerChannel) SetNATType(NATType string) { bc.lock.Lock() bc.natType = NATType @@ -142,13 +136,14 @@ func (bc *BrokerChannel) SetNATType(NATType string) { log.Printf("NAT Type: %s", NATType) } -// Implements the |Tongue| interface to catch snowflakes, using BrokerChannel. +// WebRTCDialer implements the |Tongue| interface to catch snowflakes, using BrokerChannel. type WebRTCDialer struct { *BrokerChannel webrtcConfig *webrtc.Configuration max int } +// NewWebRTCDialer constructs a new WebRTCDialer. func NewWebRTCDialer(broker *BrokerChannel, iceServers []webrtc.ICEServer, max int) *WebRTCDialer { config := webrtc.Configuration{ ICEServers: iceServers, @@ -161,14 +156,14 @@ func NewWebRTCDialer(broker *BrokerChannel, iceServers []webrtc.ICEServer, max i } } -// Initialize a WebRTC Connection by signaling through the broker. +// Catch initializes a WebRTC Connection by signaling through the BrokerChannel. func (w WebRTCDialer) Catch() (*WebRTCPeer, error) { // TODO: [#25591] Fetch ICE server information from Broker. // TODO: [#25596] Consider TURN servers here too. return NewWebRTCPeer(w.webrtcConfig, w.BrokerChannel) } -// Returns the maximum number of snowflakes to collect +// GetMax returns the maximum number of snowflakes to collect. func (w WebRTCDialer) GetMax() int { return w.max } diff --git a/client/lib/rendezvous_ampcache.go b/client/lib/rendezvous_ampcache.go index 2f1fb9f..6ac99b3 100644 --- a/client/lib/rendezvous_ampcache.go +++ b/client/lib/rendezvous_ampcache.go @@ -93,7 +93,7 @@ func (r *ampCacheRendezvous) Exchange(encPollReq []byte) ([]byte, error) { // * If the broker returns a 5xx status, the AMP cache // translates it to a 404. // https://amp.dev/documentation/guides-and-tutorials/learn/amp-caches-and-cors... - return nil, errors.New(BrokerErrorUnexpected) + return nil, errors.New(brokerErrorUnexpected) } if _, err := resp.Location(); err == nil { // The Google AMP Cache may return a "silent redirect" with @@ -103,7 +103,7 @@ func (r *ampCacheRendezvous) Exchange(encPollReq []byte) ([]byte, error) { // follow redirects nor execute JavaScript, but in any case we // cannot extract information from this response and can only // treat it as an error. - return nil, errors.New(BrokerErrorUnexpected) + return nil, errors.New(brokerErrorUnexpected) } lr := io.LimitReader(resp.Body, readLimit+1) diff --git a/client/lib/rendezvous_http.go b/client/lib/rendezvous_http.go index e020077..43ed075 100644 --- a/client/lib/rendezvous_http.go +++ b/client/lib/rendezvous_http.go @@ -60,7 +60,7 @@ func (r *httpRendezvous) Exchange(encPollReq []byte) ([]byte, error) { log.Printf("HTTP rendezvous response: %s", resp.Status) if resp.StatusCode != http.StatusOK { - return nil, errors.New(BrokerErrorUnexpected) + return nil, errors.New(brokerErrorUnexpected) } return limitedRead(resp.Body, readLimit) diff --git a/client/lib/rendezvous_test.go b/client/lib/rendezvous_test.go index 6a1a071..4bc5766 100644 --- a/client/lib/rendezvous_test.go +++ b/client/lib/rendezvous_test.go @@ -122,7 +122,7 @@ func TestHTTPRendezvous(t *testing.T) { answer, err := rend.Exchange(fakeEncPollReq) So(err, ShouldNotBeNil) So(answer, ShouldBeNil) - So(err.Error(), ShouldResemble, BrokerErrorUnexpected) + So(err.Error(), ShouldResemble, brokerErrorUnexpected) }) Convey("httpRendezvous.Exchange fails with error", func() { @@ -243,7 +243,7 @@ func TestAMPCacheRendezvous(t *testing.T) { answer, err := rend.Exchange(fakeEncPollReq) So(err, ShouldNotBeNil) So(answer, ShouldBeNil) - So(err.Error(), ShouldResemble, BrokerErrorUnexpected) + So(err.Error(), ShouldResemble, brokerErrorUnexpected) }) Convey("ampCacheRendezvous.Exchange fails with error", func() { diff --git a/client/lib/snowflake.go b/client/lib/snowflake.go index e0591a7..4b17f0b 100644 --- a/client/lib/snowflake.go +++ b/client/lib/snowflake.go @@ -17,12 +17,21 @@ import ( ) const ( + // ReconnectTimeout is the time a Snowflake client will wait before collecting + // more snowflakes. ReconnectTimeout = 10 * time.Second + // SnowflakeTimeout is the time a Snowflake client will wait before determining that + // a remote snowflake has been disconnected. If no new messages are sent or received + // in this time period, the client will terminate the connection with the remote + // peer and collect a new snowflake. SnowflakeTimeout = 20 * time.Second - // How long to wait for the OnOpen callback on a DataChannel. + // DataChannelTimeout is how long the client will wait for the OnOpen callback + // on a newly created DataChannel. DataChannelTimeout = 10 * time.Second + // WindowSize is the number of packets in the send and receive window of a KCP connection. WindowSize = 65535 + // StreamSize controls the maximum amount of in flight data between a client and server. StreamSize = 1048576 //1MB ) @@ -37,16 +46,31 @@ type Transport struct { dialer *WebRTCDialer } +// ClientConfig defines how the SnowflakeClient will connect to the broker and Snowflake proxies. type ClientConfig struct { - BrokerURL string - AmpCacheURL string - FrontDomain string - ICEAddresses []string + // BrokerURL is the full URL of the Snowflake broker that the client will connect to. + BrokerURL string + // AmpCacheURL is the full URL of a valid AMP cache. A nonzero value indicates + // that AMP cache will be used as the rendezvous method with the broker. + AmpCacheURL string + // FrontDomain is a the full URL of an optional front domain that can be used with either + // the AMP cache or HTTP domain fronting rendezvous method. + FrontDomain string + // ICEAddresses are a slice of ICE server URLs that will be used for NAT traversal and + // the creation of the client's WebRTC SDP offer. + ICEAddresses []string + // KeepLocalAddresses is an optional setting that will prevent the removal of local or + // invalid addresses from the client's SDP offer. This is useful for local deployments + // and testing. KeepLocalAddresses bool - Max int + // Max is the maximum number of snowflake proxy peers that the client should attempt to + // connect to. + Max int } -// Create a new Snowflake transport client that can spawn multiple Snowflake connections. +// NewSnowflakeClient creates a new Snowflake transport client that can spawn multiple +// Snowflake connections. +// // brokerURL and frontDomain are the urls for the broker host and domain fronting host // iceAddresses are the STUN/TURN urls needed for WebRTC negotiation // keepLocalAddresses is a flag to enable sending local network addresses (for testing purposes) @@ -82,8 +106,10 @@ func NewSnowflakeClient(config ClientConfig) (*Transport, error) { return transport, nil } -// Create a new Snowflake connection. Starts the collection of snowflakes and returns a -// smux Stream. +// Dial creates a new Snowflake connection. +// Dial starts the collection of snowflakes and returns a SnowflakeConn that is a +// wrapper around a smux.Stream that will reliably deliver data to a Snowflake +// server through one or more snowflake proxies. func (t *Transport) Dial() (net.Conn, error) { // Cleanup functions to run before returning, in case of an error. var cleanup []func() @@ -132,10 +158,12 @@ func (t *Transport) Dial() (net.Conn, error) { return &SnowflakeConn{Stream: stream, sess: sess, pconn: pconn, snowflakes: snowflakes}, nil } +// SetRendezvousMethod sets the rendezvous method to the Snowflake broker. func (t *Transport) SetRendezvousMethod(r RendezvousMethod) { t.dialer.Rendezvous = r } +// SnowflakeConn is a reliable connection to a snowflake server that implements net.Conn. type SnowflakeConn struct { *smux.Stream sess *smux.Session @@ -143,6 +171,9 @@ type SnowflakeConn struct { snowflakes *Peers } +// Close closes the connection. +// +// The collection of snowflake proxies for this connection is stopped. func (conn *SnowflakeConn) Close() error { log.Printf("---- SnowflakeConn: closed stream %v ---", conn.ID()) conn.Stream.Close() diff --git a/client/lib/webrtc.go b/client/lib/webrtc.go index 6fc24c0..538cc8b 100644 --- a/client/lib/webrtc.go +++ b/client/lib/webrtc.go @@ -12,10 +12,9 @@ import ( "github.com/pion/webrtc/v3" ) -// Remote WebRTC peer. +// WebRTCPeer represents a WebRTC connection to a remote snowflake proxy. // -// Handles preparation of go-webrtc PeerConnection. Only ever has -// one DataChannel. +// Each WebRTCPeer only ever has one DataChannel that is used as the peer's transport. type WebRTCPeer struct { id string pc *webrtc.PeerConnection @@ -35,7 +34,11 @@ type WebRTCPeer struct { bytesLogger bytesLogger } -// Construct a WebRTC PeerConnection. +// NewWebRTCPeer constructs a WebRTC PeerConnection to a snowflake proxy. +// +// The creation of the peer handles the signaling to the Snowflake broker, including +// the exchange of SDP information, the creation of a PeerConnection, and the establishment +// of a DataChannel to the Snowflake proxy. func NewWebRTCPeer(config *webrtc.Configuration, broker *BrokerChannel) (*WebRTCPeer, error) { connection := new(WebRTCPeer) @@ -79,7 +82,7 @@ func (c *WebRTCPeer) Write(b []byte) (int, error) { return len(b), nil } -//Returns a boolean indicated whether the peer is closed +// Closed returns a boolean indicated whether the peer is closed. func (c *WebRTCPeer) Closed() bool { select { case <-c.closed: @@ -89,6 +92,7 @@ func (c *WebRTCPeer) Closed() bool { return false } +// Close closes the connection the snowflake proxy. func (c *WebRTCPeer) Close() error { c.once.Do(func() { close(c.closed) @@ -225,7 +229,7 @@ func (c *WebRTCPeer) preparePeerConnection(config *webrtc.Configuration) error { return nil } -// Close all channels and transports +// cleanup closes all channels and transports func (c *WebRTCPeer) cleanup() { // Close this side of the SOCKS pipe. if c.writePipe != nil { // c.writePipe can be nil in tests.