Date: Tue, 8 Apr 2014 08:35:01 -0400
To: Jeff Layton <jlayton@redhat.com>
Cc: linux-nfs@vger.kernel.org, Andy Adamson <androsadamson@gmail.com>,
        chuck.lever@oracle.com, trond.myklebust@primarydata.com
Subject: Re: v4.0 CB_COMPOUND authentication failures
Message-ID: <20140408123501.GA3532@fieldses.org>
References: <20140408082140.340c1328@tlielax.poochiereds.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
In-Reply-To: <20140408082140.340c1328@tlielax.poochiereds.net>
From: "J. Bruce Fields" <bfields@fieldses.org>
Sender: linux-nfs-owner@vger.kernel.org

On Tue, Apr 08, 2014 at 08:21:40AM -0400, Jeff Layton wrote:
> I've recently been hunting down some problems with delegation handling
> and have run across a problem with the client authenticates CB_COMPOUND
> requests. I could use some advice on how best to fix it.
> 
> Specifically, check_gss_callback_principal() tries to look up the
> callback client and then tries to compare the ticket in it against the
> clp->cl_hostname:
> 
>         /* Expect a GSS_C_NT_HOSTBASED_NAME like "nfs@serverhostname" */
> 
>         if (memcmp(p, "nfs@", 4) != 0)
>                 return 0;
>         p += 4;
>         if (strcmp(p, clp->cl_hostname) != 0)
>                 return 0;
>         return 1;
> 
> The problem is that there is no guarantee that those hostnames will be
> the same. If, for instance, I mount "foo:/" and the SPN is
> "nfs/foo.bar.baz" that strcmp will return true, and the CB_COMPOUND
> request will get tossed out [1]. Ditto if I happen to mount a CNAME of the
> server.

It sounds like a bug to me that the mount is succeeding without the name
matching.

The security provided by krb5 is much weaker if we don't check that the
name provided on the commandline matches what the server authenticates
as.

> Now that we try to use krb5 on the callback channel even when sec=sys
> is specified, this is very problematic.

And similarly I think the attempt to opportunistically use krb5 for
state management should fail and fall back on auth_sys if the server's
name doesn't match.

> I think that the ideal thing would be to stash the SPN that we use to
> do the SETCLIENTID call and use that in the comparison above.
> Unfortunately, the rpc_cred doesn't really seem to carry this info and
> I don't see where we get enough information in the rpc.gssd downcall to
> figure out what that SPN should be.
> 
> Anyone have thoughts or should we just remove the above check until we
> come up with a better way to do this?
> 
> [1]: there's another bug that can cause the client to send a bogus
>      reply instead of dropping the request as intended, but that's
>      relatively simple to fix.

So I believe the matching really is a requirement and that it would be
wrong to weaken it.

It sounds like there's also a server bug here if it's giving out
delegations to a client that isn't responding to callbacks.

--b.