Opened 8 weeks ago

Closed 4 weeks ago

#21150 closed defect (fixed)

HiddenServiceStatistics gets set to 0 in your torrc, and stays 0 when you become a relay

Reported by: arma Owned by: dgoulet
Priority: Medium Milestone: Tor: 0.3.0.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

HiddenServiceStatistics defaults to 1:

  V(HiddenServiceStatistics,     BOOL,     "1"),

but if you're not a public relay, it gets set to 0 on startup:

    if (!public_server_mode(options)) {
      options->CellStatistics = 0;
      options->EntryStatistics = 0;
      options->ConnDirectionStatistics = 0;
      options->HiddenServiceStatistics = 0;
      options->ExitPortStatistics = 0;
    }

and then when you saveconf, it doesn't match the default so you get a new line in your torrc file:

HiddenServiceStatistics 0

And then later if you run your Tor as a relay, it looks at the torrc file and decides not to collect or report hiddenservicestatistics, which is bad because that's supposed to be the default.

I think the fix is that we need an internal variable which reflects what we've decided to do, and we need to stop modifying the config option in place.

The same issue happened in #4244, and I think we have some other recent tickets to make this change for other config options.

This ticket would resolve #17909 as well.

Child Tickets

Change History (16)

comment:1 follow-up: Changed 8 weeks ago by cypherpunks

No, respecting the user's torrc settings is not a bug ir a bad thing. The root problem here is Sponsor R, inappropriate closeness between Tor and the US government, and dictats from them being shamefully and ineffectively laundered as "community decisions".

comment:2 in reply to: ↑ 1 ; follow-ups: Changed 8 weeks ago by arma

Replying to cypherpunks:

No, respecting the user's torrc settings is not a bug ir a bad thing. The root problem here is Sponsor R, inappropriate closeness between Tor and the US government, and dictats from them being shamefully and ineffectively laundered as "community decisions".

Sorry Mr angry person, but you are mistaken.

If you want the default to be different, that's a different topic.

This bug is about Tor not respecting its default in a surprising way.

And the goal is that Tor *should* obey its torrc settings, and it shouldn't write a torrc line when the user didn't mean to or know it was doing it.

comment:3 in reply to: ↑ 2 ; follow-up: Changed 8 weeks ago by cypherpunks

Replying to arma:

the user didn't mean to or know it was doing it.

How do you know? Did you ask them? I've known it did this for a long time.

surprising

Not to me.

comment:4 in reply to: ↑ 3 Changed 8 weeks ago by arma

Replying to cypherpunks:

Replying to arma:

the user didn't mean to or know it was doing it.

How do you know? Did you ask them? I've known it did this for a long time.

To be clear, I think Tor clients should continue acting as though HiddenServiceStatistics is set to 0 when public_server_mode() is false. They just shouldn't mistakenly think that the user explicitly set a value in their torrc when they didn't.

comment:5 Changed 7 weeks ago by nickm

  • Milestone set to Tor: 0.3.0.x-final

comment:6 Changed 6 weeks ago by dgoulet

  • Owner set to dgoulet
  • Status changed from new to accepted

comment:7 follow-up: Changed 6 weeks ago by dgoulet

  • Status changed from accepted to needs_information

Hrm the solution to this is actually a bit involving... We would need some sort of global or_options_t that is *exactly* what's in your torrc (or modified by control port) which saveconf would use instead of the current global one that "tor" uses. Else we have a version of the option with let say an underscore at the end which represents what tor should use and the one with out is from the configuration file.

Both solutions requires lots of work.

Maybe a better solution is to set the default to auto and then 1 if relay 0 if not?

comment:8 in reply to: ↑ 7 ; follow-up: Changed 6 weeks ago by arma

Replying to dgoulet:

Else we have a version of the option with let say an underscore at the end which represents what tor should use and the one with out is from the configuration file.

This approach is what I was suggesting, and it's what we did for DirReqStatistics in #4244. For that one, see commit 905443 for how it was done.

Both solutions requires lots of work.

"3 files changed, 17 insertions(+), 8 deletions(-)" doesn't seem so bad. :)

Maybe a better solution is to set the default to auto and then 1 if relay 0 if not?

I could get behind that solution too. It looks like it should work too.

Speaking of which, check out this clause in config.c:

    if (!public_server_mode(options)) {
      options->CellStatistics = 0;
      options->EntryStatistics = 0;
      options->ConnDirectionStatistics = 0;
      options->HiddenServiceStatistics = 0;
      options->ExitPortStatistics = 0;
    }

So we're violating the #4244 rule of "thou shalt not modify the options to make it look like the user picked A when actually it said B" on all of these config options -- it just happens that the other ones already default to 0 so they aren't being noticed yet.

Should we make all of those auto too? And then have some function somewhere with the "real" defaults for them? Hm.

comment:9 in reply to: ↑ 8 Changed 6 weeks ago by arma

Replying to arma:

So we're violating the #4244 rule of "thou shalt not modify the options to make it look like the user picked A when actually it said B" on all of these config options -- it just happens that the other ones already default to 0 so they aren't being noticed yet.

Thinking a bit more on it, I think those other config options, where they default to 0 but we set them to 0 explicitly if we're not a relay, are also bugs: imagine the user set them to 1 in their torrc, but then spent a bit of time not being a relay. saveconf would quietly set them to 0 in their torrc. That can't be what the user expects.

comment:10 in reply to: ↑ 2 Changed 6 weeks ago by cypherpunks

Replying to arma:

Replying to cypherpunks:

No, respecting the user's torrc settings is not a bug or a bad thing. The root problem here is Sponsor R, inappropriate closeness between Tor and the US government, and dictats from them being shamefully and ineffectively laundered as "community decisions".

Sorry Mr angry person, but you are mistaken.

If you want the default to be different, that's a different topic.

The default should be 0. And let relay operators who 1. understand what it does and 2. want to do it, set it to 1 if they wish. Not the other way round!

comment:11 Changed 5 weeks ago by dgoulet

  • Status changed from needs_information to assigned

comment:12 Changed 5 weeks ago by dgoulet

  • Status changed from assigned to needs_review

I took a stab at this with bug21150_030_01.

One issue that we might not care about or might be expected is the GETCONF HiddenServiceStatistics issue. With this patch, if you are not a relay, we set HiddenServiceStatistics to 0 in our internal representation but the GETCONF command returns the torrc option which is default to 1. So actually, GETCONF is half lying. (Btw, same issue with DirReqStatistics that was fixed with this in 905443f074088cbc6322341e5c1cd6b35cd363a8).

comment:13 Changed 4 weeks ago by nickm

lgtm but needs a changes file.

comment:14 Changed 4 weeks ago by nickm

  • Status changed from needs_review to needs_revision

comment:15 Changed 4 weeks ago by dgoulet

  • Status changed from needs_revision to merge_ready

See fixup commit ca8fe58.

comment:16 Changed 4 weeks ago by nickm

  • Resolution set to fixed
  • Status changed from merge_ready to closed

Merged! (Next time, be sure to run "make check" -- there was a whitespace error.)

Note: See TracTickets for help on using tickets.