From patchwork Wed Jun 22 15:35:53 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Layton X-Patchwork-Id: 906222 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter2.kernel.org (8.14.4/8.14.4) with ESMTP id p5MFZ0lo021365 for ; Wed, 22 Jun 2011 15:35:58 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757759Ab1FVPf6 (ORCPT ); Wed, 22 Jun 2011 11:35:58 -0400 Received: from mail-qy0-f174.google.com ([209.85.216.174]:57057 "EHLO mail-qy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753924Ab1FVPf5 (ORCPT ); Wed, 22 Jun 2011 11:35:57 -0400 Received: by qyk29 with SMTP id 29so2770943qyk.19 for ; Wed, 22 Jun 2011 08:35:57 -0700 (PDT) Received: by 10.224.204.72 with SMTP id fl8mr698611qab.150.1308756956926; Wed, 22 Jun 2011 08:35:56 -0700 (PDT) Received: from salusa.poochiereds.net (cpe-076-182-054-018.nc.res.rr.com [76.182.54.18]) by mx.google.com with ESMTPS id p10sm495663qcu.25.2011.06.22.08.35.55 (version=SSLv3 cipher=OTHER); Wed, 22 Jun 2011 08:35:56 -0700 (PDT) From: Jeff Layton To: linux-nfs@vger.kernel.org Cc: neilb@suse.de, chuck.lever@oracle.com Subject: [PATCH] nfs: fix host_reliable_addrinfo (try #2) Date: Wed, 22 Jun 2011 11:35:53 -0400 Message-Id: <1308756953-10277-1-git-send-email-jlayton@redhat.com> X-Mailer: git-send-email 1.7.5.4 Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter2.kernel.org [140.211.167.43]); Wed, 22 Jun 2011 15:35:58 +0000 (UTC) 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. This bug is exploitable via the following scenario and could allow an attacker access to data that they shouldn't be able to access. Suppose you export a filesystem to some subnet or FQDN and also to a wildcard or netgroup, and I know the details of this (maybe showmount -e tells me) Suppose further that I can get IP packets to your server.. Then I create a reverse mapping for my ipaddress to a domain that I own, say "black.hat.org", and a forward mapping from that domain to my IP address, and one of your IP addresses. Then I try to mount your filesystem. The IP address gets correctly mapped to "black.hat.org" and then mapped to both my IP address and your IP address. Then you search through all of your exports and find that one of the addresses: yours - is allowed to access the filesystem. So you create an export based on the addrinfo you have which allows my IP address the same access as your IP address. 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 Reviewed-by: NeilBrown --- support/export/hostname.c | 36 ++++++++++++++++++++++++++++++------ 1 files changed, 30 insertions(+), 6 deletions(-) diff --git a/support/export/hostname.c b/support/export/hostname.c index efcb75c..3e949a1 100644 --- a/support/export/hostname.c +++ b/support/export/hostname.c @@ -258,17 +258,19 @@ host_canonname(const struct sockaddr *sap) * @sap: pointer to socket address to look up * * Reverse and forward lookups are performed to ensure the address has - * proper forward and reverse mappings. + * matching forward and reverse mappings. * - * Returns address info structure with ai_canonname filled in, or NULL - * if no information is available for @sap. Caller must free the returned - * structure with freeaddrinfo(3). + * Returns addrinfo structure with just the provided address with + * ai_canonname filled in. If there is a problem with resolution or + * the resolved records don't match up properly then it returns NULL + * + * Caller must free the returned structure with freeaddrinfo(3). */ __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 +278,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; } /**