From 6f0cec2514ce07333540b12dbcf4d7c430f489eb Mon Sep 17 00:00:00 2001 From: Casey Deccio Date: Thu, 7 Jan 2021 14:00:49 -0700 Subject: Fix typo --- dnsviz/analysis/offline.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dnsviz/analysis/offline.py b/dnsviz/analysis/offline.py index 7d08a83..a488e8e 100644 --- a/dnsviz/analysis/offline.py +++ b/dnsviz/analysis/offline.py @@ -2112,7 +2112,7 @@ class OfflineDomainNameAnalysis(OnlineDomainNameAnalysis): # is bogus because there should have been matching KSK. self.delegation_status[rdtype] = Status.DELEGATION_STATUS_BOGUS else: - # If no algorithsm are supported, then this is a + # If no algorithms are supported, then this is a # provably insecure delegation. self.delegation_status[rdtype] = Status.DELEGATION_STATUS_INSECURE else: -- cgit v1.2.3 From 2ce35fefb8d910cfff9eee652e5678bff2ff9c97 Mon Sep 17 00:00:00 2001 From: Casey Deccio Date: Thu, 7 Jan 2021 14:12:43 -0700 Subject: Sort out supported_algs in a separate method --- dnsviz/analysis/offline.py | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/dnsviz/analysis/offline.py b/dnsviz/analysis/offline.py index a488e8e..8b458df 100644 --- a/dnsviz/analysis/offline.py +++ b/dnsviz/analysis/offline.py @@ -27,6 +27,7 @@ from __future__ import unicode_literals +import copy import errno import logging @@ -787,7 +788,7 @@ class OfflineDomainNameAnalysis(OnlineDomainNameAnalysis): (not x.effective_tcp and x.udp_responsive)) and \ (not require_valid or x.is_valid_response())) - def populate_status(self, trusted_keys, supported_algs=None, supported_digest_algs=None, is_dlv=False, trace=None, follow_mx=True): + def _populate_status(self, trusted_keys, supported_algs=None, supported_digest_algs=None, is_dlv=False, trace=None, follow_mx=True): if trace is None: trace = [] @@ -804,40 +805,29 @@ class OfflineDomainNameAnalysis(OnlineDomainNameAnalysis): if self.stub: return - # identify supported algorithms as intersection of explicitly supported - # and software supported - if supported_algs is not None: - supported_algs.intersection_update(crypto._supported_algs) - else: - supported_algs = crypto._supported_algs - if supported_digest_algs is not None: - supported_digest_algs.intersection_update(crypto._supported_digest_algs) - else: - supported_digest_algs = crypto._supported_digest_algs - # populate status of dependencies for cname in self.cname_targets: for target, cname_obj in self.cname_targets[cname].items(): if cname_obj is not None: - cname_obj.populate_status(trusted_keys, supported_algs, supported_digest_algs, trace=trace + [self]) + cname_obj._populate_status(trusted_keys, supported_algs, supported_digest_algs, trace=trace + [self]) if follow_mx: for target, mx_obj in self.mx_targets.items(): if mx_obj is not None: - mx_obj.populate_status(trusted_keys, supported_algs, supported_digest_algs, trace=trace + [self], follow_mx=False) + mx_obj._populate_status(trusted_keys, supported_algs, supported_digest_algs, trace=trace + [self], follow_mx=False) for signer, signer_obj in self.external_signers.items(): if signer_obj is not None: - signer_obj.populate_status(trusted_keys, supported_algs, supported_digest_algs, trace=trace + [self]) + signer_obj._populate_status(trusted_keys, supported_algs, supported_digest_algs, trace=trace + [self]) for target, ns_obj in self.ns_dependencies.items(): if ns_obj is not None: - ns_obj.populate_status(trusted_keys, supported_algs, supported_digest_algs, trace=trace + [self]) + ns_obj._populate_status(trusted_keys, supported_algs, supported_digest_algs, trace=trace + [self]) # populate status of ancestry if self.nxdomain_ancestor is not None: - self.nxdomain_ancestor.populate_status(trusted_keys, supported_algs, supported_digest_algs, trace=trace + [self]) + self.nxdomain_ancestor._populate_status(trusted_keys, supported_algs, supported_digest_algs, trace=trace + [self]) if self.parent is not None: - self.parent.populate_status(trusted_keys, supported_algs, supported_digest_algs, trace=trace + [self]) + self.parent._populate_status(trusted_keys, supported_algs, supported_digest_algs, trace=trace + [self]) if self.dlv_parent is not None: - self.dlv_parent.populate_status(trusted_keys, supported_algs, supported_digest_algs, is_dlv=True, trace=trace + [self]) + self.dlv_parent._populate_status(trusted_keys, supported_algs, supported_digest_algs, is_dlv=True, trace=trace + [self]) _logger.debug('Assessing status of %s...' % (fmt.humanize_name(self.name))) self._populate_name_status() @@ -853,6 +843,20 @@ class OfflineDomainNameAnalysis(OnlineDomainNameAnalysis): self._populate_ds_status(dns.rdatatype.DLV, supported_algs, supported_digest_algs) self._populate_dnskey_status(trusted_keys) + def populate_status(self, trusted_keys, supported_algs=None, supported_digest_algs=None, is_dlv=False, follow_mx=True): + # identify supported algorithms as intersection of explicitly supported + # and software supported + if supported_algs is not None: + supported_algs.intersection_update(crypto._supported_algs) + else: + supported_algs = copy.copy(crypto._supported_algs) + if supported_digest_algs is not None: + supported_digest_algs.intersection_update(crypto._supported_digest_algs) + else: + supported_digest_algs = copy.copy(crypto._supported_digest_algs) + + self._populate_status(trusted_keys, supported_algs, supported_digest_algs, is_dlv, None, follow_mx) + def _populate_name_status(self, trace=None): # using trace allows _populate_name_status to be called independent of # populate_status -- cgit v1.2.3 From 992baacead282d4f927cdc2ac56a2ba0005e8457 Mon Sep 17 00:00:00 2001 From: Casey Deccio Date: Thu, 7 Jan 2021 14:19:54 -0700 Subject: Consider algorithms that MUST NOT be validated See #63 and RFC 8624. --- dnsviz/analysis/errors.py | 36 +++++++++++++++++++++++++ dnsviz/analysis/status.py | 69 ++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 99 insertions(+), 6 deletions(-) diff --git a/dnsviz/analysis/errors.py b/dnsviz/analysis/errors.py index c9da104..27a21ce 100644 --- a/dnsviz/analysis/errors.py +++ b/dnsviz/analysis/errors.py @@ -252,6 +252,25 @@ class AlgorithmNotSupported(RRSIGError): super(AlgorithmNotSupported, self).__init__(**kwargs) self.template_kwargs['algorithm_text'] = dns.dnssec.algorithm_to_text(self.template_kwargs['algorithm']) +class AlgorithmMustNotValidate(RRSIGError): + ''' + >>> e = AlgorithmMustNotValidate(algorithm=5) + >>> e.args + [5] + >>> e.description + 'DNSSEC specification prohibits validation of RRSIGs with DNSSEC algorithm 5 (RSASHA1).' + ''' + + _abstract = False + code = 'ALGORITHM_MUST_NOT_VALIDATE' + description_template = "DNSSEC specification prohibits validation of RRSIGs with DNSSEC algorithm %(algorithm)d (%(algorithm_text)s)." + references = ['RFC 8624, Sec. 3.1'] + required_params = ['algorithm'] + + def __init__(self, **kwargs): + super(AlgorithmMustNotValidate, self).__init__(**kwargs) + self.template_kwargs['algorithm_text'] = dns.dnssec.algorithm_to_text(self.template_kwargs['algorithm']) + class DNSKEYRevokedRRSIG(RRSIGError): ''' >>> e = DNSKEYRevokedRRSIG() @@ -514,6 +533,23 @@ class DigestAlgorithmNotSupported(DSDigestError): super(DigestAlgorithmNotSupported, self).__init__(**kwargs) self.template_kwargs['algorithm_text'] = fmt.DS_DIGEST_TYPES.get(self.template_kwargs['algorithm'], self.template_kwargs['algorithm']) +class DigestAlgorithmMustNotValidate(DSDigestError): + ''' + >>> e = DigestAlgorithmMustNotValidate(algorithm=5) + >>> e.description + 'DNSSEC specification prohibits validation of DS records that use digest algorithm 5 (5).' + ''' + + _abstract = False + code = 'DIGEST_ALGORITHM_MUST_NOT_VALIDATE' + description_template = "DNSSEC specification prohibits validation of DS records that use digest algorithm %(algorithm)d (%(algorithm_text)s)." + references = ['RFC 8624, Sec. 3.2'] + required_params = ['algorithm'] + + def __init__(self, **kwargs): + super(DigestAlgorithmMustNotValidate, self).__init__(**kwargs) + self.template_kwargs['algorithm_text'] = fmt.DS_DIGEST_TYPES.get(self.template_kwargs['algorithm'], self.template_kwargs['algorithm']) + class DNSKEYRevokedDS(DSDigestError): ''' >>> e = DNSKEYRevokedDS() diff --git a/dnsviz/analysis/status.py b/dnsviz/analysis/status.py index a2197a8..5ba2595 100644 --- a/dnsviz/analysis/status.py +++ b/dnsviz/analysis/status.py @@ -169,6 +169,16 @@ RRSIG_SIG_LENGTH_ERRORS = { DS_DIGEST_ALGS_STRONGER_THAN_SHA1 = (2, 4) DS_DIGEST_ALGS_IGNORING_SHA1 = (2,) +# RFC 8624 Section 3.1 +DNSKEY_ALGS_NOT_RECOMMENDED = (5, 7, 10) +DNSKEY_ALGS_MUST_NOT_SIGN = (1, 3, 6, 12) +DNSKEY_ALGS_MUST_NOT_VALIDATE = (1, 3, 6) + +# RFC 8624 Section 3.2 +DS_DIGEST_ALGS_NOT_RECOMMENDED = () +DS_DIGEST_ALGS_MUST_NOT_SIGN = (0, 1, 3) +DS_DIGEST_ALGS_MUST_NOT_VALIDATE = () + class RRSIGStatus(object): def __init__(self, rrset, rrsig, dnskey, zone_name, reference_ts, supported_algs): self.rrset = rrset @@ -186,13 +196,37 @@ class RRSIGStatus(object): self.validation_status = RRSIG_STATUS_VALID if self.signature_valid is None or self.dnskey.rdata.algorithm not in supported_algs: + # Either we can't validate the cryptographic signature, or we are + # explicitly directed to ignore the algorithm. if self.dnskey is None: + # In this case, there is no corresponding DNSKEY, so we make + # the status "INDETERMINATE". if self.validation_status == RRSIG_STATUS_VALID: self.validation_status = RRSIG_STATUS_INDETERMINATE_NO_DNSKEY + else: - if self.validation_status == RRSIG_STATUS_VALID: - self.validation_status = RRSIG_STATUS_INDETERMINATE_UNKNOWN_ALGORITHM - self.warnings.append(Errors.AlgorithmNotSupported(algorithm=self.rrsig.algorithm)) + # If there is a DNSKEY, then we look at *why* we are ignoring + # the cryptographic signature. + if self.dnskey.rdata.algorithm in DNSKEY_ALGS_MUST_NOT_VALIDATE: + # In this case, specification dictates that the algorithm + # MUST NOT be validated, so we mark it as ignored. + if self.validation_status == RRSIG_STATUS_VALID: + self.validation_status = RRSIG_STATUS_ALGORITHM_IGNORED + else: + # In this case, we can't validate this particular + # algorithm, either because the code doesn't support it, + # or because we have been explicitly directed to ignore it. + # In either case, mark it as "UNKNOWN", and warn that it is + # not supported. + if self.validation_status == RRSIG_STATUS_VALID: + self.validation_status = RRSIG_STATUS_INDETERMINATE_UNKNOWN_ALGORITHM + self.warnings.append(Errors.AlgorithmNotSupported(algorithm=self.rrsig.algorithm)) + + # Independent of whether or not we considered the cryptographic + # validation, issue a warning if we are using an algorithm for which + # validation has been prohibited. + if self.dnskey.rdata.algorithm in DNSKEY_ALGS_MUST_NOT_VALIDATE: + self.warnings.append(Errors.AlgorithmMustNotValidate(algorithm=self.rrsig.algorithm)) if self.rrset.ttl_cmp: if self.rrset.rrset.ttl != self.rrset.rrsig_info[self.rrsig].ttl: @@ -351,13 +385,36 @@ class DSStatus(object): self.validation_status = DS_STATUS_VALID if self.digest_valid is None or self.ds.digest_type not in supported_digest_algs: + # Either we cannot reproduce a digest with this type, or we are + # explicitly directed to ignore the digest type. if self.dnskey is None: + # In this case, there is no corresponding DNSKEY, so we make + # the status "INDETERMINATE". if self.validation_status == DS_STATUS_VALID: self.validation_status = DS_STATUS_INDETERMINATE_NO_DNSKEY else: - if self.validation_status == DS_STATUS_VALID: - self.validation_status = DS_STATUS_INDETERMINATE_UNKNOWN_ALGORITHM - self.warnings.append(Errors.DigestAlgorithmNotSupported(algorithm=ds.digest_type)) + # If there is a DNSKEY, then we look at *why* we are ignoring + # the digest of the DNSKEY. + if self.ds.digest_type in DS_DIGEST_ALGS_MUST_NOT_VALIDATE: + # In this case, specification dictates that the algorithm + # MUST NOT be validated, so we mark it as ignored. + if self.validation_status == DS_STATUS_VALID: + self.validation_status = DS_STATUS_ALGORITHM_IGNORED + else: + # In this case, we can't validate this particular + # digest type, either because the code doesn't support it, + # or because we have been explicitly directed to ignore it. + # In either case, mark it as "UNKNOWN", and warn that it is + # not supported. + if self.validation_status == DS_STATUS_VALID: + self.validation_status = DS_STATUS_INDETERMINATE_UNKNOWN_ALGORITHM + self.warnings.append(Errors.DigestAlgorithmNotSupported(algorithm=self.ds.digest_type)) + + # Independent of whether or not we considered the digest for + # validation, issue a warning if we are using a digest type for which + # validation has been prohibited. + if self.ds.digest_type in DS_DIGEST_ALGS_MUST_NOT_VALIDATE: + self.warnings.append(Errors.DigestAlgorithmMustNotValidate(algorithm=self.ds.digest_type)) if self.dnskey is not None and \ self.dnskey.rdata.flags & fmt.DNSKEY_FLAGS['revoke']: -- cgit v1.2.3 From 8f4080c53cf7fe10b0a8eac59e94a796ab3d99fb Mon Sep 17 00:00:00 2001 From: Casey Deccio Date: Thu, 7 Jan 2021 14:46:53 -0700 Subject: Allow prohibited algorithms to be considered with command-line option --- dnsviz/analysis/offline.py | 7 ++++++- dnsviz/commands/graph.py | 6 +++++- dnsviz/commands/grok.py | 6 +++++- dnsviz/commands/print.py | 6 +++++- doc/man/dnsviz-graph.1 | 8 ++++++++ doc/man/dnsviz-grok.1 | 8 ++++++++ doc/man/dnsviz-print.1 | 8 ++++++++ 7 files changed, 45 insertions(+), 4 deletions(-) diff --git a/dnsviz/analysis/offline.py b/dnsviz/analysis/offline.py index 8b458df..dbea3db 100644 --- a/dnsviz/analysis/offline.py +++ b/dnsviz/analysis/offline.py @@ -843,7 +843,7 @@ class OfflineDomainNameAnalysis(OnlineDomainNameAnalysis): self._populate_ds_status(dns.rdatatype.DLV, supported_algs, supported_digest_algs) self._populate_dnskey_status(trusted_keys) - def populate_status(self, trusted_keys, supported_algs=None, supported_digest_algs=None, is_dlv=False, follow_mx=True): + def populate_status(self, trusted_keys, supported_algs=None, supported_digest_algs=None, is_dlv=False, follow_mx=True, validate_prohibited_algs=False): # identify supported algorithms as intersection of explicitly supported # and software supported if supported_algs is not None: @@ -855,6 +855,11 @@ class OfflineDomainNameAnalysis(OnlineDomainNameAnalysis): else: supported_digest_algs = copy.copy(crypto._supported_digest_algs) + # unless we are overriding, mark prohibited algorithms as not supported + if not validate_prohibited_algs: + supported_algs.difference_update(Status.DNSKEY_ALGS_MUST_NOT_VALIDATE) + supported_digest_algs.difference_update(Status.DS_DIGEST_ALGS_MUST_NOT_VALIDATE) + self._populate_status(trusted_keys, supported_algs, supported_digest_algs, is_dlv, None, follow_mx) def _populate_name_status(self, trace=None): diff --git a/dnsviz/commands/graph.py b/dnsviz/commands/graph.py index 8beb188..13558e0 100644 --- a/dnsviz/commands/graph.py +++ b/dnsviz/commands/graph.py @@ -198,6 +198,10 @@ class GraphArgHelper: type=self.comma_separated_ints_set, action='store', metavar=',[...]', help='Support only the specified DNSSEC digest algorithm(s)') + self.parser.add_argument('-b', '--validate-prohibited-algs', + const=True, default=False, + action='store_const', + help='Validate algorithms for which validation is otherwise prohibited') self.parser.add_argument('-C', '--enforce-cookies', const=True, default=False, action='store_const', @@ -457,7 +461,7 @@ def main(argv): G = DNSAuthGraph() for name_obj in name_objs: - name_obj.populate_status(arghelper.trusted_keys, supported_algs=arghelper.args.algorithms, supported_digest_algs=arghelper.args.digest_algorithms) + name_obj.populate_status(arghelper.trusted_keys, supported_algs=arghelper.args.algorithms, supported_digest_algs=arghelper.args.digest_algorithms, validate_prohibited_algs=arghelper.args.validate_prohibited_algs) for qname, rdtype in name_obj.queries: if arghelper.args.rr_types is None: # if rdtypes was not specified, then graph all, with some diff --git a/dnsviz/commands/grok.py b/dnsviz/commands/grok.py index 37e612f..14fefc1 100644 --- a/dnsviz/commands/grok.py +++ b/dnsviz/commands/grok.py @@ -221,6 +221,10 @@ class GrokArgHelper: type=self.comma_separated_ints_set, action='store', metavar=',[...]', help='Support only the specified DNSSEC digest algorithm(s)') + self.parser.add_argument('-b', '--validate-prohibited-algs', + const=True, default=False, + action='store_const', + help='Validate algorithms for which validation is otherwise prohibited') self.parser.add_argument('-C', '--enforce-cookies', const=True, default=False, action='store_const', @@ -454,7 +458,7 @@ def main(argv): d = OrderedDict() for name_obj in name_objs: - name_obj.populate_status(arghelper.trusted_keys, supported_algs=arghelper.args.algorithms, supported_digest_algs=arghelper.args.digest_algorithms) + name_obj.populate_status(arghelper.trusted_keys, supported_algs=arghelper.args.algorithms, supported_digest_algs=arghelper.args.digest_algorithms, validate_prohibited_algs=arghelper.args.validate_prohibited_algs) if arghelper.trusted_keys: G = DNSAuthGraph() diff --git a/dnsviz/commands/print.py b/dnsviz/commands/print.py index d2e4524..ceffdfc 100644 --- a/dnsviz/commands/print.py +++ b/dnsviz/commands/print.py @@ -356,6 +356,10 @@ class PrintArgHelper: type=self.comma_separated_ints_set, action='store', metavar=',[...]', help='Support only the specified DNSSEC digest algorithm(s)') + self.parser.add_argument('-b', '--validate-prohibited-algs', + const=True, default=False, + action='store_const', + help='Validate algorithms for which validation is otherwise prohibited') self.parser.add_argument('-C', '--enforce-cookies', const=True, default=False, action='store_const', @@ -590,7 +594,7 @@ def main(argv): G = DNSAuthGraph() for name_obj in name_objs: - name_obj.populate_status(arghelper.trusted_keys, supported_algs=arghelper.args.algorithms, supported_digest_algs=arghelper.args.digest_algorithms) + name_obj.populate_status(arghelper.trusted_keys, supported_algs=arghelper.args.algorithms, supported_digest_algs=arghelper.args.digest_algorithms, validate_prohibited_algs=arghelper.args.validate_prohibited_algs) for qname, rdtype in name_obj.queries: if arghelper.args.rr_types is None: # if rdtypes was not specified, then graph all, with some diff --git a/doc/man/dnsviz-graph.1 b/doc/man/dnsviz-graph.1 index 2e49755..6971750 100644 --- a/doc/man/dnsviz-graph.1 +++ b/doc/man/dnsviz-graph.1 @@ -93,6 +93,14 @@ unknown. Additionally, when a zone has only DS records with unsupported digest algorithms, the zone is treated as "insecure", assuming the DS records are properly authenticated. .TP +.B -b, --validate-prohibited-algs +Validate algorithms for which validation is otherwise prohibited. Current +DNSSEC specification prohibits validators from validating older, weaker +algorithms associated with DNSKEY and DS records (see RFC 8624). If this +option is used, then a warning will be still be issued for DNSSEC records that +use these older algorithms, but the code will still assess their cryptographic +status, rather than ignoring them. +.TP .B -C, --enforce-cookies Enforce DNS cookies strictly. Require a server to return a "BADCOOKIE" response when a query contains a COOKIE option with no server cookie or with an invalid diff --git a/doc/man/dnsviz-grok.1 b/doc/man/dnsviz-grok.1 index c6773f7..c9a441d 100644 --- a/doc/man/dnsviz-grok.1 +++ b/doc/man/dnsviz-grok.1 @@ -89,6 +89,14 @@ unknown. Additionally, when a zone has only DS records with unsupported digest algorithms, the zone is treated as "insecure", assuming the DS records are properly authenticated. .TP +.B -b, --validate-prohibited-algs +Validate algorithms for which validation is otherwise prohibited. Current +DNSSEC specification prohibits validators from validating older, weaker +algorithms associated with DNSKEY and DS records (see RFC 8624). If this +option is used, then a warning will be still be issued for DNSSEC records that +use these older algorithms, but the code will still assess their cryptographic +status, rather than ignoring them. +.TP .B -C, --enforce-cookies Enforce DNS cookies strictly. Require a server to return a "BADCOOKIE" response when a query contains a COOKIE option with no server cookie or with an invalid diff --git a/doc/man/dnsviz-print.1 b/doc/man/dnsviz-print.1 index 0499e1d..6091405 100644 --- a/doc/man/dnsviz-print.1 +++ b/doc/man/dnsviz-print.1 @@ -93,6 +93,14 @@ unknown. Additionally, when a zone has only DS records with unsupported digest algorithms, the zone is treated as "insecure", assuming the DS records are properly authenticated. .TP +.B -b, --validate-prohibited-algs +Validate algorithms for which validation is otherwise prohibited. Current +DNSSEC specification prohibits validators from validating older, weaker +algorithms associated with DNSKEY and DS records (see RFC 8624). If this +option is used, then a warning will be still be issued for DNSSEC records that +use these older algorithms, but the code will still assess their cryptographic +status, rather than ignoring them. +.TP .B -C, --enforce-cookies Enforce DNS cookies strictly. Require a server to return a "BADCOOKIE" response when a query contains a COOKIE option with no server cookie or with an invalid -- cgit v1.2.3 From d5c8fda453a9355280f18a2de0dee68e79133415 Mon Sep 17 00:00:00 2001 From: Casey Deccio Date: Thu, 7 Jan 2021 20:21:27 -0700 Subject: Make wording more intuitive --- dnsviz/analysis/errors.py | 16 ++++++++-------- dnsviz/analysis/offline.py | 4 ++-- dnsviz/analysis/status.py | 20 ++++++++++---------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/dnsviz/analysis/errors.py b/dnsviz/analysis/errors.py index 27a21ce..b01b342 100644 --- a/dnsviz/analysis/errors.py +++ b/dnsviz/analysis/errors.py @@ -252,9 +252,9 @@ class AlgorithmNotSupported(RRSIGError): super(AlgorithmNotSupported, self).__init__(**kwargs) self.template_kwargs['algorithm_text'] = dns.dnssec.algorithm_to_text(self.template_kwargs['algorithm']) -class AlgorithmMustNotValidate(RRSIGError): +class AlgorithmValidationProhibited(RRSIGError): ''' - >>> e = AlgorithmMustNotValidate(algorithm=5) + >>> e = AlgorithmValidationProhibited(algorithm=5) >>> e.args [5] >>> e.description @@ -262,13 +262,13 @@ class AlgorithmMustNotValidate(RRSIGError): ''' _abstract = False - code = 'ALGORITHM_MUST_NOT_VALIDATE' + code = 'ALGORITHM_VALIDATION_PROHIBITED' description_template = "DNSSEC specification prohibits validation of RRSIGs with DNSSEC algorithm %(algorithm)d (%(algorithm_text)s)." references = ['RFC 8624, Sec. 3.1'] required_params = ['algorithm'] def __init__(self, **kwargs): - super(AlgorithmMustNotValidate, self).__init__(**kwargs) + super(AlgorithmValidationProhibited, self).__init__(**kwargs) self.template_kwargs['algorithm_text'] = dns.dnssec.algorithm_to_text(self.template_kwargs['algorithm']) class DNSKEYRevokedRRSIG(RRSIGError): @@ -533,21 +533,21 @@ class DigestAlgorithmNotSupported(DSDigestError): super(DigestAlgorithmNotSupported, self).__init__(**kwargs) self.template_kwargs['algorithm_text'] = fmt.DS_DIGEST_TYPES.get(self.template_kwargs['algorithm'], self.template_kwargs['algorithm']) -class DigestAlgorithmMustNotValidate(DSDigestError): +class DigestAlgorithmValidationProhibited(DSDigestError): ''' - >>> e = DigestAlgorithmMustNotValidate(algorithm=5) + >>> e = DigestAlgorithmValidationProhibited(algorithm=5) >>> e.description 'DNSSEC specification prohibits validation of DS records that use digest algorithm 5 (5).' ''' _abstract = False - code = 'DIGEST_ALGORITHM_MUST_NOT_VALIDATE' + code = 'DIGEST_ALGORITHM_VALIDATION_PROHIBITED' description_template = "DNSSEC specification prohibits validation of DS records that use digest algorithm %(algorithm)d (%(algorithm_text)s)." references = ['RFC 8624, Sec. 3.2'] required_params = ['algorithm'] def __init__(self, **kwargs): - super(DigestAlgorithmMustNotValidate, self).__init__(**kwargs) + super(DigestAlgorithmValidationProhibited, self).__init__(**kwargs) self.template_kwargs['algorithm_text'] = fmt.DS_DIGEST_TYPES.get(self.template_kwargs['algorithm'], self.template_kwargs['algorithm']) class DNSKEYRevokedDS(DSDigestError): diff --git a/dnsviz/analysis/offline.py b/dnsviz/analysis/offline.py index dbea3db..dcfc58a 100644 --- a/dnsviz/analysis/offline.py +++ b/dnsviz/analysis/offline.py @@ -857,8 +857,8 @@ class OfflineDomainNameAnalysis(OnlineDomainNameAnalysis): # unless we are overriding, mark prohibited algorithms as not supported if not validate_prohibited_algs: - supported_algs.difference_update(Status.DNSKEY_ALGS_MUST_NOT_VALIDATE) - supported_digest_algs.difference_update(Status.DS_DIGEST_ALGS_MUST_NOT_VALIDATE) + supported_algs.difference_update(Status.DNSKEY_ALGS_VALIDATION_PROHIBITED) + supported_digest_algs.difference_update(Status.DS_DIGEST_ALGS_VALIDATION_PROHIBITED) self._populate_status(trusted_keys, supported_algs, supported_digest_algs, is_dlv, None, follow_mx) diff --git a/dnsviz/analysis/status.py b/dnsviz/analysis/status.py index 5ba2595..ae6a572 100644 --- a/dnsviz/analysis/status.py +++ b/dnsviz/analysis/status.py @@ -171,13 +171,13 @@ DS_DIGEST_ALGS_IGNORING_SHA1 = (2,) # RFC 8624 Section 3.1 DNSKEY_ALGS_NOT_RECOMMENDED = (5, 7, 10) -DNSKEY_ALGS_MUST_NOT_SIGN = (1, 3, 6, 12) -DNSKEY_ALGS_MUST_NOT_VALIDATE = (1, 3, 6) +DNSKEY_ALGS_PROHIBITED = (1, 3, 6, 12) +DNSKEY_ALGS_VALIDATION_PROHIBITED = (1, 3, 6) # RFC 8624 Section 3.2 DS_DIGEST_ALGS_NOT_RECOMMENDED = () -DS_DIGEST_ALGS_MUST_NOT_SIGN = (0, 1, 3) -DS_DIGEST_ALGS_MUST_NOT_VALIDATE = () +DS_DIGEST_ALGS_PROHIBITED = (0, 1, 3) +DS_DIGEST_ALGS_VALIDATION_PROHIBITED = () class RRSIGStatus(object): def __init__(self, rrset, rrsig, dnskey, zone_name, reference_ts, supported_algs): @@ -207,7 +207,7 @@ class RRSIGStatus(object): else: # If there is a DNSKEY, then we look at *why* we are ignoring # the cryptographic signature. - if self.dnskey.rdata.algorithm in DNSKEY_ALGS_MUST_NOT_VALIDATE: + if self.dnskey.rdata.algorithm in DNSKEY_ALGS_VALIDATION_PROHIBITED: # In this case, specification dictates that the algorithm # MUST NOT be validated, so we mark it as ignored. if self.validation_status == RRSIG_STATUS_VALID: @@ -225,8 +225,8 @@ class RRSIGStatus(object): # Independent of whether or not we considered the cryptographic # validation, issue a warning if we are using an algorithm for which # validation has been prohibited. - if self.dnskey.rdata.algorithm in DNSKEY_ALGS_MUST_NOT_VALIDATE: - self.warnings.append(Errors.AlgorithmMustNotValidate(algorithm=self.rrsig.algorithm)) + if self.dnskey.rdata.algorithm in DNSKEY_ALGS_VALIDATION_PROHIBITED: + self.warnings.append(Errors.AlgorithmValidationProhibited(algorithm=self.rrsig.algorithm)) if self.rrset.ttl_cmp: if self.rrset.rrset.ttl != self.rrset.rrsig_info[self.rrsig].ttl: @@ -395,7 +395,7 @@ class DSStatus(object): else: # If there is a DNSKEY, then we look at *why* we are ignoring # the digest of the DNSKEY. - if self.ds.digest_type in DS_DIGEST_ALGS_MUST_NOT_VALIDATE: + if self.ds.digest_type in DS_DIGEST_ALGS_VALIDATION_PROHIBITED: # In this case, specification dictates that the algorithm # MUST NOT be validated, so we mark it as ignored. if self.validation_status == DS_STATUS_VALID: @@ -413,8 +413,8 @@ class DSStatus(object): # Independent of whether or not we considered the digest for # validation, issue a warning if we are using a digest type for which # validation has been prohibited. - if self.ds.digest_type in DS_DIGEST_ALGS_MUST_NOT_VALIDATE: - self.warnings.append(Errors.DigestAlgorithmMustNotValidate(algorithm=self.ds.digest_type)) + if self.ds.digest_type in DS_DIGEST_ALGS_VALIDATION_PROHIBITED: + self.warnings.append(Errors.DigestAlgorithmValidationProhibited(algorithm=self.ds.digest_type)) if self.dnskey is not None and \ self.dnskey.rdata.flags & fmt.DNSKEY_FLAGS['revoke']: -- cgit v1.2.3 From 2821f62657e9780636e271874c04b5f7298adec0 Mon Sep 17 00:00:00 2001 From: Casey Deccio Date: Thu, 7 Jan 2021 22:08:41 -0700 Subject: Revamp code that ignores SHA1 if something better exists --- dnsviz/analysis/status.py | 76 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 52 insertions(+), 24 deletions(-) diff --git a/dnsviz/analysis/status.py b/dnsviz/analysis/status.py index ae6a572..3d21be2 100644 --- a/dnsviz/analysis/status.py +++ b/dnsviz/analysis/status.py @@ -435,35 +435,63 @@ class DSStatus(object): # RFC 4509 if self.ds.digest_type == 1: - digest_algs = set() - my_digest_algs = {} + stronger_algs_all_ds = set() + stronger_algs_this_dnskey = set() + # Cycle through all other DS records in the DS RRset: + # 1. Create a list of digest types that are stronger than SHA1 + # and are being used by DS records across the *entire* DS + # RRset. Store them in digest_algs_all_ds. + # 2. Create a list of digest types that are stronger than SHA1, + # correspond to DS records go with the same DNSKEY as *this* + # DS record, and have valid or indeterminate status (i.e., not + # invalid). These are DS records with the same DNSSEC + # algorithm and key tag, but different digest types. Store + # them in stronger_algs_this_dnskey. + # + # Note: It is possible that a DS with a different digest + # type matches a different DNSKEY than the present DNSKEY--due + # to key tag collisions. If it does, there will be a warning, + # but it should be both rare and innocuous. for ds_rdata in self.ds_meta.rrset: - digest_algs.add(ds_rdata.digest_type) + + if ds_rdata.digest_type not in DS_DIGEST_ALGS_STRONGER_THAN_SHA1: + continue + + stronger_algs_all_ds.add(ds_rdata.digest_type) if (ds_rdata.algorithm, ds_rdata.key_tag) == (self.ds.algorithm, self.ds.key_tag): - # Here we produce a status of the DS with algorithm 2 with - # respect to the DNSKEY for comparison with the current DS - # with algorithm 1. It is possible that the DS with the - # different digest type matches a different DNSKEY than the - # present DNSKEY. If it does, there will be a warning, but - # it should be both rare and innocuous. if ds_rdata.digest_type == self.ds.digest_type: continue - elif ds_rdata.digest_type not in my_digest_algs or \ - my_digest_algs[ds_rdata.digest_type].validation_status != DS_STATUS_VALID: - my_digest_algs[ds_rdata.digest_type] = \ - DSStatus(ds_rdata, self.ds_meta, self.dnskey, supported_digest_algs) - - for digest_alg in DS_DIGEST_ALGS_STRONGER_THAN_SHA1: - if digest_alg in supported_digest_algs and digest_alg in digest_algs and \ - (digest_alg not in my_digest_algs or my_digest_algs[digest_alg].validation_status not in \ - (DS_STATUS_VALID, DS_STATUS_INDETERMINATE_NO_DNSKEY, DS_STATUS_INDETERMINATE_UNKNOWN_ALGORITHM, DS_STATUS_INDETERMINATE_MATCH_PRE_REVOKE)): - - if digest_alg in DS_DIGEST_ALGS_IGNORING_SHA1: - self.warnings.append(Errors.DSDigestAlgorithmIgnored(algorithm=1, new_algorithm=digest_alg)) - if self.validation_status == DS_STATUS_VALID: - self.validation_status = DS_STATUS_ALGORITHM_IGNORED else: - self.warnings.append(Errors.DSDigestAlgorithmMaybeIgnored(algorithm=1, new_algorithm=digest_alg)) + status = DSStatus(ds_rdata, self.ds_meta, self.dnskey, supported_digest_algs) + if status.validation_status in \ + (DS_STATUS_VALID, DS_STATUS_INDETERMINATE_NO_DNSKEY, DS_STATUS_INDETERMINATE_UNKNOWN_ALGORITHM, DS_STATUS_INDETERMINATE_MATCH_PRE_REVOKE): + stronger_algs_this_dnskey.add(ds_rdata.digest_type) + + # Consider only digest types that we actually support + stronger_algs_all_ds.intersection_update(supported_digest_algs) + stronger_algs_this_dnskey.intersection_update(supported_digest_algs) + + if stronger_algs_all_ds: + # If there are DS records in the DS RRset with digest type + # stronger than SHA1, then this one MUST be ignored by + # validators (RFC 4509). We don't actually issue a warning, + # however, unless a DS with stronger digest type is not being + # used to validate the current DNSKEY; if there is such a DS, + # then there is no reason to complain. + + if not stronger_algs_this_dnskey: + # If there are any DS records in the DS RRset with digest type + # stronger than SHA1, and none of them can properly validate + # the current DNSKEY, then this one stands alone. + for digest_alg in stronger_algs_all_ds: + if digest_alg in DS_DIGEST_ALGS_IGNORING_SHA1: + if self.validation_status == DS_STATUS_VALID: + self.validation_status = DS_STATUS_ALGORITHM_IGNORED + self.warnings.append(Errors.DSDigestAlgorithmIgnored(algorithm=1, new_algorithm=digest_alg)) + else: + self.warnings.append(Errors.DSDigestAlgorithmMaybeIgnored(algorithm=1, new_algorithm=digest_alg)) + + def __str__(self): return '%s record(s) corresponding to DNSKEY for %s (algorithm %d (%s), key tag %d)' % (dns.rdatatype.to_text(self.ds_meta.rrset.rdtype), fmt.humanize_name(self.ds_meta.rrset.name), self.ds.algorithm, fmt.DNSKEY_ALGORITHMS.get(self.ds.algorithm, self.ds.algorithm), self.ds.key_tag) -- cgit v1.2.3 From 22cdb21932a5e9f3aac5ab77dedb1a845a585768 Mon Sep 17 00:00:00 2001 From: Casey Deccio Date: Thu, 7 Jan 2021 22:16:54 -0700 Subject: Warn if zones are signed with prohibited algorithms --- dnsviz/analysis/errors.py | 72 +++++++++++++++++++++++++++++++++++++++++++++++ dnsviz/analysis/status.py | 11 +++++++- 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/dnsviz/analysis/errors.py b/dnsviz/analysis/errors.py index b01b342..54356cc 100644 --- a/dnsviz/analysis/errors.py +++ b/dnsviz/analysis/errors.py @@ -271,6 +271,44 @@ class AlgorithmValidationProhibited(RRSIGError): super(AlgorithmValidationProhibited, self).__init__(**kwargs) self.template_kwargs['algorithm_text'] = dns.dnssec.algorithm_to_text(self.template_kwargs['algorithm']) +class AlgorithmProhibited(RRSIGError): + ''' + >>> e = AlgorithmProhibited(algorithm=5) + >>> e.args + [5] + >>> e.description + 'DNSSEC specification prohibits signing with DNSSEC algorithm 5 (RSASHA1).' + ''' + + _abstract = False + code = 'ALGORITHM_PROHIBITED' + description_template = "DNSSEC specification prohibits signing with DNSSEC algorithm %(algorithm)d (%(algorithm_text)s)." + references = ['RFC 8624, Sec. 3.1'] + required_params = ['algorithm'] + + def __init__(self, **kwargs): + super(AlgorithmProhibited, self).__init__(**kwargs) + self.template_kwargs['algorithm_text'] = dns.dnssec.algorithm_to_text(self.template_kwargs['algorithm']) + +class AlgorithmNotRecommended(RRSIGError): + ''' + >>> e = AlgorithmNotRecommended(algorithm=5) + >>> e.args + [5] + >>> e.description + 'DNSSEC specification recommends not signing with DNSSEC algorithm 5 (RSASHA1).' + ''' + + _abstract = False + code = 'ALGORITHM_NOT_RECOMMENDED' + description_template = "DNSSEC specification recommends not signing with DNSSEC algorithm %(algorithm)d (%(algorithm_text)s)." + references = ['RFC 8624, Sec. 3.1'] + required_params = ['algorithm'] + + def __init__(self, **kwargs): + super(AlgorithmNotRecommended, self).__init__(**kwargs) + self.template_kwargs['algorithm_text'] = dns.dnssec.algorithm_to_text(self.template_kwargs['algorithm']) + class DNSKEYRevokedRRSIG(RRSIGError): ''' >>> e = DNSKEYRevokedRRSIG() @@ -550,6 +588,40 @@ class DigestAlgorithmValidationProhibited(DSDigestError): super(DigestAlgorithmValidationProhibited, self).__init__(**kwargs) self.template_kwargs['algorithm_text'] = fmt.DS_DIGEST_TYPES.get(self.template_kwargs['algorithm'], self.template_kwargs['algorithm']) +class DigestAlgorithmProhibited(DSDigestError): + ''' + >>> e = DigestAlgorithmProhibited(algorithm=5) + >>> e.description + 'DNSSEC specification prohibits publishing DS records that use digest algorithm 5 (5).' + ''' + + _abstract = False + code = 'DIGEST_ALGORITHM_PROHIBITED' + description_template = "DNSSEC specification prohibits publishing DS records that use digest algorithm %(algorithm)d (%(algorithm_text)s)." + references = ['RFC 8624, Sec. 3.2'] + required_params = ['algorithm'] + + def __init__(self, **kwargs): + super(DigestAlgorithmProhibited, self).__init__(**kwargs) + self.template_kwargs['algorithm_text'] = fmt.DS_DIGEST_TYPES.get(self.template_kwargs['algorithm'], self.template_kwargs['algorithm']) + +class DigestAlgorithmNotRecommended(DSDigestError): + ''' + >>> e = DigestAlgorithmNotRecommended(algorithm=5) + >>> e.description + 'DNSSEC specification recommends not publishing DS records that use digest algorithm 5 (5).' + ''' + + _abstract = False + code = 'DIGEST_ALGORITHM_NOT_RECOMMENDED' + description_template = "DNSSEC specification recommends not publishing DS records that use digest algorithm %(algorithm)d (%(algorithm_text)s)." + references = ['RFC 8624, Sec. 3.2'] + required_params = ['algorithm'] + + def __init__(self, **kwargs): + super(DigestAlgorithmNotRecommended, self).__init__(**kwargs) + self.template_kwargs['algorithm_text'] = fmt.DS_DIGEST_TYPES.get(self.template_kwargs['algorithm'], self.template_kwargs['algorithm']) + class DNSKEYRevokedDS(DSDigestError): ''' >>> e = DNSKEYRevokedDS() diff --git a/dnsviz/analysis/status.py b/dnsviz/analysis/status.py index 3d21be2..ea81ebf 100644 --- a/dnsviz/analysis/status.py +++ b/dnsviz/analysis/status.py @@ -224,9 +224,13 @@ class RRSIGStatus(object): # Independent of whether or not we considered the cryptographic # validation, issue a warning if we are using an algorithm for which - # validation has been prohibited. + # validation or signing has been prohibited. if self.dnskey.rdata.algorithm in DNSKEY_ALGS_VALIDATION_PROHIBITED: self.warnings.append(Errors.AlgorithmValidationProhibited(algorithm=self.rrsig.algorithm)) + if self.dnskey.rdata.algorithm in DNSKEY_ALGS_PROHIBITED: + self.warnings.append(Errors.AlgorithmProhibited(algorithm=self.rrsig.algorithm)) + if self.dnskey.rdata.algorithm in DNSKEY_ALGS_NOT_RECOMMENDED: + self.warnings.append(Errors.AlgorithmNotRecommended(algorithm=self.rrsig.algorithm)) if self.rrset.ttl_cmp: if self.rrset.rrset.ttl != self.rrset.rrsig_info[self.rrsig].ttl: @@ -491,6 +495,11 @@ class DSStatus(object): else: self.warnings.append(Errors.DSDigestAlgorithmMaybeIgnored(algorithm=1, new_algorithm=digest_alg)) + # For all other digest types, just add a warning here + elif self.ds.digest_type in DS_DIGEST_ALGS_PROHIBITED: + self.warnings.append(Errors.DigestAlgorithmProhibited(algorithm=self.ds.digest_type)) + elif self.ds.digest_type in DS_DIGEST_ALGS_NOT_RECOMMENDED: + self.warnings.append(Errors.DigestAlgorithmNotRecommended(algorithm=self.ds.digest_type)) def __str__(self): -- cgit v1.2.3 From 7a4b641806356df72095d794773790a97b4ced94 Mon Sep 17 00:00:00 2001 From: Casey Deccio Date: Thu, 7 Jan 2021 22:26:05 -0700 Subject: Warn if using prohibited or not recommended algorithm --- dnsviz/analysis/status.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/dnsviz/analysis/status.py b/dnsviz/analysis/status.py index ea81ebf..914d53a 100644 --- a/dnsviz/analysis/status.py +++ b/dnsviz/analysis/status.py @@ -416,9 +416,13 @@ class DSStatus(object): # Independent of whether or not we considered the digest for # validation, issue a warning if we are using a digest type for which - # validation has been prohibited. + # validation or publishing has been prohibited. if self.ds.digest_type in DS_DIGEST_ALGS_VALIDATION_PROHIBITED: self.warnings.append(Errors.DigestAlgorithmValidationProhibited(algorithm=self.ds.digest_type)) + elif self.ds.digest_type in DS_DIGEST_ALGS_PROHIBITED: + self.warnings.append(Errors.DigestAlgorithmProhibited(algorithm=self.ds.digest_type)) + elif self.ds.digest_type in DS_DIGEST_ALGS_NOT_RECOMMENDED: + self.warnings.append(Errors.DigestAlgorithmNotRecommended(algorithm=self.ds.digest_type)) if self.dnskey is not None and \ self.dnskey.rdata.flags & fmt.DNSKEY_FLAGS['revoke']: @@ -495,12 +499,6 @@ class DSStatus(object): else: self.warnings.append(Errors.DSDigestAlgorithmMaybeIgnored(algorithm=1, new_algorithm=digest_alg)) - # For all other digest types, just add a warning here - elif self.ds.digest_type in DS_DIGEST_ALGS_PROHIBITED: - self.warnings.append(Errors.DigestAlgorithmProhibited(algorithm=self.ds.digest_type)) - elif self.ds.digest_type in DS_DIGEST_ALGS_NOT_RECOMMENDED: - self.warnings.append(Errors.DigestAlgorithmNotRecommended(algorithm=self.ds.digest_type)) - def __str__(self): return '%s record(s) corresponding to DNSKEY for %s (algorithm %d (%s), key tag %d)' % (dns.rdatatype.to_text(self.ds_meta.rrset.rdtype), fmt.humanize_name(self.ds_meta.rrset.name), self.ds.algorithm, fmt.DNSKEY_ALGORITHMS.get(self.ds.algorithm, self.ds.algorithm), self.ds.key_tag) -- cgit v1.2.3 From 4819ed1fbac1848acf800e220dce186de9df7168 Mon Sep 17 00:00:00 2001 From: Casey Deccio Date: Fri, 8 Jan 2021 16:57:26 -0700 Subject: Update wording of error --- dnsviz/analysis/errors.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dnsviz/analysis/errors.py b/dnsviz/analysis/errors.py index 54356cc..4ad5769 100644 --- a/dnsviz/analysis/errors.py +++ b/dnsviz/analysis/errors.py @@ -592,12 +592,12 @@ class DigestAlgorithmProhibited(DSDigestError): ''' >>> e = DigestAlgorithmProhibited(algorithm=5) >>> e.description - 'DNSSEC specification prohibits publishing DS records that use digest algorithm 5 (5).' + 'DNSSEC specification prohibits signing with DS records that use digest algorithm 5 (5).' ''' _abstract = False code = 'DIGEST_ALGORITHM_PROHIBITED' - description_template = "DNSSEC specification prohibits publishing DS records that use digest algorithm %(algorithm)d (%(algorithm_text)s)." + description_template = "DNSSEC specification prohibits signing with DS records that use digest algorithm %(algorithm)d (%(algorithm_text)s)." references = ['RFC 8624, Sec. 3.2'] required_params = ['algorithm'] @@ -609,12 +609,12 @@ class DigestAlgorithmNotRecommended(DSDigestError): ''' >>> e = DigestAlgorithmNotRecommended(algorithm=5) >>> e.description - 'DNSSEC specification recommends not publishing DS records that use digest algorithm 5 (5).' + 'DNSSEC specification recommends not signing with DS records that use digest algorithm 5 (5).' ''' _abstract = False code = 'DIGEST_ALGORITHM_NOT_RECOMMENDED' - description_template = "DNSSEC specification recommends not publishing DS records that use digest algorithm %(algorithm)d (%(algorithm_text)s)." + description_template = "DNSSEC specification recommends not signing with DS records that use digest algorithm %(algorithm)d (%(algorithm_text)s)." references = ['RFC 8624, Sec. 3.2'] required_params = ['algorithm'] -- cgit v1.2.3 From 0871d6776ba584dc76b2fbe07f61d082a53b3ba7 Mon Sep 17 00:00:00 2001 From: Casey Deccio Date: Fri, 8 Jan 2021 17:09:32 -0700 Subject: Always warn if digest type 1 exists with stronger digest type --- dnsviz/analysis/status.py | 60 ++++++++++------------------------------------- 1 file changed, 13 insertions(+), 47 deletions(-) diff --git a/dnsviz/analysis/status.py b/dnsviz/analysis/status.py index 914d53a..879b2a6 100644 --- a/dnsviz/analysis/status.py +++ b/dnsviz/analysis/status.py @@ -444,61 +444,27 @@ class DSStatus(object): # RFC 4509 if self.ds.digest_type == 1: stronger_algs_all_ds = set() - stronger_algs_this_dnskey = set() - # Cycle through all other DS records in the DS RRset: - # 1. Create a list of digest types that are stronger than SHA1 - # and are being used by DS records across the *entire* DS - # RRset. Store them in digest_algs_all_ds. - # 2. Create a list of digest types that are stronger than SHA1, - # correspond to DS records go with the same DNSKEY as *this* - # DS record, and have valid or indeterminate status (i.e., not - # invalid). These are DS records with the same DNSSEC - # algorithm and key tag, but different digest types. Store - # them in stronger_algs_this_dnskey. - # - # Note: It is possible that a DS with a different digest - # type matches a different DNSKEY than the present DNSKEY--due - # to key tag collisions. If it does, there will be a warning, - # but it should be both rare and innocuous. + # Cycle through all other DS records in the DS RRset, and + # create a list of digest types that are stronger than SHA1 + # and are being used by DS records across the *entire* DS. for ds_rdata in self.ds_meta.rrset: - - if ds_rdata.digest_type not in DS_DIGEST_ALGS_STRONGER_THAN_SHA1: - continue - - stronger_algs_all_ds.add(ds_rdata.digest_type) - if (ds_rdata.algorithm, ds_rdata.key_tag) == (self.ds.algorithm, self.ds.key_tag): - if ds_rdata.digest_type == self.ds.digest_type: - continue - else: - status = DSStatus(ds_rdata, self.ds_meta, self.dnskey, supported_digest_algs) - if status.validation_status in \ - (DS_STATUS_VALID, DS_STATUS_INDETERMINATE_NO_DNSKEY, DS_STATUS_INDETERMINATE_UNKNOWN_ALGORITHM, DS_STATUS_INDETERMINATE_MATCH_PRE_REVOKE): - stronger_algs_this_dnskey.add(ds_rdata.digest_type) + if ds_rdata.digest_type in DS_DIGEST_ALGS_STRONGER_THAN_SHA1: + stronger_algs_all_ds.add(ds_rdata.digest_type) # Consider only digest types that we actually support stronger_algs_all_ds.intersection_update(supported_digest_algs) - stronger_algs_this_dnskey.intersection_update(supported_digest_algs) if stronger_algs_all_ds: # If there are DS records in the DS RRset with digest type # stronger than SHA1, then this one MUST be ignored by - # validators (RFC 4509). We don't actually issue a warning, - # however, unless a DS with stronger digest type is not being - # used to validate the current DNSKEY; if there is such a DS, - # then there is no reason to complain. - - if not stronger_algs_this_dnskey: - # If there are any DS records in the DS RRset with digest type - # stronger than SHA1, and none of them can properly validate - # the current DNSKEY, then this one stands alone. - for digest_alg in stronger_algs_all_ds: - if digest_alg in DS_DIGEST_ALGS_IGNORING_SHA1: - if self.validation_status == DS_STATUS_VALID: - self.validation_status = DS_STATUS_ALGORITHM_IGNORED - self.warnings.append(Errors.DSDigestAlgorithmIgnored(algorithm=1, new_algorithm=digest_alg)) - else: - self.warnings.append(Errors.DSDigestAlgorithmMaybeIgnored(algorithm=1, new_algorithm=digest_alg)) - + # validators (RFC 4509). + for digest_alg in stronger_algs_all_ds: + if digest_alg in DS_DIGEST_ALGS_IGNORING_SHA1: + if self.validation_status == DS_STATUS_VALID: + self.validation_status = DS_STATUS_ALGORITHM_IGNORED + self.warnings.append(Errors.DSDigestAlgorithmIgnored(algorithm=1, new_algorithm=digest_alg)) + else: + self.warnings.append(Errors.DSDigestAlgorithmMaybeIgnored(algorithm=1, new_algorithm=digest_alg)) def __str__(self): return '%s record(s) corresponding to DNSKEY for %s (algorithm %d (%s), key tag %d)' % (dns.rdatatype.to_text(self.ds_meta.rrset.rdtype), fmt.humanize_name(self.ds_meta.rrset.name), self.ds.algorithm, fmt.DNSKEY_ALGORITHMS.get(self.ds.algorithm, self.ds.algorithm), self.ds.key_tag) -- cgit v1.2.3 From f0e697b1d837f63299aa79d6152742bfd2dc9926 Mon Sep 17 00:00:00 2001 From: Casey Deccio Date: Fri, 8 Jan 2021 17:26:03 -0700 Subject: Fix warning logic --- dnsviz/analysis/status.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/dnsviz/analysis/status.py b/dnsviz/analysis/status.py index 879b2a6..6a68af4 100644 --- a/dnsviz/analysis/status.py +++ b/dnsviz/analysis/status.py @@ -225,11 +225,14 @@ class RRSIGStatus(object): # Independent of whether or not we considered the cryptographic # validation, issue a warning if we are using an algorithm for which # validation or signing has been prohibited. + # + # Signing is prohibited if self.dnskey.rdata.algorithm in DNSKEY_ALGS_VALIDATION_PROHIBITED: self.warnings.append(Errors.AlgorithmValidationProhibited(algorithm=self.rrsig.algorithm)) + # Validation is prohibited or, at least, not recommended if self.dnskey.rdata.algorithm in DNSKEY_ALGS_PROHIBITED: self.warnings.append(Errors.AlgorithmProhibited(algorithm=self.rrsig.algorithm)) - if self.dnskey.rdata.algorithm in DNSKEY_ALGS_NOT_RECOMMENDED: + elif self.dnskey.rdata.algorithm in DNSKEY_ALGS_NOT_RECOMMENDED: self.warnings.append(Errors.AlgorithmNotRecommended(algorithm=self.rrsig.algorithm)) if self.rrset.ttl_cmp: @@ -416,10 +419,13 @@ class DSStatus(object): # Independent of whether or not we considered the digest for # validation, issue a warning if we are using a digest type for which - # validation or publishing has been prohibited. + # validation or signing has been prohibited. + # + # Signing is prohibited if self.ds.digest_type in DS_DIGEST_ALGS_VALIDATION_PROHIBITED: self.warnings.append(Errors.DigestAlgorithmValidationProhibited(algorithm=self.ds.digest_type)) - elif self.ds.digest_type in DS_DIGEST_ALGS_PROHIBITED: + # Validation is prohibited or, at least, not recommended + if self.ds.digest_type in DS_DIGEST_ALGS_PROHIBITED: self.warnings.append(Errors.DigestAlgorithmProhibited(algorithm=self.ds.digest_type)) elif self.ds.digest_type in DS_DIGEST_ALGS_NOT_RECOMMENDED: self.warnings.append(Errors.DigestAlgorithmNotRecommended(algorithm=self.ds.digest_type)) -- cgit v1.2.3