Whamcloud - gitweb
LU-17518 gss: do not trust supp groups from client with krb
authorSebastien Buisson <sbuisson@ddn.com>
Fri, 9 Feb 2024 15:42:40 +0000 (16:42 +0100)
committerAndreas Dilger <adilger@whamcloud.com>
Sat, 27 Apr 2024 22:28:39 +0000 (22:28 +0000)
Thanks to Kerberos, Lustre does not have to trust clients anymore,
but relies on keytabs and tickets, cryptographically validated, to
recognize clients and users.
RPC provided supplementary groups should not be trusted, but checked
thanks to identity upcall and the trusted UID from the ticket.

Add sanity-krb5 test_9 to exercise this.

Lustre-change: https://review.whamcloud.com/53987
Lustre-commit: b09f56c208c6c34375d098f66075688f329b7c76

Test-Parameters: kerberos=true testlist=sanity-krb5 serverdistro=el8.8
Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
Change-Id: I4113ef654492e76fcd377b2c0cc74e484b27850b
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Aurelien Degremont <adegremont@nvidia.com>
Reviewed-on: https://review.whamcloud.com/c/ex/lustre-release/+/54801
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
lustre/include/lustre_idmap.h
lustre/mdt/mdt_lib.c
lustre/obdclass/idmap.c
lustre/tests/sanity-krb5.sh

index e90dc2a..8551673 100644 (file)
@@ -67,6 +67,7 @@ extern void lustre_groups_from_list(struct group_info *ginfo, gid_t *glist);
 extern void lustre_list_from_groups(gid_t *glist, struct group_info *ginfo);
 extern void lustre_groups_sort(struct group_info *group_info);
 extern int lustre_in_group_p(struct lu_ucred *mu, gid_t grp);
+extern int has_proper_groups(struct lu_ucred *ucred);
 
 /** @} idmap */
 
index a5916e2..4dce873 100644 (file)
@@ -295,10 +295,12 @@ static int new_init_ucred(struct mdt_thread_info *info, ucred_init_type_t type,
 
        /* check permission of setgid */
        if (setgid && !(perm & CFS_SETGID_PERM)) {
-               CDEBUG(D_SEC, "mdt blocked setgid attempt (%u:%u/%u:%u -> %u) "
-                      "from %s\n", pud->pud_uid, pud->pud_gid,
+               CDEBUG(D_SEC,
+                      "mdt blocked setgid attempt (%u:%u/%u:%u -> %d) from %s\n",
+                      pud->pud_uid, pud->pud_gid,
                       pud->pud_fsuid, pud->pud_fsgid,
-                      ucred->uc_identity->mi_gid, libcfs_nid2str(peernid));
+                      ucred->uc_identity ? ucred->uc_identity->mi_gid : -1,
+                      libcfs_nid2str(peernid));
                GOTO(out, rc = -EACCES);
        }
 
@@ -357,6 +359,27 @@ static int new_init_ucred(struct mdt_thread_info *info, ucred_init_type_t type,
                ucred->uc_cap = cap_intersect(ucred->uc_cap,
                                              mdt->mdt_enable_cap_mask);
        }
+
+       /* Thanks to Kerberos, Lustre does not have to trust clients anymore,
+        * but relies on keytabs and tickets, cryptographically validated, to
+        * recognize clients and users.
+        * RPC provided primary group should match the one got from the
+        * identity upcall.
+        * And RPC provided supplementary groups should not be trusted,
+        * but checked thanks to identity upcall and the trusted UID
+        * from the ticket.
+        */
+       if (SPTLRPC_FLVR_MECH(req->rq_flvr.sf_rpc) == SPTLRPC_MECH_GSS_KRB5) {
+               if (!has_proper_groups(ucred))
+                       GOTO(out, rc = -EACCES);
+               ucred->uc_suppgids[0] = -1;
+               ucred->uc_suppgids[1] = -1;
+               if (ucred->uc_ginfo) {
+                       put_group_info(ucred->uc_ginfo);
+                       ucred->uc_ginfo = NULL;
+               }
+       }
+
        ucred->uc_valid = UCRED_NEW;
        ucred_set_jobid(info, ucred);
        ucred_set_nid(info, ucred);
@@ -496,9 +519,10 @@ out:
 static int old_init_ucred_common(struct mdt_thread_info *info,
                                 struct lu_nodemap *nodemap)
 {
-       struct lu_ucred         *uc = mdt_ucred(info);
-       struct mdt_device       *mdt = info->mti_mdt;
-       struct md_identity      *identity = NULL;
+       struct ptlrpc_request *req = mdt_info_req(info);
+       struct lu_ucred *uc = mdt_ucred(info);
+       struct mdt_device *mdt = info->mti_mdt;
+       struct md_identity *identity = NULL;
 
        if (nodemap && uc->uc_o_uid == nodemap->nm_squash_uid
            && nodemap->nmf_deny_unknown)
@@ -546,6 +570,27 @@ static int old_init_ucred_common(struct mdt_thread_info *info,
                               libcfs_nid2str(mdt_info_req(info)->rq_peer.nid));
                uc->uc_cap = cap_intersect(uc->uc_cap,mdt->mdt_enable_cap_mask);
        }
+
+       /* Thanks to Kerberos, Lustre does not have to trust clients anymore,
+        * but relies on keytabs and tickets, cryptographically validated, to
+        * recognize clients and users.
+        * RPC provided primary group should match the one got from the
+        * identity upcall.
+        * And RPC provided supplementary groups should not be trusted,
+        * but checked thanks to identity upcall and the trusted UID
+        * from the ticket.
+        */
+       if (SPTLRPC_FLVR_MECH(req->rq_flvr.sf_rpc) == SPTLRPC_MECH_GSS_KRB5) {
+               if (!has_proper_groups(uc)) {
+                       mdt_identity_put(mdt->mdt_identity_cache,
+                                        uc->uc_identity);
+                       uc->uc_identity = NULL;
+                       RETURN(-EACCES);
+               }
+               uc->uc_suppgids[0] = -1;
+               uc->uc_suppgids[1] = -1;
+       }
+
        uc->uc_valid = UCRED_OLD;
        ucred_set_jobid(info, uc);
        ucred_set_nid(info, uc);
@@ -553,7 +598,6 @@ static int old_init_ucred_common(struct mdt_thread_info *info,
        ucred_set_rbac_roles(info, uc);
 
        EXIT;
-
        return 0;
 }
 
index 1c9af54..646bb5e 100644 (file)
@@ -180,3 +180,29 @@ int lustre_in_group_p(struct lu_ucred *mu, gid_t grp)
        return rc;
 }
 EXPORT_SYMBOL(lustre_in_group_p);
+
+/* make sure fsgid is one of primary or supplementary groups
+ * fetched from identity upcall
+ */
+int has_proper_groups(struct lu_ucred *ucred)
+{
+       struct group_info *group_info = NULL;
+       int rc;
+
+       if (!ucred->uc_identity)
+               return 1;
+
+       if (ucred->uc_fsgid == ucred->uc_identity->mi_gid)
+               return 1;
+
+       group_info = ucred->uc_identity->mi_ginfo;
+       if (!group_info)
+               return 0;
+
+       get_group_info(group_info);
+       rc = lustre_groups_search(group_info, ucred->uc_fsgid);
+       put_group_info(group_info);
+
+       return rc;
+}
+EXPORT_SYMBOL(has_proper_groups);
index e008ad0..8e2a34a 100755 (executable)
@@ -503,6 +503,57 @@ test_8()
 }
 run_test 8 "Early reply sent for slow gss context negotiation"
 
+test_9() {
+       local test9user=$(getent passwd $RUNAS_ID | cut -d: -f1)
+
+       $LFS mkdir -i 0 -c 1 $DIR/$tdir || error "mkdir $DIR/$tdir failed"
+       chmod 0777 $DIR/$tdir || error "chmod $DIR/$tdir failed"
+       $RUNAS ls -ld $DIR/$tdir
+
+       # Add group, and client to new group, on client only.
+       # Server is not aware.
+       groupadd -g 5000 grptest9
+       stack_trap "groupdel grptest9" EXIT
+
+       usermod -g grptest9 $test9user
+       stack_trap "usermod -g $test9user $test9user" EXIT
+       id $RUNAS_ID
+       # Thanks to Kerberos, client should not be able to create file
+       # with primary group not known on server side
+       $RUNAS touch $DIR/$tdir/fileA &&
+               error "server should not trust client's primary gid"
+       do_facet mds1 "lctl set_param mdt.*.identity_flush=-1"
+
+       do_facet mds1 groupadd -g 5000 grptest9
+       stack_trap "do_facet mds1 groupdel grptest9 || true" EXIT
+       do_facet mds1 usermod -a -G grptest9 $test9user
+       stack_trap "do_facet mds1 gpasswd -d $test9user grptest9 || true" EXIT
+       id $RUNAS_ID
+       do_facet mds1 "id $RUNAS_ID"
+       # Thanks to Kerberos, client should be able to create file
+       # with primary group taken as one of supp groups, as long as
+       # server side knows the supp groups.
+       $RUNAS touch $DIR/$tdir/fileA ||
+               error "server should know client's supp gid"
+       ls -l $DIR/$tdir
+       do_facet mds1 "lctl set_param mdt.*.identity_flush=-1"
+       do_facet mds1 gpasswd -d $test9user grptest9
+       do_facet mds1 groupdel grptest9
+       usermod -g $test9user $test9user
+
+       usermod -a -G grptest9 $test9user
+       stack_trap "gpasswd -d $test9user grptest9" EXIT
+       id $RUNAS_ID
+       $RUNAS touch $DIR/$tdir/fileB
+       ls -l $DIR/$tdir
+       # Thanks to Kerberos, client should not be able to chgrp
+       $RUNAS chgrp grptest9 $DIR/$tdir/fileB &&
+               error "server should not trust client's supp gid"
+       ls -l $DIR/$tdir
+       do_facet mds1 "lctl set_param mdt.*.identity_flush=-1"
+}
+run_test 9 "Do not trust primary and supp gids from client"
+
 #
 # following tests will manipulate flavors and may end with any flavor set,
 # so each test should not assume any start flavor.