Opened 4 years ago

Last modified 3 years ago

#11190 reopened defect

obfsproxy shebang should point to "python2", not "python"

Reported by: yawning Owned by: asn
Priority: Medium Milestone:
Component: Obfuscation/Obfsproxy Version:
Severity: Keywords: python version, obfsproxy, tbb, flashproxy
Cc: mcs, brade, mikeperry Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

It currently points at "python" which is not version specific and will break horribly on systems where the default system python is python3.

This isn't a issue when it is installed with setup.py, but was when I tried a TBB nightly a few days ago. As far as I can tell every system that has python2.x installed with have a "python2" symlink so changing the shebang won't break places where this works now, but will allow it to work on more systems without breaking in horrible unintuitive ways for the user.

Child Tickets

Change History (18)

comment:1 Changed 4 years ago by dcf

Keywords: flashproxy added

The same goes, I'm sure for flashproxy.

Yawning, maybe you could try building a TBB that answers this ticket? You can do it in an Ubuntu VM, I'm told. Probably, all you need to do is define a PYTHON=python2 variable and replace python with $PYTHON in these two descriptor files:

You shouldn't need to do anything in the windows descriptor because that uses py2exe so Python is built in.

For example,

-  python setup.py build --build-lib build
+  $PYTHON setup.py build --build-lib build

I'm pretty sure it suffices to change the name of the executable used to call the various setup.py's, because distutils/setuptools rewrite the shebang line in installed executables according to how it was called.

Last edited 4 years ago by dcf (previous) (diff)

comment:2 Changed 3 years ago by mikeperry

Keywords: MikePerry201403R added

comment:3 Changed 3 years ago by mikeperry

Resolution: fixed
Status: newclosed

Ok, I went ahead and did this for the Linux descriptors. We can see if it breaks the nightlies, and otherwise just go ahead with it in the next 3.6 release.

comment:4 Changed 3 years ago by yawning

Resolution: fixed
Status: closedreopened

It apparently didn't break the build, but as far as I can tell from the nightlies at least obfsproxy still has the incorrect bangpath.

cp -a bin/obfsproxy $PTDIR/obfsproxy.bin

It looks like the build descriptor just has it copying obfsproxy.bin out of the source directory, which doesn't get touched by distutils/setuptools. The right thing to do is probably to just fix all of the python scripts.

comment:5 Changed 3 years ago by yawning

Keywords: MikePerry201404R added; MikePerry201403R removed

I'm not sure how many python scripts we actually ship in TBB 3.6, but the correct fix for obfsproxy is in my userrepo as bug_11190. Other PTs that contain #!/usr/bin/python should also be changed, since it's fairly clear that the build process doesn't cause distutils/setuptools to modify all of the scripts.

comment:6 in reply to:  5 ; Changed 3 years ago by dcf

Replying to yawning:

I'm not sure how many python scripts we actually ship in TBB 3.6, but the correct fix for obfsproxy is in my userrepo as bug_11190. Other PTs that contain #!/usr/bin/python should also be changed, since it's fairly clear that the build process doesn't cause distutils/setuptools to modify all of the scripts.

I think it's just due to some setuptools weirdness in how obfsproxy is set up. The "obfsproxy" executable isn't actually mentioned in setup.py, and it should be. If it were instead listed under scripts, then its shebang would be modified appropriately.

For example, try hello:

#!/usr/bin/env python
print "Hello world"

and setup.py:

from setuptools import setup
setup(name = "hello", version="1.0", scripts=["hello"])

Depending on whether you run python setup.py build or python2 setup.py build, then shebang under the build directory will be different.

comment:7 in reply to:  6 ; Changed 3 years ago by yawning

Replying to dcf:

Replying to yawning:

I'm not sure how many python scripts we actually ship in TBB 3.6, but the correct fix for obfsproxy is in my userrepo as bug_11190. Other PTs that contain #!/usr/bin/python should also be changed, since it's fairly clear that the build process doesn't cause distutils/setuptools to modify all of the scripts.

I think it's just due to some setuptools weirdness in how obfsproxy is set up. The "obfsproxy" executable isn't actually mentioned in setup.py, and it should be. If it were instead listed under scripts, then its shebang would be modified appropriately.

Well that would explain why the descriptor is just copying the script from the source tree. asn is there any reason why this is the case?

To be honest I think doing both (changing the shebang to be more explicity) *and* fixing setup.py is the right thing to do here, if only because I hate having bin/obfsproxy being perpetually stuck in my git status as changed (Yes, I could just stick it in .git/info/exclude since it's unlikely to change much, but I shouldn't have to).

(Edit: Fixed quoting failure)

Last edited 3 years ago by yawning (previous) (diff)

comment:8 Changed 3 years ago by yawning

For what it's worth, none of the pluggable transports in the latest beta actually have the correct shebang.

From looking at gitian-pluggable-transports.yml:

  • fteproxy is built with make, and the Makefile hardcodes python when invoking setup.py.
  • flashproxy is built with make, and the Makefile tramples over PYTHON.
  • obfsproxy is built with distutils/setuptools, but the crucial script isn't, and is just copied.
Last edited 3 years ago by yawning (previous) (diff)

comment:9 in reply to:  7 Changed 3 years ago by asn

Replying to yawning:

Replying to dcf:

Replying to yawning:

I'm not sure how many python scripts we actually ship in TBB 3.6, but the correct fix for obfsproxy is in my userrepo as bug_11190. Other PTs that contain #!/usr/bin/python should also be changed, since it's fairly clear that the build process doesn't cause distutils/setuptools to modify all of the scripts.

I think it's just due to some setuptools weirdness in how obfsproxy is set up. The "obfsproxy" executable isn't actually mentioned in setup.py, and it should be. If it were instead listed under scripts, then its shebang would be modified appropriately.

Well that would explain why the descriptor is just copying the script from the source tree. asn is there any reason why this is the case?

To be honest I think doing both (changing the shebang to be more explicity) *and* fixing setup.py is the right thing to do here, if only because I hate having bin/obfsproxy being perpetually stuck in my git status as changed (Yes, I could just stick it in .git/info/exclude since it's unlikely to change much, but I shouldn't have to).

(Edit: Fixed quoting failure)

Hm, I see.

I think that the reason that obfsproxy's setup.py is weird, is because I wanted the executable to be called 'obfsproxy' even though the entry point file is called 'pyobfsproxy.py'. After some research on the net, I arrived at these lines:

    packages = find_packages(),
    entry_points = {
        'console_scripts': [
            'obfsproxy = obfsproxy.pyobfsproxy:run'
            ]
        },

So, you are saying that the setup.py completely ignores /bin/obfsproxy and instead makes its own executable with the wrong shebang? So I would need to fix both the setup.py to not ignore /bin/obfsproxy , and also /bin/obfsproxy to have a proper shebang.

I will try to look into this tomorrow.

Sorry for the shitty behavior.

Last edited 3 years ago by asn (previous) (diff)

comment:10 Changed 3 years ago by asn

FWIW, I merged Yawning's shebang fix.

I still need to fix the setup.py script.

comment:11 Changed 3 years ago by asn

FWIW, I'm not sure how to fix the setup.py issue.

That is, I'm not sure if I should keep /bin/obfsproxy and change setup.py to respect it (and use that for the executable). Or whether I should delete that file, and just let setup.py create its own executable. I will try to look into this another day; as I understand it it's not that critical issue anymore since I merged Yawning's patch.

comment:12 in reply to:  11 Changed 3 years ago by dcf

Replying to asn:

FWIW, I'm not sure how to fix the setup.py issue.

That is, I'm not sure if I should keep /bin/obfsproxy and change setup.py to respect it (and use that for the executable). Or whether I should delete that file, and just let setup.py create its own executable. I will try to look into this another day; as I understand it it's not that critical issue anymore since I merged Yawning's patch.

What you merged already seems fine. I also did not think of an obvious way to change setup.py.

comment:13 Changed 3 years ago by mikeperry

Keywords: MikePerry201404R removed

So there is nothing more on the TBB side to be done here, aside from updating to the new obfsproxy/other PT release tags?

comment:14 Changed 3 years ago by mcs

This has now been picked up for the TBB builds. But on Mac OS 10.9.2 at least, there is no /usr/bin/python2 symlink, so obfsproxy is broken. The tor/tor.real hack used in TBB on Mac OS makes the python2 unnecessary, because these two lines are executed before starting tor:

export VERSIONER_PYTHON_VERSION=2.6
export VERSIONER_PYTHON_PREFER_32_BIT=yes

I do not know what the best solution is for this problem. Mac OS 10.9.2 does have python2.5 and python2.6 symlinks, but using either one of those always selects 64-bit execution. Also, I have not checked to see what is present on older versions of Mac OS (TBB supports 10.6 and newer).

comment:15 Changed 3 years ago by mcs

Cc: mcs brade mikeperry added

comment:16 in reply to:  14 Changed 3 years ago by yawning

Replying to mcs:

I do not know what the best solution is for this problem. Mac OS 10.9.2 does have python2.5 and python2.6 symlinks, but using either one of those always selects 64-bit execution. Also, I have not checked to see what is present on older versions of Mac OS (TBB supports 10.6 and newer).

Ugh. The OSX bundles could be built with PYTHON set to python, but that won't fix obfsproxy because of the quirky build process. Seems like regardless of python or python2 being in the default obfsproxy script, some subset of systems will break.

As part of the OSX bundle build process, you could change the obfsproxy shebang with a patch (or hell, even sed).

comment:17 Changed 3 years ago by gk

In commit 5f671fa272219e1ecaf15d5d22a41c16aec64dae I replaced "python2" with "python" which makes obfsproxy working again on my 10.6.8. Let's see what it breaks on other systems...

Last edited 3 years ago by gk (previous) (diff)

comment:18 in reply to:  17 Changed 3 years ago by dcf

Replying to gk:

In commit 5f671fa272219e1ecaf15d5d22a41c16aec64dae I replaced "python2" with "python" which makes obfsproxy working again on my 10.6.8. Let's see what it breaks on other systems...

As far as we know, the only system that is broken with "python" is Arch Linux (which Yawning uses), which has /usr/bin/python symlinked to python3. (Most other distros symlink python to python2.) The only system that is broken with "python2" is OS X, which doesn't have /usr/bin/python2.

Note: See TracTickets for help on using tickets.