fix/enhance getinfo ns/id/* commands

1. Fix getinfo ns/id/* See the last few comments on #7059 (moved) with regards to the ns/id/* getinfo command.

Currently, it only returns information about tor nodes which have microdescriptors and ignores those that use "normal" descriptors, in other words, it behaves exactly like the md/id/node command.

I can understand the rationale behind having desc/id/node to only show information about tor nodes with "normal" descriptors and I can understand the reasons for md/id/node to do the same for tor nodes with microdescriptors, but ns/id/node should, in my view, be implemented in such a way to return information about all tor nodes, regardless of whether or not they use microdescriptors, particularly so because by definition of this command in control-specs.txt it is supposed to return V2 directory info on all nodes without making such distinctions.

2. Enhance ns/* - just an idea (not a bug!): currently, there is no way I could get information on a tor node by specifying its ip address.

I would love to be able to get that information by executing something like "getinfo ns/ip/<ip_address>" (note the "ip" in the middle!). This should return information on all tor nodes with the specified ip address (and/or netmask), regardless of whether they use microdescriptors or not.

Trac:
Username: mr-4

changed milestone to %Tor: unspecified

added component::core tor/tor milestone::Tor: unspecified needs-spec priority::medium reporter::mr-4 severity::normal status::new tor-client tor-control type::defect version::tor 0.2.4.7-alpha labels

Trac:
Username: mr-4
Version: N/A to Tor: 0.2.4.7-alpha

Adding a milestone so this doesn't get lost.

Trac:
Milestone: N/A to Tor: 0.2.5.x-final

Trac:
Keywords: N/A deleted, tor-client added

Trac:
Keywords: tor-client deleted, tor-client controller added

I tried fixing GETINFO ns/id/$DIGEST by searching the network status smartlist only and ignoring the microdescriptors smartlist. Is this approach valid? When I add UseMicrodescriptors 1 line to my torrc, GETINFO ns/id/$DIGEST doesn't return anything since Tor instance refrain from downloading network statuses when this configuration option is set. Is this okay? Does this bug even apply to modern Tor codebase, provided that it uses V3 directory protocol now?

Trac:
0001-GETINFO-ns-id-should-return-a-networkstatus-flavoure.patch

Damian, would this mess anything up? I wonder if we shouldn't just redesign the interface we want, and then implement it, rather than trying to reverse engineer what we meant when we designed the old thing.

Trac:
Status: new to needs_review
Cc: N/A to atagar

Currently, it only returns information about tor nodes which have microdescriptors and ignores those that use "normal" descriptors, in other words, it behaves exactly like the md/id/node command.

I'm not clear - (a) do you mean that it provides router status entries if and only if microdescriptors are available, or (b) it provides actual microdescriptors like md/id/node?

... this command in control-specs.txt it is supposed to return V2 directory info on all nodes without making such distinctions.

This was fixed in the spec a while back.

When I add UseMicrodescriptors 1 line to my torrc, GETINFO ns/id/$DIGEST doesn't return anything since Tor instance refrain from downloading network statuses when this configuration option is set.

... huh? UseMicrodescriptors 1 should mean that we download microdescriptors and router status entries, while UseMicrodescriptors 0 means that we download server descriptors and router status entries. Are you saying that we opt to only download microdescriptors by default nowadays?

Damian, would this mess anything up? I wonder if we shouldn't just redesign the interface we want, and then implement it, rather than trying to reverse engineer what we meant when we designed the old thing.

If mr-4 meant (a) then that sounds like a bug and fixing it will make controllers happy (... though I'm not sure why the relays you have microdescriptors for wouldn't match the router status entires). If he meant (b) then that sounds like a horrible bug and I'm not sure how it's possible that went undetected (... Stem has integ tests that should definitely complain about that).

Tor's "GETINFO ns/id/*" options are pretty nice as-is and used by Stem. I'm not sure what type of re-implementation you have in mind.

Cheers! -Damian

Replying to atagar:

Currently, it only returns information about tor nodes which have microdescriptors and ignores those that use "normal" descriptors, in other words, it behaves exactly like the md/id/node command.

I'm not clear - (a) do you mean that it provides router status entries if and only if microdescriptors are available, or (b) it provides actual microdescriptors like md/id/node?

The actual behavior is that it looks up the routerstatus item in the current consensus. If we're using a microdescriptor consensus, it looks at that. If we're using a networkstatus consensus, it looks at that.

It works even if we haven't fetched the microdescriptors themselves -- the issue is that it might return results from a FLAV_NS consensus.

... this command in control-specs.txt it is supposed to return V2 directory info on all nodes without making such distinctions.

This was fixed in the spec a while back.

When I add UseMicrodescriptors 1 line to my torrc, GETINFO ns/id/$DIGEST doesn't return anything since Tor instance refrain from downloading network statuses when this configuration option is set.

... huh? UseMicrodescriptors 1 should mean that we download microdescriptors and router status entries, while UseMicrodescriptors 0 means that we download server descriptors and router status entries. Are you saying that we opt to only download microdescriptors by default nowadays?

"NS" is the flavor of the old-style, pre-microdescriptor networkstatus consensus documents. I think that mr-4 is calling those networkstatuses. We don't download FLAV_NS networkstatuses by default any more when UseMicrodescriptors 0 is set; we download FLAV_MICRODESC networkstatuses instead.

Damian, would this mess anything up? I wonder if we shouldn't just redesign the interface we want, and then implement it, rather than trying to reverse engineer what we meant when we designed the old thing.

If mr-4 meant (a) then that sounds like a bug and fixing it will make controllers happy (... though I'm not sure why the relays you have microdescriptors for wouldn't match the router status entires). If he meant (b) then that sounds like a horrible bug and I'm not sure how it's possible that went undetected (... Stem has integ tests that should definitely complain about that).

Tor's "GETINFO ns/id/*" options are pretty nice as-is and used by Stem. I'm not sure what type of re-implementation you have in mind.

Hm. This suggests that the right thing to do here might be "no change in 0.2.5, just document the current behavior", and turn this into a needs-proposal thing.

Oooh, I see. Ewww, that is unpleasant.

This is a bug then that is indeed impacting Stem. Presently it assumes that the ns methods are giving us RouterStatusEntryV3 instances, but sometimes it's RouterStatusEntryMicroV3 instead. That would actually explain some confusing behavior I saw a while back.

For now I'll have Stem check UseMicrodescriptors so we can start providing the write instance types. Thanks for bringing this to my attention!

Pushed a fix to Stem to account for this.

Honestly I'm not quite sure what the right way forward is on Tor's side. If Tor only has microdescriptor router status entries available then that's clearly all it can provide controllers. Long term there's several changes I'd like in Tor's descriptors, but this isn't the place for that.

Actually... ug.

If I'm analyzing this right, the current behavior is just plain ugly. It takes the router status information, not from the node_t (where "our current opinion about the router" should be), but from the networkstatus_t (the thing we downloaded, which used to get modified with our opinions of node status, but which now is treated as immutable since we did the node_t refactor). And then it writes it in the old, pre-microdescriptor format...whether it's microdescriptor data or not, so the microdescriptor digest gets written out in the place that the descriptor digest should go, with its last 12 bytes omitted.

Yuck yuck yuck.

Trac:
Status: needs_review to needs_revision

tentatively moving these out of 0.2.5.

Trac:
Milestone: Tor: 0.2.5.x-final to Tor: 0.2.6.x-final

Trac:
Milestone: Tor: 0.2.6.x-final to Tor: 0.2.???
Status: needs_revision to new
Keywords: tor-client controller deleted, tor-client controller needs-spec added

Milestone renamed

Trac:
Milestone: Tor: 0.2.??? to Tor: 0.3.???

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

Trac:
Keywords: tor-client controller needs-spec deleted, tor-client, tor-03-unspecified-201612, controller, needs-spec added
Milestone: Tor: 0.3.??? to Tor: unspecified

Remove an old triaging keyword.

Trac:
Keywords: tor-03-unspecified-201612 deleted, N/A added

Unify controller keyword to "tor-control".

Unify "controller" keyword to "tor-control".

Trac:
Keywords: controller deleted, tor-control added

Set all open tickets without a severity to "Normal"

Trac:
Severity: N/A to Normal

The following may be relevant for this ticket. I don't think we have to have full duplicates for ControlPort commands because it confuses users. Now GETINFO commands dir/server/all and desc/all-recent return identical outputs if UseMicrodescriptors is set to 0. Which if these commands should be preferred if needed?

Trac:
Username: wagon
Sponsor: N/A to N/A
Reviewer: N/A to N/A

Now GETINFO dir/status-vote/current/consensus returns 6392 relays while GETINFO desc/all-recent returns 7294 relays. Why these numbers are different? Is it so, because descriptors are distributed in parallel to consensus and are not trusted until they enter consensus?

Trac:
Username: wagon

Replying to wagon:

The following may be relevant for this ticket. I don't think we have to have full duplicates for ControlPort commands because it confuses users.

But removing duplicates breaks old code, so we need to be careful when we do that. If you want to remove this duplicate, please open a separate ticket.

Now GETINFO commands dir/server/all and desc/all-recent return identical outputs if UseMicrodescriptors is set to 0. Which if these commands should be preferred if needed?

I don't know. If they return identical outputs for all Tor options, then it literally doesn't matter which one you use. If their outputs change when Tor's options change, then use the one that works for your code when the options change.

They are implemented differently, so dir/server/all might be more memory efficient in some cases.

You can also use md/all if you want microdescriptors, but only on Tor 0.3.5.1-alpha and later.

Replying to wagon:

Now GETINFO dir/status-vote/current/consensus returns 6392 relays while GETINFO desc/all-recent returns 7294 relays. Why these numbers are different? Is it so, because descriptors are distributed in parallel to consensus and are not trusted until they enter consensus?

Yes. You can check your answers to these questions by reading the specs: https://gitweb.torproject.org/torspec.git/tree/control-spec.txt#n637

https://gitweb.torproject.org/torspec.git/tree/control-spec.txt#n807 https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n3866

But removing duplicates breaks old code, so we need to be careful when we do that. If you want to remove this duplicate, please open a separate ticket. Well, I thought that if we have two options which do the same, one of them is deprecated and will cease to exist in some moment in future. However, you say that both of them are not deprecated now.

I asked this question because I'm not sure whether it is a bug or feature. So, if you say it is a feature, I shouldn't create a new ticket.

dir/server/all might be more memory efficient in some cases Thanks!

Trac:
Username: wagon

mentioned in issue #24110 (moved)

mentioned in issue #28676 (moved)

mentioned in issue #28982 (moved)

moved to tpo/core/tor#7646 (closed)