This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Skip logging for additional DNSSEC records from RFC4034 [BZ 14841]
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Siddhesh Poyarekar <siddhesh at redhat dot com>, 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.