From 0f38c940ad5f1aaf3192e5a86e477c9180269f55 Mon Sep 17 00:00:00 2001 From: Tony Garnock-Jones Date: Wed, 4 Apr 2012 17:56:38 -0400 Subject: [PATCH] Detect lame delegations, and ignore servers producing such delegations. --- network-query.rkt | 107 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 94 insertions(+), 13 deletions(-) diff --git a/network-query.rkt b/network-query.rkt index 69fa417..30bff4a 100644 --- a/network-query.rkt +++ b/network-query.rkt @@ -64,6 +64,45 @@ ;; possible way. ;; +;;--------------------------------------------------------------------------- + +;; DJB's djbdns logic for determining whether a response is a lame +;; referral or not is as follows (see his query.c in areas dealing +;; with the variable "flagreferral" and calls to the function +;; "log_lame"): +;; +;; If a response - +;; +;; 1. has response-code no-error (0), and +;; 2. has no CNAME records in the answer section for the domain we're +;; interested in, and +;; 3. has no records in the answer section for the domain and type +;; we're interested in, and +;; 4. has no SOA records in the authority section, and +;; 5. has at least one NS record in the authority section, and +;; 6. that NS record's name is equal to our bailiwick or is not in our +;; bailiwick, +;; +;; then it is a lame referral. +;; +;; Anything with non-zero response-code is clearly not a referral, so +;; that explains (1). If either of checks (2) and (3) fail then the +;; answer is a real, sensible answer to the question we posed. I'm not +;; 100% on why (4) is there; presumably it's to be conservative, and +;; not treat something possibly-valid as definitely-lame? Rules (5) +;; and (6) are the real heart of lameness, where a referral is given +;; to somewhere that can't be more authoritative than the responder +;; was supposed to be. +;; +;; We modify check (4) to ignore SOA records not in bailiwick, just +;; for consistency. It's correct to leave (5) and (6) alone because +;; it's incorrect for a server to refer us to anywhere at the same +;; level of the tree or further up the tree, but we do apply them to +;; every NS record rather than just the first, which is slightly +;; stricter than DJB's rule. + +;;--------------------------------------------------------------------------- + ;; A NetworkQueryResult is a ListOf, some actions to take: ;; either involved in or resulting from completion of the network ;; query. @@ -117,7 +156,8 @@ '() '())) -;; filter-dns-reply : DNSMessage DomainName -> (or Maybe 'bad-answer) +;; filter-dns-reply : Question DNSMessage DomainName +;; -> (or Maybe 'bad-answer 'lame-delegation) ;; ;; Filters RRs from the answer, authorities, and additional sections ;; of the passed-in `message`, returning the set of RRs surviving the @@ -126,14 +166,37 @@ ;; the passed-in message's `dns-message-response-code` is `'no-error`: ;; if it's `'name-error`, then `#f` is returned, and if it's any other ;; code, `'bad-answer` is returned. -(define (filter-dns-reply message zone-origin) +;; +;; In cases where a CompleteAnswer would otherwise be returned, if the +;; answer is in fact a lame delegation (see notes above), then +;; 'lame-delegation is returned instead. +(define (filter-dns-reply q message zone-origin) (case (dns-message-response-code message) [(no-error) (define (f l) (list->set (filter (lambda (claim-rr) (in-bailiwick? (rr-name claim-rr) zone-origin)) l))) - (complete-answer (f (dns-message-answers message)) - (f (dns-message-authorities message)) - (f (dns-message-additional message)))] + ;; Here's where we do the "lame referral" check. This code is + ;; nice and simple (though wrong) without it. Ho hum. + (define answers (f (dns-message-answers message))) + (define unfiltered-authorities (dns-message-authorities message)) + (define non-subzone-ns-rrs ;; all NS authorities not for a STRICT subzone of our zone-origin + (filter (lambda (rr) (and (eqv? (rr-type rr) 'ns) + (or (equal? (rr-name rr) zone-origin) + (not (in-bailiwick? (rr-name rr) zone-origin))))) + unfiltered-authorities)) + (define authorities (f unfiltered-authorities)) + (define answers-to-q ;; answers specifically to the question we asked + (set-filter (lambda (rr) (equal? (rr-name rr) (question-name q))) answers)) + (define lame? + (and (set-empty? (filter-by-type answers-to-q 'cname)) + (set-empty? (filter-rrs answers-to-q (question-type q) (question-class q))) + (set-empty? (filter-by-type authorities 'soa)) + (not (null? non-subzone-ns-rrs)))) + (if lame? + 'lame-delegation + (complete-answer answers + authorities + (f (dns-message-additional message))))] [(name-error) #f] [else (log-info (format "Abnormal response-code ~v in response to questions ~v" @@ -163,7 +226,7 @@ (match w [(network-query-state _ #f _ _ _ _) ;; No more timeouts to try, so give up. - (on-answer w (empty-complete-answer))] + (on-answer w (empty-complete-answer) #f)] [(network-query-state req timeout _ '() _ '()) ;; No more addresses to try with this timeout. Refill the list ;; and bump the timeout and retry. @@ -211,12 +274,28 @@ current-ip) (unsubscribe rpc-id))])))])) -(define (on-answer w ans) - (if (eq? ans 'bad-answer) ;; can come from filter-dns-reply - (try-next-server w) - (transition w - (send-message (network-reply (network-request-unique-id (network-query-state-request w)) - ans))))) +(define (on-answer w ans server-ip) + (match ans + ['bad-answer ;; can come from filter-dns-reply + (try-next-server w)] + ['lame-delegation ;; can come from filter-dns-reply + (match-define (network-query-state req _ known-addresses _ current-name _) w) + (match-define (network-request _ q zone-origin _ _) req) + (log-info (format "Lame delegation received from ~v (at ~v) in bailiwick ~v in response to ~v" + current-name + server-ip + zone-origin + q)) + ;; Actually remove the offending IP address so it's never tried again. + (try-next-server (struct-copy network-query-state w + [known-addresses (hash-update known-addresses + current-name + (lambda (addrs) + (remove server-ip addrs)))]))] + [else + (transition w + (send-message (network-reply (network-request-unique-id (network-query-state-request w)) + ans)))])) (define (send-request w query-id timeout server-ip) (match-define (network-request s q zone-origin _ _) (network-query-state-request w)) @@ -252,6 +331,8 @@ (dns-message-additional reply-message))) (if (not (= (dns-message-id reply-message) (dns-message-id query))) w - (extend-transition (on-answer w (filter-dns-reply reply-message zone-origin)) + (extend-transition (on-answer w + (filter-dns-reply q reply-message zone-origin) + server-ip) (unsubscribe subscription-id) (send-message (list 'release-query-id query-id))))]))))