sbws should set ConnectionPadding 0

There's no reason to pad connections in sbws: sbws is not trying to avoid netflow logs. Padding can waste bandwidth, and reduce scanner accuracy,

changed milestone to %sbws: 1.0.x-final

added component::core tor/sbws milestone::sbws: 1.0.x-final priority::medium resolution::fixed reviewer::teor severity::normal status::closed type::defect labels

Now here's a challenge: sbws should set ConnectionPadding 0 if the option is supported by Tor. If the option isn't supported by Tor, then sbws should just continue.

You can test with Tor 0.2.9 (no padding support) and Tor 0.3.3 or later (padding support).

Here's a few things to try:

• set the config option, and ignore the exception that stem returns for unsupported config options
• use GETINFO config/names to see if the option is supported

See also #28694 (moved), where we'll also want to set CircuitPadding 0 (or whatever the option ends up being called).

https://github.com/torproject/sbws/pull/305

Trac:
Status: new to needs_review

This patch makes the torrc options in sbws even more complicated.

Here's what sbws has right now:

1. a list of hard-coded torrc options in globals.py: https://github.com/torproject/sbws/blob/master/sbws/globals.py#L10
2. a list of configured torrc options in stem.py, and the hard-coded option LearnCircuitBuildTimeout 0: https://github.com/torproject/sbws/blob/master/sbws/util/stem.py#L127
3. a custom list of torrc options that the user can configure for launch: https://github.com/torproject/sbws/blob/master/sbws/util/stem.py#L142
4. a list of hard-coded torrc options in stem.py, that can't be set until after sbws has bootstrapped: https://github.com/torproject/sbws/blob/master/sbws/util/stem.py#L197

Then the pull request adds custom code to ignore the ConnectionPadding option if tor doesn't support it. This code is more complicated than it needs to be, and it has bugs.

Here's a simpler design:

• a list of torrc options in stem.py, containing 1., 2. and 4., with a flag saying when the option should be applied to tor (1. and 2. on launch, 4. after launch)
• a custom list of torrc options that the user can configure for launch (same as 3.)

(There's an even simpler design in #28712 (moved) that makes 3. into a torrc file, but it's a breaking change.)

Once we refactor the torrc option code, we can add:

• a flag to ignore failures when applying the option after launch
• a torrc option table entry: ConnectionPadding, 0, apply after launch, ignore failures

Trac:
Reviewer: N/A to teor
Status: needs_review to needs_revision

Which reminds me other refactoring i've been thinking for long ago: #28718 (moved). Again, changing all the options system would be backwards-incompatible and would take same time.

Replying to teor: [...]

Here's a simpler design:

• a list of torrc options in stem.py,

Even if only stem.py uses these options, i think they should be in globals.py, since we might need to change them and globals.py should be the place where to change sbws defaults.

containing 1., 2. and 4., with a flag saying when the option should be applied to tor (1. and 2. on launch, 4. after launch)

• a custom list of torrc options that the user can configure for launch (same as 3.)

(There's an even simpler design in #28712 (moved) that makes 3. into a torrc file, but it's a breaking change.)

There isn't a stem torrc syntax parser. This code (https://github.com/torproject/sbws/blob/master/sbws/util/stem.py#L157) was implementing part of it, it's not complete but was working with well-formatted torrc lines. I found that the parser for stem configuration (https://stem.torproject.org/api/util/conf.html) is very close to what a torrc parser would need, yet might not support all the cases (https://github.com/torproject/tor/blob/master/doc/torrc_format.txt).

Options:

1. we use stem.util.conf, let tor launch fail when options can't be parsed and inform the operator
2. we extend stem.util.conf to become a torrc parser
3. we implement a full torrc parser
4. we accept torrc options only in the form of an INI file, which would be parsed by ConfigParser and converted to the stem's torrc dictionary (i'm not sure which cases might fail here either)

I think at this stage, to be able to progress, 1. is the fastest solution with hopefully less failures

Replying to juga:

Replying to teor: [...]

Here's a simpler design:

• a list of torrc options in stem.py,

Even if only stem.py uses these options, i think they should be in globals.py, since we might need to change them and globals.py should be the place where to change sbws defaults.

Let's leave any refactoring until #28737 (moved).

containing 1., 2. and 4., with a flag saying when the option should be applied to tor (1. and 2. on launch, 4. after launch)

• a custom list of torrc options that the user can configure for launch (same as 3.)

(There's an even simpler design in #28712 (moved) that makes 3. into a torrc file, but it's a breaking change.)

There isn't a stem torrc syntax parser. This code (https://github.com/torproject/sbws/blob/master/sbws/util/stem.py#L157) was implementing part of it, it's not complete but was working with well-formatted torrc lines. I found that the parser for stem configuration (https://stem.torproject.org/api/util/conf.html) is very close to what a torrc parser would need, yet might not support all the cases (https://github.com/torproject/tor/blob/master/doc/torrc_format.txt).

Why write a parser, when tor will parse lines for us?

Options:

1. we use stem.util.conf, let tor launch fail when options can't be parsed and inform the operator
2. we extend stem.util.conf to become a torrc parser
3. we implement a full torrc parser
4. we accept torrc options only in the form of an INI file, which would be parsed by ConfigParser and converted to the stem's torrc dictionary (i'm not sure which cases might fail here either)

Option 4 is #28737 (moved), and it should work reasonably well. But it's a new feature, so it belongs in sbws 1.1.

Here's another option:

5. Fix sbws' current tor option parsing code:

• allow options with no argument (#28715 (moved))
• stop trying to merge sbws options with the same name (#28738 (moved)), until we refactor in sbws 1.1 (#28737 (moved)) Then tell the operator if tor doesn't like the options.

Now the option bugs are dealt with in other tickets, let's talk about this ticket.

Here's a simple algorithm for ignoring failing options:

1. sbws has OPTIONS, a hard-coded list of options and values, and CAN_FAIL, a hard-coded, ordered list of options that are allowed to fail
2. Try launching tor with OPTIONS
3. If tor fails to launch:

• if CAN_FAIL is not empty, remove the first option in CAN_FAIL from OPTIONS, and go to step 2
• otherwise, exit sbws with an error

4. Log a message with the Tor version, and the CAN_FAIL options that were removed from OPTIONS

Let's put this new code in a new function. launch_tor is already almost 100 lines long.

Fix sbws' current tor option parsing code

Do we know which possible options an operator might need the "extra_lines" for?. We only need them to use the testing network.

Replying to juga:

Fix sbws' current tor option parsing code

Do we know which possible options an operator might need the "extra_lines" for?. We only need them to use the testing network.

Some operators might want a fixed SOCKSPort. Some sbws tors might need custom options to bootstrap in their environment.

We could ask the current operators if they use extra_lines.

Replying to teor:

Here's a simple algorithm for ignoring failing options:

Wouldn't be easier to try to add them at runtime?. I think that was your suggestion in some other ticket.

Replying to juga:

Replying to teor:

Here's a simple algorithm for ignoring failing options:

Wouldn't be easier to try to add them at runtime?. I think that was your suggestion in some other ticket.

Setting ConnectionPadding would work at runtime. Are there any options that need to be set at launch, and could fail?

Replying to teor:

Are there any options that need to be set at launch, and could fail?

None of the ones that we currently set had fail. Is there an easy way to check that?.

I created a new PR https://github.com/torproject/sbws/pull/316, since after https://trac.torproject.org/projects/tor/ticket/28692#comment:6 it's solved in a different way. For now i've added only ConnectionPadding, but we can easily add more options to the dictionary.

Trac:
Status: needs_revision to needs_review

Replying to juga:

Replying to teor:

Are there any options that need to be set at launch, and could fail?

None of the ones that we currently set had fail. Is there an easy way to check that?.

ConnectionPadding will fail on Tor 0.2.9.

I made some suggestions on the pull request.

Trac:
Status: needs_review to needs_revision

I made the modifications that you suggested.

Trac:
Status: needs_revision to needs_review

Merged in a branch with current master and current needs_review tickets and tested a whole loop with the public Tor network

The code looks good.

I left some comments on the pull request. Please feel free to squash and merge after you've dealt with them.

Trac:
Status: needs_review to needs_revision

Squashed and rebased: https://github.com/juga0/sbws/commits/bug28692_squashed_rebased

Trac:
Status: needs_revision to merge_ready

Merged, thanks.

Trac:
Status: merge_ready to closed
Resolution: N/A to fixed

closed

mentioned in issue #28737 (moved)

mentioned in issue #28738 (moved)

moved to tpo/network-health/sbws#28692 (closed)

mentioned in issue tpo/network-health/sbws#28737 (closed)

mentioned in issue tpo/network-health/sbws#28738 (closed)