Merge ScrambleSuit v2014.01.a.

I just published version 2014.01.a of ScrambleSuit. The API is now up-to-date (it uses self.circuit and circuitCreated() instead of handshake()) and it works for me in external and managed mode on Linux.

A patch is available here: https://gitweb.torproject.org/user/phw/obfsproxy.git/shortlog/refs/heads/scramblesuit_integration. It simply creates a module named scramblesuit and updates transports.py.

The branch can be tested with our public ScrambleSuit bridge:

Bridge scramblesuit 193.10.227.195:9002 password=5TYVADJINHBB67PJSBPSWVR5IO742PVO
ClientTransportPlugin scramblesuit exec /path/to/obfsproxy managed

Is there anything else to consider? George, are you able to conveniently test this on Windows? If not, I'll have to play around with my VM.

added component::archived/obfsproxy obfsproxy owner::asn priority::medium pt resolution::fixed scramblesuit status::closed type::enhancement labels

Nice! I will do more code review and test your bridge soon.

BTW, you can make the shared-secret CLI variable mandatory (like it should be?), by doing required=True in the add_argument call.

Also, can you add your docs (spec, etc.) on the docs/ directory of obfsproxy?

Thanks!

tested your bridge and works fine!

let's do some more code review and move on to merging and deploying this!

Ah also, I don't have a Windows VM with me, and I won't have one for the next one month or so :/

Replying to asn:

BTW, you can make the shared-secret CLI variable mandatory (like it should be?), by doing required=True in the add_argument call.

Good catch. Done.

Also, can you add your docs (spec, etc.) on the docs/ directory of obfsproxy?

Sure, also added. The changes are now in the following branch: https://gitweb.torproject.org/user/phw/obfsproxy.git/shortlog/refs/heads/scramblesuit_integration-2

Also, thanks for testing. I'll see if I manage to get it to run on Windows.

Another thing which just came to mind: Do we want to use inter-arrival time obfuscation or not? Right now, it is activated (see USE_IAT_OBFUSCATION in const.py). While throughput it still reasonable, we can maximise it by completely disabling inter-arrival time obfuscation. That might not be a bad choice as timing-based attacks seem to be an exotic threat at this point. Activating it at a later point in time is a matter of simply setting the above variable to True.

Replying to phw:

Another thing which just came to mind: Do we want to use inter-arrival time obfuscation or not? Right now, it is activated (see USE_IAT_OBFUSCATION in const.py). While throughput it still reasonable, we can maximise it by completely disabling inter-arrival time obfuscation. That might not be a bad choice as timing-based attacks seem to be an exotic threat at this point. Activating it at a later point in time is a matter of simply setting the above variable to True.

I guess keeping it disabled for now makes sense.

Maybe we should add an optional CLI/managed-mode switch to scramblesuit that enables it if it's needed? So that clients can enable it dynamically if it's needed by editing their torrc.

Still looking forward to your Windows testing :3

Replying to asn:

Replying to phw:

Another thing which just came to mind: Do we want to use inter-arrival time obfuscation or not? Right now, it is activated (see USE_IAT_OBFUSCATION in const.py). While throughput it still reasonable, we can maximise it by completely disabling inter-arrival time obfuscation. That might not be a bad choice as timing-based attacks seem to be an exotic threat at this point. Activating it at a later point in time is a matter of simply setting the above variable to True.

I guess keeping it disabled for now makes sense.

Agreed.

Maybe we should add an optional CLI/managed-mode switch to scramblesuit that enables it if it's needed? So that clients can enable it dynamically if it's needed by editing their torrc.

Hmm, I'm not quite convinced that we should expose these internals to the user. If it ever becomes necessary, we can just update our bundles with the option enabled.

Still looking forward to your Windows testing :3

I just got around to testing it on a 64-bit Windows 7. It seems to work. I tested it using Python 2.7 in external mode (is it reasonable to infer that it will also work in managed mode?). I also noticed that we now need the module pyyaml which I added to setup.py. I also updated ChangeLog.

I pushed a new branch: https://gitweb.torproject.org/user/phw/obfsproxy.git/shortlog/refs/heads/scramblesuit_integration-3

Replying to phw:

Replying to asn:

Replying to phw:

Another thing which just came to mind: Do we want to use inter-arrival time obfuscation or not? Right now, it is activated (see USE_IAT_OBFUSCATION in const.py). While throughput it still reasonable, we can maximise it by completely disabling inter-arrival time obfuscation. That might not be a bad choice as timing-based attacks seem to be an exotic threat at this point. Activating it at a later point in time is a matter of simply setting the above variable to True.

