Give notifications if tor26 is missing from the consensus

It should be possible to infer whether tor26 is down or not by looking at the consensus. Would be nice to have that if it isn't too much work :)

added component::archived/doctor in Legacy / Trac owner::atagar in Legacy / Trac priority::medium in Legacy / Trac resolution::implemented in Legacy / Trac severity::normal in Legacy / Trac status::closed in Legacy / Trac type::defect in Legacy / Trac labels

Trac:
Component: Metrics/Consensus Health to Core Tor/DocTor
Owner: tom to atagar

Hi Sebastian. tor26 intentionally sandtraps DirPort requests without a '.z' suffix, in turn breaking DocTor. Excluding it is intentional...

https://gitweb.torproject.org/doctor.git/commit/?id=5bae2023

Trac:
Status: new to closed
Resolution: N/A to wontfix

atagar: would it be possible to look at the consensus and let us know if tor26 is missing from it? That has nothing to do with asking tor26 any direct questions.

We could. I'd prefer for tor26 to remove its block, but a patch is welcome if someone would care to make that adjustment.

why not have DocTor request compressed documents and/or obtain them from the OR port? re: legacy/trac#29186 (closed) legacy/trac#29240 (moved)

Hi starlight. DocTor does download compressed documents. tor26 simply breaks DirPort requests without a '.z' suffix.

interesting, so compression can be requested either via .z suffix or a HTTP attribute, why not just tack on the .z anyway?

Could. There's many hacks that can work around it. I just don't see merit in tor26's block.

If someone cares enough to submit a patch I'll merge it.

I can write one, an adjustment to _download_descriptors() in stem/descriptor/remote.py to append .z when compression is enabled covers it? Or more to it?

Before the making the change, I notice tor26 appears to have no problem serving descriptors and full consensus in PLAIN / GZIP / ZSTD form on both the dir and or ports to STEM. Perhaps the patch or iptables rule suppressing requests absent a .z suffix has been left out in recent deploys or eliminated?

an adjustment to _download_descriptors() in stem/descriptor/remote.py

Sorry, a patch to account for tor26's dysfunction will need to reside within DocTor rather than Stem.

I see

commit 5bae2023 Date: Wed Apr 11 08:58:31 2018 -0700
To cut down on abuse tor26 rejects DirPort requests without a '.z' suffix (technically it throttles them, but I've never seen one work so effectively it's blocked). Stem no longer appends this suffix, using headers to indicatee the compression it wants instead.

This could be solved by reverting 5bae2023 and 93c3a9b4 and making the change described above to STEM to add .z when any compression is requested, but it seems you dislike the approach. Or by checking the consensus for Tor26 as first indicated above, which is probably more work though I have not looked at at it. Which is "less bad?" If the .z hack were applied only when an option in STEM was present would it be more palpable? Alternately if DocTor were to check the consensus for tor26, it might as well check for the presence of all authorities and report any missing.

Seems to me nothing special should be required for testing DocTor except changing the email notification address, is is that true?

Which is "less bad?

The best option is for tor26 to remove its block. Second best is one of the following...

a. Do nothing (which is why I closed this ticket). b. Adjust DocTor's _get_documents() to monkey patch or do other hackery work around tor26's dysfunction. c. Adjust the has_authority_flag test to specially handle tor26.

Alternately if DocTor were to check the consensus for tor26, it might as well check for the presence of all authorities and report any missing.

Actually, DocTor already does...

https://gitweb.torproject.org/doctor.git/tree/consensus_health_checker.py#n632

Seems to me nothing special should be required for testing DocTor except changing the email notification address, is is that true?

You shouldn't need to do anything. When ran locally DocTor prints its notification rather than emailing it.

https://gitweb.torproject.org/doctor.git/tree/util.py#n21

Replying to atagar:

Alternately if DocTor were to check the consensus for tor26, it might as well check for the presence of all authorities and report any missing.

Actually, DocTor already does...

Would a new flag in the authority config that suppresses direct checking, in place of the present hard-coded exclusion for tor26 work? The config for tor26 goes back in with the flag set, direct checks are disabled and the existing in-consensus check will report when it's missing?

oops, I incorrectly projected 'data' had a config for authorities, so instead perhaps a dict with authorities to not check directly while eliminating the hard-coded tor26 removal from the list acquired from STEM via stem.directory.Authority.from_cache()?

next above seems a direct an reasonably clean way to solve it, easy to tweak to exclude different authorities that might become difficult to reach; will give that a try

Trac:
doctor-ticket32842.patch

git-am patch to resolve ticket

Uploaded fix, please review.

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

running 2to3 on all .py scripts updates DocTor to Python 3 successfully

Thanks starlight! Patch merged and deployed with a small adjustment.

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

I object to removing DIRAUTH_SKIP_REACHABLE and DIRAUTH_SKIP_SEEN:

a) spent several hours testing and making certain they work (touch-up fix attached) b) they are based on the code you had running in production c) because they are tested it will save time if/when needed, just add dirauth names d) required in particular when an authority is completely offline for several days or weeks

The touch-up patch contains:

Trailing comma in the single entry DIRAUTH_SKIP_CHECKS tuple def, otherwise it becomes a scalar and set() converts it to a set of characters. Important.

Fix to the seen-in-consensus test, must subtract the skip list from both known_authorities and seen_authorities.

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

Trac:
doctor-ticket32842-fix.patch

touch-up to patch

Thanks starlight, good catch. Pushed a fix for the tuples.

As for the unused constants I'd be amenable to a 'skip all checks for realz' constant for when a dirauth is down but as its operator I didn't find the 'skip reachable/seen' separation desirable.

Have you considered the case where an Authority is under prolonged DDoS
and, while functional cannot be reliably contacted?  That would be

DIRAUTH_SKIP_REACHABLE  yes
DIRAUTH_SKIP_SEEN       no
DIRAUTH_SKIP_CHECKS     yes

The case where an authority aggressively rate limits connections?

DIRAUTH_SKIP_REACHABLE  yes
DIRAUTH_SKIP_SEEN       no
DIRAUTH_SKIP_CHECKS     no

The recent case where dannenberg consensus could not sign the consensus?

DIRAUTH_SKIP_REACHABLE  no
DIRAUTH_SKIP_SEEN       no
DIRAUTH_SKIP_CHECKS     yes

A situation where an authority is not visible to the majority of
other authorities but is visible to DocTor?

DIRAUTH_SKIP_REACHABLE  no
DIRAUTH_SKIP_SEEN       yes
DIRAUTH_SKIP_CHECKS     yes

Of course the case where an authority is offline for a extended interval:

DIRAUTH_SKIP_REACHABLE  yes
DIRAUTH_SKIP_SEEN       yes
DIRAUTH_SKIP_CHECKS     yes

Hi starlight. I'm in the process of moving DocTor tickets to GitHub (https://github.com/torproject/doctor/). Your skipping mechanism is great, but at the end of the day it is a tool for DocTor's operator and as such their decision if they want it.

Would you like responsibility for running and administering DocTor? I wrote DocTor as a favor to Karsten, and I'm willing to transfer ownership if you want it. If you do then please file a ticket at https://github.com/torproject/doctor/issues and I'll discuss it with the dirauth operators.

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

closed

moved from legacy/trac#32842 (moved)

added Bug label and removed 1 deleted label

removed 1 deleted label