From 00c78b9eb52d8a631cdaef883cd507bd0889639a Mon Sep 17 00:00:00 2001
From: Amar Tumballi <amarts@redhat.com>
Date: Fri, 28 Sep 2018 12:06:09 +0530
Subject: [PATCH 396/399] protocol: remove the option 'verify-volfile-checksum'
'getspec' operation is not used between 'client' and 'server' ever since
we have off-loaded volfile management to glusterd, ie, at least 7 years.
No reason to keep the dead code! The removed option had no meaning,
as glusterd didn't provide a way to set (or unset) this option. So,
no regression should be observed from any of the existing glusterfs
deployment, supported or unsupported.
Fixes: CVE-2018-14653
BUG: 1634668
Change-Id: I8b3a4d302b3c222e065b484cfe449b9c116393f8
Signed-off-by: Amar Tumballi <amarts@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/151322
Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>
Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>
---
xlators/protocol/client/src/client-handshake.c | 83 +-------
xlators/protocol/server/src/server-handshake.c | 276 +------------------------
xlators/protocol/server/src/server.c | 3 -
3 files changed, 5 insertions(+), 357 deletions(-)
diff --git a/xlators/protocol/client/src/client-handshake.c b/xlators/protocol/client/src/client-handshake.c
index aee6b3a..7b36178 100644
--- a/xlators/protocol/client/src/client-handshake.c
+++ b/xlators/protocol/client/src/client-handshake.c
@@ -37,91 +37,10 @@ typedef struct client_fd_lk_local {
clnt_fd_ctx_t *fdctx;
}clnt_fd_lk_local_t;
-int
-client3_getspec_cbk (struct rpc_req *req, struct iovec *iov, int count,
- void *myframe)
-{
- gf_getspec_rsp rsp = {0,};
- call_frame_t *frame = NULL;
- int ret = 0;
-
- frame = myframe;
-
- if (!frame || !frame->this) {
- gf_msg (THIS->name, GF_LOG_ERROR, EINVAL, PC_MSG_INVALID_ENTRY,
- "frame not found with the request, returning EINVAL");
- rsp.op_ret = -1;
- rsp.op_errno = EINVAL;
- goto out;
- }
- if (-1 == req->rpc_status) {
- gf_msg (frame->this->name, GF_LOG_WARNING, ENOTCONN,
- PC_MSG_RPC_STATUS_ERROR, "received RPC status error, "
- "returning ENOTCONN");
- rsp.op_ret = -1;
- rsp.op_errno = ENOTCONN;
- goto out;
- }
-
- ret = xdr_to_generic (*iov, &rsp, (xdrproc_t)xdr_gf_getspec_rsp);
- if (ret < 0) {
- gf_msg (frame->this->name, GF_LOG_ERROR, EINVAL,
- PC_MSG_XDR_DECODING_FAILED,
- "XDR decoding failed, returning EINVAL");
- rsp.op_ret = -1;
- rsp.op_errno = EINVAL;
- goto out;
- }
-
- if (-1 == rsp.op_ret) {
- gf_msg (frame->this->name, GF_LOG_WARNING, 0,
- PC_MSG_VOL_FILE_NOT_FOUND, "failed to get the 'volume "
- "file' from server");
- goto out;
- }
-
-out:
- CLIENT_STACK_UNWIND (getspec, frame, rsp.op_ret, rsp.op_errno,
- rsp.spec);
-
- /* Don't use 'GF_FREE', this is allocated by libc */
- free (rsp.spec);
- free (rsp.xdata.xdata_val);
-
- return 0;
-}
-
int32_t client3_getspec (call_frame_t *frame, xlator_t *this, void *data)
{
- clnt_conf_t *conf = NULL;
- clnt_args_t *args = NULL;
- gf_getspec_req req = {0,};
- int op_errno = ESTALE;
- int ret = 0;
-
- if (!frame || !this || !data)
- goto unwind;
-
- args = data;
- conf = this->private;
- req.flags = args->flags;
- req.key = (char *)args->name;
-
- ret = client_submit_request (this, &req, frame, conf->handshake,
- GF_HNDSK_GETSPEC, client3_getspec_cbk,
- NULL, NULL, 0, NULL, 0, NULL,
- (xdrproc_t)xdr_gf_getspec_req);
-
- if (ret) {
- gf_msg (this->name, GF_LOG_WARNING, 0, PC_MSG_SEND_REQ_FAIL,
- "failed to send the request");
- }
-
- return 0;
-unwind:
- CLIENT_STACK_UNWIND (getspec, frame, -1, op_errno, NULL);
+ CLIENT_STACK_UNWIND (getspec, frame, -1, ENOSYS, NULL);
return 0;
-
}
int
diff --git a/xlators/protocol/server/src/server-handshake.c b/xlators/protocol/server/src/server-handshake.c
index 75577fa..217678a 100644
--- a/xlators/protocol/server/src/server-handshake.c
+++ b/xlators/protocol/server/src/server-handshake.c
@@ -38,204 +38,13 @@ gf_compare_client_version (rpcsvc_request_t *req, int fop_prognum,
}
int
-_volfile_update_checksum (xlator_t *this, char *key, uint32_t checksum)
-{
- server_conf_t *conf = NULL;
- struct _volfile_ctx *temp_volfile = NULL;
-
- conf = this->private;
- temp_volfile = conf->volfile;
-
- while (temp_volfile) {
- if ((NULL == key) && (NULL == temp_volfile->key))
- break;
- if ((NULL == key) || (NULL == temp_volfile->key)) {
- temp_volfile = temp_volfile->next;
- continue;
- }
- if (strcmp (temp_volfile->key, key) == 0)
- break;
- temp_volfile = temp_volfile->next;
- }
-
- if (!temp_volfile) {
- temp_volfile = GF_CALLOC (1, sizeof (struct _volfile_ctx),
- gf_server_mt_volfile_ctx_t);
- if (!temp_volfile)
- goto out;
- temp_volfile->next = conf->volfile;
- temp_volfile->key = (key)? gf_strdup (key): NULL;
- temp_volfile->checksum = checksum;
-
- conf->volfile = temp_volfile;
- goto out;
- }
-
- if (temp_volfile->checksum != checksum) {
- gf_msg (this->name, GF_LOG_INFO, 0, PS_MSG_REMOUNT_CLIENT_REQD,
- "the volume file was modified between a prior access "
- "and now. This may lead to inconsistency between "
- "clients, you are advised to remount client");
- temp_volfile->checksum = checksum;
- }
-
-out:
- return 0;
-}
-
-
-static size_t
-getspec_build_volfile_path (xlator_t *this, const char *key, char *path,
- size_t path_len)
-{
- char *filename = NULL;
- server_conf_t *conf = NULL;
- int ret = -1;
- int free_filename = 0;
- char data_key[256] = {0,};
-
- conf = this->private;
-
- /* Inform users that this option is changed now */
- ret = dict_get_str (this->options, "client-volume-filename",
- &filename);
- if (ret == 0) {
- gf_msg (this->name, GF_LOG_WARNING, 0, PS_MSG_DEFAULTING_FILE,
- "option 'client-volume-filename' is changed to "
- "'volume-filename.<key>' which now takes 'key' as an "
- "option to choose/fetch different files from server. "
- "Refer documentation or contact developers for more "
- "info. Currently defaulting to given file '%s'",
- filename);
- }
-
- if (key && !filename) {
- sprintf (data_key, "volume-filename.%s", key);
- ret = dict_get_str (this->options, data_key, &filename);
- if (ret < 0) {
- /* Make sure that key doesn't contain "../" in path */
- if ((gf_strstr (key, "/", "..")) == -1) {
- gf_msg (this->name, GF_LOG_ERROR, EINVAL,
- PS_MSG_INVALID_ENTRY, "%s: invalid "
- "key", key);
- goto out;
- }
- }
- }
-
- if (!filename) {
- ret = dict_get_str (this->options,
- "volume-filename.default", &filename);
- if (ret < 0) {
- gf_msg_debug (this->name, 0, "no default volume "
- "filename given, defaulting to %s",
- DEFAULT_VOLUME_FILE_PATH);
- }
- }
-
- if (!filename && key) {
- ret = gf_asprintf (&filename, "%s/%s.vol", conf->conf_dir, key);
- if (-1 == ret)
- goto out;
-
- free_filename = 1;
- }
- if (!filename)
- filename = DEFAULT_VOLUME_FILE_PATH;
-
- ret = -1;
-
- if ((filename) && (path_len > strlen (filename))) {
- strcpy (path, filename);
- ret = strlen (filename);
- }
-
-out:
- if (free_filename)
- GF_FREE (filename);
-
- return ret;
-}
-
-int
-_validate_volfile_checksum (xlator_t *this, char *key,
- uint32_t checksum)
-{
- char filename[PATH_MAX] = {0,};
- server_conf_t *conf = NULL;
- struct _volfile_ctx *temp_volfile = NULL;
- int ret = 0;
- int fd = 0;
- uint32_t local_checksum = 0;
-
- conf = this->private;
- temp_volfile = conf->volfile;
-
- if (!checksum)
- goto out;
-
- if (!temp_volfile) {
- ret = getspec_build_volfile_path (this, key, filename,
- sizeof (filename));
- if (ret <= 0)
- goto out;
- fd = open (filename, O_RDONLY);
- if (-1 == fd) {
- ret = 0;
- gf_msg (this->name, GF_LOG_INFO, errno,
- PS_MSG_VOL_FILE_OPEN_FAILED,
- "failed to open volume file (%s) : %s",
- filename, strerror (errno));
- goto out;
- }
- get_checksum_for_file (fd, &local_checksum);
- _volfile_update_checksum (this, key, local_checksum);
- sys_close (fd);
- }
-
- temp_volfile = conf->volfile;
- while (temp_volfile) {
- if ((NULL == key) && (NULL == temp_volfile->key))
- break;
- if ((NULL == key) || (NULL == temp_volfile->key)) {
- temp_volfile = temp_volfile->next;
- continue;
- }
- if (strcmp (temp_volfile->key, key) == 0)
- break;
- temp_volfile = temp_volfile->next;
- }
-
- if (!temp_volfile)
- goto out;
-
- if ((temp_volfile->checksum) &&
- (checksum != temp_volfile->checksum))
- ret = -1;
-
-out:
- return ret;
-}
-
-
-int
server_getspec (rpcsvc_request_t *req)
{
- int32_t ret = -1;
+ int ret = 0;
int32_t op_errno = ENOENT;
- int32_t spec_fd = -1;
- size_t file_len = 0;
- char filename[PATH_MAX] = {0,};
- struct stat stbuf = {0,};
- uint32_t checksum = 0;
- char *key = NULL;
- server_conf_t *conf = NULL;
- xlator_t *this = NULL;
gf_getspec_req args = {0,};
gf_getspec_rsp rsp = {0,};
- this = req->svc->xl;
- conf = this->private;
ret = xdr_to_generic (req->msg[0], &args,
(xdrproc_t)xdr_gf_getspec_req);
if (ret < 0) {
@@ -245,58 +54,11 @@ server_getspec (rpcsvc_request_t *req)
goto fail;
}
- ret = getspec_build_volfile_path (this, args.key,
- filename, sizeof (filename));
- if (ret > 0) {
- /* to allocate the proper buffer to hold the file data */
- ret = sys_stat (filename, &stbuf);
- if (ret < 0){
- gf_msg (this->name, GF_LOG_ERROR, errno,
- PS_MSG_STAT_ERROR, "Unable to stat %s (%s)",
- filename, strerror (errno));
- op_errno = errno;
- goto fail;
- }
-
- spec_fd = open (filename, O_RDONLY);
- if (spec_fd < 0) {
- gf_msg (this->name, GF_LOG_ERROR, errno,
- PS_MSG_FILE_OP_FAILED, "Unable to open %s "
- "(%s)", filename, strerror (errno));
- op_errno = errno;
- goto fail;
- }
- ret = file_len = stbuf.st_size;
-
- if (conf->verify_volfile) {
- get_checksum_for_file (spec_fd, &checksum);
- _volfile_update_checksum (this, key, checksum);
- }
- } else {
- op_errno = ENOENT;
- }
-
- if (file_len) {
- rsp.spec = GF_CALLOC (file_len, sizeof (char),
- gf_server_mt_rsp_buf_t);
- if (!rsp.spec) {
- ret = -1;
- op_errno = ENOMEM;
- goto fail;
- }
- ret = sys_read (spec_fd, rsp.spec, file_len);
- }
-
- /* convert to XDR */
- op_errno = errno;
+ op_errno = ENOSYS;
fail:
- if (!rsp.spec)
- rsp.spec = "";
+ rsp.spec = "<this method is not in use, use glusterd for getspec>";
rsp.op_errno = gf_errno_to_error (op_errno);
- rsp.op_ret = ret;
-
- if (spec_fd != -1)
- sys_close (spec_fd);
+ rsp.op_ret = -1;
server_submit_reply (NULL, req, &rsp, NULL, 0, NULL,
(xdrproc_t)xdr_gf_getspec_rsp);
@@ -459,9 +221,7 @@ server_setvolume (rpcsvc_request_t *req)
char *clnt_version = NULL;
xlator_t *xl = NULL;
char *msg = NULL;
- char *volfile_key = NULL;
xlator_t *this = NULL;
- uint32_t checksum = 0;
int32_t ret = -1;
int32_t op_ret = -1;
int32_t op_errno = EINVAL;
@@ -756,34 +516,6 @@ server_setvolume (rpcsvc_request_t *req)
goto fail;
}
- if (conf->verify_volfile) {
- ret = dict_get_uint32 (params, "volfile-checksum", &checksum);
- if (ret == 0) {
- ret = dict_get_str (params, "volfile-key",
- &volfile_key);
- if (ret)
- gf_msg_debug (this->name, 0, "failed to get "
- "'volfile-key'");
-
- ret = _validate_volfile_checksum (this, volfile_key,
- checksum);
- if (-1 == ret) {
- ret = dict_set_str (reply, "ERROR",
- "volume-file checksum "
- "varies from earlier "
- "access");
- if (ret < 0)
- gf_msg_debug (this->name, 0, "failed "
- "to set error msg");
-
- op_ret = -1;
- op_errno = ESTALE;
- goto fail;
- }
- }
- }
-
-
peerinfo = &req->trans->peerinfo;
if (peerinfo) {
ret = dict_set_static_ptr (params, "peer-info", peerinfo);
diff --git a/xlators/protocol/server/src/server.c b/xlators/protocol/server/src/server.c
index 11ee7ba..d0e815e 100644
--- a/xlators/protocol/server/src/server.c
+++ b/xlators/protocol/server/src/server.c
@@ -1797,9 +1797,6 @@ struct volume_options options[] = {
.description = "Specifies the limit on the number of inodes "
"in the lru list of the inode cache."
},
- { .key = {"verify-volfile-checksum"},
- .type = GF_OPTION_TYPE_BOOL
- },
{ .key = {"trace"},
.type = GF_OPTION_TYPE_BOOL
},
--
1.8.3.1