[tor-commits] [snowflake/master] Unmarshal the SDP to filter attributes

arlo at torproject.org arlo at torproject.org
Sat Feb 8 15:14:45 UTC 2020


commit 846473b3549c264245354a3117d87b96a945f287
Author: Arlo Breault <arlolra at gmail.com>
Date:   Fri Feb 7 15:39:44 2020 -0500

    Unmarshal the SDP to filter attributes
    
    Instead of string manipulation.
---
 client/lib/lib_test.go   | 18 +++++++-------
 client/lib/rendezvous.go | 64 ++++++++++++++++++++++++++++--------------------
 2 files changed, 46 insertions(+), 36 deletions(-)

diff --git a/client/lib/lib_test.go b/client/lib/lib_test.go
index adfc9ec..b413508 100644
--- a/client/lib/lib_test.go
+++ b/client/lib/lib_test.go
@@ -358,17 +358,17 @@ func TestSnowflakeClient(t *testing.T) {
 	})
 
 	Convey("Strip", t, func() {
-		const offerStart = `{"type":"offer","sdp":"v=0\r\no=- 4358805017720277108 2 IN IP4 8.8.8.8\r\ns=-\r\nt=0 0\r\na=group:BUNDLE data\r\na=msid-semantic: WMS\r\nm=application 56688 DTLS/SCTP 5000\r\nc=IN IP4 8.8.8.8\r\n`
-		const goodCandidate = `a=candidate:3769337065 1 udp 2122260223 8.8.8.8 56688 typ host generation 0 network-id 1 network-cost 50\r\n`
-		const offerEnd = `a=ice-ufrag:aMAZ\r\na=ice-pwd:jcHb08Jjgrazp2dzjdrvPPvV\r\na=ice-options:trickle\r\na=fingerprint:sha-256 C8:88:EE:B9:E7:02:2E:21:37:ED:7A:D1:EB:2B:A3:15:A2:3B:5B:1C:3D:D4:D5:1F:06:CF:52:40:03:F8:DD:66\r\na=setup:actpass\r\na=mid:data\r\na=sctpmap:5000 webrtc-datachannel 1024\r\n"}`
+		const offerStart = "v=0\r\no=- 4358805017720277108 2 IN IP4 8.8.8.8\r\ns=-\r\nt=0 0\r\na=group:BUNDLE data\r\na=msid-semantic: WMS\r\nm=application 56688 DTLS/SCTP 5000\r\nc=IN IP4 8.8.8.8\r\n"
+		const goodCandidate = "a=candidate:3769337065 1 udp 2122260223 8.8.8.8 56688 typ host generation 0 network-id 1 network-cost 50\r\n"
+		const offerEnd = "a=ice-ufrag:aMAZ\r\na=ice-pwd:jcHb08Jjgrazp2dzjdrvPPvV\r\na=ice-options:trickle\r\na=fingerprint:sha-256 C8:88:EE:B9:E7:02:2E:21:37:ED:7A:D1:EB:2B:A3:15:A2:3B:5B:1C:3D:D4:D5:1F:06:CF:52:40:03:F8:DD:66\r\na=setup:actpass\r\na=mid:data\r\na=sctpmap:5000 webrtc-datachannel 1024\r\n"
 
 		offer := offerStart + goodCandidate +
-			`a=candidate:3769337065 1 udp 2122260223 192.168.0.100 56688 typ host generation 0 network-id 1 network-cost 50\r\n` + // IsLocal IPv4
-			`a=candidate:3769337065 1 udp 2122260223 fdf8:f53b:82e4::53 56688 typ host generation 0 network-id 1 network-cost 50\r\n` + // IsLocal IPv6
-			`a=candidate:3769337065 1 udp 2122260223 0.0.0.0 56688 typ host generation 0 network-id 1 network-cost 50\r\n` + // IsUnspecified IPv4
-			`a=candidate:3769337065 1 udp 2122260223 :: 56688 typ host generation 0 network-id 1 network-cost 50\r\n` + // IsUnspecified IPv6
-			`a=candidate:3769337065 1 udp 2122260223 127.0.0.1 56688 typ host generation 0 network-id 1 network-cost 50\r\n` + // IsLoopback IPv4
-			`a=candidate:3769337065 1 udp 2122260223 ::1 56688 typ host generation 0 network-id 1 network-cost 50\r\n` + // IsLoopback IPv6
+			"a=candidate:3769337065 1 udp 2122260223 192.168.0.100 56688 typ host generation 0 network-id 1 network-cost 50\r\n" + // IsLocal IPv4
+			"a=candidate:3769337065 1 udp 2122260223 fdf8:f53b:82e4::53 56688 typ host generation 0 network-id 1 network-cost 50\r\n" + // IsLocal IPv6
+			"a=candidate:3769337065 1 udp 2122260223 0.0.0.0 56688 typ host generation 0 network-id 1 network-cost 50\r\n" + // IsUnspecified IPv4
+			"a=candidate:3769337065 1 udp 2122260223 :: 56688 typ host generation 0 network-id 1 network-cost 50\r\n" + // IsUnspecified IPv6
+			"a=candidate:3769337065 1 udp 2122260223 127.0.0.1 56688 typ host generation 0 network-id 1 network-cost 50\r\n" + // IsLoopback IPv4
+			"a=candidate:3769337065 1 udp 2122260223 ::1 56688 typ host generation 0 network-id 1 network-cost 50\r\n" + // IsLoopback IPv6
 			offerEnd
 
 		So(stripLocalAddresses(offer), ShouldEqual, offerStart+goodCandidate+offerEnd)
diff --git a/client/lib/rendezvous.go b/client/lib/rendezvous.go
index bd8ff00..190df66 100644
--- a/client/lib/rendezvous.go
+++ b/client/lib/rendezvous.go
@@ -17,7 +17,6 @@ import (
 	"net"
 	"net/http"
 	"net/url"
-	"regexp"
 
 	"github.com/pion/sdp"
 	"github.com/pion/webrtc"
@@ -95,25 +94,40 @@ func IsLocal(ip net.IP) bool {
 
 // Removes local LAN address ICE candidates
 func stripLocalAddresses(str string) string {
-	re := regexp.MustCompile(`a=candidate:.*?\\r\\n`)
-	return re.ReplaceAllStringFunc(str, func(s string) string {
-		t := s[len("a=candidate:") : len(s)-len("\\r\\n")]
-		var ice sdp.ICECandidate
-		err := ice.Unmarshal(t)
-		if err != nil {
-			return s
-		}
-		if ice.Typ == "host" {
-			ip := net.ParseIP(ice.Address)
-			if ip == nil {
-				return s
-			}
-			if IsLocal(ip) || ip.IsUnspecified() || ip.IsLoopback() {
-				return ""
+	var desc sdp.SessionDescription
+	err := desc.Unmarshal([]byte(str))
+	if err != nil {
+		return str
+	}
+	for _, m := range desc.MediaDescriptions {
+		attrs := make([]sdp.Attribute, 0)
+		for _, a := range m.Attributes {
+			if a.IsICECandidate() {
+				ice, err := a.ToICECandidate()
+				if err != nil {
+					attrs = append(attrs, a)
+					continue
+				}
+				if ice.Typ == "host" {
+					ip := net.ParseIP(ice.Address)
+					if ip == nil {
+						attrs = append(attrs, a)
+						continue
+					}
+					if IsLocal(ip) || ip.IsUnspecified() || ip.IsLoopback() {
+						continue
+					}
+				}
 			}
+			attrs = append(attrs, a)
 		}
-		return s
-	})
+		m.Attributes = attrs
+	}
+	bts, err := desc.Marshal()
+	if err != nil {
+		return str
+	}
+	return string(bts)
 }
 
 // Roundtrip HTTP POST using WebRTC SessionDescriptions.
@@ -124,20 +138,16 @@ func (bc *BrokerChannel) Negotiate(offer *webrtc.SessionDescription) (
 	*webrtc.SessionDescription, error) {
 	log.Println("Negotiating via BrokerChannel...\nTarget URL: ",
 		bc.Host, "\nFront URL:  ", bc.url.Host)
-	str := serializeSessionDescription(offer)
 	// Ideally, we could specify an `RTCIceTransportPolicy` that would handle
 	// this for us.  However, "public" was removed from the draft spec.
 	// See https://developer.mozilla.org/en-US/docs/Web/API/RTCConfiguration#RTCIceTransportPolicy_enum
-	//
-	// FIXME: We are stripping local addresses from the JSON serialized string,
-	// which is expedient but unsatisfying.  We could advocate upstream to
-	// implement a non-standard ICE transport policy, or to somehow alter
-	// APIs to avoid adding the undesirable candidates or a method to filter
-	// them from the marshalled session description.
 	if !bc.keepLocalAddresses {
-		str = stripLocalAddresses(str)
+		offer = &webrtc.SessionDescription{
+			Type: offer.Type,
+			SDP:  stripLocalAddresses(offer.SDP),
+		}
 	}
-	data := bytes.NewReader([]byte(str))
+	data := bytes.NewReader([]byte(serializeSessionDescription(offer)))
 	// Suffix with broker's client registration handler.
 	clientURL := bc.url.ResolveReference(&url.URL{Path: "client"})
 	request, err := http.NewRequest("POST", clientURL.String(), data)





More information about the tor-commits mailing list