Hi Jeff-
On Jun 22, 2011, at 6:33 AM, Jeff Layton wrote:
> According to Neil Brown:
>
> The point of the word 'reliable' is to check that the name we get
> really does belong to the host in question - ie that both the
> forward and reverse maps agree.
Should you correct the documenting comment for host_reliable_addrinfo() to reflect Neil's insight?
> But the new code doesn't do that check at all. Rather it simply
> maps the address to a name, then discards the address and maps the
> name back to a list of addresses and uses that list of addresses as
> "where the request came from" for permission checking.
The patch description might also mention why this is a problem (ie, what's our exposure due to the current behavior).
> Fix this by instead using the forward lookup of the hostname just to
> verify that the original address is in the list. Then do a numeric
> lookup using the address and stick the hostname in the ai_canonname.
>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> Reviewed-by: NeilBrown <neilb@xxxxxxx>
> ---
> support/export/hostname.c | 26 ++++++++++++++++++++++++--
> 1 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/support/export/hostname.c b/support/export/hostname.c
> index efcb75c..595333e 100644
> --- a/support/export/hostname.c
> +++ b/support/export/hostname.c
> @@ -268,7 +268,7 @@ __attribute_malloc__
> struct addrinfo *
> host_reliable_addrinfo(const struct sockaddr *sap)
> {
> - struct addrinfo *ai;
> + struct addrinfo *ai, *a;
> char *hostname;
>
> hostname = host_canonname(sap);
> @@ -276,9 +276,31 @@ host_reliable_addrinfo(const struct sockaddr *sap)
> return NULL;
>
> ai = host_addrinfo(hostname);
> + if (!ai)
> + goto out_free_hostname;
>
> - free(hostname);
> + /* make sure there's a matching address in the list */
> + for (a = ai; a; a = a->ai_next)
> + if (nfs_compare_sockaddr(a->ai_addr, sap))
> + break;
> +
> + freeaddrinfo(ai);
> + if (!a)
> + goto out_free_hostname;
> +
> + /* get addrinfo with just the original address */
> + ai = host_numeric_addrinfo(sap);
> + if (!ai)
> + goto out_free_hostname;
> +
> + /* and populate its ai_canonname field */
> + free(ai->ai_canonname);
> + ai->ai_canonname = hostname;
> return ai;
> +
> +out_free_hostname:
> + free(hostname);
> + return NULL;
> }
>
> /**
> --
> 1.7.5.4
>
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
--
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
