From f05edf8e2b92e2c018aad130cd7a1dcc00a8eeee Mon Sep 17 00:00:00 2001 From: Lai Siyao Date: Sun, 12 Jul 2020 09:15:16 +0800 Subject: [PATCH] LU-13791 sec: enable FS capabilities FS capabilities are not effective because they are dropped for non-root users for historical reason: they are used to be enforced before operations, but now they are checked in MDD layer only (see mdd_fix_attr()). Add sanity-sec 51. Signed-off-by: Lai Siyao Change-Id: I3e355f5df5eab5509b5e6774dbc8b82281a34039 Reviewed-on: https://review.whamcloud.com/39399 Tested-by: jenkins Reviewed-by: Sebastien Buisson Reviewed-by: Emoly Liu Tested-by: Maloo Reviewed-by: Oleg Drokin --- lustre/mdt/mdt_handler.c | 49 +++++++++++++++++++++++----------------------- lustre/mdt/mdt_internal.h | 1 - lustre/mdt/mdt_lib.c | 42 +++++++++++---------------------------- lustre/tests/sanity-sec.sh | 39 ++++++++++++++++++++++++++++++++++++ 4 files changed, 75 insertions(+), 56 deletions(-) diff --git a/lustre/mdt/mdt_handler.c b/lustre/mdt/mdt_handler.c index 67ebfc4..b4416c0 100644 --- a/lustre/mdt/mdt_handler.c +++ b/lustre/mdt/mdt_handler.c @@ -2162,37 +2162,38 @@ unlock_parent: static int mdt_getattr_name(struct tgt_session_info *tsi) { struct mdt_thread_info *info = tsi2mdt_info(tsi); - struct mdt_lock_handle *lhc = &info->mti_lh[MDT_LH_CHILD]; - struct mdt_body *reqbody; - struct mdt_body *repbody; - int rc, rc2; - ENTRY; + struct mdt_lock_handle *lhc = &info->mti_lh[MDT_LH_CHILD]; + struct mdt_body *reqbody; + struct mdt_body *repbody; + int rc, rc2; - reqbody = req_capsule_client_get(info->mti_pill, &RMF_MDT_BODY); - LASSERT(reqbody != NULL); - repbody = req_capsule_server_get(info->mti_pill, &RMF_MDT_BODY); - LASSERT(repbody != NULL); + ENTRY; + + reqbody = req_capsule_client_get(info->mti_pill, &RMF_MDT_BODY); + LASSERT(reqbody != NULL); + repbody = req_capsule_server_get(info->mti_pill, &RMF_MDT_BODY); + LASSERT(repbody != NULL); info->mti_cross_ref = !!(reqbody->mbo_valid & OBD_MD_FLCROSSREF); repbody->mbo_eadatasize = 0; repbody->mbo_aclsize = 0; - rc = mdt_init_ucred_intent_getattr(info, reqbody); - if (unlikely(rc)) - GOTO(out_shrink, rc); + rc = mdt_init_ucred(info, reqbody); + if (unlikely(rc)) + GOTO(out_shrink, rc); - rc = mdt_getattr_name_lock(info, lhc, MDS_INODELOCK_UPDATE, NULL); - if (lustre_handle_is_used(&lhc->mlh_reg_lh)) { - ldlm_lock_decref(&lhc->mlh_reg_lh, lhc->mlh_reg_mode); - lhc->mlh_reg_lh.cookie = 0; - } - mdt_exit_ucred(info); - EXIT; + rc = mdt_getattr_name_lock(info, lhc, MDS_INODELOCK_UPDATE, NULL); + if (lustre_handle_is_used(&lhc->mlh_reg_lh)) { + ldlm_lock_decref(&lhc->mlh_reg_lh, lhc->mlh_reg_mode); + lhc->mlh_reg_lh.cookie = 0; + } + mdt_exit_ucred(info); + EXIT; out_shrink: - mdt_client_compatibility(info); - rc2 = mdt_fix_reply(info); - if (rc == 0) - rc = rc2; + mdt_client_compatibility(info); + rc2 = mdt_fix_reply(info); + if (rc == 0) + rc = rc2; mdt_thread_info_fini(info); return rc; } @@ -4259,7 +4260,7 @@ static int mdt_intent_getattr(enum ldlm_intent_flags it_opc, GOTO(out_shrink, rc = -EINVAL); } - rc = mdt_init_ucred_intent_getattr(info, reqbody); + rc = mdt_init_ucred(info, reqbody); if (rc) GOTO(out_shrink, rc); diff --git a/lustre/mdt/mdt_internal.h b/lustre/mdt/mdt_internal.h index 8ffa943..8e22408 100644 --- a/lustre/mdt/mdt_internal.h +++ b/lustre/mdt/mdt_internal.h @@ -933,7 +933,6 @@ void mdt_dump_lmv(unsigned int level, const union lmv_mds_md *lmv); bool allow_client_chgrp(struct mdt_thread_info *info, struct lu_ucred *uc); int mdt_check_ucred(struct mdt_thread_info *); int mdt_init_ucred(struct mdt_thread_info *, struct mdt_body *); -int mdt_init_ucred_intent_getattr(struct mdt_thread_info *, struct mdt_body *); int mdt_init_ucred_reint(struct mdt_thread_info *); void mdt_exit_ucred(struct mdt_thread_info *); int mdt_version_get_check(struct mdt_thread_info *, struct mdt_object *, int); diff --git a/lustre/mdt/mdt_lib.c b/lustre/mdt/mdt_lib.c index f3a0d42..24bc1e8 100644 --- a/lustre/mdt/mdt_lib.c +++ b/lustre/mdt/mdt_lib.c @@ -166,7 +166,7 @@ static void ucred_set_audit_enabled(struct mdt_thread_info *info, } static int new_init_ucred(struct mdt_thread_info *info, ucred_init_type_t type, - void *buf, bool drop_fs_cap) + void *buf) { struct ptlrpc_request *req = mdt_info_req(info); struct mdt_device *mdt = info->mti_mdt; @@ -324,11 +324,7 @@ static int new_init_ucred(struct mdt_thread_info *info, ucred_init_type_t type, /* process root_squash here. */ mdt_root_squash(info, peernid); - /* remove fs privilege for non-root user. */ - if (ucred->uc_fsuid && drop_fs_cap) - ucred->uc_cap = pud->pud_cap & ~CFS_CAP_FS_MASK; - else - ucred->uc_cap = pud->pud_cap; + ucred->uc_cap = pud->pud_cap; ucred->uc_valid = UCRED_NEW; ucred_set_jobid(info, ucred); ucred_set_nid(info, ucred); @@ -462,8 +458,7 @@ out: } static int old_init_ucred_common(struct mdt_thread_info *info, - struct lu_nodemap *nodemap, - bool drop_fs_cap) + struct lu_nodemap *nodemap) { struct lu_ucred *uc = mdt_ucred(info); struct mdt_device *mdt = info->mti_mdt; @@ -500,9 +495,6 @@ static int old_init_ucred_common(struct mdt_thread_info *info, /* process root_squash here. */ mdt_root_squash(info, mdt_info_req(info)->rq_peer.nid); - /* remove fs privilege for non-root user. */ - if (uc->uc_fsuid && drop_fs_cap) - uc->uc_cap &= ~CFS_CAP_FS_MASK; uc->uc_valid = UCRED_OLD; ucred_set_jobid(info, uc); ucred_set_nid(info, uc); @@ -514,7 +506,7 @@ static int old_init_ucred_common(struct mdt_thread_info *info, } static int old_init_ucred(struct mdt_thread_info *info, - struct mdt_body *body, bool drop_fs_cap) + struct mdt_body *body) { struct lu_ucred *uc = mdt_ucred(info); struct lu_nodemap *nodemap; @@ -545,7 +537,7 @@ static int old_init_ucred(struct mdt_thread_info *info, uc->uc_ginfo = NULL; uc->uc_cap = body->mbo_capability; - rc = old_init_ucred_common(info, nodemap, drop_fs_cap); + rc = old_init_ucred_common(info, nodemap); nodemap_putref(nodemap); RETURN(rc); @@ -574,15 +566,14 @@ static int old_init_ucred_reint(struct mdt_thread_info *info) uc->uc_o_gid = uc->uc_o_fsgid = uc->uc_gid = uc->uc_fsgid; uc->uc_ginfo = NULL; - rc = old_init_ucred_common(info, nodemap, true); /* drop_fs_cap=true */ + rc = old_init_ucred_common(info, nodemap); nodemap_putref(nodemap); RETURN(rc); } static inline int __mdt_init_ucred(struct mdt_thread_info *info, - struct mdt_body *body, - bool drop_fs_cap) + struct mdt_body *body) { struct ptlrpc_request *req = mdt_info_req(info); struct lu_ucred *uc = mdt_ucred(info); @@ -594,25 +585,14 @@ static inline int __mdt_init_ucred(struct mdt_thread_info *info, mdt_exit_ucred(info); if (!req->rq_auth_gss || req->rq_auth_usr_mdt || !req->rq_user_desc) - return old_init_ucred(info, body, drop_fs_cap); + return old_init_ucred(info, body); else - return new_init_ucred(info, BODY_INIT, body, drop_fs_cap); + return new_init_ucred(info, BODY_INIT, body); } int mdt_init_ucred(struct mdt_thread_info *info, struct mdt_body *body) { - return __mdt_init_ucred(info, body, true); -} - -/* LU-6528 when "no_subtree_check" is set for NFS export, nfsd_set_fh_dentry() - * doesn't set correct fsuid explicitely, but raise capability to allow - * exportfs_decode_fh() to reconnect disconnected dentry into dcache. So for - * lookup (i.e. intent_getattr), we should keep FS capability, otherwise it - * will fail permission check. */ -int mdt_init_ucred_intent_getattr(struct mdt_thread_info *info, - struct mdt_body *body) -{ - return __mdt_init_ucred(info, body, false); + return __mdt_init_ucred(info, body); } int mdt_init_ucred_reint(struct mdt_thread_info *info) @@ -635,7 +615,7 @@ int mdt_init_ucred_reint(struct mdt_thread_info *info) if (!req->rq_auth_gss || req->rq_auth_usr_mdt || !req->rq_user_desc) return old_init_ucred_reint(info); else - return new_init_ucred(info, REC_INIT, NULL, true); + return new_init_ucred(info, REC_INIT, NULL); } /* copied from lov/lov_ea.c, just for debugging, will be removed later */ diff --git a/lustre/tests/sanity-sec.sh b/lustre/tests/sanity-sec.sh index 6344ebe..772a0a0 100755 --- a/lustre/tests/sanity-sec.sh +++ b/lustre/tests/sanity-sec.sh @@ -3725,6 +3725,45 @@ test_50() { } run_test 50 "DoM encrypted file" +test_51() { + [ "$MDS1_VERSION" -gt $(version_code 2.13.53) ] || + skip "Need MDS version at least 2.13.53" + + mkdir $DIR/$tdir || error "mkdir $tdir" + + touch $DIR/$tdir/$tfile || error "touch $tfile" + cp $(which chown) $DIR/$tdir || error "cp chown" + $RUNAS_CMD -u $ID0 $DIR/$tdir/chown $ID0 $DIR/$tdir/$tfile && + error "chown $tfile should fail" + setcap 'CAP_CHOWN=ep' $DIR/$tdir/chown || error "setcap CAP_CHOWN" + $RUNAS_CMD -u $ID0 $DIR/$tdir/chown $ID0 $DIR/$tdir/$tfile || + error "chown $tfile" + rm $DIR/$tdir/$tfile || error "rm $tfile" + + touch $DIR/$tdir/$tfile || error "touch $tfile" + cp $(which touch) $DIR/$tdir || error "cp touch" + $RUNAS_CMD -u $ID0 $DIR/$tdir/touch $DIR/$tdir/$tfile && + error "touch should fail" + setcap 'CAP_FOWNER=ep' $DIR/$tdir/touch || error "setcap CAP_FOWNER" + $RUNAS_CMD -u $ID0 $DIR/$tdir/touch $DIR/$tdir/$tfile || + error "touch $tfile" + rm $DIR/$tdir/$tfile || error "rm $tfile" + + local cap + for cap in "CAP_DAC_OVERRIDE" "CAP_DAC_READ_SEARCH"; do + touch $DIR/$tdir/$tfile || error "touch $tfile" + chmod 600 $DIR/$tdir/$tfile || error "chmod $tfile" + cp $(which cat) $DIR/$tdir || error "cp cat" + $RUNAS_CMD -u $ID0 $DIR/$tdir/cat $DIR/$tdir/$tfile && + error "cat should fail" + setcap $cap=ep $DIR/$tdir/cat || error "setcap $cap" + $RUNAS_CMD -u $ID0 $DIR/$tdir/cat $DIR/$tdir/$tfile || + error "cat $tfile" + rm $DIR/$tdir/$tfile || error "rm $tfile" + done +} +run_test 51 "FS capabilities ===============" + log "cleanup: ======================================================" sec_unsetup() { -- 1.8.3.1