On Thu, 2014-01-16 at 10:47 -0500, Jeff Layton wrote:
> On Wed, 15 Jan 2014 16:41:34 -0500
> Simo Sorce <simo@xxxxxxxxxx> wrote:
>
> > From 421f66b1cd0b031ef843f7680f463027904b93ca Mon Sep 17 00:00:00 2001
> > From: Simo Sorce <simo@xxxxxxxxxx>
> > Date: Wed, 15 Jan 2014 16:01:49 -0500
> > Subject: [PATCH] Improve first attempt at acquiring GSS credentials
> >
> > Since now rpc.gssd is swithing uid before attempting to acquire
> > credentials, we do not need to pass in the special uid-as-a-string name
> > to gssapi, because the process is already running under the user's
> > credentials.
> >
> > By making this optional we can fix a class of false negatives where the
> > user name does not match the actual ccache credentials and the ccache
> > type used is not one of the only 2 supported explicitly by rpc.gssd by the
> > fallback trolling done later.
> >
> > Signed-off-by: Simo Sorce <simo@xxxxxxxxxx>
> > ---
> > utils/gssd/krb5_util.c | 32 ++++++++++++++++++--------------
> > 1 file changed, 18 insertions(+), 14 deletions(-)
> >
> > diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
> > index 697d1d2e79db0cc38160ea4772d3af3a9b7d6c21..7db5baf4e4bea75ed7beebd2103afbc291efb641 100644
> > --- a/utils/gssd/krb5_util.c
> > +++ b/utils/gssd/krb5_util.c
> > @@ -1383,24 +1383,28 @@ gssd_acquire_user_cred(uid_t uid, gss_cred_id_t *gss_cred)
> > {
> > OM_uint32 maj_stat, min_stat;
> > gss_buffer_desc name_buf;
> > - gss_name_t name;
> > + gss_name_t name = GSS_C_NO_NAME;
> > char buf[11];
> > int ret;
> >
> > - ret = snprintf(buf, 11, "%u", uid);
> > - if (ret < 1 || ret > 10) {
> > - return -1;
> > - }
> > - name_buf.value = buf;
> > - name_buf.length = ret + 1;
> > + /* the follwing is useful only if change_identity() in
> > + * process_krb5_upcall() failed to change uids */
> > + if (getuid() == 0) {
> > + ret = snprintf(buf, 11, "%u", uid);
> > + if (ret < 1 || ret > 10) {
> > + return -1;
> > + }
> > + name_buf.value = buf;
> > + name_buf.length = ret + 1;
> >
>
> If change_identity() fails, then process_krb5_upcall should just give
> up and do an error downcall, so falling back to using
> GSS_C_NT_STRING_UID_NAME in that case seems unnecessary.
>
> Also, we can end up in here legitimately with uid == 0 if
> root_uses_machine_creds == 0. So I wonder if we even need the stuff
> inside this "if (getuid() == 0)" block at all...
I were under the impression that rpc.gssd could still be used without
doing the fork()/setuid() dance, and I didn't really check if it really
is conditional.
If it is not and the only case where uid = 0 is when rpc.gssd is
actually performing the operation on behalf of root, then yeah we can
simply remove everything in the if branch.
Let me know how you want to proceed.
> Other than that, I'm fine with ripping that junk out.
Ok, so should I sent a patch that just removes, instead of making
conditional, this chunk of code ?
Simo.
--
Simo Sorce * Red Hat, Inc * New York
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
