[tor-commits] [snowflake/master] Have broker pass client NAT type to proxy

cohosh at torproject.org cohosh at torproject.org
Mon Jul 6 17:27:06 UTC 2020


commit 046dab865f18eb12a473b1a1c1d7aa15cf5883c7
Author: Cecylia Bocovich <cohosh at torproject.org>
Date:   Mon Jun 22 14:04:29 2020 -0400

    Have broker pass client NAT type to proxy
    
    This will allow browser-based proxies that are unable to determine their
    NAT type to conservatively label themselves as restricted NATs if they
    fail to work with clients that have restricted NATs.
---
 broker/broker.go                | 31 ++++++++++++++++++++-----------
 broker/snowflake-broker_test.go | 26 +++++++++++++-------------
 broker/snowflake-heap.go        |  2 +-
 common/messages/proxy.go        | 24 ++++++++++++++++--------
 common/messages/proxy_test.go   | 16 +++++++++-------
 proxy/proxy-go_test.go          |  2 +-
 proxy/snowflake.go              |  2 +-
 7 files changed, 61 insertions(+), 42 deletions(-)

diff --git a/broker/broker.go b/broker/broker.go
index 9297980..99f6c69 100644
--- a/broker/broker.go
+++ b/broker/broker.go
@@ -111,17 +111,17 @@ type ProxyPoll struct {
 	id           string
 	proxyType    string
 	natType      string
-	offerChannel chan []byte
+	offerChannel chan *ClientOffer
 }
 
 // Registers a Snowflake and waits for some Client to send an offer,
 // as part of the polling logic of the proxy handler.
