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