From 6756245b1514374712136c8dc1f4ac2a19a6da21 Mon Sep 17 00:00:00 2001 From: Rajesh Amaravathi Date: Wed, 29 Feb 2012 15:16:53 +0530 Subject: fops/removexattr: prevent users from removing glusterfs xattrs * Each xlator prevents the user from removing xlator-specific xattrs like trusted.gfid by handling it in respective removexattr functions. * For xlators which did not define remove and fremovexattr, the functions have been implemented with appropriate checks. xlator | fops-added _______________|__________________________ | 1. stripe | removexattr and fremovexattr 2. quota | removexattr and fremovexattr Change-Id: I98e22109717978134378bc75b2eca83fefb2abba BUG: 783525 Signed-off-by: Rajesh Amaravathi Reviewed-on: http://review.gluster.com/2836 Tested-by: Gluster Build System Reviewed-by: Amar Tumballi Reviewed-by: Jeff Darcy --- cli/src/registry.c | 2 +- libglusterfs/src/common-utils.h | 47 +++++++--- xlators/cluster/afr/src/afr-inode-write.c | 33 +++++-- xlators/cluster/afr/src/pump.c | 14 ++- xlators/cluster/dht/src/dht-common.c | 12 ++- xlators/cluster/stripe/src/stripe.c | 128 ++++++++++++++++++++------- xlators/features/quota/src/quota.c | 115 +++++++++++++++++++----- xlators/mgmt/glusterd/src/glusterd-geo-rep.c | 2 +- xlators/mgmt/glusterd/src/glusterd-handler.c | 2 +- xlators/mount/fuse/src/fuse-bridge.c | 6 ++ xlators/protocol/client/src/client.c | 2 +- 11 files changed, 279 insertions(+), 84 deletions(-) diff --git a/cli/src/registry.c b/cli/src/registry.c index 5b63e82b1..fc3b2decd 100644 --- a/cli/src/registry.c +++ b/cli/src/registry.c @@ -387,7 +387,7 @@ cli_cmd_register (struct cli_cmd_tree *tree, struct cli_cmd *cmd) char **tokens = NULL; int ret = 0; - GF_ASSERT (cmd) + GF_ASSERT (cmd); if (cmd->reg_cbk) cmd->reg_cbk (cmd); diff --git a/libglusterfs/src/common-utils.h b/libglusterfs/src/common-utils.h index 67ad5ac8e..8c9b26f76 100644 --- a/libglusterfs/src/common-utils.h +++ b/libglusterfs/src/common-utils.h @@ -128,7 +128,7 @@ extern char *gf_mgmt_list[GF_MGMT_MAXVALUE]; "invalid argument: " #arg); \ goto label; \ } \ - } while (0); + } while (0) #define GF_VALIDATE_OR_GOTO(name,arg,label) do { \ if (!arg) { \ @@ -137,7 +137,7 @@ extern char *gf_mgmt_list[GF_MGMT_MAXVALUE]; "invalid argument: " #arg); \ goto label; \ } \ - } while (0); + } while (0) #define GF_VALIDATE_OR_GOTO_WITH_ERROR(name, arg, label, errno, error) do { \ if (!arg) { \ @@ -146,15 +146,15 @@ extern char *gf_mgmt_list[GF_MGMT_MAXVALUE]; "invalid argument: " #arg); \ goto label; \ } \ - }while (0); + }while (0) #define GF_ASSERT_AND_GOTO_WITH_ERROR(name, arg, label, errno, error) do { \ if (!arg) { \ - GF_ASSERT (0) \ + GF_ASSERT (0); \ errno = error; \ goto label; \ } \ - }while (0); + }while (0) #define GF_VALIDATE_ABSOLUTE_PATH_OR_GOTO(name,arg,label) \ do { \ @@ -165,7 +165,7 @@ extern char *gf_mgmt_list[GF_MGMT_MAXVALUE]; "invalid argument: " #arg); \ goto label; \ } \ - } while (0); + } while (0) #define GF_REMOVE_SLASH_FROM_PATH(path, string) \ do { \ @@ -175,28 +175,47 @@ extern char *gf_mgmt_list[GF_MGMT_MAXVALUE]; if (string[i-1] == '/') \ string[i-1] = '-'; \ } \ - } while (0); \ + } while (0) #define GF_IF_INTERNAL_XATTR_GOTO(pattern, dict, trav, op_errno, label) \ - do{ \ + do { \ if (!dict) { \ - gf_log (THIS->name, GF_LOG_ERROR, \ + gf_log (this->name, GF_LOG_ERROR, \ "setxattr dict is null"); \ goto label; \ } \ trav = dict->members_list; \ while (trav) { \ if (!fnmatch (pattern, trav->key, 0)) { \ - gf_log (THIS->name, GF_LOG_ERROR, \ - "attempt to set internal" \ - " xattr: %s", trav->key); \ op_errno = EPERM; \ + gf_log (this->name, GF_LOG_ERROR, \ + "attempt to set internal" \ + " xattr: %s: %s", trav->key, \ + strerror (op_errno)); \ goto label; \ } \ trav = trav->next; \ } \ - } while(0); \ + } while (0) + +#define GF_IF_NATIVE_XATTR_GOTO(pattern, key, op_errno, label) \ + do { \ + if (!key) { \ + gf_log (this->name, GF_LOG_ERROR, \ + "no key for removexattr"); \ + goto label; \ + } \ + if (!fnmatch (pattern, key, 0)) { \ + op_errno = EPERM; \ + gf_log (this->name, GF_LOG_ERROR, \ + "attempt to remove internal " \ + "xattr: %s: %s", key, \ + strerror (op_errno)); \ + goto label; \ + } \ + } while (0) + #define GF_FILE_CONTENT_REQUESTED(_xattr_req,_content_limit) \ (dict_get_uint64 (_xattr_req, "glusterfs.content", _content_limit) == 0) @@ -210,7 +229,7 @@ extern char *gf_mgmt_list[GF_MGMT_MAXVALUE]; gf_log_callingfn ("", GF_LOG_ERROR, \ "Assertion failed: " #x); \ } \ - } while (0); + } while (0) #endif #define GF_UUID_ASSERT(u) \ diff --git a/xlators/cluster/afr/src/afr-inode-write.c b/xlators/cluster/afr/src/afr-inode-write.c index 3dc1a4186..1d25949db 100644 --- a/xlators/cluster/afr/src/afr-inode-write.c +++ b/xlators/cluster/afr/src/afr-inode-write.c @@ -1474,9 +1474,7 @@ afr_setxattr (call_frame_t *frame, xlator_t *this, int ret = -1; int op_errno = EINVAL; - VALIDATE_OR_GOTO (frame, out); VALIDATE_OR_GOTO (this, out); - VALIDATE_OR_GOTO (this->private, out); GF_IF_INTERNAL_XATTR_GOTO ("trusted.afr.*", dict, trav, op_errno, out); @@ -1484,6 +1482,9 @@ afr_setxattr (call_frame_t *frame, xlator_t *this, GF_IF_INTERNAL_XATTR_GOTO ("trusted.glusterfs.afr.*", dict, trav, op_errno, out); + VALIDATE_OR_GOTO (frame, out); + VALIDATE_OR_GOTO (this->private, out); + priv = this->private; QUORUM_CHECK(setxattr,out); @@ -1844,14 +1845,21 @@ int afr_removexattr (call_frame_t *frame, xlator_t *this, loc_t *loc, const char *name) { - afr_private_t * priv = NULL; - afr_local_t * local = NULL; - call_frame_t *transaction_frame = NULL; - int ret = -1; - int op_errno = 0; + afr_private_t *priv = NULL; + afr_local_t *local = NULL; + call_frame_t *transaction_frame = NULL; + int ret = -1; + int op_errno = 0; - VALIDATE_OR_GOTO (frame, out); VALIDATE_OR_GOTO (this, out); + + GF_IF_NATIVE_XATTR_GOTO ("trusted.afr.*", + name, op_errno, out); + + GF_IF_NATIVE_XATTR_GOTO ("trusted.glusterfs.afr.*", + name, op_errno, out); + + VALIDATE_OR_GOTO (frame, out); VALIDATE_OR_GOTO (this->private, out); VALIDATE_OR_GOTO (loc, out); @@ -2027,8 +2035,15 @@ afr_fremovexattr (call_frame_t *frame, xlator_t *this, int op_ret = -1; int op_errno = 0; - VALIDATE_OR_GOTO (frame, out); VALIDATE_OR_GOTO (this, out); + + GF_IF_NATIVE_XATTR_GOTO ("trusted.afr.*", + name, op_errno, out); + + GF_IF_NATIVE_XATTR_GOTO ("trusted.glusterfs.afr.*", + name, op_errno, out); + + VALIDATE_OR_GOTO (frame, out); VALIDATE_OR_GOTO (this->private, out); priv = this->private; diff --git a/xlators/cluster/afr/src/pump.c b/xlators/cluster/afr/src/pump.c index e795c38e0..9c6f9a52a 100644 --- a/xlators/cluster/afr/src/pump.c +++ b/xlators/cluster/afr/src/pump.c @@ -2165,7 +2165,15 @@ pump_removexattr (call_frame_t *frame, loc_t *loc, const char *name) { - afr_private_t *priv = NULL; + afr_private_t *priv = NULL; + int op_errno = -1; + + VALIDATE_OR_GOTO (this, out); + + GF_IF_NATIVE_XATTR_GOTO ("trusted.glusterfs.pump*", + name, op_errno, out); + + op_errno = 0; priv = this->private; if (!priv->use_afr_in_pump) { STACK_WIND (frame, @@ -2177,6 +2185,10 @@ pump_removexattr (call_frame_t *frame, return 0; } afr_removexattr (frame, this, loc, name); + + out: + if (op_errno) + AFR_STACK_UNWIND (removexattr, frame, -1, op_errno); return 0; } diff --git a/xlators/cluster/dht/src/dht-common.c b/xlators/cluster/dht/src/dht-common.c index 0d0df320d..d76abdd59 100644 --- a/xlators/cluster/dht/src/dht-common.c +++ b/xlators/cluster/dht/src/dht-common.c @@ -2385,8 +2385,12 @@ dht_removexattr (call_frame_t *frame, xlator_t *this, int i; - VALIDATE_OR_GOTO (frame, err); VALIDATE_OR_GOTO (this, err); + + GF_IF_NATIVE_XATTR_GOTO ("trusted.glusterfs.dht*", + key, op_errno, err); + + VALIDATE_OR_GOTO (frame, err); VALIDATE_OR_GOTO (loc, err); VALIDATE_OR_GOTO (loc->inode, err); VALIDATE_OR_GOTO (loc->path, err); @@ -2444,9 +2448,13 @@ dht_fremovexattr (call_frame_t *frame, xlator_t *this, int i; - VALIDATE_OR_GOTO (frame, err); VALIDATE_OR_GOTO (this, err); + GF_IF_NATIVE_XATTR_GOTO ("trusted.glusterfs.dht*", + key, op_errno, err); + + VALIDATE_OR_GOTO (frame, err); + local = dht_local_init (frame, NULL, fd, GF_FOP_FREMOVEXATTR); if (!local) { op_errno = ENOMEM; diff --git a/xlators/cluster/stripe/src/stripe.c b/xlators/cluster/stripe/src/stripe.c index 227af8c9e..5b1610724 100644 --- a/xlators/cluster/stripe/src/stripe.c +++ b/xlators/cluster/stripe/src/stripe.c @@ -4042,10 +4042,76 @@ stripe_fsetxattr (call_frame_t *frame, xlator_t *this, fd_t *fd, return 0; } +int +stripe_removexattr_cbk (call_frame_t *frame, void *cookie, + xlator_t *this, int32_t op_ret, int32_t op_errno) +{ + STRIPE_STACK_UNWIND (removexattr, frame, op_ret, op_errno); + return 0; +} + +int +stripe_removexattr (call_frame_t *frame, xlator_t *this, + loc_t *loc, const char *name) +{ + int32_t op_errno = EINVAL; + + VALIDATE_OR_GOTO (this, err); + + GF_IF_NATIVE_XATTR_GOTO ("trusted.*stripe*", + name, op_errno, err); + + VALIDATE_OR_GOTO (frame, err); + VALIDATE_OR_GOTO (loc, err); + + STACK_WIND (frame, stripe_removexattr_cbk, + FIRST_CHILD(this), + FIRST_CHILD(this)->fops->removexattr, + loc, name); + return 0; +err: + STRIPE_STACK_UNWIND (removexattr, frame, -1, op_errno); + return 0; +} + + +int +stripe_fremovexattr_cbk (call_frame_t *frame, void *cookie, + xlator_t *this, int32_t op_ret, int32_t op_errno) +{ + STRIPE_STACK_UNWIND (fremovexattr, frame, op_ret, op_errno); + return 0; +} + +int +stripe_fremovexattr (call_frame_t *frame, xlator_t *this, + fd_t *fd, const char *name) +{ + int32_t op_ret = -1; + int32_t op_errno = EINVAL; + + VALIDATE_OR_GOTO (frame, err); + VALIDATE_OR_GOTO (this, err); + VALIDATE_OR_GOTO (fd, err); + + GF_IF_NATIVE_XATTR_GOTO ("trusted.*stripe*", + name, op_errno, err); + + STACK_WIND (frame, stripe_fremovexattr_cbk, + FIRST_CHILD(this), + FIRST_CHILD(this)->fops->fremovexattr, + fd, name); + return 0; + err: + STRIPE_STACK_UNWIND (fremovexattr, frame, op_ret, op_errno); + return 0; +} + int32_t -stripe_readdirp_lookup_cbk (call_frame_t *frame, void *cookie, xlator_t *this, - int op_ret, int op_errno, inode_t *inode, - struct iatt *stbuf, dict_t *xattr, struct iatt *parent) +stripe_readdirp_lookup_cbk (call_frame_t *frame, void *cookie, + xlator_t *this, int op_ret, int op_errno, + inode_t *inode, struct iatt *stbuf, + dict_t *xattr, struct iatt *parent) { stripe_local_t *local = NULL; call_frame_t *main_frame = NULL; @@ -4961,33 +5027,35 @@ out: } struct xlator_fops fops = { - .stat = stripe_stat, - .unlink = stripe_unlink, - .rename = stripe_rename, - .link = stripe_link, - .truncate = stripe_truncate, - .create = stripe_create, - .open = stripe_open, - .readv = stripe_readv, - .writev = stripe_writev, - .statfs = stripe_statfs, - .flush = stripe_flush, - .fsync = stripe_fsync, - .ftruncate = stripe_ftruncate, - .fstat = stripe_fstat, - .mkdir = stripe_mkdir, - .rmdir = stripe_rmdir, - .lk = stripe_lk, - .opendir = stripe_opendir, - .fsyncdir = stripe_fsyncdir, - .setattr = stripe_setattr, - .fsetattr = stripe_fsetattr, - .lookup = stripe_lookup, - .mknod = stripe_mknod, - .setxattr = stripe_setxattr, - .fsetxattr = stripe_fsetxattr, - .getxattr = stripe_getxattr, - .readdirp = stripe_readdirp, + .stat = stripe_stat, + .unlink = stripe_unlink, + .rename = stripe_rename, + .link = stripe_link, + .truncate = stripe_truncate, + .create = stripe_create, + .open = stripe_open, + .readv = stripe_readv, + .writev = stripe_writev, + .statfs = stripe_statfs, + .flush = stripe_flush, + .fsync = stripe_fsync, + .ftruncate = stripe_ftruncate, + .fstat = stripe_fstat, + .mkdir = stripe_mkdir, + .rmdir = stripe_rmdir, + .lk = stripe_lk, + .opendir = stripe_opendir, + .fsyncdir = stripe_fsyncdir, + .setattr = stripe_setattr, + .fsetattr = stripe_fsetattr, + .lookup = stripe_lookup, + .mknod = stripe_mknod, + .setxattr = stripe_setxattr, + .fsetxattr = stripe_fsetxattr, + .getxattr = stripe_getxattr, + .removexattr = stripe_removexattr, + .fremovexattr = stripe_fremovexattr, + .readdirp = stripe_readdirp, }; struct xlator_cbks cbks = { diff --git a/xlators/features/quota/src/quota.c b/xlators/features/quota/src/quota.c index d819ea784..844a0981d 100644 --- a/xlators/features/quota/src/quota.c +++ b/xlators/features/quota/src/quota.c @@ -2721,6 +2721,71 @@ quota_fsetxattr (call_frame_t *frame, xlator_t *this, fd_t *fd, } +int +quota_removexattr_cbk (call_frame_t *frame, void *cookie, + xlator_t *this, int32_t op_ret, int32_t op_errno) +{ + QUOTA_STACK_UNWIND (removexattr, frame, op_ret, op_errno); + return 0; +} + +int +quota_removexattr (call_frame_t *frame, xlator_t *this, + loc_t *loc, const char *name) +{ + int32_t op_errno = EINVAL; + + VALIDATE_OR_GOTO (this, err); + + GF_IF_NATIVE_XATTR_GOTO ("trusted.quota*", + name, op_errno, err); + + VALIDATE_OR_GOTO (frame, err); + VALIDATE_OR_GOTO (loc, err); + + STACK_WIND (frame, quota_removexattr_cbk, + FIRST_CHILD(this), + FIRST_CHILD(this)->fops->removexattr, + loc, name); + return 0; +err: + QUOTA_STACK_UNWIND (removexattr, frame, -1, op_errno); + return 0; +} + + +int +quota_fremovexattr_cbk (call_frame_t *frame, void *cookie, + xlator_t *this, int32_t op_ret, int32_t op_errno) +{ + QUOTA_STACK_UNWIND (fremovexattr, frame, op_ret, op_errno); + return 0; +} + +int +quota_fremovexattr (call_frame_t *frame, xlator_t *this, + fd_t *fd, const char *name) +{ + int32_t op_ret = -1; + int32_t op_errno = EINVAL; + + VALIDATE_OR_GOTO (frame, err); + VALIDATE_OR_GOTO (this, err); + VALIDATE_OR_GOTO (fd, err); + + GF_IF_NATIVE_XATTR_GOTO ("trusted.quota*", + name, op_errno, err); + + STACK_WIND (frame, quota_fremovexattr_cbk, + FIRST_CHILD(this), + FIRST_CHILD(this)->fops->fremovexattr, + fd, name); + return 0; + err: + QUOTA_STACK_UNWIND (fremovexattr, frame, op_ret, op_errno); + return 0; +} + int32_t quota_statfs_cbk (call_frame_t *frame, void *cookie, xlator_t *this, int32_t op_ret, int32_t op_errno, struct statvfs *buf) @@ -3054,30 +3119,32 @@ fini (xlator_t *this) struct xlator_fops fops = { - .statfs = quota_statfs, - .lookup = quota_lookup, - .writev = quota_writev, - .create = quota_create, - .mkdir = quota_mkdir, - .truncate = quota_truncate, - .ftruncate = quota_ftruncate, - .unlink = quota_unlink, - .symlink = quota_symlink, - .link = quota_link, - .rename = quota_rename, - .getxattr = quota_getxattr, - .fgetxattr = quota_fgetxattr, - .stat = quota_stat, - .fstat = quota_fstat, - .readlink = quota_readlink, - .readv = quota_readv, - .fsync = quota_fsync, - .setattr = quota_setattr, - .fsetattr = quota_fsetattr, - .mknod = quota_mknod, - .setxattr = quota_setxattr, - .fsetxattr = quota_fsetxattr, - .readdirp = quota_readdirp, + .statfs = quota_statfs, + .lookup = quota_lookup, + .writev = quota_writev, + .create = quota_create, + .mkdir = quota_mkdir, + .truncate = quota_truncate, + .ftruncate = quota_ftruncate, + .unlink = quota_unlink, + .symlink = quota_symlink, + .link = quota_link, + .rename = quota_rename, + .getxattr = quota_getxattr, + .fgetxattr = quota_fgetxattr, + .stat = quota_stat, + .fstat = quota_fstat, + .readlink = quota_readlink, + .readv = quota_readv, + .fsync = quota_fsync, + .setattr = quota_setattr, + .fsetattr = quota_fsetattr, + .mknod = quota_mknod, + .setxattr = quota_setxattr, + .fsetxattr = quota_fsetxattr, + .removexattr = quota_removexattr, + .fremovexattr = quota_fremovexattr, + .readdirp = quota_readdirp, }; struct xlator_cbks cbks = { diff --git a/xlators/mgmt/glusterd/src/glusterd-geo-rep.c b/xlators/mgmt/glusterd/src/glusterd-geo-rep.c index 3a62899f8..5e12e1996 100644 --- a/xlators/mgmt/glusterd/src/glusterd-geo-rep.c +++ b/xlators/mgmt/glusterd/src/glusterd-geo-rep.c @@ -804,7 +804,7 @@ glusterd_store_slave_in_info (glusterd_volinfo_t *volinfo, char *slave, * assert an uuid mismatch */ t = strtail (slaveentry, host_uuid); - GF_ASSERT (!t || *t != ':') + GF_ASSERT (!t || *t != ':'); gf_log ("", GF_LOG_ERROR, GEOREP" has already been invoked for " "the %s (master) and %s (slave) " diff --git a/xlators/mgmt/glusterd/src/glusterd-handler.c b/xlators/mgmt/glusterd/src/glusterd-handler.c index b87c2cd8e..f446d2b58 100644 --- a/xlators/mgmt/glusterd/src/glusterd-handler.c +++ b/xlators/mgmt/glusterd/src/glusterd-handler.c @@ -2135,7 +2135,7 @@ glusterd_friend_add (const char *hoststr, int port, gf_boolean_t handover = _gf_false; conf = THIS->private; - GF_ASSERT (conf) + GF_ASSERT (conf); GF_ASSERT (hoststr); peerctx = GF_CALLOC (1, sizeof (*peerctx), gf_gld_mt_peerctx_t); diff --git a/xlators/mount/fuse/src/fuse-bridge.c b/xlators/mount/fuse/src/fuse-bridge.c index b50598788..3f3962cbc 100644 --- a/xlators/mount/fuse/src/fuse-bridge.c +++ b/xlators/mount/fuse/src/fuse-bridge.c @@ -3009,6 +3009,12 @@ fuse_removexattr (xlator_t *this, fuse_in_header_t *finh, void *msg) int32_t ret = -1; char *newkey = NULL; + if (!strcmp (GFID_XATTR_KEY, name)) { + send_fuse_err (this, finh, EPERM); + GF_FREE (finh); + return; + } + priv = this->private; GET_STATE (this, finh, state); diff --git a/xlators/protocol/client/src/client.c b/xlators/protocol/client/src/client.c index 8fadc0400..19c8a62ee 100644 --- a/xlators/protocol/client/src/client.c +++ b/xlators/protocol/client/src/client.c @@ -45,7 +45,7 @@ int client_mark_fd_bad (xlator_t *this); int32_t client_type_to_gf_type (short l_type) { - int32_t gf_type; + int32_t gf_type = GF_LK_EOL; switch (l_type) { case F_RDLCK: -- cgit