Setting ORPort [ipv6]:auto mistakenly advertises port 94

Start your Tor with

ORPort 9001
ORPort [2a01:238:43e4:2b00:ae5a:a980:1f63:cc5e]:auto nolisten
DirPort 9030

and let it start up. Then go to http://127.0.0.1:9030/tor/server/authority and check out how it has the line

or-address [2a01:238:43e4:2b00:ae5a:a980:1f63:cc5e]:94

First: How did it pick that number? It's a weird choice for a port.

Second: If this is actually your ipv6 address, you can leave out the nolisten, and that's where things get interesting. Your logs will say something like

Nov 24 01:53:42 v4 tor[10988]: Nov 24 01:53:42.001 [notice] Opening OR listener on [2a01:238:43e4:2b00:ae5a:a980:1f63:cc5e]:0
Nov 24 01:53:42 v4 tor[10988]: Nov 24 01:53:42.002 [notice] Opened OR listener on [2a01:238:43e4:2b00:ae5a:a980:1f63:cc5e]:41535

but then your descriptor will still say :94.

This is happening right now to relay "Testbit": they have set :auto for their ipv6 address in their ORPort, they get the above line about how it's opened on port 41535, and yet their descriptor says it's on port 94.

changed milestone to %Tor: 0.4.3.x-final

Trac:
Parent Ticket: #33048 (moved)

added 035-backport 041-backport 042-backport 043-backport 043-should actualpoints::1.1 component::core tor/tor consider-backport-after-0435 ipv6 milestone::Tor: 0.4.3.x-final network-team-roadmap-2020Q1 owner::teor parent::33048 points::1 priority::medium reviewer::nickm reviewer::teor severity::normal sponsor::55-must status::merge-ready type::defect version::tor 0.2.3.9-alpha labels

last two bytes of ipv6 address parsed as portnumber

0x5e = 94 in decimal :)

I agree that this is a bug.

But another issue is that "auto" and "nolisten" don't make a lot of sense together. "Auto" is "pick a port for me"; "nolisten" is "don't actually listen on this address, I'll set it up in a firewall rule or something".

Replying to nickm:

I agree that this is a bug.

But another issue is that "auto" and "nolisten" don't make a lot of sense together. "Auto" is "pick a port for me"; "nolisten" is "don't actually listen on this address, I'll set it up in a firewall rule or something".

No, the NoListen is just here so you can reproduce the bug. The bug exists when you say :auto on your IPv6 ORPort -- the log tells you what port it picked, and it listens on that port, and then it puts the wrong number in your descriptor.

Trac:
Milestone: N/A to Tor: 0.4.3.x-final

Trac:
Owner: N/A to neel
Cc: N/A to neel
Keywords: N/A deleted, ipv6 added
Status: new to assigned

The IPv6 descriptor code can never work for auto ports: https://github.com/torproject/tor/blob/master/src/feature/relay/router.c#L1991

It should be like the IPv4 descriptor port code: https://github.com/torproject/tor/blob/master/src/feature/relay/router.c#L1978 And call router_get_advertised_or_port_by_af() to get the IPv6 ORPort.

Ideally, we should add a new router_get_advertised_ipv6_or_port() function, which searches for an address like this: https://github.com/torproject/tor/blob/master/src/feature/relay/router.c#L1991

But searches for a port like this: (if the discovered port is 0) router_get_advertised_or_port_by_af(… , AF_INET6).

I still can't work out how the port ends up being 94. Maybe we're overwriting some memory somewhere? I think we should try to find the memory issue, before we call this bug "fixed".

Trac:
Keywords: N/A deleted, security-low, memory-safety added
Status: assigned to needs_information

It's also worth noting that we're getting 0x5e = 94 in the output, and not 0xcc5e or 0x5ecc. So it's just a one byte overflow. And it's happening some time after the port is opened, but before the relay descriptor is built.

And it only seems to affect auto IPv6 ORPorts.

Does the issue still happen if the IPv6 address is much shorter than the maximum length? For example, does "ORPort [::1]:auto" give you a port of 1? You might need to set some custom directory authorities to be allowed to listen on an internal address.

Trac:
Description: Start your Tor with

ORPort 9001
ORPort [2a01:238:43e4:2b00:ae5a:a980:1f63:cc5e]:auto nolisten
DirPort 9030

and let it start up. Then go to http://127.0.0.1/tor/server/authority and check out how it has the line

