> On May 25, 2018, at 9:47 AM, Olga Kornievskaia <aglo@xxxxxxxxx> wrote: > > On Fri, May 25, 2018 at 12:44 PM, Olga Kornievskaia <aglo@xxxxxxxxx> wrote: >> On Fri, May 25, 2018 at 12:24 PM, Chuck Lever <chuck.lever@xxxxxxxxxx> wrote: >>> Hi Olga- >>> >>>> On May 25, 2018, at 7:02 AM, Olga Kornievskaia <aglo@xxxxxxxxx> wrote: >>>> >>>> Thank you for the comments. Will hopefully address them in the next version. >>>> >>>> On Thu, May 24, 2018 at 8:50 PM, Chuck Lever <chuck.lever@xxxxxxxxxx> wrote: >>>>> Hi Olga- >>>>> >>>>>> On May 24, 2018, at 1:05 PM, Olga Kornievskaia <kolga@xxxxxxxxxx> wrote: >>>>>> >>>>>> If the user supplies a clientaddr value, >>>>> >>>>> Please say "NFS client administrator" not "user". A >>>>> "user" is any non-privileged user, so saying that a >>>>> "user" can set this value is misleading. >>>> >>>> Ok will change it. >>>> >>>>>> it should be either >>>>>> a special value of either IPV4/IPV6 any address or a local address >>>>>> on the same network that the server being mounted. >>>>> >>>>> This option should allow any local address the client has, >>>>> not just an address that is on the same network as the >>>>> server. See below for further explanation. >>>> >>>> Ok, I added this to the comment specifically as I didn't know if this >>>> would pose a problem. I didn't know if allowing any address was useful >>>> as when it's not specified the address on the same network as the >>>> server is chosen. >>> >>> Yep, any of the client's local addresses should be allowed. >>> >>> >>>>>> Otherwise, we >>>>>> disallow the client to use an arbitrary value of the clientaddr value. >>>>>> This value is used to construct a client id of SETCLIENTID and >>>>>> providing a false value can interfere with the real owner's mount. >>>>> >>>>> The patch description is misleading: >>>>> >>>>> Interference occurs only if the real owner's lease is >>>>> not protected by Kerberos AND this client has the same >>>>> client ID string as another client. >>>> >>>> Ok I will add this more explicit detail when the interference occurs >>>> (when neither of the machines are using Kerberos and the other client >>>> machine is not using a module parameter to add a unique identifier to >>>> the client ID string). I think otherwise it is knowns that client ID >>>> is created with the value of the clientaddr. >>> >>> The only way a problem occurs is if the clientaddr is the >>> same AND the cl_nodename is the same. How is that happening? >> >> Client ID in the SETCLIENTID is constructed by >> nfs4_init_nonuniform_client_string() function and it uses cl_ipaddr >> and not cl_nodename. >> >>> Why are the cl_nodenames the same? If they are not the same, >>> then it is not possible that the clients' leases are inter- >>> fering with each other, and something else is going on. >>> >>> >>>>> The Linux client's client ID string also contains the >>>>> system's cl_nodename. Both the cl_nodename and the >>>>> callback address have to be the same as some other >>>>> client's, and they both have to be Linux, for this to >>>>> be a problem. >>>>> >>>>> It's more likely that the customer's clients are all >>>>> named the same (maybe they are copied from the same >>>>> system image), and reverse DNS lookup is giving them >>>>> all the same clientaddr= . That's an unsupported >>>>> configuration and there are already ways to address >>>>> this. >>>>> >>>>> Or perhaps I don't understand the use case that is >>>>> causing the problem. Can the patch description explain >>>>> why your customer is trying to set clientaddr= ? >>>> >>>> The customer case was a simple mistake of including the wrong address. >>> >>> But that doesn't answer the question. Why did the >>> customer feel the need to set clientaddr= ? >> >> I don't know. In the end they decided they didn't need the clientaddr at all. >> >>>> Do you fundamentally disagree that there is a case for >>>> denial-of-service here? >>> >>> The only service that is affected if the clientaddr is > > Forgot to address this. This is definitely not true. Both clients are > effected as they end up "sharing" the lease. So each client once it > gets a lease error has to go and renegotiate the lease and can perhaps > get an operation in before the other client will send something which > would then get and error and it would deals with the lease error. My claim is correct for the uniform client ID string. I did not realize that the non-uniform client ID string is not well-constructed. >>> set incorrectly is on the client where the mistake >>> occurs. If the cl_nodenames are all unique then other >>> clients should not be affected by the mistake. If >>> that is happening, that's a server bug. >>> >>> If the problem was that the customer set the wrong >>> address, let's say that, rather than claiming that the >>> patch prevents lease tampering. >> >> Ok I can change it to lease tampering (I really don't care that much). >> But to just to discuss a bit further, how's lease tampering not a >> denial-of-service? It interfere with a client's ability to make >> progress. >> >>> >>> >>> -- >>> Chuck Lever -- 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
