| Message ID | 1308745999-6551-1-git-send-email-jlayton@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@redhat.com> > Reviewed-by: NeilBrown <neilb@suse.de> > --- > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; } /**
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. 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. 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@redhat.com> Reviewed-by: NeilBrown <neilb@suse.de> --- support/export/hostname.c | 26 ++++++++++++++++++++++++-- 1 files changed, 24 insertions(+), 2 deletions(-)