Bug 1125 - unbound could reuse an answer packet incorrectly for clients with different EDNS parameters
unbound could reuse an answer packet incorrectly for clients with different E...
Status: RESOLVED FIXED
Product: unbound
Classification: Unclassified
Component: server
unspecified
Other All
: P5 enhancement
Assigned To: unbound team
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2016-10-05 20:39 CEST by JINMEI Tatuya
Modified: 2016-10-18 15:42 CEST (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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