From 1c54b49ea67233893b748885c2c6abae914b078c Mon Sep 17 00:00:00 2001 From: davidb Date: Tue, 8 Jun 2010 11:20:50 -0400 Subject: [PATCH] sync with external work, part 2: this adds an error list, and fixes a major NSEC3 validation bug --- .../versign/tat/dnssec/CaptiveValidator.java | 78 +++++++++++-------- src/com/versign/tat/dnssec/NSEC3ValUtils.java | 39 +++++++--- 2 files changed, 73 insertions(+), 44 deletions(-) diff --git a/src/com/versign/tat/dnssec/CaptiveValidator.java b/src/com/versign/tat/dnssec/CaptiveValidator.java index 9bfba09..be3c15b 100644 --- a/src/com/versign/tat/dnssec/CaptiveValidator.java +++ b/src/com/versign/tat/dnssec/CaptiveValidator.java @@ -50,10 +50,13 @@ public class CaptiveValidator { private DnsSecVerifier mVerifier; private Logger log = Logger.getLogger(this.getClass()); + private List mErrorList; + public CaptiveValidator() { mVerifier = new DnsSecVerifier(); mValUtils = new ValUtils(mVerifier); mTrustedKeys = new TrustAnchorStore(); + mErrorList = new ArrayList(); } // ---------------- Module Initialization ------------------- @@ -208,7 +211,7 @@ public class CaptiveValidator { // Follow the CNAME chain. if (type == Type.CNAME) { if (rrset.size() > 1) { - log.debug("Found CNAME rrset with size > 1: " + rrset); + mErrorList.add("Found CNAME rrset with size > 1: " + rrset); m.setStatus(SecurityStatus.INVALID); return m; @@ -366,8 +369,8 @@ public class CaptiveValidator { // If the (answer) rrset failed to validate, then this message is // BAD. if (status != SecurityStatus.SECURE) { - log.debug("Positive response has failed ANSWER rrset: " + - rrsets[i]); + mErrorList.add("Positive response has failed ANSWER rrset: " + + rrsets[i]); m.setStatus(SecurityStatus.BOGUS); return; @@ -392,11 +395,10 @@ public class CaptiveValidator { int status = mValUtils.verifySRRset(rrsets[i], key_rrset); // If anything in the authority section fails to be secure, we have - // a - // bad message. + // a bad message. if (status != SecurityStatus.SECURE) { - log.debug("Positive response has failed AUTHORITY rrset: " + - rrsets[i]); + mErrorList.add("Positive response has failed AUTHORITY rrset: " + + rrsets[i]); m.setStatus(SecurityStatus.BOGUS); return; @@ -413,8 +415,7 @@ public class CaptiveValidator { Name nsec_wc = ValUtils.nsecWildcard(qname, nsec); if (!wc.equals(nsec_wc)) { - // log.debug("Positive wildcard response wasn't generated " - // + "by the correct wildcard"); + mErrorList.add("Positive wildcard response wasn't generated by the correct wildcard"); m.setStatus(SecurityStatus.BOGUS); return; @@ -440,7 +441,7 @@ public class CaptiveValidator { // records. if ((wc != null) && !wcNSEC_ok && (nsec3s != null)) { if (NSEC3ValUtils.proveWildcard(nsec3s, qname, key_rrset.getName(), - wc)) { + wc, mErrorList)) { wcNSEC_ok = true; } } @@ -492,8 +493,8 @@ public class CaptiveValidator { // have // a bad message. if (status != SecurityStatus.SECURE) { - log.debug("Positive response has failed AUTHORITY rrset: " + - rrsets[i]); + mErrorList.add("Positive response has failed AUTHORITY rrset: " + + rrsets[i]); m.setStatus(SecurityStatus.BOGUS); return; @@ -542,6 +543,7 @@ public class CaptiveValidator { // Now to check to see if we have a valid combination of things. if (delegation == null) { // somehow we have a referral without an NS rrset. + mErrorList.add("Apparent referral does not contain NS RRset"); m.setStatus(SecurityStatus.BOGUS); return; @@ -550,6 +552,7 @@ public class CaptiveValidator { if (secure_delegation) { if ((nsec != null) || ((nsec3s != null) && (nsec3s.size() > 0))) { // we found both a DS rrset *and* NSEC/NSEC3 rrsets! + mErrorList.add("Referral contains both DS and NSEC/NSEC3 RRsets"); m.setStatus(SecurityStatus.BOGUS); return; @@ -568,6 +571,7 @@ public class CaptiveValidator { if (status != SecurityStatus.SECURE) { // The NSEC *must* prove that there was no DS record. The // INSECURE state here is still bogus. + mErrorList.add("Referral does not contain a NSEC record proving no DS"); m.setStatus(SecurityStatus.BOGUS); return; @@ -579,11 +583,12 @@ public class CaptiveValidator { } if (nsec3s.size() > 0) { - byte status = NSEC3ValUtils.proveNoDS(nsec3s, delegation, nsec3zone); + byte status = NSEC3ValUtils.proveNoDS(nsec3s, delegation, nsec3zone, mErrorList); if (status != SecurityStatus.SECURE) { // the NSEC3 RRs MUST prove no DS, so the INDETERMINATE state is // actually bogus + mErrorList.add("Referral does not contain NSEC3 record(s) proving no DS"); m.setStatus(SecurityStatus.BOGUS); return; @@ -595,9 +600,11 @@ public class CaptiveValidator { } // failed to find proof either way. + mErrorList.add("Referral does not contain proof of no DS"); m.setStatus(SecurityStatus.BOGUS); } + // FIXME: write CNAME validation code. private void validateCNAMEResponse(SMessage message, SRRset key_rrset) {} /** @@ -642,8 +649,8 @@ public class CaptiveValidator { // If the (answer) rrset failed to validate, then this message is // BAD. if (status != SecurityStatus.SECURE) { - log.debug("Positive response has failed ANSWER rrset: " + - rrsets[i]); + mErrorList.add("Positive response has failed ANSWER rrset: " + + rrsets[i]); m.setStatus(SecurityStatus.BOGUS); return; @@ -658,11 +665,10 @@ public class CaptiveValidator { int status = mValUtils.verifySRRset(rrsets[i], key_rrset); // If anything in the authority section fails to be secure, we have - // a - // bad message. + // a bad message. if (status != SecurityStatus.SECURE) { - log.debug("Positive response has failed AUTHORITY rrset: " + - rrsets[i]); + mErrorList.add("Positive response has failed AUTHORITY rrset: " + + rrsets[i]); m.setStatus(SecurityStatus.BOGUS); return; @@ -691,7 +697,7 @@ public class CaptiveValidator { * @param key_rrset * The trusted DNSKEY rrset that signs this response. */ - private void validateNodataResponse(SMessage message, SRRset key_rrset) { + private void validateNodataResponse(SMessage message, SRRset key_rrset, List errorList) { Name qname = message.getQName(); int qtype = message.getQType(); @@ -724,8 +730,8 @@ public class CaptiveValidator { int status = mValUtils.verifySRRset(rrsets[i], key_rrset); if (status != SecurityStatus.SECURE) { - log.debug("NODATA response has failed AUTHORITY rrset: " + - rrsets[i]); + mErrorList.add("NODATA response has failed AUTHORITY rrset: " + + rrsets[i]); m.setStatus(SecurityStatus.BOGUS); return; @@ -781,13 +787,14 @@ public class CaptiveValidator { if (!hasValidNSEC && (nsec3s != null) && (nsec3s.size() > 0)) { // try to prove NODATA with our NSEC3 record(s) hasValidNSEC = NSEC3ValUtils.proveNodata(nsec3s, qname, qtype, - nsec3Signer); + nsec3Signer, errorList); } if (!hasValidNSEC) { log.debug("NODATA response failed to prove NODATA " + "status with NSEC/NSEC3"); log.trace("Failed NODATA:\n" + m); + mErrorList.add("NODATA response failed to prove NODATA status with NSEC/NSEC3"); m.setStatus(SecurityStatus.BOGUS); return; @@ -821,7 +828,8 @@ public class CaptiveValidator { if (message.getCount(Section.ANSWER) > 0) { log.warn( - "NAME ERROR response contained records in the ANSWER SECTION"); + "NameError response contained records in the ANSWER SECTION"); + mErrorList.add("NameError response contained records in the ANSWER SECTION"); message.setStatus(SecurityStatus.INVALID); return; @@ -840,8 +848,8 @@ public class CaptiveValidator { int status = mValUtils.verifySRRset(rrsets[i], key_rrset); if (status != SecurityStatus.SECURE) { - log.debug("NameError response has failed AUTHORITY rrset: " + - rrsets[i]); + mErrorList.add("NameError response has failed AUTHORITY rrset: " + + rrsets[i]); m.setStatus(SecurityStatus.BOGUS); return; @@ -885,7 +893,7 @@ public class CaptiveValidator { } hasValidNSEC = NSEC3ValUtils.proveNameError(nsec3s, qname, - nsec3Signer); + nsec3Signer, mErrorList); // Note that we assume that the NSEC3ValUtils proofs encompass the // wildcard part of the proof. @@ -894,16 +902,14 @@ public class CaptiveValidator { // If the message fails to prove either condition, it is bogus. if (!hasValidNSEC) { - log.debug("NameError response has failed to prove: " + - "qname does not exist"); + mErrorList.add("NameError response has failed to prove qname does not exist"); m.setStatus(SecurityStatus.BOGUS); return; } if (!hasValidWCNSEC) { - log.debug("NameError response has failed to prove: " + - "covering wildcard does not exist"); + mErrorList.add("NameError response has failed to prove covering wildcard does not exist"); m.setStatus(SecurityStatus.BOGUS); return; @@ -915,6 +921,7 @@ public class CaptiveValidator { } public byte validateMessage(SMessage message, Name zone) { + mErrorList.clear(); if (!zone.isAbsolute()) { try { zone = Name.concatenate(zone, Name.root); @@ -936,6 +943,7 @@ public class CaptiveValidator { SRRset key_rrset = findKeys(message); if (key_rrset == null) { + mErrorList.add("Failed to find matching DNSKEYs for the response"); return SecurityStatus.BOGUS; } @@ -955,7 +963,7 @@ public class CaptiveValidator { case NODATA: log.trace("Validating a NODATA response"); - validateNodataResponse(message, key_rrset); + validateNodataResponse(message, key_rrset, mErrorList); break; @@ -995,4 +1003,8 @@ public class CaptiveValidator { public List listTrustedKeys() { return mTrustedKeys.listTrustAnchors(); } -} + + public List getErrorList() { + return mErrorList; + } + } diff --git a/src/com/versign/tat/dnssec/NSEC3ValUtils.java b/src/com/versign/tat/dnssec/NSEC3ValUtils.java index 9eb6698..5d25c0a 100644 --- a/src/com/versign/tat/dnssec/NSEC3ValUtils.java +++ b/src/com/versign/tat/dnssec/NSEC3ValUtils.java @@ -233,14 +233,14 @@ public class NSEC3ValUtils { */ private static boolean nsec3Covers(NSEC3Record nsec3, byte [] hash, ByteArrayComparator bac) { - byte [] owner = hash(nsec3.getName(), nsec3); + Name ownerName = nsec3.getName(); + byte [] owner = b32.fromString(ownerName.getLabelString(0)); byte [] next = nsec3.getNext(); // This is the "normal case: owner < next and owner < hash < next if ((bac.compare(owner, hash) < 0) && (bac.compare(hash, next) < 0)) { return true; } - // this is the end of zone case: next < owner && hash > owner || hash < // next if ((bac.compare(next, owner) <= 0) && @@ -318,7 +318,7 @@ public class NSEC3ValUtils { // FIXME: modify so that the NSEC3 matching the zone apex need not be // present. while (n.labels() >= zonename.labels()) { - nsec3 = findMatchingNSEC3(hash(n, params), zonename, + nsec3 = findMatchingNSEC3(hash(n, params), zonename, nsec3s, params, bac); if (nsec3 != null) { @@ -353,11 +353,12 @@ public class NSEC3ValUtils { */ private static CEResponse proveClosestEncloser(Name qname, Name zonename, List nsec3s, NSEC3Parameters params, - ByteArrayComparator bac, boolean proveDoesNotExist) { + ByteArrayComparator bac, boolean proveDoesNotExist, List errorList) { CEResponse candidate = findClosestEncloser(qname, zonename, nsec3s, params, bac); if (candidate == null) { + errorList.add("Could not find a candidate for the closest encloser"); st_log.debug("proveClosestEncloser: could not find a " + "candidate for the closest encloser."); @@ -366,6 +367,7 @@ public class NSEC3ValUtils { if (candidate.closestEncloser.equals(qname)) { if (proveDoesNotExist) { + errorList.add("Proven closest encloser proved that the qname existed and should not have"); st_log.debug("proveClosestEncloser: proved that qname existed!"); return null; @@ -382,6 +384,7 @@ public class NSEC3ValUtils { // a DNAME response. if (candidate.ce_nsec3.hasType(Type.NS) && !candidate.ce_nsec3.hasType(Type.SOA)) { + errorList.add("Proven closest encloser was a delegation"); st_log.debug("proveClosestEncloser: closest encloser " + "was a delegation!"); @@ -389,6 +392,7 @@ public class NSEC3ValUtils { } if (candidate.ce_nsec3.hasType(Type.DNAME)) { + errorList.add("Proven closest encloser was a DNAME"); st_log.debug("proveClosestEncloser: closest encloser was a DNAME!"); return null; @@ -402,6 +406,8 @@ public class NSEC3ValUtils { params, bac); if (candidate.nc_nsec3 == null) { + errorList.add("Could not find proof that the closest encloser was the closest encloser"); + errorList.add("hash " + hashName(nc_hash, zonename) + " is not covered by any NSEC3 RRs"); st_log.debug("Could not find proof that the " + "closest encloser was the closest encloser"); @@ -520,7 +526,7 @@ public class NSEC3ValUtils { * ignored. */ public static boolean proveNameError(List nsec3s, Name qname, - Name zonename) { + Name zonename, List errorList) { if ((nsec3s == null) || (nsec3s.size() == 0)) { return false; } @@ -528,6 +534,7 @@ public class NSEC3ValUtils { NSEC3Parameters nsec3params = nsec3Parameters(nsec3s); if (nsec3params == null) { + errorList.add("Could not find a single set of NSEC3 parameters (multiple parameters present"); st_log.debug("Could not find a single set of " + "NSEC3 parameters (multiple parameters present)."); @@ -539,9 +546,10 @@ public class NSEC3ValUtils { // First locate and prove the closest encloser to qname. We will use the // variant that fails if the closest encloser turns out to be qname. CEResponse ce = proveClosestEncloser(qname, zonename, nsec3s, - nsec3params, bac, true); + nsec3params, bac, true, errorList); if (ce == null) { + errorList.add("Failed to find the closest encloser as part of the NSEC3 proof"); st_log.debug("proveNameError: failed to prove a closest encloser."); return false; @@ -556,6 +564,7 @@ public class NSEC3ValUtils { nsec3params, bac); if (nsec3 == null) { + errorList.add("Failed to prove that the applicable wildcard did not exist"); st_log.debug("proveNameError: could not prove that the " + "applicable wildcard did not exist."); @@ -596,7 +605,7 @@ public class NSEC3ValUtils { * @return true if the NSEC3s prove the proposition. */ public static boolean proveNodata(List nsec3s, Name qname, - int qtype, Name zonename) { + int qtype, Name zonename, List errorList) { if ((nsec3s == null) || (nsec3s.size() == 0)) { return false; } @@ -638,7 +647,7 @@ public class NSEC3ValUtils { // match qname. Although, at this point, we know that it won't since we // just checked that. CEResponse ce = proveClosestEncloser(qname, zonename, nsec3s, - nsec3params, bac, true); + nsec3params, bac, true, errorList); // At this point, not finding a match or a proven closest encloser is a // problem. @@ -700,7 +709,7 @@ public class NSEC3ValUtils { * @return true if the NSEC3 records prove this case. */ public static boolean proveWildcard(List nsec3s, Name qname, - Name zonename, Name wildcard) { + Name zonename, Name wildcard, List errorList) { if ((nsec3s == null) || (nsec3s.size() == 0)) { return false; } @@ -712,6 +721,7 @@ public class NSEC3ValUtils { NSEC3Parameters nsec3params = nsec3Parameters(nsec3s); if (nsec3params == null) { + errorList.add("Could not find a single set of NSEC3 parameters (multiple parameters present)"); st_log.debug( "couldn't find a single set of NSEC3 parameters (multiple parameters present)."); @@ -732,6 +742,9 @@ public class NSEC3ValUtils { zonename, nsec3s, nsec3params, bac); if (candidate.nc_nsec3 == null) { + errorList.add("Did not find a NSEC3 that covered the next closer name to '" + + qname + "' from '" + candidate.closestEncloser + "' (derived from the wildcard: " + + wildcard + ")"); st_log.debug("proveWildcard: did not find a covering NSEC3 " + "that covered the next closer name to " + qname + " from " + candidate.closestEncloser + " (derived from wildcard " + @@ -763,7 +776,7 @@ public class NSEC3ValUtils { * work out. */ public static byte proveNoDS(List nsec3s, Name qname, - Name zonename) { + Name zonename, List errorList) { if ((nsec3s == null) || (nsec3s.size() == 0)) { return SecurityStatus.BOGUS; } @@ -771,6 +784,7 @@ public class NSEC3ValUtils { NSEC3Parameters nsec3params = nsec3Parameters(nsec3s); if (nsec3params == null) { + errorList.add("Could not find a single set of NSEC3 parameters (multiple parameters present)"); st_log.debug("couldn't find a single set of " + "NSEC3 parameters (multiple parameters present)."); @@ -788,6 +802,7 @@ public class NSEC3ValUtils { // zone (the child instead of the parent). If it has the DS bit set, // then we were lied to. if (nsec3.hasType(Type.SOA) || nsec3.hasType(Type.DS)) { + errorList.add("Matching NSEC3 is incorrectly from the child instead of the parent (SOA or DS bit set)"); return SecurityStatus.BOGUS; } @@ -803,9 +818,10 @@ public class NSEC3ValUtils { // Otherwise, we are probably in the opt-in case. CEResponse ce = proveClosestEncloser(qname, zonename, nsec3s, - nsec3params, bac, true); + nsec3params, bac, true, errorList); if (ce == null) { + errorList.add("Failed to prove the closest encloser as part of a 'No DS' proof"); return SecurityStatus.BOGUS; } @@ -816,6 +832,7 @@ public class NSEC3ValUtils { return SecurityStatus.SECURE; } + errorList.add("Failed to find a covering NSEC3 for 'No DS' proof"); return SecurityStatus.BOGUS; }