or-address [2a01:238:43e4:2b00:ae5a:a980:1f63:cc5e]:94

First: How did it pick that number? It's a weird choice for a port.

Second: If this is actually your ipv6 address, you can leave out the nolisten, and that's where things get interesting. Your logs will say something like

Nov 24 01:53:42 v4 tor[10988]: Nov 24 01:53:42.001 [notice] Opening OR listener on [2a01:238:43e4:2b00:ae5a:a980:1f63:cc5e]:0
Nov 24 01:53:42 v4 tor[10988]: Nov 24 01:53:42.002 [notice] Opened OR listener on [2a01:238:43e4:2b00:ae5a:a980:1f63:cc5e]:41535

but then your descriptor will still say :94.

This is happening right now to relay "Testbit": they have set :auto for their ipv6 address in their ORPort, they get the above line about how it's opened on port 41535, and yet their descriptor says it's on port 94.

to

Start your Tor with

ORPort 9001
ORPort [2a01:238:43e4:2b00:ae5a:a980:1f63:cc5e]:auto nolisten
DirPort 9030

and let it start up. Then go to http://127.0.0.1:9030/tor/server/authority and check out how it has the line

or-address [2a01:238:43e4:2b00:ae5a:a980:1f63:cc5e]:94

First: How did it pick that number? It's a weird choice for a port.

Second: If this is actually your ipv6 address, you can leave out the nolisten, and that's where things get interesting. Your logs will say something like

Nov 24 01:53:42 v4 tor[10988]: Nov 24 01:53:42.001 [notice] Opening OR listener on [2a01:238:43e4:2b00:ae5a:a980:1f63:cc5e]:0
Nov 24 01:53:42 v4 tor[10988]: Nov 24 01:53:42.002 [notice] Opened OR listener on [2a01:238:43e4:2b00:ae5a:a980:1f63:cc5e]:41535

but then your descriptor will still say :94.

This is happening right now to relay "Testbit": they have set :auto for their ipv6 address in their ORPort, they get the above line about how it's opened on port 41535, and yet their descriptor says it's on port 94.

Anybody should be able to reproduce the bug with the initial instructions -- set two orports and a dirport, and then you can look at your descriptor on your local dirport.

When I set my ORPort to [::2]:auto it still publishes

or-address [::2]:94

in its descriptor.

Same with [::5] and [::10].

Here's a draft PR:

• don't merge: https://github.com/torproject/tor/pull/1624

I still need to:

• check how far to backport
• make a changes file
• open a separate ticket for the related authority change

I'll attach the test script I was using.

Trac:
Status: needs_information to needs_review

Trac:
relay.sh

Tor test script for IPv6 ORPort auto bug

See my PR:

• 0.3.5: https://github.com/torproject/tor/pull/1625

I'd like an initial review.

I will try to write some unit tests on Monday.

Trac:
Points: N/A to 1
Version: Tor: 0.4.1.6 to Tor: 0.2.3.9-alpha
Actualpoints: N/A to 0.4
Keywords: N/A deleted, 041-backport, 035-backport, 040-backport, 042-backport added

Test branches are here:

• https://github.com/teor2345/tor/branches/all?query=bug32588_

Hmmm CI exploded for one specific builds with interesting stacktrace:

https://travis-ci.org/torproject/tor/jobs/627571176

Trac:
Status: needs_review to needs_revision

Trac:
Cc: neel to N/A

Trac:
Status: needs_revision to needs_review

Trac:
Status: needs_review to needs_revision

Replying to dgoulet:

Hmmm CI exploded for one specific builds with interesting stacktrace:

https://travis-ci.org/torproject/tor/jobs/627571176

I forgot to check for a NULL IPv6 address.

I re-did the patch, and split the commits into:

• bug fix (and code movement)
• refactoring

I still need to write some tests, but I'd like a review :-) I also need to fix up #32822 (moved), because it's based on this branch.

Trac:
Actualpoints: 0.4 to 0.6
Status: needs_revision to needs_review

Trac:
Keywords: 042-backport deleted, 042-backport 043-should added

Trac:
Status: needs_review to assigned
Owner: neel to N/A

Trac:
Status: assigned to needs_review

Trac:
Owner: N/A to teor
Status: needs_review to assigned

Trac:
Status: assigned to needs_review

I made some fixes suggested by dgoulet in #32822 (moved), and squashed them in to a new PR:

