From c2f5027fcbfd9236d2e2ad26c7491704c2d812c0 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Marek=20Vavru=C5=A1a?= Date: Tue, 5 Jun 2018 22:23:43 -0700 Subject: [PATCH] policy: set NS set, support insecure forward in stub This allows policy filter to modify NS set in the checkout layer. It also fixes a bug in which invalid peer address would be used if the first UDP retransmit fails (`choice` would be set to memory past the `task->addrlist` in `qr_task_step`). --- daemon/worker.c | 47 +++++++++++++++++++++++++-------------- modules/daf/daf.lua | 30 +++++++++++++++++++++++-- modules/policy/policy.lua | 32 +++++++++++++++++++++----- 3 files changed, 85 insertions(+), 24 deletions(-) diff --git a/daemon/worker.c b/daemon/worker.c index 839332570..9f1716473 100644 --- a/daemon/worker.c +++ b/daemon/worker.c @@ -63,7 +63,6 @@ struct qr_task qr_tasklist_t waiting; uv_handle_t *pending[MAX_PENDING]; uint16_t pending_count; - uint16_t addrlist_count; uint16_t addrlist_turn; uint16_t timeouts; uint16_t iter_count; @@ -1349,8 +1348,11 @@ static void on_udp_timeout(uv_timer_t *timer) if (task->leading && task->pending_count > 0) { struct kr_query *qry = array_tail(task->ctx->req.rplan.pending); struct sockaddr_in6 *addrlist = (struct sockaddr_in6 *)task->addrlist; - for (uint16_t i = 0; i < MIN(task->pending_count, task->addrlist_count); ++i) { + for (uint16_t i = 0; i < MIN(task->pending_count, KR_NSREP_MAXADDR); ++i) { struct sockaddr *choice = (struct sockaddr *)(&addrlist[i]); + if (choice->sa_family == AF_UNSPEC) { + break; + } WITH_VERBOSE(qry) { char addr_str[INET6_ADDRSTRLEN]; inet_ntop(choice->sa_family, kr_inaddr(choice), addr_str, sizeof(addr_str)); @@ -1384,18 +1386,33 @@ static void on_session_idle_timeout(uv_timer_t *timer) static uv_handle_t *retransmit(struct qr_task *task) { uv_handle_t *ret = NULL; - if (task && task->addrlist && task->addrlist_count > 0) { - struct sockaddr_in6 *choice = &((struct sockaddr_in6 *)task->addrlist)[task->addrlist_turn]; - if (!choice) { + if (task && task->addrlist) { + /* Select next available address from the list */ + struct sockaddr_in6 *choice = NULL; + for (size_t i = 0; i < KR_NSREP_MAXADDR; ++i) { + choice = &((struct sockaddr_in6 *)task->addrlist)[(task->addrlist_turn + i) % KR_NSREP_MAXADDR]; + if (choice->sin6_family != AF_UNSPEC) { + break; + } + } + + /* Check if a valid address exists */ + if (choice == NULL) { return ret; } /* Checkout query before sending it */ struct request_ctx *ctx = task->ctx; if (kr_resolve_checkout(&ctx->req, NULL, (struct sockaddr *)choice, SOCK_DGRAM, task->pktbuf) != 0) { - task->addrlist_turn = (task->addrlist_turn + 1) % task->addrlist_count; /* Round robin */ + task->addrlist_turn += 1; + return ret; + } + + /* Check that the selected address is still valid */ + if (choice->sin6_family != AF_INET && choice->sin6_family != AF_INET6) { return ret; } + ret = ioreq_spawn(task, SOCK_DGRAM, choice->sin6_family); if (!ret) { return ret; @@ -1406,8 +1423,7 @@ static uv_handle_t *retransmit(struct qr_task *task) memcpy(&session->peer, addr, sizeof(session->peer)); if (qr_task_send(task, ret, (struct sockaddr *)choice, task->pktbuf) == 0) { - task->addrlist_turn = (task->addrlist_turn + 1) % - task->addrlist_count; /* Round robin */ + task->addrlist_turn += 1; } else { /* Didn't create request or message wasn't sent */ ret = NULL; @@ -1580,7 +1596,6 @@ static int qr_task_step(struct qr_task *task, struct worker_ctx *worker = ctx->worker; int sock_type = -1; task->addrlist = NULL; - task->addrlist_count = 0; task->addrlist_turn = 0; req->has_tls = (ctx->source.session && ctx->source.session->has_tls); @@ -1613,13 +1628,6 @@ static int qr_task_step(struct qr_task *task, return qr_task_step(task, NULL, NULL); } - /* Count available address choices */ - struct sockaddr_in6 *choice = (struct sockaddr_in6 *)task->addrlist; - for (size_t i = 0; i < KR_NSREP_MAXADDR && choice->sin6_family != AF_UNSPEC; ++i) { - task->addrlist_count += 1; - choice += 1; - } - /* Start fast retransmit with UDP, otherwise connect. */ int ret = 0; if (sock_type == SOCK_DGRAM) { @@ -1630,7 +1638,7 @@ static int qr_task_step(struct qr_task *task, /* Start transmitting */ uv_handle_t *handle = retransmit(task); if (handle == NULL) { - return qr_task_step(task, (struct sockaddr *)choice, NULL); + return qr_task_step(task, task->addrlist, NULL); } /* Check current query NSLIST */ struct kr_query *qry = array_tail(req->rplan.pending); @@ -1670,6 +1678,11 @@ static int qr_task_step(struct qr_task *task, subreq_finalize(task, packet_source, packet); return qr_task_finalize(task, KR_STATE_FAIL); } + /* CHeck that the selected address is still valid */ + if (addr->sa_family != AF_INET && addr->sa_family != AF_INET6) { + subreq_finalize(task, packet_source, packet); + return qr_task_finalize(task, KR_STATE_FAIL); + } struct session* session = NULL; if ((session = worker_find_tcp_waiting(ctx->worker, addr)) != NULL) { assert(session->outgoing); diff --git a/modules/daf/daf.lua b/modules/daf/daf.lua index eea1ee9ff..0bb390506 100644 --- a/modules/daf/daf.lua +++ b/modules/daf/daf.lua @@ -16,6 +16,7 @@ M.phases = { reroute = 'finish', rewrite = 'finish', features = 'checkout', + nsset = 'checkout', } -- Actions @@ -35,11 +36,36 @@ M.actions = { forward = function (g) local addrs = {} local tok = g() - for addr in string.gmatch(tok, '[^,]+') do - table.insert(addrs, addr) + while tok do + for addr in string.gmatch(tok, '[^,]+') do + table.insert(addrs, addr) + end + tok = g() end return policy.FORWARD(addrs) end, + forward_insecure = function (g) + local addrs = {} + local tok = g() + while tok do + for addr in string.gmatch(tok, '[^,]+') do + table.insert(addrs, addr) + end + tok = g() + end + return policy.STUB(addrs) + end, + nsset = function (g) + local addrs = {} + local tok = g() + while tok do + for addr in string.gmatch(tok, '[^,]+') do + table.insert(addrs, addr) + end + tok = g() + end + return policy.NSSET(addrs) + end, mirror = function (g) return policy.MIRROR(g()) end, diff --git a/modules/policy/policy.lua b/modules/policy/policy.lua index 08241460b..24aeb3a2b 100644 --- a/modules/policy/policy.lua +++ b/modules/policy/policy.lua @@ -81,13 +81,11 @@ end -- Override the list of nameservers (forwarders) local function set_nslist(qry, list) - for i, ns in ipairs(list) do + local ns_count = #list + for i = 1, 4 do + local ns = (i <= ns_count) and list[i] or nil assert(ffi.C.kr_nsrep_set(qry, i - 1, ns) == 0); end - -- If less than maximum NSs, insert guard to terminate the list - if #list < 4 then - assert(ffi.C.kr_nsrep_set(qry, #list, nil) == 0); - end end -- Forward request, and solve as stub query @@ -135,6 +133,30 @@ function policy.FORWARD(target) end end +-- Set NS set for given request +function policy.NSSET(target) + local list = {} + if type(target) == 'table' then + for _, v in pairs(target) do + table.insert(list, addr2sock(v, 53)) + assert(#list <= 4, 'at most 4 NS targets are supported') + end + else + table.insert(list, addr2sock(target, 53)) + end + return function(state, req, qry) + if not qry then return end + local vars = req:vars() + -- Make sure the NS set is updated only once for each query + if vars.policy_nsset_set == qry then + return + end + vars.policy_nsset_set = qry + set_nslist(qry, list) + return state + end +end + -- object must be non-empty string or non-empty table of non-empty strings local function is_nonempty_string_or_table(object) if type(object) == 'string' then -- 2.47.3