You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by udit043 <gi...@git.apache.org> on 2016/07/30 18:04:21 UTC

[GitHub] thrift pull request #1057: Update TSSLSocket.cpp

GitHub user udit043 opened a pull request:

    https://github.com/apache/thrift/pull/1057

    Update TSSLSocket.cpp

    Older version of Open SSL may create problem because they do not support 'SSL_set_tlsext_host_name()'.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/udit043/thrift patch-1

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/thrift/pull/1057.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1057
    
----
commit 606750e9bb2e75e93d90bc20260986274b1b3729
Author: Udit Raikwar <ud...@users.noreply.github.com>
Date:   2016-07-30T18:00:25Z

    Update TSSLSocket.cpp
    
    Older version of Open SSL may create problem because they do not support 'SSL_set_tlsext_host_name()'.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] thrift pull request #1057: Update TSSLSocket.cpp

Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on a diff in the pull request:

    https://github.com/apache/thrift/pull/1057#discussion_r72976807
  
    --- Diff: lib/cpp/src/thrift/transport/TSSLSocket.cpp ---
    @@ -471,8 +471,10 @@ void TSSLSocket::checkHandshake() {
           }
         } while (rc == 2);
       } else {
    -    // set the SNI hostname
    -    SSL_set_tlsext_host_name(ssl_, getHost().c_str());
    +    /* OpenSSL < 0.9.8f does not have SSL_set_tlsext_host_name() */
    +    #if defined(SSL_set_tlsext_host_name) // set the SNI hostname
    --- End diff --
    
    Typically we would add something to the build system environment to differentiate this; also is there an alternative that can be used with older OpenSSL?  Other folks who are committers will need to decide if it is worth supporting an older and likely quite vulnerable (to hacks) OpenSSL library.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] thrift issue #1057: Update TSSLSocket.cpp

Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:

    https://github.com/apache/thrift/pull/1057
  
    Hi, could you please create a ticket for this in Apache Jira following the instructions at: https://thrift.apache.org/docs/HowToContribute
    
    Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] thrift pull request #1057: Update TSSLSocket.cpp

Posted by udit043 <gi...@git.apache.org>.
Github user udit043 commented on a diff in the pull request:

    https://github.com/apache/thrift/pull/1057#discussion_r73373066
  
    --- Diff: lib/cpp/src/thrift/transport/TSSLSocket.cpp ---
    @@ -471,8 +471,10 @@ void TSSLSocket::checkHandshake() {
           }
         } while (rc == 2);
       } else {
    -    // set the SNI hostname
    -    SSL_set_tlsext_host_name(ssl_, getHost().c_str());
    +    /* OpenSSL < 0.9.8f does not have SSL_set_tlsext_host_name() */
    +    #if defined(SSL_set_tlsext_host_name) // set the SNI hostname
    --- End diff --
    
    I have created a issue regarding this: https://issues.apache.org/jira/browse/THRIFT-3892
    No, there is not any alternative for older version. There are very less possibilities of someone using very old OpenSSL library. This issue is negligible, it is just for improving the project.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] thrift issue #1057: Update TSSLSocket.cpp

Posted by udit043 <gi...@git.apache.org>.
Github user udit043 commented on the issue:

    https://github.com/apache/thrift/pull/1057
  
    Okay


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] thrift pull request #1057: Update TSSLSocket.cpp

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/thrift/pull/1057


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---