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. > 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. > 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. 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= ? > Signed-off-by: Olga Kornievskaia <kolga@xxxxxxxxxx> > --- > utils/mount/stropts.c | 24 ++++++++++++++++++++---- > 1 file changed, 20 insertions(+), 4 deletions(-) > > diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c > index d1b0708..44a6ff5 100644 > --- a/utils/mount/stropts.c > +++ b/utils/mount/stropts.c > @@ -229,7 +229,8 @@ static int nfs_append_addr_option(const struct sockaddr *sap, > > /* > * Called to discover our address and append an appropriate 'clientaddr=' > - * option to the options string. > + * option to the options string. If the supplied 'clientaddr=' value does > + * not match either IPV4/IPv6 any or a local address, then fail the mount. > * > * Returns 1 if 'clientaddr=' option created successfully or if > * 'clientaddr=' option is already present; otherwise zero. > @@ -242,11 +243,26 @@ 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 (!strcmp(addr, "0.0.0.0") || !strcmp(addr, "::")) > + return 1; IN6ADDR_ANY can be spelled in other ways than "::". Please don't compare presentation address strings. First step is to convert the clientaddr= value to an nfs_sockaddr. If that cannot be done, then clearly this is not a valid mount option. > + if (!nfs_present_sockaddr(my_addr, my_len, address, > + sizeof(address))) > + goto out; > + > + if (strcmp(addr, address)) { > + nfs_error(_("%s: failed to validate clientaddr " > + "address"), progname); > + return 0; > + } This needs to check whether the supplied address is a local address on _any_ of the client's interfaces. That's the point of allowing an admin to set clientaddr= ... sometimes the automatic setting (which comes from nfs_callback_address) is wrong. Think of a multi-homed client, or a client with public and private interfaces, or a weird firewall configuration. This check will break all those cases. So here, use a reliable mechanism for determining whether the passed-in clientaddr= value specifies the address of one of the local interfaces on the client. > + return 1; > + } > +out: > return nfs_append_generic_address_option(my_addr, my_len, > "clientaddr", options); > } -- 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