diff options
Diffstat (limited to '0141-socket-ssl-fix-crl-handling.patch')
| -rw-r--r-- | 0141-socket-ssl-fix-crl-handling.patch | 295 |
1 files changed, 295 insertions, 0 deletions
diff --git a/0141-socket-ssl-fix-crl-handling.patch b/0141-socket-ssl-fix-crl-handling.patch new file mode 100644 index 0000000..4c51ad0 --- /dev/null +++ b/0141-socket-ssl-fix-crl-handling.patch @@ -0,0 +1,295 @@ +From e3020e43344ddbc32e62e06bbbf88a4f5d7cdc82 Mon Sep 17 00:00:00 2001 +From: Mohit Agrawal <moagrawa@redhat.com> +Date: Fri, 10 May 2019 11:13:45 +0530 +Subject: [PATCH 141/141] socket/ssl: fix crl handling + +Problem: +Just setting the path to the CRL directory in socket_init() wasn't working. + +Solution: +Need to use special API to retrieve and set X509_VERIFY_PARAM and set +the CRL checking flags explicitly. +Also, setting the CRL checking flags is a big pain, since the connection +is declared as failed if any CRL isn't found in the designated file or +directory. A comment has been added to the code appropriately. + +> Change-Id: I8a8ed2ddaf4b5eb974387d2f7b1a85c1ca39fe79 +> fixes: bz#1687326 +> Signed-off-by: Milind Changire <mchangir@redhat.com> +> (Cherry pick from commit 06fa261207f0f0625c52fa977b96e5875e9a91e0) +> (Reviewed on upstream link https://review.gluster.org/#/c/glusterfs/+/22334/) + +Change-Id: I0958e9890035fd376f1e1eafc1452caf3edd184b +BUG: 1583585 +Signed-off-by: Mohit Agrawal <moagrawa@redhat.com> +Reviewed-on: https://code.engineering.redhat.com/gerrit/166458 +Tested-by: RHGS Build Bot <nigelb@redhat.com> +Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com> +--- + configure.ac | 2 + + rpc/rpc-transport/socket/src/socket.c | 110 ++++++++++++++++++++++++++++------ + rpc/rpc-transport/socket/src/socket.h | 2 + + tests/features/ssl-ciphers.t | 13 +++- + 4 files changed, 107 insertions(+), 20 deletions(-) + +diff --git a/configure.ac b/configure.ac +index 3065077..0e11d4c 100644 +--- a/configure.ac ++++ b/configure.ac +@@ -491,6 +491,8 @@ AC_CHECK_HEADERS([openssl/dh.h]) + + AC_CHECK_HEADERS([openssl/ecdh.h]) + ++AC_CHECK_LIB([ssl], [SSL_CTX_get0_param], [AC_DEFINE([HAVE_SSL_CTX_GET0_PARAM], [1], [define if found OpenSSL SSL_CTX_get0_param])]) ++ + dnl Math library + AC_CHECK_LIB([m], [pow], [MATH_LIB='-lm'], [MATH_LIB='']) + AC_SUBST(MATH_LIB) +diff --git a/rpc/rpc-transport/socket/src/socket.c b/rpc/rpc-transport/socket/src/socket.c +index f6de1d3..bf2fa71 100644 +--- a/rpc/rpc-transport/socket/src/socket.c ++++ b/rpc/rpc-transport/socket/src/socket.c +@@ -308,8 +308,65 @@ out: + #define ssl_write_one(t, b, l) \ + ssl_do((t), (b), (l), (SSL_trinary_func *)SSL_write) + ++/* set crl verify flags only for server */ ++/* see man X509_VERIFY_PARAM_SET_FLAGS(3) ++ * X509_V_FLAG_CRL_CHECK enables CRL checking for the certificate chain ++ * leaf certificate. An error occurs if a suitable CRL cannot be found. ++ * Since we're never going to revoke a gluster node cert, we better disable ++ * CRL check for server certs to avoid getting error and failed connection ++ * attempts. ++ */ ++static void ++ssl_clear_crl_verify_flags(SSL_CTX *ssl_ctx) ++{ ++#ifdef X509_V_FLAG_CRL_CHECK_ALL ++#ifdef HAVE_SSL_CTX_GET0_PARAM ++ X509_VERIFY_PARAM *vpm; ++ ++ vpm = SSL_CTX_get0_param(ssl_ctx); ++ if (vpm) { ++ X509_VERIFY_PARAM_clear_flags( ++ vpm, (X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL)); ++ } ++#else ++ /* CRL verify flag need not be cleared for rhel6 kind of clients */ ++#endif ++#else ++ gf_log(this->name, GF_LOG_ERROR, "OpenSSL version does not support CRL"); ++#endif ++ return; ++} ++ ++/* set crl verify flags only for server */ ++static void ++ssl_set_crl_verify_flags(SSL_CTX *ssl_ctx) ++{ ++#ifdef X509_V_FLAG_CRL_CHECK_ALL ++#ifdef HAVE_SSL_CTX_GET0_PARAM ++ X509_VERIFY_PARAM *vpm; ++ ++ vpm = SSL_CTX_get0_param(ssl_ctx); ++ if (vpm) { ++ unsigned long flags; ++ ++ flags = X509_VERIFY_PARAM_get_flags(vpm); ++ flags |= (X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL); ++ X509_VERIFY_PARAM_set_flags(vpm, flags); ++ } ++#else ++ X509_STORE *x509store; ++ ++ x509store = SSL_CTX_get_cert_store(ssl_ctx); ++ X509_STORE_set_flags(x509store, ++ X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL); ++#endif ++#else ++ gf_log(this->name, GF_LOG_ERROR, "OpenSSL version does not support CRL"); ++#endif ++} ++ + int +-ssl_setup_connection_prefix(rpc_transport_t *this) ++ssl_setup_connection_prefix(rpc_transport_t *this, gf_boolean_t server) + { + int ret = -1; + socket_private_t *priv = NULL; +@@ -332,6 +389,9 @@ ssl_setup_connection_prefix(rpc_transport_t *this) + priv->ssl_accepted = _gf_false; + priv->ssl_context_created = _gf_false; + ++ if (!server && priv->crl_path) ++ ssl_clear_crl_verify_flags(priv->ssl_ctx); ++ + priv->ssl_ssl = SSL_new(priv->ssl_ctx); + if (!priv->ssl_ssl) { + gf_log(this->name, GF_LOG_ERROR, "SSL_new failed"); +@@ -2664,7 +2724,7 @@ ssl_handle_server_connection_attempt(rpc_transport_t *this) + fd = priv->sock; + + if (!priv->ssl_context_created) { +- ret = ssl_setup_connection_prefix(this); ++ ret = ssl_setup_connection_prefix(this, _gf_true); + if (ret < 0) { + gf_log(this->name, GF_LOG_TRACE, + "> ssl_setup_connection_prefix() failed!"); +@@ -2718,7 +2778,7 @@ ssl_handle_client_connection_attempt(rpc_transport_t *this) + ret = -1; + } else { + if (!priv->ssl_context_created) { +- ret = ssl_setup_connection_prefix(this); ++ ret = ssl_setup_connection_prefix(this, _gf_false); + if (ret < 0) { + gf_log(this->name, GF_LOG_TRACE, + "> ssl_setup_connection_prefix() " +@@ -3085,7 +3145,30 @@ socket_server_event_handler(int fd, int idx, int gen, void *data, int poll_in, + gf_log(this->name, GF_LOG_TRACE, "XXX server:%s, client:%s", + new_trans->myinfo.identifier, new_trans->peerinfo.identifier); + ++ /* Make options available to local socket_init() to create new ++ * SSL_CTX per transport. A separate SSL_CTX per transport is ++ * required to avoid setting crl checking options for client ++ * connections. The verification options eventually get copied ++ * to the SSL object. Unfortunately, there's no way to identify ++ * whether socket_init() is being called after a client-side ++ * connect() or a server-side accept(). Although, we could pass ++ * a flag from the transport init() to the socket_init() and ++ * from this place, this doesn't identify the case where the ++ * server-side transport loading is done for the first time. ++ * Also, SSL doesn't apply for UNIX sockets. ++ */ ++ if (new_sockaddr.ss_family != AF_UNIX) ++ new_trans->options = dict_ref(this->options); ++ new_trans->ctx = this->ctx; ++ + ret = socket_init(new_trans); ++ ++ /* reset options to NULL to avoid double free */ ++ if (new_sockaddr.ss_family != AF_UNIX) { ++ dict_unref(new_trans->options); ++ new_trans->options = NULL; ++ } ++ + if (ret != 0) { + gf_log(this->name, GF_LOG_WARNING, + "initialization of new_trans " +@@ -4150,7 +4233,6 @@ ssl_setup_connection_params(rpc_transport_t *this) + char *cipher_list = DEFAULT_CIPHER_LIST; + char *dh_param = DEFAULT_DH_PARAM; + char *ec_curve = DEFAULT_EC_CURVE; +- char *crl_path = NULL; + + priv = this->private; + +@@ -4192,6 +4274,7 @@ ssl_setup_connection_params(rpc_transport_t *this) + } + priv->ssl_ca_list = gf_strdup(priv->ssl_ca_list); + ++ optstr = NULL; + if (dict_get_str(this->options, SSL_CRL_PATH_OPT, &optstr) == 0) { + if (!priv->ssl_enabled) { + gf_log(this->name, GF_LOG_WARNING, +@@ -4199,9 +4282,9 @@ ssl_setup_connection_params(rpc_transport_t *this) + SSL_ENABLED_OPT); + } + if (strcasecmp(optstr, "NULL") == 0) +- crl_path = NULL; ++ priv->crl_path = NULL; + else +- crl_path = optstr; ++ priv->crl_path = gf_strdup(optstr); + } + + gf_log(this->name, priv->ssl_enabled ? GF_LOG_INFO : GF_LOG_DEBUG, +@@ -4343,24 +4426,15 @@ ssl_setup_connection_params(rpc_transport_t *this) + } + + if (!SSL_CTX_load_verify_locations(priv->ssl_ctx, priv->ssl_ca_list, +- crl_path)) { ++ priv->crl_path)) { + gf_log(this->name, GF_LOG_ERROR, "could not load CA list"); + goto err; + } + + SSL_CTX_set_verify_depth(priv->ssl_ctx, cert_depth); + +- if (crl_path) { +-#ifdef X509_V_FLAG_CRL_CHECK_ALL +- X509_STORE *x509store; +- +- x509store = SSL_CTX_get_cert_store(priv->ssl_ctx); +- X509_STORE_set_flags( +- x509store, X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL); +-#else +- gf_log(this->name, GF_LOG_ERROR, +- "OpenSSL version does not support CRL"); +-#endif ++ if (priv->crl_path) { ++ ssl_set_crl_verify_flags(priv->ssl_ctx); + } + + priv->ssl_session_id = session_id++; +diff --git a/rpc/rpc-transport/socket/src/socket.h b/rpc/rpc-transport/socket/src/socket.h +index e1ccae2..e7c0090 100644 +--- a/rpc/rpc-transport/socket/src/socket.h ++++ b/rpc/rpc-transport/socket/src/socket.h +@@ -14,6 +14,7 @@ + #include <openssl/ssl.h> + #include <openssl/err.h> + #include <openssl/x509v3.h> ++#include <openssl/x509_vfy.h> + #ifdef HAVE_OPENSSL_DH_H + #include <openssl/dh.h> + #endif +@@ -246,6 +247,7 @@ typedef struct { + char *ssl_own_cert; + char *ssl_private_key; + char *ssl_ca_list; ++ char *crl_path; + int pipe[2]; + struct gf_sock_incoming incoming; + /* -1 = not connected. 0 = in progress. 1 = connected */ +diff --git a/tests/features/ssl-ciphers.t b/tests/features/ssl-ciphers.t +index 563d37c..7e1e199 100644 +--- a/tests/features/ssl-ciphers.t ++++ b/tests/features/ssl-ciphers.t +@@ -175,8 +175,6 @@ BRICK_PORT=`brick_port $V0` + EXPECT "Y" openssl_connect -cipher EECDH -connect $H0:$BRICK_PORT + + # test revocation +-# no need to restart the volume since the options are used +-# by the client here. + TEST $CLI volume set $V0 ssl.crl-path $TMPDIR + EXPECT $TMPDIR volume_option $V0 ssl.crl-path + $GFS --volfile-id=$V0 --volfile-server=$H0 $M0 +@@ -189,14 +187,25 @@ TEST openssl ca -batch -config $SSL_CFG -revoke $SSL_CERT 2>&1 + TEST openssl ca -config $SSL_CFG -gencrl -out $SSL_CRL 2>&1 + + # Failed once revoked ++# Although client fails to mount without restarting the server after crl-path ++# is set when no actual crl file is found on the client, it would also fail ++# when server is restarted for the same reason. Since the socket initialization ++# code is the same for client and server, the crl verification flags need to ++# be turned off for the client to avoid SSL searching for CRLs in the ++# ssl.crl-path. If no CRL files are found in the ssl.crl-path, SSL fails the ++# connect() attempt on the client. ++TEST $CLI volume stop $V0 ++TEST $CLI volume start $V0 + $GFS --volfile-id=$V0 --volfile-server=$H0 $M0 + EXPECT "N" wait_mount $M0 + TEST ! test -f $TEST_FILE + EXPECT_WITHIN $UMOUNT_TIMEOUT "Y" force_umount $M0 + + # Succeed with CRL disabled ++TEST $CLI volume stop $V0 + TEST $CLI volume set $V0 ssl.crl-path NULL + EXPECT NULL volume_option $V0 ssl.crl-path ++TEST $CLI volume start $V0 + $GFS --volfile-id=$V0 --volfile-server=$H0 $M0 + EXPECT "Y" wait_mount $M0 + TEST test -f $TEST_FILE +-- +1.8.3.1 + |