Re: [PATCH 1/1] nfs-utils: Add check of clientaddr argument

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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?
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= ?


> Do you fundamentally disagree that there is a case for
> denial-of-service here?

The only service that is affected if the clientaddr is
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.


--
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




[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux