Opened 7 years ago

Closed 18 months ago

#8323 closed enhancement (fixed)

Missing 'GETINFO md/all'

Reported by: atagar Owned by: rl1987
Priority: High Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-client tor-control microdesc
Cc: Actual Points:
Parent ID: Points:
Reviewer: asn Sponsor:

Description

Hi Nick. The control spec has GETINFO options to fetch a microdescriptor via its fingerprint or nickname ('md/id/*' and 'md/name/*') but no method for getting them all (like 'ns/all' or 'desc/all-recent').

Processing all descriptors is a pretty common need, hence the priority (feel free to lower it if appropriate). In the meantime I'll hack around this in stem by having controller read microdescriptors from the data directory.

Child Tickets

TicketTypeStatusOwnerSummary
#26211defectclosedtorspec update for GETINFO md/all

Change History (18)

comment:1 Changed 7 years ago by nickm

Keywords: tor-client added
Milestone: Tor: 0.2.5.x-final

I'll leave this as major. Timing makes it 0.2.5.

I'm assuming it's not a problem for you that microdescriptors don't tell you which microdescriptor they are unless you compute their SHA256 themselves? That is, unless you know the corresponding entry from the consensus networkstatus, you won't know which microdesc is which node.

comment:2 Changed 7 years ago by atagar

I'm assuming it's not a problem for you that microdescriptors don't tell you which microdescriptor they are unless you compute their SHA256 themselves?

It's not ideal. I was a bit sad yesterday when I realized that they lack the fingerprint since it changes simple tasks like 'tell me all of the exits' from...

for desc in controller.get_microdescriptors():
  if desc.exit_policy.is_exiting_allowed():
    print "%s is an exit" % desc.fingerprint

... to...

exit_onion_keys = set()

for desc in controller.get_microdescriptors():
  if desc.exit_policy.is_exiting_allowed():
    exit_onion_keys.add(desc.onion_key)

for desc in controller.get_network_statuses():
  if desc.digest in exit_onion_keys:
    print "%s is an exit" % desc.fingerprint

But not the end of the world. :)

comment:3 Changed 7 years ago by nickm

FWIW, that's almost right, but the thing that you have in the networkstatus is not a digest of the onion key, but rather a digest of the microdescriptor. So you have to do something like :

for desc in controller.get_microdescriptors():
  if desc.exit_policy.is_exiting_allowed():
    exit_digests.add(sha256(str(desc.text))) # or whatever this is called

for desc in controller.get_network_statuses():
  if desc.digest in exit_digests:
    print "%s is an exit" % desc.fingerprint

comment:4 Changed 6 years ago by nickm

Milestone: Tor: 0.2.5.x-finalTor: 0.2.???

comment:5 Changed 3 years ago by teor

Milestone: Tor: 0.2.???Tor: 0.3.???

Milestone renamed

comment:6 Changed 3 years ago by nickm

Keywords: tor-03-unspecified-201612 added
Milestone: Tor: 0.3.???Tor: unspecified

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

comment:7 Changed 3 years ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:8 Changed 3 years ago by nickm

Keywords: tor-control microdesc added
Severity: Normal
Type: defectenhancement

comment:9 Changed 19 months ago by rl1987

Owner: set to rl1987
Status: newaccepted

comment:10 Changed 19 months ago by rl1987

Status: acceptedneeds_review

https://github.com/torproject/tor/pull/122

I suppose integration test should be done in stem?

comment:11 Changed 19 months ago by teor

Milestone: Tor: unspecifiedTor: 0.3.5.x-final

Putting in 0.3.5 because 0.3.4 is closed to new features.

Do we have unit tests for control commands?
If we do, then please add a unit test.

Yes, please add a stem integration test, maybe ask atagar how it's done.

And we'll also need a control spec patch.

Please open child tickets for stem and the control spec, so we can merge those patches separately if we need to.

comment:12 Changed 18 months ago by dgoulet

Reviewer: asn

comment:13 Changed 18 months ago by asn

Status: needs_reviewneeds_revision

Thanks for the code!

I did an initial review on your PR! Some small changes will be needed, but you are pretty much there.

Also, perhaps we should have some unittests for this new control command. SEe src/test/test_controller.c for some examples of other getinfo unittests.

Cheers!

comment:14 Changed 18 months ago by rl1987

Status: needs_revisionneeds_review

Fixed the code to use existing function and wrote unit tests.

comment:15 Changed 18 months ago by rl1987

comment:16 Changed 18 months ago by asn

Status: needs_reviewmerge_ready

LGTM!

comment:17 Changed 18 months ago by nickm

Merged!

Please close or un-parent the child tickets as appropriate, and then close this?

comment:18 Changed 18 months ago by teor

Resolution: fixed
Status: merge_readyclosed

#26210 hasn't been implemented, so I un-parented it.

Note: See TracTickets for help on using tickets.