Make client-side optimistic data behavior configurable

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

changed milestone to %Tor: 0.2.3.x-final

added component::core tor/tor milestone::Tor: 0.2.3.x-final parent::1849 priority::medium resolution::implemented status::closed tor-client type::defect labels

Trac:
Cc: N/A to iang

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?

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.)

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.

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

Trac:
Status: new to needs_review

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?

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

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.

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.

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. ;) )

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.

Added a fixup commit to the branch. Better now?

Great, thanks. ;-)

Merged. :)

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

Trac:
Keywords: N/A deleted, tor-client added

Trac:
Component: Tor Client to Tor

closed

moved to tpo/core/tor#3617 (closed)