I had been asking myself, "hey, doesn't the new code need to look at
this "info" parameter? The old code did!" But it turns out that the
old code hasn't, since 05f7336624.
So instead of "support this!" the comment now says "we can remove
this!"
George pointed out that (-1,0,1) for (never usable, maybe usable
later, usable right now) was a pretty rotten convention that made
the code harder to read.
Here we handle most (all?) of the remaining tasks, and fix some
bugs, in the prop271 bridge implementation.
* We record bridge identities as we learn them.
* We only call deprecated functions from bridges.c when the
deprecated guard algorithm is in use.
* We update any_bridge_descriptors_known() and
num_bridges_usable() to work correctly with the new backend
code. (Previously, they called into the guard selection logic.
* We update bridge directory fetches to work with the new
guard code.
* We remove some erroneous assertions where we assumed that we'd
never load a guard that wasn't for the current selection.
Also, we fix a couple of typos.
Still missing is functionality for picking bridges when we don't
know a descriptor for them yet, and functionality for learning a
bridge ID.
Everything else remains (basically) the same. Neat!
This includes:
* making bridge_info_t exposed but opaque
* allowing guards where we don't know an identity
* making it possible to learn the identity of a guard
* creating a guard that lacks a node_t
* remembering a guard's address and port.
* Looking up a guard by address and port.
* Only enforcing the rule that we need a live consensus to update
the "listed" status for guards when we are not using bridges.
This is safe, because no entry_guard_t ever outlives its
guard_selection_t.
I want this because now that multiple guard selections can be active
during one tor session, we should make sure that any information we
register about guards is with respect to the selection that they came
from.
Currently, this code doesn't actually have the contexts behave
differently, (except for the legacy context), but it does switch
back and forth between them nicely.
If a guard becomes primary as a result of confirming it, consider
the circuit through that guard as a primary circuit.
Also, note open questions on behavior when confirming nonprimary guards
Some of these will get torrc options to override them too; this
is just the mechanical conversion.
Also, add documentation for a couple of undocumented (but now used)
parameters.
The new HS circuitmap API replaces old public functions as follows:
circuit_clear_rend_token -> hs_circuitmap_remove_circuit
circuit_get_rendezvous -> hs_circuitmap_get_rend_circ
circuit_get_intro_point -> hs_circuitmap_get_intro_circ_v2
circuit_set_rendezvous_cookie -> hs_circuitmap_register_rend_circ
circuit_set_intro_point_digest -> hs_circuitmap_register_intro_circ_v2
This commit also removes the old rendinfo code that is now unused.
It also fixes the broken rendinfo unittests.
The HS circuitmap is a hash table that maps introduction and rendezvous
tokens to specific circuits such that given a token it's easy to find
the corresponding circuit. It supports rend circuits and v2/v3 intro
circuits.
It will be used by the prop224 ESTABLISH_INTRO code to register and
lookup v3 introduction circuits.
The next commit after this removes the old code and fixes the unittests.
Please consult both commits while reviewing functionality differences
between the old and new code. Let me know if you want this rebased
differently :)
WRT architectural differences, this commit removes the rendinfo pointer
from or_circuit_t. It then adds an hs_token_t pointer and a hashtable
node for the HS circuitmap. IIUC, this adds another pointer to the
weight of or_circuit_t. Let me know if you don't like this, or if you
have suggestions on improving it.
Back when Roger had do do most of our testing on the moria host, we
needed a higher limit for the number of relays running on a single
IP address when that limit was shared with an authority. Nowadays,
the idea is pretty obsolete.
Also remove the router_addr_is_trusted_dir() function, which served
no other purpose.
Closes ticket 20960.
In c35fad2bde, merged in
0.2.4.7-alpha, we removed the code to parse v1 directory
objects. When we did so, we removed everything that could set the
CST_CHECK_AUTHORITY flag for check_signature_token().
So in this code, we remove the flag itself, the code to handle the
flag, and a function that only existed to handle the flag.
The signed_descriptor_move() was not releasing memory inside the destination
object before overwriting it with the source object. This commit adds a reset
function that free that memory inside a signed descriptor object and zero it.
Closes#20715.
Signed-off-by: David Goulet <dgoulet@torproject.org>
If a node can prove its Ed25519 identity, don't consider connections
to it canonical unless they match both identities.
Includes link handshake changes needed to avoid crashing with bug
warnings, since the tests now reach more parts of the code.
Closes ticket 20355
(Only run the connection_or_group_set_badness_() function on groups
of channels that have the same RSA and Ed25519 identities.)
There's a possible opportunity here where we might want to set a
channel to "bad" if it has no ed25519 identity and some other
channel has some. Also there's an opportunity to add a warning if
we ever have an Ed mismatch on open connections with the same RSA
ID.
This function has never gotten testing for the case where an
identity had been set, and then got set to something else. Rather
than make it handle those cases, we forbid them.
If there is some horrible bug in our ed25519 link authentication
code that causes us to label every single ed25519-having node as
non-running, we'll be glad we had this. Otherwise we can remove it
later.
This patch makes two absolutely critical changes:
- If an ed25519 identity is not as expected when creating a channel,
we call that channel unsuccessful and close it.
- When a client creating a channel or an extend cell for a circuit, we
only include the ed25519 identity if we believe that the node on
the other side supports ed25519 link authentication (from
#15055). Otherwise we will insist on nodes without the right
link protocol authenticating themselves.
- When deciding to extend to another relay, we only upgrade the
extend to extend by ed25519 ID when we know the ed25519 ID _and_
we know that the other side can authenticate.
This patch also tells directory servers, when probing nodes, to
try to check their ed25519 identities too (if they can authenticate
by ed25519 identity).
Also, handle the case where we connect by RSA Id, and learn the
ED25519 ID for the node in doing so.
I need to be able to turn on Ed25519 support in client generation
of extend cells so I can test it, but leave it off-by-default until
enough clients support it for us to turn it on for a bunch at once.
This is part of #15056 / prop#220.
- forbid extending to the previous hop by Ed25519 ID.
- If we know the Ed25519 ID for the next hop and the client doesn't,
insist on the one from the consensus.
Right now, there's only a mechanism to look for a channel where the
RSA ID matches *and* the ED ID matches. We can add a separate map
later if we want.
This resolves two issues:
* the checks in rend_add_services were only being performed when adding
the service, and not when the service was validated,
(this meant that duplicate checks were not being performed, and some SETCONF
commands appeared to succeed when they actually failed), and
* if one service failed while services were being added, then the service
list would be left in an inconsistent state (tor dies when this happens,
but the code is cleaner now).
Fixes#20860.
When computing old Tor protocol line version in protover, we were looking at
0.2.7.5 twice instead of the specific case for 0.2.9.1-alpha.
Fixes#20810
Signed-off-by: David Goulet <dgoulet@torproject.org>
newconn->address is strdup'ed twice when new_type == CONN_TYPE_AP
and conn->socket_family == AF_UNIX. Whilst here, juggle code to
make sure newconn->port is assigned from an initialised value in
the above case.
Instead, refuse to start tor until the misconfigurations have been corrected.
Fixes bug 20559; bugfix on multiple commits in 0.2.7.1-alpha and earlier.
Instead, refuse to start tor if any hidden service key has been used in
a different hidden service anonymity mode.
Fixes bug 20638; bugfix on 17178 in 0.2.9.3-alpha; reported by ahf.
The original single onion service poisoning code checked poisoning state
in options_validate, and poisoned in options_act. This was problematic,
because the global array of hidden services had not been populated in
options_validate (and there were ordrering issues with hidden service
directory creation).
This patch fixes this issue in rend_service_check_dir_and_add, which:
* creates the directory, or checks permissions on an existing directory, then
* checks the poisoning state of the directory, then
* poisons the directory.
When validating, only the permissions checks and the poisoning state checks
are perfomed (the directory is not modified).
To do this, it makes sense to treat legacy guards as a separate
guard_selection_t *, and handle them separately. This also means we
add support here for having multiple guard selections.
Note that we don't persist pathbias information yet; that will take
some refactoring.
This patch doesn't cover every case; omitted cases are marked with
"XXXX prop271", as usual. It leaves both the old interface and the
new interface for guard status notification, since they don't
actually work in the same way: the new API wants to be told when a
circuit has failed or succeeded, whereas the old API wants to know
when a channel has failed or succeeded.
I ran into some trouble with directory guard stuff, since when we
pick the directory guard, we don't actually have a circuit to
associate it with. I solved that by allowing guard states to be
associated with directory connections, not just circuits.
I expect we'll be ripping this out somewhere in 0.3.0, but let's
keep it around for a little while in case it turns out to be the
only way to avert disaster?
This state corresponds to the WAITING_FOR_BETTER_GUARD state; it's
for circuits that are 100% constructed, but which we won't use until
we are sure that we wouldn't use circuits with a better guard.
When a nonprimary guard's circuit is complete, we don't call it
actually usable until we are pretty sure that every better guard
is indeed not going to give us a working circuit.
Here we add a little bit of state to origin circuits, and set up
the necessary functions for the circuit code to call in order to
find guards, use guards, and decide when circuits can be used.
There's also an incomplete function for the hard part of the
circuit-maintenance code, where we figure out whether any waiting
guards are ready to become usable.
(This patch finally uses the handle.c code to make safe handles to
entry_guard_t objects, so that we are allowed to free an
entry_guard_t without checking whether any origin_circuit_t is
holding a reference to it.)
This code handles:
* Maintaining the sampled set, the filtered set, and the
usable_filtered set.
* Maintaining the confirmed and primary guard lists.
* Picking guards for circuits, and updating guard state when
circuit state changes.
Additionally, I've done code structure movement: even more constants
and structures from entrynodes.c have become ENTRYNODES_PRIVATE
fields of entrynodes.h.
I've also included a bunch of documentation and a bunch of unit
tests. Coverage on the new code is pretty high.
I've noted important things to resolve before this branch is done
with the /XXXX.*prop271/ regex.
These are taken from the proposal, and defined there. Some of them
should turn into consensus parameters.
Also, remove some dead code that was there to make compilation work,
and use ATTR_UNUSED like a normal person.
The previous commit, in moving a bunch of functions to bridges.c,
broke compilation because bridges.c required two entry points to
entrynodes.c it didn't have.
This patch is just:
* Code movement
* Adding headers here and there as needed
* Adding a bridges_free_all() with a call to it.
It breaks compilation, since the bridge code needed to make exactly
2 calls into entrynodes.c internals. I'll fix those in the next
commit.
The encoding code is very straightforward. The decoding code is a
bit tricky, but clean-ish. The sampling code is untested and
probably needs more work.
This was a relatively mechanical change. First, I added an accessor
function for the pathbias-state field of a guard. Then I did a
search-and-replace in circpathbias.c to replace "guard->pb." with
"pb->". Finally, I made sure that "pb" was declared whenever it was
needed.
The entry_guard_t structure should really be opaque, so that we
can change its contents and have the rest of Tor not care.
This commit makes it "mostly opaque" -- circpathbias.c can still see
inside it. (I'm making circpathbias.c exempt since it's the only
part of Tor outside of entrynodes.c that made serious use of
entry_guard_t internals.)
This affects clients with FetchUselessDescriptors 1.
It might also cause subtle bugs on directory mirrors and authorities,
causing them to consider all full descriptors as failed or old.
Improve the messages logged when Tor wants or needs to load the master ed25519 identity key so the user is explicitly informed when further action is required or not. Fixes ticket #20650.
