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.
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.
