Generate and store all supported microdescriptor formats. Generate
votes with one "m" line for each format. Only "m" lines with version
info matching chosen consensus method will be voted upon.
An optimisation would be to combine "m" lines with identical hashes,
i.e. instead of "m 1,2,3 H1" and "m 4,5 H1", say "m 1,2,3,4,5 H1".
Define new new consensus method 14 adding "a" lines to vote and
consensus documents.
From proposal 186:
As with other data in the vote derived from the descriptor, the
consensus will include whichever set of "a" lines are given by the
most authorities who voted for the descriptor digest that will be
used for the router.
This patch implements this.
Allow one-hop directory fetching circuits the full "circuit build timeout"
period, rather than just half of it, before failing them and marking
the relay down. This fix should help reduce cases where clients declare
relays (or worse, bridges) unreachable because the TLS handshake takes
a few seconds to complete.
Fixes bug 6743 (one piece of bug 3443); bugfix on 0.2.2.2-alpha, where
we changed the timeout from a static 30 seconds.
We've had over two months to fix them, and didn't. Now we need
0.2.3.x stable. Yes, it would be cool to get this working in
0.2.3.x, but not at the expense of delaying every other feature that
_does_ work in 0.2.3.x. We can do a real fix in 0.2.4.
This is important, since otherwise an attacker can use timing info
to probe the internal network.
Also, add an option (ExtendAllowPrivateAddresses) so that
TestingTorNetwork won't break.
Fix for bug 6710; bugfix on all released versions of Tor.
Move extend_info_from_router() from circuitbuild.c to router.c and
make it static.
Add get_configured_bridge_by_orports_digest() and have
get_configured_bridge_by_routerinfo() and
node_is_a_configured_bridge() use it. We now consider all OR ports of
a bridge when looking for it.
Move node_get_*_orport to nodelist.c.
Fix a cut'n'paste error in header of nodelist.h.
Add node_assert_ok().
Add router_get_all_orports(). It's duplicating code from
node_get_all_orports(). Worth fixing at the cost of complicating the
API slightly?
There was some code in the "err:" block that would always log a
warning, reporting an "unknown error" if we hadn't set err_msg. But
there were also plenty of "goto err" blocks that did their own
logging, and never set err_msg at all. Now we should only log when
we have an error message to log.
This fixes bug 6638, from no released Tor version.
Failure to do this would lead to double-free cases and similar,
especially when the exit's DNS was broken. See bug 6472 for full
details; this is a fix for 6472.
Anonymous patch from "cypherpunks" on trac.
Long ago, before we had cell queues, it was necessary to maybe call
connection_handle_write() from connectino_write_to_buf_impl() on OR
connections, so that we wouldn't get into a loop of reading infinite
amounts of data and queueing it all on an outbuf before bothering to
write any data.
If that doesn't sounds like what our code does now, you're right:
right now, we won't stick more than OR_CONN_HIGHWATER bytes of cells
on an outbuf, and we won't suck more than CELL_QUEUE_HIGHWATER_SIZE
cells off any edge connection. So, there's no more call for that
code.
Removing this code will simplify our data flow, and that should be
something we can all get behind.
This is done to avoid spurious warns. Additional log lines are also
added to try to track down the codepaths where we are somehow overcounting
success counts.
This patch extracts the inner part of config_register_addressmaps --
the part that knows about detecting wildcard addresses addresses --
and makes it into a new function. The new function is deliberately
not moved or reindented, so that the diff is smaller.
I need this to fix bug 6244.
Extend cells aren't allowed to have a stream_id, but we were only
blocking them when they had a stream_id that corresponded to a
connection. As far as I can tell, this change is harmless: it will
make some kinds of broken clients not work any more, but afaik nobody
actually make a client that was broken in that way.
Found while hunting for other places where we made the same mistake
as in 6271.
Bugfix on d7f50337c1 back from May 2003, which introduced
telescoping circuit construction into 0.0.2pre8.
Thanks to the changes we started making with SocksPort and friends
in 0.2.3.3-alpha, any of our code that did "if (options->Sockport)"
became wrong, since "SocksPort 0" would make that test true whereas
using the default SocksPort value would make it false. (We didn't
actually do "if (options->SockPort)" but we did have tests for
TransPort. When we moved DirPort, ORPort, and ControlPort over to
the same system in 0.2.3.9-alpha, the problem got worse, since our
code is littered with checks for DirPort and ORPort as booleans.
This code renames the current linelist-based FooPort options to
FooPort_lines, and adds new FooPort_set options which get set at
parse-and-validate time on the or_options_t. FooPort_set is true
iff we will actually try to open a listener of the given type. (I
renamed the FooPort options rather than leave them alone so that
every previous user of a FooPort would need to get inspected, and so
that any new code that forgetfully uses FooPort will need fail to
compile.)
Fix for bug 6507.
With this patch, I dump the old kludge of using magic negative
numbers to indicate unknown bandwidths. I also compute each node's
weighted bandwidth exactly once, rather than computing it once in
a loop to compute the total weighted bandwidth and a second time in
a loop to find which one we picked.
Previously, we had incremented rand_bw so that when we later tested
"tmp >= rand_bw", we wouldn't have an off-by-one error. But instead,
it makes more sense to leave rand_bw alone and test "tmp > rand_bw".
Note that this is still safe. To take the example from the bug1203
writeup: Suppose that we have 3 nodes with bandwidth 1. So the
bandwidth array is { 1, 1, 1 }, and the total bandwidth is 3. We
choose rand_bw == 0, 1, or 2. With the first iteration of the loop,
tmp is now 1; with the second, tmp is 2; with the third, tmp is 3.
Now that our check is tmp > rand_bw, we will set i in the first
iteration of the loop iff rand_bw == 0; in the second iteration of
the loop iff rand_bw == 1, and in the third iff rand_bw == 2.
That's what we want.
Incidentally, this change makes the bug 6538 fix more ironclad: once
rand_bw is set to UINT64_MAX, tmp > rand_bw is obviously false
regardless of the value of tmp.
The old approach, because of its "tmp >= rand_bw &&
!i_has_been_chosen" check, would run through the second part of the
loop slightly slower than the first part. Now, we remove
i_has_been_chosen, and instead set rand_bw = UINT64_MAX, so that
every instance of the loop will do exactly the same amount of work
regardless of the initial value of rand_bw.
Fix for bug 6538.
This should make our preferred solution to #6538 easier to
implement, avoid a bunch of potential nastiness with excessive
int-vs-double math, and generally make the code there a little less
scary.
"But wait!" you say. "Is it really safe to do this? Won't the
results come out differently?"
Yes, but not much. We now round every weighted bandwidth to the
nearest byte before computing on it. This will make every node that
had a fractional part of its weighted bandwidth before either
slighty more likely or slightly less likely. Further, the rand_bw
value was only ever set with integer precision, so it can't
accurately sample routers with tiny fractional bandwidth values
anyway. Finally, doing repeated double-vs-uint64 comparisons is
just plain sad; it will involve an implicit cast to double, which is
never a fun thing.
This gives us a few benefits:
1) make -j clean all
this will start working, as it should. It currently doesn't.
2) increased parallel build
recursive make will max out at number of files in a directory,
non-recursive make doesn't have such a limitation
3) Removal of duplicate information in make files,
less error prone
I've also slightly updated how we call AM_INIT_AUTOMAKE, as the way
that was used was not only deprecated but will be *removed* in the next
major automake release (1.13).... so probably best that we can continue
to bulid tor without requiring old automake.
(see http://www.gnu.org/software/automake/manual/html_node/Public-Macros.html )
For more reasons why, see resources such as:
http://miller.emu.id.au/pmiller/books/rmch/
In 0.2.3.18-rc, we started warning on this case while building a
list of missing microdescriptor digests. That turned out to spam
the logs; instead let's warn at parse time.
Partial fix for bug 6404.
The spec requires that every router in a microdesc consensus have an
m line; we weren't obeying that spec.
This creates a new consensus method (13) to allow voting to continue
to work right. Partial fix for bug 6404; fix on 0.2.2.6-alpha.
Test for config option AuthDirPublishIPv6 == 1 rather than for running
as a bridge authority when deciding whether to care or not about IPv6
OR ports in descriptors.
Implements enhancement #6406.
We can end up in dirserv_orconn_tls_done() with a node missing
routerinfo in at least two cases -- command_process_certs_cell() and
connection_or_check_valid_tls_handshake() -- and probably more.
You can say "struct foo_t" as much as you want, but you'd better not
have "typedef struct foo_t foo_t" more than once.
Fix for bug 6416. Bug not in any released version of Tor.
This avoids a possible crash bug in flush_from_first_active_circuit.
Fixes bug 6341; bugfix on 0.2.2.7-alpha.
Bug reported and fixed by a pseudonymous user on IRC.
I only check on circuits, not streams, since bloating your stream
window past the initial circuit window can't help you much.
Also, I compare to CIRCWINDOW_START_MAX so we don't have surprising
races if we lower CIRCWINDOW_START for an experiment.
The SMARTLIST_FOREACH macro is more convenient than BEGIN/END when
you have a nice short loop body, but using it for long bodies makes
your preprocessor tell the compiler that all the code is on the same
line. That causes grief, since compiler warnings and debugger lines
will all refer to that one line.
So, here's a new style rule: SMARTLIST_FOREACH blocks need to be
short.
- Add a changes/ file.
- Make it compile under --enable-gcc-warnings.
- Update the file-level documentation of src/or/transports.c.
- Only update descriptor if at least a managed proxy was configured.
- Add our external IP address to the extra-info descriptor instead of 0.0.0.0.
This could result in bizarre window values. Report and patch
contributed pseudymously. Fixes part of bug 6271. This bug was
introduced before the first Tor release, in svn commit r152.
(bug 6271, part a.)
This reverts commit c32ec9c425.
It turns out the two sides of the circuit don't actually stay in sync,
so it is perfectly normal for the circuit window on the exit relay to
grow to 2000+. We should fix that bug and then reconsider this patch.
I only check on circuits, not streams, since bloating your stream
window past the initial circuit window can't help you much.
Also, I compare to CIRCWINDOW_START_MAX so we don't have surprising
races if we lower CIRCWINDOW_START for an experiment.
We were doing a tor_strclear() on client_keys_str when it might not
even be set.
Fix for bug 6255; bug not in any release of Tor. Thanks to katmagic
for finding this one!
The functions parse_{s,c}method_line() were using
tor_addr_port_lookup() which is capable of doing DNS lookups. DNS
lookups should not be necessary when parsing {C,S}METHOD lines.
We now catch bare {s that should be on the previous line with a do,
while, if, or for, and elses that should share a line with their
preceding }.
That is,
if (foo)
{
and
if (foo) {
...
}
else
are now detected.
We should think about maybe making Tor uncrustify-clean some day,
but configuring uncrustify is an exercise in bizarreness, and
reformatting huge gobs of Tor is always painful.
The code that detected the source of a remapped address checked that
an address mapping's source was a given rewrite rule if addr_orig had
no .exit, and addr did have a .exit after processing that rule. But
addr_orig was formatted for logging: it was not the original address
at all, but rather was the address escaped for logging and possibly
replaced with "[scrubbed]".
This new logic will correctly set ADDRMAPSRC_NONE in the case when the
address starts life as a .exit address, so that AllowDotExit can work
again.
Fixes bug 6211; bugfix on 0.2.3.17-beta
It turns out this can happen. Even though there is no reason for
connections to be marked but reading, we leave them reading anyway,
so warning here is unwarranted. Let's turn that back on once we do
something sensible and disable reading when we mark. Bugfix for
6203 on Tor 0.2.3.17-beta.
Thanks to cypherpunks for pointing out the general stupidity of the
original code here.
Now it's an orthodox "goto err/done" exit path, and it isn't some
screwy thing where we stick err/done at the end of a loop and
duplicate our cleanup code.
Previously, a directory would check the latest NS consensus for
having the signatures the client wanted, and use that consensus's
valid_until time to set the HTTP lifetime. With this patch, the
directory looks at NS consensus or the microdesc consensus,
depending on what the client asked for.
I saw 72% on a test run with 26 circuits. 70% might be a little close to the
line. That, or min_circs is too low and we need to be more patient. We still
need to test/simulate more.
The defense counts the circuit failure rate for each guard for the past N
circuits. Failure is defined as the ability to complete a first hop, but not
finish completing the circuit all the way to the exit.
If the failure rate exceeds a certain amount, a notice is emitted.
If it exceeds a greater amount, a warn is emitted and the guard is disabled.
These values are governed by consensus parameters which we intend to tune as
we perform experiments and statistical simulations.
The warning message of validate_pluggable_transports_config() is
superseded by the changes in the warning message of
connection_or_connect() when the proxy credentials can't be found.
There is a bug causing busy loops in Libevent and infinite loops in
the Shadow simulator. A connection that is marked for close, wants
to flush, is held open to flush, but is rate limited (the token
bucket is empty) triggers the bug.
This commit fixes the bug. Details are below.
This currently happens on read and write callbacks when the active
socket is marked for close. In this case, Tor doesn't actually try
to complete the read or write (it returns from those methods when
marked), but instead tries to clear the connection with
conn_close_if_marked(). Tor will not close a marked connection that
contains data: it must be flushed first. The bug occurs when this
flush operation on the marked connection can not occur because the
connection is rate-limited (its write token bucket is empty).
The fix is to detect when rate limiting is preventing a marked
connection from properly flushing. In this case, it should be
flagged as read/write_blocked_on_bandwidth and the read/write events
de-registered from Libevent. When the token bucket gets refilled, it
will check the associated read/write_blocked_on_bandwidth flag, and
add the read/write event back to Libevent, which will cause it to
fire. This time, it will be properly flushed and closed.
The reason that both read and write events are both de-registered
when the marked connection can not flush is because both result in
the same behavior. Both read/write events on marked connections will
never again do any actual reads/writes, and are only useful to
trigger the flush and close the connection. By setting the
associated read/write_blocked_on_bandwidth flag, we ensure that the
event will get added back to Libevent, properly flushed, and closed.
Why is this important? Every Shadow event occurs at a discrete time
instant. If Tor does not properly deregister Libevent events that
fire but result in Tor essentially doing nothing, Libevent will
repeatedly fire the event. In Shadow this means infinite loop,
outside of Shadow this means wasted CPU cycles.
From what I can tell, this configuration is usually a mistake, and
leads people to think that all their traffic is getting proxied when
in fact practically none of it is. Resolves the issue behind "bug"
4663.
The function is not guaranteed to NUL-terminate its output. It
*is*, however, guaranteed not to generate more than two bytes per
multibyte character (plus terminating nul), so the general approach
I'm taking is to try to allocate enough space, AND to manually add a
NUL at the end of each buffer just in case I screwed up the "enough
space" thing.
Fixes bug 5909.
This feature can make Tor relays less identifiable by their use of the
mod_ssl DH group, but at the cost of some usability (#4721) and bridge
tracing (#6087) regressions.
We should try to turn this on by default again if we find that the
mod_ssl group is uncommon and/or we move to a different DH group size
(see #6088). Before we can do so, we need a fix for bugs #6087 and
Resolves ticket #5598 for now.
These stats are currently discarded, but we might as well
hard-disable them on bridges, to be clean.
Fix for bug 5824; bugfix on 0.2.1.17-rc.
Patch originally by Karsten Loesing.
Also, try to resolve some doxygen issues. First, define a magic
"This is doxygen!" macro so that we take the correct branch in
various #if/#else/#endifs in order to get the right documentation.
Second, add in a few grouping @{ and @} entries in order to get some
variables and fields to get grouped together.
This code shouldn't have any effect in 0.2.3, since we already accept
(and handle) data received while we are expecting a renegotiation.
(That's because the 0.2.3.x handshake _does_ have data there instead of
the renegotiation.)
I'm leaving it in anyway, since if it breaks anything, we'll want it
broken in master too so we can find out about it. I added an XXX023
comment so that we can come back later and fix that.
This fixes a DoS issue where a client could send so much data in 5
minutes that they exhausted the server's RAM. Fix for bug 5934 and
6007. Bugfix on 0.2.0.20-rc, which enabled the v2 handshake.
This solves bug 5283, where client traffic could get sent over the
same circuit as an anonymized connection to a directory, even if
that circuit used an exit node unsuitable for clients. By marking
the directory connection as needs_internal, we ensure that the
(non-internal!) client-traffic connection won't be sent over the
same circuit.
On Windows, getsockname() on a nonblocking apparently won't work
until the connection is done connecting. On XP, it seems to fail by
reporting success and declaring that your address is INADDR_ANY. On the
Win8 preview, though, it fails more loudly and says WSAEINVAL.
Fix for bug 5374; bugfix on 0.1.1.14-alpha.
Roger explains at
http://archives.seul.org/tor/talk/Nov-2011/msg00209.html :
"If you list your bridge as part of your family in the relay
descriptor, then everybody can learn your bridge fingerprint, and
they can look up your bridge's descriptor (and thus location) at
the bridge directory authority."
Now, we can't stop relays from listing bridges, but we can warn when
we notice a bridge listing anybody, which might help some.
This fixes bug 4657; it's a fix on 0.2.0.3-alpha, where bridges were
first introduced.
To hit this leak, you need to be a relay that gets a RESOLVE request
or an exit node getting a BEGIN or RESOLVE request. You must either
have unconfigured (and unconfigurable) nameservers, or you must have
somehow set DisableNetwork after a network request arrived but
before you managed to process it.
So, I doubt this is reached often. Still, a leak's a leak. Fix for
bug 5916; bugfix on 0.2.3.9-alpha and 0.1.2.1-alpha.
%f is correct; %lf is only needed with scanf. Apparently, on some
old BSDs, %lf is deprecated.
Didn't we do this before? Yes, we did. But we only got the
instances of %lf, not more complicated things like %.5lf . This
patch tries to get everything.
Based on a patch for 3894 by grarpamp.
These errors usually mean address exhaustion; reporting them as such
lets clients adjust their load to try other exits.
Fix for bug 4710; bugfix on 0.1.0.1-rc, which started using
END_STREAM_REASON_RESOURCELIMIT.
Silences the log message:
[warn] {BUG} _connection_mark_unattached_ap(): Bug: stream (marked at connection_edge.c:2224) sending two socks replies?
after the client triggered the "Tor is not an HTTP Proxy" response.
No additional socks reply was sent, though.
Previously, we only did this check at startup, which could lead to
us holding a guard indefinitely, and give weird results. Fixes bug
5380; bugfix on 0.2.1.14-rc.
(Patch by Roger; changes file and commit message by Nick)
Previously, we skipped everything that got invoked from
options_init_from_torrc. But some of the stuff in
options_act_reversible and options_act is actually important, like
reopening the logs.
Now, a SIGHUP always makes the effects of an options_set() happen,
even though the options haven't changed.
Fix for bug 5095; bugfix on 0.2.1.9-alpha, which introduced
__ReloadTorrcOnSIGHUP.
This would happen if the deliver window could become negative
because of an nonexistent connection. (Fortunately, _that_ can't
occur, thanks to circuit_consider_sending_sendme. Still, if we
change our windowing logic at all, we won't want this to become
triggerable.) Fix for bug 5541. Bugfix on 4a66865d, back from
0.0.2pre14. asn found this. Nice catch, asn!
This tells the windows headers to give us definitions that didn't
exist before XP -- like the ones that we need for IPv6 support.
See bug #5861. We didn't run into this issue with mingw, since
mingw doesn't respect _WIN32_WINNT as well as it should for some of
its definitions.
MSVC warns if you declare a function as having a "int foo" argument
and then implement it with a "const int foo" argument, even though
the latter "const" is not a part of the function's interface.
Instead, allow packagers to put a 'TOR_BUILD_TAG' field in the
server descriptor to indicate a platform-specific value, if they
need to. (According to weasel, this was his use for the git- tag
previously.)
This is part of 2988
This time, I follow grarpamp's suggestion and move the check for
.exit+AllowDotExit 0 to the top of connection_ap_rewrite_and_attach,
before any rewriting occurs. This way, .exit addresses are
forbidden as they arrive from a socks connection or a DNSPort
request, and not otherwise.
It _is_ a little more complicated than that, though. We need to
treat any .exit addresses whose source is TrackHostExits as meaning
that we can retry without that exit. We also need to treat any
.exit address that comes from an AutomapHostsOnResolve operation as
user-provided (and thus forbidden if AllowDotExits==0), so that
transitioning from AllowDotExits==1 to AllowDotExits==0 will
actually turn off automapped .exit addresses.
This patch changes the total serverdesc threshold from 25% to 75%
and the exit threshold from 33% to 50%. The goal is to make
initially constructed circuits less horrible, and to make initial
less awful (since fetching directory information in parallel with
whatever the user is trying to do can hurt their performance).
Implements ticket 3196.
We were doing an O(n) strlen in router_get_extrainfo_hash() for
every one we tried to parse. Instead, have
router_get_extrainfo_hash() take the length of the extrainfo as an
argument, so that when it's called from
extrainfo_parse_from_string(), it doesn't do a strlen() over the
whole pile of extrainfos.
If the authorities agreed on a sufficiently bad bwweightscale value
(<=0 or == INT32_MAX), the bandwidth algorithm could make the voters
assert while computing the consensus.
Fix for bug5786; bugfix on 0.2.2.17-alpha
This is how IPv6 says "0.0.0.0" and something we will have to
translate into a globally reachable address before putting it in a
descriptor.
The fix is a short term solution until a real one is implemented.
Closes#5146.
Fixes bug #4528 "read_to_buf_tls(): Inconsistency in code".
This check was added back in 0.1.0.3-rc, but somehow we forgot to
leave it in when we refactored read_to_buf_tls in 0.1.0.5-rc.
(patch by Arturo; commit message and changes file by nickm)
Instead of checking for 'rejected' and calling everything else okay,
let's check for 'outdated' and call everythign else a problem. This
way we don't risk missing future errors so much.
When logging a message that _looks_ like an error message at info, we
should mention that it isn't really a problem.
Previously, we would reset it at the drop of a hat -- every time a second
passes without any of the intro-point circs already launched for the
service failing.
Fixes bug 4607.
In general, whenever we can, we should be doing
base64_decode(buf, sizeof(buf), s, strlen(s)),
and not
base_64_decode(buf, expr1, s, expr2)
where we hope that expr1 is a good name for the size of buf and expr2
is a good formula for the length of the base64 expression in s.
* Document fmt_addr_impl() and friends.
* Parenthesize macro arguments.
* Rename get_first_listener_addrport_for_pt() to
get_first_listener_addrport_string().
* Handle port_cfg_t with no_listen.
* Handle failure of router_get_active_listener_port_by_type().
* Add an XXX to router_get_active_listener_port_by_type().
This is just refactoring work here. The old logic was kind of
convoluted, especially after the bug 5572 fix. We don't actually need to
distinguish so many cases here. Dropping detection of the
"!old_options || !old_options->DynamicDHGroups" case is fine because
that's the same that we'd do for clients.
Also add a changes file for bug 5572.
The message only means that we're publishing a new descriptor when we
are actually in some kind of server mode, and publication is on.
Fix for bug 3942; bugfix on 0.2.3.2-alpha.
This fixes a side-channel attack on the (fortunately unused!)
BridgePassword option for bridge authorities. Fix for bug 5543;
bugfix on 0.2.0.14-alpha.
Introduce get_first_listener_addrport_for_pt() which returns a string
containing the addrport of the first listener we could find. Use it to
form the TOR_PT_ORPORT managed proxy protocol line.
This is ticket 2479. Roger's original explanation was:
We have a series of bugs where relays publish a descriptor within
12 hours of their last descriptor, but the authorities drop it
because it's not different "enough" from the last one and it's
too close to the last one.
The original goal of this idea was to a) reduce the number of new
descriptors authorities accept (and thus have to store) and b)
reduce the total number of descriptors that clients and mirrors
fetch. It's a defense against bugs where relays publish a new
descriptor every minute.
Now that we're putting out one consensus per hour, we're doing
better at the total damage that can be caused by 'b'.
There are broader-scale design changes that would help here, and
we've had a trac entry open for years about how relays should
recognize that they're not in the consensus, or recognize when
their publish failed, and republish sooner.
In the mean time, I think we should change some of the parameters
to make the problem less painful.
When we started RefuseUnknownExits back in 0.2.2.11-alpha, we
started making exits act like they cache directory info (since they
need an up-to-date idea of who is really a router). But this
included fetching needless (unrecognized) authorities' certs, which
doesn't make any sense for them.
This is related to, but not necessarily the same as, the issue that
Ian reported for bug #2297.
(This patch is based on a patch from a user who I believe has asked
not to be named. If I'm wrong about that, please add the
appropriate name onto the changelog.)
A previous commit in the 5527 branch had moved
router_get_mutable_by_digest(digest_rcvd) to happen before we did
tor_assert(digest_rcvd), which would have defeated the purpose of
the assert.
Specifically, I believe it dates back to when extend cells had address:port
but no digest in them. The special edge case is certainly not worth the
complexity these days.
This mitigates an attack proposed by wanoskarnet, in which all of a
client's bridges collude to restrict the exit nodes that the client
knows about. Fixes bug 5343.
In the distant past, connection_handle_read() could be called when there
are pending bytes in the TLS object during the main loop. The design
since then has been to always read all pending bytes immediately, so
read events only trigger when the socket actually has bytes to read.
Resolves bug 5324.
The invariant is: unconfigured_proxies_n is exactly the number of
managed_proxy_t not in state PT_PROTO_COMPLETED.
To maintain this, we need to stop overloading unconfigured_proxies_n
to also count managed_proxy_t items that are in PT_PROTO_COMPLETED but
which might need relaunching. To make it so we can detect those, we
introduce another variable.
This commit also adds a function to assert that we haven't broken the
invariant.
Fix for bug 5084; bugfix on 0.2.3.6-alpha, I think.
In some cases, we solve this by doing a SMARTLIST_DEL_CURRENT before
calling managed_proxy_destroy. But for a trickier one, we just make a
copy of the list before iterating over it, so that changes to the
manage proxy list don't hurt our iteration.
This could be related to bug 5084.
in Makefile.am, we used it without quoting it, causing build failure if
your openssl/sed/sha1sum happened to live in a directory with a space in
it (very common on windows)
If we don't do this, [::] can be interpreted to mean all v4 and all
v6 addresses. Found by dcf. Fixes bug 4760. See RFC 3493 section
5.3 for more info.
There was one MS_WINDOWS that remained because it wasn't on a macro
line; a few remaining uses (and the definition!) in configure.in;
and a now-nonsensical stanza of eventdns_tor.h that previously
defined 'WIN32' if it didn't exist.
This commit is completely mechanical; I used this perl script to make it:
#!/usr/bin/perl -w -i.bak -p
if (/^\s*\#/) {
s/MS_WINDOWS/_WIN32/g;
s/\bWIN32\b/_WIN32/g;
}
Previously the client would ask the bridge for microdescriptors, which are
only supported in 0.2.3.x and later, and then fail to bootstrap when it
didn't get the answers it wanted. Fixes bug 4013; bugfix on 0.2.3.2-alpha.
The fix here is to revert to using normal descriptors if any of our
bridges are known to not support microdescs. This is not ideal, a) because
we'll start downloading a microdesc consensus as soon as we get a bridge
descriptor, and that will waste time if we later get a bridge descriptor
that tells us we don't like microdescriptors; and b) by changing our mind
we're leaking to our other bridges that we have an old-version bridge.
The alternate fix would have been to change
we_use_microdescriptors_for_circuits() to ask if *any* of our bridges
can support microdescriptors, and then change the directory logic that
picks a bridge to only select from those that do. For people living in
the future, where 0.2.2.x is obsolete, there won't be a difference.
Note that in either of these potential fixes, we have risk of oscillation
if our one funny-looking bridges goes away / comes back.
These were found by looking for tor_snprintf() instances that were
preceeded closely by tor_malloc(), though I probably converted some
more snprintfs as well.
(In every case, make sure that the length variable (if any) is
removed, renamed, or lowered, so that anything else that might have
assumed a longer buffer doesn't exist.)
These were found by looking for tor_snprintf() instances that were
followed closely by tor_strdup(), though I probably converted some
other snprintfs as well.
(To ensure correctness, in every case, make sure that the temporary
variable is deleted, renamed, or lowered in scope, so we can't have
any bugs related to accidentally relying on the no-longer-filled
variable.)
Fixes bug #4897, not yet in any release.
Using n_circ_id alone here (and below, when n_conn is NULL) really sucks,
but that's a separate bug which will need a changes/ file.
When we have an effective bandwidthrate configured so that we cannot
exceed our bandwidth limit in one accounting interval, don't disable
advertising the dirport. Implements ticket 2434.
We used to do this as a workaround for older Tors, but now it's never
the correct thing to do (especially since anything that didn't
understand RELAY_EARLY is now deprecated hard).
This patch should make us reject every Tor that was vulnerable to
CVE-2011-0427. Additionally, it makes us reject every Tor that couldn't
handle RELAY_EARLY cells, which helps with proposal 110 (#4339).
Back in #1240, r1eo linked to information about how this could happen
with older Linux kernels in response to nmap. Bugs #4545 and #4547
are about how our approach to trying to deal with this condition was
broken and stupid. Thanks to wanoskarnet for reminding us about #1240.
This is a fix for the abovementioned bugs, and is a bugfix on
0.1.0.3-rc.
Preprocessor directives should not be put inside the arguments
of a macro. This is not supported on older GCC releases (< 3.3)
thus broke compilation on Haiku (running gcc2).
It's inefficient, but the more efficient solution (only try to attach
streams aiming for this HS) would require far more complexity for a gain
that should be tiny.
The client's anonymity when accessing a non-HS address in tor2web-mode
would be easily nuked by inserting an inline image with a .onion URL, so
don't even pretend to access non-HS addresses through Tor.
The Tor2webMode torrc option is still required to run a Tor client in
'tor2web mode', but now it can't be turned on at runtime in a normal build
of Tor. (And a tor2web build of Tor can't be used as a normal Tor client,
so we don't have to worry as much about someone distributing packages with
this particular pistol accessible to normal users.)
This resolves a loop warning on "MapAddress *.example.com
example.com", makes the rewrite log messages correct, and fixes the
behavior of "MapAddress *.a *.b" when just given "a" as an input.
MapAddress *.torproject.org torproject.org would have been interpreted
as a map from a domain to itself, and would have cleared the mapping.
Now we require not only a match of domains, but of wildcards.
Incidentally, we've got 30969 lines in master with a comma
in them, of which 1995 have a comma followed by a non-newline,
non-space character. So about 93% of our commas are right,
but we have a substantial number of "crowded" lines.
It might be nice to support this someday, but for now it would fail
with an infinite remap cycle. (If I say "remap * *.foo.exit",
then example.com ->
example.com.foo.exit ->
example.com.foo.exit.foo.exit ->
example.com.foo.exit.foo.exit.foo.exit -> ...)
In this new representation for wildcarded addresses, there are no
longer any 'magic addresses': rather, "a.b c.d", "*.a.b c.d" and
"*.a.b *.c.d" are all represented by a mapping from "a.b" to "c.d". we
now distinguish them by setting bits in the addressmap_entry_t
structure, where src_wildcard is set if the source address had a
wildcard, and dst_wildcard is set if the target address had a
wildcard.
This lets the case where "*.a.b *.c.d" or "*.a.b c.d" remap the
address "a.b" get handled trivially, and lets us simplify and improve
the addressmap_match_superdomains implementation: we can now have it
run in O(parts of address) rather than O(entries in addressmap).
1. Only allow '*.' in MapAddress expressions. Ignore '*ample.com' and '.example.com'.
This has resulted in a slight refactoring of config_register_addressmaps.
2. Add some more detail to the man page entry for AddressMap.
3. Fix initialization of a pointer to NULL rather than 0.
4. Update the unit tests to cater for the changes in 1 and test more explicitly for
recursive mapping.
1. Implement the following mapping rules:
MapAddress a.b.c d.e.f # This is what we have now
MapAddress .a.b.c d.e.f # Replaces any address ending with .a.b.c with d.e.f
MapAddress .a.b.c .d.e.f # Replaces the .a.b.c at the end of any addr with .d.e.f
(Note that 'a.b.c .d.e.f' is invalid, and will be rejected.)
2. Add tests for the new rules.
3. Allow proper wildcard annotation, i.e. '*.d.e' '.d.e' will still work.
4. Update addressmap_entry_t with an is_wildcard member.
This keeps the IP address and TCP for a given OR port together,
reducing the risk of using an address for one address family with a
port of another.
Make node_get_addr() a wrapper function for compatibility.
This is not as conservative as we could do it, f.ex. by looking at the
connection and only do this for connections to bridges. A non-bridge
should never have anything else than its primary IPv4 address set
though, so I think this is safe.
Don't touch the string representation in routerinfo_t->address.
Also, set or clear the routerinfo_t->ipv6_preferred flag based on the
address family of the bridge.
Comments below focus on changes, see diff for added code.
New type tor_addr_port_t holding an IP address and a TCP/UDP port.
New flag in routerinfo_t, ipv6_preferred. This should go in the
node_t instead but not now.
Replace node_get_addr() with
- node_get_prim_addr() for primary address, i.e. IPv4 for now
- node_get_pref_addr() for preferred address, IPv4 or IPv6.
Rename node_get_addr_ipv4h() node_get_prim_addr_ipv4h() for
consistency. The primary address will not allways be an IPv4 address.
Same for node_get_orport() -> node_get_prim_orport().
Rewrite node_is_a_configured_bridge() to take all OR ports into account.
Extend argument list to extend_info_from_node and
extend_info_from_router with a flag indicating if we want to use the
routers primary address or the preferred address. Use the preferred
address in as few situtations as possible for allowing clients to
connect to bridges over IPv6.
This code handles the new ORPort options, and incidentally makes all
remaining port types use the new port configuration systems.
There are some rough edges! It doesn't do well in the case where your
Address says one thing but you say to Advertise another ORPort. It
doesn't handle AllAddrs. It doesn't actually advertise anything besides
the first listed advertised IPv4 ORPort and DirPort. It doesn't do
port forwarding to them either.
It's not tested either, it needs more documentation, and it probably
forgets to put the milk back in the refrigerator.
Some controllers want this so they can mess with Tor's configuration
for a while via the control port before actually letting Tor out of
the house.
We do this with a new DisableNetwork option, that prevents Tor from
making any outbound connections or binding any non-control
listeners. Additionally, it shuts down the same functionality as
shuts down when we are hibernating, plus the code that launches
directory downloads.
To make sure I didn't miss anything, I added a clause straight to
connection_connect, so that we won't even try to open an outbound
socket when the network is disabled. In my testing, I made this an
assert, but since I probably missed something, I've turned it into a
BUG warning for testing.
This will mainly help distributors by giving a way to set system or package
defaults that a user can override, and that a later package can replace.
No promises about the particular future location or semantics for this:
we will probably want to tweak it some before 0.2.3.x-rc
The file is searched for in CONFDIR/torrc-defaults , which can be
overridden with the "--defaults-torrc" option on the command line.
This starts an effort to refactor torrc handling code to make it easier
to live with. It makes it possible to override exit policies from the
command line, and possible to override (rather than append to) socksport
lists from the command line.
It'll be necessary to make a "base" torrc implementation work at all.
Instead of only writing the dynamic DH prime modulus to a file, write
the whole DH parameters set for forward compatibility. At the moment
we only accept '2' as the group generator.
The DH parameters gets stored in base64-ed DER format to the
'dynamic_dh_params' file.
Let's *not* expose more cross-platform-compatibility structures, or
expect code to use them right.
Also, don't fclose() stdout_handle and stdin_handle until we do
tor_process_handle_destroy, or we risk a double-fclose.
We used to do init_keys() if DynamicDHGroups changed after a HUP, so
that the dynamic DH modulus was stored on the disk. Since we are now
doing dynamic DH modulus storing in crypto.c, we can simply initialize
the TLS context and be good with it.
Introduce a new function router_initialize_tls_context() which
initializes the TLS context and use it appropriately.
The function is over 10 or 20% on some of Moritz's profiles, depending
on how you could.
Since it's checking for a multi-hour timeout, this is safe to do.
Fixes bug 4518.
Completely disable stats if we aren't running as a relay. We won't
collect any anyway, so setting up the infrastructure for them and
logging about them is wrong. This also removes a confusing log
message that clients without a geoip db would have seen.
Fixes bug 4353.
When running with IOCP, we are in theory able to use userspace-
allocated buffers to avoid filling up the stingy amount of kernel
space allocated for sockets buffers.
The bufferevent_async implementation in Libevent provides this
ability, in theory. (There are likely to be remaining bugs). This
patch adds a new option that, when using IOCP bufferevents, sets
each socket's send and receive buffers to 0, so that we should use
this ability.
When all the bugs are worked out here, if we are right about bug 98,
this might solve or mitigate bug 98.
This option is experimental and will likely require lots of testing
and debugging.
Let's make it more obvious to the everyday reader that eventdns.c is
a) Based on Libevent's evdns.c
b) Slated for demolition
c) Supposed to keep API-compatibility with Libevent.
d) Not worth tweaking unless there's a bug.
- Add a LOG_WARN message when registering the transports of a server
managed proxy, so that the bridge operator can see in what ports the
transports spawned and notify his/her clients.
The Right Way to expire an intro point is to establish a new one to
replace it, publish a new descriptor that doesn't list any expiring intro
points, and *then*, once our upload attempts for the new descriptor have
ended (whether in success or failure), close the expiring intro points.
Unfortunately, we can't find out when the new descriptor has actually been
uploaded, so we'll have to settle for a five-minute timer.
There should be no significant behaviour changes due to this commit (only
a log-message change or two), despite the rather massive overhaul, so this
commit doesn't include a changes/ file. (The commit that teaches
intro_point_should_expire_now to return non-zero gets a changes/ file,
though.)
We would stash the certs in the handshake state before checking them
for validity... and then if they turned out to be invalid, we'd give
an error and free them. Then, later, we'd free them again when we
tore down the connection.
Fixes bug 4343; fix on 0.2.3.6-alpha.
This way, all of the DA operators can upgrade immediately, without nuking
every client's set of entry guards as soon as a majority of them upgrade.
Until enough guards have upgraded, a majority of dirauths should set this
config option so that there are still enough guards in the network. After
a few days pass, all dirauths should use the default.
The patch for 3228 made us try to run init_keys() before we had loaded
our state file, resulting in an assert inside init_keys. We had moved
it too early in the function.
Now it's later in the function, but still above the accounting calls.
Previously we did this nearer to the end (in the old_options &&
transition_affects_workers() block). But other stuff cares about
keys being consistent with options... particularly anything which
tries to access a key, which can die in assert_identity_keys_ok().
Fixes bug 3228; bugfix on 0.2.2.18-alpha.
Conflicts:
src/or/config.c
When we added support for separate client tls certs on bridges in
a2bb0bfdd5 we forgot to correctly initialize this when changing
from relay to bridge or vice versa while Tor is running. Fix that
by always initializing keys when the state changes.
Fixes bug 2433.
Conflicts:
src/or/config.c
We use a hash of the identity key to seed a prng to tell when an
accounting period should end. But thanks to the bug998 changes,
clients no longer have server-identity keys to use as a long-term seed
in accounting calculations. In any case, their identity keys (as used
in TLS) were never never fixed. So we can just set the wakeup time
from a random seed instead there. Still open is whether everybody
should be random.
This patch fixes bug 2235, which was introduced in 0.2.2.18-alpha.
Diagnosed with help from boboper on irc.
In a2bb0bf we started using a separate client identity key. When we are
in "public server mode" (that means not a bridge) we will use the same
key. Reusing the key without doing the proper refcounting leads to a
segfault on cleanup during shutdown. Fix that.
Also introduce an assert that triggers if our refcount falls below 0.
That should never happen.
We now require that:
- Only actual servers should ever call get_server_identity_key
- If you're being a client or bridge, the client and server keys should
differ.
- If you're being a public relay, the client and server keys
should be the same.
* Make tor_tls_context_new internal to tortls.c, and return the new
tor_tls_context_t from it.
* Add a public tor_tls_context_init wrapper function to replace it.
Conflicts:
src/or/main.c
src/or/router.c
Since we check for naughty renegotiations using
tor_tls_t.server_handshake_count we don't need that semi-broken
function (at least till there is a way to disable rfc5746
renegotiations too).
When you're doing malloc(sizeof(int)), something may well have gone
wrong.
This technique is a bit abusive, but we're already relying on it
working correctly in geoip.c.
To get a better idea what's going on on Tonga, add some code to report
how often the most and least frequently fetched descriptor was fetched,
as well as 25, 50, 75 percentile.
Also ensure we only count bridge descriptors here.
- Add a tor_process_get_pid() function that returns the PID of a
process_handle_t.
- Conform to make check-spaces.
- Add some more documentation.
- Improve some log messages.
We used to try to terminate the managed proxy process even if it
failed while launching. We introduce a new managed proxy state, to
represent a *broken* and *not launched* proxy.
This is used for the bridge authority currently, to get a better
intuition on how many descriptors are actually fetched from it and how
many fetches happen in total.
Implements ticket 4200.
Fixes bug 4259, bugfix on 0.2.2.25-alpha. Bugfix by "Tey'".
Original message by submitter:
Changing nodes restrictions using a controller while Tor is doing
DNS resolution could makes Tor crashes (on WinXP at least). The
problem can be repeated by trying to reach a non-existent domain
using Tor:
curl --socks4a 127.0.0.1:9050 inexistantdomain.ext
.. and changing the ExitNodes parameter through the control port
before Tor returns a DNS resolution error (of course, the following
command won't work directly if the control port is password
protected):
echo SETCONF ExitNodes=TinyTurtle | nc -v 127.0.0.1 9051
Using a non-existent domain is needed to repeat the issue so that
Tor takes a few seconds for resolving the domain (which allows us to
change the configuration). Tor will crash while processing the
configuration change.
The bug is located in the addressmap_clear_excluded_trackexithosts
method which iterates over the entries of the addresses map in order
to check whether the changes made to the configuration will impact
those entries. When a DNS resolving is in progress, the new_adress
field of the associated entry will be set to NULL. The method
doesn't expect this field to be NULL, hence the crash.
Previously, we would treat an intro circuit failure as a timeout iff the
circuit failed due to a mismatch in relay identity keys. (Due to a bug
elsewhere, we only recognize relay identity-key mismatches on the first
hop, so this isn't as bad as it could have been.)
Bugfix on commit eaed37d14c, not yet in any
release.
It's too risky to have a function where if you leave one parameter
NULL, it splits up address:port strings, but if you set it, it does
hostname resolution.
Under the new convention, having a tor_addr.*lookup function that
doesn't do hostname resolution is too close for comfort.
I used this script here, and have made no other changes.
s/tor_addr_parse_reverse_lookup_name/tor_addr_parse_PTR_name/g;
s/tor_addr_to_reverse_lookup_name/tor_addr_to_PTR_name/g;
Now let's have "lookup" indicate that there can be a hostname
resolution, and "parse" indicate that there wasn't. Previously, we
had one "lookup" function that did resolution; four "parse" functions,
half of which did resolution; and a "from_str()" function that didn't
do resolution. That's confusing and error-prone!
The code changes in this commit are exactly the result of this perl
script, run under "perl -p -i.bak" :
s/tor_addr_port_parse/tor_addr_port_lookup/g;
s/parse_addr_port(?=[^_])/addr_port_lookup/g;
s/tor_addr_from_str/tor_addr_parse/g;
This patch leaves aton and pton alone: their naming convention and
behavior is is determined by the sockets API.
More renaming may be needed.
Right now we can take the digests only of an RSA key, and only expect to
take the digests of an RSA key. The old tor_cert_get_id_digests() would
return a good set of digests for an RSA key, and an all-zero one for a
non-RSA key. This behavior is too error-prone: it carries the risk that
we will someday check two non-RSA keys for equality and conclude that
they must be equal because they both have the same (zero) "digest".
Instead, let's have tor_cert_get_id_digests() return NULL for keys we
can't handle, and make its callers explicitly test for NULL.
Also, define all commands > 128 as variable-length when using
v3 or later link protocol. Running into a var cell with an
unrecognized type is no longer a bug.
Without this patch, Tor wasn't sure whether it would be hibernating or
not, so it postponed opening listeners until after the privs had been
dropped. This doesn't work so well for low ports. Bug was introduced in
the fix for bug 2003. Fixes bug 4217, reported by Zax and katmagic.
Thanks!
One of its callers assumes a non-zero result indicates a permanent failure
(i.e. the current attempt to connect to this HS either has failed or is
doomed). The other caller only requires that this function's result
never equal -2.
Bug reported by Sebastian Hahn.
Change the default values for collecting directory request statistics and
inlcuding them in extra-info descriptors to 1.
Don't break if we are configured to collect directory request or entry
statistics and don't have a GeoIP database. Instead, print out a notice
and skip initializing the affected statistics code.
This is the cherry-picked 499661524b.
Previously Tor would always claim to have been given a hostname
by the client, while actually only verifying that the client
is using SOCKS4A or SOCKS5 with hostnames. Both protocol versions
allow IP addresses, too, in which case the log messages were wrong.
Fixes#4094.
Previously, we wouldn't refetch an HS's descriptor unless we didn't
have one at all. That was equivalent to refetching iff we didn't have
a usable one, but the next commit will make us keep some non-usable HS
descriptors around in our cache.
Code bugfix on the release that introduced the v2 HS directory system,
because rend_client_refetch_v2_renddesc's documentation comment should
have described what it actually did, not what its behaviour happened
to be equivalent to; no behaviour change in this commit.
Otherwise, we can wind up munging our reference counts if we set it in
the middle of loading the nodes. This happens because
nodelist_set_consensus() and microdesc_reload_cache() are both in the
business of adjusting microdescriptors' references.
we don't need to check whether we don't have enough guards right after
concluding that we do have enough.
slight efficiency fix suggested by an anonymous fellow on irc.
We were doing "divide bandwidth by 1000, then multiply by msec", but
that would lose accuracy: instead of getting your full bandwidth,
you'd lose up to 999 bytes per sec. (Not a big deal, but every byte
helps.)
Instead, do the multiply first, then the division. This can easily
overflow a 32-bit value, so make sure to do it as a 64-bit operation.
With managed proxies you would always get the error message:
"You have a Bridge line using the X pluggable transport, but there
doesn't seem to be a corresponding ClientTransportPlugin line."
because the check happened directly after parse_client_transport_line()
when managed proxies were not fully configured and their transports
were not registered.
The fix is to move the validation to run_scheduled_events() and make
sure that all managed proxies are configured first.
* Create mark/sweep functions for transports.
* Create a transport_resolve_conflicts() function that tries to
resolve conflicts when registering transports.
Previously the FooPort was ignored and the default used instead,
causing Tor to bind to the wrong port if FooPort and the default
port don't match or the CONN_TYPE_FOO_LISTENER has no default port.
Fixes#3936.
Right now we only force a new descriptor upload every 18 hours.
This can make servers become unlisted if they upload a descriptor at
time T which the authorities reject as being "too similar" to one
they uploaded before. Nothing will actually make the server upload a
new descriptor later on, until another 18 hours have passed.
This patch changes the upload behavior so that the 18 hour interval
applies only when we're listed in a live consensus with a descriptor
published within the last 18 hours. Otherwise--if we're not listed
in the live consensus, or if we're listed with a publication time
over 18 hours in the past--we upload a new descriptor every 90
minutes.
This is an attempted bugfix for #3327. If we merge it, it should
obsolete #535.
Conflicts:
src/or/connection.c
src/or/connection_edge.c
src/or/connection_edge.h
src/or/dnsserv.c
Some of these were a little tricky, since they touched code that
changed because of the prop171 fixes.
Add a "default" state which we use until we've decided whether we're
live or hibernating. This allows us to properly track whether we're
resuming a hibernation period or not. Fixes bug 2003.
For printf, %f and %lf are synonymous, since floats are promoted to
doubles when passed as varargs. It's only for scanf that we need to
say "%lf" for doubles and "%f" for floats.
Apparenly, some older compilers think it's naughty to say %lf and like
to spew warnings about it.
Found by grarpamp.
For bufferevents, we had all of connection_buckets_decrement() stubbed
out. But that's not actually right! The rephist_* parts were
essential for, inter alia, recording our own bandwidth. This patch
splits out the rephist parts of connection_buckets_decrement() into their
own function, and makes the bufferevent code call that new function.
Fixes bug 3803, and probably 3824 and 3826 too. Bugfix on 0.2.3.1-alpha.
Previously, if you were set up to use microdescriptors, and you
weren't a cache, you'd never fetch router descriptors (except for
bridges). Now FetchUselessDescriptors causes descriptors and
mirodescs to get cached. Also, FetchUselessDescriptors changes the
behavior of "UseMicrodescriptors auto" to be off, since there's no
point in saying "UseMicrodescriptors 1" when you have full descriptors
too.
Fix for bug 3851; bugfix on 0.2.3.1-alpha.
Because tunneled connections are implemented with buffervent_pair,
writing to them can cause an immediate flush. This means that
added to them and then checking to see whether their outbuf is
empty is _not_ an adequate way to see whether you added anything.
This caused a problem in directory server connections, since they
would try spooling a little more data out, and then close the
connection if there was no queued data to send.
This fix should improve matters; it only closes the connection if
there is no more data to spool, and all of the spooling callbacks
are supposed to put the dirconn into dir_spool_none on completion.
This is bug 3814; Sebastian found it; bugfix on 0.2.3.1-alpha.
When we're doing filtering ssl bufferevents, we want the rate-limits
to apply to the lowest level of the bufferevent stack, so that we're
actually limiting bytes sent on the network. Otherwise, we'll read
from the network aggressively, and only limit stuff as we process it.
This behavior is normal when we want more data than the evbuffer
actually has for us. We'll ask for (say) 7 bytes, get only 5
(because that's all there is), try to parse the 5 bytes, and get
told "no, I want 7". One option would be to bail out early whenever
want_length is > buflen, but sometimes we use an over-large
want_length. So instead, let's just remove the warning here: it's
not a bug after all.
* Use strcmpstart() instead of strcmp(x,y,strlen(y)).
* Warn the user if the managed proxy failed to launch.
* Improve function documentation.
* Use smartlist_len() instead of n_unconfigured_proxies.
* Split managed_proxy_destroy() to managed_proxy_destroy()
and managed_proxy_destroy_with_transports().
* Constification.
It turns out that it wasn't enough to set the configuration to
"auto", since the correct behavior for "auto" had been disabled in
microdesc.c. :p
(Hasn't been in a release yet, so doesn't need a changes entry.)
Now we track *which* stream with ISO_STREAM set is associated to a
particular circuit, so that we won't think that stream is incompatible
with its circuit and launch another one a second later, and we use that
same field to mark circuits which have had an ISO_STREAM stream attached
to them, so that we won't ever put a second stream on that circuit.
Fixes bug 3695.
Only write a bridge-stats string if bridge stats have been
initialized. This behavior is similar to dirreq-stats, entry-stats,
etc.
Also add a few unit tests for the bridge-stats code.
This patch separates the generation of a dirreq-stats string from
actually writing it to disk. The new geoip_format_dirreq_stats()
generates a dirreq-stats string that geoip_dirreq_stats_write() writes
to disk. All the state changing (e.g., resetting the dirreq-stats
history and initializing the next measurement interval) takes place in
geoip_dirreq_stats_write(). That allows us to finally test the
dirreq-stats code better.
Now that formatting the buffer-stats string is separate from writing
it to disk, we can also decouple the logic to extract stats from
circuits and finally write some unit tests for the history code.
The new rep_hist_format_buffer_stats() generates a buffer-stats string
that rep_hist_buffer_stats_write() writes to disk. All the state
changing (e.g., resetting the buffer-stats history and initializing
the next measurement interval) takes place in
rep_hist_buffer_stats_write(). That allows us to finally test the
buffer-stats code better.
So far, if we didn't see a single circuit, we refrained from
generating a cell-stats string and logged a warning. Nobody will
notice the warning, and people will wonder why there's no cell-stats
string in the extra-info descriptor. The better behavior is to
generate a cell-stats string with all zeros.
Right now, we append statistics to files in the stats/ directory for
half of the statistics, whereas we overwrite these files for the other
half. In particular, we append buffer, dirreq, and entry stats and
overwrite exit, connection, and bridge stats.
Appending to files was useful when we didn't include stats in extra-info
descriptors, because otherwise we'd have to copy them away to prevent
Tor from overwriting them.
But now that we include statistics in extra-info descriptors, it makes
no sense to keep the old statistics forever. We should change the
behavior to overwriting instead of appending for all statistics.
Implements #2930.
They *are* non-NUL-terminated, after all (and they have to be, since
the SOCKS5 spec allows them to contain embedded NULs. But the code
to implement proposal 171 was copying them with tor_strdup and
comparing them with strcmp_opt.
Fix for bug on 3683; bug not present in any yet-released version.
Previously we'd just looked at the connection type, but that's
always CONN_TYPE_AP. Instead, we should be looking at the type of
the listener that created the connection.
Spotted by rransom; fixes bug 3636.
We'll still need to tweak it so that it looks for includes and
libraries somewhere more sensible than "where we happened to find
them on Erinn's system"; so that tests and tools get built too;
so that it's a bit documented; and so that we actually try running
the output.
Work done with Erinn Clark.
The conflicts are with the proposal 171 circuit isolation code, and
they're all trivial: they're just a matter of both branches adding
some unrelated code in the same places.
Conflicts:
src/or/circuituse.c
src/or/connection.c
The problem was that we weren't initializing want_length to 0 before
calling parse_socks() the first time, so it looked like we were
risking an infinite loop when in fact we were safe.
Fixes 3615; bugfix on 0.2.3.2-alpha.
Back when I added this logic in 20c0581a79, the rule was that whenever
a circuit finished building, we cleared its isolation info. I did that
so that we would still use the circuit even if all the streams that
had previously led us to tentatively set its isolation info had closed.
But there were problems with that approach: We could pretty easily get
into a case where S1 had led us to launch C1 and S2 had led us to
launch C2, but when C1 finished, we cleared its isolation and attached
S2 first. Since C2 was still marked in a way that made S1
unattachable to it, we'd then launch another circuit needlessly.
So instead, we try the following approach now: when a circuit is done
building, we try to attach streams to it. If it remains unused after
we try attaching streams, then we clear its isolation info, and try
again to attach streams.
Thanks to Sebastian for helping me figure this out.
One-hop dirconn streams all share a session group, and get the
ISO_SESSIONGRP flag: they may share circuits with each other and
nothing else.
Anonymized dirconn streams get a new internal-use-only ISO_STREAM
flag: they may not share circuits with anything, including each other.
The new candidate rule, which arma suggested and I like, is that
the original address as received from the client connection or as
rewritten by the controller is the address that counts.
This is mainly meant as a way to keep clients from accidentally
DOSing themselves by (e.g.) enabling IsolateDestAddr or
IsolateDestPort on a port that they use for HTTP.
Our old "do we need to launch a circuit for stream S" logic was,
more or less, that if we had a pending circuit that could handle S,
we didn't need to launch a new one.
But now that we have streams isolated from one another, we need
something stronger here: It's possible that some pending C can
handle either S1 or S2, but not both.
This patch reuses the existing isolation logic for a simple
solution: when we decide during circuit launching that some pending
C would satisfy stream S1, we "hypothetically" mark C as though S1
had been connected to it. Now if S2 is incompatible with S1, it
won't be something that can attach to C, and so we'll launch a new
stream.
When the circuit becomes OPEN for the first time (with no streams
attached to it), we reset the circuit's isolation status. I'm not
too sure about this part: I wanted some way to be sure that, if all
streams that would have used a circuit die before the circuit is
done, the circuit can still get used. But I worry that this
approach could also lead to us launching too many circuits. Careful
thought needed here.
This is the meat of proposal 171: we change circuit_is_acceptable()
to require that the connection is compatible with every connection
that has been linked to the circuit; we update circuit_is_better to
prefer attaching streams to circuits in the way that decreases the
circuits' usefulness the least; and we update link_apconn_to_circ()
to do the appropriate bookkeeping.
The "nym epoch" of a stream is defined as the number of times that
NEWNYM had been called before the stream was opened. All streams
are isolated by nym epoch.
This feature should be redundant with existing signewnym stuff, but
it provides a good belt-and-suspenders way for us to avoid ever
letting any circuit type bypass signewnym.
This patch adds fields to track how streams should be isolated, and
ensures that those fields are set correctly. It also adds fields to
track what streams can go on a circuit, and adds functions to see
whether a streams can go on a circuit and update the circuit
accordingly. Those functions aren't yet called.
Proposal 171 gives us a new syntax for parsing client port options.
You can now have as many FooPort options as you want (for Foo in
Socks, Trans, DNS, NATD), and they can have address:port arguments,
and you can specify the level of isolation on those ports.
Additionally, this patch refactors the client port parsing logic to
use a new type, port_cfg_t. Previously, ports to be bound were
half-parsed in config.c, and later re-parsed in connection.c when
we're about to bind them. Now, parsing a port means converting it
into a port_cfg_t, and binding it uses only a port_cfg_t, without
needing to parse the user-provided strings at all.
We should do a related refactoring on other port types. For
control ports, that'll be easy enough. For ORPort and DirPort,
we'll want to do this when we solve proposal 118 (letting servers
bind to and advertise multiple ports).
This implements tickets 3514 and 3515.
This adds a little code complexity: we need to remember for each
node whether it supports the right feature, and then check for each
connection whether it's exiting at such a node. We store this in a
flag in the edge_connection_t, and set that flag at link time.
- Conform to make check-spaces
- Build without warnings from passing size_t to %d
- Use connection_get_inbuf_len(), not buf_datalen (otherwise bufferevents
won't work).
- Don't log that we're using this feature at warn.
Previously we were using router_get_by_id(foo) to test "do we have a
descriptor that will let us make an anonymous circuit to foo". But
that isn't right for microdescs: we should have been using node_t.
Fixes bug 3601; bugfix on 0.2.3.1-alpha.
Instead, use compare_tor_addr_to_node_policy everywhere.
One advantage of this is that compare_tor_addr_to_node_policy can
better distinguish 0.0.0.0 from "unknown", which caused a nasty bug
with microdesc users.
Previously, we had an issue where we'd treat an unknown address as
0, which turned into "0.0.0.0", which looked like a rejected
address. This meant in practice that as soon as we started doing
comparisons of unknown uint32 addresses to short policies, we'd get
'rejected' right away. Because of the circumstances under which
this would be called, it would only happen when we had local DNS
cached entries and we were looking to launch new circuits.
* Add some utility transport functions in circuitbuild.[ch] so that we
can use them from pt.c.
* Make the accounting system consider traffic coming from proxies.
* Make sure that we only fetch bridge descriptors when all the
transports are configured.
Rationale: right now there seems to be no way for our bootstrap
status to dip under 100% once it has reached 100%. Thus, recording
broken connections after that point is useless, and wastes memory.
If at some point in the future we allow our bootstrap level to go
backwards, then we should change this rule so that we disable
recording broken connection states _as long as_ the bootstrap status
is 100%.
- We were reporting the _bottom_ N failing states, not the top N.
- With bufferevents enabled, we logged all TLS states as being "in
bufferevent", which isn't actually informative.
- When we had nothing to report, we reported nothing too loudly.
- Also, we needed documentation.
This code lets us record the state of any outgoing OR connection
that fails before it becomes open, so we can notice if they're all
dying in the same SSL state or the same OR handshake state.
More work is still needed:
- We need documentation
- We need to actually call the code that reports the failure when
we realize that we're having a hard time connecting out or
making circuits.
- We need to periodically clear out all this data -- perhaps,
whenever we build a circuit successfully?
- We'll eventually want to expose it to controllers, perhaps.
Partial implementation of feature 3116.
It's very easy for nodelist_add_node_family(sl,node) to accidentally
add 'node', and kind of hard to make sure that it omits it. Instead
of taking pains to leave 'node' out, let's instead make sure that we
always include it.
I also rename the function to nodelist_add_node_and_family, and
audit its users so that they don't add the node itself any longer,
since the function will take care of that for them.
Resolves bug 2616, which was not actually a bug.
Previously, we'd just take all the nodes in EntryNodes, see which
ones were already in the guard list, and add the ones that weren't.
There were some problems there, though:
* We'd add _every_ entry in EntryNodes, and add them in the order
they appeared in the routerlist. This wasn't a problem
until we added the ability to give country-code or IP-range
entries in the EntryNodes set, but now that we did, it is.
(Fix: We now shuffle the entry nodes before adding them; only
add up to 10*NumEntryGuards)
* We didn't screen EntryNodes for the Guard flag. That's okay
if the user has specified two or three entry nodes manually,
but if they have listed a whole subcontinent, we should
restrict ourselves to the entries that are currently guards.
(Fix: separate out the new guard from the new non-guard nodes,
and add the Guards first.)
* We'd prepend new EntryNodes _before_ the already configured
EntryNodes. This could lead to churn.
(Fix: don't prepend these.)
This patch also pre-screens EntryNodes entries for
reachableaddresses/excludenodes, even though we check for that
later. This is important now, since we cap the number of entries
we'll add.
Just looking at the "latest" consensus could give us a microdesc
consensus, if microdescs were enabled. That would make us decide
that every routerdesc was unlisted in the latest consensus and drop
them all: Ouch.
Fixes bug 3113; bugfix on 0.2.3.1-alpha.
We have an invariant that a node_t should have an md only if it has
a routerstatus. nodelist_purge tried to preserve this by removing
all nodes without a routerstatus or a routerinfo. But this left
nodes with a routerinfo and a microdesc untouched, even if they had
a routerstatus.
Bug found by frosty_un.
All of the routerset_contains*() functions return 0 if their
routerset_t argument is NULL. Therefore, there's no point in
doing "if (ExcludeNodes && routerset_contains*(ExcludeNodes...))",
for example.
This patch fixes every instance of
if (X && routerstatus_contains*(X,...))
Note that there are other patterns that _aren't_ redundant. For
example, we *don't* want to change:
if (EntryNodes && !routerstatus_contains(EntryNodes,...))
Fixes#2797. No bug here; just needless code.
Previously, we'd get a new descriptor for free when
public_server_mode() changed, since it would count as
affects_workers, which would call init_keys(), which would make us
regenerate a new descriptor. But now that we fixed bug 3263,
init_keys() is no longer necessarily a new descriptor, and so we
need to make sure that public_server_mode() counts as a descriptor
transition.
The issue was that we overlooked the possibility of reverse DNS success
at the end of connection_ap_handshake_socks_resolved(). Issue discovered
by katmagic, thanks!
Returning a tristate is needless here; we can just use the yielded
transport/proxy_type field to tell whether there's a proxy, and have
the return indicate success/failure.
Also, store the proxy_type in the or_connection_t rather than letting
it get out of sync if a configuration reload happens between launching
the or_connection and deciding what to say with it.
- const-ify some transport_t pointers
- Remove a vestigial argument to parse_bridge_line
- Make it compile without warnings on my laptop with
--enable-gcc-warnings
We were using strncpy before, which isn't our style for stuff like
this.
This isn't a bug, though: before calling strncpy, we were checking
that strlen(src) was indeed == HEX_DIGEST_LEN, which is less than
sizeof(dst), so there was no way we could fail to NUL-terminate.
Still, strncpy(a,b,sizeof(a)) is an idiom that we ought to squash
everyplace.
Fixes CID #427.
Using strncpy meant that if listenaddress were ever >=
sizeof(sockaddr_un.sun_path), we would fail to nul-terminate
sun_path. This isn't a big deal: we never read sun_path, and the
kernel is smart enough to reject the sockaddr_un if it isn't
nul-terminated. Nonetheless, it's a dumb failure mode. Instead, we
should reject addresses that don't fit in sockaddr_un.sun_path.
Coverity found this; it's CID 428. Bugfix on 0.2.0.3-alpha.
When we rejected a descriptor for not being the one we wanted, we
were letting the parsed descriptor go out of scope.
Found by Coverity; CID # 30.
Bugfix on 0.2.1.26.
(No changes file yet, since this is not in any 0.2.1.x release.)
Every node_t has either a routerinfo_t or a routerstatus_t, so every
node_t *should* have a nickname. Nonetheless, let's make sure in
hex_digest_nickname_matches().
Should quiet CID 434.
This is a little error-prone when the local has a different type
from the parameter, and is very error-prone with both have the same
type. Let's not do this.
Fixes CID #437,438,439,440,441.
For some inexplicable reason, Coverity departs from its usual
standards of avoiding false positives here, and warns about all
sscanf usage, even when the formatting strings are totally safe.
Addresses CID # 447, 446.
Previously, fetch_from_buf_socks() might return 0 if there was still
data on the buffer and a subsequent call to fetch_from_buf_socks()
would return 1. This was making some of the socks5 unit tests
harder to write, and could potentially have caused misbehavior with
some overly verbose SOCKS implementations. Now,
fetch_from_buf_socks() does as much processing as it can, and
returns 0 only if it really needs more data. This brings it into
line with the evbuffer socks implementation.
We added this back in 0649fa14 in 2006, to deal with the case where
the client unconditionally sent us authentication data. Hopefully,
that's not needed any longer, since we now can actually parse
authentication data.
This change also requires us to add and use a pair of
allocator/deallocator functions for socks_request_t, instead of
using tor_malloc_zero/tor_free directly.
In the code as it stood, we would accept any number of socks5
username/password authentication messages, regardless of whether we
had actually negotiated username/password authentication. Instead,
we should only accept one, and only if we have really negotiated
username/password authentication.
This patch also makes some fields of socks_request_t into uint8_t,
for safety.
Multiple Bridge lines can point to the same one ClientTransportPlugin
line, and we can have multiple ClientTransportPlugin lines in our
configuration file that don't match with a bridge. We also issue a
warning when we have a Bridge line with a pluggable transport but we
can't match it to a ClientTransportPlugin line.
* Renamed transport_info_t to transport_t.
* Introduced transport_get_by_name().
* Killed match_bridges_with_transports().
We currently *don't* detect whether any bridges miss their transports,
of if any transports miss their bridges.
* Various code and aesthetic tweaks and English language changes.
A couple of places in control.c were using connection_handle_write()
to flush important stuff (the response to a SIGNAL command, an
ERR-level status event) before Tor went down. But
connection_handle_write() isn't meaningful for bufferevents, so we'd
crash.
This patch adds a new connection_flush() that works for all connection
backends, and makes control.c use that instead.
Fix for bug 3367; bugfix on 0.2.3.1-alpha.
debug-level since it will be quite common. logged at both client
and server side. this step should help us track what's going on
with people filtering tor connections by our ssl habits.
This lets us make a lot of other stuff const, allows the compiler to
generate (slightly) better code, and will make me get slightly fewer
patches from folks who stick mutable stuff into or_options_t.
const: because not every input is an output!
Original message from bug3393:
check_private_dir() to ensure that ControlSocketsGroupWritable is
safe to use. Unfortunately, check_private_dir() only checks against
the currently running user… which can be root until privileges are
dropped to the user and group configured by the User config option.
The attached patch fixes the issue by adding a new effective_user
argument to check_private_dir() and updating the callers. It might
not be the best way to fix the issue, but it did in my tests.
(Code by lunar; changelog by nickm)
* Improved function documentation.
* Renamed find_bridge_transport_by_addrport() to
find_transport_by_bridge_addrport().
* Sanitized log severities we use.
* Ran check-spaces.
When we set a networkstatus in the non-preferred flavor, we'd check
the time in the current_consensus. But that might have been NULL,
which could produce a crash as seen in bug 3361.
When we added the check for key size, we required that the keys be
128 bytes. But RSA_size (which defers to BN_num_bytes) will return
128 for keys of length 1017..1024. This patch adds a new
crypto_pk_num_bits() that returns the actual number of significant
bits in the modulus, and uses that to enforce key sizes.
Also, credit the original bug3318 in the changes file.
UseBridges 1 now means "connect only to bridges; if you know no
bridges, don't make connections." UseBridges auto means "Use bridges
if they are known, and we have no EntryNodes set, and we aren't a
server." UseBridges 0 means "don't use bridges."
This merge was a bit nontrivial, since I had to write a new
node_is_a_configured_bridge to parallel router_is_a_configured_bridge.
Conflicts:
src/or/circuitbuild.c
options->DirPort is 0 in the unit tests, so
router_get_advertised_dir_port() would return 0 so we wouldn't pick a
dirport. This isn't what we want for the unit tests. Fixes bug
introduced in 95ac3ea594.
Previously, Tor would dereference a NULL pointer and crash if
lookup_last_hid_serv_request were called before the first call to
directory_clean_last_hid_serv_requests. As far as I can tell, that's
currently impossible, but I want that undocumented invariant to go away
in case I^Wwe break it someday.
If set to 1, Tor will attempt to prevent basic debugging
attachment attempts by other processes. (Default: 1)
Supports Mac OS X and Gnu/Linux.
Sebastian provided useful feedback and refactoring suggestions.
Signed-off-by: Jacob Appelbaum <jacob@appelbaum.net>
When we introduced NEED_KEY_1024 in routerparse.c back in
0.2.0.1-alpha, I forgot to add a *8 when logging the length of a
bad-length key.
Bugfix for 3318 on 0.2.0.1-alpha.
The patch for 3228 made us try to run init_keys() before we had loaded
our state file, resulting in an assert inside init_keys. We had moved
it too early in the function.
Now it's later in the function, but still above the accounting calls.
The conflicts were mainly caused by the routerinfo->node transition.
Conflicts:
src/or/circuitbuild.c
src/or/command.c
src/or/connection_edge.c
src/or/directory.c
src/or/dirserv.c
src/or/relay.c
src/or/rendservice.c
src/or/routerlist.c
The comment fixes are trivial. The defensive programming trick is to
tolerate receiving NULL inputs on the describe functions. That should
never actually happen, but it seems like the likeliest mistake for us
to make in the future.
Previously we did this nearer to the end (in the old_options &&
transition_affects_workers() block). But other stuff cares about
keys being consistent with options... particularly anything which
tries to access a key, which can die in assert_identity_keys_ok().
Fixes bug 3228; bugfix on 0.2.2.18-alpha.
Fixes part of #1297; bugfix on 48e0228f1e,
when circuit_expire_building was changed to assume that timestamp_dirty
was set when a circuit changed purpose to _C_REND_READY. (It wasn't.)
The previous attempt was incomplete: it told us not to publish a
descriptor, but didn't stop us from generating one. Now we treat an
absent OR port the same as not knowing our address. (This means
that when we _do_ get an OR port, we need to mark the descriptor
dirty.)
More attempt to fix bug3216.
This situation can happen easily if you set 'ORPort auto' and
'AccountingMax'. Doing so means that when you have no ORPort, you
won't be able to set an ORPort in a descriptor, so instead you would
just generate lots of invalid descriptors, freaking out all the time.
Possible fix for 3216; fix on 0.2.2.26-beta.
We had all the code in place to handle this right... except that we
were unconditionally opening a PF_INET socket instead of looking at
sa_family. Ow.
Fixes bug 2574; not a bugfix on any particular version, since this
never worked before.
Most instances were dead code; for those, I removed the assignments.
Some were pieces of info we don't currently plan to use, but which
we might in the future. For those, I added an explicit cast-to-void
to indicate that we know that the thing's unused. Finally, one was
a case where we were testing the wrong variable in a unit test.
That one I fixed.
This resolves bug 3208.
On win64, sockets are of type UINT_PTR; on win32 they're u_int;
elsewhere they're int. The correct windows way to check a socket for
being set is to compare it with INVALID_SOCKET; elsewhere you see if
it is negative.
On Libevent 2, all callbacks take sockets as evutil_socket_t; we've
been passing them int.
This patch should fix compilation and correctness when built for
64-bit windows. Fixes bug 3270.
We used to regenerate our descriptor whenever we'd get a sighup. This
was caused by a bug in options_transition_affects_workers() that would
return true even if the options were exactly the same. Down the call
path we'd call init_keys(), which made us make a new descriptor which
the authorities would reject, and the node would subsequently fall out
of the consensus.
This patch fixes only the first part of this bug:
options_transition_affects_workers() behaves correctly now. The second
part still wants a fix.
tor_process_monitor_new can't currently return NULL, but if it ever can,
we want that to be an explicitly fatal error, without relying on the fact
that monitor_owning_controller_process's chain of caller will exit if it
fails.
When we configure a new bridge via the controller, don't wait up to ten
seconds before trying to fetch its descriptor. This wasn't so bad when
you listed your bridges in torrc, but it's dreadful if you configure
your bridges via vidalia.
Bumped the char maximum to 512 for HTTPProxyAuthenticator &
HTTPSProxyAuthenticator. Now stripping all '\n' after base64
encoding in alloc_http_authenticator.
Rename crypto_pk_check_key_public_exponent to crypto_pk_public_exponent_ok:
it's nice to name predicates s.t. you can tell how to interpret true
and false.
Fixed a trivial conflict where this and the ControlSocketGroupWritable
code both added different functions to the same part of connection.c.
Conflicts:
src/or/connection.c
This patch introduces a few new functions in router.c to produce a
more helpful description of a node than its nickame, and then tweaks
nearly all log messages taking a nickname as an argument to call these
functions instead.
There are a few cases where I left the old log messages alone: in
these cases, the nickname was that of an authority (whose nicknames
are useful and unique), or the message already included an identity
and/or an address. I might have missed a couple more too.
This is a fix for bug 3045.
When running a system-wide instance of Tor on Unix-like systems, having
a ControlSocket is a quite handy mechanism to access Tor control
channel. But it would be easier if access to the Unix domain socket can
be granted by making control users members of the group running the Tor
process.
This change introduces a UnixSocketsGroupWritable option, which will
create Unix domain sockets (and thus ControlSocket) 'g+rw'. This allows
ControlSocket to offer same access control measures than
ControlPort+CookieAuthFileGroupReadable.
See <http://bugs.debian.org/552556> for more details.
This code changes it so that we don't remove bridges immediately when
we start re-parsing our configuration. Instead, we mark them all, and
remove all the marked ones after re-parsing our bridge lines. As we
add a bridge, we see if it's already in the list. If so, we just
unmark it.
This new behavior will lose the property we used to have that bridges
were in bridge_list in the same order in which they appeared in the
torrc. I took a quick look through the code, and I'm pretty sure we
didn't actually depend on that anywhere.
This is for bug 3019; it's a fix on 0.2.0.3-alpha.
rransom notes correctly that now that we aren't checking our HSDir
flag, we have no actual reason to check whether we are listed in the
consensus at all when determining if we should act like a hidden
service directory.
Previously, if they changed in torrc during a SIGHUP, all was well,
since we would just clear all transient entries from the addrmap
thanks to bug 1345. But if you changed them from the controller, Tor
would leave old mappings in place.
The VirtualAddrNetwork bug has been here since 0.1.1.19-rc; the
AutomapHosts* bug has been here since 0.2.0.1-alpha.
This bug couldn't happen when TrackExitHosts changed in torrc, since
the SIGHUP to reload the torrc would clear out all the transient
addressmap entries before. But if you used SETCONF to change
TrackExitHosts, old entries would be left alone: that's a bug, and so
this is a bugfix on Tor 0.1.0.1-rc.
If you really want to purge the client DNS cache, the TrackHostExits
mappings, and the virtual address mappings, you should be using NEWNYM
instead.
Fixes bug 1345; bugfix on Tor 0.1.0.1-rc.
Note that this needs more work: now that we aren't nuking the
transient addressmap entries on HUP, we need to make sure that
configuration changes to VirtualAddressMap and TrackHostExits actually
have a reasonable effect.
We'll eventually want to do more work here to make sure that the ports
are stable over multiple invocations. Otherwise, turning your node on
and off will get you a new DirPort/ORPort needlessly.
Otherwise, it will just immediately close any port declared with "auto"
on the grounds that it wasn't configured. Now, it will allow "auto" to
match any port.
This means FWIW if you configure a socks port with SocksPort 9999
and then transition to SocksPort auto, the original socksport will
not get closed and reopened. I'm considering this a feature.
HTTPS error code 403 is now reported as:
"The https proxy refused to allow connection".
Used a switch statement for additional error codes to be explained
in the future.
On IRC, wanoskarnet notes that if we ever do microdesc_free() on a
microdesc that's in the nodelist, we're in trouble. Also, we're in
trouble if we free one that's still in the microdesc_cache map.
This code adds a flag to microdesc_t to note where the microdesc is
referenced from, and checks those flags from microdesc_free(). I
don't believe we have any errors here now, but if we introduce some
later, let's log and recover from them rather than introducing
heisenbugs later on.
Addresses bug 3153.
The old behavior contributed to unreliability when hidden services and
hsdirs had different consensus versions, and so had different opinions
about who should be cacheing hsdir info.
Bugfix on 0.2.0.10-alpha; based on discussions surrounding bug 2732.
The new behavior is to try to rename the old file if there is one there
that we can't read. In all likelihood, that will fail too, but at least
we tried, and at least it won't crash.
Conflicts in various places, mainly node-related. Resolved them in
favor of HEAD, with copying of tor_mem* operations from bug3122_memcmp_022.
src/common/Makefile.am
src/or/circuitlist.c
src/or/connection_edge.c
src/or/directory.c
src/or/microdesc.c
src/or/networkstatus.c
src/or/router.c
src/or/routerlist.c
src/test/test_util.c
Conflicts throughout. All resolved in favor of taking HEAD and
adding tor_mem* or fast_mem* ops as appropriate.
src/common/Makefile.am
src/or/circuitbuild.c
src/or/directory.c
src/or/dirserv.c
src/or/dirvote.c
src/or/networkstatus.c
src/or/rendclient.c
src/or/rendservice.c
src/or/router.c
src/or/routerlist.c
src/or/routerparse.c
src/or/test.c
Here I looked at the results of the automated conversion and cleaned
them up as follows:
If there was a tor_memcmp or tor_memeq that was in fact "safe"[*] I
changed it to a fast_memcmp or fast_memeq.
Otherwise if there was a tor_memcmp that could turn into a
tor_memneq or tor_memeq, I converted it.
This wants close attention.
[*] I'm erring on the side of caution here, and leaving some things
as tor_memcmp that could in my opinion use the data-dependent
fast_memcmp variant.
Make that explicit by adding an assert and removing a null-check. All of
its callers currently depend on the argument being non-null anyway.
Silences a few clang complaints.
The analyzer assumed that bootstrap_percent could be less than 0 when we
call control_event_bootstrap_problem(), which would mean we're calling
log_fn() with undefined values. The assert makes it clear this can't
happen.
When configure tor with --enable-bufferevents and
--enable-static-libevent, libevent_openssl would still be linked
dynamically. Fix this and refactor src/or/Makefile.am along the way.
To make sure that a server learns if its IP has changed, the server
sometimes launches authority.z descriptor fetches from
update_router_descriptor_downloads. That's nice, but we're moving
towards a situation where update_router_descriptor_downloads doesn't
always get called. So this patch breaks the authority.z
check-and-fetch into a new function.
This function also renames last_routerdesc_download to a more
appropriate last_descriptor_download, and adds a new
update_all_descriptor_downloads() function.
(For now, this is unnecessary, since servers don't actually use
microdescriptors. But that could change, or bridges could start
using microdescriptors, and then we'll be glad this is refactored
nicely.)
To turn this on, set UseMicrodescriptors to "1" (or "auto" if you
want it on-if-you're-a-client). It should go auto-by-default once
0.2.3.1-alpha is released.
Because of our node logic, directory caches will never use
microdescriptors when they have the right routerinfo available.
See bug 2850 for rationale: it appears that on some busy exits, the OS
decides that every single port is now unusable because they have been
all used too recently.
Previously we ensured that it would get called periodically by doing
it from inside the code that added microdescriptors. That won't work
though: it would interfere with our code that tried to read microdescs
from disk initially. Instead, we should consider rebuilding the cache
periodically, and on startup.
Previously on 0.2.2, we'd never clean the cache. Now that we can
clean it, we want to add a condition to rebuild it: that should happen
whenever we have dropped enough microdescriptors that we could save a
lot of space.
No changes file, since 0.2.3 doesn't need one and 0.2.2 already has some
changes files for the backport of the microdesc_clean_cahce() function.
n_supported[i] has a random value prior to initialization, so a node
that doesn't have routerinfo available can have a random priority.
Patch contributed by wanoskarnet from #tor. Thanks!
Instead of just saying "boogity boogity!" let's actually warn people
that they need to configure stuff right to be safe, and point them
at instructions for how to do that.
Resolves bug 2474.
Clients and relays haven't used them since early 0.2.0.x. The only
remaining use by authorities learning about new relays ahead of scedule;
see proposal 147 for what we intend to do about that.
We're leaving in an option (FetchV2Networkstatus) to manually fetch v2
networkstatuses, because apparently dnsel and maybe bwauth want them.
This fixes bug 3022.
Previously it would erroneously return true if ListenAddr was set for
a client port, even if that port itself was 0. This would give false
positives, which were not previously harmful... but which were about
to become.
A v0 HS authority stores v0 HS descriptors in the same descriptor
cache that its HS client functionality uses. Thus, if the HS
authority operator clears its client HS descriptor cache, ALL v0
HS descriptors will be lost. That would be bad.
If the user sent a SIGNAL NEWNYM command after we fetched a rendezvous
descriptor, while we were building the introduction-point circuit, we
would give up entirely on trying to connect to the hidden service.
Original patch by rransom slightly edited to go into 0.2.1
Previously, it would remove every trackhostexits-derived mapping
*from* xyz.<exitname>.exit; it was supposed to remove every
trackhostexits-derived mapping *to* xyz.<exitname>.exit.
Bugfix on 0.2.0.20-rc: fixes an XXX020 added while staring at bug-1090
issues.
Resolved conflicts in:
doc/tor.1.txt
src/or/circuitbuild.c
src/or/circuituse.c
src/or/connection_edge.c
src/or/connection_edge.h
src/or/directory.c
src/or/rendclient.c
src/or/routerlist.c
src/or/routerlist.h
These were mostly releated to the routerinfo_t->node_t conversion.
Now we believe it to be the case that we never build a circuit for our
stream that has an unsuitable exit, so we'll never need to use such
a circuit. The risk is that we have some code that builds the circuit,
but now we refuse to use it, meaning we just build a bazillion circuits
and ignore them all.
This looked at first like another fun way around our node selection
logic: if we had introduction circuits, and we wound up building too
many, we would turn extras into general-purpose circuits. But when we
did so, we wouldn't necessarily check whether the general-purpose
circuits conformed to our node constraints. For example, the last
node could totally be in ExcludedExitNodes and we wouldn't have cared...
...except that the circuit should already be internal, so it won't get user
streams attached to it, so the transition should generally be allowed.
Add an assert to make sure we're right about this, and have it not
check whether ExitNodes is set, since that's irrelevant to internal
circuits.
IOW, if we were using TrackExitHosts, and we added an excluded node or
removed a node from exitnodes, we wouldn't actually remove the mapping
that points us at the new node.
Also, note with an XXX022 comment a place that I think we are looking
at the wrong string.
The routerset_equal function explicitly handles NULL inputs, so
there's no need to check inputs for NULL before calling it.
Also fix a bug in routerset_equal where a non-NULL routerset with no
entries didn't get counted as equal to a NULL routerset. This was
untriggerable, I think, but potentially annoying down the road.
ExcludeExitNodes foo now means that foo.exit doesn't work. If
StrictNodes is set, then ExcludeNodes foo also overrides foo.exit.
foo.exit , however, still works even if foo is not listed in ExitNodes.
This once maybe made sense when ExitNodes meant "Here are 3 exits;
use them all", but now it more typically means "Here are 3
countries; exit from there." Using non-Fast/Stable exits created a
potential partitioning opportunity and an annoying stability
problem.
(Don't worry about the case where all of our ExitNodes are non-Fast
or non-Stable: we handle that later in the function by retrying with
need_capacity and need_uptime set to 0.)
