On Fri, May 18, 2018 at 3:23 PM, Chuck Lever <chuck.lever@xxxxxxxxxx> wrote: > > >> On May 18, 2018, at 2:53 PM, Olga Kornievskaia <aglo@xxxxxxxxx> wrote: >> >> Hi Chuck, >> >> I'm not convinced that "srchost=" is necessary. I believe that >> everything that is needed is suppose to be encoded in the "target=" >> option. > > I don't believe target= has what we want. For NFSv4.0 callback: > > A. The callback source principal needs to be the same as the client's > NFS server target principal. Correct and by specifying "nfs" and "*" I think that signals to the gssd to pick the NFS server principal (which is what the client would used as the target for the client to server authentication). > B. The callback target principal needs to be the same as the client's > NFS server source principal. > > Today, for NFSv4.0 callbacks, the kernel passes to gssd: > > target= it uses B for this > > service= it always sets this to "nfs" > > And gssd "makes up" the hostname part of A using gethostname(3), which > is bogus for multi-homed NFS servers. I agree that's bogus. Instead we should grab the domain from the "target=" string. > So my patch series does the following for NFSv4.0 callbacks: > > 1. It acquires the actual target principal the client used to establish > it's lease. This is A above. > > 2. Instead of always passing "nfs" as the service= value, it passes > the non-hostname part of A. That should be "nfs" but it doesn't > have to be. Ok so yes it has historically have always been "nfs" (even thought it's only RECOMMENDED by the spec) and I don't think this is the case to add complexity to change a well established behavior? > 3. Instead of letting gssd make up the hostname part of the source > principal, it passes the hostname part of A. I'm arguing that hostname part of A should have been a part of the "target=" > > That should provide the correct source principal in the multi-home > case, and it should be backwards-compatible with older gssd's. > > > Now here's why it's not enough to use getsockname(3) on target= > to predict the correct source hostname: > > If the client has established the lease when mounting from one > particular hostname, and then mounts from another, it will have > to continue managing the lease using the principal it has already > established with the first hostname. NFSD knows what that is, and > can tell gssd what it needs to be. gssd, using only getsockname(3), > would probably pick something that is wrong. > > >> I thought target just needed to correctly identify the domain for >> which authentication is taking place. > > That's what srchost= does. We can call it something else if that > helps. > > >> Then I think more changes should >> be in nfs-utils to make sure that we find credentials for that >> particular domain instead of going by the gethostbyname() results. > > That's a patch to gssd that I didn't post. I will post that later if > we all agree srchost= is OK. > > srchost= is optional. If it's not present in the upcall, gssd will > continue to use the result of gethostname(3) to construct the source > principal. > > >> On Fri, May 18, 2018 at 11:39 AM, Chuck Lever <chuck.lever@xxxxxxxxxx> wrote: >>> I've been experimenting with this series that modifies NFSD to >>> discover and use the correct GSS service principal when constructing >>> its NFSv4.0 callback channels. I'm interested in review of this >>> approach. There are a couple of code comments marked with XXX that >>> also need some attention. >>> >>> The rpc.gssd change mentioned in 1/4 is unremarkable and will be >>> made available once there is consensus about the kernel changes >>> in this series. No gssproxy changes are necessary. >>> >>> --- >>> >>> Chuck Lever (4): >>> sunrpc: Enable the kernel to specify the hostname part of service principals >>> sunrpc: Extract target name into svc_cred >>> nfsd: Use correct credential for NFSv4.0 callback with GSS >>> nfsd: Remove callback_cred >>> >>> >>> fs/nfsd/nfs4callback.c | 29 ++++---------- >>> fs/nfsd/nfs4state.c | 17 +++----- >>> fs/nfsd/state.h | 2 - >>> include/linux/sunrpc/svcauth.h | 3 + >>> net/sunrpc/auth_gss/auth_gss.c | 20 ++++++++-- >>> net/sunrpc/auth_gss/gss_rpc_upcall.c | 70 ++++++++++++++++++++++------------ >>> 6 files changed, 80 insertions(+), 61 deletions(-) >>> >>> -- >>> Chuck Lever >>> -- >>> 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 >> -- >> 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 > > -- > Chuck Lever > > > -- 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
