Our public key functions assumed that they were always writing into a
large enough buffer. In one case, they weren't.
(Incorporates fixes from sebastian)
When intro->extend_info is created for an introduction point, it
only starts out with a nickname, not necessarily an identity digest.
Thus, doing router_get_by_digest isn't necessarily safe.
https://trac.torproject.org/projects/tor/ticket/1859
Use router_get_by_digest() instead of router_get_by_hexdigest()
in circuit_discard_optional_exit_enclaves() and
rend_client_get_random_intro(), per Nick's comments.
Using router_get_by_digest() in rend_client_get_random_intro() will
break hidden services published by Tor versions pre 0.1.2.18 and
0.2.07-alpha as they only publish by nickname. This is acceptable
however as these versions only publish to authority tor26 and
don't work for versions in the 0.2.2.x series anyway.
https://trac.torproject.org/projects/tor/ticket/1859
There are two problems in this bug:
1. When an OP makes a .exit request specifying itself as the exit, and the exit
is not yet listed, Tor gets all the routerinfos needed for the circuit but
discovers in circuit_is_acceptable() that its own routerinfo is not in the
routerdigest list and cannot be used. Tor then gets locked in a cycle of
repeating these two steps. When gathering the routerinfos for a circuit,
specifically when the exit has been chosen by .exit notation, Tor needs to
apply the same rules it uses later on when deciding if it can build a
circuit with those routerinfos.
2. A different bug arises in the above situation when the Tor instance's
routerinfo *is* listed in the routerlist, it shares its nickname with a
number of other Tor nodes, and it does not have 'Named' rights to its
nickname.
So for example, if (i) there are five nodes named Bob in the network, (ii) I
am running one of them but am flagged as 'Unnamed' because someone else
claimed the 'Bob' nickname first, and (iii) I run my Tor as both client
and exit the following can happen to me:
- I go to www.evil.com
- I click on a link www.evil.com.bob.exit
- My request will exit through my own Tor node rather than the 'Named'
node Bob or any of the others.
- www.evil.com now knows I am actually browsing from the same computer
that is running my 'Bob' node
So to solve both issues we need to ensure:
- When fulfilling a .exit request we only choose a routerinfo if it exists in
the routerlist, even when that routerinfo is ours.
- When getting a router by nickname we only return our own router information
if it is not going to be used for building a circuit.
We ensure this by removing the special treatment afforded our own router in
router_get_by_nickname(). This means the function will only return the
routerinfo of our own router if it is in the routerlist built from authority
info and has a unique nickname or is bound to a non-unique nickname.
There are some uses of router_get_by_nickname() where we are looking for the
router by name because of a configuration directive, specifically local
declaration of NodeFamilies and EntryNodes and other routers' declaration of
MyFamily. In these cases it is not at first clear if we need to continue
returning our own routerinfo even if our router is not listed and/or has a
non-unique nickname with the Unnamed flag.
The patch treats each of these cases as follows:
Other Routers' Declaration of MyFamily
This happens in routerlist_add_family(). If another router declares our router
in its family and our router has the Unnamed flag or is not in the routerlist
yet, should we take advantage of the fact that we know our own routerinfo to
add us in anyway? This patch says 'no, treat our own router just like any
other'. This is a safe choice because it ensures our client has the same view
of the network as other clients. We also have no good way of knowing if our
router is Named or not independently of the authorities, so we have to rely on
them in this.
Local declaration of NodeFamilies
Again, we have no way of knowing if the declaration 'NodeFamilies
Bob,Alice,Ringo' refers to our router Bob or the Named router Bob, so we have
to defer to the authorities and treat our own router like any other.
Local declaration of NodeFamilies
Again, same as above. There's also no good reason we would want our client to
choose it's own router as an entry guard if it does not meet the requirements
expected of any other router on the network.
In order to reduce the possibility of error, the patch also replaces two
instances where we were using router_get_by_nickname() with calls to
router_get_by_hexdigest() where the identity digest of the router
is available.
When intro->extend_info is created for an introduction point, it
only starts out with a nickname, not necessarily an identity digest.
Thus, doing router_get_by_digest isn't necessarily safe.
https://trac.torproject.org/projects/tor/ticket/1859
Use router_get_by_digest() instead of router_get_by_hexdigest()
in circuit_discard_optional_exit_enclaves() and
rend_client_get_random_intro(), per Nick's comments.
Using router_get_by_digest() in rend_client_get_random_intro() will
break hidden services published by Tor versions pre 0.1.2.18 and
0.2.07-alpha as they only publish by nickname. This is acceptable
however as these versions only publish to authority tor26 and
don't work for versions in the 0.2.2.x series anyway.
https://trac.torproject.org/projects/tor/ticket/1859
There are two problems in this bug:
1. When an OP makes a .exit request specifying itself as the exit, and the exit
is not yet listed, Tor gets all the routerinfos needed for the circuit but
discovers in circuit_is_acceptable() that its own routerinfo is not in the
routerdigest list and cannot be used. Tor then gets locked in a cycle of
repeating these two steps. When gathering the routerinfos for a circuit,
specifically when the exit has been chosen by .exit notation, Tor needs to
apply the same rules it uses later on when deciding if it can build a
circuit with those routerinfos.
2. A different bug arises in the above situation when the Tor instance's
routerinfo *is* listed in the routerlist, it shares its nickname with a
number of other Tor nodes, and it does not have 'Named' rights to its
nickname.
So for example, if (i) there are five nodes named Bob in the network, (ii) I
am running one of them but am flagged as 'Unnamed' because someone else
claimed the 'Bob' nickname first, and (iii) I run my Tor as both client
and exit the following can happen to me:
- I go to www.evil.com
- I click on a link www.evil.com.bob.exit
- My request will exit through my own Tor node rather than the 'Named'
node Bob or any of the others.
- www.evil.com now knows I am actually browsing from the same computer
that is running my 'Bob' node
So to solve both issues we need to ensure:
- When fulfilling a .exit request we only choose a routerinfo if it exists in
the routerlist, even when that routerinfo is ours.
- When getting a router by nickname we only return our own router information
if it is not going to be used for building a circuit.
We ensure this by removing the special treatment afforded our own router in
router_get_by_nickname(). This means the function will only return the
routerinfo of our own router if it is in the routerlist built from authority
info and has a unique nickname or is bound to a non-unique nickname.
There are some uses of router_get_by_nickname() where we are looking for the
router by name because of a configuration directive, specifically local
declaration of NodeFamilies and EntryNodes and other routers' declaration of
MyFamily. In these cases it is not at first clear if we need to continue
returning our own routerinfo even if our router is not listed and/or has a
non-unique nickname with the Unnamed flag.
The patch treats each of these cases as follows:
Other Routers' Declaration of MyFamily
This happens in routerlist_add_family(). If another router declares our router
in its family and our router has the Unnamed flag or is not in the routerlist
yet, should we take advantage of the fact that we know our own routerinfo to
add us in anyway? This patch says 'no, treat our own router just like any
other'. This is a safe choice because it ensures our client has the same view
of the network as other clients. We also have no good way of knowing if our
router is Named or not independently of the authorities, so we have to rely on
them in this.
Local declaration of NodeFamilies
Again, we have no way of knowing if the declaration 'NodeFamilies
Bob,Alice,Ringo' refers to our router Bob or the Named router Bob, so we have
to defer to the authorities and treat our own router like any other.
Local declaration of NodeFamilies
Again, same as above. There's also no good reason we would want our client to
choose it's own router as an entry guard if it does not meet the requirements
expected of any other router on the network.
In order to reduce the possibility of error, the patch also replaces two
instances where we were using router_get_by_nickname() with calls to
router_get_by_hexdigest() where the identity digest of the router
is available.
A node_t is an abstraction over routerstatus_t, routerinfo_t, and
microdesc_t. It should try to present a consistent interface to all
of them. There should be a node_t for a server whenever there is
* A routerinfo_t for it in the routerlist
* A routerstatus_t in the current_consensus.
(note that a microdesc_t alone isn't enough to make a node_t exist,
since microdescriptors aren't usable on their own.)
There are three ways to get a node_t right now: looking it up by ID,
looking it up by nickname, and iterating over the whole list of
microdescriptors.
All (or nearly all) functions that are supposed to return "a router"
-- especially those used in building connections and circuits --
should return a node_t, not a routerinfo_t or a routerstatus_t.
A node_t should hold all the *mutable* flags about a node. This
patch moves the is_foo flags from routerinfo_t into node_t. The
flags in routerstatus_t remain, but they get set from the consensus
and should not change.
Some other highlights of this patch are:
* Looking up routerinfo and routerstatus by nickname is now
unified and based on the "look up a node by nickname" function.
This tries to look only at the values from current consensus,
and not get confused by the routerinfo_t->is_named flag, which
could get set for other weird reasons. This changes the
behavior of how authorities (when acting as clients) deal with
nodes that have been listed by nickname.
* I tried not to artificially increase the size of the diff here
by moving functions around. As a result, some functions that
now operate on nodes are now in the wrong file -- they should
get moved to nodelist.c once this refactoring settles down.
This moving should happen as part of a patch that moves
functions AND NOTHING ELSE.
* Some old code is now left around inside #if 0/1 blocks, and
should get removed once I've verified that I don't want it
sitting around to see how we used to do things.
There are still some unimplemented functions: these are flagged
with "UNIMPLEMENTED_NODELIST()." I'll work on filling in the
implementation here, piece by piece.
I wish this patch could have been smaller, but there did not seem to
be any piece of it that was independent from the rest. Moving flags
forces many functions that once returned routerinfo_t * to return
node_t *, which forces their friends to change, and so on.
The problem was that we didn't allocate enough memory on 32-bit
platforms with 64-bit time_t. The memory leak occured every time
we fetched a hidden service descriptor we've fetched before.
The new rule is: safe_str_X() means "this string is a piece of X
information; make it safe to log." safe_str() on its own means
"this string is a piece of who-knows-what; make it safe to log".
See task 1114. The most plausible explanation for someone sending us weak
DH keys is that they experiment with their Tor code or implement a new Tor
client. Usually, we don't care about such events, especially not on warn
level. If we really care about someone not following the Tor protocol, we
can set ProtocolWarnings to 1.
Tor now reads the "circwindow" parameter out of the consensus,
and uses that value for its circuit package window rather than the
default of 1000 cells. Begins the implementation of proposal 168.
The internal error "could not find intro key" occurs when we want to send
an INTRODUCE1 cell over a recently finished introduction circuit and think
we built the introduction circuit with a v2 hidden service descriptor, but
cannot find the introduction key in our descriptor.
My first guess how we can end up in this situation is that we are wrong in
thinking that we built the introduction circuit based on a v2 hidden
service descriptor. This patch checks if we have a v0 descriptor, too, and
uses that instead.
The subversion $Id$ fields made every commit force a rebuild of
whatever file got committed. They were not actually useful for
telling the version of Tor files in the wild.
svn:r17867
Initial conversion of uint32_t addr to tor_addr_t addr in connection_t and related types. Most of the Tor wire formats using these new types are in, but the code to generate and use it is not. This is a big patch. Let me know what it breaks for you.
svn:r16435
Part of fix for bug 617: allow connection_ap_handshake_attach_circuit() to mark connections, to avoid double-mark warnings. Note that this is an incomplete refactoring.
svn:r14066
Resolved problems with (re-)fetching hidden service descriptors.
Before, v0 descriptors were not fetched at all (fix on 0.2.0.18-alpha),
re-fetching of v2 descriptors did not stop when a v0 descriptor was
received (fix on 0.2.0.18-alpha), and re-fetching of v2 descriptors did
not work in all cases (fix on 0.2.0.19-alpha).
svn:r13540
time you use a given introduction point for your service, but
on subsequent requests we'd be using garbage memory. Fixed by
Karsten Loesing. Bugfix on 0.2.0.12-alpha.
svn:r12913
Try to make hidden service directory lookup functions a bit more efficient: go for fewer O(n) operations, and look at the consensus rather than the routerinfo list.
svn:r12361
Make all LD_BUG log messsages get prefixed with "Bug: ". Remove manually-generated "Bug: "s from log-messages. (Apparently, we remembered to add them about 40% of the time.)
svn:r9733
Try to compile with fewer warnings on irix64's MIPSpro compiler /
environment, which apparently believes that:
- off_t can be bigger than size_t.
- only mean kids assign things they do not subsequently inspect.
I don't try to fix the "error" that makes it say:
cc-3970 cc: WARNING File = main.c, Line = 1277
conversion from pointer to same-sized integral type (potential portability
problem)
uintptr_t sig = (uintptr_t)arg;
Because really, what can you do about a compiler that claims to be c99
but doesn't understand that void* x = NULL; uintptr_t y = (uintptr_t) x;
is safe?
svn:r8948
Apply patch from Mike Perry: add more reasons for circuit destroys. (Slightly tweaked to avoid allocating a number for an "internal" reason.)
svn:r8739
Refactor connection_t into edge, or, dir, control, and base subtypes. This might save some RAM on busy exit servers, but really matters most in terms of correctness.
svn:r6906
No circuit can be both an intro point and a rend point, so we can merge both the cookie and the pk digest into one "rend_token" field for or circuits. This saves another 20 bytes per or circuit.
svn:r6904
circuit_t into origin_circuit_t and or_circuit_t. I fixed some
segaults; there may be more. We still need to move more rendezvous
stuff into subtypes.
This is a trial run for splitting up connection_t; if the approach is
insane, please say so soon so we can do something smarter.
Also, this discards the old HALF_OPEN code, which nobody seems to
want.
svn:r6817
FetchServerDescriptors and FetchHidServDescriptors for whether
to fetch server info and hidserv info or let the controller do it,
and also PublishServerDescriptor and PublishHidServDescriptors.
Add AllDirActionsPrivate undocumented option -- if you set it, you'll
need the controller to bootstrap you enough to build your first circuits.
svn:r6047
we screwed up the formatting in wild and unpredictable ways.
fix it before it becomes convention to format logs in wild and
unpredictable ways.
still need to do src/common/ someday.
svn:r5551
warn when we'recalling a non-named server by its nickname;
don't warn twice about the same name.
Fix a bug in routers_update_status_from_networkstatus that made nearly
all clients never update routerinfo_t.is_named.
Try to list MyFamily elements by key, not by nickname.
Only warn about names that we generated ourself, or got from the local
user.
On TLS handshake, only check the other router's nickname against its
expected nickname if is_named is set.
svn:r5185
- Add a new extend_info_t datatype to hold information needed to
extend a circuit (addr,port,keyid,onion_key). Use it in cpath and
build_state. Make appropriate functions take or return it instead of
routerinfo_t or keyid.
- #if 0 needless check in circuit_get_by_edge_conn; if nobody triggers this
error in 0.1.0.10, nobody will trigger it.
- Implement new hidden service descriptor format, which contains "extend
info" for introduction points, along with protocol version list.
- Parse new format.
- Generate new format
- Cache old and new formats alongside each other.
- Directories serve "old" format if asked in old way, "newest available"
format if asked in new way.
- Use new format to find introduction points if possible; otherwise fall
back. Keep nickname lists and extendinfo lists in sync.
- Tests for new format.
- Implement new "v2" INTRODUCE cell format.
- Accept new format
- Use new format if we have a versioned service descriptor that says the
server accepts the new format.
- Add documentation for functions and data types.
svn:r4506
60 seconds fetching the hidserv descriptor, which made them
more likely to fail on the first attempt, yet they work fine
on the second. so now give them extra time for the first try.
svn:r4127
until none are left, then we try to refetch the descriptor. If it's
the same one we had before, then close streams right then. Whenever
a new stream arrives, even if it's right after, optimistically try
refetching the descriptor, just in case.
svn:r3379
a general circ, and called rend_client_rendcirc_has_opened(), which
called connection_ap_attach_pending(), which was needing a rendezvous
circ, so it cannibalized a general circuit, and called ...
svn:r3370
to the exit policy of the last hop. Intro and rendezvous circs must
be internal circs, to avoid leaking information. Resolve and connect
streams can use internal circs if they want.
New circuit pooling algorithm: make sure to have enough circs around
to satisfy any predicted ports, and also make sure to have 2 internal
circs around if we've required internal circs lately (with high uptime
if we've seen that lately).
Split NewCircuitPeriod config option into NewCircuitPeriod (30 secs),
which describes how often we retry making new circuits if current ones
are dirty, and MaxCircuitDirtiness (10 mins), which describes how long
we're willing to make use of an already-dirty circuit.
Once rendezvous circuits are established, keep using the same circuit as
long as you attach a new stream to it at least every 10 minutes. (So web
browsing doesn't require you to build new rend circs every 30 seconds.)
Cannibalize GENERAL circs to be C_REND, C_INTRO, S_INTRO, and S_REND
circ as necessary, if there are any completed ones lying around when
we try to launch one.
Re-instate the ifdef's to use version-0 style introduce cells, since
there was yet another bug in handling version-1 style. We'll try switching
over again after 0.0.9 is obsolete.
Bugfix: when choosing an exit node for a new non-internal circ, don't take
into account whether it'll be useful for any pending x.onion addresses --
it won't.
Bugfix: we weren't actually publishing the hidden service descriptor when
it became dirty. So we only published it every 20 minutes or so, which
means when you first start your Tor, the hidden service will seem broken.
svn:r3360
