Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug #27049: Consider hs circs before deciding no circuits are open. #263

Open
wants to merge 4 commits into
base: maint-0.3.3
Choose a base branch
from

Conversation

Labels
None yet
Projects
None yet
4 participants
@mikeperry-tor
Copy link
Contributor

@mikeperry-tor mikeperry-tor commented Aug 6, 2018

No description provided.

@coveralls
Copy link

@coveralls coveralls commented Aug 22, 2018

Pull Request Test Coverage Report for Build 3183

  • 5 of 7 (71.43%) changed or added relevant lines in 1 file are covered.
  • 2532 unchanged lines in 21 files lost coverage.
  • Overall coverage decreased (-0.3%) to 57.943%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/or/circuitlist.c 5 7 71.43%
Files with Coverage Reduction New Missed Lines %
src/or/geoip.c 1 67.12%
src/or/rephist.c 2 36.55%
src/or/dns.c 3 15.84%
src/or/shared_random.c 3 82.53%
src/or/channelpadding.c 4 97.3%
src/or/routerparse.c 9 75.99%
src/or/main.c 13 22.41%
src/common/address.c 21 90.24%
src/or/protover.c 26 92.35%
src/common/tortls.c 28 96.08%
Totals Coverage Status
Change from base Build 1593: -0.3%
Covered Lines: 40295
Relevant Lines: 69542

💛 - Coveralls

@@ -625,7 +625,7 @@ circuit_any_opened_circuits(void)
TO_CIRCUIT(next_circ)->state == CIRCUIT_STATE_OPEN &&
TO_CIRCUIT(next_circ)->purpose != CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT &&
next_circ->build_state &&
next_circ->build_state->desired_path_len == DEFAULT_ROUTE_LEN) {
next_circ->build_state->desired_path_len >= DEFAULT_ROUTE_LEN) {
Copy link
Contributor

@dgoulet-tor dgoulet-tor Aug 28, 2018

How does that work with single onion service circuits? I'm not entirely sure, because of preemptive circuit building, if a tor client can NOT have at least 3-hop circuits but I'm guessing their could be a window that it can happen?

Copy link
Contributor Author

@mikeperry-tor mikeperry-tor Dec 3, 2018

Hrmm yeah. Ok. We should count SoS circuits here, too. Tried to do that in this commit: 8c950e7

changes/bug27409 Outdated
@@ -0,0 +1,6 @@
o Minor bugfixes (onion services):
- When circuits timeout, count other other onion service circuits when
Copy link
Contributor

@dgoulet-tor dgoulet-tor Aug 28, 2018

Double "other" ?

Copy link
Contributor Author

@mikeperry-tor mikeperry-tor Dec 3, 2018

if (!circ->build_state)
return 0;

if ((circ->rend_data || circ->hs_ident) &&
Copy link
Contributor

@teor2345 teor2345 Dec 4, 2018

Does this test cover single-hop into and rend circuits?
(HSDir circuits are always 3 hops or more for single onion services.)

Copy link
Contributor

@dgoulet-tor dgoulet-tor Dec 4, 2018

Every HS circuits have the identifier attached to them. I think this is enough to cover the intro/rend circuits because of the path length of 1.

The HSDir circuit with 3 hops will be caught by the next condition below it seems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment