Implement a way to tell if an IP address is a known relay

For the DoS mitigation subsystem (legacy/trac#24902 (moved)), we need a way to lookup the connection address and quickly know if it is a known relay or not. This is also needed for legacy/trac#2667 (moved) which would prevent Exit to reenter the network.

nickm has started a branch to have IP addresses with bloom filter: address_set_029.

Next step is to use that and bridge it with the nodelist so we can lookup an IP address and learn if it is known or not (not get the node_t back, that would require more work and probably lose in performance).

changed milestone to %Tor: 0.3.3.x-final in legacy/trac

added 029-backport in Legacy / Trac 031-backport in Legacy / Trac 032-backport in Legacy / Trac component::core tor/tor in Legacy / Trac milestone::Tor: 0.3.3.x-final in Legacy / Trac owner::dgoulet in Legacy / Trac parent::24902 in Legacy / Trac priority::medium in Legacy / Trac resolution::fixed in Legacy / Trac reviewer::dgoulet in Legacy / Trac severity::normal in Legacy / Trac status::closed in Legacy / Trac tor-dos in Legacy / Trac tor-relay in Legacy / Trac type::defect in Legacy / Trac labels

I've added some "is this a known relay" code to that branch too. Still needs tests though; dgoulet has offered to write them.

I've written unit tests for both using address_set_t and the integration with the nodelist.

Quick thing though, I've renamed addressset.c to address_set.c because that triple 's' was kind of intense and difficult to brain-parse on what it is exactly. Furthermore, the namespace for this file is address_set_* so that just makes sense I think.

Branch: ticket25183_029_01

Trac:
Status: assigned to needs_review

Got a lgtm by nickm on IRC.

So we'll make this part of legacy/trac#24902 (moved) so this is now merged in branch ticket24902_029_05.

Going back in accepted and we'll close it once legacy/trac#24902 (moved) is closed and backported.

Trac:
Owner: nickm to dgoulet
Status: needs_review to accepted

Merged into master. Will be backported through legacy/trac#24902 (moved).

Trac:
Resolution: N/A to fixed
Status: accepted to closed
Reviewer: N/A to dgoulet

There's a comment typo in address_set.c:

since we split the siphash outcome two 32-bit values

since we split the siphash outcome into two 32-bit values

Trac:
Status: closed to reopened
Resolution: fixed to N/A

Trac:
Status: reopened to needs_revision

Also, if we ever store an AF_UNSPEC address, these lines should probably hash the constant:

  case AF_UNSPEC:
    return 0x4e4d5342;

For consistency, we should hash the constant in both the keyed and unkeyed versions of the function.

Replying to teor:

Also, if we ever store an AF_UNSPEC address, these lines should probably hash the constant: {{{ case AF_UNSPEC: return 0x4e4d5342; }}}

For consistency, we should hash the constant in both the keyed and unkeyed versions of the function.

Not sure here... First, I don't think it should ever be called with AF_UNSPEC but apart from that, imo we want to save CPU cycles as much as possible since this is called very very often. Hashing a constant value will just make us be less efficient without any gain. I would probably go for a comment explaining why we do return that value or wrap it in some macro with a semantic that describes it as a "hash value".

And I believe the goal behind having a constant value for the AF_UNSPEC here is that if we ever store one or a billion of those, the function is to return quickly "yes we do have an unspecified address". I honestly don't see a use case for that apart from very efficiently ignoring any of them if they ever go through that function.

nickm just fixed both above in nickm/ticket24902_029_05.

I've pushed those two commits in my ticket24902_029_05 branch.

Trac:
Resolution: N/A to fixed
Status: needs_revision to closed

To give my rationale for making the AF_UNSPEC change: I think it's harmless to have AF_UNSPEC be a constant in the current usage, but it's fragile. When we promise a keyed hash, we ought to implement one, or we might find ourselves surprised later on.

closed

mentioned in issue legacy/trac#25193 (moved)

moved from legacy/trac#25183 (moved)

added Bug label and removed 1 deleted label

added Relay + 1 deleted label and removed 1 deleted label

added DoS label and removed 1 deleted label