• 0.3.5: https://github.com/torproject/tor/pull/1654

The fixup commits are in the old PR.

Trac:
Keywords: 042-backport 043-should deleted, 042-backport added

I made some fixes, but I still need to do unit tests, so I'm leaving it in needs_revision.

Trac:
Status: needs_review to needs_revision
Keywords: N/A deleted, 043-should added

Replying to teor:

I made some fixes suggested by dgoulet in #32822 (moved), and squashed them in to a new PR:

• 0.3.5: https://github.com/torproject/tor/pull/1654

I added some tests for auto port parsing. They don't actually trigger this bug, but they do increase the auto port config code coverage:

• master: https://github.com/torproject/tor/pull/1660

Trac:
Actualpoints: 0.6 to 0.8

This fix should go in 0.4.3, someone just needs to write a unit test or a script test.

Or we can decide that it's too hard to test, and the reviewer can just check it manually.

Trac:
Status: needs_revision to assigned
Owner: teor to N/A

Trac:
Status: assigned to needs_revision

Trac:
Owner: N/A to ahf
Status: needs_revision to assigned

Trac:
Status: assigned to needs_revision

I'm going to need the new function in this branch for Sponsor 55, Objective 1.1.

ahf, do you think you can get the remaining tests done this week?

If not, I can take this task back.

Trac:
Sponsor: N/A to Sponsor55-must
Parent: N/A to #33048 (moved)

Trac:
Owner: ahf to teor
Status: needs_revision to assigned
Reviewer: N/A to ahf

Trac:
Status: assigned to needs_revision

I'll try doing the tests here to take some load off teor.

Trac:
Owner: teor to nickm
Status: needs_revision to accepted

FWIW, this isn't an "uninitialized RAM" bug: The reason it says 94 is that the "ipv6_port" field in the cfg_port_t is set to CFG_AUTO_PORT, which is 0xc4005e in hex. Once that's cast to a uint16_t, we get 0x5e, which is 94.

I wrote some tests on top of your branch, in a branch called `bug32588_035_tests. I made a PR at https://github.com/torproject/tor/pull/1811 . There will be merge conflicts later on here, but they look to be straightforward. Please let me know if you want help with those: I'm going under the assumption you might want to edit this branch some more.

I don't know whether this is everything you wanted to test or not.

Trac:
Actualpoints: 0.8 to 1

Replying to nickm:

FWIW, this isn't an "uninitialized RAM" bug: The reason it says 94 is that the "ipv6_port" field in the cfg_port_t is set to CFG_AUTO_PORT, which is 0xc4005e in hex. Once that's cast to a uint16_t, we get 0x5e, which is 94.

That makes sense! I misread the hex constant as 0x405e, so I couldn't work out how the cast was doing that, without some memory misalignment.

Thanks, this is exactly what I wanted to test. Honestly, I was a bit stuck trying to work out how.

I'll do a few tweaks, and then merge.

Thanks so much!

Trac:
Owner: nickm to teor
Status: accepted to assigned

Trac:
Status: assigned to needs_revision
Reviewer: ahf to nickm, teor

I tweaked a few of the tests, and fixed the changes file.

See my PR:

• 0.3.5: https://github.com/torproject/tor/pull/1826

I'll squash and create merge forward branches after the tests pass.

Trac:
Keywords: security-low, 040-backport, memory-safety deleted, consider-backport-after-0435, 043-backport added
Actualpoints: 1 to 1.1
Milestone: Tor: 0.4.3.x-final to Tor: 0.4.4.x-final

I squashed my changes, nick's changes, my extra tests, and some minor test fixes.

See my PRs:

• 0.3.5: https://github.com/torproject/tor/pull/1827
• 0.4.1: https://github.com/torproject/tor/pull/1828
• 0.4.3: https://github.com/torproject/tor/pull/1829
• master: https://github.com/torproject/tor/pull/1830

Test branches are here:

• https://github.com/teor2345/tor/branches/all?query=32588

Anyone can merge after CI passes.

Trac:
Status: needs_revision to merge_ready

0.4.1 and 0.4.2 failed on Travis, but they pass locally.

And Travis is having a database downtime, so I can't check why. See: https://www.traviscistatus.com/

Okay, I'm merging this to master and marking for a 0.4.3 backport.

Trac:
Milestone: Tor: 0.4.4.x-final to Tor: 0.4.3.x-final