You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by gadLinux <gi...@git.apache.org> on 2017/02/09 16:27:48 UTC
[GitHub] thrift pull request #1185: Thrift 3369 - Third try to merge SSL c_glib
GitHub user gadLinux opened a pull request:
https://github.com/apache/thrift/pull/1185
Thrift 3369 - Third try to merge SSL c_glib
This pull request is the third try to merge against master.
It implements SSL on c_glib with certificate pining included.
The old pull request is 930.
https://github.com/apache/thrift/pull/930
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/gadLinux/thrift thrift-3369
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/thrift/pull/1185.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 #1185
----
commit b1815ec42f848798422765d955cea0a815fcb525
Author: Gonzalo Aguilar Delgado <ga...@level2crm.com>
Date: 2016-03-07T12:42:44Z
Implement basic version of the TLS transport
commit d4c6207d9dabc138ff06a2b21a4477a5c440187c
Author: Gonzalo Aguilar Delgado <ga...@level2crm.com>
Date: 2016-03-07T14:27:22Z
Certificate pinning features incorporated
commit b691372edfadfc124152645ec527686a960cb84b
Author: Gonzalo Aguilar Delgado <ga...@level2crm.com>
Date: 2016-03-07T14:29:10Z
Add test for certificate pinning
commit f09d8eb55246638d8078857d7ef84459caf84c5d
Author: Gonzalo Aguilar Delgado <ga...@level2crm.com>
Date: 2016-03-07T14:30:10Z
Add level2 test public key chain
commit 2003ea188de1be4a13e8b2e859cc3b3d1a39e541
Author: Gonzalo Aguilar Delgado <ga...@level2crm.com>
Date: 2016-03-07T20:49:42Z
Fix dependency of ssl for tests
commit 0f8699ddc3e7f5d2b8c8c36283b0c520d30f7ddf
Author: Gonzalo Aguilar Delgado <ga...@seglan.com>
Date: 2016-08-04T23:33:29Z
Enhace code, fix some issues and plug some bugs.
Fix pull request #930
commit d0f2ac1a957912a59131b2e93028e3022d2a9f82
Author: Gonzalo Aguilar Delgado <ga...@seglan.com>
Date: 2016-08-04T23:40:57Z
Fix style
commit 01dcd95dda883f65ce6baa296815f964ce6f3904
Author: Gonzalo Aguilar Delgado <ga...@seglan.com>
Date: 2016-08-04T23:58:10Z
Add file to cmake
commit 5d9cb52119872f431f187f7166b374e6488b6229
Author: Gonzalo Aguilar Delgado <ga...@seglan.com>
Date: 2016-08-05T09:09:18Z
Fix tests and the bug that produced it.
commit d070e6dc6502e4e9cb8ba5ef3a68c0c00a09800f
Author: Gonzalo Aguilar Delgado <ga...@level2crm.com>
Date: 2016-12-29T11:42:04Z
Fix tests so we don't include tests that require a server until server is ready.
Fix compilation with cmake
commit 40ec9d1bb9469a59ebfe9fea308cb9cebca8157d
Author: Gonzalo Aguilar Delgado <ga...@level2crm.com>
Date: 2016-12-29T11:46:30Z
Fix tests in cmake
commit cada6b8a8907ac00ed9c6995b1d88efd9f0471cc
Author: Gonzalo Aguilar Delgado <ga...@level2crm.com>
Date: 2017-01-26T16:18:16Z
Remove the public key in favor of the existing one.
Re-add the test that was unintentionally removed.
----
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1185
Given we have a failing cross test, merging it doesn't make sense because the evidence suggests it doesn't work. I'll see if I can resolve it.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1185#discussion_r100670306
--- Diff: test/c_glib/src/test_client.c ---
@@ -181,11 +194,22 @@ main (int argc, char **argv)
sigpipe_action.sa_flags = SA_RESETHAND;
sigaction (SIGPIPE, &sigpipe_action, NULL);
+ if (ssl) {
+ thrift_ssl_socket_initialize_openssl();
+ }
+
/* Establish all our connection objects */
- socket = g_object_new (THRIFT_TYPE_SOCKET,
+ socket = g_object_new (socket_type,
"hostname", host,
"port", port,
+ "ssl_accept_selfsigned", TRUE,
NULL);
+ if (ssl && !thrift_ssl_load_cert_from_file(THRIFT_SSL_SOCKET(socket), "../keys/client.crt")) {
--- End diff --
My mistake - I added that line of code in my first pull request and it caused the next issue to occur.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1185#discussion_r100670299
--- Diff: lib/c_glib/src/thrift/c_glib/transport/thrift_ssl_socket.c ---
@@ -0,0 +1,772 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <errno.h>
+#include <netdb.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <openssl/ssl.h>
+#include <pthread.h>
+
+#include <thrift/c_glib/thrift.h>
+#include <thrift/c_glib/transport/thrift_transport.h>
+#include <thrift/c_glib/transport/thrift_socket.h>
+#include <thrift/c_glib/transport/thrift_ssl_socket.h>
+
+#if defined(WIN32)
+#define MUTEX_TYPE HANDLE
+#define MUTEX_SETUP(x) (x) = CreateMutex(NULL, FALSE, NULL)
+#define MUTEX_CLEANUP(x) CloseHandle(x)
+#define MUTEX_LOCK(x) WaitForSingleObject((x), INFINITE)
+#define MUTEX_UNLOCK(x) ReleaseMutex(x)
+#else
+#define MUTEX_TYPE pthread_mutex_t
+#define MUTEX_SETUP(x) pthread_mutex_init(&(x), NULL)
+#define MUTEX_CLEANUP(x) pthread_mutex_destroy(&(x))
+#define MUTEX_LOCK(x) pthread_mutex_lock(&(x))
+#define MUTEX_UNLOCK(x) pthread_mutex_unlock(&(x))
+#endif
+
+#define OPENSSL_VERSION_NO_THREAD_ID 0x10000000L
+
+
+/* object properties */
+enum _ThriftSSLSocketProperties
+{
+ PROP_THRIFT_SSL_SOCKET_CONTEXT = 3,
+ PROP_THRIFT_SSL_SELF_SIGNED
+};
+
+/* To hold a global state management of openssl for all instances */
+static gboolean thrift_ssl_socket_openssl_initialized=FALSE;
+/* Should this be keept at class level? */
+static SSL_CTX* thrift_ssl_socket_global_context=NULL;
+/* This array will store all of the mutexes available to OpenSSL. */
+static MUTEX_TYPE *thrift_ssl_socket_global_mutex_buf=NULL;
+
+
+/**
+ * OpenSSL uniq id function.
+ *
+ * @return thread id
+ */
+static unsigned long thrift_ssl_socket_static_id_function(void)
+{
+#if defined(WIN32)
+ return GetCurrentThreadId();
+#else
+ return ((unsigned long) pthread_self());
+#endif
+}
+
+static void thrift_ssl_socket_static_locking_callback(int mode, int n, const char* unk, int id) {
+ if (mode & CRYPTO_LOCK)
+ MUTEX_LOCK(thrift_ssl_socket_global_mutex_buf[n]);
+ else
+ MUTEX_UNLOCK(thrift_ssl_socket_global_mutex_buf[n]);
+}
+
+static int thrift_ssl_socket_static_thread_setup(void)
+{
+ int i;
+
+ thrift_ssl_socket_global_mutex_buf = malloc(CRYPTO_num_locks() * sizeof(MUTEX_TYPE));
+ if (!thrift_ssl_socket_global_mutex_buf)
+ return 0;
+ for (i = 0; i < CRYPTO_num_locks( ); i++)
+ MUTEX_SETUP(thrift_ssl_socket_global_mutex_buf[i]);
+ CRYPTO_set_id_callback(thrift_ssl_socket_static_id_function);
+ CRYPTO_set_locking_callback(thrift_ssl_socket_static_locking_callback);
+ return 1;
+}
+
+static int thrift_ssl_socket_static_thread_cleanup(void)
+{
+ int i;
+ if (!thrift_ssl_socket_global_mutex_buf)
+ return 0;
+ CRYPTO_set_id_callback(NULL);
+ CRYPTO_set_locking_callback(NULL);
+ for (i = 0; i < CRYPTO_num_locks( ); i++)
+ MUTEX_CLEANUP(thrift_ssl_socket_global_mutex_buf[i]);
+ free(thrift_ssl_socket_global_mutex_buf);
+ thrift_ssl_socket_global_mutex_buf = NULL;
+ return 1;
+}
+
+/*
+static void* thrift_ssl_socket_dyn_lock_create_callback(const char* unk, int id) {
+ g_print("We should create a lock\n");
+ return NULL;
+}
+
+static void thrift_ssl_socket_dyn_lock_callback(int mode, void* lock, const char* unk, int id) {
+ if (lock != NULL) {
+ if (mode & CRYPTO_LOCK) {
+ g_printf("We should lock thread %d\n");
+ } else {
+ g_printf("We should unlock thread %d\n");
+ }
+ }
+}
+
+static void thrift_ssl_socket_dyn_lock_destroy_callback(void* lock, const char* unk, int id) {
+ g_printf("We must destroy the lock\n");
+}
+ */
+
+
+G_DEFINE_TYPE(ThriftSSLSocket, thrift_ssl_socket, THRIFT_TYPE_SOCKET)
+
+
+/* implements thrift_transport_is_open */
+gboolean
+thrift_ssl_socket_is_open (ThriftTransport *transport)
+{
+ return thrift_socket_is_open(transport);
+}
+
+/* overrides thrift_transport_peek */
+gboolean
+thrift_ssl_socket_peek (ThriftTransport *transport, GError **error)
+{
+ gboolean retval = FALSE;
+ ThriftSSLSocket *ssl_socket = THRIFT_SSL_SOCKET (transport);
+ if(ssl_socket!=NULL && THRIFT_SSL_SOCKET_GET_CLASS(ssl_socket)->handle_handshake(transport, error)){
+ if (thrift_ssl_socket_is_open (transport))
+ {
+ int rc;
+ gchar byte;
+ rc = SSL_peek(ssl_socket->ssl, &byte, 1);
+ if (rc < 0) {
+ g_set_error (error,
+ THRIFT_TRANSPORT_ERROR,
+ THRIFT_SSL_SOCKET_ERROR_SSL,
+ "failed to peek at socket - id?");
+ }
+ if (rc == 0) {
+ ERR_clear_error();
+ }
+ retval = (rc > 0);
+ }
+ }
+ return retval;
+}
+
+/* implements thrift_transport_open */
+gboolean
+thrift_ssl_socket_open (ThriftTransport *transport, GError **error)
+{
+ return thrift_socket_open(transport, error);
+}
+
+/* implements thrift_transport_close */
+gboolean
+thrift_ssl_socket_close (ThriftTransport *transport, GError **error)
+{
+ gboolean retval = FALSE;
+ if(THRIFT_SSL_SOCKET(transport)->ssl) {
+ int rc = SSL_shutdown(THRIFT_SSL_SOCKET(transport)->ssl);
+ if (rc < 0) {
+ int errno_copy = THRIFT_SSL_SOCKET_ERROR_SSL;
+ }
+ SSL_free(THRIFT_SSL_SOCKET(transport)->ssl);
+ THRIFT_SSL_SOCKET(transport)->ssl = NULL;
+ ERR_remove_state(0);
+ }
+ return thrift_socket_close(transport, error);
+}
+
+/* implements thrift_transport_read */
+gint32
+thrift_ssl_socket_read (ThriftTransport *transport, gpointer buf,
+ guint32 len, GError **error)
+{
+ guint maxRecvRetries_ = 10;
+ ThriftSSLSocket *ssl_socket = THRIFT_SSL_SOCKET (transport);
+ guint bytes = 0;
+ guint retries = 0;
+ if(ssl_socket!=NULL && THRIFT_SSL_SOCKET_GET_CLASS(ssl_socket)->handle_handshake(transport, error)){
+ for (retries=0; retries < maxRecvRetries_; retries++) {
+ bytes = SSL_read(ssl_socket->ssl, buf, len);
+ if (bytes >= 0)
+ break;
+ int errno_copy = THRIFT_GET_SOCKET_ERROR;
+ if (SSL_get_error(ssl_socket->ssl, bytes) == SSL_ERROR_SYSCALL) {
+ if (ERR_get_error() == 0 && errno_copy == THRIFT_EINTR) {
+ continue;
+ }
+ }
+ g_set_error (error, THRIFT_TRANSPORT_ERROR,
+ THRIFT_TRANSPORT_ERROR_RECEIVE,
+ "failed to read %d bytes - %s", len, strerror(errno));
+ return -1;
+ }
+ }
+ return bytes;
+}
+
+/* implements thrift_transport_read_end
+ * called when write is complete. nothing to do on our end. */
+gboolean
+thrift_ssl_socket_read_end (ThriftTransport *transport, GError **error)
+{
+ /* satisfy -Wall */
+ THRIFT_UNUSED_VAR (transport);
+ THRIFT_UNUSED_VAR (error);
+ return TRUE;
+}
+
+/* implements thrift_transport_write */
+gboolean
+thrift_ssl_socket_write (ThriftTransport *transport, const gpointer buf,
+ const guint32 len, GError **error)
+{
+ ThriftSSLSocket *ssl_socket = THRIFT_SSL_SOCKET (transport);
+ gint ret = 0;
+ guint sent = 0;
+ if(THRIFT_SSL_SOCKET_GET_CLASS(ssl_socket)->handle_handshake(transport, error)){
+
+ ThriftSocket *socket = THRIFT_SSL_SOCKET (transport);
+ g_return_val_if_fail (socket->sd != THRIFT_INVALID_SOCKET, FALSE);
+
+ while (sent < len)
+ {
+ ret = SSL_write (ssl_socket->ssl, (guint8 *)buf + sent, len - sent);
+ if (ret < 0)
+ {
+ g_set_error (error, THRIFT_TRANSPORT_ERROR,
+ THRIFT_TRANSPORT_ERROR_SEND,
+ "failed to send %d bytes - %s", len, strerror(errno));
+ return FALSE;
+ }
+ sent += ret;
+ }
+
+ }
+ return sent==len;
+}
+
+/* implements thrift_transport_write_end
+ * called when write is complete. nothing to do on our end. */
+gboolean
+thrift_ssl_socket_write_end (ThriftTransport *transport, GError **error)
+{
+ /* satisfy -Wall */
+ THRIFT_UNUSED_VAR (transport);
+ THRIFT_UNUSED_VAR (error);
+ return TRUE;
+}
+
+/* implements thrift_transport_flush
+ * flush pending data. since we are not buffered, this is a no-op */
+gboolean
+thrift_ssl_socket_flush (ThriftTransport *transport, GError **error)
+{
+ ThriftSSLSocket *ssl_socket = THRIFT_SSL_SOCKET (transport);
+ gint ret = 0;
+ guint sent = 0;
+ if(THRIFT_SSL_SOCKET_GET_CLASS(ssl_socket)->handle_handshake(transport, error)){
+
+ BIO* bio = SSL_get_wbio(ssl_socket->ssl);
+ if (bio == NULL) {
+ g_set_error (error, THRIFT_TRANSPORT_ERROR,
+ THRIFT_TRANSPORT_ERROR_SEND,
+ "failed to flush, wbio returned null");
+ return FALSE;
+ }
+ if (BIO_flush(bio) != 1) {
+ g_set_error (error, THRIFT_TRANSPORT_ERROR,
+ THRIFT_TRANSPORT_ERROR_SEND,
+ "failed to flush it returned error");
+ return FALSE;
+ }
+
+ }
+ return TRUE;
+}
+
+
+gboolean
+thrift_ssl_socket_handle_handshake(ThriftTransport * transport, GError **error)
+{
+ ThriftSSLSocket *ssl_socket = THRIFT_SSL_SOCKET (transport);
+ ThriftSocket *socket = THRIFT_SOCKET (transport);
+ g_return_val_if_fail (thrift_transport_is_open (transport), FALSE);
+
+ if(ssl_socket==NULL || ssl_socket->ssl!=NULL){
+ return TRUE;
+ }
+
+ if(THRIFT_SSL_SOCKET_GET_CLASS(ssl_socket)->create_ssl_context(transport, error)){
+ /*Context created*/
+ SSL_set_fd(ssl_socket->ssl, socket->sd);
+ int rc;
+ if(ssl_socket->server){
+ rc = SSL_accept(ssl_socket->ssl);
+ }else{
+ rc = SSL_connect(ssl_socket->ssl);
+ }
+ if (rc <= 0) {
+ fprintf(stderr,"The error returned was %d\n", SSL_get_error(ssl_socket->ssl, rc));
+ thrift_ssl_socket_get_error(error, "Not possible to connect", THRIFT_SSL_SOCKET_ERROR_CIPHER_NOT_AVAILABLE);
+ return FALSE;
+ }
+ }else
+ return FALSE;
+
+ return thrift_ssl_socket_authorize(transport, error);
+}
+
+gboolean
+thrift_ssl_socket_create_ssl_context(ThriftTransport * transport, GError **error)
+{
+ ThriftSSLSocket *socket = THRIFT_SSL_SOCKET (transport);
+
+ if(socket->ctx!=NULL){
+ if(socket->ssl!=NULL) {
+ return TRUE;
+ }
+
+ socket->ssl = SSL_new(socket->ctx);
+ if (socket->ssl == NULL) {
+ g_set_error (error, THRIFT_TRANSPORT_ERROR,
+ THRIFT_SSL_SOCKET_ERROR_TRANSPORT,
+ "Unable to create SSL context");
+ return FALSE;
+ }
+ }
+
+ return TRUE;
+}
+
+/**
+ *
+ * @param ssl_socket The ssl socket
+ * @param file_name The file name of the PEM certificate chain
+ * @return
+ */
+gboolean thrift_ssl_load_cert_from_file(ThriftSSLSocket *ssl_socket, const char *file_name)
+{
+ char error_buffer[255];
+ if (!thrift_ssl_socket_openssl_initialized) {
+ g_error("OpenSSL is not initialized yet");
+ return FALSE;
+ }
+ int rc = SSL_CTX_load_verify_locations(ssl_socket->ctx, file_name, NULL);
+ if (rc != 1) { /*verify authentication result*/
+ ERR_error_string_n(ERR_get_error(), error_buffer, 254);
+ g_warning("Load of certificates failed: %s!", error_buffer);
+ return FALSE;
+ }
+ return TRUE;
+}
+
+/**
+ * Load a certificate chain from memory
+ * @param ssl_socket the ssl socket
+ * @param chain_certs the buffer to load PEM from
+ * @return
+ */
+gboolean thrift_ssl_load_cert_from_buffer(ThriftSSLSocket *ssl_socket, const char chain_certs[])
+{
+ gboolean retval = FALSE;
+ /* Load chain of certs*/
+ X509 *cacert=NULL;
+ BIO *mem = BIO_new_mem_buf(chain_certs,strlen(chain_certs));
+ X509_STORE *cert_store = SSL_CTX_get_cert_store(ssl_socket->ctx);
+
+ if(cert_store!=NULL){
+ int index = 0;
+ while ((cacert = PEM_read_bio_X509(mem, NULL, 0, NULL))!=NULL) {
+ if(cacert) {
+ g_debug("Our certificate name is %s", cacert->name);
+ X509_STORE_add_cert(cert_store, cacert);
+ X509_free(cacert);
+ cacert=NULL;
+ } /* Free immediately */
+ index++;
+ }
+ retval=TRUE;
+ }
+ BIO_free(mem);
+ return retval;
+}
+
+gboolean
+thrift_ssl_socket_authorize(ThriftTransport * transport, GError **error)
+{
+ ThriftSocket *socket = THRIFT_SOCKET (transport);
+ ThriftSSLSocket *ssl_socket = THRIFT_SSL_SOCKET (transport);
+ ThriftSSLSocketClass *cls = THRIFT_SSL_SOCKET_GET_CLASS(ssl_socket);
+ gboolean authorization_result = FALSE;
+ /* We still don't support it */
+ if(cls!=NULL && ssl_socket->ssl!=NULL){
+ int rc = SSL_get_verify_result(ssl_socket->ssl);
+ if (rc != X509_V_OK) { /* verify authentication result */
+ if(rc==X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT && ssl_socket->allow_selfsigned){
--- End diff --
Okay that makes sense to me. I just sent @gadLinux a pull request that gets make cross to pass so he can validate any further changes like the ones you suggest.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1185
@gadLinux The current standard for thrift is to use the SSLv23 handshake, but to negotiate at TLSv1.0 or later. See Apache Jira https://issues.apache.org/jira/browse/THRIFT-3165 for that. Code to be checked in today for SSL support should use "SSLTLS" and disable SSL v2 and v3 negotiation which is what I did. So no, we cannot change it to anything else than what I put in my 3rd PR to you.
As a consuming application one should be able to set the accepted version. I know you can do this in C++ and in Java because the constructors to TSSLSocket take an enumeration. In your implementation here how can we change it so that when a thrift_ssl_socket class is initialized, the consuming application can decide the SSL versions to support? I'm not sure this implementation allows for that control as-is.
@nsuke I think I found a way to make the code conditional on OP_NO_SSLv3 support instead of a Python version. I'll post once I get the syntax right.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/thrift/pull/1185
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by nsuke <gi...@git.apache.org>.
Github user nsuke commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1185#discussion_r100669633
--- Diff: test/c_glib/src/test_client.c ---
@@ -181,11 +194,22 @@ main (int argc, char **argv)
sigpipe_action.sa_flags = SA_RESETHAND;
sigaction (SIGPIPE, &sigpipe_action, NULL);
+ if (ssl) {
+ thrift_ssl_socket_initialize_openssl();
+ }
+
/* Establish all our connection objects */
- socket = g_object_new (THRIFT_TYPE_SOCKET,
+ socket = g_object_new (socket_type,
"hostname", host,
"port", port,
+ "ssl_accept_selfsigned", TRUE,
NULL);
+ if (ssl && !thrift_ssl_load_cert_from_file(THRIFT_SSL_SOCKET(socket), "../keys/client.crt")) {
--- End diff --
client.crt is a client certificate file. What you want here is CA.pem.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1185
@gadLinux I submitted a pull request against your branch (https://github.com/gadLinux/thrift/pull/1) to put the cross testing code in place; it is seg faulting so hopefully you can figure out why in short order. With that in place, c_glib ssl will be tested against every server that supports ssl as part of the build.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by nsuke <gi...@git.apache.org>.
Github user nsuke commented on the issue:
https://github.com/apache/thrift/pull/1185
@jeking3 I can confirm that our C# and Python2 doesn't work for TLS 1.1/1.2.
For Python2, it requires Python >= 2.7.9 which includes a lot of security API updates.
Only Debian image has this, among our CI images.
For C#, Mono version itself was actually sufficiently new, if I remember correctly.
We would only need to drop .NET 3.5 compatibility.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1185
This could be issues in the other implementations. While cpp and java passed ssl with c_glib, others like d and csharp did not. I have a docker instance running here, so I will run the full suite and try to figure out what happened.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1185
Interesting failure now. The non-ssl testing against java and cpp work properly but the ssl variant seems to be skewed / off by one. When testVoid() is sent, testString() is returned. Everything is off by one.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by gadLinux <gi...@git.apache.org>.
Github user gadLinux commented on the issue:
https://github.com/apache/thrift/pull/1185
jeking3 Thank you a lot for your patches. They seem to fix the problem you found. But I don't really know why so I will investigate it a little bit after. I want to run my own tests anyway because I'm not sure if pinning will work well like this.
About nsuke concerns, the code can load the CA certificate as well. I will try later to see if it helps.
For now it seems lot better.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1185
@nsuke I am using the ubuntu docker image with python 2.7.6 in it and I changed the ssl server code in python to use SSLv23 unconditionally as a test and ran make cross, and everything passes:
```
diff --git a/lib/py/src/transport/TSSLSocket.py b/lib/py/src/transport/TSSLSocket.py
index 0f7d45e9..86968081 100644
--- a/lib/py/src/transport/TSSLSocket.py
+++ b/lib/py/src/transport/TSSLSocket.py
@@ -40,13 +40,8 @@ class TSSLBase(object):
# ciphers argument is not available for Python < 2.7.0
_has_ciphers = sys.hexversion >= 0x020700F0
- # For pythoon >= 2.7.9, use latest TLS that both client and server
- # supports.
- # SSL 2.0 and 3.0 are disabled via ssl.OP_NO_SSLv2 and ssl.OP_NO_SSLv3.
- # For pythoon < 2.7.9, use TLS 1.0 since TLSv1_X nor OP_NO_SSLvX is
- # unavailable.
- _default_protocol = ssl.PROTOCOL_SSLv23 if _has_ssl_context else \
- ssl.PROTOCOL_TLSv1
+ # SSL 2.0 and 3.0 are disabled via ssl.OP_NO_SSLv2 and ssl.OP_NO_SSLv3
+ _default_protocol = ssl.PROTOCOL_SSLv23
def _init_context(self, ssl_version):
if self._has_ssl_context:
```
By specifying PROTOCOL_TLSv1 it breaks compatibility with the universal SSLv3 hello handshake from which SSL determines the best protocol. This is the reason why the csharp and d servers are not compatible with all the other clients right now. PROTOCOL_SSLv23 is the best choice. The python doesn't break saying the OP_NO_SSLv* are not defined.
I looked at the .NET documentation for the csharp SslProtocol enumeration. I don't see one that allows for the SSLv23 handshake AND disables negotiation of SSLv2 or SSLv3.
There is a story in the backlog I originated to push every SSL implementation by default to negotiate only at TLSv1_2 by default, and the consuming application can then decide if it wants to relax that. This would make consumption of thrift safer. When we do that story, this SSLv23 hello handshake issue will no longer be an issue for the CI build / make cross.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by nsuke <gi...@git.apache.org>.
Github user nsuke commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1185#discussion_r100669840
--- Diff: lib/c_glib/src/thrift/c_glib/transport/thrift_ssl_socket.c ---
@@ -0,0 +1,772 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <errno.h>
+#include <netdb.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <openssl/ssl.h>
+#include <pthread.h>
+
+#include <thrift/c_glib/thrift.h>
+#include <thrift/c_glib/transport/thrift_transport.h>
+#include <thrift/c_glib/transport/thrift_socket.h>
+#include <thrift/c_glib/transport/thrift_ssl_socket.h>
+
+#if defined(WIN32)
+#define MUTEX_TYPE HANDLE
+#define MUTEX_SETUP(x) (x) = CreateMutex(NULL, FALSE, NULL)
+#define MUTEX_CLEANUP(x) CloseHandle(x)
+#define MUTEX_LOCK(x) WaitForSingleObject((x), INFINITE)
+#define MUTEX_UNLOCK(x) ReleaseMutex(x)
+#else
+#define MUTEX_TYPE pthread_mutex_t
+#define MUTEX_SETUP(x) pthread_mutex_init(&(x), NULL)
+#define MUTEX_CLEANUP(x) pthread_mutex_destroy(&(x))
+#define MUTEX_LOCK(x) pthread_mutex_lock(&(x))
+#define MUTEX_UNLOCK(x) pthread_mutex_unlock(&(x))
+#endif
+
+#define OPENSSL_VERSION_NO_THREAD_ID 0x10000000L
+
+
+/* object properties */
+enum _ThriftSSLSocketProperties
+{
+ PROP_THRIFT_SSL_SOCKET_CONTEXT = 3,
+ PROP_THRIFT_SSL_SELF_SIGNED
+};
+
+/* To hold a global state management of openssl for all instances */
+static gboolean thrift_ssl_socket_openssl_initialized=FALSE;
+/* Should this be keept at class level? */
+static SSL_CTX* thrift_ssl_socket_global_context=NULL;
+/* This array will store all of the mutexes available to OpenSSL. */
+static MUTEX_TYPE *thrift_ssl_socket_global_mutex_buf=NULL;
+
+
+/**
+ * OpenSSL uniq id function.
+ *
+ * @return thread id
+ */
+static unsigned long thrift_ssl_socket_static_id_function(void)
+{
+#if defined(WIN32)
+ return GetCurrentThreadId();
+#else
+ return ((unsigned long) pthread_self());
+#endif
+}
+
+static void thrift_ssl_socket_static_locking_callback(int mode, int n, const char* unk, int id) {
+ if (mode & CRYPTO_LOCK)
+ MUTEX_LOCK(thrift_ssl_socket_global_mutex_buf[n]);
+ else
+ MUTEX_UNLOCK(thrift_ssl_socket_global_mutex_buf[n]);
+}
+
+static int thrift_ssl_socket_static_thread_setup(void)
+{
+ int i;
+
+ thrift_ssl_socket_global_mutex_buf = malloc(CRYPTO_num_locks() * sizeof(MUTEX_TYPE));
+ if (!thrift_ssl_socket_global_mutex_buf)
+ return 0;
+ for (i = 0; i < CRYPTO_num_locks( ); i++)
+ MUTEX_SETUP(thrift_ssl_socket_global_mutex_buf[i]);
+ CRYPTO_set_id_callback(thrift_ssl_socket_static_id_function);
+ CRYPTO_set_locking_callback(thrift_ssl_socket_static_locking_callback);
+ return 1;
+}
+
+static int thrift_ssl_socket_static_thread_cleanup(void)
+{
+ int i;
+ if (!thrift_ssl_socket_global_mutex_buf)
+ return 0;
+ CRYPTO_set_id_callback(NULL);
+ CRYPTO_set_locking_callback(NULL);
+ for (i = 0; i < CRYPTO_num_locks( ); i++)
+ MUTEX_CLEANUP(thrift_ssl_socket_global_mutex_buf[i]);
+ free(thrift_ssl_socket_global_mutex_buf);
+ thrift_ssl_socket_global_mutex_buf = NULL;
+ return 1;
+}
+
+/*
+static void* thrift_ssl_socket_dyn_lock_create_callback(const char* unk, int id) {
+ g_print("We should create a lock\n");
+ return NULL;
+}
+
+static void thrift_ssl_socket_dyn_lock_callback(int mode, void* lock, const char* unk, int id) {
+ if (lock != NULL) {
+ if (mode & CRYPTO_LOCK) {
+ g_printf("We should lock thread %d\n");
+ } else {
+ g_printf("We should unlock thread %d\n");
+ }
+ }
+}
+
+static void thrift_ssl_socket_dyn_lock_destroy_callback(void* lock, const char* unk, int id) {
+ g_printf("We must destroy the lock\n");
+}
+ */
+
+
+G_DEFINE_TYPE(ThriftSSLSocket, thrift_ssl_socket, THRIFT_TYPE_SOCKET)
+
+
+/* implements thrift_transport_is_open */
+gboolean
+thrift_ssl_socket_is_open (ThriftTransport *transport)
+{
+ return thrift_socket_is_open(transport);
+}
+
+/* overrides thrift_transport_peek */
+gboolean
+thrift_ssl_socket_peek (ThriftTransport *transport, GError **error)
+{
+ gboolean retval = FALSE;
+ ThriftSSLSocket *ssl_socket = THRIFT_SSL_SOCKET (transport);
+ if(ssl_socket!=NULL && THRIFT_SSL_SOCKET_GET_CLASS(ssl_socket)->handle_handshake(transport, error)){
+ if (thrift_ssl_socket_is_open (transport))
+ {
+ int rc;
+ gchar byte;
+ rc = SSL_peek(ssl_socket->ssl, &byte, 1);
+ if (rc < 0) {
+ g_set_error (error,
+ THRIFT_TRANSPORT_ERROR,
+ THRIFT_SSL_SOCKET_ERROR_SSL,
+ "failed to peek at socket - id?");
+ }
+ if (rc == 0) {
+ ERR_clear_error();
+ }
+ retval = (rc > 0);
+ }
+ }
+ return retval;
+}
+
+/* implements thrift_transport_open */
+gboolean
+thrift_ssl_socket_open (ThriftTransport *transport, GError **error)
+{
+ return thrift_socket_open(transport, error);
+}
+
+/* implements thrift_transport_close */
+gboolean
+thrift_ssl_socket_close (ThriftTransport *transport, GError **error)
+{
+ gboolean retval = FALSE;
+ if(THRIFT_SSL_SOCKET(transport)->ssl) {
+ int rc = SSL_shutdown(THRIFT_SSL_SOCKET(transport)->ssl);
+ if (rc < 0) {
+ int errno_copy = THRIFT_SSL_SOCKET_ERROR_SSL;
+ }
+ SSL_free(THRIFT_SSL_SOCKET(transport)->ssl);
+ THRIFT_SSL_SOCKET(transport)->ssl = NULL;
+ ERR_remove_state(0);
+ }
+ return thrift_socket_close(transport, error);
+}
+
+/* implements thrift_transport_read */
+gint32
+thrift_ssl_socket_read (ThriftTransport *transport, gpointer buf,
+ guint32 len, GError **error)
+{
+ guint maxRecvRetries_ = 10;
+ ThriftSSLSocket *ssl_socket = THRIFT_SSL_SOCKET (transport);
+ guint bytes = 0;
+ guint retries = 0;
+ if(ssl_socket!=NULL && THRIFT_SSL_SOCKET_GET_CLASS(ssl_socket)->handle_handshake(transport, error)){
+ for (retries=0; retries < maxRecvRetries_; retries++) {
+ bytes = SSL_read(ssl_socket->ssl, buf, len);
+ if (bytes >= 0)
+ break;
+ int errno_copy = THRIFT_GET_SOCKET_ERROR;
+ if (SSL_get_error(ssl_socket->ssl, bytes) == SSL_ERROR_SYSCALL) {
+ if (ERR_get_error() == 0 && errno_copy == THRIFT_EINTR) {
+ continue;
+ }
+ }
+ g_set_error (error, THRIFT_TRANSPORT_ERROR,
+ THRIFT_TRANSPORT_ERROR_RECEIVE,
+ "failed to read %d bytes - %s", len, strerror(errno));
+ return -1;
+ }
+ }
+ return bytes;
+}
+
+/* implements thrift_transport_read_end
+ * called when write is complete. nothing to do on our end. */
+gboolean
+thrift_ssl_socket_read_end (ThriftTransport *transport, GError **error)
+{
+ /* satisfy -Wall */
+ THRIFT_UNUSED_VAR (transport);
+ THRIFT_UNUSED_VAR (error);
+ return TRUE;
+}
+
+/* implements thrift_transport_write */
+gboolean
+thrift_ssl_socket_write (ThriftTransport *transport, const gpointer buf,
+ const guint32 len, GError **error)
+{
+ ThriftSSLSocket *ssl_socket = THRIFT_SSL_SOCKET (transport);
+ gint ret = 0;
+ guint sent = 0;
+ if(THRIFT_SSL_SOCKET_GET_CLASS(ssl_socket)->handle_handshake(transport, error)){
+
+ ThriftSocket *socket = THRIFT_SSL_SOCKET (transport);
+ g_return_val_if_fail (socket->sd != THRIFT_INVALID_SOCKET, FALSE);
+
+ while (sent < len)
+ {
+ ret = SSL_write (ssl_socket->ssl, (guint8 *)buf + sent, len - sent);
+ if (ret < 0)
+ {
+ g_set_error (error, THRIFT_TRANSPORT_ERROR,
+ THRIFT_TRANSPORT_ERROR_SEND,
+ "failed to send %d bytes - %s", len, strerror(errno));
+ return FALSE;
+ }
+ sent += ret;
+ }
+
+ }
+ return sent==len;
+}
+
+/* implements thrift_transport_write_end
+ * called when write is complete. nothing to do on our end. */
+gboolean
+thrift_ssl_socket_write_end (ThriftTransport *transport, GError **error)
+{
+ /* satisfy -Wall */
+ THRIFT_UNUSED_VAR (transport);
+ THRIFT_UNUSED_VAR (error);
+ return TRUE;
+}
+
+/* implements thrift_transport_flush
+ * flush pending data. since we are not buffered, this is a no-op */
+gboolean
+thrift_ssl_socket_flush (ThriftTransport *transport, GError **error)
+{
+ ThriftSSLSocket *ssl_socket = THRIFT_SSL_SOCKET (transport);
+ gint ret = 0;
+ guint sent = 0;
+ if(THRIFT_SSL_SOCKET_GET_CLASS(ssl_socket)->handle_handshake(transport, error)){
+
+ BIO* bio = SSL_get_wbio(ssl_socket->ssl);
+ if (bio == NULL) {
+ g_set_error (error, THRIFT_TRANSPORT_ERROR,
+ THRIFT_TRANSPORT_ERROR_SEND,
+ "failed to flush, wbio returned null");
+ return FALSE;
+ }
+ if (BIO_flush(bio) != 1) {
+ g_set_error (error, THRIFT_TRANSPORT_ERROR,
+ THRIFT_TRANSPORT_ERROR_SEND,
+ "failed to flush it returned error");
+ return FALSE;
+ }
+
+ }
+ return TRUE;
+}
+
+
+gboolean
+thrift_ssl_socket_handle_handshake(ThriftTransport * transport, GError **error)
+{
+ ThriftSSLSocket *ssl_socket = THRIFT_SSL_SOCKET (transport);
+ ThriftSocket *socket = THRIFT_SOCKET (transport);
+ g_return_val_if_fail (thrift_transport_is_open (transport), FALSE);
+
+ if(ssl_socket==NULL || ssl_socket->ssl!=NULL){
+ return TRUE;
+ }
+
+ if(THRIFT_SSL_SOCKET_GET_CLASS(ssl_socket)->create_ssl_context(transport, error)){
+ /*Context created*/
+ SSL_set_fd(ssl_socket->ssl, socket->sd);
+ int rc;
+ if(ssl_socket->server){
+ rc = SSL_accept(ssl_socket->ssl);
+ }else{
+ rc = SSL_connect(ssl_socket->ssl);
+ }
+ if (rc <= 0) {
+ fprintf(stderr,"The error returned was %d\n", SSL_get_error(ssl_socket->ssl, rc));
+ thrift_ssl_socket_get_error(error, "Not possible to connect", THRIFT_SSL_SOCKET_ERROR_CIPHER_NOT_AVAILABLE);
+ return FALSE;
+ }
+ }else
+ return FALSE;
+
+ return thrift_ssl_socket_authorize(transport, error);
+}
+
+gboolean
+thrift_ssl_socket_create_ssl_context(ThriftTransport * transport, GError **error)
+{
+ ThriftSSLSocket *socket = THRIFT_SSL_SOCKET (transport);
+
+ if(socket->ctx!=NULL){
+ if(socket->ssl!=NULL) {
+ return TRUE;
+ }
+
+ socket->ssl = SSL_new(socket->ctx);
+ if (socket->ssl == NULL) {
+ g_set_error (error, THRIFT_TRANSPORT_ERROR,
+ THRIFT_SSL_SOCKET_ERROR_TRANSPORT,
+ "Unable to create SSL context");
+ return FALSE;
+ }
+ }
+
+ return TRUE;
+}
+
+/**
+ *
+ * @param ssl_socket The ssl socket
+ * @param file_name The file name of the PEM certificate chain
+ * @return
+ */
+gboolean thrift_ssl_load_cert_from_file(ThriftSSLSocket *ssl_socket, const char *file_name)
+{
+ char error_buffer[255];
+ if (!thrift_ssl_socket_openssl_initialized) {
+ g_error("OpenSSL is not initialized yet");
+ return FALSE;
+ }
+ int rc = SSL_CTX_load_verify_locations(ssl_socket->ctx, file_name, NULL);
+ if (rc != 1) { /*verify authentication result*/
+ ERR_error_string_n(ERR_get_error(), error_buffer, 254);
+ g_warning("Load of certificates failed: %s!", error_buffer);
+ return FALSE;
+ }
+ return TRUE;
+}
+
+/**
+ * Load a certificate chain from memory
+ * @param ssl_socket the ssl socket
+ * @param chain_certs the buffer to load PEM from
+ * @return
+ */
+gboolean thrift_ssl_load_cert_from_buffer(ThriftSSLSocket *ssl_socket, const char chain_certs[])
+{
+ gboolean retval = FALSE;
+ /* Load chain of certs*/
+ X509 *cacert=NULL;
+ BIO *mem = BIO_new_mem_buf(chain_certs,strlen(chain_certs));
+ X509_STORE *cert_store = SSL_CTX_get_cert_store(ssl_socket->ctx);
+
+ if(cert_store!=NULL){
+ int index = 0;
+ while ((cacert = PEM_read_bio_X509(mem, NULL, 0, NULL))!=NULL) {
+ if(cacert) {
+ g_debug("Our certificate name is %s", cacert->name);
+ X509_STORE_add_cert(cert_store, cacert);
+ X509_free(cacert);
+ cacert=NULL;
+ } /* Free immediately */
+ index++;
+ }
+ retval=TRUE;
+ }
+ BIO_free(mem);
+ return retval;
+}
+
+gboolean
+thrift_ssl_socket_authorize(ThriftTransport * transport, GError **error)
+{
+ ThriftSocket *socket = THRIFT_SOCKET (transport);
+ ThriftSSLSocket *ssl_socket = THRIFT_SSL_SOCKET (transport);
+ ThriftSSLSocketClass *cls = THRIFT_SSL_SOCKET_GET_CLASS(ssl_socket);
+ gboolean authorization_result = FALSE;
+ /* We still don't support it */
+ if(cls!=NULL && ssl_socket->ssl!=NULL){
+ int rc = SSL_get_verify_result(ssl_socket->ssl);
+ if (rc != X509_V_OK) { /* verify authentication result */
+ if(rc==X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT && ssl_socket->allow_selfsigned){
--- End diff --
I hear this error is for **untrusted** root certificate (couldn't find definitive documentation).
I suspect that once the CA.pem path is fixed for cross tests, the original error would disappear.
If it's the case, allow_selfsigned option would be dangerously misleading and should be removed.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1185
@nsuke something like this seems to work in the ubuntu docker image:
```
has_secure_ssl_handshake = 0
try:
from OpenSSL.SSL import OP_NO_SSLv3
has_secure_ssl_handshake = 1
except ImportError:
has_secure_ssl_handshake = 0
then later...
# SSL 2.0 and 3.0 are disabled via ssl.OP_NO_SSLv2 and ssl.OP_NO_SSLv3
if (has_secure_ssl_handshake):
_default_protocol = ssl.PROTOCOL_SSLv23
else:
_default_protocol = ssl.PROTOCOL_TLSv1
```
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by gadLinux <gi...@git.apache.org>.
Github user gadLinux commented on a diff in the pull request:
https://github.com/apache/thrift/pull/1185#discussion_r100681388
--- Diff: lib/c_glib/src/thrift/c_glib/transport/thrift_ssl_socket.c ---
@@ -0,0 +1,772 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <errno.h>
+#include <netdb.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <openssl/ssl.h>
+#include <pthread.h>
+
+#include <thrift/c_glib/thrift.h>
+#include <thrift/c_glib/transport/thrift_transport.h>
+#include <thrift/c_glib/transport/thrift_socket.h>
+#include <thrift/c_glib/transport/thrift_ssl_socket.h>
+
+#if defined(WIN32)
+#define MUTEX_TYPE HANDLE
+#define MUTEX_SETUP(x) (x) = CreateMutex(NULL, FALSE, NULL)
+#define MUTEX_CLEANUP(x) CloseHandle(x)
+#define MUTEX_LOCK(x) WaitForSingleObject((x), INFINITE)
+#define MUTEX_UNLOCK(x) ReleaseMutex(x)
+#else
+#define MUTEX_TYPE pthread_mutex_t
+#define MUTEX_SETUP(x) pthread_mutex_init(&(x), NULL)
+#define MUTEX_CLEANUP(x) pthread_mutex_destroy(&(x))
+#define MUTEX_LOCK(x) pthread_mutex_lock(&(x))
+#define MUTEX_UNLOCK(x) pthread_mutex_unlock(&(x))
+#endif
+
+#define OPENSSL_VERSION_NO_THREAD_ID 0x10000000L
+
+
+/* object properties */
+enum _ThriftSSLSocketProperties
+{
+ PROP_THRIFT_SSL_SOCKET_CONTEXT = 3,
+ PROP_THRIFT_SSL_SELF_SIGNED
+};
+
+/* To hold a global state management of openssl for all instances */
+static gboolean thrift_ssl_socket_openssl_initialized=FALSE;
+/* Should this be keept at class level? */
+static SSL_CTX* thrift_ssl_socket_global_context=NULL;
+/* This array will store all of the mutexes available to OpenSSL. */
+static MUTEX_TYPE *thrift_ssl_socket_global_mutex_buf=NULL;
+
+
+/**
+ * OpenSSL uniq id function.
+ *
+ * @return thread id
+ */
+static unsigned long thrift_ssl_socket_static_id_function(void)
+{
+#if defined(WIN32)
+ return GetCurrentThreadId();
+#else
+ return ((unsigned long) pthread_self());
+#endif
+}
+
+static void thrift_ssl_socket_static_locking_callback(int mode, int n, const char* unk, int id) {
+ if (mode & CRYPTO_LOCK)
+ MUTEX_LOCK(thrift_ssl_socket_global_mutex_buf[n]);
+ else
+ MUTEX_UNLOCK(thrift_ssl_socket_global_mutex_buf[n]);
+}
+
+static int thrift_ssl_socket_static_thread_setup(void)
+{
+ int i;
+
+ thrift_ssl_socket_global_mutex_buf = malloc(CRYPTO_num_locks() * sizeof(MUTEX_TYPE));
+ if (!thrift_ssl_socket_global_mutex_buf)
+ return 0;
+ for (i = 0; i < CRYPTO_num_locks( ); i++)
+ MUTEX_SETUP(thrift_ssl_socket_global_mutex_buf[i]);
+ CRYPTO_set_id_callback(thrift_ssl_socket_static_id_function);
+ CRYPTO_set_locking_callback(thrift_ssl_socket_static_locking_callback);
+ return 1;
+}
+
+static int thrift_ssl_socket_static_thread_cleanup(void)
+{
+ int i;
+ if (!thrift_ssl_socket_global_mutex_buf)
+ return 0;
+ CRYPTO_set_id_callback(NULL);
+ CRYPTO_set_locking_callback(NULL);
+ for (i = 0; i < CRYPTO_num_locks( ); i++)
+ MUTEX_CLEANUP(thrift_ssl_socket_global_mutex_buf[i]);
+ free(thrift_ssl_socket_global_mutex_buf);
+ thrift_ssl_socket_global_mutex_buf = NULL;
+ return 1;
+}
+
+/*
+static void* thrift_ssl_socket_dyn_lock_create_callback(const char* unk, int id) {
+ g_print("We should create a lock\n");
+ return NULL;
+}
+
+static void thrift_ssl_socket_dyn_lock_callback(int mode, void* lock, const char* unk, int id) {
+ if (lock != NULL) {
+ if (mode & CRYPTO_LOCK) {
+ g_printf("We should lock thread %d\n");
+ } else {
+ g_printf("We should unlock thread %d\n");
+ }
+ }
+}
+
+static void thrift_ssl_socket_dyn_lock_destroy_callback(void* lock, const char* unk, int id) {
+ g_printf("We must destroy the lock\n");
+}
+ */
+
+
+G_DEFINE_TYPE(ThriftSSLSocket, thrift_ssl_socket, THRIFT_TYPE_SOCKET)
+
+
+/* implements thrift_transport_is_open */
+gboolean
+thrift_ssl_socket_is_open (ThriftTransport *transport)
+{
+ return thrift_socket_is_open(transport);
+}
+
+/* overrides thrift_transport_peek */
+gboolean
+thrift_ssl_socket_peek (ThriftTransport *transport, GError **error)
+{
+ gboolean retval = FALSE;
+ ThriftSSLSocket *ssl_socket = THRIFT_SSL_SOCKET (transport);
+ if(ssl_socket!=NULL && THRIFT_SSL_SOCKET_GET_CLASS(ssl_socket)->handle_handshake(transport, error)){
+ if (thrift_ssl_socket_is_open (transport))
+ {
+ int rc;
+ gchar byte;
+ rc = SSL_peek(ssl_socket->ssl, &byte, 1);
+ if (rc < 0) {
+ g_set_error (error,
+ THRIFT_TRANSPORT_ERROR,
+ THRIFT_SSL_SOCKET_ERROR_SSL,
+ "failed to peek at socket - id?");
+ }
+ if (rc == 0) {
+ ERR_clear_error();
+ }
+ retval = (rc > 0);
+ }
+ }
+ return retval;
+}
+
+/* implements thrift_transport_open */
+gboolean
+thrift_ssl_socket_open (ThriftTransport *transport, GError **error)
+{
+ return thrift_socket_open(transport, error);
+}
+
+/* implements thrift_transport_close */
+gboolean
+thrift_ssl_socket_close (ThriftTransport *transport, GError **error)
+{
+ gboolean retval = FALSE;
+ if(THRIFT_SSL_SOCKET(transport)->ssl) {
+ int rc = SSL_shutdown(THRIFT_SSL_SOCKET(transport)->ssl);
+ if (rc < 0) {
+ int errno_copy = THRIFT_SSL_SOCKET_ERROR_SSL;
+ }
+ SSL_free(THRIFT_SSL_SOCKET(transport)->ssl);
+ THRIFT_SSL_SOCKET(transport)->ssl = NULL;
+ ERR_remove_state(0);
+ }
+ return thrift_socket_close(transport, error);
+}
+
+/* implements thrift_transport_read */
+gint32
+thrift_ssl_socket_read (ThriftTransport *transport, gpointer buf,
+ guint32 len, GError **error)
+{
+ guint maxRecvRetries_ = 10;
+ ThriftSSLSocket *ssl_socket = THRIFT_SSL_SOCKET (transport);
+ guint bytes = 0;
+ guint retries = 0;
+ if(ssl_socket!=NULL && THRIFT_SSL_SOCKET_GET_CLASS(ssl_socket)->handle_handshake(transport, error)){
+ for (retries=0; retries < maxRecvRetries_; retries++) {
+ bytes = SSL_read(ssl_socket->ssl, buf, len);
+ if (bytes >= 0)
+ break;
+ int errno_copy = THRIFT_GET_SOCKET_ERROR;
+ if (SSL_get_error(ssl_socket->ssl, bytes) == SSL_ERROR_SYSCALL) {
+ if (ERR_get_error() == 0 && errno_copy == THRIFT_EINTR) {
+ continue;
+ }
+ }
+ g_set_error (error, THRIFT_TRANSPORT_ERROR,
+ THRIFT_TRANSPORT_ERROR_RECEIVE,
+ "failed to read %d bytes - %s", len, strerror(errno));
+ return -1;
+ }
+ }
+ return bytes;
+}
+
+/* implements thrift_transport_read_end
+ * called when write is complete. nothing to do on our end. */
+gboolean
+thrift_ssl_socket_read_end (ThriftTransport *transport, GError **error)
+{
+ /* satisfy -Wall */
+ THRIFT_UNUSED_VAR (transport);
+ THRIFT_UNUSED_VAR (error);
+ return TRUE;
+}
+
+/* implements thrift_transport_write */
+gboolean
+thrift_ssl_socket_write (ThriftTransport *transport, const gpointer buf,
+ const guint32 len, GError **error)
+{
+ ThriftSSLSocket *ssl_socket = THRIFT_SSL_SOCKET (transport);
+ gint ret = 0;
+ guint sent = 0;
+ if(THRIFT_SSL_SOCKET_GET_CLASS(ssl_socket)->handle_handshake(transport, error)){
+
+ ThriftSocket *socket = THRIFT_SSL_SOCKET (transport);
+ g_return_val_if_fail (socket->sd != THRIFT_INVALID_SOCKET, FALSE);
+
+ while (sent < len)
+ {
+ ret = SSL_write (ssl_socket->ssl, (guint8 *)buf + sent, len - sent);
+ if (ret < 0)
+ {
+ g_set_error (error, THRIFT_TRANSPORT_ERROR,
+ THRIFT_TRANSPORT_ERROR_SEND,
+ "failed to send %d bytes - %s", len, strerror(errno));
+ return FALSE;
+ }
+ sent += ret;
+ }
+
+ }
+ return sent==len;
+}
+
+/* implements thrift_transport_write_end
+ * called when write is complete. nothing to do on our end. */
+gboolean
+thrift_ssl_socket_write_end (ThriftTransport *transport, GError **error)
+{
+ /* satisfy -Wall */
+ THRIFT_UNUSED_VAR (transport);
+ THRIFT_UNUSED_VAR (error);
+ return TRUE;
+}
+
+/* implements thrift_transport_flush
+ * flush pending data. since we are not buffered, this is a no-op */
+gboolean
+thrift_ssl_socket_flush (ThriftTransport *transport, GError **error)
+{
+ ThriftSSLSocket *ssl_socket = THRIFT_SSL_SOCKET (transport);
+ gint ret = 0;
+ guint sent = 0;
+ if(THRIFT_SSL_SOCKET_GET_CLASS(ssl_socket)->handle_handshake(transport, error)){
+
+ BIO* bio = SSL_get_wbio(ssl_socket->ssl);
+ if (bio == NULL) {
+ g_set_error (error, THRIFT_TRANSPORT_ERROR,
+ THRIFT_TRANSPORT_ERROR_SEND,
+ "failed to flush, wbio returned null");
+ return FALSE;
+ }
+ if (BIO_flush(bio) != 1) {
+ g_set_error (error, THRIFT_TRANSPORT_ERROR,
+ THRIFT_TRANSPORT_ERROR_SEND,
+ "failed to flush it returned error");
+ return FALSE;
+ }
+
+ }
+ return TRUE;
+}
+
+
+gboolean
+thrift_ssl_socket_handle_handshake(ThriftTransport * transport, GError **error)
+{
+ ThriftSSLSocket *ssl_socket = THRIFT_SSL_SOCKET (transport);
+ ThriftSocket *socket = THRIFT_SOCKET (transport);
+ g_return_val_if_fail (thrift_transport_is_open (transport), FALSE);
+
+ if(ssl_socket==NULL || ssl_socket->ssl!=NULL){
+ return TRUE;
+ }
+
+ if(THRIFT_SSL_SOCKET_GET_CLASS(ssl_socket)->create_ssl_context(transport, error)){
+ /*Context created*/
+ SSL_set_fd(ssl_socket->ssl, socket->sd);
+ int rc;
+ if(ssl_socket->server){
+ rc = SSL_accept(ssl_socket->ssl);
+ }else{
+ rc = SSL_connect(ssl_socket->ssl);
+ }
+ if (rc <= 0) {
+ fprintf(stderr,"The error returned was %d\n", SSL_get_error(ssl_socket->ssl, rc));
+ thrift_ssl_socket_get_error(error, "Not possible to connect", THRIFT_SSL_SOCKET_ERROR_CIPHER_NOT_AVAILABLE);
+ return FALSE;
+ }
+ }else
+ return FALSE;
+
+ return thrift_ssl_socket_authorize(transport, error);
+}
+
+gboolean
+thrift_ssl_socket_create_ssl_context(ThriftTransport * transport, GError **error)
+{
+ ThriftSSLSocket *socket = THRIFT_SSL_SOCKET (transport);
+
+ if(socket->ctx!=NULL){
+ if(socket->ssl!=NULL) {
+ return TRUE;
+ }
+
+ socket->ssl = SSL_new(socket->ctx);
+ if (socket->ssl == NULL) {
+ g_set_error (error, THRIFT_TRANSPORT_ERROR,
+ THRIFT_SSL_SOCKET_ERROR_TRANSPORT,
+ "Unable to create SSL context");
+ return FALSE;
+ }
+ }
+
+ return TRUE;
+}
+
+/**
+ *
+ * @param ssl_socket The ssl socket
+ * @param file_name The file name of the PEM certificate chain
+ * @return
+ */
+gboolean thrift_ssl_load_cert_from_file(ThriftSSLSocket *ssl_socket, const char *file_name)
+{
+ char error_buffer[255];
+ if (!thrift_ssl_socket_openssl_initialized) {
+ g_error("OpenSSL is not initialized yet");
+ return FALSE;
+ }
+ int rc = SSL_CTX_load_verify_locations(ssl_socket->ctx, file_name, NULL);
+ if (rc != 1) { /*verify authentication result*/
+ ERR_error_string_n(ERR_get_error(), error_buffer, 254);
+ g_warning("Load of certificates failed: %s!", error_buffer);
+ return FALSE;
+ }
+ return TRUE;
+}
+
+/**
+ * Load a certificate chain from memory
+ * @param ssl_socket the ssl socket
+ * @param chain_certs the buffer to load PEM from
+ * @return
+ */
+gboolean thrift_ssl_load_cert_from_buffer(ThriftSSLSocket *ssl_socket, const char chain_certs[])
+{
+ gboolean retval = FALSE;
+ /* Load chain of certs*/
+ X509 *cacert=NULL;
+ BIO *mem = BIO_new_mem_buf(chain_certs,strlen(chain_certs));
+ X509_STORE *cert_store = SSL_CTX_get_cert_store(ssl_socket->ctx);
+
+ if(cert_store!=NULL){
+ int index = 0;
+ while ((cacert = PEM_read_bio_X509(mem, NULL, 0, NULL))!=NULL) {
+ if(cacert) {
+ g_debug("Our certificate name is %s", cacert->name);
+ X509_STORE_add_cert(cert_store, cacert);
+ X509_free(cacert);
+ cacert=NULL;
+ } /* Free immediately */
+ index++;
+ }
+ retval=TRUE;
+ }
+ BIO_free(mem);
+ return retval;
+}
+
+gboolean
+thrift_ssl_socket_authorize(ThriftTransport * transport, GError **error)
+{
+ ThriftSocket *socket = THRIFT_SOCKET (transport);
+ ThriftSSLSocket *ssl_socket = THRIFT_SSL_SOCKET (transport);
+ ThriftSSLSocketClass *cls = THRIFT_SSL_SOCKET_GET_CLASS(ssl_socket);
+ gboolean authorization_result = FALSE;
+ /* We still don't support it */
+ if(cls!=NULL && ssl_socket->ssl!=NULL){
+ int rc = SSL_get_verify_result(ssl_socket->ssl);
+ if (rc != X509_V_OK) { /* verify authentication result */
+ if(rc==X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT && ssl_socket->allow_selfsigned){
--- End diff --
The selfsigned verification can be also skipped by doing other kind of initialization in openssl. This is why we provide our own function. To make sure it will enforce the check. So I'm not happy with it but it's necessary for now. Hope we can remove in the near future.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by gadLinux <gi...@git.apache.org>.
Github user gadLinux commented on the issue:
https://github.com/apache/thrift/pull/1185
Hi Jeking3,
Yes there's only the client at this time. Cause I had no time to implement server. The tests can be run against Java or any other server. I will add after this commit is approved.
The tests I added are rather limited because the lack of c_glib server. So yes I will add them at latter step.
About cross checks. It's the first time I see about it. In fact all tests I did where against Java so they are naturally cross language. Again this should be added at a latter step. I need the client to be "in" the next release cause lot of effort has been set securing it. So the patch is big enough to split it in different issues.
We can discuss later about a common library between C++ and c_glib, more testing, server and many other improvements. But I think it's a good idea to have this in now that's running. In our case is in production. Tested daily against hundreds of clients. And it's working fine.
Can you merge it as is, please?
And prepare after in the maillist the next features.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1185
autoconf is deprecated and there is an epic in the Jira backlog to remove it in favor of cmake. I agree maintaining two build systems is a bit painful. I believe it is possible to have "make cross" do a client-only transport. I'm going to take your pull request and see if I can enable cross test on the ip-ssl transport and run it in the docker container like the build would.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by gadLinux <gi...@git.apache.org>.
Github user gadLinux commented on the issue:
https://github.com/apache/thrift/pull/1185
Thank you for the effort
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by gadLinux <gi...@git.apache.org>.
Github user gadLinux commented on the issue:
https://github.com/apache/thrift/pull/1185
Okay. I revamped a little bit the system to allow it to run the cross tests. Since you are not using specific functions provided by the API it was causing to not initialize the instance properly.
I added sane default values for SSL. It will use TLSv12 by default with dissallowed selfsigned certificates and basic pining function.
So I also changed the test unit to allow selfsigned certificates when the client is created.
Now the tests run but fails. But there I don't what's the reason. It's because Java, because the test is wrong, or the code in SSL branch is wrong. The latter seems difficult since it works in our environment. So maybe you can help to fix the tests. The important thing is that now it seems to be properly configured even if no API provided is used.
Also a couple of straightforward errors on test were corrected.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1185
One issue I found was that TLSv1_2 was being specified. We use SSLv23 with SSLv2 and SSLv3 disabled; this allows OpenSSL to negotiate at the highest possible level including TLSv1.2. So I changed the c_glib ssl client socket code to use SSLTLS as the default protocol (which is SSLv23 as described above).
At that point I expected it to interop with the python ssl server in make cross. The docker image has python 2.7.6 and there was some conditional code in the python server class that seems to be restricting it to TLSv1_0 specifically. I set it to use the same logic (SSLv23 with v2 and v3 disabled).
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by nsuke <gi...@git.apache.org>.
Github user nsuke commented on the issue:
https://github.com/apache/thrift/pull/1185
@jeking3 in that case, Ubuntu might be using a patched version of Python.
According to the [doc](https://docs.python.org/2/library/ssl.html#ssl.OP_NO_SSLv2), it's added in 2.7.9.
> ssl.OP_NO_SSLv2
> Prevents an SSLv2 connection. This option is only applicable in conjunction with PROTOCOL_SSLv23. It prevents the peers from choosing SSLv2 as the protocol version.
> New in version 2.7.9.
We may want to directly check the existence of `OP_NO_SSLLv*` instead of current blanket version check.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by gadLinux <gi...@git.apache.org>.
Github user gadLinux commented on the issue:
https://github.com/apache/thrift/pull/1185
Hi, I must say that TLS1.2 is the recommended version for SSL because SSL3 is so buggy and insecure that everyone is filtering it. For testing can be ok.
I would not change the default one. I would change just for the test. I suppose I have to write a brief documentation to let people know how to do it. i will do in my blog but I can send the patches for the apache web.
One question? Do you sleep? Cause I have responses at all hours! You are so great. Thank you so much.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by gadLinux <gi...@git.apache.org>.
Github user gadLinux commented on the issue:
https://github.com/apache/thrift/pull/1185
Let's fix it latter, please. I'm tired of getting this issue around. I will open a bug report if you want to track it.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by Jens-G <gi...@git.apache.org>.
Github user Jens-G commented on the issue:
https://github.com/apache/thrift/pull/1185
We are in middle of the transition process from automake to cmake. Until we completed this task, both are relevant.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1185
I opened THRIFT-4084 as a mechanism to verify all the SSL server implementations are secure "enough" at their default settings.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1185
My guess is that the python tests are no longer correct, or the method of making python use SSLv23 in the code we merged is wrong. What I would suggest is backing out the last patch db9ac77f1e9c01ae57a9c6df48cb8670e9045ebb which changed the python stuff. I have cherry picked that into THRIFT-4084 which I am working on so you can drop it. I will send you another PR shortly which eliminates this non-c_glib stuff from your effort and passes both "make check" and "make cross" locally.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by gadLinux <gi...@git.apache.org>.
Github user gadLinux commented on the issue:
https://github.com/apache/thrift/pull/1185
Oh, it failed again. How can this be? Should I rebase again?
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by gadLinux <gi...@git.apache.org>.
Github user gadLinux commented on the issue:
https://github.com/apache/thrift/pull/1185
Seems it's still failing. Do you think I can modify the back the protocol and use SSL2-3 for the tests?
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1185
Thanks, I'll wait for the CI build results before I comment.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by gadLinux <gi...@git.apache.org>.
Github user gadLinux commented on the issue:
https://github.com/apache/thrift/pull/1185
I cross my fingers so it passes the tests. I want to continue coding but I don't want to do it until we have this issue merged.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1185
I sent you a pull request that fixes the c_glib library and allows the ssl make cross tests to pass with a c_glib ip-sssl client against cpp and java servers. Didn't test the others, we'll let CI do that.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by gadLinux <gi...@git.apache.org>.
Github user gadLinux commented on the issue:
https://github.com/apache/thrift/pull/1185
Hi again,
As the code is pure c we should be able to support all configurations. It makes me a little surprised to mix mechanism in handshake and negotiation but as far as you know what you are requesting is ok.
I'm working passing some certifications PCI, VISA and Master and none of them allows SSL3 to run. Even for handshake. This is the reason why we stick to TLS_1.2.
I will check the code a little bit for this way of doing when I have a time slot today. Is there a predefined server that does this way. I suppose that Java is already doing and suppose it's already working since some tests are passing. Can you confirm, please?
Thank you a lot for your effort again.
Best regards,
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by gadLinux <gi...@git.apache.org>.
Github user gadLinux commented on the issue:
https://github.com/apache/thrift/pull/1185
I forgot to say that also the code was rebased. A curious thing is that everytime I do a rebase the system says that my code diverged... I don't know why since always rebasing the upstream master branch.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by gadLinux <gi...@git.apache.org>.
Github user gadLinux commented on the issue:
https://github.com/apache/thrift/pull/1185
It seems it still not fine... Do I have to see something?
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1185
I've also found that the csharp and the d interop for ssl have been almost completely disabled by the known_test_failures file. In looking at the csharp implementation it does not accept SSLv3 at all, which means it won't accept the more universal SSLv23 handshake after which it bumps up to TLSv1_2. This is probably why all of them are disabled; I suspect the "d" server is affected the same way. I'm okay adding the c_glib failing tests to these blocks for now. I'll send you another PR that has make cross passing.
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by gadLinux <gi...@git.apache.org>.
Github user gadLinux commented on the issue:
https://github.com/apache/thrift/pull/1185
Just a question. Why all CMake stuff. Having two compilation environments seems to me somewhat overloaded... Does it worth it?
---
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 #1185: Thrift 3369 - Third try to merge SSL c_glib
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1185
@gadLinux sent you a pull request and changed to use the proper validation cert @nsuke pointed out as part of it
---
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.
---