Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#6244 closed defect (fixed)

MapAddress wildcarding not working right

Reported by: grarpamp Owned by:
Priority: High Milestone: Tor: 0.2.3.x-final
Component: Core Tor/Tor Version: Tor: 0.2.3.19-rc
Severity: Keywords: mapaddress tor-client
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

MapAddress wildcarding does not seem to be working right.
Note the following three issues via the controller...

1) When using the man documented format to enter the map:

mapaddress *.example.com=*.example.com.<fingerprint>.exit
512 syntax error: invalid address '*.example.com.<fingerprint>.exit'

With no map, I did not check the streams.

2) When chancing upon an alternate format that returns 250:

mapaddress *.example.com=<fingerprint>.exit
250 *.example.com=<fingerprint>.exit

This results in these streams. Which appear to leak badly in that
the first stream to a destination does not honor the exit.

172506 NEW 0 www.example.com:443 PURPOSE=USER
172507 NEW 0 www.example.com:443 PURPOSE=USER
172506 SUCCEEDED 121217 64.34.110.174:443
172507 SUCCEEDED 121219 64.34.110.174.<fingerprint>.exit:443

Note that entering the following related map syntax does not
SUCCEED, which is weird given the above streams:

mapaddress foo.bar.com=<fingerprint>.exit

This one does, which is documented expectation:

mapaddress foo.bar.com=foo.bar.com.<fingerprint>.exit

3) The map in (2) is not removable by the syntax of setting the
source (left) equal to (=) the destination (right):

mapaddress *.example.com=*.example.com
512 syntax error: invalid address '*.example.com'

Seems the solution may/should be...

  • Adopt the syntax documented in (1), removing that acting in (2).
  • Fix the stream issue shown in (2).
  • Allow removal by syntax in (3).
  • Document the map removal syntax in the control spec page.
  • Check that removal from torrc and sighup works (as that is the

form of the various options in the man page).

  • Lint the inputted map to reject '=<fingerprint>.exit' form.

Adding applicable environment...

trackhostexits .
trackhostexitsexpire 604800
allowdotexit 1
signal NEWNYM

Child Tickets

Change History (17)

comment:1 Changed 7 years ago by nickm

Keywords: mapaddress added
Priority: normalmajor

Looks like the "invalid address '%s'" warning is in handle_control_mapaddress() in particular. The first step debugging here is probably to figure out how much of this is controller-specific, and how much applies to all MapAddress usage.

Item 1 and Item 3 above look very much like they could be fixed by removing the address_is_invalid_destination() check in that function, or by teaching it about *.xxxx and * as possible destinations.

Item 2 looks like it might be a case of #6211, though I can't be cure; testing it on maint-0.2.3 might turn up some more info.

comment:2 Changed 7 years ago by grarpamp

In 0.2.3.17-beta, we fixed #3940, which was related to MaPAddress and .exit. That introduced/exposed another bug in that code, #6211, which should be fixed in git.
Could you see how much of this you can reproduce on the current Tor maint-0.2.3 branch?

Queued...

comment:3 Changed 7 years ago by grarpamp

On maint-0.2.3 6330d2d now.
Well, either it, or me, is quite messed up :) Hope this covers it.

With the same torrc as above, a fresh exec of tor per item, and
giving three successive invocations of...
curl --socks5-hostname 127.0.0.1:9050 -o /dev/null -s http://www.openbsd.org/

I grouped setevents stream and address-mappings/all output by
invocation.

Not sure why the map expiry times differ. Such as today, vs. a week
away. Nor about the 3hr offsets.

Not sure why each invocation grows additional maps.

It's mostly the same, just noted more bugs and added streams to (1).

Items...

1) With no map we have...

151 NEW 0 www.openbsd.org:80 SOURCE_ADDR=127.0.0.1:60019 PURPOSE=USER
151 SENTCONNECT 41 www.openbsd.org:80
151 REMAP 41 142.244.12.42:80 SOURCE=EXIT
151 SUCCEEDED 41 142.244.12.42:80
151 CLOSED 41 142.244.12.42:80 REASON=DONE
www.openbsd.org www.openbsd.org._FP1_.exit "2012-07-04 03:18:45"

