From 81cb71e9317e380b1d414038223c72643b35e664 Mon Sep 17 00:00:00 2001 From: Jiffin Tony Thottan Date: Wed, 10 Jun 2015 00:08:39 +0530 Subject: access-control : validating context of access control translator By introduction of new acl conversion from http://review.gluster.org/#/c/9627/, an acl can be set using GF_POSIX_ACL_*_KEY xattrs without notifying the access-control translator. So evenif an acl is set correctly at the backend, it might not work properly because access-control holds wrong acl information in its context about that file. Note : This is a simple workaround. The actual solution consists of three steps: 1.) Use new acl api's for acl conversion. 2.) Move the acl conversion part from access-control translator 3.) Introduces standard acl structures and libaries in access-translator for caching, enforcing purposes. Change-Id: Iacb6b323810ebe82f7f171f20be16429463cbcf0 BUG: 1229860 Signed-off-by: Jiffin Tony Thottan Reviewed-on: http://review.gluster.org/11144 Reviewed-by: Niels de Vos Tested-by: Gluster Build System Reviewed-by: Kaleb KEITHLEY --- xlators/storage/posix/src/posix-messages.h | 8 ++ xlators/storage/posix/src/posix.c | 103 +++++++++++++++++++++++- xlators/storage/posix/src/posix.h | 2 + xlators/system/posix-acl/src/posix-acl.c | 121 ++++++++++++++++++++++++++++- 4 files changed, 227 insertions(+), 7 deletions(-) (limited to 'xlators') diff --git a/xlators/storage/posix/src/posix-messages.h b/xlators/storage/posix/src/posix-messages.h index 9916aed92cd..9f267dfe0d9 100644 --- a/xlators/storage/posix/src/posix-messages.h +++ b/xlators/storage/posix/src/posix-messages.h @@ -893,6 +893,14 @@ * */ +#define P_MSG_BUFFER_OVERFLOW (POSIX_COMP_BASE + 105) +/*! + * @messageid + * @diagnosis + * @recommendedaction + * + */ + /*------------*/ #define glfs_msg_end_x GLFS_MSGID_END, "Invalid: End of messages" diff --git a/xlators/storage/posix/src/posix.c b/xlators/storage/posix/src/posix.c index 5e9dbf7ee5c..36d2c857835 100644 --- a/xlators/storage/posix/src/posix.c +++ b/xlators/storage/posix/src/posix.c @@ -3266,8 +3266,10 @@ posix_setxattr (call_frame_t *frame, xlator_t *this, int32_t op_ret = -1; int32_t op_errno = 0; char * real_path = NULL; + char *acl_xattr = NULL; struct iatt stbuf = {0}; int32_t ret = 0; + size_t acl_size = 0; dict_t *xattr = NULL; posix_xattr_filler_t filler = {0,}; @@ -3306,6 +3308,10 @@ posix_setxattr (call_frame_t *frame, xlator_t *this, goto out; } + xattr = dict_new(); + if (!xattr) + goto out; + /* * FIXFIX: Send the stbuf info in the xdata for now * This is used by DHT to redirect FOPs if the file is being migrated @@ -3316,10 +3322,98 @@ posix_setxattr (call_frame_t *frame, xlator_t *this, if (ret) goto out; - xattr = dict_new(); - if (!xattr) + ret = posix_set_iatt_in_dict (xattr, &stbuf); + } + +/* + * ACL can be set on a file/folder using GF_POSIX_ACL_*_KEY xattrs which + * won't aware of access-control xlator. To update its context correctly, + * POSIX_ACL_*_XATTR stored in xdata which is send in the call_back path. + */ + if (dict_get (dict, GF_POSIX_ACL_ACCESS)) { + + /* + * The size of buffer will be know after calling sys_lgetxattr, + * so first we allocate buffer with large size(~4k), then we + * reduced into required size using GF_REALLO(). + */ + acl_xattr = GF_CALLOC (1, ACL_BUFFER_MAX, gf_posix_mt_char); + if (!acl_xattr) { + ret = -1; goto out; - ret = posix_set_iatt_in_dict (xattr, &stbuf); + } + + acl_size = sys_lgetxattr (real_path, POSIX_ACL_ACCESS_XATTR, + acl_xattr, ACL_BUFFER_MAX); + + if (acl_size < 0) { + gf_msg (this->name, GF_LOG_WARNING, errno, + P_MSG_XATTR_FAILED, "Posix acl is not set " + "properly at the backend"); + ret = -1; + goto out; + } + + /* If acl_size is more than max buffer size, just ignore it */ + if (acl_size >= ACL_BUFFER_MAX) { + gf_msg (this->name, GF_LOG_WARNING, ENOMEM, + P_MSG_BUFFER_OVERFLOW, "size of acl is more" + "than the buffer"); + ret = -1; + goto out; + } + + acl_xattr = GF_REALLOC (acl_xattr, acl_size); + ret = dict_set_bin (xattr, POSIX_ACL_ACCESS_XATTR, + acl_xattr, acl_size); + if (ret) { + gf_msg (this->name, GF_LOG_WARNING, 0, + P_MSG_SET_XDATA_FAIL, "failed to set" + "xdata for acl"); + ret = -1; + goto out; + } + } + + GF_FREE (acl_xattr); + acl_xattr = NULL; + + if (dict_get (dict, GF_POSIX_ACL_DEFAULT)) { + + acl_xattr = GF_CALLOC (1, ACL_BUFFER_MAX, gf_posix_mt_char); + if (!acl_xattr) { + ret = -1; + goto out; + } + acl_size = sys_lgetxattr (real_path, POSIX_ACL_DEFAULT_XATTR, + acl_xattr, ACL_BUFFER_MAX); + + if (acl_size < 0) { + gf_msg (this->name, GF_LOG_WARNING, errno, + P_MSG_XATTR_FAILED, "Posix acl is not set " + "properly at the backend"); + ret = -1; + goto out; + } + + if (acl_size >= ACL_BUFFER_MAX) { + gf_msg (this->name, GF_LOG_WARNING, ENOMEM, + P_MSG_BUFFER_OVERFLOW, "size of acl is more" + "than the buffer"); + ret = -1; + goto out; + } + + acl_xattr = GF_REALLOC (acl_xattr, acl_size); + ret = dict_set_bin (xattr, POSIX_ACL_DEFAULT_XATTR, + acl_xattr, acl_size); + if (ret) { + gf_msg (this->name, GF_LOG_WARNING, 0, + P_MSG_SET_XDATA_FAIL, "failed to set" + "xdata for acl"); + ret = -1; + goto out; + } } out: SET_TO_OLD_FS_ID (); @@ -3329,6 +3423,9 @@ out: if (xattr) dict_unref(xattr); + if (acl_xattr) + GF_FREE (acl_xattr); + return 0; } diff --git a/xlators/storage/posix/src/posix.h b/xlators/storage/posix/src/posix.h index b85338e930e..cc7ed0402fe 100644 --- a/xlators/storage/posix/src/posix.h +++ b/xlators/storage/posix/src/posix.h @@ -48,6 +48,8 @@ #define VECTOR_SIZE 64 * 1024 /* vector size 64KB*/ #define MAX_NO_VECT 1024 +#define ACL_BUFFER_MAX 4096 /* size of character buffer */ + #define LINKTO "trusted.glusterfs.dht.linkto" #define POSIX_GFID_HANDLE_SIZE(base_path_len) (base_path_len + SLEN("/") \ diff --git a/xlators/system/posix-acl/src/posix-acl.c b/xlators/system/posix-acl/src/posix-acl.c index 05608696ea6..da2ccbd1c54 100644 --- a/xlators/system/posix-acl/src/posix-acl.c +++ b/xlators/system/posix-acl/src/posix-acl.c @@ -306,6 +306,26 @@ posix_acl_ctx_get (inode_t *inode, xlator_t *this) return ctx; } +int +__posix_acl_set_specific (inode_t *inode, xlator_t *this, + gf_boolean_t is_access, struct posix_acl *acl) +{ + int ret = 0; + struct posix_acl_ctx *ctx = NULL; + + ctx = posix_acl_ctx_get (inode, this); + if (!ctx) { + ret = -1; + goto out; + } + + if (is_access) + ctx->acl_access = acl; + else + ctx->acl_default = acl; +out: + return ret; +} int __posix_acl_set (inode_t *inode, xlator_t *this, struct posix_acl *acl_access, @@ -426,6 +446,36 @@ posix_acl_unref (xlator_t *this, struct posix_acl *acl) posix_acl_destroy (this, acl); } +int +posix_acl_set_specific (inode_t *inode, xlator_t *this, struct posix_acl *acl, + gf_boolean_t is_access) +{ + int ret = 0; + int oldret = 0; + struct posix_acl *old_acl = NULL; + struct posix_acl_conf *conf = NULL; + + conf = this->private; + + LOCK (&conf->acl_lock); + { + if (is_access) + oldret = __posix_acl_get (inode, this, &old_acl, NULL); + else + oldret = __posix_acl_get (inode, this, NULL, &old_acl); + if (acl) + acl->refcnt++; + ret = __posix_acl_set_specific (inode, this, is_access, acl); + } + UNLOCK (&conf->acl_lock); + + if (oldret == 0) { + if (old_acl) + posix_acl_unref (this, old_acl); + } + + return ret; +} int posix_acl_set (inode_t *inode, xlator_t *this, struct posix_acl *acl_access, @@ -497,7 +547,6 @@ unlock: return ret; } - mode_t posix_acl_inherit_mode (struct posix_acl *acl, mode_t modein) { @@ -1883,11 +1932,71 @@ posix_acl_setxattr_update (xlator_t *this, inode_t *inode, dict_t *xattr) return ret; } +/* * + * Posix acl can be set using other xattr such as GF_POSIX_ACL_ACCESS/ + * GF_POSIX_ACL_DEFAULT which requires to update the context of + * access-control translator + */ +int +handling_other_acl_related_xattr (xlator_t *this, inode_t *inode, dict_t *xattr) +{ + struct posix_acl *acl = NULL; + struct posix_acl_ctx *ctx = NULL; + data_t *data = NULL; + int ret = 0; + + ctx = posix_acl_ctx_get (inode, this); + if (!ctx) { + ret = -1; + goto out; + } + + data = dict_get (xattr, POSIX_ACL_ACCESS_XATTR); + if (data) { + ctx = posix_acl_ctx_get (inode, this); + if (!ctx) { + ret = -1; + goto out; + } + + acl = posix_acl_from_xattr (this, data->data, data->len); + ret = posix_acl_set_specific (inode, this, acl, _gf_true); + if (ret) + goto out; + + if (acl) + posix_acl_access_set_mode (acl, ctx); + + } + + if (acl) + posix_acl_unref (this, acl); + + data = dict_get (xattr, POSIX_ACL_DEFAULT_XATTR); + if (data) { + acl = posix_acl_from_xattr (this, data->data, data->len); + + ret = posix_acl_set_specific (inode, this, acl, _gf_false); + } + +out: + if (acl) + posix_acl_unref (this, acl); + + return ret; +} int posix_acl_setxattr_cbk (call_frame_t *frame, void *cookie, xlator_t *this, int op_ret, int op_errno, dict_t *xdata) { + + /* + * Update the context of posix_acl_translator, if any of + * POSIX_ACL_*_XATTR set in the call back + */ + handling_other_acl_related_xattr (this, (inode_t *)cookie, xdata); + STACK_UNWIND_STRICT (setxattr, frame, op_ret, op_errno, xdata); return 0; @@ -1907,9 +2016,13 @@ posix_acl_setxattr (call_frame_t *frame, xlator_t *this, loc_t *loc, posix_acl_setxattr_update (this, loc->inode, xattr); - STACK_WIND (frame, posix_acl_setxattr_cbk, - FIRST_CHILD(this), FIRST_CHILD(this)->fops->setxattr, - loc, xattr, flags, xdata); + /* + * inode is required in call back function to update the context + * this translator + */ + STACK_WIND_COOKIE (frame, posix_acl_setxattr_cbk, loc->inode, + FIRST_CHILD(this), FIRST_CHILD(this)->fops->setxattr, + loc, xattr, flags, xdata); return 0; red: STACK_UNWIND_STRICT (setxattr, frame, -1, op_errno, xdata); -- cgit