Chiebot-Mirror
/
c-ares
mirror of https://github.com/c-ares/c-ares.git
8
0
Fork 0
Code Issues Projects Releases Wiki Activity

server cookie wasn't being passed back due to missing length

Browse Source 
During the implementation of server cookies, test cases were missing
to validate the server cookie in a prior reply was passed back, and
it turns out they were not.

This also adds tests for verification.

Fix By: Brad House (@bradh352)
pull/834/head
Brad House 4 months ago
parent 2614afc553
commit c9c235761f
5 changed files with 74 additions and 37 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 3
      src/lib/ares_cookie.c
  2. 2
      test/ares-test-misc.cc
  3. 37
      test/ares-test-mock.cc
  4. 65
      test/dns-proto.cc
  5. 4
      test/dns-proto.h

3
src/lib/ares_cookie.c
Unescape View File

@ -409,7 +409,8 @@ ares_status_t ares_cookie_validate(struct query *query,
/* If client cookie hasn't been rotated, save the returned server cookie */
if (memcmp(cookie->client, req_cookie, sizeof(cookie->client)) == 0) {
memcpy(cookie->server, resp_cookie + 8, resp_cookie_len - 8);
cookie->server_len = resp_cookie_len - 8;
memcpy(cookie->server, resp_cookie + 8, cookie->server_len);
}
}

2
test/ares-test-misc.cc vendored
Unescape View File

@ -519,7 +519,7 @@ TEST_F(LibraryTest, CreateEDNSQuery) {
std::string actual = PacketToString(data);
DNSPacket pkt;
pkt.set_qid(0x1234).add_question(new DNSQuestion("example.com", T_A))
.add_additional(new DNSOptRR(0, 0, 0, 1280, { }, { } /* No server cookie */));
.add_additional(new DNSOptRR(0, 0, 0, 1280, { }, { } /* No server cookie */, false));
std::string expected = PacketToString(pkt.data());
EXPECT_EQ(expected, actual);
}

37
test/ares-test-mock.cc vendored
Unescape View File

