From patchwork Wed Oct 30 16:45:56 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Simon Marchi (Code Review)" X-Patchwork-Id: 35478 Received: (qmail 76895 invoked by alias); 30 Oct 2019 16:46:06 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 76887 invoked by uid 89); 30 Oct 2019 16:46:05 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT autolearn=ham version=3.3.1 spammy=circuit, enhanced, Transaction, amended X-HELO: mx1.osci.io X-Gerrit-PatchSet: 1 Date: Wed, 30 Oct 2019 12:45:56 -0400 From: "Florian Weimer (Code Review)" To: libc-alpha@sourceware.org Cc: Florian Weimer Message-ID: Auto-Submitted: auto-generated X-Gerrit-MessageType: newchange Subject: [review] resolv: Implement trust-ad option for /etc/resolv.conf [BZ #20358] X-Gerrit-Change-Id: Ibfe0f7c73ea221c35979842c5c3b6ed486495ccc X-Gerrit-Change-Number: 461 X-Gerrit-ChangeURL: X-Gerrit-Commit: e699c65b8c6a54b962cb27dcdad8dc45dcfa61c9 References: Reply-To: fweimer@redhat.com, fweimer@redhat.com, libc-alpha@sourceware.org MIME-Version: 1.0 Content-Disposition: inline User-Agent: Gerrit/3.0.3-75-g9005159e5d Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/461 ...................................................................... resolv: Implement trust-ad option for /etc/resolv.conf [BZ #20358] This introduces a concept of trusted name servers, for which the AD bit is passed through to applications. For untrusted name servers (the default), the AD bit in responses are cleared, to provide a safe default. This approach is very similar to the one suggested by Pavel Šimerda in . The DNS test framework in support/ is enhanced with support for setting the AD bit in responses. Tested on x86_64-linux-gnu. Change-Id: Ibfe0f7c73ea221c35979842c5c3b6ed486495ccc Reviewed-by: Carlos O'Donell --- M NEWS M resolv/Makefile M resolv/res_debug.c M resolv/res_init.c M resolv/res_mkquery.c M resolv/res_send.c M resolv/resolv.h M resolv/tst-resolv-res_init-skeleton.c A resolv/tst-resolv-trustad.c M support/resolv_test.c M support/resolv_test.h 11 files changed, 253 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index d728684..a6221a3 100644 --- a/NEWS +++ b/NEWS @@ -21,6 +21,15 @@ 18661-1:2014 and TS 18661-3:2015 as amended by the resolution of Clarification Request 13 to TS 18661-3. +* The DNS stub resolver will optionally send the AD (authenticated data) bit + in queries if the trust-ad option is set via the options directive in + /etc/resolv.conf (or if RES_TRUSTAD is set in _res.options). In this + mode, the AD bit, as provided by the name server, is available to + applications which call res_search and related functions. In the default + mode, the AD bit is not set in queries, and it is automatically cleared in + responses, indicating a lack of DNSSEC validation. (Therefore, the name + servers and the network path to them are treated as untrusted.) + Deprecated and removed features, and other changes affecting compatibility: * The totalorder and totalordermag functions, and the corresponding diff --git a/resolv/Makefile b/resolv/Makefile index 3cb64e6..cd097bd 100644 --- a/resolv/Makefile +++ b/resolv/Makefile @@ -68,6 +68,7 @@ tst-resolv-ai_idn-latin1 \ tst-resolv-ai_idn-nolibidn2 \ tst-resolv-canonname \ + tst-resolv-trustad \ # Needs resolv_context. tests-internal += \ @@ -199,6 +200,7 @@ $(libdl) $(objpfx)libresolv.so $(shared-thread-library) $(objpfx)tst-resolv-canonname: \ $(libdl) $(objpfx)libresolv.so $(shared-thread-library) +$(objpfx)tst-resolv-trustad: $(objpfx)libresolv.so $(shared-thread-library) $(objpfx)tst-ns_name: $(objpfx)libresolv.so $(objpfx)tst-ns_name.out: tst-ns_name.data diff --git a/resolv/res_debug.c b/resolv/res_debug.c index 4dac71f..844269c 100644 --- a/resolv/res_debug.c +++ b/resolv/res_debug.c @@ -612,6 +612,7 @@ case RES_USE_DNSSEC: return "dnssec"; case RES_NOTLDQUERY: return "no-tld-query"; case RES_NORELOAD: return "no-reload"; + case RES_TRUSTAD: return "trust-ad"; /* XXX nonreentrant */ default: sprintf(nbuf, "?0x%lx?", (u_long)option); return (nbuf); diff --git a/resolv/res_init.c b/resolv/res_init.c index 1b8e062..c2f2c7b 100644 --- a/resolv/res_init.c +++ b/resolv/res_init.c @@ -679,7 +679,8 @@ { STRnLEN ("no_tld_query"), 0, RES_NOTLDQUERY }, { STRnLEN ("no-tld-query"), 0, RES_NOTLDQUERY }, { STRnLEN ("no-reload"), 0, RES_NORELOAD }, - { STRnLEN ("use-vc"), 0, RES_USEVC } + { STRnLEN ("use-vc"), 0, RES_USEVC }, + { STRnLEN ("trust-ad"), 0, RES_TRUSTAD }, }; #define noptions (sizeof (options) / sizeof (options[0])) for (int i = 0; i < noptions; ++i) diff --git a/resolv/res_mkquery.c b/resolv/res_mkquery.c index cf0c9e2..0d39353 100644 --- a/resolv/res_mkquery.c +++ b/resolv/res_mkquery.c @@ -118,6 +118,8 @@ the application does multiple requests. */ hp->id = random_bits (); hp->opcode = op; + if (ctx->resp->options & RES_TRUSTAD) + hp->ad = 1; hp->rd = (ctx->resp->options & RES_RECURSE) != 0; hp->rcode = NOERROR; cp = buf + HFIXEDSZ; diff --git a/resolv/res_send.c b/resolv/res_send.c index 6b9c73f..0545d58 100644 --- a/resolv/res_send.c +++ b/resolv/res_send.c @@ -337,6 +337,15 @@ } } +/* Clear the AD bit unless the trust-ad option was specified in the + resolver configuration. */ +static void +mask_ad_bit (struct resolv_context *ctx, void *buf) +{ + if (!(ctx->resp->options & RES_TRUSTAD)) + ((HEADER *) buf)->ad = 0; +} + /* int * res_queriesmatch(buf1, eom1, buf2, eom2) * is there a 1:1 mapping of (name,type,class) @@ -530,6 +539,18 @@ resplen = n; + /* Mask the AD bit in both responses unless it is + marked trusted. */ + if (resplen > HFIXEDSZ) + { + if (ansp != NULL) + mask_ad_bit (ctx, *ansp); + else + mask_ad_bit (ctx, ans); + } + if (resplen2 != NULL && *resplen2 > HFIXEDSZ) + mask_ad_bit (ctx, *ansp2); + /* * If we have temporarily opened a virtual circuit, * or if we haven't been asked to keep a socket open, diff --git a/resolv/resolv.h b/resolv/resolv.h index 7a8023a..a039a9e 100644 --- a/resolv/resolv.h +++ b/resolv/resolv.h @@ -131,6 +131,7 @@ #define RES_NOTLDQUERY 0x01000000 /* Do not look up unqualified name as a TLD. */ #define RES_NORELOAD 0x02000000 /* No automatic configuration reload. */ +#define RES_TRUSTAD 0x04000000 /* Request AD bit, keep it in responses. */ #define RES_DEFAULT (RES_RECURSE|RES_DEFNAMES|RES_DNSRCH) diff --git a/resolv/tst-resolv-res_init-skeleton.c b/resolv/tst-resolv-res_init-skeleton.c index d4da8b6..9934d23 100644 --- a/resolv/tst-resolv-res_init-skeleton.c +++ b/resolv/tst-resolv-res_init-skeleton.c @@ -127,6 +127,7 @@ "single-request-reopen"); print_option_flag (fp, &options, RES_NOTLDQUERY, "no-tld-query"); print_option_flag (fp, &options, RES_NORELOAD, "no-reload"); + print_option_flag (fp, &options, RES_TRUSTAD, "trust-ad"); fputc ('\n', fp); if (options != 0) fprintf (fp, "; error: unresolved option bits: 0x%x\n", options); @@ -711,6 +712,15 @@ "nameserver 192.0.2.1\n" "; nameserver[0]: [192.0.2.1]:53\n" }, + {.name = "trust-ad flag", + .conf = "options trust-ad\n" + "nameserver 192.0.2.1\n", + .expected = "options trust-ad\n" + "search example.com\n" + "; search[0]: example.com\n" + "nameserver 192.0.2.1\n" + "; nameserver[0]: [192.0.2.1]:53\n" + }, { NULL } }; diff --git a/resolv/tst-resolv-trustad.c b/resolv/tst-resolv-trustad.c new file mode 100644 index 0000000..fc9ae54 --- /dev/null +++ b/resolv/tst-resolv-trustad.c @@ -0,0 +1,200 @@ +/* Test the behavior of the trust-ad option. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include +#include +#include +#include +#include +#include + +/* This controls properties of the response. volatile because + __res_send is incorrectly declared as __THROW. */ +static volatile unsigned char response_number; +static volatile bool response_ad_bit; +static volatile bool query_ad_bit; + +static void +response (const struct resolv_response_context *ctx, + struct resolv_response_builder *b, + const char *qname, uint16_t qclass, uint16_t qtype) +{ + TEST_COMPARE (qclass, C_IN); + TEST_COMPARE (qtype, T_A); + TEST_COMPARE_STRING (qname, "www.example"); + + HEADER header; + memcpy (&header, ctx->query_buffer, sizeof (header)); + TEST_COMPARE (header.ad, query_ad_bit); + + struct resolv_response_flags flags = { .ad = response_ad_bit, }; + resolv_response_init (b, flags); + resolv_response_add_question (b, qname, qclass, qtype); + resolv_response_section (b, ns_s_an); + resolv_response_open_record (b, qname, qclass, T_A, 0x12345678); + char addr[4] = { 192, 0, 2, response_number }; + resolv_response_add_data (b, addr, sizeof (addr)); + resolv_response_close_record (b); +} + +static void +check_answer (const unsigned char *buffer, size_t buffer_length, + bool expected_ad) +{ + HEADER header; + TEST_VERIFY (buffer_length > sizeof (header)); + memcpy (&header, buffer, sizeof (header)); + TEST_COMPARE (0, header.aa); + TEST_COMPARE (expected_ad, header.ad); + TEST_COMPARE (0, header.opcode); + TEST_COMPARE (1, header.qr); + TEST_COMPARE (0, header.rcode); + TEST_COMPARE (1, header.rd); + TEST_COMPARE (0, header.tc); + TEST_COMPARE (1, ntohs (header.qdcount)); + TEST_COMPARE (1, ntohs (header.ancount)); + TEST_COMPARE (0, ntohs (header.nscount)); + TEST_COMPARE (0, ntohs (header.arcount)); + + char *description = xasprintf ("response=%d ad=%d", + response_number, expected_ad); + char *expected = xasprintf ("name: www.example\n" + "address: 192.0.2.%d\n", response_number); + check_dns_packet (description, buffer, buffer_length, expected); + free (expected); + free (description); +} + +static int +do_test (void) +{ + struct resolv_test *aux = resolv_test_start + ((struct resolv_redirect_config) + { + .response_callback = response, + }); + + /* By default, the resolver is not trusted, and the AD bit is + cleared. */ + + static const unsigned char hand_crafted_query[] = + { + 10, 11, /* Transaction ID. */ + 1, 0x20, /* Query with RD, AD flags. */ + 0, 1, /* One question. */ + 0, 0, 0, 0, 0, 0, /* The other sections are empty. */ + 3, 'w', 'w', 'w', 7, 'e', 'x', 'a', 'm', 'p', 'l', 'e', 0, + 0, T_A, /* A query. */ + 0, 1, /* Class IN. */ + }; + + ++response_number; + response_ad_bit = false; + + unsigned char buffer[512]; + memset (buffer, 255, sizeof (buffer)); + query_ad_bit = true; + int ret = res_send (hand_crafted_query, sizeof (hand_crafted_query), + buffer, sizeof (buffer)); + TEST_VERIFY (ret > 0); + check_answer (buffer, ret, false); + + ++response_number; + memset (buffer, 255, sizeof (buffer)); + query_ad_bit = false; + ret = res_query ("www.example", C_IN, T_A, buffer, sizeof (buffer)); + TEST_VERIFY (ret > 0); + check_answer (buffer, ret, false); + response_ad_bit = true; + + response_ad_bit = true; + + ++response_number; + query_ad_bit = true; + ret = res_send (hand_crafted_query, sizeof (hand_crafted_query), + buffer, sizeof (buffer)); + TEST_VERIFY (ret > 0); + check_answer (buffer, ret, false); + + ++response_number; + memset (buffer, 255, sizeof (buffer)); + query_ad_bit = false; + ret = res_query ("www.example", C_IN, T_A, buffer, sizeof (buffer)); + TEST_VERIFY (ret > 0); + check_answer (buffer, ret, false); + + /* No AD bit set in generated queries. */ + memset (buffer, 255, sizeof (buffer)); + ret = res_mkquery (QUERY, "www.example", C_IN, T_A, + (const unsigned char *) "", 0, NULL, + buffer, sizeof (buffer)); + HEADER header; + memcpy (&header, buffer, sizeof (header)); + TEST_VERIFY (!header.ad); + + /* With RES_TRUSTAD, the AD bit is passed through if it set in the + response. It is also included in queries. */ + + _res.options |= RES_TRUSTAD; + query_ad_bit = true; + + response_ad_bit = false; + + ++response_number; + memset (buffer, 255, sizeof (buffer)); + ret = res_send (hand_crafted_query, sizeof (hand_crafted_query), + buffer, sizeof (buffer)); + TEST_VERIFY (ret > 0); + check_answer (buffer, ret, false); + + ++response_number; + memset (buffer, 255, sizeof (buffer)); + ret = res_query ("www.example", C_IN, T_A, buffer, sizeof (buffer)); + TEST_VERIFY (ret > 0); + check_answer (buffer, ret, false); + + response_ad_bit = true; + + ++response_number; + memset (buffer, 0, sizeof (buffer)); + ret = res_send (hand_crafted_query, sizeof (hand_crafted_query), + buffer, sizeof (buffer)); + TEST_VERIFY (ret > 0); + check_answer (buffer, ret, true); + + ++response_number; + memset (buffer, 0, sizeof (buffer)); + ret = res_query ("www.example", C_IN, T_A, buffer, sizeof (buffer)); + TEST_VERIFY (ret > 0); + check_answer (buffer, ret, true); + + /* AD bit set in generated queries. */ + memset (buffer, 0, sizeof (buffer)); + ret = res_mkquery (QUERY, "www.example", C_IN, T_A, + (const unsigned char *) "", 0, NULL, + buffer, sizeof (buffer)); + memcpy (&header, buffer, sizeof (header)); + TEST_VERIFY (header.ad); + + resolv_test_end (aux); + + return 0; +} + +#include diff --git a/support/resolv_test.c b/support/resolv_test.c index aeca519..b1c745d 100644 --- a/support/resolv_test.c +++ b/support/resolv_test.c @@ -182,6 +182,8 @@ if (flags.tc) b->buffer[2] |= 0x02; b->buffer[3] = 0x80 | flags.rcode; /* Always set RA. */ + if (flags.ad) + b->buffer[3] |= 0x20; /* Fill in the initial section count values. */ b->buffer[4] = flags.qdcount >> 8; diff --git a/support/resolv_test.h b/support/resolv_test.h index e36e3e3..02a11bd 100644 --- a/support/resolv_test.h +++ b/support/resolv_test.h @@ -134,6 +134,9 @@ /* If true, the TC (truncation) flag will be set. */ bool tc; + /* If true, the AD (authenticated data) flag will be set. */ + bool ad; + /* Initial section count values. Can be used to artificially increase the counts, for malformed packet testing.*/ unsigned short qdcount;