Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#3617 closed defect (implemented)

Make client-side optimistic data behavior configurable

Reported by: nickm Owned by:
Priority: Medium Milestone: Tor: 0.2.3.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-client
Cc: iang Actual Points:
Parent ID: #1849 Points:
Reviewer: Sponsor:

Description

We want a tristate in the configuration file to enable/disable optimistic data, with the default behavior being "do what the consensus says."

Child Tickets

Change History (16)

comment:1 Changed 8 years ago by nickm

Cc: iang added

comment:2 Changed 8 years ago by Sebastian

Hrm. I'm not so sure. Are we worried that those using the feature partition themselves too much as it stands now? Or do we want it just because some people might want to disable it?

comment:3 in reply to:  2 Changed 8 years ago by iang

Replying to Sebastian:

Hrm. I'm not so sure. Are we worried that those using the feature partition themselves too much as it stands now? Or do we want it just because some people might want to disable it?

The intent was the former. An exit node can easily see which circuits are built by upgraded clients and which are not. (Presuming the SOCKS client is optimistic-data-aware, of course.)

comment:4 Changed 8 years ago by Sebastian

Right, the constraint that we need a special client for that was what made me think that the partitioning wouldn't be a big issue - nobody will use it before we ship it in our TBB bundles. But then maybe we should delay turning the feature on by default even further. I guess I wouldn't mind yet another option too much.

comment:5 Changed 8 years ago by nickm

Status: newneeds_review

If we still think this wise, I've got a patch for review in "feature3617" in my public repository.

comment:6 Changed 8 years ago by iang

I'm a little nervous that the field called "exit_allows_optimistic_data" now really means "exit allows optimistic data *and* the consensus/torrc also allow it.  Could the field be renamed to "may_use_optimistic_data" or something like that?

comment:7 Changed 8 years ago by nickm

Sounds good by me. I've made the change in a new rebased branch "feature3617-v2". Better now?

comment:8 Changed 8 years ago by arma

It looks like our default default (what do you do if you want to use what the consensus says but the consensus doesn't say anything) is 0.

So if we like our design and implementation, we'll be stuck with an extra param in the consensus forever (or at least until we change the default default and wait for those versions to go away).

The alternative is for auto to default to 1 if no param is listed. That's riskier short-term (if we don't like our design and implementation, we'll need to list a param of 0 in the consensus until these versions go away).

It looks like refuseunknownexits's auto default is 1, for comparison. Do we want to try to build a convention here, or is that crazy-talk?

If we want to get super over engineering about it, we could have auto default to 0 for now but plan to change it to default to 1 before 0.2.3 goes live.

comment:9 Changed 8 years ago by nickm

If we want to get super over engineering about it, we could have auto default to 0 for now but plan to change it to default to 1 before 0.2.3 goes live.

If by "live" you mean "stable", sure.

comment:10 in reply to:  9 Changed 8 years ago by nickm

Replying to nickm:

If we want to get super over engineering about it, we could have auto default to 0 for now but plan to change it to default to 1 before 0.2.3 goes live.

If by "live" you mean "stable", sure.

(And I'm not just saying that because it involves making no additional changes to my code. ;) )

comment:11 in reply to:  7 Changed 8 years ago by iang

Replying to nickm:

Sounds good by me. I've made the change in a new rebased branch "feature3617-v2". Better now?

The comment in or.h should be updated to match, I think.

comment:12 Changed 8 years ago by nickm

Added a fixup commit to the branch. Better now?

comment:13 Changed 8 years ago by iang

Great, thanks. ;-)

comment:14 Changed 8 years ago by nickm

Resolution: implemented
Status: needs_reviewclosed

Merged. :)

comment:15 Changed 7 years ago by nickm

Keywords: tor-client added

comment:16 Changed 7 years ago by nickm

Component: Tor ClientTor
Note: See TracTickets for help on using tickets.