summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMilind Changire <mchangir@redhat.com>2019-03-14 10:55:52 +0530
committerRaghavendra G <rgowdapp@redhat.com>2019-03-19 09:38:28 +0000
commit06fa261207f0f0625c52fa977b96e5875e9a91e0 (patch)
treea8d5e215d7fdbbc52c4dac8a4baffde1f0978bf7
parent43092dfd25295aba9d2426a82ea4027e08a7a2c5 (diff)
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>
-rw-r--r--configure.ac2
-rw-r--r--rpc/rpc-transport/socket/src/socket.c111
-rw-r--r--rpc/rpc-transport/socket/src/socket.h2
-rw-r--r--tests/features/ssl-ciphers.t13
4 files changed, 107 insertions, 21 deletions
diff --git a/configure.ac b/configure.ac
index a4cec90..89e330a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -489,6 +489,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 26dbe0b..ecd27e0 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");
@@ -2681,7 +2741,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!");
@@ -2735,7 +2795,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() "
@@ -3102,7 +3162,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 "
@@ -4167,7 +4250,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;
@@ -4209,6 +4291,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,
@@ -4216,9 +4299,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,
@@ -4360,25 +4443,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++;
SSL_CTX_set_session_id_context(priv->ssl_ctx,
diff --git a/rpc/rpc-transport/socket/src/socket.h b/rpc/rpc-transport/socket/src/socket.h
index 32339d3..897d98d 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
@@ -245,6 +246,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