>Number:         27047
>Category:       bin
>Synopsis:       "host" restriction feature of supfilesrv is broken
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Sep 27 09:59:00 UTC 2004
>Closed-Date:
>Last-Modified:
>Originator:     Noriyuki Soda
>Release:        NetBSD 2.0_BETA
>Organization:
The NetBSD Project
>Environment:
System: NetBSD www.netbsd.org 2.0_BETA NetBSD 2.0_BETA (NBMAIL) #0: Wed Sep 22 21:06:38 UTC 2004 tls@ADMIN:/usr/src/sys/arch/i386/compile/NBMAIL i386
Architecture: i386
Machine: i386

>Description:

/usr/sbin/supfilesrv has a feature to restrict clients by
using a file <base-directory>/sup/<collection>/host.
This file consists of a list of hosts in each line.
The host in each line can be either numeric address or hostname.

But since revision 1.11 of usr.sbin/sup/source/scm.c,
hostname becomes not allowed in the "host" file, because matchhost()
function in scm.c specifies AI_NUMERICHOST.
This breaks our configuration on www.netbsd.org.

Also, matchhost() since revision 1.11 uses the result of getnameinfo()
to see whether the client is allowed to access or not. Doesn't this
has security risk, if the client provides forged PTR record for its
address? Or, getnameinfo() is guaranteed that it doesn't return
forged name?

>How-To-Repeat:

Try to run stock supfilesrv on www.netbsd.org, and see that all clients
are rejected with the mesage "supfile[21912]: Permission denied" 
in /var/log/messages.

>Fix:

I'm not sure whether it's correct or not about the comparison of 
sin6_scope_id at sockaddrsamehost() in the following patch, though.

Index: scm.c
===================================================================
RCS file: /cvsroot/src/usr.sbin/sup/source/scm.c,v
retrieving revision 1.17
diff -c -r1.17 scm.c
*** scm.c	3 Apr 2003 17:15:22 -0000	1.17
--- scm.c	27 Sep 2004 09:32:14 -0000
***************
*** 607,649 ****
  	return (0);
  }
  
! int 
! matchhost(char *name)
! {				/* is this name of remote host? */
! 	char h1[NI_MAXHOST], h2[NI_MAXHOST];
! #ifdef NI_WITHSCOPEID
! 	const int niflags = NI_NUMERICHOST | NI_WITHSCOPEID;
! #else
! 	const int niflags = NI_NUMERICHOST;
! #endif
! 	struct addrinfo hints, *res0, *res;
! 
! 	if (getnameinfo((struct sockaddr *) & remoteaddr,
  #ifdef BSD4_4
! 	    remoteaddr.ss_len,
! #else
! 	    sizeof(struct sockaddr),
! #endif
! 	    h1, sizeof(h1), NULL, 0, niflags))
  		return (0);
! 	memset(&hints, 0, sizeof(hints));
! 	hints.ai_family = PF_UNSPEC;
! 	hints.ai_socktype = SOCK_DGRAM;	/* dummy */
! 	hints.ai_flags = AI_NUMERICHOST;
! 	if (getaddrinfo(name, "0", &hints, &res0) != 0)
  		return (0);
  	for (res = res0; res; res = res->ai_next) {
  		if (remoteaddr.ss_family != res->ai_family)
  			continue;
! 		if (getnameinfo(res->ai_addr, res->ai_addrlen,
! 		    h2, sizeof(h2), NULL, 0, niflags))
  			continue;
! 		if (strcmp(h1, h2) == 0) {
! 			freeaddrinfo(res0);
  			return (1);
  		}
  	}
! 	freeaddrinfo(res0);
  	return (0);
  }
  
--- 607,684 ----
  	return (0);
  }
  
! int
! sockaddrsamehost(struct sockaddr *sa1, struct sockaddr *sa2)
! {
! 	if (sa1->sa_family != sa2->sa_family)
! 		return (0);
  #ifdef BSD4_4
! 	if (sa1->sa_len != sa2->sa_len)
  		return (0);
! #endif
! 	switch (sa1->sa_family) {
! 	case AF_INET:
! 		return (memcmp(
! 		    &((struct sockaddr_in *)sa1)->sin_addr,
! 		    &((struct sockaddr_in *)sa2)->sin_addr,
! 		    sizeof(((struct sockaddr_in *)sa1)->sin_addr)) == 0);
! 	case AF_INET6:
! 		if (((struct sockaddr_in6 *)sa1)->sin6_scope_id != 
! 		    ((struct sockaddr_in6 *)sa2)->sin6_scope_id)
! 			return (0);
! 		return (memcmp(
! 		    &((struct sockaddr_in6 *)sa1)->sin6_addr,
! 		    &((struct sockaddr_in6 *)sa2)->sin6_addr,
! 		    sizeof(((struct sockaddr_in6 *)sa1)->sin6_addr)) == 0);
! 	default: /* XXX We don't support other protocol families for now */
  		return (0);
+ 	}
+ }
+ 
+ int
+ matchaddrinfo(struct addrinfo *res0)
+ {
+ 	struct addrinfo hints, *res;
+ 
  	for (res = res0; res; res = res->ai_next) {
  		if (remoteaddr.ss_family != res->ai_family)
  			continue;
! #ifdef BSD4_4
! 		if (remoteaddr.ss_len != res->ai_addrlen)
  			continue;
! #endif
! 		if (sockaddrsamehost((struct sockaddr *)&remoteaddr,
! 		    res->ai_addr)) {
  			return (1);
  		}
  	}
! 	return (0);
! }
! 
! int 
! matchhost(char *name)
! {				/* is this name of remote host? */
! 	struct addrinfo hints, *res0;
! 	int rv;
! 
! 	memset(&hints, 0, sizeof(hints));
! 	hints.ai_family = PF_UNSPEC;
! 	hints.ai_socktype = SOCK_DGRAM;	/* dummy */
! 	hints.ai_flags = AI_NUMERICHOST;
! 	if (getaddrinfo(name, "0", &hints, &res0) == 0) {
! 		rv = matchaddrinfo(res0);
! 		freeaddrinfo(res0);
! 		return (rv);
! 	}
! 
! 	memset(&hints, 0, sizeof(hints));
! 	hints.ai_family = PF_UNSPEC;
! 	hints.ai_socktype = SOCK_DGRAM;	/* dummy */
! 	if (getaddrinfo(name, "0", &hints, &res0) == 0) {
! 		rv = matchaddrinfo(res0);
! 		freeaddrinfo(res0);
! 		return (rv);
! 	}
  	return (0);
  }
  
>Release-Note:
>Audit-Trail:
>Unformatted: