Re: [tor-bugs] #19026 [Circumvention/Snowflake]: Remove local LAN address ICE candidates

[Tor Bug Tracker & Wiki]

#19026: Remove local LAN address ICE candidates
-+-
 Reporter:  dcf  |  Owner:  arlolra
 Type:  enhancement  | Status:  closed
 Priority:  Medium   |  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:  fixed
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:  cohosh   |Sponsor:
-+-
Changes (by arlolra):

 * status:  merge_ready => closed
 * resolution:   => fixed


Comment:

 Merged as,
 https://gitweb.torproject.org/pluggable-
 transports/snowflake.git/commit/?id=670e4ba4380b3fa5cf82043559dcb8c2ca790a7d
 https://gitweb.torproject.org/pluggable-
 transports/snowflake.git/commit/?id=1867f89562fb25bf9a3c2172a7b6f0a198c81adb

 > I guess for browser-based proxies that depends on how much effort it is.
 I'm okay with making it lower priority, but if that's the case let's
 either leave this ticket open or make a new ticket to track it.

 Ok, it's probably not too great an effort, just not really a priority.  I
 filed #33744 for that.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19026 [Circumvention/Snowflake]: Remove local LAN address ICE candidates

[Tor Bug Tracker & Wiki]

#19026: Remove local LAN address ICE candidates
-+--
 Reporter:  dcf  |  Owner:  arlolra
 Type:  enhancement  | Status:  needs_review
 Priority:  Medium   |  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:  cohosh   |Sponsor:
-+--

Comment (by cohosh):

 LGTM. I guess for browser-based proxies that depends on how much effort it
 is. I'm okay with making it lower priority, but if that's the case let's
 either leave this ticket open or make a new ticket to track it.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19026 [Circumvention/Snowflake]: Remove local LAN address ICE candidates

[Tor Bug Tracker & Wiki]

#19026: Remove local LAN address ICE candidates
-+-
 Reporter:  dcf  |  Owner:  arlolra
 Type:  enhancement  | Status:  merge_ready
 Priority:  Medium   |  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:  cohosh   |Sponsor:
-+-
Changes (by cohosh):

 * status:  needs_review => merge_ready


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19026 [Circumvention/Snowflake]: Remove local LAN address ICE candidates

[Tor Bug Tracker & Wiki]

#19026: Remove local LAN address ICE candidates
-+--
 Reporter:  dcf  |  Owner:  arlolra
 Type:  enhancement  | Status:  needs_review
 Priority:  Medium   |  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:  cohosh   |Sponsor:
-+--
Changes (by arlolra):

 * status:  assigned => needs_review


Comment:

 > What's left to do is a follow up for the proxies, as suggested in
 comment:10

 Here's a branch for proxy-go,
 https://github.com/keroserene/snowflake/commits/lan

 Is it worth the effort for the JS proxies?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19026 [Circumvention/Snowflake]: Remove local LAN address ICE candidates

[Tor Bug Tracker & Wiki]

#19026: Remove local LAN address ICE candidates
-+--
 Reporter:  dcf  |  Owner:  arlolra
 Type:  enhancement  | Status:  assigned
 Priority:  Medium   |  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:  cohosh   |Sponsor:
-+--
Changes (by arlolra):

 * status:  needs_review => assigned


Comment:

 Ok, merged starting at,
 https://gitweb.torproject.org/pluggable-
 transports/snowflake.git/commit/?id=0fae4ee8ea487c3b4384217e193e5b9a9088e7de

 What's left to do is a follow up for the proxies, as suggested in
 comment:10

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19026 [Circumvention/Snowflake]: Remove local LAN address ICE candidates

[Tor Bug Tracker & Wiki]

#19026: Remove local LAN address ICE candidates
-+--
 Reporter:  dcf  |  Owner:  arlolra
 Type:  enhancement  | Status:  needs_review
 Priority:  Medium   |  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:  cohosh   |Sponsor:
-+--

Comment (by dcf):

 Replying to [comment:17 arlolra]:
 > It could, but since `offer *webrtc.SessionDescription` comes from a call
 to `pc.LocalDescription()`, I didn't want to invalidate the cached parsed
 description in that structure,
 > https://github.com/pion/webrtc/blob/master/sessiondescription.go#L10-L13
 >
 > You could skip it, yes, but I felt the cheap string check was preferable
 attempting a parse,
 >
 