153 NEW 0 www.openbsd.org:80 SOURCE_ADDR=127.0.0.1:55073 PURPOSE=USER
153 REMAP 0 www.openbsd.org.___FP1___.exit:80 SOURCE=CACHE
153 SENTCONNECT 41 www.openbsd.org.___FP1___.exit:80
153 REMAP 41 142.244.12.42._FP1___.exit:80 SOURCE=EXIT
153 SUCCEEDED 41 142.244.12.42.
_FP1___.exit:80
153 CLOSED 41 142.244.12.42._FP1___.exit:80 REASON=DONE
www.openbsd.org www.openbsd.org.
_FP1_.exit "2012-07-04 03:19:02"
www.openbsd.org.
_FP1_.exit 142.244.12.42._FP1_.exit "2012-06-27 06:19:02"

155 NEW 0 www.openbsd.org:80 SOURCE_ADDR=127.0.0.1:18968 PURPOSE=USER
155 REMAP 0 142.244.12.42._FP1___.exit:80 SOURCE=CACHE
155 SENTCONNECT 41 142.244.12.42.
_FP1___.exit:80
155 REMAP 41 142.244.12.42._FP1___.exit:80 SOURCE=EXIT
155 SUCCEEDED 41 142.244.12.42.
_FP1___.exit:80
155 CLOSED 41 142.244.12.42._FP1___.exit:80 REASON=DONE
www.openbsd.org www.openbsd.org.
_FP1_.exit "2012-07-04 03:19:02"
www.openbsd.org.
_FP1_.exit 142.244.12.42._FP1_.exit "2012-06-27 06:19:02"
142.244.12.42 142.244.12.42.
_FP1_.exit "2012-07-04 03:19:13"

2) With the undocumented broken format we have...

mapaddress *.openbsd.org=_FP2_.exit

Mapaddress is not honored, otherwise same as in (1). Except add
this to the mapping output:

*.openbsd.org _FP2_.exit NEVER

3) Removal of (2) fails...

mapaddress *.openbsd.org=*.openbsd.org

512 syntax error: invalid address '*.openbsd.org'
[warn] Skipping invalid argument '*.openbsd.org' in MapAddress msg

4) Addition (documented format) fails...

mapaddress *.openbsd.org=*.openbsd.org._FP2_.exit

512 syntax error: invalid address
[warn] Skipping invalid argument '*.openbsd.org._FP2_.exit' in MapAddress msg

5) When this is in torrc (stars)...

mapaddress *.openbsd.org *.openbsd.org._FP2_.exit

this appears in the map (no stars)...

openbsd.org openbsd.org._FP2_.exit NEVER

when I'd expect exactly what is in the torrc to appear.

However the map is then honored...

106 NEW 0 www.openbsd.org:80 SOURCE_ADDR=127.0.0.1:63567 PURPOSE=USER
106 REMAP 0 www.openbsd.org.___FP2___.exit:80 SOURCE=CACHE
106 SENTCONNECT 38 www.openbsd.org.___FP2___.exit:80
106 REMAP 38 142.244.12.42._FP2___.exit:80 SOURCE=EXIT
106 SUCCEEDED 38 142.244.12.42.
_FP2___.exit:80
106 CLOSED 38 142.244.12.42._FP2___.exit:80 REASON=DONE
openbsd.org openbsd.org.
_FP2_.exit NEVER
www.openbsd.org www.openbsd.org.
_FP2_.exit "2012-07-04 05:41:34"
www.openbsd.org.
_FP2_.exit 142.244.12.42._FP2_.exit "2012-06-27 08:41:38"

And it still grows this map line on next invocation...

142.244.12.42 142.244.12.42._FP2_.exit "2012-07-04 05:41:55"

And can be removed with 'mapaddress openbsd.org=openbsd.org'. Which
isn't what we want, as that might be a dynamic or a more specific
manual static entry. So we really want the stars on this one when
it's all fixed.