I guess keeping it disabled for now makes sense.

Agreed.

Maybe we should add an optional CLI/managed-mode switch to scramblesuit that enables it if it's needed? So that clients can enable it dynamically if it's needed by editing their torrc.

Hmm, I'm not quite convinced that we should expose these internals to the user. If it ever becomes necessary, we can just update our bundles with the option enabled.

Still looking forward to your Windows testing :3

I just got around to testing it on a 64-bit Windows 7. It seems to work. I tested it using Python 2.7 in external mode (is it reasonable to infer that it will also work in managed mode?). I also noticed that we now need the module pyyaml which I added to setup.py. I also updated ChangeLog.

Yes, I guess your testing should be sufficient.

You might also want to test starting up the listeners in managed mode in Windows. You can do that by using the following environment variables:

export TOR_PT_STATE_LOCATION="/pt_stat"
export TOR_PT_MANAGED_TRANSPORT_VER="1"
export TOR_PT_CLIENT_TRANSPORTS="scramblesuit"

export TOR_PT_STATE_LOCATION="/"
export TOR_PT_MANAGED_TRANSPORT_VER="1"
export TOR_PT_EXTENDED_SERVER_PORT=
export TOR_PT_ORPORT="127.0.0.1:9001"
export TOR_PT_SERVER_BINDADDR="scramblesuit-127.0.0.1:8000"
export TOR_PT_SERVER_TRANSPORTS="scramblesuit"

The first are for the client, the second are for the server case. Put them in a file (say test_managed_client.sh) and do source test_managed_client.sh. Then run obfsproxy --log-min-severity=debug --log-file=fun managed and check whether the listener was started and if the logs have anything weird. Starting up both listeners in managed mode should give us some more confidence that it works well on Windows.

I pushed a new branch: https://gitweb.torproject.org/user/phw/obfsproxy.git/shortlog/refs/heads/scramblesuit_integration-3

I think this last branch scramblesuit_integration-3 does not contain the scramblesuit docs in the /doc directory.

I will add them and merge the whole branch to obfsproxy Real Soon Now (today or tomorrow).

Cheers!

Replying to asn:

You might also want to test starting up the listeners in managed mode in Windows. You can do that by using the following environment variables: [...]

Thanks, I will give this a try tomorrow (I currently don't have access to the Windows VM).

I pushed a new branch: https://gitweb.torproject.org/user/phw/obfsproxy.git/shortlog/refs/heads/scramblesuit_integration-3

I think this last branch scramblesuit_integration-3 does not contain the scramblesuit docs in the /doc directory.

Ugh, good catch, sorry for that.

I will add them and merge the whole branch to obfsproxy Real Soon Now (today or tomorrow).

I already added it in a separate commit to the scramblesuit_integration-3 branch. But please give me one more day to test it on Windows in the simulated external mode.

Also, it would be nice if we could test scramblesuit automatically using the integration tester in obfsproxy/test/tester.py. It's pretty easy: we just need to add:

+class DirectScramblesuit(DirectTest, unittest.TestCase):
+    transport = "scramblesuit"
+    server_args = ("scramblesuit", "server",
+                   "127.0.0.1:%d" % SERVER_PORT,
+                   "--shared-secret=AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA,
+                   "--dest=127.0.0.1:%d" % EXIT_PORT)
+    client_args = ("scramblesuit", "client",
+                   "127.0.0.1:%d" % ENTRY_PORT,
+                   "--shared-secret=AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",
+                   "--dest=127.0.0.1:%d" % SERVER_PORT)
+
+

But maybe it would be nice to clean up the session ticket after such a test. How should we do this? We could add a hidden test-only CLI switch to scramblesuit that disables session resumption but it would be a bit ugly. Maybe we can do post-test cleanup in unittest?

Replying to asn:

Also, it would be nice if we could test scramblesuit automatically using the integration tester in obfsproxy/test/tester.py.

Good point, keep 'em coming. I added another commit to the scramblesuit_integration-3 branch.

But maybe it would be nice to clean up the session ticket after such a test. How should we do this? We could add a hidden test-only CLI switch to scramblesuit that disables session resumption but it would be a bit ugly. Maybe we can do post-test cleanup in unittest?

I think we should adapt our unit tests to our code and not the other way around. In my commit, I overwrote setUp() and tearDown() to create and delete the two temporary directories.

Nice.

BTW, I noticed that you use "--shared-secret" for external mode but "password" everywhere else (in handle_socks_args() and in the server-side parameters). Maybe we should use the same switch in all cases? I'm using "--shared-secret" in obfs2, but feel free to choose whichever one you want.

Please see branch scramblesuit_init for some simplifications of the __init__() function.

Replying to asn:

BTW, I noticed that you use "--shared-secret" for external mode but "password" everywhere else (in handle_socks_args() and in the server-side parameters). Maybe we should use the same switch in all cases? I'm using "--shared-secret" in obfs2, but feel free to choose whichever one you want.

You're on a bug hunting spree. I did a s/shared-secret/password/ for everything which is exposed to users and rebased to scramblesuit_integration-4: https://gitweb.torproject.org/user/phw/obfsproxy.git/shortlog/refs/heads/scramblesuit_integration-4

Replying to asn:

You might also want to test starting up the listeners in managed mode in Windows. You can do that by using the following environment variables:

export TOR_PT_STATE_LOCATION="/pt_stat"
export TOR_PT_MANAGED_TRANSPORT_VER="1"
export TOR_PT_CLIENT_TRANSPORTS="scramblesuit"
}}}

