From 00c78b9eb52d8a631cdaef883cd507bd0889639a Mon Sep 17 00:00:00 2001 From: Amar Tumballi 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 Reviewed-on: https://code.engineering.redhat.com/gerrit/151322 Reviewed-by: Pranith Kumar Karampuri Reviewed-by: Sunil Kumar Heggodu Gopala Acharya --- 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.' 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 = ""; 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