From 79a8db1ee184ca9e6fa1e265c1970f7074c0bea1 Mon Sep 17 00:00:00 2001 From: "W.C.A. Wijngaards" Date: Tue, 13 Oct 2020 08:28:59 +0200 Subject: [PATCH 1/7] - Fix #323: unbound testsuite fails on mock build in systemd-nspawn if systemd support is build. --- doc/Changelog | 4 ++++ testcode/do-tests.sh | 3 +++ testcode/run_vm.sh | 2 ++ testcode/testbound.c | 4 ++++ 4 files changed, 13 insertions(+) diff --git a/doc/Changelog b/doc/Changelog index e98876fca..f8cec1158 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -1,3 +1,7 @@ +13 October 2020: Wouter + - Fix #323: unbound testsuite fails on mock build in systemd-nspawn + if systemd support is build. + 9 October 2020: Wouter - Fix dnstap socket and the chroot not applied properly to the dnstap socket path. diff --git a/testcode/do-tests.sh b/testcode/do-tests.sh index 5439f0f28..effb7c16a 100755 --- a/testcode/do-tests.sh +++ b/testcode/do-tests.sh @@ -29,6 +29,9 @@ else HAVE_MINGW=no fi +# stop tests from notifying systemd, if that is compiled in. +export -n NOTIFY_SOCKET + cd testdata; sh ../testcode/mini_tdir.sh clean rm -f .perfstats.txt diff --git a/testcode/run_vm.sh b/testcode/run_vm.sh index 5f599e144..363a32b52 100644 --- a/testcode/run_vm.sh +++ b/testcode/run_vm.sh @@ -40,6 +40,8 @@ cleanup() { exit 0 } trap cleanup INT +# stop tests from notifying systemd, if that is compiled in. +export -n NOTIFY_SOCKET for t in $RUNLIST do diff --git a/testcode/testbound.c b/testcode/testbound.c index 602dffaff..3f3e106b0 100644 --- a/testcode/testbound.c +++ b/testcode/testbound.c @@ -362,6 +362,10 @@ main(int argc, char* argv[]) /* we do not want the test to depend on the timezone */ (void)putenv("TZ=UTC"); memset(pass_argv, 0, sizeof(pass_argv)); +#ifdef HAVE_SYSTEMD + /* we do not want the test to use systemd daemon startup notification*/ + (void)unsetenv("NOTIFY_SOCKET"); +#endif /* HAVE_SYSTEMD */ log_init(NULL, 0, NULL); /* determine commandline options for the daemon */ From 72032a95bb254c6c1b16ddcdf50352152141b524 Mon Sep 17 00:00:00 2001 From: "W.C.A. Wijngaards" Date: Wed, 14 Oct 2020 10:06:28 +0200 Subject: [PATCH 2/7] - Fix for python reply callback to see mesh state reply_list member, it only removes it briefly for the commpoint call so that it does not drop it and attempt to modify the reply list during reply. --- doc/Changelog | 5 +++++ services/mesh.c | 29 +++++++++++++---------------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/doc/Changelog b/doc/Changelog index f8cec1158..97959cf26 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -1,3 +1,8 @@ +14 October 2020: Wouter + - Fix for python reply callback to see mesh state reply_list member, + it only removes it briefly for the commpoint call so that it does + not drop it and attempt to modify the reply list during reply. + 13 October 2020: Wouter - Fix #323: unbound testsuite fails on mock build in systemd-nspawn if systemd support is build. diff --git a/services/mesh.c b/services/mesh.c index 52ff97e4a..8f746ea8e 100644 --- a/services/mesh.c +++ b/services/mesh.c @@ -1196,6 +1196,12 @@ mesh_send_reply(struct mesh_state* m, int rcode, struct reply_info* rep, /* Copy the client's EDNS for later restore, to make sure the edns * compare is with the correct edns options. */ struct edns_data edns_bak = r->edns; + /* briefly set the replylist to null in case the + * meshsendreply calls tcpreqinfo sendreply that + * comm_point_drops because of size, and then the + * null stops the mesh state remove and thus + * reply_list modification and accounting */ + struct mesh_reply* rlist = m->reply_list; /* examine security status */ if(m->s.env->need_to_validate && (!(r->qflags&BIT_CD) || m->s.env->cfg->ignore_cd) && rep && @@ -1236,7 +1242,9 @@ mesh_send_reply(struct mesh_state* m, int rcode, struct reply_info* rep, sldns_buffer_write_at(r_buffer, 0, &r->qid, sizeof(uint16_t)); sldns_buffer_write_at(r_buffer, 12, r->qname, m->s.qinfo.qname_len); + m->reply_list = NULL; comm_point_send_reply(&r->query_reply); + m->reply_list = rlist; } else if(rcode) { m->s.qinfo.qname = r->qname; m->s.qinfo.local_alias = r->local_alias; @@ -1251,7 +1259,9 @@ mesh_send_reply(struct mesh_state* m, int rcode, struct reply_info* rep, } error_encode(r_buffer, rcode, &m->s.qinfo, r->qid, r->qflags, &r->edns); + m->reply_list = NULL; comm_point_send_reply(&r->query_reply); + m->reply_list = rlist; } else { size_t udp_size = r->edns.udp_size; r->edns.edns_version = EDNS_ADVERTISED_VERSION; @@ -1277,7 +1287,9 @@ mesh_send_reply(struct mesh_state* m, int rcode, struct reply_info* rep, &m->s.qinfo, r->qid, r->qflags, &r->edns); } r->edns = edns_bak; + m->reply_list = NULL; comm_point_send_reply(&r->query_reply); + m->reply_list = rlist; } /* account */ log_assert(m->s.env->mesh->num_reply_addrs > 0); @@ -1365,20 +1377,12 @@ void mesh_query_done(struct mesh_state* mstate) mstate->reply_list = reply_list; } else { struct sldns_buffer* r_buffer = r->query_reply.c->buffer; - struct mesh_reply* rlist = mstate->reply_list; if(r->query_reply.c->tcp_req_info) { r_buffer = r->query_reply.c->tcp_req_info->spool_buffer; prev_buffer = NULL; } - /* briefly set the replylist to null in case the - * meshsendreply calls tcpreqinfo sendreply that - * comm_point_drops because of size, and then the - * null stops the mesh state remove and thus - * reply_list modification and accounting */ - mstate->reply_list = NULL; mesh_send_reply(mstate, mstate->s.return_rcode, rep, r, r_buffer, prev, prev_buffer); - mstate->reply_list = rlist; if(r->query_reply.c->tcp_req_info) { tcp_req_info_remove_mesh_state(r->query_reply.c->tcp_req_info, mstate); r_buffer = NULL; @@ -1894,7 +1898,7 @@ mesh_serve_expired_callback(void* arg) { struct mesh_state* mstate = (struct mesh_state*) arg; struct module_qstate* qstate = &mstate->s; - struct mesh_reply* r, *rlist; + struct mesh_reply* r; struct mesh_area* mesh = qstate->env->mesh; struct dns_msg* msg; struct mesh_cb* c; @@ -1999,15 +2003,8 @@ mesh_serve_expired_callback(void* arg) r_buffer = r->query_reply.c->buffer; if(r->query_reply.c->tcp_req_info) r_buffer = r->query_reply.c->tcp_req_info->spool_buffer; - /* briefly set the replylist to null in case the meshsendreply - * calls tcpreqinfo sendreply that comm_point_drops because - * of size, and then the null stops the mesh state remove and - * thus reply_list modification and accounting */ - rlist = mstate->reply_list; - mstate->reply_list = NULL; mesh_send_reply(mstate, LDNS_RCODE_NOERROR, msg->rep, r, r_buffer, prev, prev_buffer); - mstate->reply_list = rlist; if(r->query_reply.c->tcp_req_info) tcp_req_info_remove_mesh_state(r->query_reply.c->tcp_req_info, mstate); prev = r; From a9e13f3590f2e6bdba50eb8a3825d2c8e2362778 Mon Sep 17 00:00:00 2001 From: "W.C.A. Wijngaards" Date: Wed, 14 Oct 2020 14:01:47 +0200 Subject: [PATCH 3/7] - Fix that if there are on reply callbacks, those are called per reply and a new message created if that was modified by the call. --- doc/Changelog | 2 ++ services/mesh.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/Changelog b/doc/Changelog index 97959cf26..4d35a747e 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -2,6 +2,8 @@ - Fix for python reply callback to see mesh state reply_list member, it only removes it briefly for the commpoint call so that it does not drop it and attempt to modify the reply list during reply. + - Fix that if there are on reply callbacks, those are called per + reply and a new message created if that was modified by the call. 13 October 2020: Wouter - Fix #323: unbound testsuite fails on mock build in systemd-nspawn diff --git a/services/mesh.c b/services/mesh.c index 8f746ea8e..c22ef43fa 100644 --- a/services/mesh.c +++ b/services/mesh.c @@ -1235,7 +1235,7 @@ mesh_send_reply(struct mesh_state* m, int rcode, struct reply_info* rep, 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) { + == 0 && !env->inplace_cb_lists[inplace_cb_reply]) { /* if the previous reply is identical to this one, fix ID */ if(prev_buffer != r_buffer) sldns_buffer_copy(r_buffer, prev_buffer); From f0c19be06f7148edaf14f159ea3e671075f45e99 Mon Sep 17 00:00:00 2001 From: "W.C.A. Wijngaards" Date: Wed, 14 Oct 2020 14:03:04 +0200 Subject: [PATCH 4/7] - Fix that if there are on reply callbacks, those are called per reply and a new message created if that was modified by the call. --- services/mesh.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/mesh.c b/services/mesh.c index c22ef43fa..810865933 100644 --- a/services/mesh.c +++ b/services/mesh.c @@ -1235,7 +1235,7 @@ mesh_send_reply(struct mesh_state* m, int rcode, struct reply_info* rep, 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 && !env->inplace_cb_lists[inplace_cb_reply]) { + == 0 && !m->s.env->inplace_cb_lists[inplace_cb_reply]) { /* if the previous reply is identical to this one, fix ID */ if(prev_buffer != r_buffer) sldns_buffer_copy(r_buffer, prev_buffer); From 890c8deb0fdc05cbf03c1c15ffa8698ece07d655 Mon Sep 17 00:00:00 2001 From: "W.C.A. Wijngaards" Date: Wed, 14 Oct 2020 14:20:16 +0200 Subject: [PATCH 5/7] - Free up auth zone parse region after use for lookup of host --- doc/Changelog | 1 + services/authzone.c | 2 ++ 2 files changed, 3 insertions(+) diff --git a/doc/Changelog b/doc/Changelog index 4d35a747e..c7076fd62 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -4,6 +4,7 @@ not drop it and attempt to modify the reply list during reply. - Fix that if there are on reply callbacks, those are called per reply and a new message created if that was modified by the call. + - Free up auth zone parse region after use for lookup of host 13 October 2020: Wouter - Fix #323: unbound testsuite fails on mock build in systemd-nspawn diff --git a/services/authzone.c b/services/authzone.c index a26d1003a..15be5d60c 100644 --- a/services/authzone.c +++ b/services/authzone.c @@ -5387,6 +5387,7 @@ void auth_xfer_transfer_lookup_callback(void* arg, int rcode, sldns_buffer* buf, verbose(VERB_ALGO, "auth zone %s host %s type %s transfer lookup has no answer", zname, xfr->task_transfer->lookup_target->host, (xfr->task_transfer->lookup_aaaa?"AAAA":"A")); } } + regional_free_all(temp); } else { if(verbosity >= VERB_ALGO) { char zname[255+1]; @@ -6444,6 +6445,7 @@ void auth_xfer_probe_lookup_callback(void* arg, int rcode, sldns_buffer* buf, verbose(VERB_ALGO, "auth zone %s host %s type %s probe lookup has no address", zname, xfr->task_probe->lookup_target->host, (xfr->task_probe->lookup_aaaa?"AAAA":"A")); } } + regional_free_all(temp); } else { if(verbosity >= VERB_ALGO) { char zname[255+1]; From b1a50720e5675f86e9ffa9042453a5d686366ba0 Mon Sep 17 00:00:00 2001 From: netblue30 Date: Wed, 14 Oct 2020 11:32:14 -0400 Subject: [PATCH 6/7] DoH: implement content-lenght header field --- services/listen_dnsport.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/services/listen_dnsport.c b/services/listen_dnsport.c index 3a98c2642..796d62a37 100644 --- a/services/listen_dnsport.c +++ b/services/listen_dnsport.c @@ -2177,9 +2177,10 @@ int http2_submit_dns_response(struct http2_session* h2_session) int ret; nghttp2_data_provider data_prd; char status[4]; - nghttp2_nv headers[2]; + nghttp2_nv headers[3]; struct http2_stream* h2_stream = h2_session->c->h2_stream; size_t rlen; + char rlen_str[6]; // big enough to hold a uint16_t number if(h2_stream->rbuffer) { log_err("http2 submit response error: rbuffer already " @@ -2198,6 +2199,13 @@ int http2_submit_dns_response(struct http2_session* h2_session) } rlen = sldns_buffer_remaining(h2_session->c->buffer); + int rv = snprintf(rlen_str, sizeof(rlen_str), "%u", rlen); + if (rv <= 0 || rv >= sizeof(rlen_str)) { + verbose(VERB_QUERY, "http2: submit response error: " + "data buffer too large"); + return 0; + } + lock_basic_lock(&http2_response_buffer_count_lock); if(http2_response_buffer_count + rlen > http2_response_buffer_max) { lock_basic_unlock(&http2_response_buffer_count_lock); @@ -2228,13 +2236,11 @@ int http2_submit_dns_response(struct http2_session* h2_session) headers[1].valuelen = 23; headers[1].flags = NGHTTP2_NV_FLAG_NONE; - /*TODO be nice and add the content-length header headers[2].name = (uint8_t*)"content-length"; headers[2].namelen = 14; - headers[2].value = - headers[2].valuelen = + headers[2].value = rlen_str; + headers[2].valuelen = strlen(rlen_str); headers[2].flags = NGHTTP2_NV_FLAG_NONE; - */ sldns_buffer_write(h2_stream->rbuffer, sldns_buffer_current(h2_session->c->buffer), @@ -2244,7 +2250,7 @@ int http2_submit_dns_response(struct http2_session* h2_session) data_prd.source.ptr = h2_session; data_prd.read_callback = http2_submit_response_read_callback; ret = nghttp2_submit_response(h2_session->session, h2_stream->stream_id, - headers, 2, &data_prd); + headers, 3, &data_prd); if(ret) { verbose(VERB_QUERY, "http2: set_stream_user_data failed, " "error: %s", nghttp2_strerror(ret)); From edc8f363a72951ad86112b07f64ce0a8f0495723 Mon Sep 17 00:00:00 2001 From: "W.C.A. Wijngaards" Date: Thu, 15 Oct 2020 08:22:42 +0200 Subject: [PATCH 7/7] Changelog note for #326 and changes: - DoH content length, simplify code, remove declaration after statement and fix cast warning. --- doc/Changelog | 6 ++++++ services/listen_dnsport.c | 11 +++-------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/doc/Changelog b/doc/Changelog index c7076fd62..2d8c69e37 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -1,3 +1,9 @@ +15 October 2020: Wouter + - Merge PR #326 from netblue30: DoH: implement content-length + header field + - DoH content length, simplify code, remove declaration after + statement and fix cast warning. + 14 October 2020: Wouter - Fix for python reply callback to see mesh state reply_list member, it only removes it briefly for the commpoint call so that it does diff --git a/services/listen_dnsport.c b/services/listen_dnsport.c index 796d62a37..1cdb32a9c 100644 --- a/services/listen_dnsport.c +++ b/services/listen_dnsport.c @@ -2180,7 +2180,7 @@ int http2_submit_dns_response(struct http2_session* h2_session) nghttp2_nv headers[3]; struct http2_stream* h2_stream = h2_session->c->h2_stream; size_t rlen; - char rlen_str[6]; // big enough to hold a uint16_t number + char rlen_str[32]; if(h2_stream->rbuffer) { log_err("http2 submit response error: rbuffer already " @@ -2199,12 +2199,7 @@ int http2_submit_dns_response(struct http2_session* h2_session) } rlen = sldns_buffer_remaining(h2_session->c->buffer); - int rv = snprintf(rlen_str, sizeof(rlen_str), "%u", rlen); - if (rv <= 0 || rv >= sizeof(rlen_str)) { - verbose(VERB_QUERY, "http2: submit response error: " - "data buffer too large"); - return 0; - } + snprintf(rlen_str, sizeof(rlen_str), "%u", rlen); lock_basic_lock(&http2_response_buffer_count_lock); if(http2_response_buffer_count + rlen > http2_response_buffer_max) { @@ -2238,7 +2233,7 @@ int http2_submit_dns_response(struct http2_session* h2_session) headers[2].name = (uint8_t*)"content-length"; headers[2].namelen = 14; - headers[2].value = rlen_str; + headers[2].value = (uint8_t*)rlen_str; headers[2].valuelen = strlen(rlen_str); headers[2].flags = NGHTTP2_NV_FLAG_NONE;