[tx-trac] #739: transifex-client doesn't validate remote host certificate
Transifex Trac
-----------------------+------------------------
Reporter: mellos | Owner:
Type: defect | Status: new
Priority: minor | Milestone:
Component: Core | Version:
Keywords: | Blocked By:
Blocking: | Easy Task: 0
Needs Patch: 1 | Ready for Review: 0
Patch Review: |
-----------------------+------------------------
As reported by another user, the transifex-client doesn't validate https
host certificates, thus enabling a malicious user to get the username/pass
of another by using a mitm attack. Since urllib2 doens't do this by
default, we could either do it using the ssl Python library or extend the
default urllib2 class to allow the verification of https certificates.
--
Ticket URL: <http://transifex.org/ticket/739>
Transifex <http://transifex.org/>
Integrated translation platform
Transifex Trac
Type: defect | Status: new
Priority: minor | Milestone:
Component: Core | Version:
Easy Task: 0 | Needs Patch: 1
Ready for Review: 0 | Patch Review:
Comment (by runa):
Tomás and I worked on a patch to make the client verify SSL certs
(attached above).
--
Ticket URL: <http://transifex.org/ticket/739#comment:1>
Transifex Trac
Reporter: mellos | Owner: kbairak
Type: defect | Status: assigned
Component: Core | Version:
Easy Task: 0 | Needs Patch: 1
Ready for Review: 0 | Patch Review:
Changes (by kbairak):
* owner: => kbairak
* status: new => assigned
--
Ticket URL: <http://transifex.org/ticket/739#comment:2>
Transifex Trac
Reporter: mellos | Owner: kbairak
Type: defect | Status: assigned
Component: Core | Version:
Easy Task: 0 | Needs Patch: 1
Ready for Review: 0 | Patch Review:
Comment (by mpessas):
A problem with the attached patch is that it uses the /etc/ssl/certs
directory to look for certificates, which in fedora at least does not even
exist (fedora uses /etc/pki/). So, the necessary certificates are not
found. However, the line
if ctx.load_verify_locations(capath='/etc/ssl/certs/') != 1: raise
Exception('No CA certs')
does not catch this problem, ie load_verify_locations always returns 1. I
haven't managed to figure out why SSL_CTX_load_verify_locations (which is
actually called) does not return 0 to indicate error(1).
So, a SSLError is thrown afterwards due to not being able to verify the
certificate. Do you know of any way to automatically find the correct path
of the certificates?
Btw, another thing with M2Crypto's urllib2 is that I get sometimes errors
with unicode requests, probably something to do with the encoding to
strings.
(1) The code of openssl is too big to follow all execution paths. Is it,
maybe, because (quoting the man page) "The certificates in CApath are only
looked up when required, e.g. when building the certificate chain or when
actually performing the verification of a peer certificate."
--
Ticket URL: <http://transifex.org/ticket/739#comment:3>
Transifex Trac
Reporter: mellos | Owner: kbairak
Type: defect | Status: assigned
Component: Core | Version:
Easy Task: 0 | Needs Patch: 1
Ready for Review: 0 | Patch Review:
Comment (by runa):
When testing this patch, I never had a problem with load_verify_locations
always returning 1. I'm running Debian. I assume you're using Fedora?
When testing the patch as it is, I have no problems pushing/pulling files
and using the transifex-client as normal. However, if I rename
/etc/ssl/certs/Equifax_Secure_CA.pem to
/etc/ssl/certs/Equifax_Secure_CA.pem.orig, the transifex-client will
return 1 and say that it couldn't very the SSL certificate.
I'm not sure if there's an easy way to automatically find the correct path
of the certificates. Maybe we can ask the user about this during the
install of the transifex-client, or when the user runs 'tx init' (and add
capath="" in .transifexrc)?
As for the unicode request problems; got an example?
--
Ticket URL: <http://transifex.org/ticket/739#comment:4>
Transifex Trac
Reporter: mellos | Owner: kbairak
Type: defect | Status: assigned
Component: Core | Version:
Easy Task: 0 | Needs Patch: 1
Ready for Review: 0 | Patch Review:
Comment (by runa):
Seems like I misunderstood your first comment; load_verify_locations
always returns 1. But apart from that, the patch works fine here.
--
Ticket URL: <http://transifex.org/ticket/739#comment:5>
Transifex Trac
Reporter: mellos | Owner: kbairak
Type: defect | Status: assigned
Component: Core | Version:
Easy Task: 0 | Needs Patch: 1
Ready for Review: 0 | Patch Review:
Comment (by mpessas):
As far as load_verify_locations is concerned, let's forget it. It is a
technical issue. The real problem was the hard-coded use of the path to
the certificates: /etc/ssl/certs. As I said, Fedora does not use this.
Instead, it uses /etc/pki/tls/certs. Debian, Arch (the one I use) and I
think opensuse use /etc/ssl/certs, indeed. And I think that is the
standard path.
Anyway, there is the function X509_get_default_cert_dir in openssl, which
returns the X509_CERT_DIR variable, but I cannot find anything related in
M2Crypto. If we had something like that, the problem would be solved, as
then the tx client would work not only on unix based platforms but on
windows as well.
Asking the user about the correct path is not a very good idea, since, as
far as I know, that would not work in windows. It would be nice to have
support for windows, too.
As far as the unicode problems are concerned, I will look into it a bit
more first, to see what actually happens. The problem is that sometimes
the path in the GET request is filled with 0, like
"G\x00\x00\x00E\x00\x00\x00T\x00\x00\x00, which does not happen when using
str objects for the url. We probably need a encode('UTF-8')(1) somewhere.
(1) Is UTF-8 the correct encoding for URLs?
--
Ticket URL: <http://transifex.org/ticket/739#comment:6>
Transifex Trac
Reporter: mellos | Owner: kbairak
Type: defect | Status: assigned
Component: Core | Version:
Easy Task: 0 | Needs Patch: 1
Ready for Review: 0 | Patch Review:
Comment (by runa):
There is a patch available for Python 3.2 which includes a method called
SSL_CTX_set_default_verify_paths(). This method loads a set of default CA
certificates from a filesystem path defined when building OpenSSL:
http://bugs.python.org/issue10443
I also came across a similar bug in Ubuntu: https://bugs.launchpad.net
/ubuntuone-client/+bug/487333. It seems like the solution there was to
hardcode the path to the CA certificates for various operating systems.
It's not a pretty solution, and I'm not sure how well it would work on
Windows.
As for the unicode problems; Maybe urllib.urlencode() will do the trick?
--
Ticket URL: <http://transifex.org/ticket/739#comment:7>
Transifex Trac
Reporter: mellos | Owner: kbairak
Type: defect | Status: assigned
Component: Core | Version:
Easy Task: 0 | Needs Patch: 1
Ready for Review: 0 | Patch Review:
Comment (by mpessas):
Would it be enough to verify the SSL certificate only if:
a) M2Crypto is installed
b) there is a suitable setting in .tx/config with the path where the
installed certificates are?
Else, fall back to previous behavior?
In this way, M2Crypto is not added as a default dependency for the client
and the client will work in all platforms currently supported.
The client could check for a list of default paths (/etc/ssl/certs,
/etc/pki/CA/, whatever...) on initialization to use.
As for the unicode problems, it is strictly a "wrong encoding" problem.
The standard (http://www.w3.org/International/articles/idn-and-
iri/#iritechnical) dictates a UTF-8 encoding and we send a whatever-
python-uses-internally as raw bytes, which is not exactly what urlencode()
fixes (please, correct me if I am wrong). I have made a patch for that.
--
Ticket URL: <http://transifex.org/ticket/739#comment:8>
Transifex Trac
Reporter: mellos | Owner: kbairak
Type: defect | Status: assigned
Component: Core | Version:
Easy Task: 0 | Needs Patch: 1
Ready for Review: 0 | Patch Review:
Comment (by runa):
I guess that would be ok, as long as everything's explained on
http://help.transifex.net/user-guide/client/index.html as well. Users
should definitely know about this "optional feature".
--
Ticket URL: <http://transifex.org/ticket/739#comment:9>
Transifex Trac
Reporter: mellos | Owner: kbairak
Type: defect | Status: closed
Component: Core | Version:
Easy Task: 0 | Needs Patch: 1
Ready for Review: 0 | Patch Review:
Changes (by mpessas):
* status: assigned => closed
* resolution: => wontfix
Comment:
The tickt has been moved to https://github.com/transifex/transifex-
client/issues/1.
Since trac.transifex.org is deprecated and will shut down, you can follow
the development in github.
Thanks,
Apostolis
--
Ticket URL: <http://transifex.org/ticket/739#comment:10>