https://github.com/pion/sdp/blob/03441e3c706c7c3b719ee75194049a31cbb2eb7e/common_description.go#L112-L122

 Okay, works for me.

 > Yeah, that was ugly.  I pushed commit for this suggestion,
 >
 
https://github.com/keroserene/snowflake/commit/edd53af92ac868cf3ba57988e14de887f088a47b

 Everything looks good from my point of view.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19026 [Circumvention/Snowflake]: Remove local LAN address ICE candidates

[Tor Bug Tracker & Wiki]

#19026: Remove local LAN address ICE candidates
-+--
 Reporter:  dcf  |  Owner:  arlolra
 Type:  enhancement  | Status:  needs_review
 Priority:  Medium   |  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:  cohosh   |Sponsor:
-+--

Comment (by arlolra):

 Replying to [comment:16 dcf]:
 > Could
 > {{{
 >   if !bc.keepLocalAddresses {
 >   offer = {
 >   Type: offer.Type,
 >   SDP:  stripLocalAddresses(offer.SDP),
 >   }
 >   }
 > }}}
 > be instead
 > {{{
 >   if !bc.keepLocalAddresses {
 >   offer.SDP = stripLocalAddresses(offer.SDP)
 >   }
 > }}}
 > ?

 It could, but since `offer *webrtc.SessionDescription` comes from a call
 to `pc.LocalDescription()`, I didn't want to invalidate the cached parsed
 description in that structure,
 https://github.com/pion/webrtc/blob/master/sessiondescription.go#L10-L13


 > https://github.com/keroserene/snowflake/compare/trac19026#diff-
 0f3a063993ea3b440ad2ce0abb6ac195R105
 >
 > {{{
 >   if a.IsICECandidate() {
 > }}}
 >
 > Is it necessary to test `IsICECandidate`, or could you skip it and just
 check the `err` result of `ToICECandidate`?

 You could skip it, yes, but I felt the cheap string check was preferable
 attempting a parse,
 
https://github.com/pion/sdp/blob/03441e3c706c7c3b719ee75194049a31cbb2eb7e/common_description.go#L112-L122

 > 
 >
 > The attributes loop is structured like this, with `attrs = append(attrs,
 a)` in three places:
 > {{{
 > for a in 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) {
 >   /* no append in this case */
 >   continue
 >   }
 >   }
 >   }
 >   attrs = append(attrs, a)
 > }
 > }}}
 >
 > Consider restructuring so that you only `continue` in the "skip" case,
 and all other cases fall through to `attrs = append(attrs, a)`. Expressing
 the logic: if a candidate, and type=="host", and an IP address, and IP
 address is local, skip; otherwise keep.
 > {{{
 > for a in attributes {
 >   if a.IsICECandidate() {
 >   ice, err = a.ToICECandidate()
 >   if err == nil && ice.Typ == "host" {
 >   ip = net.ParseIP(ice.Address)
 >   if ip != nil && IsLocal(ip) {
 >   /* no append in this case */
 >   continue
 >   }
 >   }
 >   }
 >   attrs = append(attrs, a)
 > }
 > }}}
 >
 > But also possibly with my note about `ToICECandidate` above:
 > {{{
 > for a in attributes {
 >   if ice, err = a.ToICECandidate(); err == nil {
 >   if ice.Typ == "host" {
 >   ip = net.ParseIP(ice.Address)
 >   if ip != nil && IsLocal(ip) {
 >   /* no append in this case */
 >   continue
 >   }
 >   }
 >   }
 >   attrs = append(attrs, a)
 > }
 > }}}

 Yeah, that was ugly.  I pushed commit for this suggestion,
 
https://github.com/keroserene/snowflake/commit/edd53af92ac868cf3ba57988e14de887f088a47b

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19026 [Circumvention/Snowflake]: Remove local LAN address ICE candidates

[Tor Bug Tracker & Wiki]

#19026: Remove local LAN address ICE candidates
-+--
 Reporter:  dcf  |  Owner:  arlolra
 Type:  enhancement  | Status:  needs_review
 Priority:  Medium   |  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:  cohosh   |Sponsor:
-+--

