- processed RPZ review feedback

- fix potential locking issue
  - add extra out of bound checks
This commit is contained in:
Ralph Dolmans 2020-01-15 22:45:29 +01:00
parent e098285892
commit 14913d75c0
6 changed files with 58 additions and 39 deletions

View file

@ -529,7 +529,7 @@ rpz_insert_qname_trigger(struct rpz* r, uint8_t* dname, size_t dnamelen,
/** Insert RR into RPZ's respip_set */
static int
rpz_insert_response_ip_trigger(struct rpz* r, uint8_t* dname,
rpz_insert_response_ip_trigger(struct rpz* r, uint8_t* dname, size_t dnamelen,
enum rpz_action a, uint16_t rrtype, uint16_t rrclass, uint32_t ttl,
uint8_t* rdata, size_t rdata_len, uint8_t* rr, size_t rr_len)
{
@ -547,7 +547,7 @@ rpz_insert_response_ip_trigger(struct rpz* r, uint8_t* dname,
return 0;
}
if(!netblockdnametoaddr(dname, &addr, &addrlen, &net, &af))
if(!netblockdnametoaddr(dname, dnamelen, &addr, &addrlen, &net, &af))
return 0;
lock_rw_wrlock(&r->respip_set->lock);
@ -599,8 +599,6 @@ rpz_insert_rr(struct rpz* r, size_t aznamelen, uint8_t* dname,
return 0;
}
t = rpz_dname_to_trigger(policydname, policydnamelen);
verbose(VERB_OPS, "RPZ: found trigger: %s",
rpz_trigger_to_string(t));
if(t == RPZ_INVALID_TRIGGER) {
free(policydname);
verbose(VERB_ALGO, "RPZ: skipping invalid trigger");
@ -612,7 +610,7 @@ rpz_insert_rr(struct rpz* r, size_t aznamelen, uint8_t* dname,
rr_len);
}
else if(t == RPZ_RESPONSE_IP_TRIGGER) {
rpz_insert_response_ip_trigger(r, policydname,
rpz_insert_response_ip_trigger(r, policydname, policydnamelen,
a, rr_type, rr_class, rr_ttl, rdatawl, rdatalen, rr,
rr_len);
free(policydname);
@ -633,11 +631,13 @@ rpz_insert_rr(struct rpz* r, size_t aznamelen, uint8_t* dname,
* @param qclass: qclass
* @param only_exact: if 1 only excact (non wildcard) matches are returned
* @param wr: get write lock for local-zone if 1, read lock if 0
* @param zones_keep_lock: if set do not release the r->local_zones lock, this
* makes the caller of this function responsible for releasing the lock.
* @return: NULL or local-zone holding rd or wr lock
*/
static struct local_zone*
rpz_find_zone(struct rpz* r, uint8_t* qname, size_t qname_len, uint16_t qclass,
int only_exact, int wr)
int only_exact, int wr, int zones_keep_lock)
{
uint8_t* ce;
size_t ce_len, ce_labs;
@ -661,7 +661,9 @@ rpz_find_zone(struct rpz* r, uint8_t* qname, size_t qname_len, uint16_t qclass,
} else {
lock_rw_rdlock(&z->lock);
}
lock_rw_unlock(&r->local_zones->lock);
if(!zones_keep_lock) {
lock_rw_unlock(&r->local_zones->lock);
}
if(exact)
return z;
@ -685,10 +687,12 @@ rpz_find_zone(struct rpz* r, uint8_t* qname, size_t qname_len, uint16_t qclass,
memmove(wc+2, ce, ce_len);
lock_rw_unlock(&z->lock);
if(wr) {
lock_rw_wrlock(&r->local_zones->lock);
} else {
lock_rw_rdlock(&r->local_zones->lock);
if(!zones_keep_lock) {
if(wr) {
lock_rw_wrlock(&r->local_zones->lock);
} else {
lock_rw_rdlock(&r->local_zones->lock);
}
}
z = local_zones_find_le(r->local_zones, wc,
ce_len+2, ce_labs+1, qclass, &exact);
@ -701,7 +705,9 @@ rpz_find_zone(struct rpz* r, uint8_t* qname, size_t qname_len, uint16_t qclass,
} else {
lock_rw_rdlock(&z->lock);
}
lock_rw_unlock(&r->local_zones->lock);
if(!zones_keep_lock) {
lock_rw_unlock(&r->local_zones->lock);
}
return z;
}
@ -739,7 +745,6 @@ rpz_data_delete_rr(struct local_zone* z, uint8_t* policydname,
/* no memory recycling for zone deletions ... */
if(prev) prev->next = p->next;
else ld->rrsets = p->next;
}
if(d->count > 1) {
if(!local_rrset_remove_rr(d, index))
@ -797,7 +802,7 @@ rpz_remove_qname_trigger(struct rpz* r, uint8_t* dname, size_t dnamelen,
struct local_zone* z;
int delete_zone = 1;
z = rpz_find_zone(r, dname, dnamelen, rr_class,
1 /* only exact */, 1 /* wr lock */);
1 /* only exact */, 1 /* wr lock */, 1 /* keep lock*/);
if(!z) {
verbose(VERB_ALGO, "RPZ: cannot remove RR from IXFR, "
"RPZ domain not found");
@ -813,12 +818,13 @@ rpz_remove_qname_trigger(struct rpz* r, uint8_t* dname, size_t dnamelen,
if(delete_zone) {
local_zones_del_zone(r->local_zones, z);
}
lock_rw_unlock(&r->local_zones->lock);
return;
}
static void
rpz_remove_response_ip_trigger(struct rpz* r, uint8_t* dname, enum rpz_action a,
uint16_t rr_type, uint8_t* rdatawl, size_t rdatalen)
rpz_remove_response_ip_trigger(struct rpz* r, uint8_t* dname, size_t dnamelen,
enum rpz_action a, uint16_t rr_type, uint8_t* rdatawl, size_t rdatalen)
{
struct resp_addr* node;
struct sockaddr_storage addr;
@ -826,7 +832,7 @@ rpz_remove_response_ip_trigger(struct rpz* r, uint8_t* dname, enum rpz_action a,
int net, af;
int delete_respip = 1;
if(!netblockdnametoaddr(dname, &addr, &addrlen, &net, &af))
if(!netblockdnametoaddr(dname, dnamelen, &addr, &addrlen, &net, &af))
return;
lock_rw_wrlock(&r->respip_set->lock);
@ -877,8 +883,8 @@ rpz_remove_rr(struct rpz* r, size_t aznamelen, uint8_t* dname, size_t dnamelen,
rpz_remove_qname_trigger(r, policydname, policydnamelen, a,
rr_type, rr_class, rdatawl, rdatalen);
} else if(t == RPZ_RESPONSE_IP_TRIGGER) {
rpz_remove_response_ip_trigger(r, policydname, a, rr_type,
rdatawl, rdatalen);
rpz_remove_response_ip_trigger(r, policydname, policydnamelen,
a, rr_type, rdatawl, rdatalen);
}
free(policydname);
}
@ -921,7 +927,7 @@ rpz_apply_qname_trigger(struct auth_zones* az, struct module_env* env,
if(!r->taglist || taglist_intersect(r->taglist,
r->taglistlen, taglist, taglen)) {
z = rpz_find_zone(r, qinfo->qname, qinfo->qname_len,
qinfo->qclass, 0, 0);
qinfo->qclass, 0, 0, 0);
if(z && r->action_override == RPZ_DISABLED_ACTION) {
if(r->log)
log_rpz_apply(z->name,

View file

@ -547,14 +547,14 @@ dname_lab_startswith(uint8_t* label, char* prefix, char** endptr)
}
int
dname_has_label(uint8_t* dname, uint8_t* label)
dname_has_label(uint8_t* dname, size_t dnamelen, uint8_t* label)
{
uint8_t lablen = *dname;
while(lablen) {
if(lablen == *label && memlowercmp(dname, label, lablen) == 0)
int len = *dname;
while(*dname && len <= dnamelen) {
if(*dname == *label && memlowercmp(dname, label, *dname) == 0)
return 1;
dname += lablen;
lablen = *dname;
len += *dname;
dname += *dname;
}
return 0;
}

View file

@ -199,10 +199,11 @@ int dname_lab_startswith(uint8_t* label, char* prefix, char** endptr);
/**
* Check if dname contains label
* @param dname: dname
* @param dnamelen: length of dname
* @param label: label to be checked for presence in dname
* @return: 1 if dname has this label, 0 otherwise
*/
int dname_has_label(uint8_t* dname, uint8_t* label);
int dname_has_label(uint8_t* dname, size_t dnamelen, uint8_t* label);
/**
* See if domain name d1 is a strict subdomain of d2.

View file

@ -448,6 +448,7 @@ struct ub_packed_rrset_key* packed_rrset_copy_alloc(
/**
* Find RR index in packed rrset
* Raw comparison, does not canonicalize RDATA
* @param d: packed rrset
* @param rdata: RDATA of RR to find
* @param len: length of rdata

View file

@ -285,19 +285,22 @@ int netblockstrtoaddr(const char* str, int port, struct sockaddr_storage* addr,
}
/* RPZ format address dname to network byte order address */
static int ipdnametoaddr(uint8_t* dname, struct sockaddr_storage* addr,
socklen_t* addrlen, int* af)
static int ipdnametoaddr(uint8_t* dname, size_t dnamelen,
struct sockaddr_storage* addr, socklen_t* addrlen, int* af)
{
uint8_t* ia;
size_t dnamelabs = dname_count_labels(dname);
uint8_t lablen;
char* e = NULL;
int z = 0;
int len = 0;
int i;
*af = AF_INET;
if(dnamelabs > 6 || dname_has_label(dname, (uint8_t*)"\002zz")) {
if(dnamelabs > 6 ||
dname_has_label(dname, dnamelen, (uint8_t*)"\002zz")) {
*af = AF_INET6;
}
len = *dname;
lablen = *dname++;
i = (*af == AF_INET) ? 3 : 15;
if(*af == AF_INET6) {
@ -313,21 +316,21 @@ static int ipdnametoaddr(uint8_t* dname, struct sockaddr_storage* addr,
sa->sin_family = AF_INET;
ia = (uint8_t*)&sa->sin_addr;
}
while(lablen && i >= 0) {
char buff[lablen+1];
while(lablen && i >= 0 && len <= dnamelen) {
char buff[LDNS_MAX_LABELLEN+1];
uint16_t chunk; /* big enough to not overflow on IPv6 hextet */
if((*af == AF_INET && (lablen > 3 || dnamelabs > 6)) ||
(*af == AF_INET6 && (lablen > 4 || dnamelabs > 10))) {
return 0;
}
if(memcmp(dname, "zz", 2) == 0 && *af == AF_INET6) {
/* add one or more 0 labels */
/* Add one or more 0 labels. Address is initialised at
* 0, so just skip the zero part. */
int zl = 11 - dnamelabs;
if(z || zl < 0)
return 0;
z = 1;
i -= (zl*2);
memset(ia+(i+1), 0, zl*2);
} else {
memcpy(buff, dname, lablen);
buff[lablen] = '\0';
@ -335,9 +338,11 @@ static int ipdnametoaddr(uint8_t* dname, struct sockaddr_storage* addr,
if(!e || *e != '\0' || (*af == AF_INET && chunk > 255))
return 0;
if(*af == AF_INET) {
log_assert(i < 4 && i >= 0);
ia[i] = (uint8_t)chunk;
i--;
} else {
log_assert(i < 15 && i >= 1);
/* ia in network byte order */
ia[i-1] = (uint8_t)(chunk >> 8);
ia[i] = (uint8_t)(chunk & 0x00FF);
@ -346,6 +351,7 @@ static int ipdnametoaddr(uint8_t* dname, struct sockaddr_storage* addr,
}
dname += lablen;
lablen = *dname++;
len += lablen;
}
if(i != -1)
/* input too short */
@ -353,8 +359,8 @@ static int ipdnametoaddr(uint8_t* dname, struct sockaddr_storage* addr,
return 1;
}
int netblockdnametoaddr(uint8_t* dname, struct sockaddr_storage* addr,
socklen_t* addrlen, int* net, int* af)
int netblockdnametoaddr(uint8_t* dname, size_t dnamelen,
struct sockaddr_storage* addr, socklen_t* addrlen, int* net, int* af)
{
char buff[3 /* 3 digit netblock */ + 1];
if(*dname > 3)
@ -363,9 +369,13 @@ int netblockdnametoaddr(uint8_t* dname, struct sockaddr_storage* addr,
memcpy(buff, dname+1, *dname);
buff[*dname] = '\0';
*net = atoi(buff);
if(*net == 0 && strcmp(buff, "0") != 0)
return 0;
dname += *dname;
dname++;
if(!ipdnametoaddr(dname, addr, addrlen, af))
if(!ipdnametoaddr(dname, dnamelen, addr, addrlen, af))
return 0;
if((*af == AF_INET6 && *net > 128) || (*af == AF_INET && *net > 32))
return 0;
return 1;
}

View file

@ -477,12 +477,13 @@ void listen_sslctx_delete_ticket_keys(void);
* - 24.10.100.51.198.rpz-ip -> 198.51.100.10/24
* - 32.10.zz.db8.2001.rpz-ip -> 2001:db8:0:0:0:0:0:10/32
* @param dname: the dname containing RPZ format netblock
* @param dnamelen: length of dname
* @param addr: where to store sockaddr.
* @param addrlen: length of stored sockaddr is returned.
* @param net: where to store netmask
* @param af: where to store address family.
* @return 0 on error.
*/
int netblockdnametoaddr(uint8_t* dname, struct sockaddr_storage* addr,
socklen_t* addrlen, int* net, int* af);
int netblockdnametoaddr(uint8_t* dname, size_t dnamelen,
struct sockaddr_storage* addr, socklen_t* addrlen, int* net, int* af);
#endif /* NET_HELP_H */