From: George Thessalonikefs Date: Thu, 15 Oct 2020 13:53:16 +0000 (+0200) Subject: - Fix that if there are reply callbacks for the given rcode, those X-Git-Tag: release-1.13.0rc1~33 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d55084ea9e9089174ab9b4bc218f884d2095b50f;p=thirdparty%2Funbound.git - Fix that if there are reply callbacks for the given rcode, those are called per reply and a new message created if that was modified by the call. - Pass the comm_reply information to the inplace_cb_reply* functions during the mesh state and update the documentation on that. --- diff --git a/doc/Changelog b/doc/Changelog index 2d8c69e37..c97b24ec0 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -1,3 +1,10 @@ +15 October 2020: George + - Fix that if there are reply callbacks for the given rcode, those + are called per reply and a new message created if that was modified + by the call. + - Pass the comm_reply information to the inplace_cb_reply* functions + during the mesh state and update the documentation on that. + 15 October 2020: Wouter - Merge PR #326 from netblue30: DoH: implement content-length header field diff --git a/pythonmod/doc/examples/example6.rst b/pythonmod/doc/examples/example6.rst index d294fb8be..fd6caf74d 100644 --- a/pythonmod/doc/examples/example6.rst +++ b/pythonmod/doc/examples/example6.rst @@ -60,7 +60,6 @@ The callback function's prototype is the following: :param **kwargs: Dictionary that may contain parameters added in a future release. Current parameters: ``repinfo``: Reply information for a communication point (comm_reply). - It is None when the callback happens in the mesh states. :return: True on success, False on failure. @@ -105,8 +104,6 @@ The callback function's prototype is the following: :param **kwargs: Dictionary that may contain parameters added in a future release. Current parameters: ``repinfo``: Reply information for a communication point (comm_reply). - It is None when the callback happens in the mesh - states(modules). :return: True on success, False on failure. @@ -154,8 +151,6 @@ The callback function's prototype is the following: :param **kwargs: Dictionary that may contain parameters added in a future release. Current parameters: ``repinfo``: Reply information for a communication point (comm_reply). - It is None when the callback happens in the mesh - states(modules). :return: True on success, False on failure. @@ -201,8 +196,6 @@ The callback function's prototype is the following: :param **kwargs: Dictionary that may contain parameters added in a future release. Current parameters: ``repinfo``: Reply information for a communication point (comm_reply). - It is None when the callback happens in the mesh - states(modules). :return: True on success, False on failure. diff --git a/pythonmod/doc/modules/functions.rst b/pythonmod/doc/modules/functions.rst index 43c66eb38..6ded3bf7a 100644 --- a/pythonmod/doc/modules/functions.rst +++ b/pythonmod/doc/modules/functions.rst @@ -89,7 +89,7 @@ EDNS options Inplace callbacks ----------------- -.. function:: inplace_cb_reply(qinfo, qstate, rep, rcode, edns, opt_list_out, region) +.. function:: inplace_cb_reply(qinfo, qstate, rep, rcode, edns, opt_list_out, region, \*\*kwargs) Function prototype for callback functions used in `register_inplace_cb_reply`_, `register_inplace_cb_reply_cache`_, @@ -102,6 +102,9 @@ Inplace callbacks :param edns: :class:`edns_data` :param opt_list_out: :class:`edns_option`. EDNS option list to append options to. :param region: :class:`regional` + :param \*\*kwargs: Dictionary that may contain parameters added in a future + release. Current parameters: + ``repinfo``: :class:`comm_reply`. Reply information for a communication point. .. function:: inplace_cb_query(qinfo, flags, qstate, addr, zone, region) diff --git a/pythonmod/examples/inplace_callbacks.py b/pythonmod/examples/inplace_callbacks.py index 768c2d013..de375b4e1 100644 --- a/pythonmod/examples/inplace_callbacks.py +++ b/pythonmod/examples/inplace_callbacks.py @@ -43,7 +43,7 @@ # This query returns SERVFAIL as the txt record of bogus.nlnetlabs.nl is # intentionally bogus. The reply will contain an empty EDNS option # with option code 65003. -# Unbound will also log the source address(es) of the client(s) that made +# Unbound will also log the source address of the client that made # the request. # (unbound needs to be validating for this example to work) @@ -91,8 +91,6 @@ def inplace_reply_callback(qinfo, qstate, rep, rcode, edns, opt_list_out, :param **kwargs: Dictionary that may contain parameters added in a future release. Current parameters: ``repinfo``: Reply information for a communication point (comm_reply). - It is None when the callback happens in the mesh - states(modules). :return: True on success, False on failure. @@ -121,8 +119,6 @@ def inplace_cache_callback(qinfo, qstate, rep, rcode, edns, opt_list_out, :param **kwargs: Dictionary that may contain parameters added in a future release. Current parameters: ``repinfo``: Reply information for a communication point (comm_reply). - It is None when the callback happens in the mesh - states(modules). :return: True on success, False on failure. @@ -173,8 +169,6 @@ def inplace_local_callback(qinfo, qstate, rep, rcode, edns, opt_list_out, :param **kwargs: Dictionary that may contain parameters added in a future release. Current parameters: ``repinfo``: Reply information for a communication point (comm_reply). - It is None when the callback happens in the mesh - states(modules). :return: True on success, False on failure. @@ -205,13 +199,11 @@ def inplace_servfail_callback(qinfo, qstate, rep, rcode, edns, opt_list_out, :param **kwargs: Dictionary that may contain parameters added in a future release. Current parameters: ``repinfo``: Reply information for a communication point (comm_reply). - It is None when the callback happens in the mesh - states(modules). :return: True on success, False on failure. For demonstration purposes we want to reply with an empty EDNS code '65003' - and log the IP address(es) of the client(s). + and log the IP address of the client. """ log_info("python: called back while servfail.") @@ -219,30 +211,14 @@ def inplace_servfail_callback(qinfo, qstate, rep, rcode, edns, opt_list_out, b = bytearray.fromhex("") edns_opt_list_append(opt_list_out, 65003, b, region) - # Log the client(s) IP address(es) + # Log the client's IP address comm_reply = kwargs['repinfo'] if comm_reply: - # If it is not None this callback was called before the query reached - # the mesh states(modules). There is only one client associated with - # this query. addr = comm_reply.addr port = comm_reply.port addr_family = comm_reply.family log_info("python: Client IP: {}({}), port: {}" "".format(addr, addr_family, port)) - else: - # If it is not None this callback was called while the query is in the - # mesh states(modules). In this case they may be multiple clients - # waiting for this query. - # The following code is the same as with the resip.py example. - rl = qstate.mesh_info.reply_list - while (rl): - if rl.query_reply: - q = rl.query_reply - log_info("python: Client IP: {}({}), port: {}" - "".format(q.addr, q.family, q.port)) - rl = rl.next - return True diff --git a/services/mesh.c b/services/mesh.c index 810865933..cd9050936 100644 --- a/services/mesh.c +++ b/services/mesh.c @@ -1224,18 +1224,24 @@ mesh_send_reply(struct mesh_state* m, int rcode, struct reply_info* rep, r->h2_stream->mesh_state = NULL; } /* send the reply */ - /* We don't reuse the encoded answer if either the previous or current - * response has a local alias. We could compare the alias records - * and still reuse the previous answer if they are the same, but that - * would be complicated and error prone for the relatively minor case. - * So we err on the side of safety. */ - if(prev && prev_buffer && prev->qflags == r->qflags && + /* We don't reuse the encoded answer if: + * - either the previous or current response has a local alias. We could + * compare the alias records and still reuse the previous answer if they + * are the same, but that would be complicated and error prone for the + * relatively minor case. So we err on the side of safety. + * - there are registered callback functions for the given rcode, as these + * need to be called for each reply. */ + if(((rcode != LDNS_RCODE_SERVFAIL && + !m->s.env->inplace_cb_lists[inplace_cb_reply]) || + (rcode == LDNS_RCODE_SERVFAIL && + !m->s.env->inplace_cb_lists[inplace_cb_reply_servfail])) && + prev && prev_buffer && prev->qflags == r->qflags && !prev->local_alias && !r->local_alias && - prev->edns.edns_present == r->edns.edns_present && - prev->edns.bits == r->edns.bits && + 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) - == 0 && !m->s.env->inplace_cb_lists[inplace_cb_reply]) { + == 0) { /* if the previous reply is identical to this one, fix ID */ if(prev_buffer != r_buffer) sldns_buffer_copy(r_buffer, prev_buffer); @@ -1250,11 +1256,11 @@ mesh_send_reply(struct mesh_state* m, int rcode, struct reply_info* rep, m->s.qinfo.local_alias = r->local_alias; if(rcode == LDNS_RCODE_SERVFAIL) { if(!inplace_cb_reply_servfail_call(m->s.env, &m->s.qinfo, &m->s, - rep, rcode, &r->edns, NULL, m->s.region)) + rep, rcode, &r->edns, &r->query_reply, m->s.region)) r->edns.opt_list = NULL; } else { if(!inplace_cb_reply_call(m->s.env, &m->s.qinfo, &m->s, rep, rcode, - &r->edns, NULL, m->s.region)) + &r->edns, &r->query_reply, m->s.region)) r->edns.opt_list = NULL; } error_encode(r_buffer, rcode, &m->s.qinfo, r->qid, @@ -1271,7 +1277,7 @@ mesh_send_reply(struct mesh_state* m, int rcode, struct reply_info* rep, m->s.qinfo.qname = r->qname; m->s.qinfo.local_alias = r->local_alias; if(!inplace_cb_reply_call(m->s.env, &m->s.qinfo, &m->s, rep, - LDNS_RCODE_NOERROR, &r->edns, NULL, m->s.region) || + LDNS_RCODE_NOERROR, &r->edns, &r->query_reply, m->s.region) || !apply_edns_options(&r->edns, &edns_bak, m->s.env->cfg, r->query_reply.c, m->s.region) || @@ -1281,7 +1287,7 @@ mesh_send_reply(struct mesh_state* m, int rcode, struct reply_info* rep, secure)) { if(!inplace_cb_reply_servfail_call(m->s.env, &m->s.qinfo, &m->s, - rep, LDNS_RCODE_SERVFAIL, &r->edns, NULL, m->s.region)) + rep, LDNS_RCODE_SERVFAIL, &r->edns, &r->query_reply, m->s.region)) r->edns.opt_list = NULL; error_encode(r_buffer, LDNS_RCODE_SERVFAIL, &m->s.qinfo, r->qid, r->qflags, &r->edns); diff --git a/util/data/msgreply.h b/util/data/msgreply.h index 8d75f9b12..385780268 100644 --- a/util/data/msgreply.h +++ b/util/data/msgreply.h @@ -552,7 +552,7 @@ struct edns_option* edns_opt_list_find(struct edns_option* list, uint16_t code); * @param rep: Reply info. Could be NULL. * @param rcode: return code. * @param edns: edns data of the reply. - * @param repinfo: comm_reply. NULL. + * @param repinfo: comm_reply. Reply information for a communication point. * @param region: region to store data. * @return false on failure (a callback function returned an error). */