Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

transifex-client: insufficient CVE-2013-2073 fix #42

Closed
thoger opened this issue Oct 30, 2013 · 9 comments
Closed

transifex-client: insufficient CVE-2013-2073 fix #42

thoger opened this issue Oct 30, 2013 · 9 comments

Comments

@thoger
Copy link

thoger commented Oct 30, 2013

Commit e24ea95 (plus few follow up commits) was added to add verification of HTTPS certificate to avoid MITM attacks. The issue got CVE-2013-2073 assigned.

However, the fix is not really correct. The way it works is:

  • verify_ssl opens connection to the target host and verifies provided certificate, including name check
  • connection opened by verify_ssl is closed immediately
  • actual communication with the server is done via urllib2, which opens its own connection(s)

So the actual data is sent over unverified connections opened by urllib2. MITM attacker should be able to steal all transferred data by allowing "probe" connection opened by verify_ssl to connect to the real transifex server and only intercept subsequent urllib2 connection.

@diegobz
Copy link
Contributor

diegobz commented Oct 30, 2013

@thoger Thanks for bringing it up. Do you think you can help us with a patch?

@thoger
Copy link
Author

thoger commented Oct 31, 2013

@diegobz Sorry, I have no ready to use fix that avoids adding extra dependency (which I believe is the reason why proposed fix using requests was not used). Quick search suggests it should be possible to do with custom opener. I found this, but I haven't tested it yet at all: https://gist.github.com/schlamar/2993700

@mpessas
Copy link
Contributor

mpessas commented Nov 1, 2013

@thoger The problem with requests is that it uses the Apache license, which is incompatible with GPLv2. We are going to use urllib3, though. A patch should be ready soon.

@mpessas
Copy link
Contributor

mpessas commented Nov 1, 2013

Handled in 6d69d61

@mpessas mpessas closed this as completed in e0d1f8b Dec 2, 2013
@thoger
Copy link
Author

thoger commented Dec 13, 2013

Can anyone hint an easy way to test the fix? I'm not familiar with tx, I used few variants of tx set that I expect to trigger SSLError when pointed to a host with invalid certificate, but I failed to get such error.

Hence I tried this:

$ cat txctest.py
from txclib.utils import get_details
import sys
get_details('get_resources', 'testu', 'testp', hostname=sys.argv[1], project='foo')

and the exception I get implies certificate check passed. Server error log shows 404 response was sent.

$ python txctest.py https://127.0.0.1
Traceback (most recent call last):
  File "txctest.py", line 4, in <module>
    get_details('get_resources', 'testu', 'testp', hostname=sys.argv[1], project='foo')
  File "/path/to/transifex-client-0.10/txclib/utils.py", line 85, in get_details
    remote_project = parse_json(r.data)
  File "/usr/lib64/python2.7/json/__init__.py", line 338, in loads
    return _default_decoder.decode(s)
  File "/usr/lib64/python2.7/json/decoder.py", line 365, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/usr/lib64/python2.7/json/decoder.py", line 383, in raw_decode
    raise ValueError("No JSON object could be decoded")
ValueError: No JSON object could be decoded

@mpessas
Copy link
Contributor

mpessas commented Dec 13, 2013

You are right, it does not. I will get beack to you.

@mpessas mpessas reopened this Dec 13, 2013
@thoger
Copy link
Author

thoger commented Dec 13, 2013

FYI, I tried to make txclib/utils.py import system urllib3 instead of the bundled one and I'm getting expected SSLError:

urllib3.exceptions.SSLError: [Errno 1] _ssl.c:504: error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed

or

urllib3.exceptions.SSLError: hostname 'www.transifex.org' doesn't match either of '*.transifex.com', 'transifex.com'

It seems bundled urllib3 has some issue.

@mpessas
Copy link
Contributor

mpessas commented Dec 18, 2013

@thoger Hey, could you check #47 ?

@diegobz
Copy link
Contributor

diegobz commented Mar 19, 2014

@thoger We have merged the #47 pull request into master. From our tests, it seems now all connections are encrypted, but if you could give it a try as well, before we roll out a release, it would be great.

Thanks.

@diegobz diegobz closed this as completed Mar 19, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants