locks/fencing: Add a security knob for fencing

There is a low level security issue with fencing since one client
can preempt another client's lock.

This patch does not completely eliminate the issue of a client
misbehaving, but certainly it adds a security layer for default use cases
that does not need fencing.

Change-Id: I55cd15f2ed1ae0f2556e3d27a2ef4bc10fdada1c
updates: #466
Signed-off-by: Susant Palai <spalai@redhat.com>

6 files changed, 79 insertions, 9 deletions

diff --git a/tests/basic/fencing/fence-basic.t b/tests/basic/fencing/fence-basic.t
index 080507c2ba5..30f379e7b20 100755
--- a/tests/basic/fencing/fence-basic.t
+++ b/tests/basic/fencing/fence-basic.t
@@ -18,6 +18,8 @@ EXPECT 'Started' volinfo_field $V0 'Status';
 TEST $CLI volume set $V0 diagnostics.client-log-flush-timeout 30
 TEST $CLI volume set $V0 performance.write-behind off
 TEST $CLI volume set $V0 locks.mandatory-locking forced
+TEST $CLI volume set $V0 enforce-mandatory-lock on
+
 
 logdir=`gluster --print-logdir`
 
diff --git a/tests/basic/fencing/fencing-crash-conistency.t b/tests/basic/fencing/fencing-crash-conistency.t
index cca8ee7343f..0c69411e90c 100644
--- a/tests/basic/fencing/fencing-crash-conistency.t
+++ b/tests/basic/fencing/fencing-crash-conistency.t
@@ -26,6 +26,7 @@ TEST "truncate -s 0 $M0/file"
 
 #enable mandatory locking
 TEST $CLI volume set $V0 locks.mandatory-locking forced
+TEST $CLI volume set $V0 enforce-mandatory-lock on
 
 #write should pass
 TEST "echo "test" >> $M0/file"
diff --git a/tests/basic/fencing/test-fence-option.t b/tests/basic/fencing/test-fence-option.t
new file mode 100644
index 00000000000..115cbe7dbdf
--- /dev/null
+++ b/tests/basic/fencing/test-fence-option.t
@@ -0,0 +1,37 @@
+#!/bin/bash
+
+. $(dirname $0)/../../include.rc
+. $(dirname $0)/../../volume.rc
+
+cleanup;
+
+# with lock enforcement flag write should fail with out lock
+
+TEST glusterd
+TEST pidof glusterd
+TEST $CLI volume info;
+
+TEST $CLI volume create $V0 $H0:$B0/${V0}1
+EXPECT 'Created' volinfo_field $V0 'Status';
+TEST $CLI volume start $V0;
+EXPECT 'Started' volinfo_field $V0 'Status';
+TEST glusterfs -s $H0 --volfile-id $V0 $M0
+
+TEST touch $M0/file
+
+#setfattr for mandatory-enforcement will fail
+TEST ! setfattr -n trusted.glusterfs.enforce-mandatory-lock -v 1 $M0/file
+
+#enable mandatory locking
+TEST $CLI volume set $V0 locks.mandatory-locking forced
+
+#setfattr will fail
+TEST ! setfattr -n trusted.glusterfs.enforce-mandatory-lock -v 1 $M0/file
+
+#set lock-enforcement option
+TEST $CLI volume set $V0 enforce-mandatory-lock on
+
+#setfattr should succeed
+TEST setfattr -n trusted.glusterfs.enforce-mandatory-lock -v 1 $M0/file
+
+cleanup;
\ No newline at end of file
diff --git a/xlators/features/locks/src/locks.h b/xlators/features/locks/src/locks.h
index 842c44140fe..b817960a4c4 100644
--- a/xlators/features/locks/src/locks.h
+++ b/xlators/features/locks/src/locks.h
@@ -223,6 +223,7 @@ typedef struct {
     gf_boolean_t monkey_unlocking;
     gf_boolean_t revocation_clear_all;
     gf_boolean_t notify_contention;
+    gf_boolean_t mlock_enforced;
 } posix_locks_private_t;
 
 typedef struct {
diff --git a/xlators/features/locks/src/posix.c b/xlators/features/locks/src/posix.c
index 0ab38ee6563..0feb11e3b78 100644
--- a/xlators/features/locks/src/posix.c
+++ b/xlators/features/locks/src/posix.c
@@ -119,16 +119,19 @@ fetch_pathinfo(xlator_t *, inode_t *, int32_t *, char **);
         }                                                                      \
     } while (0)
 
