Carlos O'Donell - Re: [PATCH] Skip logging for additional DNSSEC records from RFC4034 [BZ (original) (raw)

This is the mail archive of the libc-alpha@sourceware.orgmailing list for the glibc project.

• From: "Carlos O'Donell"
• To: Siddhesh Poyarekar , libc-alpha at sourceware dot org
• Date: Thu, 19 Feb 2015 12:06:04 -0500
• Subject: Re: [PATCH] Skip logging for additional DNSSEC records from RFC4034 [BZ 14841]
• Authentication-results: sourceware.org; auth=none
• References: <20150219170031 dot GA14158 at spoyarek dot pnq dot redhat dot com>

On 02/19/2015 12:00 PM, Siddhesh Poyarekar wrote:

RFC 4034 specifies 3 more record types (RRSIG, NSEC, DNSKEY) that the glibc resolver does not identify. The resolver would log a message in syslog if debugging is enabled in resolv.conf and RES_USE_DNSSEC is set in the _res struct. This was fine before since we did not set the DO bit, but we do so now, so skip logging the message when we have requested DNSSEC.

Exactly, there is no reason to log anything if we are asking for DNSSEC support.

I have not added the identifiers to res_debug.c on purpose - it serves no value other than adding an ABI event for libresolv since we're just ignoring these records.

Agreed (somewhat, see below).

Tested on x86_64.

[BZ #14841]
* resolv/arpa/nameser.h (__ns_type): Add ns_t_rrsig, ns_t_nsec
and ns_t_dnskey.
* resolv/arpa/nameser_compat.h (T_RRSIG, T_NSEC, T_DNSKEY):
Define.
* resolv/gethnamaddr.c (getanswer): Use them to ignore DNSSEC
records.  Skip logging if RES_USE_DNSSEC is set.
* resolv/nss_dns/dns-host.c (getanswer_r): Likewise.

Almost there.

One request:

Add comments to res_debug.c to indicate that these identifiers need to be added when required. This way a reviewer does not see them missing and wonders why they weren't added.

---

resolv/arpa/nameser.h | 3 +++ resolv/arpa/nameser_compat.h | 3 +++ resolv/gethnamaddr.c | 21 +++++++++++++-------- resolv/nss_dns/dns-host.c | 19 +++++++++++++------ 4 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/resolv/arpa/nameser.h b/resolv/arpa/nameser.h index fb8513b..372d5cd 100644 --- a/resolv/arpa/nameser.h +++ b/resolv/arpa/nameser.h @@ -293,6 +293,9 @@ typedef enum __ns_type { ns_t_sink = 40, /%< Kitchen sink (experimentatl) / ns_t_opt = 41, /%< EDNS0 option (meta-RR) / ns_t_apl = 42, /%< Address prefix list (RFC3123) / + ns_t_rrsig = 46, /%< DNSSEC RRset Signature (RFC4034) / + ns_t_nsec = 47, /%< DNSSEC Next-Secure Record (RFC4034)/ + ns_t_dnskey = 48, /*%< DNSSEC key record (RFC4034) */

OK.

 ns_t_tkey = 249,	/*%< Transaction key */
 ns_t_tsig = 250,	/*%< Transaction signature. */
 ns_t_ixfr = 251,	/*%< Incremental zone transfer. */

diff --git a/resolv/arpa/nameser_compat.h b/resolv/arpa/nameser_compat.h index d59c9e4..284bff7 100644 --- a/resolv/arpa/nameser_compat.h +++ b/resolv/arpa/nameser_compat.h @@ -164,6 +164,9 @@ typedef struct { #define T_NAPTR ns_t_naptr #define T_A6 ns_t_a6 #define T_DNAME ns_t_dname +#define T_RRSIG ns_t_rrsig +#define T_NSEC ns_t_nsec +#define T_DNSKEY ns_t_dnskey

OK.

#define T_TSIG ns_t_tsig #define T_IXFR ns_t_ixfr #define T_AXFR ns_t_axfr diff --git a/resolv/gethnamaddr.c b/resolv/gethnamaddr.c index a861a84..9e0c498 100644 --- a/resolv/gethnamaddr.c +++ b/resolv/gethnamaddr.c @@ -331,15 +331,20 @@ getanswer (const querybuf *answer, int anslen, const char qname, int qtype) buflen -= n; continue; } - if ((type == T_SIG) || (type == T_KEY) || (type == T_NXT)) { - / We don't support DNSSEC yet. For now, ignore - * the record and send a low priority message - * to syslog. - / - syslog(LOG_DEBUG|LOG_AUTH, + if ((type == T_SIG) || (type == T_KEY) || (type == T_NXT) + || (type == T_RRSIG) || (type == T_NSEC) + || (type == T_DNSKEY)) { + / We don't support DNSSEC responses yet, but we do + * allow setting the DO bit. If the DNS server sent us + * these records without us asking for it, ignore the + * record and send a low priority message to syslog. + / + if ((_res.options & RES_USE_DNSSEC) == 0) { + syslog(LOG_DEBUG|LOG_AUTH, "gethostby.getanswer: asked for "%s %s %s", got type "%s"", - qname, p_class(C_IN), p_type(qtype), - p_type(type)); + qname, p_class(C_IN), p_type(qtype), + p_type(type)); + }

OK.

         cp += n;
         continue;
     }

diff --git a/resolv/nss_dns/dns-host.c b/resolv/nss_dns/dns-host.c index f715ab0..b10c94e 100644 --- a/resolv/nss_dns/dns-host.c +++ b/resolv/nss_dns/dns-host.c @@ -822,13 +822,20 @@ getanswer_r (const querybuf *answer, int anslen, const char *qname, int qtype, } if (__builtin_expect (type == T_SIG, 0) || __builtin_expect (type == T_KEY, 0) - || __builtin_expect (type == T_NXT, 0)) + || __builtin_expect (type == T_NXT, 0) + || __builtin_expect (type == T_RRSIG, 0) + || __builtin_expect (type == T_NSEC, 0) + || __builtin_expect (type == T_DNSKEY, 0))

OK.

 {

•  /* We don't support DNSSEC yet.  For now, ignore the record

•     and send a low priority message to syslog.  */

•  syslog (LOG_DEBUG | LOG_AUTH,

•       "gethostby*.getanswer: asked for \"%s %s %s\", got type \"%s\"",

•     qname, p_class (C_IN), p_type(qtype), p_type (type));

•  /* We don't support DNSSEC responses yet, but we do allow setting the

•     DO bit.  If the DNS server sent us these records without us asking

•     for it, ignore the record and send a low priority message to

•     syslog.  */

•  if ((_res.options & RES_USE_DNSSEC) == 0)

•    syslog (LOG_DEBUG | LOG_AUTH,

•       "gethostby*.getanswer: asked for \"%s %s %s\", "

•       "got type \"%s\"",

•       qname, p_class (C_IN), p_type(qtype), p_type (type));

OK.

   cp += n;
   continue;
 }

Cheers, Carlos.

• References:
  • [PATCH] Skip logging for additional DNSSEC records from RFC4034 [BZ 14841]
    * From: Siddhesh Poyarekar