Alex Vasylenko
2004-Apr-01 16:31 UTC
[patch] net/rsync: problems in client name lookup code
>Submitter-Id: current-users >Originator: Alex Vasylenko >Organization: >Confidential: no >Synopsis: [patch] net/rsync: problems in client name lookup code >Severity: non-critical >Priority: low >Category: ports >Class: sw-bug >Release: FreeBSD 4.8-RELEASE-p16 i386 >Environment:System: FreeBSD 4.8-RELEASE-p16>Description:rsync does reverse name lookups on a client address in lookup_name (called from client_name) in clientname.c The code in client_name does two things incorrectly: - fails to set ss_len member of struct sockaddr_storage, which when passed to getnameinfo causes the latter to fail; - detection of IPv6 via counting dots is plain silly and doesn't work; The attached patch attempts to address these two issues.>How-To-Repeat:The code in question gets exercised when "hosts allow" specifies a hostname>Fix:--- proto.h.orig Sat Dec 6 16:07:27 2003 +++ proto.h Thu Apr 1 10:54:19 2004 @@ -42,7 +42,6 @@ struct sockaddr_storage *ss, socklen_t *ss_len); int lookup_name(int fd, const struct sockaddr_storage *ss, - socklen_t ss_len, char *name_buf, size_t name_buf_len, char *port_buf, size_t port_buf_len); int compare_addrinfo_sockaddr(const struct addrinfo *ai, --- clientname.c.orig Fri Jan 10 21:05:56 2003 +++ clientname.c Thu Apr 1 10:57:05 2004 @@ -101,58 +101,58 @@ char *client_name(int fd) static char name_buf[100]; static char port_buf[100]; static int initialised; - struct sockaddr_storage ss, *ssp; - struct sockaddr_in sin; -#ifdef INET6 - struct sockaddr_in6 sin6; -#endif - socklen_t ss_len; + struct sockaddr_storage ss; - if (initialised) return name_buf; + if (initialised) + return name_buf; strcpy(name_buf, default_name); initialised = 1; + memset(&ss, 0, sizeof(ss)); + if (am_server) { /* daemon over --rsh mode */ char *addr = client_addr(fd); -#ifdef INET6 - int dots = 0; - char *p; - for (p = addr; *p && (dots <= 3); p++) { - if (*p == '.') - dots++; - } - if (dots > 3) { - /* more than 4 parts to IP address, must be ipv6 */ - ssp = (struct sockaddr_storage *) &sin6; - ss_len = sizeof sin6; - memset(ssp, 0, ss_len); - inet_pton(AF_INET6, addr, &sin6.sin6_addr); - sin6.sin6_family = AF_INET6; - } else -#endif - { - ssp = (struct sockaddr_storage *) &sin; - ss_len = sizeof sin; - memset(ssp, 0, ss_len); - inet_pton(AF_INET, addr, &sin.sin_addr); - sin.sin_family = AF_INET; + struct addrinfo hint; + struct addrinfo *answer; + int err; + + memset(&hint, 0, sizeof(hint)); + + hint.ai_flags = AI_NUMERICHOST; + hint.ai_socktype = SOCK_STREAM; + + err = getaddrinfo(addr, NULL, &hint, &answer); + if (err) { + rprintf(FERROR, RSYNC_NAME ": malformed address %s: %s\n", + addr, gai_strerror(err)); + return name_buf; } + switch (answer->ai_family) { + case AF_INET: + memcpy(&ss, answer->ai_addr, sizeof(struct sockaddr_in)); + break; +#ifdef INET6 + case AF_INET6: + memcpy(&ss, answer->ai_addr, sizeof(struct sockaddr_in6)); + break; + } +#endif + freeaddrinfo(answer); } else { - ss_len = sizeof ss; - ssp = &ss; + socklen_t ss_len = sizeof ss; client_sockaddr(fd, &ss, &ss_len); - + ss.ss_len = ss_len; } - if (!lookup_name(fd, ssp, ss_len, name_buf, sizeof name_buf, + if (!lookup_name(fd, &ss, name_buf, sizeof name_buf, port_buf, sizeof port_buf)) - check_name(fd, ssp, name_buf); + check_name(fd, &ss, name_buf); return name_buf; } @@ -216,14 +216,13 @@ void client_sockaddr(int fd, * @param fd file descriptor for client socket. **/ int lookup_name(int fd, const struct sockaddr_storage *ss, - socklen_t ss_len, char *name_buf, size_t name_buf_len, char *port_buf, size_t port_buf_len) { int name_err; /* reverse lookup */ - name_err = getnameinfo((struct sockaddr *) ss, ss_len, + name_err = getnameinfo((struct sockaddr *) ss, ss->ss_len, name_buf, name_buf_len, port_buf, port_buf_len, NI_NAMEREQD | NI_NUMERICSERV);
Wayne Davison
2004-Apr-01 22:01 UTC
[patch] net/rsync: problems in client name lookup code
On Thu, Apr 01, 2004 at 11:31:00AM -0500, Alex Vasylenko wrote:> - detection of IPv6 via counting dots is plain silly and doesn't work;Thanks, I've tweaked your patch for this a little and checked it in. It's attached to this email.> - fails to set ss_len member of struct sockaddr_storage, which when > passed to getnameinfo causes the latter to fail;Not everyone has ss_len in sockaddr_storage, so this part of the patch doesn't work. It looked like the only place you set the value was after calling client_sockaddr() (though you only affected one of the two places it was called). So, might it be better to set the value inside the client_sockaddr() routine? Attached is my guess at how this might be fixed. This looks odd, though, so I think this is probably not the right fix. Can you (or anyone with access to a system that uses ss_len) let me know if this second patch is what we need, and if not, what the correct fix is. Thanks, ..wayne.. -------------- next part -------------- --- clientname.c 1 Apr 2004 21:08:24 -0000 1.20 +++ clientname.c 1 Apr 2004 21:36:31 -0000 @@ -100,11 +100,7 @@ char *client_name(int fd) static char name_buf[100]; static char port_buf[100]; static int initialised; - struct sockaddr_storage ss, *ssp; - struct sockaddr_in sin; -#ifdef INET6 - struct sockaddr_in6 sin6; -#endif + struct sockaddr_storage ss; socklen_t ss_len; if (initialised) @@ -113,43 +109,45 @@ char *client_name(int fd) strcpy(name_buf, default_name); initialised = 1; + memset(&ss, 0, sizeof ss); + if (am_server) { /* daemon over --rsh mode */ char *addr = client_addr(fd); -#ifdef INET6 - int dots = 0; - char *p; + struct addrinfo hint, *answer; + int err; + + memset(&hint, 0, sizeof hint); - for (p = addr; *p && (dots <= 3); p++) { - if (*p == '.') - dots++; + hint.ai_flags = AI_NUMERICHOST; + hint.ai_socktype = SOCK_STREAM; + + if ((err = getaddrinfo(addr, NULL, &hint, &answer)) != 0) { + rprintf(FERROR, RSYNC_NAME ": malformed address %s: %s\n", + addr, gai_strerror(err)); + return name_buf; } - if (dots > 3) { - /* more than 4 parts to IP address, must be ipv6 */ - ssp = (struct sockaddr_storage *) &sin6; - ss_len = sizeof sin6; - memset(ssp, 0, ss_len); - inet_pton(AF_INET6, addr, &sin6.sin6_addr); - sin6.sin6_family = AF_INET6; - } else + + switch (answer->ai_family) { + case AF_INET: + ss_len = sizeof (struct sockaddr_in); + memcpy(&ss, answer->ai_addr, ss_len); + break; +#ifdef INET6 + case AF_INET6: + ss_len = sizeof (struct sockaddr_in6); + memcpy(&ss, answer->ai_addr, ss_len); + break; #endif - { - ssp = (struct sockaddr_storage *) &sin; - ss_len = sizeof sin; - memset(ssp, 0, ss_len); - inet_pton(AF_INET, addr, &sin.sin_addr); - sin.sin_family = AF_INET; } - + freeaddrinfo(answer); } else { ss_len = sizeof ss; - ssp = &ss; - client_sockaddr(fd, &ss, &ss_len); } - if (!lookup_name(fd, ssp, ss_len, name_buf, sizeof name_buf, + if (!lookup_name(fd, &ss, ss_len, name_buf, sizeof name_buf, port_buf, sizeof port_buf)) - check_name(fd, ssp, name_buf); + check_name(fd, &ss, name_buf); return name_buf; } -------------- next part -------------- --- clientname.c 1 Apr 2004 21:39:35 -0000 1.21 +++ clientname.c 1 Apr 2004 22:00:22 -0000 @@ -190,9 +190,6 @@ void client_sockaddr(int fd, memset(sin, 0, sizeof *sin); sin->sin_family = AF_INET; *ss_len = sizeof (struct sockaddr_in); -#if HAVE_SOCKADDR_IN_LEN - sin->sin_len = *ss_len; -#endif sin->sin_port = sin6.sin6_port; /* There is a macro to extract the mapped part @@ -201,6 +198,10 @@ void client_sockaddr(int fd, memcpy(&sin->sin_addr, &sin6.sin6_addr.s6_addr[12], sizeof sin->sin_addr); } +#endif /* INET6 */ + +#if HAVE_SOCKADDR_IN_LEN + ss->ss_len = *ss_len; #endif }
Maybe Matching Threads
- Name lookup failures and CIDR regression under cygwin ( patch attached )
- DO NOT REPLY [Bug 5851] New: Name lookup failures and CIDR regression
- getaddrinfo() problem with AIX 4.3.3 and rsync 2.5.2?
- [patch] Correct configure test for sin_len to compile on Tru64 Unix
- rsync 2.5.1 server - strange logs