From aebf1db11692c5652bfc0d2ce3d3d2fe1431b015 Mon Sep 17 00:00:00 2001 From: TCY16 Date: Mon, 20 Jun 2022 15:41:11 +0200 Subject: [PATCH] add step in rpl to verify the updated server cookie and fix bug with unupdated rdlen --- iterator/iterator.c | 42 +++++++++++++---------- testcode/testpkts.c | 42 ++++++++++++++++++++--- testcode/testpkts.h | 3 ++ testdata/edns_upstream_cookies.rpl | 54 ++++++++++++++++++++++++++++++ 4 files changed, 119 insertions(+), 22 deletions(-) diff --git a/iterator/iterator.c b/iterator/iterator.c index fce728448..9c3cab478 100644 --- a/iterator/iterator.c +++ b/iterator/iterator.c @@ -3825,25 +3825,33 @@ process_response(struct module_qstate* qstate, struct iter_qstate* iq, } /* handle the upstream response cookie if enabled*/ - if(qstate->env->cfg->upstream_cookies && - (cookie = edns_list_get_option(edns.opt_list_in, LDNS_EDNS_COOKIE))) { - /* verify this is a 'complete cookie' (client+server) (RFC9018) with - * the length and store the complete cookie in the infra_cache. Do - * nothing when the cookie is already known and update when the - * server cookie changed*/ - if (cookie->opt_len == 24 && - infra_set_server_cookie(qstate->env->infra_cache, - &qstate->reply->addr, qstate->reply->addrlen, - iq->dp->name, iq->dp->namelen, cookie) >= 0) { - /* log_hex() uses the verbosity levels of verbose() */ - log_hex("complete cookie: ", cookie->opt_data, - cookie->opt_len); + if(qstate->env->cfg->upstream_cookies) { + if (edns.opt_list_in && + (cookie = edns_list_get_option(edns.opt_list_in, + LDNS_EDNS_COOKIE))){ + /* verify this is a 'complete cookie' (client+server) + * (RFC9018) with the length and store the complete + * cookie in the infra_cache. Do nothing when the cookie + * is already known and update when the server cookie + * changed */ + if (cookie->opt_len == 24 && + infra_set_server_cookie(qstate->env->infra_cache, + &qstate->reply->addr, qstate->reply->addrlen, + iq->dp->name, iq->dp->namelen, cookie) >= 0) { + /* log_hex() uses the verbosity levels of verbose() */ + log_hex("complete cookie: ", cookie->opt_data, + cookie->opt_len); + } else { + log_info("upstream response server cookie is not " + "added to cache; dropping response"); + goto handle_it; + } } else { - log_info("upstream response server cookie is not added to;" - " cache dropping response"); - goto handle_it; + //@TODO think about what we do if we did send a cookie + // but did not get one back? for now we log_err() + log_err("upstream has not responded with a cookie"); } - } //@TODO think about what we do if we did send a cookie but did not get one back? + } /* Copy the edns options we may got from the back end */ if(edns.opt_list_in) { diff --git a/testcode/testpkts.c b/testcode/testpkts.c index 123d46075..0219e6f94 100644 --- a/testcode/testpkts.c +++ b/testcode/testpkts.c @@ -145,6 +145,9 @@ static void matchline(char* line, struct entry* e) e->match_ednsdata_raw = 1; } else if(str_keyword(&parse, "random_client_cookie")) { e->match_random_client_cookie = 1; + } else if (str_keyword(&parse, "random_complete_cookie_renewed")) { + e->match_random_complete_cookie = 1; + e->match_random_complete_cookie_renewed = 1; } else if(str_keyword(&parse, "random_complete_cookie")) { e->match_random_complete_cookie = 1; } else if(str_keyword(&parse, "UDP")) { @@ -1544,9 +1547,9 @@ match_random_client_cookie(uint8_t* query, size_t query_len) } /** verify that a complete EDNS cookie (client+server) (RFC9018) of length 24 - * is in the EDNS data of the query */ + * is in the EDNS data of the query and the hardcoded cookie is the same */ static int -match_random_complete_cookie(uint8_t* query, size_t query_len) +match_random_complete_cookie(uint8_t* query, size_t query_len, struct entry* p) { uint8_t* walk_query = query; size_t walk_query_len = query_len; @@ -1562,15 +1565,40 @@ match_random_complete_cookie(uint8_t* query, size_t query_len) return 0; } - if (sldns_read_uint16(walk_query+8) != 10 /* LDNS_EDNS_COOKIE */) { + walk_query += 8; + walk_query_len -= 8; + + if (sldns_read_uint16(walk_query) != 10 /* LDNS_EDNS_COOKIE */) { verbose(3, "EDNS option is not a cookie"); return 0; } - if (sldns_read_uint16(walk_query+10) != 24) { + if (sldns_read_uint16(walk_query+2) != 24) { verbose(3, "EDNS cookie is not 24 bytes, so not a correct complete cookie"); return 0; } + if (p->match_random_complete_cookie_renewed) { + uint8_t renewed_cookie[16]; + + /* shuffle the hardcoded cookie like adjust_packet() does */ + memcpy(renewed_cookie, hardcoded_server_cookie+8, 8); + memcpy(renewed_cookie+8, hardcoded_server_cookie, 8); + + /* client_cookie = 8 */ + if (!(memcmp(walk_query+8, renewed_cookie, 16))) { + verbose(3, "EDNS server cookie does not match the renewed" + "cookie, so not a correct complete cookie"); + return 0; + } + } + + /* client_cookie = 8 */ + if (!(memcmp(walk_query+8, hardcoded_server_cookie, 16))) { + verbose(3, "EDNS server cookie does not match the hardcoded cookie, " + "so not a correct complete cookie"); + return 0; + } + return 1; } @@ -1675,7 +1703,7 @@ find_match(struct entry* entries, uint8_t* query_pkt, size_t len, continue; } if (p->match_random_complete_cookie && - !match_random_complete_cookie(query_pkt, len)) { + !match_random_complete_cookie(query_pkt, len, p)) { verbose(3, "bad complete cookie match.\n"); continue; } @@ -1866,6 +1894,10 @@ adjust_packet(struct entry* match, uint8_t** answer_pkt, size_t *answer_len, reslen = origlen + 28; } else if (sldns_read_uint16(walk_query+2) == 24) { + /* update the RDLEN and OPTLEN */ + sldns_write_uint16(rdlen_ptr_response, 28); + sldns_write_uint16(walk_response+2, 24); + /* we fake verification of the cookie and send * it back like it's still valid. We renew the cookie * if this desired */ diff --git a/testcode/testpkts.h b/testcode/testpkts.h index df87fc645..f4aef0515 100644 --- a/testcode/testpkts.h +++ b/testcode/testpkts.h @@ -219,6 +219,9 @@ struct entry { /** match an EDNS cookie (RFC7873) of length 24, we call * this "complete" (RFC9018) */ uint8_t match_random_complete_cookie; + /** match an EDNS compelete cookie (RFC7873) of length 24, which cannot be + * the same as the hardcoded client cookie */ + uint8_t match_random_complete_cookie_renewed; /** match query serial with this value. */ uint32_t ixfr_soa_serial; /** match on UDP/TCP */ diff --git a/testdata/edns_upstream_cookies.rpl b/testdata/edns_upstream_cookies.rpl index d0840fd41..03090ac97 100644 --- a/testdata/edns_upstream_cookies.rpl +++ b/testdata/edns_upstream_cookies.rpl @@ -177,4 +177,58 @@ ENTRY_BEGIN HEX_EDNSDATA_END ENTRY_END + +; Verify that the cookie was updated since the timestep + +; Client query for upstream to Unbound +STEP 31 QUERY +ENTRY_BEGIN + REPLY RD + SECTION QUESTION + d.example.com. IN A + SECTION ADDITIONAL + HEX_EDNSDATA_BEGIN + HEX_EDNSDATA_END +ENTRY_END + +; Check that we send a server cookie to the upstream +STEP 32 CHECK_OUT_QUERY +ENTRY_BEGIN + MATCH qname qtype random_complete_cookie_renewed + SECTION QUESTION + d.example.com. IN A + SECTION ADDITIONAL + HEX_EDNSDATA_BEGIN + HEX_EDNSDATA_END +ENTRY_END + +; Reply with an updated server cookie that +STEP 33 REPLY +ENTRY_BEGIN + REPLY QR NOERROR + ADJUST copy_id server_cookie_renew + SECTION QUESTION + d.example.com. IN A + SECTION ANSWER + d.example.com. IN A 1.2.3.4 + SECTION ADDITIONAL + HEX_EDNSDATA_BEGIN + HEX_EDNSDATA_END +ENTRY_END + + +; Check the answer from Unbound for the client +STEP 34 CHECK_ANSWER +ENTRY_BEGIN + MATCH all + REPLY QR RA RD NOERROR + SECTION QUESTION + d.example.com. IN A + SECTION ANSWER + d.example.com. IN A 1.2.3.4 + SECTION ADDITIONAL + HEX_EDNSDATA_BEGIN + HEX_EDNSDATA_END +ENTRY_END + SCENARIO_END