From b5e421625be474c74e8467a6fb7089f974898558 Mon Sep 17 00:00:00 2001 From: Sebastien Buisson Date: Tue, 12 Mar 2024 15:12:38 +0100 Subject: [PATCH] EX-9392 sec: use dedicated INTERNAL upcall cache Implement the INTERNAL upcall cache as a dedicated, separate cache. This makes it distinct from the regular identity upcall cache that can be defined to use any upcall including NONE, per an MDT side tuning. The INTERNAL upcall cache becomes accessible only to clients that belong to a nodemap for which the 'server_upcall' rbac role is not enabled. Dedicated mdt-side tunables are created to configure the entry expiry time and the acquire expire time for INTERNAL, as well as a tunable to flush the INTERNAL upcall cache. Signed-off-by: Sebastien Buisson Change-Id: I0267182fbfa646de40ac62f832e89fbfd8477822 Reviewed-on: https://review.whamcloud.com/c/ex/lustre-release/+/54361 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Andreas Dilger --- lustre/mdt/mdt_handler.c | 22 ++++++- lustre/mdt/mdt_internal.h | 1 + lustre/mdt/mdt_lib.c | 37 ++++++++---- lustre/mdt/mdt_lproc.c | 126 ++++++++++++++++++++++++++++++++++------- lustre/obdclass/upcall_cache.c | 4 -- lustre/tests/runas.c | 2 +- lustre/tests/sanity-sec.sh | 80 +++++++++++++++++++------- 7 files changed, 213 insertions(+), 59 deletions(-) diff --git a/lustre/mdt/mdt_handler.c b/lustre/mdt/mdt_handler.c index 2a9db61..ec108f7 100644 --- a/lustre/mdt/mdt_handler.c +++ b/lustre/mdt/mdt_handler.c @@ -5957,6 +5957,8 @@ static void mdt_fini(const struct lu_env *env, struct mdt_device *m) target_recovery_fini(obd); upcall_cache_cleanup(m->mdt_identity_cache); m->mdt_identity_cache = NULL; + upcall_cache_cleanup(m->mdt_identity_cache_int); + m->mdt_identity_cache_int = NULL; tgt_fini(env, &m->mdt_lut); @@ -6009,6 +6011,7 @@ static int mdt_init0(const struct lu_env *env, struct mdt_device *m, struct lu_site *s; struct seq_server_site *ss_site; const char *identity_upcall = "NONE"; + char cache_internal[NAME_MAX + 1] = { 0 }; struct md_device *next; struct lu_fid fid; int rc; @@ -6209,7 +6212,6 @@ static int mdt_init0(const struct lu_env *env, struct mdt_device *m, * by default, otherwise, maybe got unexpected -EACCESS. */ if (m->mdt_opts.mo_acl) identity_upcall = MDT_IDENTITY_UPCALL_PATH; - m->mdt_identity_cache = upcall_cache_init(mdt_obd_name(m), identity_upcall, UC_IDCACHE_HASH_SIZE, @@ -6223,6 +6225,21 @@ static int mdt_init0(const struct lu_env *env, struct mdt_device *m, GOTO(err_free_hsm, rc); } + snprintf(cache_internal, sizeof(cache_internal), "%s_int", + mdt_obd_name(m)); + m->mdt_identity_cache_int = upcall_cache_init(cache_internal, + "INTERNAL", + UC_IDCACHE_HASH_SIZE, + 1200, /* entry expire: 20 mn */ + 30, /* acquire expire: 30 s */ + true, /* acquire can replay */ + &mdt_identity_upcall_cache_ops); + if (IS_ERR(m->mdt_identity_cache_int)) { + rc = PTR_ERR(m->mdt_identity_cache_int); + m->mdt_identity_cache_int = NULL; + GOTO(err_cache, rc); + } + rc = mdt_tunables_init(m, dev); if (rc) { CERROR("Can't init MDT lprocfs, rc %d\n", rc); @@ -6263,6 +6280,9 @@ err_ping_evictor: err_procfs: mdt_tunables_fini(m); err_recovery: + upcall_cache_cleanup(m->mdt_identity_cache_int); + m->mdt_identity_cache_int = NULL; +err_cache: upcall_cache_cleanup(m->mdt_identity_cache); m->mdt_identity_cache = NULL; err_free_hsm: diff --git a/lustre/mdt/mdt_internal.h b/lustre/mdt/mdt_internal.h index cb74cfe..9586236 100644 --- a/lustre/mdt/mdt_internal.h +++ b/lustre/mdt/mdt_internal.h @@ -293,6 +293,7 @@ struct mdt_device { __u32 mdt_brw_size; struct upcall_cache *mdt_identity_cache; + struct upcall_cache *mdt_identity_cache_int; unsigned int mdt_evict_tgt_nids:1, mdt_dom_read_open:1, diff --git a/lustre/mdt/mdt_lib.c b/lustre/mdt/mdt_lib.c index 1198ca2..a0ca65b 100644 --- a/lustre/mdt/mdt_lib.c +++ b/lustre/mdt/mdt_lib.c @@ -62,10 +62,25 @@ static __u64 get_mrc_cr_flags(struct mdt_rec_create *mrc) return (__u64)(mrc->cr_flags_l) | ((__u64)mrc->cr_flags_h << 32); } +static struct upcall_cache *get_cache(struct mdt_thread_info *info) +{ + /* If nodemap does not have server_upcall role, + * use dedicated INTERNAL upcall cache, unless + * Kerberos is enforced, in which case we do not trust the client's + * provided supplementary groups. + */ + struct ptlrpc_request *req = mdt_info_req(info); + + if (SPTLRPC_FLVR_MECH(req->rq_flvr.sf_rpc) == SPTLRPC_MECH_GSS_KRB5 || + mdt_ucred(info)->uc_rbac_server_upcall) + return info->mti_mdt->mdt_identity_cache; + + return info->mti_mdt->mdt_identity_cache_int; +} + void mdt_exit_ucred(struct mdt_thread_info *info) { struct lu_ucred *uc = mdt_ucred(info); - struct mdt_device *mdt = info->mti_mdt; LASSERT(uc != NULL); if (uc->uc_valid != UCRED_INIT) { @@ -75,8 +90,7 @@ void mdt_exit_ucred(struct mdt_thread_info *info) uc->uc_ginfo = NULL; } if (uc->uc_identity) { - mdt_identity_put(mdt->mdt_identity_cache, - uc->uc_identity); + mdt_identity_put(get_cache(info), uc->uc_identity); uc->uc_identity = NULL; } uc->uc_valid = UCRED_INIT; @@ -255,13 +269,15 @@ static int new_init_ucred(struct mdt_thread_info *info, ucred_init_type_t type, /* deny access before we get identity ref */ GOTO(out, rc = -EACCES); - if (is_identity_get_disabled(mdt->mdt_identity_cache)) { + ucred_set_rbac_roles(info, ucred); + + if (is_identity_get_disabled(get_cache(info))) { ucred->uc_identity = NULL; perm = CFS_SETUID_PERM | CFS_SETGID_PERM | CFS_SETGRP_PERM; } else { struct md_identity *identity; - identity = mdt_identity_get(mdt->mdt_identity_cache, + identity = mdt_identity_get(get_cache(info), pud->pud_uid, info); if (IS_ERR(identity)) { if (unlikely(PTR_ERR(identity) == -EREMCHG)) { @@ -385,7 +401,6 @@ static int new_init_ucred(struct mdt_thread_info *info, ucred_init_type_t type, ucred_set_jobid(info, ucred); ucred_set_nid(info, ucred); ucred_set_audit_enabled(info, ucred); - ucred_set_rbac_roles(info, ucred); EXIT; @@ -396,8 +411,7 @@ out: ucred->uc_ginfo = NULL; } if (ucred->uc_identity) { - mdt_identity_put(mdt->mdt_identity_cache, - ucred->uc_identity); + mdt_identity_put(get_cache(info), ucred->uc_identity); ucred->uc_identity = NULL; } } @@ -530,8 +544,10 @@ static int old_init_ucred_common(struct mdt_thread_info *info, /* deny access before we get identity ref */ RETURN(-EACCES); - if (!is_identity_get_disabled(mdt->mdt_identity_cache)) { - identity = mdt_identity_get(mdt->mdt_identity_cache, + ucred_set_rbac_roles(info, uc); + + if (!is_identity_get_disabled(get_cache(info))) { + identity = mdt_identity_get(get_cache(info), uc->uc_fsuid, info); if (IS_ERR(identity)) { if (unlikely(PTR_ERR(identity) == -EREMCHG || @@ -596,7 +612,6 @@ static int old_init_ucred_common(struct mdt_thread_info *info, ucred_set_jobid(info, uc); ucred_set_nid(info, uc); ucred_set_audit_enabled(info, uc); - ucred_set_rbac_roles(info, uc); EXIT; return 0; diff --git a/lustre/mdt/mdt_lproc.c b/lustre/mdt/mdt_lproc.c index af0e875..924935a 100644 --- a/lustre/mdt/mdt_lproc.c +++ b/lustre/mdt/mdt_lproc.c @@ -216,13 +216,9 @@ static ssize_t identity_expire_show(struct kobject *kobj, mdt->mdt_identity_cache->uc_entry_expire); } -static ssize_t identity_expire_store(struct kobject *kobj, - struct attribute *attr, - const char *buffer, size_t count) +static ssize_t entry_expire_store(struct upcall_cache *cache, + const char *buffer, size_t count) { - struct obd_device *obd = container_of(kobj, struct obd_device, - obd_kset.kobj); - struct mdt_device *mdt = mdt_dev(obd->obd_lu_dev); time64_t val; int rc; @@ -233,10 +229,21 @@ static ssize_t identity_expire_store(struct kobject *kobj, if (val < 0) return -ERANGE; - mdt->mdt_identity_cache->uc_entry_expire = val; + cache->uc_entry_expire = val; return count; } + +static ssize_t identity_expire_store(struct kobject *kobj, + struct attribute *attr, + const char *buffer, size_t count) +{ + struct obd_device *obd = container_of(kobj, struct obd_device, + obd_kset.kobj); + struct mdt_device *mdt = mdt_dev(obd->obd_lu_dev); + + return entry_expire_store(mdt->mdt_identity_cache, buffer, count); +} LUSTRE_RW_ATTR(identity_expire); static ssize_t identity_acquire_expire_show(struct kobject *kobj, @@ -250,13 +257,9 @@ static ssize_t identity_acquire_expire_show(struct kobject *kobj, mdt->mdt_identity_cache->uc_acquire_expire); } -static ssize_t identity_acquire_expire_store(struct kobject *kobj, - struct attribute *attr, - const char *buffer, size_t count) +static ssize_t acquire_expire_store(struct upcall_cache *cache, + const char *buffer, size_t count) { - struct obd_device *obd = container_of(kobj, struct obd_device, - obd_kset.kobj); - struct mdt_device *mdt = mdt_dev(obd->obd_lu_dev); time64_t val; int rc; @@ -267,10 +270,21 @@ static ssize_t identity_acquire_expire_store(struct kobject *kobj, if (val < 0 || val > INT_MAX) return -ERANGE; - mdt->mdt_identity_cache->uc_acquire_expire = val; + cache->uc_acquire_expire = val; return count; } + +static ssize_t identity_acquire_expire_store(struct kobject *kobj, + struct attribute *attr, + const char *buffer, size_t count) +{ + struct obd_device *obd = container_of(kobj, struct obd_device, + obd_kset.kobj); + struct mdt_device *mdt = mdt_dev(obd->obd_lu_dev); + + return acquire_expire_store(mdt->mdt_identity_cache, buffer, count); +} LUSTRE_RW_ATTR(identity_acquire_expire); static ssize_t identity_upcall_show(struct kobject *kobj, @@ -320,13 +334,9 @@ static ssize_t identity_upcall_store(struct kobject *kobj, } LUSTRE_RW_ATTR(identity_upcall); -static ssize_t identity_flush_store(struct kobject *kobj, - struct attribute *attr, - const char *buffer, size_t count) +static ssize_t flush_store(struct upcall_cache *cache, + const char *buffer, size_t count) { - struct obd_device *obd = container_of(kobj, struct obd_device, - obd_kset.kobj); - struct mdt_device *mdt = mdt_dev(obd->obd_lu_dev); int uid; int rc; @@ -334,9 +344,20 @@ static ssize_t identity_flush_store(struct kobject *kobj, if (rc) return rc; - mdt_flush_identity(mdt->mdt_identity_cache, uid); + mdt_flush_identity(cache, uid); return count; } + +static ssize_t identity_flush_store(struct kobject *kobj, + struct attribute *attr, + const char *buffer, size_t count) +{ + struct obd_device *obd = container_of(kobj, struct obd_device, + obd_kset.kobj); + struct mdt_device *mdt = mdt_dev(obd->obd_lu_dev); + + return flush_store(mdt->mdt_identity_cache, buffer, count); +} LUSTRE_WO_ATTR(identity_flush); static ssize_t @@ -407,6 +428,66 @@ out: } LPROC_SEQ_FOPS_WR_ONLY(mdt, identity_info); +static ssize_t identity_int_expire_show(struct kobject *kobj, + struct attribute *attr, char *buf) +{ + struct obd_device *obd = container_of(kobj, struct obd_device, + obd_kset.kobj); + struct mdt_device *mdt = mdt_dev(obd->obd_lu_dev); + + return scnprintf(buf, PAGE_SIZE, "%lld\n", + mdt->mdt_identity_cache_int->uc_entry_expire); +} + +static ssize_t identity_int_expire_store(struct kobject *kobj, + struct attribute *attr, + const char *buffer, size_t count) +{ + struct obd_device *obd = container_of(kobj, struct obd_device, + obd_kset.kobj); + struct mdt_device *mdt = mdt_dev(obd->obd_lu_dev); + + return entry_expire_store(mdt->mdt_identity_cache_int, buffer, count); +} +LUSTRE_RW_ATTR(identity_int_expire); + +static ssize_t identity_int_acquire_expire_show(struct kobject *kobj, + struct attribute *attr, + char *buf) +{ + struct obd_device *obd = container_of(kobj, struct obd_device, + obd_kset.kobj); + struct mdt_device *mdt = mdt_dev(obd->obd_lu_dev); + + return scnprintf(buf, PAGE_SIZE, "%lld\n", + mdt->mdt_identity_cache_int->uc_acquire_expire); +} + +static ssize_t identity_int_acquire_expire_store(struct kobject *kobj, + struct attribute *attr, + const char *buffer, + size_t count) +{ + struct obd_device *obd = container_of(kobj, struct obd_device, + obd_kset.kobj); + struct mdt_device *mdt = mdt_dev(obd->obd_lu_dev); + + return acquire_expire_store(mdt->mdt_identity_cache_int, buffer, count); +} +LUSTRE_RW_ATTR(identity_int_acquire_expire); + +static ssize_t identity_int_flush_store(struct kobject *kobj, + struct attribute *attr, + const char *buffer, size_t count) +{ + struct obd_device *obd = container_of(kobj, struct obd_device, + obd_kset.kobj); + struct mdt_device *mdt = mdt_dev(obd->obd_lu_dev); + + return flush_store(mdt->mdt_identity_cache_int, buffer, count); +} +LUSTRE_WO_ATTR(identity_int_flush); + static int mdt_site_stats_seq_show(struct seq_file *m, void *data) { struct obd_device *obd = m->private; @@ -1396,6 +1477,9 @@ static struct attribute *mdt_attrs[] = { &lustre_attr_identity_acquire_expire.attr, &lustre_attr_identity_upcall.attr, &lustre_attr_identity_flush.attr, + &lustre_attr_identity_int_expire.attr, + &lustre_attr_identity_int_acquire_expire.attr, + &lustre_attr_identity_int_flush.attr, &lustre_attr_evict_tgt_nids.attr, &lustre_attr_enable_cap_mask.attr, &lustre_attr_enable_chprojid_gid.attr, diff --git a/lustre/obdclass/upcall_cache.c b/lustre/obdclass/upcall_cache.c index 5b5db34..f96307f 100644 --- a/lustre/obdclass/upcall_cache.c +++ b/lustre/obdclass/upcall_cache.c @@ -152,10 +152,6 @@ int upcall_cache_set_upcall(struct upcall_cache *cache, const char *buffer, snprintf(upcall, count + 1, "NONE"); goto valid; } - if (strcasecmp(upcall, "INTERNAL") == 0) { - snprintf(upcall, count + 1, "INTERNAL"); - goto valid; - } invalid: OBD_FREE(upcall, count + 1); diff --git a/lustre/tests/runas.c b/lustre/tests/runas.c index 3468d1c..a0fb2c9 100644 --- a/lustre/tests/runas.c +++ b/lustre/tests/runas.c @@ -76,7 +76,7 @@ int main(int argc, char **argv) } /* get UID and GID */ - while ((c = getopt(argc, argv, "+u:g:v:j:hG::")) != -1) { + while ((c = getopt(argc, argv, "+u:g:v:j:hG:")) != -1) { switch (c) { case 'u': if (!isdigit(optarg[0])) { diff --git a/lustre/tests/sanity-sec.sh b/lustre/tests/sanity-sec.sh index a31ed1e..004b441 100755 --- a/lustre/tests/sanity-sec.sh +++ b/lustre/tests/sanity-sec.sh @@ -5768,7 +5768,7 @@ test_64a() { done do_facet mgs $LCTL nodemap_modify --name c0 \ - --property rbac --value file_perms + --property rbac --value server_upcall,file_perms wait_nm_sync c0 rbac touch $testfile stack_trap "set +vx" @@ -5779,7 +5779,8 @@ test_64a() { $LFS project -p 1000 $testfile || error "setting project failed" set +vx rm -f $testfile - do_facet mgs $LCTL nodemap_modify --name c0 --property rbac --value none + do_facet mgs $LCTL nodemap_modify --name c0 --property rbac \ + --value server_upcall wait_nm_sync c0 rbac touch $testfile set -vx @@ -5813,7 +5814,7 @@ test_64b() { stack_trap "do_nodes $(comma_list $(all_mdts_nodes)) \ $LCTL set_param mdt.*.enable_dir_restripe=$dir_restripe" EXIT do_facet mgs $LCTL nodemap_modify --name c0 --property rbac \ - --value dne_ops + --value server_upcall,dne_ops wait_nm_sync c0 rbac $LFS mkdir -i 0 ${testdir}_for_migr || error "$LFS mkdir ${testdir}_for_migr failed (1)" @@ -5845,7 +5846,8 @@ test_64b() { $LFS mkdir -i 1 ${testdir}_mdt1 || error "$LFS mkdir ${testdir}_mdt1 failed (2)" - do_facet mgs $LCTL nodemap_modify --name c0 --property rbac --value none + do_facet mgs $LCTL nodemap_modify --name c0 --property rbac \ + --value server_upcall wait_nm_sync c0 rbac set -vx $LFS mkdir -i 1 $testdir && error "$LFS mkdir should fail (1)" @@ -5870,7 +5872,7 @@ test_64c() { setup_64 do_facet mgs $LCTL nodemap_modify --name c0 \ - --property rbac --value quota_ops + --property rbac --value server_upcall,quota_ops wait_nm_sync c0 rbac set -vx $LFS setquota -u $USER0 -b 307200 -B 309200 -i 10000 -I 11000 $MOUNT || @@ -5903,7 +5905,8 @@ test_64c() { $LFS setquota -p 1000 --delete $MOUNT set +vx - do_facet mgs $LCTL nodemap_modify --name c0 --property rbac --value none + do_facet mgs $LCTL nodemap_modify --name c0 --property rbac \ + --value server_upcall wait_nm_sync c0 rbac set -vx @@ -5948,7 +5951,7 @@ test_64d() { setup_64 do_facet mgs $LCTL nodemap_modify --name c0 \ - --property rbac --value byfid_ops + --property rbac --value server_upcall,byfid_ops wait_nm_sync c0 rbac touch $testfile @@ -5959,7 +5962,8 @@ test_64d() { lfs rmfid $MOUNT $fid || error "lfs rmfid failed" set +vx - do_facet mgs $LCTL nodemap_modify --name c0 --property rbac --value none + do_facet mgs $LCTL nodemap_modify --name c0 --property rbac \ + --value server_upcall wait_nm_sync c0 rbac touch $testfile @@ -5996,7 +6000,7 @@ test_64e() { touch $testfile || error "failed to touch $testfile" do_facet mgs $LCTL nodemap_modify --name c0 \ - --property rbac --value chlg_ops + --property rbac --value server_upcall,chlg_ops wait_nm_sync c0 rbac # access changelogs @@ -6007,7 +6011,8 @@ test_64e() { rm -rf $testdir $testfile || error "rm -rf $testdir $testfile failed" - do_facet mgs $LCTL nodemap_modify --name c0 --property rbac --value none + do_facet mgs $LCTL nodemap_modify --name c0 --property rbac \ + --value server_upcall wait_nm_sync c0 rbac # do some IOs @@ -6048,7 +6053,7 @@ test_64f() { # file_perms is required because fscrypt uses chmod/chown do_facet mgs $LCTL nodemap_modify --name c0 --property rbac \ - --value fscrypt_admin,file_perms + --value server_upcall,fscrypt_admin,file_perms wait_nm_sync c0 rbac mkdir -p $vaultdir @@ -6068,7 +6073,7 @@ test_64f() { cancel_lru_locks # file_perms is required because fscrypt uses chmod/chown do_facet mgs $LCTL nodemap_modify --name c0 --property rbac \ - --value file_perms + --value server_upcall,file_perms wait_nm_sync c0 rbac set -vx @@ -6101,6 +6106,46 @@ test_64f() { } run_test 64f "Nodemap enforces fscrypt_admin RBAC roles" +test_64g() { + local testfile=$DIR/$tdir/$tfile + local fid + + (( MDS1_VERSION >= $(version_code 2.14.0.138) )) || + skip "Need MDS >= 2.14.0.138 for role-based controls" + + # Add groups, and client to new group, on client only. + # Server is not aware. + groupadd -g 5000 grptest64g1 + stack_trap "groupdel grptest64g1" EXIT + groupadd -g 5001 grptest64g2 + stack_trap "groupdel grptest64g2" EXIT + groupadd -g 5002 grptest64g3 + stack_trap "groupdel grptest64g3" EXIT + + mkdir -p $DIR/$tdir || error "mkdir $DIR/$tdir failed" + chmod 750 $DIR/$tdir + chgrp grptest64g1 $DIR/$tdir + echo hi > $DIR/$tdir/fileA + chmod 640 $DIR/$tdir/fileA + chgrp grptest64g3 $DIR/$tdir/fileA + setfacl -m g:grptest64g2:r $DIR/$tdir/fileA + ls -lR $DIR/$tdir + + setup_64 + stack_trap cleanup_64 EXIT + + # remove server_upcall from rbac roles, + # to make this client use INTERNAL upcall + do_facet mgs $LCTL nodemap_modify --name c0 \ + --property rbac --value file_perms + wait_nm_sync c0 rbac + + $RUNAS cat $DIR/$tdir/fileA && error "cat $DIR/$tdir/fileA should fail" + $RUNAS -G 5000,5001 cat $DIR/$tdir/fileA || + error "cat $DIR/$tdir/fileA failed" +} +run_test 64g "Nodemap enforces server_upcall RBAC role" + look_for_files() { local pattern=$1 local neg=$2 @@ -6386,22 +6431,15 @@ test_69() { stack_trap "do_facet mds1 $LCTL set_param $param=$orig" EXIT # identity_upcall accepts a path to an executable, - # or NONE (case insensitive) or INTERNAL (case insensitive) + # or NONE (case insensitive) do_facet mds1 $LCTL set_param $param=/path/to/prog || error "set_param $param=/path/to/prog failed" do_facet mds1 $LCTL set_param $param=prog && - error "set_param $param=prog should failed" + error "set_param $param=prog should fail" do_facet mds1 $LCTL set_param $param=NONE || error "set_param $param=NONE failed" do_facet mds1 $LCTL set_param $param=none || error "set_param $param=none failed" - # INTERNAL upcall is not on master yet - if (( MDS1_VERSION < $(version_code 2.15.0) )); then - do_facet mds1 $LCTL set_param $param=INTERNAL || - error "set_param $param=INTERNAL failed" - do_facet mds1 $LCTL set_param $param=internal || - error "set_param $param=internal failed" - fi if $GSS; then param="sptlrpc.gss.rsi_upcall" -- 1.8.3.1