Comment (by dcf):

 Replying to [comment:14 arlolra]:
 > Here's a branch for this now,
 > https://github.com/keroserene/snowflake/compare/trac19026

 https://github.com/keroserene/snowflake/compare/trac19026#diff-
 0f3a063993ea3b440ad2ce0abb6ac195R144

 Could
 {{{
 if !bc.keepLocalAddresses {
 offer = {
 Type: offer.Type,
 SDP:  stripLocalAddresses(offer.SDP),
 }
 }
 }}}
 be instead
 {{{
 if !bc.keepLocalAddresses {
 offer.SDP = stripLocalAddresses(offer.SDP)
 }
 }}}
 ?

 https://github.com/keroserene/snowflake/compare/trac19026#diff-
 0f3a063993ea3b440ad2ce0abb6ac195R105

 {{{
 if a.IsICECandidate() {
 }}}

 Is it necessary to test `IsICECandidate`, or could you skip it and just
 check the `err` result of `ToICECandidate`?

 

 The attributes loop is structured like this, with `attrs = append(attrs,
 a)` in three places:
 {{{
 for a in 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) {
 /* no append in this case */
 continue
 }
 }
 }
 attrs = append(attrs, a)
 }
 }}}

 Consider restructuring so that you only `continue` in the "skip" case, and
 all other cases fall through to `attrs = append(attrs, a)`. Expressing the
 logic: if a candidate, and type=="host", and an IP address, and IP address
 is local, skip; otherwise keep.
 {{{
 for a in attributes {
 if a.IsICECandidate() {
 ice, err = a.ToICECandidate()
 if err == nil && ice.Typ == "host" {
 ip = net.ParseIP(ice.Address)
 if ip != nil && IsLocal(ip) {
 /* no append in this case */
 continue
 }
 }
 }
 attrs = append(attrs, a)
 }
 }}}

 But also possibly with my note about `ToICECandidate` above:
 {{{
 for a in attributes {
 if ice, err = a.ToICECandidate(); err == nil {
 if ice.Typ == "host" {
 ip = net.ParseIP(ice.Address)
 if ip != nil && IsLocal(ip) {
 /* no append in this case */
 continue
 }
 }
 }
 attrs = append(attrs, a)
 }
 }}}

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19026 [Circumvention/Snowflake]: Remove local LAN address ICE candidates

[Tor Bug Tracker & Wiki]

#19026: Remove local LAN address ICE candidates
-+--
 Reporter:  dcf  |  Owner:  arlolra
 Type:  enhancement  | Status:  needs_review
 Priority:  Medium   |  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:  cohosh   |Sponsor:
-+--
Changes (by arlolra):

 * status:  merge_ready => needs_review


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19026 [Circumvention/Snowflake]: Remove local LAN address ICE candidates

[Tor Bug Tracker & Wiki]

#19026: Remove local LAN address ICE candidates
-+-
 Reporter:  dcf  |  Owner:  arlolra
 Type:  enhancement  | Status:  merge_ready
 Priority:  Medium   |  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:  cohosh   |Sponsor:
-+-

Comment (by arlolra):

 > However, I missed that Attributes are public on the SessionDescription,
 and can be manipulated directly.

 I think I must have confused `sdp.SessionDescription` with
 `webrtc.SessionDescription`

 Thanks for catching that, dcf

 > Will fix, thanks.

 Here's a branch for this now,
 https://github.com/keroserene/snowflake/compare/trac19026

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19026 [Circumvention/Snowflake]: Remove local LAN address ICE candidates

[Tor Bug Tracker & Wiki]

#19026: Remove local LAN address ICE candidates
-+-
 Reporter:  dcf  |  Owner:  arlolra
 Type:  enhancement  | Status:  merge_ready
 Priority:  Medium   |  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:  cohosh   |Sponsor:
-+-

Comment (by arlolra):

 > Sorry to bring it up at this late stage, but the pion/sdp package is an
 alternative to string operations.

 The patch already uses that parse candidate lines,
 
https://github.com/keroserene/snowflake/commit/dbd133b6e1196e4ec7550f0ebb52854cb00d1007
 #diff-0f3a063993ea3b440ad2ce0abb6ac195R101

 However, I missed that Attributes are public on the SessionDescription,
 and can be manipulated directly.

 Will fix, thanks.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19026 [Circumvention/Snowflake]: Remove local LAN address ICE candidates