-func (ctx *BrokerContext) RequestOffer(id string, proxyType string, natType string) []byte {
+func (ctx *BrokerContext) RequestOffer(id string, proxyType string, natType string) *ClientOffer {
 	request := new(ProxyPoll)
 	request.id = id
 	request.proxyType = proxyType
 	request.natType = natType
-	request.offerChannel = make(chan []byte)
+	request.offerChannel = make(chan *ClientOffer)
 	ctx.proxyPolls <- request
 	// Block until an offer is available, or timeout which sends a nil offer.
 	offer := <-request.offerChannel
@@ -165,7 +165,7 @@ func (ctx *BrokerContext) AddSnowflake(id string, proxyType string, natType stri
 	snowflake.id = id
 	snowflake.clients = 0
 	snowflake.proxyType = proxyType
-	snowflake.offerChannel = make(chan []byte)
+	snowflake.offerChannel = make(chan *ClientOffer)
 	snowflake.answerChannel = make(chan []byte)
 	ctx.snowflakeLock.Lock()
 	if natType == NATRestricted {
@@ -213,7 +213,7 @@ func proxyPolls(ctx *BrokerContext, w http.ResponseWriter, r *http.Request) {
 		ctx.metrics.proxyIdleCount++
 		ctx.metrics.lock.Unlock()
 
-		b, err = messages.EncodePollResponse("", false)
+		b, err = messages.EncodePollResponse("", false, "")
 		if err != nil {
 			w.WriteHeader(http.StatusInternalServerError)
 			return
@@ -222,7 +222,7 @@ func proxyPolls(ctx *BrokerContext, w http.ResponseWriter, r *http.Request) {
 		w.Write(b)
 		return
 	}
-	b, err = messages.EncodePollResponse(string(offer), true)
+	b, err = messages.EncodePollResponse(string(offer.sdp), true, offer.natType)
 	if err != nil {
 		w.WriteHeader(http.StatusInternalServerError)
 		return
@@ -232,28 +232,37 @@ func proxyPolls(ctx *BrokerContext, w http.ResponseWriter, r *http.Request) {
 	}
 }
 
+// Client offer contains an SDP and the NAT type of the client
+type ClientOffer struct {
+	natType string
+	sdp     []byte
+}
+
 /*
 Expects a WebRTC SDP offer in the Request to give to an assigned
 snowflake proxy, which responds with the SDP answer to be sent in
 the HTTP response back to the client.
 */
 func clientOffers(ctx *BrokerContext, w http.ResponseWriter, r *http.Request) {
+	var err error
+
 	startTime := time.Now()
-	offer, err := ioutil.ReadAll(http.MaxBytesReader(w, r.Body, readLimit))
+	offer := &ClientOffer{}
+	offer.sdp, err = ioutil.ReadAll(http.MaxBytesReader(w, r.Body, readLimit))
 	if nil != err {
 		log.Println("Invalid data.")
 		w.WriteHeader(http.StatusBadRequest)
 		return
 	}
 
-	natType := r.Header.Get("Snowflake-NAT-Type")
-	if natType == "" {
-		natType = NATUnknown
+	offer.natType = r.Header.Get("Snowflake-NAT-Type")
+	if offer.natType == "" {
+		offer.natType = NATUnknown
 	}
 
 	// Only hand out known restricted snowflakes to unrestricted clients
 	var snowflakeHeap *SnowflakeHeap
-	if natType == NATUnrestricted {
+	if offer.natType == NATUnrestricted {
 		snowflakeHeap = ctx.restrictedSnowflakes
 	} else {
 		snowflakeHeap = ctx.snowflakes
diff --git a/broker/snowflake-broker_test.go b/broker/snowflake-broker_test.go
index 91383a1..d03dca7 100644
--- a/broker/snowflake-broker_test.go
+++ b/broker/snowflake-broker_test.go
@@ -37,7 +37,7 @@ func TestBroker(t *testing.T) {
 		Convey("Broker goroutine matches clients with proxies", func() {
 			p := new(ProxyPoll)
 			p.id = "test"
-			p.offerChannel = make(chan []byte)
+			p.offerChannel = make(chan *ClientOffer)
 			go func(ctx *BrokerContext) {
 				ctx.proxyPolls <- p
 				close(ctx.proxyPolls)
@@ -45,23 +45,23 @@ func TestBroker(t *testing.T) {
 			ctx.Broker()
 			So(ctx.snowflakes.Len(), ShouldEqual, 1)
 			snowflake := heap.Pop(ctx.snowflakes).(*Snowflake)
-			snowflake.offerChannel <- []byte("test offer")
+			snowflake.offerChannel <- &ClientOffer{sdp: []byte("test offer")}
 			offer := <-p.offerChannel
 			So(ctx.idToSnowflake["test"], ShouldNotBeNil)
-			So(offer, ShouldResemble, []byte("test offer"))
+			So(offer.sdp, ShouldResemble, []byte("test offer"))
 			So(ctx.snowflakes.Len(), ShouldEqual, 0)
 		})
 
 		Convey("Request an offer from the Snowflake Heap", func() {
-			done := make(chan []byte)
+			done := make(chan *ClientOffer)
 			go func() {
 				offer := ctx.RequestOffer("test", "", NATUnknown)
 				done <- offer
 			}()
 			request := <-ctx.proxyPolls
-			request.offerChannel <- []byte("test offer")
+			request.offerChannel <- &ClientOffer{sdp: []byte("test offer")}
 			offer := <-done
-			So(offer, ShouldResemble, []byte("test offer"))
+			So(offer.sdp, ShouldResemble, []byte("test offer"))
 		})
 
 		Convey("Responds to client offers...", func() {
@@ -85,7 +85,7 @@ func TestBroker(t *testing.T) {
 					done <- true
 				}()
 				offer := <-snowflake.offerChannel
-				So(offer, ShouldResemble, []byte("test"))
+				So(offer.sdp, ShouldResemble, []byte("test"))
 				snowflake.answerChannel <- []byte("fake answer")
 				<-done
 				So(w.Body.String(), ShouldEqual, "fake answer")
@@ -104,7 +104,7 @@ func TestBroker(t *testing.T) {
 					done <- true
 				}()
 				offer := <-snowflake.offerChannel
-				So(offer, ShouldResemble, []byte("test"))
+				So(offer.sdp, ShouldResemble, []byte("test"))
 				<-done
 				So(w.Code, ShouldEqual, http.StatusGatewayTimeout)
 			})
@@ -125,10 +125,10 @@ func TestBroker(t *testing.T) {
 				// Pass a fake client offer to this proxy
 				p := <-ctx.proxyPolls
 				So(p.id, ShouldEqual, "ymbcCMto7KHNGYlp")
-				p.offerChannel <- []byte("fake offer")
+				p.offerChannel <- &ClientOffer{sdp: []byte("fake offer")}
 				<-done
 				So(w.Code, ShouldEqual, http.StatusOK)
-				So(w.Body.String(), ShouldEqual, `{"Status":"client match","Offer":"fake offer"}`)
+				So(w.Body.String(), ShouldEqual, `{"Status":"client match","Offer":"fake offer","NAT":""}`)
 			})
 
 			Convey("return empty 200 OK when no client offer is available.", func() {
@@ -141,7 +141,7 @@ func TestBroker(t *testing.T) {
 				// nil means timeout
 				p.offerChannel <- nil
 				<-done
-				So(w.Body.String(), ShouldEqual, `{"Status":"no match","Offer":""}`)
+				So(w.Body.String(), ShouldEqual, `{"Status":"no match","Offer":"","NAT":""}`)
 				So(w.Code, ShouldEqual, http.StatusOK)
 			})
 		})
@@ -279,7 +279,7 @@ func TestBroker(t *testing.T) {
 
 			<-polled
 			So(wP.Code, ShouldEqual, http.StatusOK)
-			So(wP.Body.String(), ShouldResemble, `{"Status":"client match","Offer":"fake offer"}`)
+			So(wP.Body.String(), ShouldResemble, `{"Status":"client match","Offer":"fake offer","NAT":"unknown"}`)
 			So(ctx.idToSnowflake["ymbcCMto7KHNGYlp"], ShouldNotBeNil)
 			// Follow up with the answer request afterwards
 			wA := httptest.NewRecorder()
@@ -543,7 +543,7 @@ func TestMetrics(t *testing.T) {
 				done <- true
 			}()
 			offer := <-snowflake.offerChannel
-			So(offer, ShouldResemble, []byte("test"))
+			So(offer.sdp, ShouldResemble, []byte("test"))
 			snowflake.answerChannel <- []byte("fake answer")
 			<-done
 
diff --git a/broker/snowflake-heap.go b/broker/snowflake-heap.go
index 19a64b2..12fe557 100644
--- a/broker/snowflake-heap.go
+++ b/broker/snowflake-heap.go
@@ -11,7 +11,7 @@ over the offer and answer channels.
 type Snowflake struct {
 	id            string
 	proxyType     string
-	offerChannel  chan []byte
+	offerChannel  chan *ClientOffer
 	answerChannel chan []byte
 	clients       int
 	index         int
diff --git a/common/messages/proxy.go b/common/messages/proxy.go
index 923189b..2d9e58d 100644
--- a/common/messages/proxy.go
+++ b/common/messages/proxy.go
@@ -29,7 +29,8 @@ HTTP 200 OK
   {
     type: offer,
     sdp: [WebRTC SDP]
-  }
+  },
+  NAT: ["unknown"|"restricted"|"unrestricted"]
 }
 
 2) If a client is not matched:
@@ -120,13 +121,15 @@ func DecodePollRequest(data []byte) (string, string, string, error) {
 type ProxyPollResponse struct {
 	Status string
 	Offer  string
+	NAT    string
 }
 
-func EncodePollResponse(offer string, success bool) ([]byte, error) {
+func EncodePollResponse(offer string, success bool, natType string) ([]byte, error) {
 	if success {
 		return json.Marshal(ProxyPollResponse{
 			Status: "client match",
 			Offer:  offer,
+			NAT:    natType,
 		})
 
 	}
@@ -135,28 +138,33 @@ func EncodePollResponse(offer string, success bool) ([]byte, error) {
 	})
 }
 
-// Decodes a poll response from the broker and returns an offer
+// Decodes a poll response from the broker and returns an offer and the client's NAT type
 // If there is a client match, the returned offer string will be non-empty
-func DecodePollResponse(data []byte) (string, error) {
+func DecodePollResponse(data []byte) (string, string, error) {
 	var message ProxyPollResponse
 
 	err := json.Unmarshal(data, &message)
 	if err != nil {
-		return "", err
+		return "", "", err
 	}
 	if message.Status == "" {
-		return "", fmt.Errorf("received invalid data")
+		return "", "", fmt.Errorf("received invalid data")
 	}
 
 	if message.Status == "client match" {
 		if message.Offer == "" {
-			return "", fmt.Errorf("no supplied offer")
+			return "", "", fmt.Errorf("no supplied offer")
 		}
 	} else {
 		message.Offer = ""
 	}
 
-	return message.Offer, nil
+	natType := message.NAT
+	if natType == "" {
+		natType = "unknown"
+	}
+
+	return message.Offer, natType, nil
 }
 
 type ProxyAnswerRequest struct {
diff --git a/common/messages/proxy_test.go b/common/messages/proxy_test.go
index 3aa67fb..f4191e1 100644
--- a/common/messages/proxy_test.go
+++ b/common/messages/proxy_test.go
@@ -109,7 +109,7 @@ func TestDecodeProxyPollResponse(t *testing.T) {
 		}{
 			{
 				"fake offer",
-				`{"Status":"client match","Offer":"fake offer"}`,
+				`{"Status":"client match","Offer":"fake offer","NAT":"unknown"}`,
 				nil,
 			},
 			{
@@ -128,9 +128,9 @@ func TestDecodeProxyPollResponse(t *testing.T) {
 				fmt.Errorf(""),
 			},
 		} {
-			offer, err := DecodePollResponse([]byte(test.data))
-			So(offer, ShouldResemble, test.offer)
+			offer, _, err := DecodePollResponse([]byte(test.data))
 			So(err, ShouldHaveSameTypeAs, test.err)
+			So(offer, ShouldResemble, test.offer)
 		}
 
 	})
@@ -138,16 +138,18 @@ func TestDecodeProxyPollResponse(t *testing.T) {
 
 func TestEncodeProxyPollResponse(t *testing.T) {
 	Convey("Context", t, func() {
-		b, err := EncodePollResponse("fake offer", true)
+		b, err := EncodePollResponse("fake offer", true, "restricted")
 		So(err, ShouldEqual, nil)
-		offer, err := DecodePollResponse(b)
+		offer, natType, err := DecodePollResponse(b)
 		So(offer, ShouldEqual, "fake offer")
+		So(natType, ShouldEqual, "restricted")
 		So(err, ShouldEqual, nil)
 
-		b, err = EncodePollResponse("", false)
+		b, err = EncodePollResponse("", false, "unknown")
 		So(err, ShouldEqual, nil)
-		offer, err = DecodePollResponse(b)
+		offer, natType, err = DecodePollResponse(b)
 		So(offer, ShouldEqual, "")
+		So(natType, ShouldEqual, "unknown")
 		So(err, ShouldEqual, nil)
 	})
 }
diff --git a/proxy/proxy-go_test.go b/proxy/proxy-go_test.go
index 03d7307..168ca25 100644
--- a/proxy/proxy-go_test.go
+++ b/proxy/proxy-go_test.go
@@ -249,7 +249,7 @@ func TestBrokerInteractions(t *testing.T) {
 		Convey("polls broker correctly", func() {
 			var err error
 
-			b, err := messages.EncodePollResponse(sampleOffer, true)
+			b, err := messages.EncodePollResponse(sampleOffer, true, "unknown")
 			So(err, ShouldEqual, nil)
 			broker.transport = &MockTransport{
 				http.StatusOK,
diff --git a/proxy/snowflake.go b/proxy/snowflake.go
index ac67748..a1886c2 100644
--- a/proxy/snowflake.go
+++ b/proxy/snowflake.go
@@ -201,7 +201,7 @@ func (b *Broker) pollOffer(sid string) *webrtc.SessionDescription {
 					log.Printf("error reading broker response: %s", err)
 				} else {
 
-					offer, err := messages.DecodePollResponse(body)
+					offer, _, err := messages.DecodePollResponse(body)
 					if err != nil {
 						log.Printf("error reading broker response: %s", err.Error())
 						log.Printf("body: %s", body)



More information about the tor-commits mailing list