miniupnpd: fix eport selection and error handling
The find_available_eport function that was intended to check if at least one eport is allowed for a given iaddr/iport does not work as intended; for example, it does not properly handle rule precedence (i.e., it considers allow rules even if they are effectively masked by earlier deny rules), and it also does not handle the case where no rules are specified at all (which should default to accept in order to be consistent with check_upnp_rule_against_permissions). The present change removes this function and instead integrates the check into the existing while loop that iterates over all eports.
This commit is contained in:
parent
ecf414e160
commit
f4f4573f53
|
@ -266,29 +266,31 @@ void ProcessIncomingNATPMPPacket(int s, unsigned char *msg_buff, int len,
|
|||
} else if(iport==0) {
|
||||
resp[3] = 2; /* Not Authorized/Refused */
|
||||
} else { /* iport > 0 && lifetime > 0 */
|
||||
unsigned short eport_first;
|
||||
unsigned short eport_first = 0;
|
||||
int any_eport_allowed = 0;
|
||||
char desc[64];
|
||||
if(!check_upnp_rule_against_permissions(upnppermlist, num_upnpperm, eport, senderaddr->sin_addr, iport)) {
|
||||
/* if the mapping is forbidden because of eport only
|
||||
* (ie iaddr/iport are ok with another eport)
|
||||
* change eport value ! */
|
||||
if(!find_allowed_eport(upnppermlist, num_upnpperm, senderaddr->sin_addr, iport, &eport)) {
|
||||
/* no rule allow a mapping with this iaddr/iport */
|
||||
resp[3] = 2; /* Not Authorized/Refused */
|
||||
}
|
||||
}
|
||||
eport_first = eport;
|
||||
while(resp[3] == 0) {
|
||||
if(!check_upnp_rule_against_permissions(upnppermlist, num_upnpperm, eport, senderaddr->sin_addr, iport)) {
|
||||
eport++;
|
||||
if(eport == 0) eport++; /* skip port zero */
|
||||
if(eport == eport_first) { /* no external port available */
|
||||
if(eport_first == 0) { /* first time in loop */
|
||||
eport_first = eport;
|
||||
} else if(eport == eport_first) { /* no eport available */
|
||||
if(any_eport_allowed == 0) { /* all eports rejected by permissions */
|
||||
syslog(LOG_ERR, "No allowed eport for NAT-PMP %hu %s->%s:%hu",
|
||||
eport, (proto==IPPROTO_TCP)?"tcp":"udp", senderaddrstr, iport);
|
||||
resp[3] = 2; /* Not Authorized/Refused */
|
||||
} else { /* at least one eport allowed (but none available) */
|
||||
syslog(LOG_ERR, "Failed to find available eport for NAT-PMP %hu %s->%s:%hu",
|
||||
eport, (proto==IPPROTO_TCP)?"tcp":"udp", senderaddrstr, iport);
|
||||
resp[3] = 4; /* Out of resources */
|
||||
resp[3] = 4; /* Out of resources */
|
||||
}
|
||||
break;
|
||||
}
|
||||
if(!check_upnp_rule_against_permissions(upnppermlist, num_upnpperm, eport, senderaddr->sin_addr, iport)) {
|
||||
eport++;
|
||||
if(eport == 0) eport++; /* skip port zero */
|
||||
continue;
|
||||
}
|
||||
any_eport_allowed = 1;
|
||||
r = get_redirect_rule(ext_if_name, eport, proto,
|
||||
iaddr_old, sizeof(iaddr_old),
|
||||
&iport_old, 0, 0, 0, 0,
|
||||
|
@ -306,12 +308,7 @@ void ProcessIncomingNATPMPPacket(int s, unsigned char *msg_buff, int len,
|
|||
}
|
||||
} else {
|
||||
eport++;
|
||||
if(eport == 0) eport++; /* skip port zero */
|
||||
if(eport == eport_first) { /* no external port available */
|
||||
syslog(LOG_ERR, "Failed to find available eport for NAT-PMP %hu %s->%s:%hu",
|
||||
eport, (proto==IPPROTO_TCP)?"tcp":"udp", senderaddrstr, iport);
|
||||
resp[3] = 4; /* Out of resources */
|
||||
}
|
||||
if(eport == 0) eport++; /* skip port zero */
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -260,21 +260,3 @@ check_upnp_rule_against_permissions(const struct upnpperm * permary,
|
|||
return 1; /* Default : accept */
|
||||
}
|
||||
|
||||
int
|
||||
find_allowed_eport(const struct upnpperm * permary,
|
||||
int n_perms,
|
||||
struct in_addr address, u_short iport,
|
||||
u_short *allowed_eport)
|
||||
{
|
||||
int i;
|
||||
for(i=0; i<n_perms; i++)
|
||||
{
|
||||
if(permary[i].type == UPNPPERM_ALLOW
|
||||
&& match_permission_internal(permary + i, address, iport)) {
|
||||
*allowed_eport = permary[i].eport_min;
|
||||
return 1;
|
||||
}
|
||||
}
|
||||
return 0; /* no eport found */
|
||||
}
|
||||
|
||||
|
|
|
@ -45,15 +45,6 @@ check_upnp_rule_against_permissions(const struct upnpperm * permary,
|
|||
u_short eport, struct in_addr address,
|
||||
u_short iport);
|
||||
|
||||
/* find_allowed_eport()
|
||||
* returns: 0 if no allowed eport for (address, iport) was found
|
||||
* 1 and allowed_eport filled */
|
||||
int
|
||||
find_allowed_eport(const struct upnpperm * permary,
|
||||
int n_perms,
|
||||
struct in_addr address, u_short iport,
|
||||
u_short *allowed_eport);
|
||||
|
||||
#ifdef USE_MINIUPNPDCTL
|
||||
void
|
||||
write_permlist(int fd, const struct upnpperm * permary,
|
||||
|
|
Loading…
Reference in New Issue