@ -1570,7 +1570,7 @@ TEST_P(MockUDPChannelTest, DNSCookieSingle) {
reply.set_response().set_aa()
.add_question(new DNSQuestion("www.google.com", T_A))
.add_answer(new DNSARR("www.google.com", 0x0100, {0x01, 0x02, 0x03, 0x04}))
.add_additional(new DNSOptRR(0, 0, 0, 1280, { }, server_cookie));
.add_additional(new DNSOptRR(0, 0, 0, 1280, { }, server_cookie, false));
EXPECT_CALL(server_, OnRequest("www.google.com", T_A))
.WillOnce(SetReply(&server_, &reply));
@ -1587,26 +1587,33 @@ TEST_P(MockUDPChannelTest, DNSCookieSingle) {
}
TEST_P(MockUDPChannelTest, DNSCookieMissingAfterGood) {
DNSPacket reply;
std::vector<byte> server_cookie = { 1, 2, 3, 4, 5, 6, 7, 8 };
DNSPacket reply;
reply.set_response().set_aa()
.add_question(new DNSQuestion("www.google.com", T_A))
.add_answer(new DNSARR("www.google.com", 0x0100, {0x01, 0x02, 0x03, 0x04}))
.add_additional(new DNSOptRR(0, 0, 0, 1280, { }, server_cookie));
.add_additional(new DNSOptRR(0, 0, 0, 1280, { }, server_cookie, false));
DNSPacket reply_nocookie;
reply_nocookie.set_response().set_aa()
.add_question(new DNSQuestion("www.google.com", T_A))
.add_answer(new DNSARR("www.google.com", 0x0100, {0x01, 0x02, 0x03, 0x04}))
.add_additional(new DNSOptRR(0, 0, 0, 1280, { }, { }));
.add_additional(new DNSOptRR(0, 0, 0, 1280, { }, { }, false));
DNSPacket reply_ensurecookie;
reply_ensurecookie.set_response().set_aa()
.add_question(new DNSQuestion("www.google.com", T_A))
.add_answer(new DNSARR("www.google.com", 0x0100, {0x01, 0x02, 0x03, 0x04}))
.add_additional(new DNSOptRR(0, 0, 0, 1280, { }, server_cookie, true));
EXPECT_CALL(server_, OnRequest("www.google.com", T_A))
.WillOnce(SetReply(&server_, &reply))
.WillOnce(SetReply(&server_, &reply_nocookie))
.WillOnce(SetReply(&server_, &reply));
.WillOnce(SetReply(&server_, &reply_ensurecookie));
/* This test will establish the server supports cookies, then the next reply
* will be missing the server cookie and therefore be rejected and timeout, then
* an internal retry will occur and the cookie will be present again. */
* an internal retry will occur and the cookie will be present again and it
* will be verified a server cookie was actually present that matches the
* server cookie. */
QueryResult result1;
ares_query_dnsrec(channel_, "www.google.com", ARES_CLASS_IN, ARES_REC_TYPE_A, QueryCallback, &result1, NULL);
Process();
@ -1637,12 +1644,12 @@ TEST_P(MockUDPChannelTest, DNSCookieBadLen) {
reply.set_response().set_aa()
.add_question(new DNSQuestion("www.google.com", T_A))
.add_answer(new DNSARR("www.google.com", 0x0100, {0x01, 0x02, 0x03, 0x04}))
.add_additional(new DNSOptRR(0, 0, 0, 1280, { }, server_cookie));
.add_additional(new DNSOptRR(0, 0, 0, 1280, { }, server_cookie, false));
DNSPacket reply_badcookielen;
reply_badcookielen.set_response().set_aa()
.add_question(new DNSQuestion("www.google.com", T_A))
.add_answer(new DNSARR("www.google.com", 0x0100, {0x01, 0x02, 0x03, 0x04}))
.add_additional(new DNSOptRR(0, 0, 0, 1280, { }, server_cookie_bad ));
.add_additional(new DNSOptRR(0, 0, 0, 1280, { }, server_cookie_bad, false));
EXPECT_CALL(server_, OnRequest("www.google.com", T_A))
.WillOnce(SetReply(&server_, &reply_badcookielen))
@ -1665,17 +1672,17 @@ TEST_P(MockUDPChannelTest, DNSCookieServerRotate) {
reply_cookie1.set_response().set_aa()
.add_question(new DNSQuestion("www.google.com", T_A))
.add_answer(new DNSARR("www.google.com", 0x0100, {0x01, 0x02, 0x03, 0x04}))
.add_additional(new DNSOptRR(0, 0, 0, 1280, {}, server_cookie));
.add_additional(new DNSOptRR(0, 0, 0, 1280, {}, server_cookie, false));
DNSPacket reply_cookie2_badcookie;
reply_cookie2_badcookie.set_response().set_aa().set_rcode(ARES_RCODE_BADCOOKIE & 0xF)
.add_question(new DNSQuestion("www.google.com", T_A))
.add_answer(new DNSARR("www.google.com", 0x0100, {0x01, 0x02, 0x03, 0x04}))
.add_additional(new DNSOptRR((ARES_RCODE_BADCOOKIE >> 4) & 0xFF, 0, 0, 1280, { }, server_cookie_rotate));
.add_additional(new DNSOptRR((ARES_RCODE_BADCOOKIE >> 4) & 0xFF, 0, 0, 1280, { }, server_cookie_rotate, false));
DNSPacket reply_cookie2;
reply_cookie2.set_response().set_aa()
.add_question(new DNSQuestion("www.google.com", T_A))
.add_answer(new DNSARR("www.google.com", 0x0100, {0x01, 0x02, 0x03, 0x04}))
.add_additional(new DNSOptRR(0, 0, 0, 1280, { }, server_cookie_rotate));
.add_additional(new DNSOptRR(0, 0, 0, 1280, { }, server_cookie_rotate, true));
EXPECT_CALL(server_, OnRequest("www.google.com", T_A))
.WillOnce(SetReply(&server_, &reply_cookie1))
@ -1717,12 +1724,12 @@ TEST_P(MockUDPChannelTest, DNSCookieSpoof) {
reply_spoof.set_response().set_aa()
.add_question(new DNSQuestion("www.google.com", T_A))
.add_answer(new DNSARR("www.google.com", 0x0100, {0x01, 0x02, 0x03, 0x04}))
.add_additional(new DNSOptRR(0, 0, 0, 1280, client_cookie, server_cookie));
.add_additional(new DNSOptRR(0, 0, 0, 1280, client_cookie, server_cookie, false));
DNSPacket reply;
reply.set_response().set_aa()
.add_question(new DNSQuestion("www.google.com", T_A))
.add_answer(new DNSARR("www.google.com", 0x0100, {0x01, 0x02, 0x03, 0x04}))
.add_additional(new DNSOptRR(0, 0, 0, 1280, { }, server_cookie));
.add_additional(new DNSOptRR(0, 0, 0, 1280, { }, server_cookie, false));
EXPECT_CALL(server_, OnRequest("www.google.com", T_A))
.WillOnce(SetReply(&server_, &reply_spoof))
@ -1745,12 +1752,12 @@ TEST_P(MockUDPChannelTest, DNSCookieTCPUpgrade) {
reply_badcookie.set_response().set_aa().set_rcode(ARES_RCODE_BADCOOKIE & 0xF)
.add_question(new DNSQuestion("www.google.com", T_A))
.add_answer(new DNSARR("www.google.com", 0x0100, {0x01, 0x02, 0x03, 0x04}))
.add_additional(new DNSOptRR((ARES_RCODE_BADCOOKIE >> 4) & 0xFF, 0, 0, 1280, { }, server_cookie));
.add_additional(new DNSOptRR((ARES_RCODE_BADCOOKIE >> 4) & 0xFF, 0, 0, 1280, { }, server_cookie, false));
DNSPacket reply;
reply.set_response().set_aa()
.add_question(new DNSQuestion("www.google.com", T_A))
.add_answer(new DNSARR("www.google.com", 0x0100, {0x01, 0x02, 0x03, 0x04}))
.add_additional(new DNSOptRR(0, 0, 0, 1280, { }, { }));
.add_additional(new DNSOptRR(0, 0, 0, 1280, { }, { }, false));
EXPECT_CALL(server_, OnRequest("www.google.com", T_A))
.WillOnce(SetReply(&server_, &reply_badcookie))

65
test/dns-proto.cc vendored
Unescape View File

@ -662,28 +662,39 @@ const ares_dns_rr_t *fetch_rr_opt(const ares_dns_record_t *rec)
}
std::vector<byte> DNSOptRR::data(const ares_dns_record_t *dnsrec) const {
std::vector<byte> data = DNSRR::data(dnsrec);
int len = 0;
std::vector<byte> cookie;
std::vector<byte> data = DNSRR::data(dnsrec);
int len = 0;
std::vector<byte> cookie;
const ares_dns_rr_t *rr = fetch_rr_opt(dnsrec);
size_t passed_cookie_len = 0;
const unsigned char *passed_cookie = NULL;
ares_dns_rr_get_opt_byid(rr, ARES_RR_OPT_OPTIONS, ARES_OPT_PARAM_COOKIE,
&passed_cookie, &passed_cookie_len);
/* Error out if we expected a server cookie but didn't get one, or if the
* passed in server cookie doesn't match our expected value */
if (expect_server_cookie_ &&
(passed_cookie_len <= 8 ||
passed_cookie_len - 8 != server_cookie_.size() ||
memcmp(passed_cookie + 8, server_cookie_.data(), server_cookie_.size()) != 0
)
) {
data.clear();
return data;
}
/* See if we should be applying a server cookie */
if (server_cookie_.size()) {
const ares_dns_rr_t *rr = fetch_rr_opt(dnsrec);
const unsigned char *val = NULL;
size_t len = 0;
if (ares_dns_rr_get_opt_byid(rr, ARES_RR_OPT_OPTIONS, ARES_OPT_PARAM_COOKIE,
&val, &len)) {
/* If client cookie was provided to test framework, we are overwriting
* the one received from the client. This is likely to test failure
* scenarios */
if (client_cookie_.size()) {
cookie.insert(cookie.end(), client_cookie_.begin(), client_cookie_.end());
} else {
cookie.insert(cookie.end(), val, val+8);
}
cookie.insert(cookie.end(), server_cookie_.begin(), server_cookie_.end());
if (server_cookie_.size() && passed_cookie_len >= 8) {
/* If client cookie was provided to test framework, we are overwriting
* the one received from the client. This is likely to test failure
* scenarios */
if (client_cookie_.size()) {
cookie.insert(cookie.end(), client_cookie_.begin(), client_cookie_.end());
} else {
cookie.insert(cookie.end(), passed_cookie, passed_cookie+8);
}
cookie.insert(cookie.end(), server_cookie_.begin(), server_cookie_.end());
}
if (cookie.size()) {
@ -755,18 +766,34 @@ std::vector<byte> DNSPacket::data(const char *request_name, const ares_dns_recor
for (const std::unique_ptr<DNSQuestion>& question : questions_) {
std::vector<byte> qdata = question->data(request_name, dnsrec);
if (qdata.size() == 0) {
data.clear();
return data;
}
data.insert(data.end(), qdata.begin(), qdata.end());
}
for (const std::unique_ptr<DNSRR>& rr : answers_) {
std::vector<byte> rrdata = rr->data(dnsrec);
if (rrdata.size() == 0) {
data.clear();
return data;
}
data.insert(data.end(), rrdata.begin(), rrdata.end());
}
for (const std::unique_ptr<DNSRR>& rr : auths_) {
std::vector<byte> rrdata = rr->data(dnsrec);
if (rrdata.size() == 0) {
data.clear();
return data;
}
data.insert(data.end(), rrdata.begin(), rrdata.end());
}
for (const std::unique_ptr<DNSRR>& rr : adds_) {
std::vector<byte> rrdata = rr->data(dnsrec);
if (rrdata.size() == 0) {
data.clear();
return data;
}
data.insert(data.end(), rrdata.begin(), rrdata.end());
}
return data;

4
test/dns-proto.h vendored
Unescape View File

@ -288,17 +288,19 @@ struct DNSOption {
};
struct DNSOptRR : public DNSRR {
DNSOptRR(unsigned char extrcode, unsigned char version, unsigned short flags, int udpsize, std::vector<byte> client_cookie, std::vector<byte> server_cookie)
DNSOptRR(unsigned char extrcode, unsigned char version, unsigned short flags, int udpsize, std::vector<byte> client_cookie, std::vector<byte> server_cookie, bool expect_server_cookie)
: DNSRR("", T_OPT, static_cast<int>(udpsize), ((int)extrcode) << 24 | ((int)version) << 16 | ((int)flags)/* ttl */)
{
client_cookie_ = client_cookie;
server_cookie_ = server_cookie;
expect_server_cookie_ = expect_server_cookie;
}
virtual std::vector<byte> data(const ares_dns_record_t *dnsrec) const;
std::vector<DNSOption> opts_;
std::vector<byte> client_cookie_;
std::vector<byte> server_cookie_;
bool expect_server_cookie_;
};
struct DNSPacket {

Write Preview
Loading…
Cancel
Save