sync with external work, part 2: this adds an error list, and fixes a major NSEC3 validation bug

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<String>   mErrorList;
+
     public CaptiveValidator() {
         mVerifier        = new DnsSecVerifier();
         mValUtils        = new ValUtils(mVerifier);
         mTrustedKeys     = new TrustAnchorStore();
+        mErrorList       = new ArrayList<String>();
     }
 
     // ---------------- 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: " +
+                mErrorList.add("Positive response has failed ANSWER rrset: " +
-                    rrsets[i]);
+                         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
+            // a bad message.
-            // bad message.
             if (status != SecurityStatus.SECURE) {
-                log.debug("Positive response has failed AUTHORITY rrset: " +
+                mErrorList.add("Positive response has failed AUTHORITY rrset: " +
-                    rrsets[i]);
+                         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 "
+                        mErrorList.add("Positive wildcard response wasn't generated by the correct wildcard");
-                        // + "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: " +
+                    mErrorList.add("Positive response has failed AUTHORITY rrset: " +
-                        rrsets[i]);
+                             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: " +
+                mErrorList.add("Positive response has failed ANSWER rrset: " +
-                    rrsets[i]);
+                         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
+            // a bad message.
-            // bad message.
             if (status != SecurityStatus.SECURE) {
-                log.debug("Positive response has failed AUTHORITY rrset: " +
+                mErrorList.add("Positive response has failed AUTHORITY rrset: " +
-                    rrsets[i]);
+                         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<String> 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: " +
+                mErrorList.add("NODATA response has failed AUTHORITY rrset: " +
-                    rrsets[i]);
+                         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: " +
+                mErrorList.add("NameError response has failed AUTHORITY rrset: " +
-                    rrsets[i]);
+                         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: " +
+            mErrorList.add("NameError response has failed to prove qname does not exist");
-                "qname does not exist");
             m.setStatus(SecurityStatus.BOGUS);
 
             return;
         }
 
         if (!hasValidWCNSEC) {
-            log.debug("NameError response has failed to prove: " +
+            mErrorList.add("NameError response has failed to prove covering wildcard does not exist");
-                "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<String> listTrustedKeys() {
         return mTrustedKeys.listTrustAnchors();
     }
-}
+
+    public List<String> getErrorList() {
+        return mErrorList;
+    }
+ }

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<NSEC3Record> nsec3s, NSEC3Parameters params,
-        ByteArrayComparator bac, boolean proveDoesNotExist) {
+        ByteArrayComparator bac, boolean proveDoesNotExist, List<String> 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<NSEC3Record> nsec3s, Name qname,
-        Name zonename) {
+        Name zonename, List<String> 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<NSEC3Record> nsec3s, Name qname,
-        int qtype, Name zonename) {
+        int qtype, Name zonename, List<String> 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<NSEC3Record> nsec3s, Name qname,
-        Name zonename, Name wildcard) {
+        Name zonename, Name wildcard, List<String> 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<NSEC3Record> nsec3s, Name qname,
-        Name zonename) {
+        Name zonename, List<String> 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;
     }