From e2712fbd38477e736f157c9dbfbbae9c253b6c13 Mon Sep 17 00:00:00 2001 From: Amar Tumballi Date: Thu, 1 Nov 2018 07:16:32 +0530 Subject: 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. Updates: CVE-2018-14653 Updates: bz#1644756 Change-Id: I4a2e0f673c5bcd4644976a61dbd2d37003a428eb Signed-off-by: Amar Tumballi --- xlators/protocol/client/src/client-handshake.c | 85 +------- xlators/protocol/server/src/server-handshake.c | 273 +------------------------ xlators/protocol/server/src/server.c | 1 - 3 files changed, 5 insertions(+), 354 deletions(-) diff --git a/xlators/protocol/client/src/client-handshake.c b/xlators/protocol/client/src/client-handshake.c index 5feeca5411d..ed9d0a5d9d8 100644 --- a/xlators/protocol/client/src/client-handshake.c +++ b/xlators/protocol/client/src/client-handshake.c @@ -34,94 +34,11 @@ 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; } diff --git a/xlators/protocol/server/src/server-handshake.c b/xlators/protocol/server/src/server-handshake.c index 5f6b96792fa..f2838832b59 100644 --- a/xlators/protocol/server/src/server-handshake.c +++ b/xlators/protocol/server/src/server-handshake.c @@ -36,202 +36,12 @@ gf_compare_client_version(rpcsvc_request_t *req, int fop_prognum, return ret; } -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; + int32_t 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, }; @@ -239,8 +49,6 @@ server_getspec(rpcsvc_request_t *req) 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) { // failed to decode msg; @@ -249,55 +57,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); - } - } - - 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); @@ -472,9 +236,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; @@ -742,33 +504,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 cf8e40eaad0..4ec6fe8049a 100644 --- a/xlators/protocol/server/src/server.c +++ b/xlators/protocol/server/src/server.c @@ -1696,7 +1696,6 @@ struct volume_options server_options[] = { "in the lru list of the inode cache.", .op_version = {1}, .flags = OPT_FLAG_SETTABLE | OPT_FLAG_DOC}, - {.key = {"verify-volfile-checksum"}, .type = GF_OPTION_TYPE_BOOL}, {.key = {"trace"}, .type = GF_OPTION_TYPE_BOOL}, { .key = {"config-directory", "conf-dir"}, -- cgit