From: aland Date: Fri, 11 Dec 2009 13:48:45 +0000 (+0100) Subject: Fix weird issue where it would stop proxying packets. X-Git-Tag: release_2_1_8~23 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5c01d4544ca2da41864e00529b213a9ef4d8867a;p=thirdparty%2Ffreeradius-server.git Fix weird issue where it would stop proxying packets. If you had 4 servers, 1 of which was up and 3 down, it would mark the 3 servers "dead", and then *erroneosly* mark the "live" server as dead, too. The symptom was that the home server would respond, but the proxy would never see the response. With some test configs from Bjorn Mork, this was tracked down to b5b1ffbc7. Since the "stable" branch didn't have this problem, the solution was to revert packet_list_alloc() to the pre-b5b... code, and then apply selective changes from the "stable" branch --- diff --git a/src/lib/packet.c b/src/lib/packet.c index 7937878e235..ffb4e4c2491 100644 --- a/src/lib/packet.c +++ b/src/lib/packet.c @@ -154,6 +154,30 @@ int fr_packet_cmp(const RADIUS_PACKET *a, const RADIUS_PACKET *b) } +static int fr_inaddr_any(fr_ipaddr_t *ipaddr) +{ + + if (ipaddr->af == AF_INET) { + if (ipaddr->ipaddr.ip4addr.s_addr == INADDR_ANY) { + return 1; + } + +#ifdef HAVE_STRUCT_SOCKADDR_IN6 + } else if (ipaddr->af == AF_INET6) { + if (IN6_IS_ADDR_UNSPECIFIED(&(ipaddr->ipaddr.ip6addr))) { + return 1; + } +#endif + + } else { + fr_strerror_printf("Unknown address family"); + return -1; + } + + return 0; +} + + /* * Create a fake "request" from a reply, for later lookup. */ @@ -632,12 +656,38 @@ int fr_packet_list_id_alloc(fr_packet_list_t *pl, RADIUS_PACKET *request) { int i, id, start, fd; + int src_any = 0; uint32_t free_mask; fr_packet_dst2id_t my_pd, *pd; fr_packet_socket_t *ps; if (!pl || !pl->alloc_id || !request) return 0; + /* + * Error out if no destination is specified. + */ + if ((request->dst_ipaddr.af == AF_UNSPEC) || + (request->dst_port == 0)) { + fr_strerror_printf("No destination address/port specified"); + return 0; + } + + /* + * Special case: unspec == "don't care" + */ + if (request->src_ipaddr.af == AF_UNSPEC) { + memset(&request->src_ipaddr, 0, sizeof(request->src_ipaddr)); + request->src_ipaddr.af = request->dst_ipaddr.af; + } + + src_any = fr_inaddr_any(&request->src_ipaddr); + if (src_any < 0) return 0; + + /* + * MUST specify a destination address. + */ + if (fr_inaddr_any(&request->dst_ipaddr) != 0) return 0; + my_pd.dst_ipaddr = request->dst_ipaddr; my_pd.dst_port = request->dst_port; @@ -679,48 +729,37 @@ int fr_packet_list_id_alloc(fr_packet_list_t *pl, free_mask = ~((~pd->id[id]) & pl->mask); - /* - * This ID has at least one socket free. Check the sockets - * to see if they are satisfactory for the caller. - */ - fd = -1; + start = -1; for (i = 0; i < MAX_SOCKETS; i++) { if (pl->sockets[i].sockfd == -1) continue; /* paranoia */ + ps = &(pl->sockets[i]); + /* - * This ID is allocated. - */ - if ((free_mask & (1 << i)) != 0) continue; - - /* - * If the caller cares about the source address, - * try to re-use that. This means that the - * requested source address is set, AND this - * socket wasn't bound to "*", AND the requested - * source address is the same as this socket - * address. + * We're sourcing from *, and they asked for a + * specific source address: ignore it. */ - if ((request->src_ipaddr.af != AF_UNSPEC) && - !pl->sockets[i].inaddr_any && - (fr_ipaddr_cmp(&request->src_ipaddr, &pl->sockets[i].ipaddr) != 0)) continue; + if (ps->inaddr_any && !src_any) continue; /* - * They asked for a specific address, and this socket - * is bound to a wildcard address. Ignore this one, too. + * We're sourcing from a specific IP, and they + * asked for a source IP that isn't us: ignore + * it. */ - if ((request->src_ipaddr.af != AF_UNSPEC) && - pl->sockets[i].inaddr_any) continue; - - fd = i; - break; - } + if (!ps->inaddr_any && !src_any && + (fr_ipaddr_cmp(&request->src_ipaddr, + &ps->ipaddr) != 0)) continue; - if (fd < 0) { - goto redo; /* keep searching IDs */ + if ((free_mask & (1 << i)) == 0) { + start = i; + break; + } } - pd->id[id] |= (1 << fd); - ps = &pl->sockets[fd]; + if (start < 0) return 0; /* bad error */ + + pd->id[id] |= (1 << start); + ps = &pl->sockets[start]; ps->num_outgoing++; pl->num_outgoing++;