{{{
export TOR_PT_STATE_LOCATION="/"
export TOR_PT_MANAGED_TRANSPORT_VER="1"
export TOR_PT_EXTENDED_SERVER_PORT=
export TOR_PT_ORPORT="127.0.0.1:9001"
export TOR_PT_SERVER_BINDADDR="scramblesuit-127.0.0.1:8000"
export TOR_PT_SERVER_TRANSPORTS="scramblesuit"
}}}

I gave this a shot and it seems to work. For the record, this is how you seem to set environment variables on Windows: {{{ SET TOR_PT_MANAGED_TRANSPORT_VER=1

And that's **not** the same as the following which is incorrect:

SET TOR_PT_MANAGED_TRANSPORT_VER="1"

Replying to asn:

Please see branch scramblesuit_init for some simplifications of the __init__() function.

Commit 4ecfb762:

• Nitpick: assert is a statement and not a function, i.e., you don't need braces.
• In fact, I wouldn't even use assert in that case. When Python is invoked with -O, asserts are ignored. I would turn the else branches into an elif and append an else branch which throws an exception. (I found this link to have a good explanation on when to use asserts).
• I don't really like that we are using "client", "server" etc. as arguments. How about using symbolic constants instead? Since we are already using Twisted, this might be interesting: https://twistedmatrix.com/documents/12.3.0/core/howto/constants.html
• The if-branch in server.py seems unnecessary as both, "server" and "ext_server" lead to weAreClient being set to False.

Commit ff86f6a2, f15be57b, d921f485, and ae86d3d4:

• The changes look good to me. Could you please turn this into a separate branch forked from the ScrambleSuit repository? I would prefer to merge changes from the ScrambleSuit repository into the obfsproxy repository and not the other way around.

Replying to phw:

Replying to asn:

Please see branch scramblesuit_init for some simplifications of the __init__() function.

Commit 4ecfb762:

• Nitpick: assert is a statement and not a function, i.e., you don't need braces.
• In fact, I wouldn't even use assert in that case. When Python is invoked with -O, asserts are ignored. I would turn the else branches into an elif and append an else branch which throws an exception. (I found this link to have a good explanation on when to use asserts).
• I don't really like that we are using "client", "server" etc. as arguments. How about using symbolic constants instead? Since we are already using Twisted, this might be interesting: https://twistedmatrix.com/documents/12.3.0/core/howto/constants.html
• The if-branch in server.py seems unnecessary as both, "server" and "ext_server" lead to weAreClient being set to False.

Commit ff86f6a2, f15be57b, d921f485, and ae86d3d4:

• The changes look good to me. Could you please turn this into a separate branch forked from the ScrambleSuit repository? I would prefer to merge changes from the ScrambleSuit repository into the obfsproxy repository and not the other way around.

Makes sense. You can find my scramblesuit-only branch at https://git.gitorious.org/scramblesuit/scramblesuit.git with branch name scramblesuit_init.

I also isolated the obfsproxy-only changes to branch init_transport_config in my personal obfsproxy repo.

BTW Philipp, you might be able to use the get_public_server_options() transport method that David Stainton recently added to obfsproxy, so that you generate a password if the admin hadn't specified one.

Then you can save it to a file in TOR_PT_STATE_LOCATION and use it the next time you boot up.

Does this make sense? You think it might be too messy?

Replying to asn:

Makes sense. You can find my scramblesuit-only branch at https://git.gitorious.org/scramblesuit/scramblesuit.git with branch name scramblesuit_init.

Merged, thanks!

I also isolated the obfsproxy-only changes to branch init_transport_config in my personal obfsproxy repo.

OK, I will then rebase a new revision on top of init_transport_config.

Replying to asn:

BTW Philipp, you might be able to use the get_public_server_options() transport method that David Stainton recently added to obfsproxy, so that you generate a password if the admin hadn't specified one.

Then you can save it to a file in TOR_PT_STATE_LOCATION and use it the next time you boot up.

Does this make sense? You think it might be too messy?

That sounds indeed promising, I'll give it a try.

Replying to asn:

BTW Philipp, you might be able to use the get_public_server_options() transport method that David Stainton recently added to obfsproxy, so that you generate a password if the admin hadn't specified one.

Then you can save it to a file in TOR_PT_STATE_LOCATION and use it the next time you boot up.

Does this make sense? You think it might be too messy?

I just added this to master: https://gitweb.torproject.org/user/phw/scramblesuit.git/commitdiff/7c3c519889ffb721bc1f71fdd3d1550ffe0dfb39

I think it's a good idea and the benefit clearly outweighs the (small) messiness: We no longer need to force bridge operators to set ServerTransportOptions. I'll do some more testing and then create another obfsproxy branch.

Replying to phw:

Replying to asn:

BTW Philipp, you might be able to use the get_public_server_options() transport method that David Stainton recently added to obfsproxy, so that you generate a password if the admin hadn't specified one.

Then you can save it to a file in TOR_PT_STATE_LOCATION and use it the next time you boot up.

Does this make sense? You think it might be too messy?

I just added this to master: https://gitweb.torproject.org/user/phw/scramblesuit.git/commitdiff/7c3c519889ffb721bc1f71fdd3d1550ffe0dfb39

I think it's a good idea and the benefit clearly outweighs the (small) messiness: We no longer need to force bridge operators to set ServerTransportOptions. I'll do some more testing and then create another obfsproxy branch.

Hm. I just skimmed over the patch. I will look more into it soon.

I wonder if we can move all this file-creation/reading logic to __init__() instead of get_public_server_options(). It would be more suiting I think.

Also where is SERVER_PASSWORD_FILE initialized in the pushed code?

We should also think what we want to happen if a person sets up a ServerTransportOptions line after a SERVER_PASSWORD_FILE has been created.

Maybe we could have a more descriptive log message than No password found in transport options.. Maybe it should be directed to the bridge operator and say No password was specified. Generating and caching a new one.. Maybe there should also be a log message for Found a cached password. Using this one.

We should also figure out what happens if writeToFile fails. This might be better handled by moving this logic to __init__() I think.

Replying to asn:

We should also think what we want to happen if a person sets up a ServerTransportOptions line after a SERVER_PASSWORD_FILE has been created.

It overwrites the fallback password.

Updated master. Any plans regarding when you want to merge init_transport_config?

Replying to phw:

Replying to asn:

We should also think what we want to happen if a person sets up a ServerTransportOptions line after a SERVER_PASSWORD_FILE has been created.

It overwrites the fallback password.

I like the changes. What do you think about renaming the new State.password element to State.fallback_password or something?

Updated master. Any plans regarding when you want to merge init_transport_config?

Soon. Maybe even tomorrow or the day after (Monday, Tuesday). Do you think anything else remains to do before the merge?

Replying to asn:

I like the changes. What do you think about renaming the new State.password element to State.fallback_password or something?

Done.

Updated master. Any plans regarding when you want to merge init_transport_config?

Soon. Maybe even tomorrow or the day after (Monday, Tuesday). Do you think anything else remains to do before the merge?

I can't think of anything at this point. And I'm not familiar enough with the bundle building process to know if there's anything which could cause issues.

Next revision is now available here: https://gitweb.torproject.org/user/phw/obfsproxy.git/tree/refs/heads/scramblesuit_integration-5

For the record: the previous branch had a broken history. The final branch is: https://gitweb.torproject.org/user/phw/obfsproxy.git/shortlog/refs/heads/scramblesuit_integration-6

ScrambleSuit is now merged to obfsproxy in commit 258a26ad.

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

closed