6) A sighup removes and adds based on torrc. But [re] adding (5)
does not honor the map while the dynamic map entries from the former
invocation are still there. ie: sighup doesn't seem to 'signal
NEWNYM' to clear them out.

comment:4 Changed 7 years ago by grarpamp

Specific maps still work fine, which are what I've been using till now.

mapaddress www.openbsd.org www.openbsd.org._FP_.exit NEVER
www.openbsd.org._FP_.exit 142.244.12.42._FP_.exit "2012-06-27 09:25:10"
142.244.12.42 142.244.12.42._FP_.exit "2012-07-04 06:29:12"

comment:5 Changed 7 years ago by nickm

Status: newneeds_review

See branch bug6244_part1 in my public repository for a fix for the part of this involving conrollers not accepting MapAddress targets starting with *.

comment:6 Changed 7 years ago by nickm

Status: needs_reviewnew

arma liked the fix ok, so merging it.

comment:7 Changed 7 years ago by nickm

Status: newneeds_review

I note in passing, BTW, that this is one ticket with 3 numbered issues with a followup comment with 6 numbered issues. Hard to keep track of! The commit above should fix 3 and 4 in your most recent comment.

The part of 5 about the output format should be fixed in a new "bug6244_part2" branch in my public repository.

The rest of 5 and 6 needs more investigating.

comment:8 in reply to:  7 Changed 7 years ago by arma

Replying to nickm:

a new "bug6244_part2" branch in my public repository.

Looks cursorily ok.

comment:9 Changed 7 years ago by grarpamp

There appear no way I can edit what I post, otherwise I would have combined with 1st OP, so ignore all of 1st OP.

Todo of reject 'whatever=<fingerprint>.exit' as bad right side syntax applies for 2nd OP part two (always need something between = and FP). Will probably be byproduct of rest of fixes.

Will have time to test repo in day or two. (note for me: double check stream leak thing).

Add to expiry note: clock and tz set properly, local time is not three hours from UTC.

comment:10 Changed 7 years ago by nickm

Status: needs_reviewnew

Merged bug6244_part2 into maint-0.2.3

comment:11 Changed 7 years ago by grarpamp

git 9c5a118

Fixed...

  • can add * maps via controller.
  • can remove * maps previously entered via controller.
  • * adds via torrc show in controller as *.

Not fixed...

  • * maps do not work at all.

Notes...
Host maps still work.

I don't want to fully lint things till both * (catchall) and host (specific) maps are working.
I am changing allowdotexit to default to 0 until further notice.

Do we need more spec on how all the cases should work?
Where should the items/lint go?

Here is some...

  • Cannot remove * maps entered via torrc unless they are hup'ed out. I think this should be possible. ie: allow to config all but controller access via controller. Leaving hup as convenient reset to torrc 'defaults'.
  • If initial * map is add via torrc, second add of same src-dst map via controller is permitted. I think this should not be possible.
  • Getinfo and streams need to display all FP's as lower or upper case. I think upper will stand out more clearly since hostnames are lower.

comment:12 Changed 7 years ago by grarpamp

detail: * maps via controller fail, * maps via torrc work.

comment:13 Changed 7 years ago by grarpamp

Version: Tor: 0.2.3.17-betaTor: 0.2.3.19-rc

comment:14 in reply to:  12 Changed 7 years ago by nickm

Status: newneeds_review

Replying to grarpamp:

detail: * maps via controller fail, * maps via torrc work.

Ah, that's the key. Review now needed for branch "bug6244_part_c" in my public repository.

comment:15 Changed 7 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Yeah, this still seems to work for me. I think this is the last fix; please reopen if there are remaining issues here. (Or really, if there are multiple remaining issues here, maybe open separate tickets?)

comment:16 Changed 7 years ago by nickm

Keywords: tor-client added

comment:17 Changed 7 years ago by nickm

Component: Tor ClientTor
Note: See TracTickets for help on using tickets.