On Tue, May 22, 2018 at 4:08 PM, Chuck Lever <chuck.lever@xxxxxxxxxx> wrote:
>
>
>> On May 22, 2018, at 1:03 PM, Olga Kornievskaia <aglo@xxxxxxxxx> wrote:
>>
>> I'm looking for comments on the approach to deal with the following
>> denial-of-service issue.
>>
>> Currently, during the nfs4.0 mount, the code takes the content
>> supplied by the user in the mount command for "clientaddr" and that
>> becomes part of the content of the SETCLIENTID client id. There are no
>> verifications that the supplied address belongs to the client
>> initiating the mount.
>>
>> A denial of services comes from where there are 2 clients with IP A
>> and IP B (bad one). Client IP A mounts and has "IP A" in the
>> SETCLIENTID. Client IP B does a mount and specified "clientaddr=IP A".
>> This causes the server to invalidate the lease for the legitimate
>> client IP A.
>
> Generally if this is a concern, Kerberos can be used during
> the SETCLIENTID to mutually authenticate the client and
> server. Shouldn't that prevent client B from tampering with
> client A's lease?
It turns out to be a concern by folks (customers) that are using the
code. Kerberos does not help here. Client IP B can have a valid
Kerberos identity and still supply "clientaddr=" not belonging to it
for the SETCLIENTID and interfere with the other's lease.
>> My suggested approach to fixing it, is to have nfs-utils do a sanity
>> checking that will check if the clientaddr that's suppose matches the
>> IP of the machine. Then currently, if it doesn't then it will ignore
>> the supplied value and use the IP of the machine. Whether this is
>> desirable vs say failing the mount and forcing the user to specify the
>> correct value is up for debate. Also, I'm not sure if the check for
>> the value of clientaddr should be done in the kernel itself instead of
>> the nfs-utils.
>>
>> Below is the rough fix to the nfs-utils. Please comment.
>
> One thing we want to be able to continue to do is specify
> "clientaddr=0.0.0.0" to disable NFSv4.0 callback (and thus
> delegation).
Ok so some extra logic to check for the special value?
>
>
>> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
>> index 1217823..982927e 100644
>> --- a/utils/mount/stropts.c
>> +++ b/utils/mount/stropts.c
>> @@ -242,11 +242,21 @@ static int nfs_append_clientaddr_option(const
>> struct sockaddr *sap,
>> struct sockaddr *my_addr = &address.sa;
>> socklen_t my_len = sizeof(address);
>>
>> - if (po_contains(options, "clientaddr") == PO_FOUND)
>> - return 1;
>> -
>> nfs_callback_address(sap, salen, my_addr, &my_len);
>>
>> + if (po_contains(options, "clientaddr") == PO_FOUND) {
>> + char *addr = po_get(options, "clientaddr");
>> + char address[NI_MAXHOST];
>> +
>> + if (!nfs_present_sockaddr(my_addr, my_len, address,
>> + sizeof(address)))
>> + goto out;
>> +
>> + if (strcmp(addr, address))
>> + goto out;
>> + return 1;
>> + }
>> +out:
>> return nfs_append_generic_address_option(my_addr, my_len,
>> "clientaddr", options);
>> }
>> --
>> 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
