#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 Changed 11 months 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 ; Changed 11 months 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 ; Changed 11 months 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 11 months 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 11 months ago by nickm

Milestone: Tor: 0.3.0.x-final

comment:6 Changed 10 months ago by dgoulet

Owner: set to dgoulet
Status: newaccepted

comment:7 Changed 10 months ago by dgoulet

Status: acceptedneeds_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 ; Changed 10 months 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 10 months 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 10 months 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 10 months ago by dgoulet

Status: needs_informationassigned

comment:12 Changed 10 months ago by dgoulet

Status: assignedneeds_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 10 months ago by nickm

lgtm but needs a changes file.

comment:14 Changed 10 months ago by nickm

Status: needs_reviewneeds_revision

comment:15 Changed 10 months ago by dgoulet

Status: needs_revisionmerge_ready

See fixup commit ca8fe58.

comment:16 Changed 10 months ago by nickm

Resolution: fixed
Status: merge_readyclosed

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

Note: See TracTickets for help on using tickets.