From 08b80d5abda7d7fb93862d960e2a7363df46c53d Mon Sep 17 00:00:00 2001 From: Vladislav Grishenko Date: Thu, 2 May 2019 15:28:52 +0500 Subject: [PATCH] miniupnpd: fix ssdp notify on unrelated interfaces If several different interfaces share same ipv4 address on different subnets (i.e. eth0 192.168.1.1/24 + eth1 192.168.1.1/16), miniupnpd may pick any one of them, possibly wrong one w/o respecting exact listening_ip interface. syslog will contain something similar to: miniupnpd: sendto(udp_notify=6, 192.168.1.1): No such device miniupnpd: sendto(udp_notify=6, 192.168.1.1): No such device miniupnpd: try_sendto(sock=6, len=464, dest=239.255.255.250:1900): sendto: No such device miniupnpd: try_sendto(sock=6, len=464, dest=239.255.255.250:1900): sendto: No such device miniupnpd: try_sendto failed to send 11 packets Fix that with specifying exact outgoing mcast interface for each notify socket with help of IP_MULTICAST_IF/mreqn struct. Since OpenAndConfSSDPNotifySocket() now takes lan_addr_s struct, OpenAndConfSSDPNotifySocketIPv6() was similary changed for api consistency. --- miniupnpd/Changelog.txt | 3 +++ miniupnpd/minissdp.c | 35 ++++++++++++++++++++++------------- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/miniupnpd/Changelog.txt b/miniupnpd/Changelog.txt index ddf83be..16f6b31 100644 --- a/miniupnpd/Changelog.txt +++ b/miniupnpd/Changelog.txt @@ -1,5 +1,8 @@ $Id: Changelog.txt,v 1.446 2019/04/09 20:04:32 nanard Exp $ +2019/05/02: + Fix ssdp notify on unrelated interfaces + 2019/04/09: Fix buffer over-read in upnpevents.c with urls in the form http://ip:port diff --git a/miniupnpd/minissdp.c b/miniupnpd/minissdp.c index 7b4dde2..f63dfc1 100644 --- a/miniupnpd/minissdp.c +++ b/miniupnpd/minissdp.c @@ -83,11 +83,7 @@ AddMulticastMembership(int s, struct lan_addr_s * lan_addr) #endif /* MULTIPLE_EXTERNAL_IP */ #endif /* HAVE_IP_MREQN */ -#ifndef HAVE_IP_MREQN - if (setsockopt(s, IPPROTO_IP, IP_ADD_MEMBERSHIP, (void *)&imr, sizeof(struct ip_mreq)) < 0) -#else /* HAVE_IP_MREQN */ - if (setsockopt(s, IPPROTO_IP, IP_ADD_MEMBERSHIP, (void *)&imr, sizeof(struct ip_mreqn)) < 0) -#endif /* HAVE_IP_MREQN */ + if (setsockopt(s, IPPROTO_IP, IP_ADD_MEMBERSHIP, (void *)&imr, sizeof(imr)) < 0) { syslog(LOG_ERR, "setsockopt(udp, IP_ADD_MEMBERSHIP): %m"); return -1; @@ -289,7 +285,7 @@ OpenAndConfSSDPReceiveSocket(int ipv6) /* open the UDP socket used to send SSDP notifications to * the multicast group reserved for them */ static int -OpenAndConfSSDPNotifySocket(in_addr_t addr) +OpenAndConfSSDPNotifySocket(struct lan_addr_s * lan_addr) { int s; unsigned char loopchar = 0; @@ -298,7 +294,11 @@ OpenAndConfSSDPNotifySocket(in_addr_t addr) The TTL for the IP packet SHOULD default to 2 and SHOULD be configurable. */ /* TODO: Make TTL be configurable */ +#ifndef HAVE_IP_MREQN struct in_addr mc_if; +#else /* HAVE_IP_MREQN */ + struct ip_mreqn mc_if; +#endif /* HAVE_IP_MREQN */ struct sockaddr_in sockname; if( (s = socket(PF_INET, SOCK_DGRAM, 0)) < 0) @@ -307,7 +307,16 @@ OpenAndConfSSDPNotifySocket(in_addr_t addr) return -1; } - mc_if.s_addr = addr; /*inet_addr(addr);*/ +#ifndef HAVE_IP_MREQN + mc_if.s_addr = lan_addr->addr.s_addr; /*inet_addr(addr);*/ +#else /* HAVE_IP_MREQN */ + mc_if.imr_address.s_addr = lan_addr->addr.s_addr; /*inet_addr(addr);*/ +#ifdef ENABLE_IPV6 + mc_if.imr_ifindex = lan_addr->index; +#else /* ENABLE_IPV6 */ + mc_if.imr_ifindex = if_nametoindex(lan_addr->ifname); +#endif /* ENABLE_IPV6 */ +#endif /* HAVE_IP_MREQN */ if(setsockopt(s, IPPROTO_IP, IP_MULTICAST_LOOP, (char *)&loopchar, sizeof(loopchar)) < 0) { @@ -341,7 +350,7 @@ OpenAndConfSSDPNotifySocket(in_addr_t addr) * here it is used to se a specific sending address */ memset(&sockname, 0, sizeof(struct sockaddr_in)); sockname.sin_family = AF_INET; - sockname.sin_addr.s_addr = addr; /*inet_addr(addr);*/ + sockname.sin_addr.s_addr = lan_addr->addr.s_addr; /*inet_addr(addr);*/ if (bind(s, (struct sockaddr *)&sockname, sizeof(struct sockaddr_in)) < 0) { @@ -357,7 +366,7 @@ OpenAndConfSSDPNotifySocket(in_addr_t addr) /* open the UDP socket used to send SSDP notifications to * the multicast group reserved for them. IPv6 */ static int -OpenAndConfSSDPNotifySocketIPv6(unsigned int if_index) +OpenAndConfSSDPNotifySocketIPv6(struct lan_addr_s * lan_addr) { int s; unsigned int loop = 0; @@ -372,9 +381,9 @@ OpenAndConfSSDPNotifySocketIPv6(unsigned int if_index) syslog(LOG_ERR, "socket(udp_notify IPv6): %m"); return -1; } - if(setsockopt(s, IPPROTO_IPV6, IPV6_MULTICAST_IF, &if_index, sizeof(if_index)) < 0) + if(setsockopt(s, IPPROTO_IPV6, IPV6_MULTICAST_IF, &lan_addr->index, sizeof(lan_addr->index)) < 0) { - syslog(LOG_ERR, "setsockopt(udp_notify IPv6, IPV6_MULTICAST_IF, %u): %m", if_index); + syslog(LOG_ERR, "setsockopt(udp_notify IPv6, IPV6_MULTICAST_IF, %u): %m", lan_addr->index); close(s); return -1; } @@ -423,7 +432,7 @@ OpenAndConfSSDPNotifySockets(int * sockets) lan_addr != NULL; lan_addr = lan_addr->list.le_next) { - sockets[i] = OpenAndConfSSDPNotifySocket(lan_addr->addr.s_addr); + sockets[i] = OpenAndConfSSDPNotifySocket(lan_addr); if(sockets[i] < 0) goto error; i++; @@ -434,7 +443,7 @@ OpenAndConfSSDPNotifySockets(int * sockets) } else { - sockets[i] = OpenAndConfSSDPNotifySocketIPv6(lan_addr->index); + sockets[i] = OpenAndConfSSDPNotifySocketIPv6(lan_addr); if(sockets[i] < 0) goto error; }