[tor-bugs] #13338 [Tor]: Rewrite tor-fw-helper in Go (or another memory-safe language)

[Tor Bug Tracker & Wiki]

#13338: Rewrite tor-fw-helper in Go (or another memory-safe language)
-----------------------------+------------------------------
     Reporter:  arma         |      Owner:  yawning
         Type:  enhancement  |     Status:  needs_review
     Priority:  minor        |  Milestone:  Tor: unspecified
    Component:  Tor          |    Version:
   Resolution:               |   Keywords:  flashproxy
Actual Points:               |  Parent ID:  #5213
       Points:               |
-----------------------------+------------------------------

Comment (by yawning):

 Commenting on the stuff that doesn't require changes, fixes will happen
 sometime over the weekend, maybe tonight if I decide not to go out.

 Replying to [comment:18 dcf]:
 > I was confused by this in [https://github.com/Yawning/go-fw-
 helper/blob/4ffcbdde7063ac343e7f02a3e31c7243ec0f2bad/natclient/natpmp/packet.go
 natclient/natpmp/packet.go]:
 > {{{
 >       opRespOffset = 128
 >       ...
 >       if h.op != opExternalAddress+opRespOffset {
 > }}}
 > I think it would be more clear that you are checking the most-
 significant bit if it were
 > {{{
 >       opRespFlag = 0x80
 >       ...
 >       if h.op != opExternalAddress|opRespFlag {
 > }}}

 The NAT-PMP RFC defines responses the way it's laid out in the code in
 their packet diagrams (Eg: "OP = 128 + 0"), though I may do what you
 suggested here.

 > What is this stuff at the bottom of some files?
 > {{{
 > var _ packetReq = (*externalAddressReq)(nil)
 > var _ packetReq = (*requestMappingReq)(nil)
 > }}}

 It makes it immediately obvious that the concrete types do not implement
 the base interface, as errors in the file where the concrete types are
 defined.  It's a development/debugging convenience for when the base
 interface changes for any reason to make it immediately obvious that I
 forgot to change the concrete implementations.

 > What happens in [https://github.com/Yawning/go-fw-
 helper/blob/4ffcbdde7063ac343e7f02a3e31c7243ec0f2bad/natclient/natpmp/getgateway_windows.go
 natclient/natpmp/getgateway_windows.go] if the primary route is IPv6? It
 looks like it at least won't crash; does it still do anything meaningful?

 I'm not sure here as MSDN doesn't say what happens.  I assume an error is
 returned since the function definition and data structures involved all
 specify IP addresses as `DWORDS`s.  NAT-PMP as a protocol explicitly does
 not support IPv6 (PCP does, but the protocol is more complex), so any
 behavior here including (unlikely) getting a garbage return address is ok
 since the actual discovery will fail.

 > Technically, it seems like [https://github.com/Yawning/go-fw-
 helper/blob/4ffcbdde7063ac343e7f02a3e31c7243ec0f2bad/natclient/upnp/httpu/httpu.go
 httpu] should do [https://golang.org/pkg/net/http/#RoundTripper RoundTrip]
 rather than [https://golang.org/pkg/net/http/#Client.Do Do], because
 RoundTrip is for a single transaction and Do "follows policy (e.g.
 redirects, cookies, auth)." That is, you might have to do less HTTP if you
 introduce the UDP stuff at a lower layer (RoundTripper rather than
 Client). But if it's at all awkward to do, then it doesn't matter; I'm
 sure what's there will work perfectly well.

 httpu's `Do` returns a slice of responses while the runtime's doesn't as
 well, with the idea that the caller knows how to deal with getting
 multiple responses, so the choice of `Do` here is more of a "trying to
 make it easy to understand what is going on" rather than "implementing an
 interface".  I think things will be awkward regardless of what I do here,
 so I'm inclined to leave it as is.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/13338#comment:19>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online