Qpid
  1. Qpid
  2. QPID-3175

SSL support in Python client libraries

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.8
    • Fix Version/s: 0.24
    • Component/s: Python Client, Python Tools
    • Labels:
      None
    • Environment:

      Windows XP, Python 2.7.1, (broker Red Hat MRG 1.3 on RHEL 5.5)

      Description

      I was trying to connect to my broker with SSL encrypted connection (both PLAIN and EXTERNAL authentication methods). However, it seems to be not working. I get following error messages:

      Traceback (most recent call last):
      File "ssl-external.py", line 20, in <module>
      connection.open()
      File "<string>", line 6, in open
      File "c:\opt!_EUREX14\tests\qpid.python-0.8\python\qpid\messaging\endpoints.py", line 244, in open
      self.attach()
      File "<string>", line 6, in attach
      File "c:\opt!_EUREX14\tests\qpid.python-0.8\python\qpid\messaging\endpoints.py", line 262, in attach
      self._ewait(lambda: self._transport_connected and not self._unlinked())
      File "c:\opt!_EUREX14\tests\qpid.python-0.8\python\qpid\messaging\endpoints.py", line 197, in _ewait
      self.check_error()
      File "c:\opt!_EUREX14\tests\qpid.python-0.8\python\qpid\messaging\endpoints.py", line 190, in check_error
      raise self.error
      qpid.messaging.exceptions.ConnectError: [Errno 1] _ssl.c:499: error:14094412:SSL routines:SSL3_READ_BYTES:sslv3 alert bad certificate

      In the source codes (messaging/transports.py), the SSL seems to be supported and implemented, but it is not working. I didn't found any possibilities how to pass the certificates to the SSL libraries and the wrap_socket call in transports.py is calling the wrap_socket without any additional attributes except the original socket.

      I didn't had the chance to test other platforms or Python versions, except Python 2.4.3 on RHEL 5.5, where the SSL is not supported at all (the SSL support in Python changed significantly with 2.6)

      1. QPID-3175.patch
        3 kB
        JAkub Scholz
      2. QPID-3175a.patch
        3 kB
        JAkub Scholz
      3. sasl_external.patch
        1 kB
        JAkub Scholz

        Issue Links

          Activity

          Hide
          Ken Giusti added a comment -

          I'm closing this bug as I believe all the issues raised by it have been resolved since at least release 0.24. See:

          https://issues.apache.org/jira/browse/QPID-4872
          https://issues.apache.org/jira/browse/QPID-4337

          Feel free to reopen the bug if you disagree. If there are new/different issues with the python client's SSL support, please open a new JIRA to track those issues.

          Show
          Ken Giusti added a comment - I'm closing this bug as I believe all the issues raised by it have been resolved since at least release 0.24. See: https://issues.apache.org/jira/browse/QPID-4872 https://issues.apache.org/jira/browse/QPID-4337 Feel free to reopen the bug if you disagree. If there are new/different issues with the python client's SSL support, please open a new JIRA to track those issues.
          Hide
          Michal Zerola added a comment -

          Hi,

          we are encountering problems when using the ssl transport layer in Python clients. When the client is sending messages in burst to the broker in asynchronous manner (sync=False in Sender.send) the exception is occasionally thrown with the following output:

          [Errno 1] _ssl.c:1217: error:1409F07F:SSL routines:SSL3_WRITE_PENDING:bad write retry

          It seems, like the client's socket gets full, so the next underlying SSLSocket.write() throws the SSLError (with SSL_ERROR_WANT_WRITE as a code) but this situation is not handled properly. One can see, that in qpid/messaging/transports.py in the constructor of the SSL transport the socket is set to NON BLOCKING. Such a non blocking socket then behaves that write() doesn't wait till there is enough space on the socket and may throw the above exception. The question is therefore:

          • Why is SSLSocket set to NON BLOCKING state in contrast to the non SSL transport?
          • Is handling of the above SSL_ERROR_WANT_ {WRITE,READ}

            errors implemented properly in the Python's API?

          Thanks for answers. Best,

          Michal

          Show
          Michal Zerola added a comment - Hi, we are encountering problems when using the ssl transport layer in Python clients. When the client is sending messages in burst to the broker in asynchronous manner (sync=False in Sender.send) the exception is occasionally thrown with the following output: [Errno 1] _ssl.c:1217: error:1409F07F:SSL routines:SSL3_WRITE_PENDING:bad write retry It seems, like the client's socket gets full, so the next underlying SSLSocket.write() throws the SSLError (with SSL_ERROR_WANT_WRITE as a code) but this situation is not handled properly. One can see, that in qpid/messaging/transports.py in the constructor of the SSL transport the socket is set to NON BLOCKING. Such a non blocking socket then behaves that write() doesn't wait till there is enough space on the socket and may throw the above exception. The question is therefore: Why is SSLSocket set to NON BLOCKING state in contrast to the non SSL transport? Is handling of the above SSL_ERROR_WANT_ {WRITE,READ} errors implemented properly in the Python's API? Thanks for answers. Best, Michal
          Hide
          JAkub Scholz added a comment -

          This patch fixes the support for EXTERNAL mechanism even when the real SASL library is used using the python-saslwrapper and the WrapperClient class.

          Show
          JAkub Scholz added a comment - This patch fixes the support for EXTERNAL mechanism even when the real SASL library is used using the python-saslwrapper and the WrapperClient class.
          Hide
          JAkub Scholz added a comment -

          Hi,

          It is great to see the fix committed to trunk. Nevertheless I found one related problem ... it adds the support for EXTERNAL mechanism only to the PlainClient class. But the WrapperClient class remained unchanged. And since on systems where the python-saslwrapper is installed, it uses the WrapperClient and the SASL library instead of the PlainClient, the EXTERNAL mechanism doesn't work.

          I prepared and attached another patch which fixes this problem as well. It modifies the messaging/driver.py to detect the EXTERNAL mechanism and sets the "externaluser" attribute for the WrapperClient as well as for the PlainClient. That makes the EXTERNAL mechanism really work in both situations ...

          Regards
          Jakub

          Show
          JAkub Scholz added a comment - Hi, It is great to see the fix committed to trunk. Nevertheless I found one related problem ... it adds the support for EXTERNAL mechanism only to the PlainClient class. But the WrapperClient class remained unchanged. And since on systems where the python-saslwrapper is installed, it uses the WrapperClient and the SASL library instead of the PlainClient, the EXTERNAL mechanism doesn't work. I prepared and attached another patch which fixes this problem as well. It modifies the messaging/driver.py to detect the EXTERNAL mechanism and sets the "externaluser" attribute for the WrapperClient as well as for the PlainClient. That makes the EXTERNAL mechanism really work in both situations ... Regards Jakub
          Hide
          Keith Wall added a comment -

          I think the change of the default SASL mechanism to ANONYMOUS broken the Python/Javabuild:

          
          Error during test:  Traceback (most recent call last):
              File "/home/jenkins/jenkins-slave/workspace/Qpid-Python-Java-Test/trunk/qpid/python/qpid-python-test", line 340, in run
                phase()
              File "/home/jenkins/jenkins-slave/workspace/Qpid-Python-Java-Test/trunk/qpid/python/qpid/tests/messaging/endpoints.py", line 34, in testEstablish
                self.conn = Connection.establish(self.broker, **self.connection_options())
              File "/home/jenkins/jenkins-slave/workspace/Qpid-Python-Java-Test/trunk/qpid/python/qpid/messaging/endpoints.py", line 68, in establish
                conn.open()
              File "<string>", line 6, in open
              File "/home/jenkins/jenkins-slave/workspace/Qpid-Python-Java-Test/trunk/qpid/python/qpid/messaging/endpoints.py", line 255, in open
                self.attach()
              File "<string>", line 6, in attach
              File "/home/jenkins/jenkins-slave/workspace/Qpid-Python-Java-Test/trunk/qpid/python/qpid/messaging/endpoints.py", line 273, in attach
                self._ewait(lambda: self._transport_connected and not self._unlinked())
              File "/home/jenkins/jenkins-slave/workspace/Qpid-Python-Java-Test/trunk/qpid/python/qpid/messaging/endpoints.py", line 208, in _ewait
                self.check_error()
              File "/home/jenkins/jenkins-slave/workspace/Qpid-Python-Java-Test/trunk/qpid/python/qpid/messaging/endpoints.py", line 201, in check_error
                raise self.error
            AuthenticationFailure: sasl negotiation failed: no mechanism agreed
          
          

          See:

          https://builds.apache.org/view/M-R/view/Qpid/job/Qpid-Python-Java-Test/lastCompletedBuild/testReport/

          Whilst I think we could change the test harness configuration to pass through the sasl_mechanisms of PLAIN, I wonder why the decision to default has been made? I don't see how this contributes to SSL support.

          The old code would default to PLAIN if username/password was supplied and PLAIN was support by the Broker (sasl.py:89) which I think was a useful default regardless of Broker choice.

          Any thoughts please?

          Show
          Keith Wall added a comment - I think the change of the default SASL mechanism to ANONYMOUS broken the Python/Javabuild: Error during test: Traceback (most recent call last): File "/home/jenkins/jenkins-slave/workspace/Qpid-Python-Java-Test/trunk/qpid/python/qpid-python-test" , line 340, in run phase() File "/home/jenkins/jenkins-slave/workspace/Qpid-Python-Java-Test/trunk/qpid/python/qpid/tests/messaging/endpoints.py" , line 34, in testEstablish self.conn = Connection.establish(self.broker, **self.connection_options()) File "/home/jenkins/jenkins-slave/workspace/Qpid-Python-Java-Test/trunk/qpid/python/qpid/messaging/endpoints.py" , line 68, in establish conn.open() File "<string>" , line 6, in open File "/home/jenkins/jenkins-slave/workspace/Qpid-Python-Java-Test/trunk/qpid/python/qpid/messaging/endpoints.py" , line 255, in open self.attach() File "<string>" , line 6, in attach File "/home/jenkins/jenkins-slave/workspace/Qpid-Python-Java-Test/trunk/qpid/python/qpid/messaging/endpoints.py" , line 273, in attach self._ewait(lambda: self._transport_connected and not self._unlinked()) File "/home/jenkins/jenkins-slave/workspace/Qpid-Python-Java-Test/trunk/qpid/python/qpid/messaging/endpoints.py" , line 208, in _ewait self.check_error() File "/home/jenkins/jenkins-slave/workspace/Qpid-Python-Java-Test/trunk/qpid/python/qpid/messaging/endpoints.py" , line 201, in check_error raise self.error AuthenticationFailure: sasl negotiation failed: no mechanism agreed See: https://builds.apache.org/view/M-R/view/Qpid/job/Qpid-Python-Java-Test/lastCompletedBuild/testReport/ Whilst I think we could change the test harness configuration to pass through the sasl_mechanisms of PLAIN, I wonder why the decision to default has been made? I don't see how this contributes to SSL support. The old code would default to PLAIN if username/password was supplied and PLAIN was support by the Broker (sasl.py:89) which I think was a useful default regardless of Broker choice. Any thoughts please?
          Hide
          JAkub Scholz added a comment -

          New version of the patch - SSL certificates were removed from the SSL transport constructor and are taken directly from the connection object.

          Show
          JAkub Scholz added a comment - New version of the patch - SSL certificates were removed from the SSL transport constructor and are taken directly from the connection object.
          Hide
          Rafael H. Schloming added a comment -

          One comment on the patch. Is there a reason to change the arguments to the transport constructors? Would it be possible to have the keyfile, certfile, etc pulled out of the connection object that is passed to the transport rather than adding them as explicit arguments?

          Show
          Rafael H. Schloming added a comment - One comment on the patch. Is there a reason to change the arguments to the transport constructors? Would it be possible to have the keyfile, certfile, etc pulled out of the connection object that is passed to the transport rather than adding them as explicit arguments?
          Hide
          JAkub Scholz added a comment -

          Attached it the patch described in previous comment. The patch is created against revision 1087326.

          If you would like to change something, let me know, I will see what I can do about it ...

          It would be great if someone can test this on other platforms and versions of Python.

          Show
          JAkub Scholz added a comment - Attached it the patch described in previous comment. The patch is created against revision 1087326. If you would like to change something, let me know, I will see what I can do about it ... It would be great if someone can test this on other platforms and versions of Python.
          Hide
          JAkub Scholz added a comment -

          I tried to solve this issue and got to the following:

          1) Added support for EXTERNAL authentication into sasl.py
          2) Added new attributes to the Connection class in messaging/endpoints.py
          3) Modified the connect method of class Driver in messaging/driver.py
          4) Modifies the TLS class - added certificates as parameters and used them wrap_socket call in messaginf/transports.py

          In general, I added 3 new attributes:
          a) keyfile - client's private key in PEM format
          b) certfile - client's public key (eventually both, public and private keys - in such case the keyfile can be omitted) in PEM format
          c) trustfile - public key of the broker, to verify brokers identity (CRT file is working for me)

          The user uses them when creating new Connection object together with specifiyng the transport (ssl) and the sasl_mechanism (PLAIN or EXTERNAL).

          With these changes I managed to get this to work for unencrypted+PLAIN, ssl+PLAIN and ssl+EXTERNAL.

          However, there are some smaller issues with my solution:
          I) The certificate in trustfile is not verified against the broker - you don't even need to pass the brokers public key to the Connection object and it still works. This seems to be a problem of the Python SSL library. However, I still left the trustfile attribute there, since it may be fixed in the future.
          II) With EXTERNAL authentication, there doesn't seem to be any easy way how to get the username (CN=xxxx in certificates subject) from the certificate. The only option seems to be reading the file and searching for the CN, which I'm not sure is a good idea. Therefore, my current implementation expect the user to enter the username as a normal username attribute when creating the Connection object.
          III) The the PEM certificates are encrypted, the Python asks for password. so it is kinda working. I didn't found any way how to enter the password inside the script.
          IV) The broker URL supports SSL by using the "ampqs://" protocol specification. However, I'm not aware of any existing parameters in the URL string to pass the certificates. Therefore my modifications are working only when passing the specific parameters manually when creating the Connection object instead of using the URL.

          Despite of these problems, I believe it would be worth to commit my changes (or some changes based on this), because the resulting functionality would be better then the existing is.

          Show
          JAkub Scholz added a comment - I tried to solve this issue and got to the following: 1) Added support for EXTERNAL authentication into sasl.py 2) Added new attributes to the Connection class in messaging/endpoints.py 3) Modified the connect method of class Driver in messaging/driver.py 4) Modifies the TLS class - added certificates as parameters and used them wrap_socket call in messaginf/transports.py In general, I added 3 new attributes: a) keyfile - client's private key in PEM format b) certfile - client's public key (eventually both, public and private keys - in such case the keyfile can be omitted) in PEM format c) trustfile - public key of the broker, to verify brokers identity (CRT file is working for me) The user uses them when creating new Connection object together with specifiyng the transport (ssl) and the sasl_mechanism (PLAIN or EXTERNAL). With these changes I managed to get this to work for unencrypted+PLAIN, ssl+PLAIN and ssl+EXTERNAL. However, there are some smaller issues with my solution: I) The certificate in trustfile is not verified against the broker - you don't even need to pass the brokers public key to the Connection object and it still works. This seems to be a problem of the Python SSL library. However, I still left the trustfile attribute there, since it may be fixed in the future. II) With EXTERNAL authentication, there doesn't seem to be any easy way how to get the username (CN=xxxx in certificates subject) from the certificate. The only option seems to be reading the file and searching for the CN, which I'm not sure is a good idea. Therefore, my current implementation expect the user to enter the username as a normal username attribute when creating the Connection object. III) The the PEM certificates are encrypted, the Python asks for password. so it is kinda working. I didn't found any way how to enter the password inside the script. IV) The broker URL supports SSL by using the "ampqs://" protocol specification. However, I'm not aware of any existing parameters in the URL string to pass the certificates. Therefore my modifications are working only when passing the specific parameters manually when creating the Connection object instead of using the URL. Despite of these problems, I believe it would be worth to commit my changes (or some changes based on this), because the resulting functionality would be better then the existing is.

            People

            • Assignee:
              Ken Giusti
              Reporter:
              JAkub Scholz
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development