-#define PL_CHECK_LOCK_ENFORCE_KEY(frame, dict, name, this, loc, fd)            \
+#define PL_CHECK_LOCK_ENFORCE_KEY(frame, dict, name, this, loc, fd, priv)      \
     do {                                                                       \
         if (dict_get(dict, GF_ENFORCE_MANDATORY_LOCK) ||                       \
             (name && (strcmp(name, GF_ENFORCE_MANDATORY_LOCK) == 0))) {        \
             inode_t *__inode = (loc ? loc->inode : fd->inode);                 \
             pl_inode_t *__pl_inode = pl_inode_get(this, __inode, NULL);        \
-            if (!pl_is_mandatory_locking_enabled(__pl_inode)) {                \
+            if (!pl_is_mandatory_locking_enabled(__pl_inode) ||                \
+                !priv->mlock_enforced) {                                       \
                 op_ret = -1;                                                   \
-                gf_msg(this->name, GF_LOG_ERROR, EINVAL, 0,                    \
-                       "option %s would need mandatory lock to be enabled",    \
+                gf_msg(this->name, GF_LOG_DEBUG, EINVAL, 0,                    \
+                       "option %s would need mandatory lock to be enabled "    \
+                       "and feature.enforce-mandatory-lock option to be set "  \
+                       "to on",                                                \
                        GF_ENFORCE_MANDATORY_LOCK);                             \
                 op_errno = EINVAL;                                             \
                 goto unwind;                                                   \
@@ -1654,6 +1657,7 @@ pl_fsetxattr(call_frame_t *frame, xlator_t *this, fd_t *fd, dict_t *dict,
     void *lockinfo_buf = NULL;
     int len = 0;
     char *name = NULL;
+    posix_locks_private_t *priv = this->private;
 
     int32_t op_ret = dict_get_ptr_and_len(dict, GF_XATTR_LOCKINFO_KEY,
                                           &lockinfo_buf, &len);
@@ -1670,7 +1674,8 @@ pl_fsetxattr(call_frame_t *frame, xlator_t *this, fd_t *fd, dict_t *dict,
 usual:
     PL_LOCAL_GET_REQUESTS(frame, this, xdata, fd, NULL, NULL);
 
-    PL_CHECK_LOCK_ENFORCE_KEY(frame, dict, name, this, ((loc_t *)NULL), fd);
+    PL_CHECK_LOCK_ENFORCE_KEY(frame, dict, name, this, ((loc_t *)NULL), fd,
+                              priv);
 
     STACK_WIND(frame, pl_fsetxattr_cbk, FIRST_CHILD(this),
                FIRST_CHILD(this)->fops->fsetxattr, fd, dict, flags, xdata);
@@ -3367,6 +3372,7 @@ pl_setxattr(call_frame_t *frame, xlator_t *this, loc_t *loc, dict_t *dict,
     int op_errno = EINVAL;
     dict_t *xdata_rsp = NULL;
     char *name = NULL;
+    posix_locks_private_t *priv = this->private;
 
     PL_LOCAL_GET_REQUESTS(frame, this, xdata, ((fd_t *)NULL), loc, NULL);
 
@@ -3384,7 +3390,8 @@ pl_setxattr(call_frame_t *frame, xlator_t *this, loc_t *loc, dict_t *dict,
     return 0;
 
 usual:
-    PL_CHECK_LOCK_ENFORCE_KEY(frame, dict, name, this, loc, ((fd_t *)NULL));
+    PL_CHECK_LOCK_ENFORCE_KEY(frame, dict, name, this, loc, ((fd_t *)NULL),
+                              priv);
 
     STACK_WIND(frame, pl_setxattr_cbk, FIRST_CHILD(this),
                FIRST_CHILD(this)->fops->setxattr, loc, dict, flags, xdata);
@@ -3897,6 +3904,10 @@ reconfigure(xlator_t *this, dict_t *options)
                      options, uint32, out);
 
     GF_OPTION_RECONF("mandatory-locking", tmp_str, options, str, out);
+
+    GF_OPTION_RECONF("enforce-mandatory-lock", priv->mlock_enforced, options,
+                     bool, out);
+
     if (!strcmp(tmp_str, "forced"))
         priv->mandatory_mode = MLK_FORCED;
     else if (!strcmp(tmp_str, "file"))
@@ -3973,6 +3984,8 @@ init(xlator_t *this)
     GF_OPTION_INIT("notify-contention-delay", priv->notify_contention_delay,
                    uint32, out);
 
+    GF_OPTION_INIT("enforce-mandatory-lock", priv->mlock_enforced, bool, out);
+
     this->local_pool = mem_pool_new(pl_local_t, 32);
     if (!this->local_pool) {
         ret = -1;
@@ -4410,11 +4423,12 @@ pl_removexattr(call_frame_t *frame, xlator_t *this, loc_t *loc,
 {
     int op_ret = 0;
     int op_errno = EINVAL;
+    posix_locks_private_t *priv = this->private;
 
     PL_LOCAL_GET_REQUESTS(frame, this, xdata, ((fd_t *)NULL), loc, NULL);
 
     PL_CHECK_LOCK_ENFORCE_KEY(frame, ((dict_t *)NULL), name, this, loc,
-                              ((fd_t *)NULL));
+                              ((fd_t *)NULL), priv);
 
     STACK_WIND(frame, pl_removexattr_cbk, FIRST_CHILD(this),
                FIRST_CHILD(this)->fops->removexattr, loc, name, xdata);
@@ -4463,11 +4477,12 @@ pl_fremovexattr(call_frame_t *frame, xlator_t *this, fd_t *fd, const char *name,
 {
     int op_ret = -1;
     int op_errno = EINVAL;
+    posix_locks_private_t *priv = this->private;
 
     PL_LOCAL_GET_REQUESTS(frame, this, xdata, fd, NULL, NULL);
 
     PL_CHECK_LOCK_ENFORCE_KEY(frame, ((dict_t *)NULL), name, this,
-                              ((loc_t *)NULL), fd);
+                              ((loc_t *)NULL), fd, priv);
 
     STACK_WIND(frame, pl_fremovexattr_cbk, FIRST_CHILD(this),
                FIRST_CHILD(this)->fops->fremovexattr, fd, name, xdata);
@@ -4802,6 +4817,12 @@ struct volume_options options[] = {
                     "on the same inode. If multiple lock requests are "
                     "received during this period, only one upcall will "
                     "be sent."},
+    {.key = {"enforce-mandatory-lock"},
+     .type = GF_OPTION_TYPE_BOOL,
+     .default_value = "off",
+     .flags = OPT_FLAG_SETTABLE,
+     .op_version = {GD_OP_VERSION_6_0},
+     .description = "option to enable lock enforcement"},
     {.key = {NULL}},
 };
 
@@ -4817,4 +4838,4 @@ xlator_api_t xlator_api = {
     .options = options,
     .identifier = "locks",
     .category = GF_MAINTAINED,
-};
\ No newline at end of file
+};
diff --git a/xlators/mgmt/glusterd/src/glusterd-volume-set.c b/xlators/mgmt/glusterd/src/glusterd-volume-set.c
index fd5a8227c6b..d952a39d23e 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volume-set.c
+++ b/xlators/mgmt/glusterd/src/glusterd-volume-set.c
@@ -2947,4 +2947,12 @@ struct volopt_map_entry glusterd_volopt_map[] = {
      .voltype = "features/cloudsync",
      .op_version = GD_OP_VERSION_5_0,
      .flags = VOLOPT_FLAG_CLIENT_OPT},
+    {.key = "features.enforce-mandatory-lock",
+     .voltype = "features/locks",
+     .value = "off",
+     .type = NO_DOC,
+     .op_version = GD_OP_VERSION_6_0,
+     .validate_fn = validate_boolean,
+     .description = "option to enforce mandatory lock on a file",
+     .flags = VOLOPT_FLAG_XLATOR_OPT},
     {.key = NULL}};