On Fri, May 18, 2018 at 4:11 PM, Olga Kornievskaia <aglo@xxxxxxxxx> wrote: > 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. >> I think the srchost is needed only if we are going to deal with doing something cross-DNS domains so say if host@xxxxxxxxxxxxxxxxxxxxxxx will authenticate to nfs@xxxxxxxxxxxxxxxxxxxxxxxxx, then the target's domain would not be sufficient to match which source domain name to use. So if this the use case, then sorry I just had to think out-loud to get here, then we do need srcdomain (and it's really a domain isn't it and not host that's of interest?). >> >>> 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
