Handle DNSSEC downgrades better

2 files changed, 55 insertions, 11 deletions

diff --git a/dnsviz/analysis/errors.py b/dnsviz/analysis/errors.py
index da65f8f..f336045 100644
--- a/dnsviz/analysis/errors.py
+++ b/dnsviz/analysis/errors.py
@@ -1299,6 +1299,40 @@ class EDNSUndefinedFlagsSet(EDNSError):
         super(EDNSUndefinedFlagsSet, self).__init__(**kwargs)
         self.template_kwargs['flags_text'] = '0x%x' % (self.template_kwargs['flags'])
 
+class DNSSECDowngrade(EDNSError):
+    description_template = "DNSSEC was effectively downgraded because %(response_error_description)s with %(precondition)s."
+    required_params = ['response_error']
+    precondition = None
+
+    def __init__(self, *args, **kwargs):
+        super(DNSSECDowngrade, self).__init__(**kwargs)
+        self.template_kwargs['response_error_description'] = self.template_kwargs['response_error'].description[0].lower() + self.template_kwargs['response_error'].description[1:-1]
+        self.template_kwargs['precondition'] = self.precondition
+
+class DNSSECDowngradeDOBitCleared(DNSSECDowngrade):
+    '''
+    >>> e = DNSSECDowngradeDOBitCleared(response_error=Timeout(tcp=False, attempts=3))
+    >>> e.description
+    'DNSSEC was effectively downgraded because no response was received from the server over UDP (tried 3 times) with the DO bit set.'
+    '''
+
+    _abstract = False
+    code = 'DNSSEC_DOWNGRADE_DO_CLEARED'
+    precondition = 'the DO bit set'
+    references = ['RFC 4035, Sec. 3.2.1']
+
+class DNSSECDowngradeEDNSDisabled(DNSSECDowngrade):
+    '''
+    >>> e = DNSSECDowngradeEDNSDisabled(response_error=Timeout(tcp=False, attempts=3), query_specific=False)
+    >>> e.description
+    'DNSSEC was effectively downgraded because no response was received from the server over UDP (tried 3 times) with EDNS enabled.'
+    '''
+
+    _abstract = False
+    code = 'DNSSEC_DOWNGRADE_EDNS_DISABLED'
+    precondition = 'EDNS enabled'
+    references = ['RFC 6891, Sec. 7', 'RFC 2671, Sec. 5.3']
+
 class DNSCookieError(ResponseError):
     pass
 
diff --git a/dnsviz/analysis/offline.py b/dnsviz/analysis/offline.py
index eee4970..dcc22f1 100644
--- a/dnsviz/analysis/offline.py
+++ b/dnsviz/analysis/offline.py
@@ -943,7 +943,7 @@ class OfflineDomainNameAnalysis(OnlineDomainNameAnalysis):
             action_err_kwargs = {}
 
             require_valid = False
-            dnssec_downgrade = False
+            dnssec_downgrade_class = None
 
             #TODO - look for success ratio to servers due to timeout or network
             # error, for better determining if a problem is intermittent
@@ -971,11 +971,21 @@ class OfflineDomainNameAnalysis(OnlineDomainNameAnalysis):
 
             # Invalid RCODE - kwargs: rcode; require a valid response
             elif retry.cause == Q.RETRY_CAUSE_RCODE:
+                # If the RCODE was FORMERR, SERVFAIL, or NOTIMP, then this is a
+                # signal to the client that the server doesn't support EDNS.
+                # Thus, *independent of action*, we mark this as a DNSSEC
+                # downgrade, if the zone is signed.
+                if retry.cause_arg in (dns.rcode.FORMERR, dns.rcode.SERVFAIL, dns.rcode.NOTIMP) and \
+                        qname_obj is not None and qname_obj.zone.signed:
+                    dnssec_downgrade_class = Errors.DNSSECDowngradeEDNSDisabled
+
                 # if the RCODE was FORMERR, SERVFAIL, or NOTIMP, and the
                 # corresponding action was to disable EDNS, then this was a
-                # reasonable response from a server that doesn't support EDNS
+                # reasonable response from a server that doesn't support EDNS,
+                # but it's only innocuous if the zone is not signed.
                 if retry.cause_arg in (dns.rcode.FORMERR, dns.rcode.SERVFAIL, dns.rcode.NOTIMP) and \
-                        retry.action == Q.RETRY_ACTION_DISABLE_EDNS:
+                        retry.action == Q.RETRY_ACTION_DISABLE_EDNS and \
+                        not (qname_obj is not None and qname_obj.zone.signed):
                     pass
 
                 # or if the RCODE was BADVERS, and the corresponding action was
@@ -1002,7 +1012,6 @@ class OfflineDomainNameAnalysis(OnlineDomainNameAnalysis):
                         retry.action == Q.RETRY_ACTION_UPDATE_DNS_COOKIE:
                     pass
 
-
                 # or if the RCODE was FORMERR, and the COOKIE opt we sent
                 # contained a malformed cookie, then this was a reasonable
                 # response from a server that supports cookies
@@ -1058,7 +1067,7 @@ class OfflineDomainNameAnalysis(OnlineDomainNameAnalysis):
                 action_err_class = Errors.ResponseErrorWithEDNS
 
                 # DNSSEC was downgraded because DO bit is no longer available
-                dnssec_downgrade = True
+                dnssec_downgrade_class = Errors.DNSSECDowngradeEDNSDisabled
 
             # The EDNS UDP max payload size was changed to elicit a response;
             # kwargs: pmtu_lower_bound, pmtu_upper_bound
@@ -1084,7 +1093,7 @@ class OfflineDomainNameAnalysis(OnlineDomainNameAnalysis):
 
                 # if this was the DO flag, then DNSSEC was downgraded
                 if retry.action_arg == dns.flags.DO:
-                    dnssec_downgrade = True
+                    dnssec_downgrade_class = Errors.DNSSECDowngradeDOBitCleared
 
             # An EDNS option was added to elicit a response; kwargs: option
             elif retry.action == Q.RETRY_ACTION_ADD_EDNS_OPTION:
@@ -1115,16 +1124,17 @@ class OfflineDomainNameAnalysis(OnlineDomainNameAnalysis):
                     query_specific = True
                 else:
                     query_specific = False
-                change_err = action_err_class(response_error=cause_err_class(**cause_err_kwargs), query_specific=query_specific, **action_err_kwargs)
+                cause_err = cause_err_class(**cause_err_kwargs)
+                change_err = action_err_class(response_error=cause_err, query_specific=query_specific, **action_err_kwargs)
 
         if change_err is not None:
             # if the error really matters (e.g., due to DNSSEC), note an error
-            if dnssec_downgrade and qname_obj is not None and qname_obj.zone.signed:
-                group = errors
+            if dnssec_downgrade_class is not None and qname_obj is not None and qname_obj.zone.signed:
+                Errors.DomainNameAnalysisError.insert_into_list(change_err, errors, server, client, response)
+                Errors.DomainNameAnalysisError.insert_into_list(dnssec_downgrade_class(response_error=cause_err), errors, server, client, response)
             # otherwise, warn
             else:
-                group = warnings
-            Errors.DomainNameAnalysisError.insert_into_list(change_err, group, server, client, response)
+                Errors.DomainNameAnalysisError.insert_into_list(change_err, warnings, server, client, response)
 
     def _populate_edns_errors(self, qname_obj, response, server, client, warnings, errors):