Bug 1125

Summary: unbound could reuse an answer packet incorrectly for clients with different EDNS parameters
Product: unbound Reporter: JINMEI Tatuya <jtatuya>
Component: serverAssignee: unbound team <unbound-team>
Status: RESOLVED FIXED    
Severity: enhancement CC: cathya, wouter
Priority: P5    
Version: unspecified   
Hardware: Other   
OS: All   

Description JINMEI Tatuya 2016-10-05 20:39:46 CEST
...and, less harmfully, it could skip reusing the answer when it actually can.

The problem is in services/mesh.c:mesh_send_reply().  It tries to determine whether it could reuse already encoded data (=prev) as follows:

	if(prev && prev->qflags == r->qflags && 
		prev->edns.edns_present == r->edns.edns_present && 
		prev->edns.bits == r->edns.bits && 
		prev->edns.udp_size == r->edns.udp_size &&
		edns_opt_list_compare(prev->edns.opt_list, r->edns.opt_list)

But this same function overrides r->edns (which will be used in prev->edns in the next call) first time and when it determines it cannot reuse 'prev':

		r->edns.edns_version = EDNS_ADVERTISED_VERSION;
		r->edns.udp_size = EDNS_ADVERTISED_SIZE;
		r->edns.ext_rcode = 0;
		r->edns.bits &= EDNS_DO;

So, on a subsequent call it can use the incorrect prev->edns parameters and could make a wrong conclusion.

Fixing this will be easy, but I guess you might also want to write a new automated test to exercise this regression.  This is quite subtle and it won't be very likely to notice it quickly in the field even if you re-introduce the same/similar regression.  In fact, I noticed it when I tried to write an automated test for my own project.
Comment 1 Wouter Wijngaards 2016-10-18 15:42:32 CEST
Hi Jinmei,

Thank you for the patch, I have applied it.

Best regards, Wouter