[Tor Bug Tracker & Wiki]

#19026: Remove local LAN address ICE candidates
-+-
 Reporter:  dcf  |  Owner:  arlolra
 Type:  enhancement  | Status:  merge_ready
 Priority:  Medium   |  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:  cohosh   |Sponsor:
-+-

Comment (by dcf):

 Sorry to bring it up at this late stage, but the
 [https://github.com/pion/sdp pion/sdp] package is an alternative to string
 operations. There's a
 [https://godoc.org/github.com/pion/sdp#SessionDescription
 SessionDescription] type with
 [https://godoc.org/github.com/pion/sdp#SessionDescription.Marshal Marshal]
 and [https://godoc.org/github.com/pion/sdp#SessionDescription.Unmarshal
 Unmarshal] methods, containing an array of
 [https://godoc.org/github.com/pion/sdp#Attribute Attribute]s that each
 have a [https://godoc.org/github.com/pion/sdp#Attribute.ToICECandidate
 ToICECandidate] method.

 Replying to [comment:6 cohosh]:
 > - {{{FIXME: Should this check ip.IsLoopback() and others?}}} I'd like to
 include `0.0.0.0` and `127.0.0.1` addresses in this, especially after dcf
 found #33157.

 I feel that the 0.0.0.0 is unrelated to anything involving candidate
 addresses. 0.0.0.0 is appearing in the `o=` origin field and the `c=`
 connection information field (the latter is what `remoteIPFromSDP` looks
 at), not in an `a=candidate` attribute.

 However, I imagine that if a LAN address appears as a `a=candidate`, that
 may also cause it to appear in `o=` or `c=`, and we may have to
 additionally deal with that when #33157 is figured out.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19026 [Circumvention/Snowflake]: Remove local LAN address ICE candidates

[Tor Bug Tracker & Wiki]

#19026: Remove local LAN address ICE candidates
-+-
 Reporter:  dcf  |  Owner:  arlolra
 Type:  enhancement  | Status:  merge_ready
 Priority:  Medium   |  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:  cohosh   |Sponsor:
-+-

Comment (by cohosh):

 Replying to [comment:9 arlolra]:
 > > especially after dcf found #33157.
 >
 > Hmm, after reading that, I noticed we have,
 > https://github.com/keroserene/snowflake/blob/master/proxy-
 go/snowflake.go#L60-L70
 >
 > I should probably consolidate that with some of the nastiness added here
 to grep the sdp.

 That's a good idea. We can discuss this on #33157.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19026 [Circumvention/Snowflake]: Remove local LAN address ICE candidates

[Tor Bug Tracker & Wiki]

#19026: Remove local LAN address ICE candidates
-+-
 Reporter:  dcf  |  Owner:  arlolra
 Type:  enhancement  | Status:  merge_ready
 Priority:  Medium   |  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:  cohosh   |Sponsor:
-+-
Changes (by cohosh):

 * status:  needs_review => merge_ready


Comment:

 These changes looks good to me. Let's merge this and follow up with a port
 to proxies as you mentioned. Thanks!

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19026 [Circumvention/Snowflake]: Remove local LAN address ICE candidates

[Tor Bug Tracker & Wiki]

#19026: Remove local LAN address ICE candidates
-+--
 Reporter:  dcf  |  Owner:  arlolra
 Type:  enhancement  | Status:  needs_review
 Priority:  Medium   |  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:  cohosh   |Sponsor:
-+--

Comment (by arlolra):

 > especially after dcf found #33157.

 Hmm, after reading that, I noticed we have,
 https://github.com/keroserene/snowflake/blob/master/proxy-
 go/snowflake.go#L60-L70

 I should probably consolidate that with some of the nastiness added here
 to grep the sdp.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19026 [Circumvention/Snowflake]: Remove local LAN address ICE candidates

[Tor Bug Tracker & Wiki]

#19026: Remove local LAN address ICE candidates
-+--
 Reporter:  dcf  |  Owner:  arlolra
 Type:  enhancement  | Status:  needs_review
 Priority:  Medium   |  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:  cohosh   |Sponsor:
-+--
Changes (by arlolra):

 * status:  needs_revision => needs_review


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19026 [Circumvention/Snowflake]: Remove local LAN address ICE candidates

[Tor Bug Tracker & Wiki]

#19026: Remove local LAN address ICE candidates
-+
 Reporter:  dcf  |  Owner:  arlolra
 Type:  enhancement  | Status:  needs_revision
 Priority:  Medium   |  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:  cohosh   |Sponsor:
-+

Comment (by arlolra):

 Ok, new commit at
 
https://github.com/keroserene/snowflake/commit/dbd133b6e1196e4ec7550f0ebb52854cb00d1007

 > Some comments:
 >
 > - {{{FIXME: Should this check ip.IsLoopback() and others?}}} I'd like to
 include `0.0.0.0` and `127.0.0.1` addresses in this, especially after dcf
 found #33157.

 The condition now looks like `IsLocal(ip) || ip.IsUnspecified() ||
 ip.IsLoopback()`, which includes these addresses.

 > - Let's expand the tests and include one for each type of local address

 Done

 > - It's worth implementing this for each of the proxies as well.

 Can this be a follow up?

 > - Is there a way for us to use the other built-in functions in the `net`
 package for determining whether or not the IP address is local?

 Not that I'm aware of.  The included function was referenced from,
 https://github.com/golang/go/issues/29146

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19026 [Circumvention/Snowflake]: Remove local LAN address ICE candidates

[Tor Bug Tracker & Wiki]

#19026: Remove local LAN address ICE candidates
-+
 Reporter:  dcf  |  Owner:  arlolra
 Type:  enhancement  | Status:  needs_revision
 Priority:  Medium   |  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:  cohosh   |Sponsor:
-+
Changes (by cohosh):

 * status:  needs_review => needs_revision


Comment:

 Thanks for working on this! Thanks for keeping the option to leave the
 local addresses in as well.

 That's unfortunate that the `"public"` `RTCIceTransportPolicy` was removed
 from the specification. It would be nicer if we could prevent the
 candidate from being included in the SDP instead of grepping for it
 afterwards, but I don't see a way to do that with the `OnICECandidate`
 callback.

 Some comments:

 - {{{FIXME: Should this check ip.IsLoopback() and others?}}} I'd like to
 include `0.0.0.0` and `127.0.0.1` addresses in this, especially after dcf
 found #33157.

 - Let's expand the tests and include one for each type of local address

 - It's worth implementing this for each of the proxies as well.

 - Is there a way for us to use the other built-in functions in the `net`
 package for determining whether or not the IP address is local?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19026 [Circumvention/Snowflake]: Remove local LAN address ICE candidates

[Tor Bug Tracker & Wiki]

#19026: Remove local LAN address ICE candidates
-+--
 Reporter:  dcf  |  Owner:  arlolra
 Type:  enhancement  | Status:  needs_review
 Priority:  Medium   |  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:  cohosh   |Sponsor:
-+--
Changes (by cohosh):

 * reviewer:   => cohosh


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19026 [Circumvention/Snowflake]: Remove local LAN address ICE candidates

[Tor Bug Tracker & Wiki]

#19026: Remove local LAN address ICE candidates
-+--
 Reporter:  dcf  |  Owner:  arlolra
 Type:  enhancement  | Status:  needs_review
 Priority:  Medium   |  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+--
Changes (by arlolra):

 * cc: cohosh, phw (added)


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19026 [Circumvention/Snowflake]: Remove local LAN address ICE candidates

[Tor Bug Tracker & Wiki]

#19026: Remove local LAN address ICE candidates
-+--
 Reporter:  dcf  |  Owner:  arlolra
 Type:  enhancement  | Status:  needs_review
 Priority:  Medium   |  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+--
Changes (by arlolra):

 * status:  assigned => needs_review


Comment:

 Here's an attempt at this,
 
https://github.com/keroserene/snowflake/commit/71934f1db34bec354a94266c79f2e631be604a03

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19026 [Circumvention/Snowflake]: Remove local LAN address ICE candidates

[Tor Bug Tracker & Wiki]

#19026: Remove local LAN address ICE candidates
-+--
 Reporter:  dcf  |  Owner:  arlolra
 Type:  enhancement  | Status:  assigned
 Priority:  Medium   |  Milestone:
Component:  Circumvention/Snowflake  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+--
Changes (by arlolra):

 * status:  new => assigned
 * owner:  (none) => arlolra


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs