LU-17518 gss: do not trust supp groups from client with krb

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.

Test-Parameters: kerberos=true testlist=sanity-krb5
Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
Change-Id: I4113ef654492e76fcd377b2c0cc74e484b27850b
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/53987
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Aurelien Degremont <adegremont@nvidia.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>

diff --git a/lustre/include/lustre_idmap.h b/lustre/include/lustre_idmap.h
index 5ba270f..1f986ed 100644 (file)
--- a/lustre/include/lustre_idmap.h
+++ b/lustre/include/lustre_idmap.h
@@ -64,7 +64,9 @@ struct lu_ucred;
 
 extern void lustre_groups_from_list(struct group_info *ginfo, gid_t *glist);
 extern void lustre_groups_sort(struct group_info *group_info);
+extern int lustre_groups_search(struct group_info *group_info, gid_t grp);
 extern int lustre_in_group_p(struct lu_ucred *mu, gid_t grp);
+extern int has_proper_groups(struct lu_ucred *ucred);
 
 /** @} idmap */
 

diff --git a/lustre/mdt/mdt_lib.c b/lustre/mdt/mdt_lib.c
index ffb950e..f69d19f 100644 (file)
--- a/lustre/mdt/mdt_lib.c
+++ b/lustre/mdt/mdt_lib.c
@@ -296,10 +296,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_nidstr(&peernid));
+                      ucred->uc_identity ? ucred->uc_identity->mi_gid : -1,
+                      libcfs_nidstr(&peernid));
                GOTO(out, rc = -EACCES);
        }
 
@@ -359,6 +361,26 @@ static int new_init_ucred(struct mdt_thread_info *info, ucred_init_type_t type,
                                              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);
@@ -499,9 +521,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)
@@ -550,6 +573,27 @@ static int old_init_ucred_common(struct mdt_thread_info *info,
                               libcfs_nidstr(&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);
@@ -557,7 +601,6 @@ static int old_init_ucred_common(struct mdt_thread_info *info,
        ucred_set_rbac_roles(info, uc);
 
        EXIT;
-
        return 0;
 }
 

diff --git a/lustre/obdclass/idmap.c b/lustre/obdclass/idmap.c
index 0a5b593..dab099b 100644 (file)
--- a/lustre/obdclass/idmap.c
+++ b/lustre/obdclass/idmap.c
@@ -49,8 +49,7 @@
  * groups_search() is copied from linux kernel!
  * A simple bsearch.
  */
-static int lustre_groups_search(struct group_info *group_info,
-                               gid_t grp)
+int lustre_groups_search(struct group_info *group_info, gid_t grp)
 {
        int left, right;
 
@@ -73,6 +72,7 @@ static int lustre_groups_search(struct group_info *group_info,
        }
        return 0;
 }
+EXPORT_SYMBOL(lustre_groups_search);
 
 void lustre_groups_from_list(struct group_info *ginfo, gid_t *glist)
 {
@@ -158,3 +158,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);

diff --git a/lustre/tests/sanity-krb5.sh b/lustre/tests/sanity-krb5.sh
index 6ce9c6b..fea6e38 100755 (executable)
--- a/lustre/tests/sanity-krb5.sh
+++ b/lustre/tests/sanity-krb5.sh
@@ -459,6 +459,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.