General cleanup origin
authorDavid Blacka <david@blacka.com>
Sun, 26 Apr 2009 02:41:43 +0000 (22:41 -0400)
committerDavid Blacka <david@blacka.com>
Sun, 26 Apr 2009 02:41:43 +0000 (22:41 -0400)
src/com/versign/tat/dnssec/CaptiveValidator.java
src/com/versign/tat/dnssec/DnsSecVerifier.java
src/com/versign/tat/dnssec/NSEC3ValUtils.java
src/com/versign/tat/dnssec/SecurityStatus.java
src/com/versign/tat/dnssec/SignUtils.java
src/com/versign/tat/dnssec/Util.java
src/com/versign/tat/dnssec/ValUtils.java

index f32b863..afd5689 100644 (file)
@@ -52,10 +52,13 @@ public class CaptiveValidator {
     // The local verification utility.
     private DnsSecVerifier   mVerifier;
 
+    private List<String>     mErrors;
+
     public CaptiveValidator() {
         mVerifier = new DnsSecVerifier();
         mValUtils = new ValUtils(mVerifier);
         mTrustedKeys = new TrustAnchorStore();
+        mErrors = new LinkedList<String>();
     }
 
     // ---------------- Module Initialization -------------------
@@ -114,7 +117,7 @@ public class CaptiveValidator {
             mTrustedKeys.store(cur_rrset);
         }
     }
-    
+
     public void addTrustedKeysFromResponse(Message m) {
         RRset[] rrsets = m.getSectionRRsets(Section.ANSWER);
         for (int i = 0; i < rrsets.length; ++i) {
@@ -178,15 +181,15 @@ public class CaptiveValidator {
 
                     sname = cname_alias;
                 } catch (NameTooLongException e) {
-//                    log.debug("not adding synthesized CNAME -- "
-//                              + "generated name is too long", e);
+                    // log.debug("not adding synthesized CNAME -- "
+                    // + "generated name is too long", e);
                 }
                 continue;
             }
 
             // The only records in the ANSWER section not allowed to
             if (!n.equals(sname)) {
-//                log.debug("normalize: removing irrelevant rrset: " + rrset);
+                // log.debug("normalize: removing irrelevant rrset: " + rrset);
                 i.remove();
                 continue;
             }
@@ -194,8 +197,8 @@ public class CaptiveValidator {
             // Follow the CNAME chain.
             if (type == Type.CNAME) {
                 if (rrset.size() > 1) {
-//                    log.debug("Found CNAME rrset with size > 1: " + rrset);
-//                    return Util.errorMessage(m, Rcode.SERVFAIL);
+                    // log.debug("Found CNAME rrset with size > 1: " + rrset);
+                    // return Util.errorMessage(m, Rcode.SERVFAIL);
                     return null; // FIXME
                 }
 
@@ -206,7 +209,7 @@ public class CaptiveValidator {
 
             // Otherwise, make sure that the RRset matches the qtype.
             if (qtype != Type.ANY && qtype != type) {
-//                log.debug("normalize: removing irrelevant rrset: " + rrset);
+                // log.debug("normalize: removing irrelevant rrset: " + rrset);
                 i.remove();
             }
 
@@ -236,7 +239,7 @@ public class CaptiveValidator {
 
         return m;
     }
-    
+
     /**
      * Extract additional names from the records in an rrset.
      * 
@@ -256,7 +259,7 @@ public class CaptiveValidator {
             }
         }
     }
-    
+
     private SRRset findKeys(SMessage message) {
         Name qname = message.getQName();
         int qclass = message.getQClass();
@@ -287,8 +290,9 @@ public class CaptiveValidator {
             // log.trace("non-answer: " + response);
             return false;
         }
-        
-        if (!mTrustedKeys.isBelowTrustAnchor(message.getQName(), message.getQClass())) {
+
+        if (!mTrustedKeys.isBelowTrustAnchor(message.getQName(),
+                                             message.getQClass())) {
             return false;
         }
         return true;
@@ -433,7 +437,7 @@ public class CaptiveValidator {
 
         boolean secure_delegation = false;
         Name delegation = null;
-        Name nsec3zone  = null;
+        Name nsec3zone = null;
         NSECRecord nsec = null;
         List<NSEC3Record> nsec3s = null;
 
@@ -466,14 +470,15 @@ public class CaptiveValidator {
                 if (nsec3s == null) nsec3s = new ArrayList<NSEC3Record>();
                 NSEC3Record nsec3 = (NSEC3Record) rrsets[i].first();
                 nsec3s.add(nsec3);
-                nsec3zone = rrsets[i].getSignerName(); // this is a hack of sorts.
+                nsec3zone = rrsets[i].getSignerName(); // this is a hack of
+                                                       // sorts.
                 break;
             default:
                 // FIXME: should probably whine if we see something else.
                 break;
             }
         }
-        
+
         // At this point, all validatable RRsets have been validated.
         // Now to check to see if we have a valid combination of things.
         if (delegation == null) {
@@ -481,10 +486,10 @@ public class CaptiveValidator {
             m.setStatus(SecurityStatus.BOGUS);
             return;
         }
-        
+
         if (secure_delegation) {
             if (nsec != null || nsec3s.size() > 0) {
-                // we found both a DS rrset *and* NSEC/NSEC3 rrsets! 
+                // we found both a DS rrset *and* NSEC/NSEC3 rrsets!
                 m.setStatus(SecurityStatus.BOGUS);
                 return;
             }
@@ -497,23 +502,25 @@ public class CaptiveValidator {
         if (nsec != null) {
             byte status = ValUtils.nsecProvesNoDS(nsec, delegation);
             if (status != SecurityStatus.SECURE) {
-                // The NSEC *must* prove that there was no DS record.  The INSECURE state here is still bogus.
+                // The NSEC *must* prove that there was no DS record. The
+                // INSECURE state here is still bogus.
                 m.setStatus(SecurityStatus.BOGUS);
                 return;
             }
             m.setStatus(SecurityStatus.SECURE);
             return;
         }
-        
+
         if (nsec3s.size() > 0) {
             byte status = NSEC3ValUtils.proveNoDS(nsec3s, delegation, nsec3zone);
             if (status != SecurityStatus.SECURE) {
-                // the NSEC3 RRs MUST prove no DS, so the INDETERMINATE state is actually bogus
+                // the NSEC3 RRs MUST prove no DS, so the INDETERMINATE state is
+                // actually bogus
                 m.setStatus(SecurityStatus.BOGUS);
                 return;
             }
             m.setStatus(SecurityStatus.SECURE);
-            return;            
+            return;
         }
 
         // failed to find proof either way.
@@ -628,7 +635,8 @@ public class CaptiveValidator {
         // closest encloser.
         NSECRecord wc = null; // for wildcard NODATA responses. This is the
         // wildcard NSEC.
-        List<NSEC3Record> nsec3s = null; // A collection of NSEC3 RRs found in the authority
+        List<NSEC3Record> nsec3s = null; // A collection of NSEC3 RRs found in
+                                         // the authority
         // section.
         Name nsec3Signer = null; // The RRSIG signer field for the NSEC3 RRs.
 
@@ -806,11 +814,11 @@ public class CaptiveValidator {
         // FIXME: it is unclear if we should actually normalize our responses
         // Instead, maybe we should just fail if they are not normal?
         message = normalize(message);
-        
-        if (! needsValidation(message)) {
+
+        if (!needsValidation(message)) {
             return SecurityStatus.UNCHECKED;
         }
-        
+
         SRRset key_rrset = findKeys(message);
         if (key_rrset == null) {
             return SecurityStatus.BOGUS;
index e43b987..7b19f1d 100644 (file)
@@ -35,9 +35,6 @@ import java.security.*;
 import org.xbill.DNS.*;
 import org.xbill.DNS.security.*;
 
-import com.versign.tat.dnssec.SecurityStatus;
-import com.versign.tat.dnssec.Util;
-
 
 /**
  * A class for performing basic DNSSEC verification. The DNSJAVA package
@@ -70,18 +67,21 @@ public class DnsSecVerifier
     }
   }
 
-  public DnsSecVerifier()
-  {
-    mAlgorithmMap = new HashMap<Integer, AlgEntry>();
-
-    // set the default algorithm map.
-    mAlgorithmMap.put(new Integer(DNSSEC.RSAMD5), new AlgEntry("MD5withRSA",
-        DNSSEC.RSAMD5, false));
-    mAlgorithmMap.put(new Integer(DNSSEC.DSA), new AlgEntry("SHA1withDSA", DNSSEC.DSA,
-        true));
-    mAlgorithmMap.put(new Integer(DNSSEC.RSASHA1), new AlgEntry(
-        "SHA1withRSA", DNSSEC.RSASHA1, false));
-  }
+  public DnsSecVerifier() {
+        mAlgorithmMap = new HashMap<Integer, AlgEntry>();
+
+        // set the default algorithm map.
+        mAlgorithmMap.put(new Integer(DNSSEC.RSAMD5), new AlgEntry(
+                "MD5withRSA", DNSSEC.RSAMD5, false));
+        mAlgorithmMap.put(new Integer(DNSSEC.DSA), new AlgEntry("SHA1withDSA",
+                DNSSEC.DSA, true));
+        mAlgorithmMap.put(new Integer(DNSSEC.RSASHA1), new AlgEntry(
+                "SHA1withRSA", DNSSEC.RSASHA1, false));
+        mAlgorithmMap.put(new Integer(DNSSEC.DSA_NSEC3_SHA1), new AlgEntry(
+                "SHA1withDSA", DNSSEC.DSA_NSEC3_SHA1, true));
+        mAlgorithmMap.put(new Integer(DNSSEC.RSA_NSEC3_SHA1), new AlgEntry(
+                "SHA1withRSA", DNSSEC.RSA_NSEC3_SHA1, false));
+    }
 
   private boolean isDSA(int algorithm)
   {
@@ -95,48 +95,6 @@ public class DnsSecVerifier
     return false;
   }
 
-  public void init(Properties config)
-  {
-    if (config == null) return;
-
-    // Algorithm configuration
-
-    // For now, we just accept new identifiers for existing algoirthms.
-    // FIXME: handle private identifiers.
-    List<Util.ConfigEntry> aliases = Util.parseConfigPrefix(config, "dns.algorithm.");
-
-    for (Util.ConfigEntry entry : aliases) {
-      Integer alg_alias = new Integer(Util.parseInt(entry.key, -1));
-      Integer alg_orig = new Integer(Util.parseInt(entry.value, -1));
-
-      if (!mAlgorithmMap.containsKey(alg_orig))
-      {
-//        log.warn("Unable to alias " + alg_alias + " to unknown algorithm "
-//            + alg_orig);
-        continue;
-      }
-
-      if (mAlgorithmMap.containsKey(alg_alias))
-      {
-//        log.warn("Algorithm alias " + alg_alias
-//            + " is already defined and cannot be redefined");
-        continue;
-      }
-
-      mAlgorithmMap.put(alg_alias, mAlgorithmMap.get(alg_orig));
-    }
-
-    // for debugging purposes, log the entire algorithm map table.
-//    for (Integer alg : mAlgorithmMap.keySet()) {
-//      AlgEntry entry =  mAlgorithmMap.get(alg);
-//      if (entry == null) 
-//        log.warn("DNSSEC alg " + alg + " has a null entry!");
-//      else
-//        log.debug("DNSSEC alg " + alg + " maps to " + entry.jcaName
-//            + " (" + entry.dnssecAlg + ")");
-//    }
-  }
-
   /**
    * Find the matching DNSKEY(s) to an RRSIG within a DNSKEY rrset. Normally
    * this will only return one DNSKEY. It can return more than one, since
index a4316dd..374a0db 100644 (file)
@@ -40,8 +40,7 @@ import com.versign.tat.dnssec.SignUtils.ByteArrayComparator;
 public class NSEC3ValUtils {
 
     // FIXME: should probably refactor to handle different NSEC3 parameters more
-    // efficiently.
-    // Given a list of NSEC3 RRs, they should be grouped according to
+    // efficiently.  Given a list of NSEC3 RRs, they should be grouped according to
     // parameters. The idea is to hash and compare for each group independently,
     // instead of having to skip NSEC3 RRs with the wrong parameters.
 
@@ -319,8 +318,7 @@ public class NSEC3ValUtils {
         NSEC3Record nsec3;
 
         // This scans from longest name to shortest, so the first match we find
-        // is
-        // the only viable candidate.
+        // is the only viable candidate.
         // FIXME: modify so that the NSEC3 matching the zone apex need not be
         // present.
         while (n.labels() >= zonename.labels()) {
index ee59058..6f9d190 100644 (file)
@@ -1,6 +1,4 @@
 /*
- * $Id$
- * 
  * Copyright (c) 2005 VeriSign. All rights reserved.
  * 
  * Redistribution and use in source and binary forms, with or without
@@ -31,82 +29,73 @@ package com.versign.tat.dnssec;
 
 /**
  * Codes for DNSSEC security statuses.
- * 
- * @author davidb
  */
-public class SecurityStatus
-{
+public class SecurityStatus {
+
+    /**
+     * UNCHECKED means that object has yet to be validated.
+     */
+    public static final byte UNCHECKED     = 0;
+    /**
+     * BOGUS means that the object (RRset or message) failed to validate
+     * (according to local policy), but should have validated.
+     */
+    public static final byte BOGUS         = 1;
+    /**
+     * BAD is a synonym for BOGUS.
+     */
+    public static final byte BAD           = BOGUS;
+    /**
+     * INDTERMINATE means that the object is insecure, but not authoritatively
+     * so. Generally this means that the RRset is not below a configured trust
+     * anchor.
+     */
+    public static final byte INDETERMINATE = 2;
+    /**
+     * INSECURE means that the object is authoritatively known to be insecure.
+     * Generally this means that this RRset is below a trust anchor, but also
+     * below a verified, insecure delegation.
+     */
+    public static final byte INSECURE      = 3;
+    /**
+     * SECURE means that the object (RRset or message) validated according to
+     * local policy.
+     */
+    public static final byte SECURE        = 4;
+
+    private byte             status;
 
-  /**
-   * UNCHECKED means that object has yet to be validated.
-   */
-  public static final byte UNCHECKED     = 0;
-  /**
-   * BOGUS means that the object (RRset or message) failed to validate
-   * (according to local policy), but should have validated.
-   */
-  public static final byte BOGUS         = 1;
-  /**
-   * BAD is a synonym for BOGUS.
-   */
-  public static final byte BAD           = BOGUS;
-  /**
-   * INDTERMINATE means that the object is insecure, but not authoritatively
-   * so. Generally this means that the RRset is not below a configured trust
-   * anchor.
-   */
-  public static final byte INDETERMINATE = 2;
-  /**
-   * INSECURE means that the object is authoritatively known to be insecure.
-   * Generally this means that this RRset is below a trust anchor, but also
-   * below a verified, insecure delegation.
-   */
-  public static final byte INSECURE      = 3;
-  /**
-   * SECURE means that the object (RRset or message) validated according to
-   * local policy.
-   */
-  public static final byte SECURE        = 4;
+    public static String string(int status) {
+        switch (status) {
+        case BOGUS:
+            return "Bogus";
+        case SECURE:
+            return "Secure";
+        case INSECURE:
+            return "Insecure";
+        case INDETERMINATE:
+            return "Indeterminate";
+        case UNCHECKED:
+            return "Unchecked";
+        default:
+            return "UNKNOWN";
+        }
+    }
 
-  private byte              status;
+    public SecurityStatus() {
+        status = UNCHECKED;
+    }
 
-  public static String string(int status)
-  {
-    switch (status)
-    {
-      case BOGUS :
-        return "Bogus";
-      case SECURE :
-        return "Secure";
-      case INSECURE :
-        return "Insecure";
-      case INDETERMINATE :
-        return "Indeterminate";
-      case UNCHECKED :
-        return "Unchecked";
-      default :
-        return "UNKNOWN";
+    public SecurityStatus(byte status) {
+        setStatus(status);
     }
-  }
 
-  public SecurityStatus() 
-  {
-    status = UNCHECKED;
-  }
-  
-  public SecurityStatus(byte status)
-  {
-    setStatus(status);
-  }
-  
-  public byte getStatus()
-  {
-    return status;
-  }
+    public byte getStatus() {
+        return status;
+    }
 
-  public void setStatus(byte status)
-  {
-    this.status = status;
-  }
+    public void setStatus(byte status) {
+        this.status = status;
+    }
 
 }
index 8316407..de23ee9 100644 (file)
@@ -1,6 +1,4 @@
 /*
- * $Id$
- *
  * Copyright (c) 2005 VeriSign, Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -185,7 +183,7 @@ public class SignUtils {
         // Generate the binary image;
         DNSOutput image = new DNSOutput();
 
-        // precalculate some things
+        // pre-calculate some things
         int start_time = (int) (presig.getTimeSigned().getTime() / 1000);
         int expire_time = (int) (presig.getExpire().getTime() / 1000);
         Name signer = presig.getSigner();
@@ -212,7 +210,7 @@ public class SignUtils {
      * @param ttl
      *            the TTL to use when canonicalizing -- this is generally the
      *            TTL of the signature if there is a pre-existing signature. If
-     *            not it is just the ttl of the rrset itself.
+     *            not it is just the TTL of the rrset itself.
      * @param labels
      *            the labels field of the signature, or 0.
      * @return the canonical wire line format of the rrset. This is the second
index 967be77..3348515 100644 (file)
@@ -1,6 +1,4 @@
 /*
- * $Id$
- * 
  * Copyright (c) 2005 VeriSign. All rights reserved.
  * 
  * Redistribution and use in source and binary forms, with or without
 
 package com.versign.tat.dnssec;
 
-import java.util.*;
-
 import org.xbill.DNS.Name;
 
 /**
  * Some basic utility functions.
  */
-public class Util
-{
-
-  /**
-   * Convert a DNS name into a string suitable for use as a cache key.
-   * 
-   * @param name The name to convert.
-   * @return A string representing the name. This isn't ever meant to be
-   *         converted back into a DNS name.
-   */
-  public static String nameToString(Name name)
-  {
-    if (name.equals(Name.root)) return ".";
+public class Util {
 
-    String n = name.toString().toLowerCase();
-    if (n.endsWith(".")) n = n.substring(0, n.length() - 1);
+    /**
+     * Convert a DNS name into a string suitable for use as a cache key.
+     * 
+     * @param name
+     *            The name to convert.
+     * @return A string representing the name. This isn't ever meant to be
+     *         converted back into a DNS name.
+     */
+    public static String nameToString(Name name) {
+        if (name.equals(Name.root)) return ".";
 
-    return n;
-  }
+        String n = name.toString().toLowerCase();
+        if (n.endsWith(".")) n = n.substring(0, n.length() - 1);
 
-  public static int parseInt(String s, int def)
-  {
-    if (s == null) return def;
-    try
-    {
-      return Integer.parseInt(s);
+        return n;
     }
-    catch (NumberFormatException e)
-    {
-      return def;
-    }
-  }
 
-  public static long parseLong(String s, long def)
-  {
-    if (s == null) return def;
-    try
-    {
-      return Long.parseLong(s);
-    }
-    catch (NumberFormatException e)
-    {
-      return def;
-    }
-  }
-  
-  public static class ConfigEntry
-  {
-    public String key;
-    public String value;
-    
-    public ConfigEntry(String key, String value)
-    {
-      this.key = key; this.value = value;
-    }
-  }
-  
-  public static List<ConfigEntry> parseConfigPrefix(Properties config, String prefix)
-  {
-    if (! prefix.endsWith("."))
-    {
-      prefix = prefix + ".";
+    public static int parseInt(String s, int def) {
+        if (s == null) return def;
+        try {
+            return Integer.parseInt(s);
+        } catch (NumberFormatException e) {
+            return def;
+        }
     }
-    
-    List<ConfigEntry> res = new ArrayList<ConfigEntry>();
-    
-    for (Map.Entry<Object, Object> entry : config.entrySet()) {
-        String key = (String) entry.getKey();
-        if (key.startsWith(prefix)) {
-            key = key.substring(prefix.length());
-            res.add(new ConfigEntry(key, (String) entry.getValue()));
+
+    public static long parseLong(String s, long def) {
+        if (s == null) return def;
+        try {
+            return Long.parseLong(s);
+        } catch (NumberFormatException e) {
+            return def;
         }
     }
-    
-    return res;
-  }
 }
index 873733f..e427978 100644 (file)
@@ -37,7 +37,7 @@ import org.xbill.DNS.*;
 /**
  * This is a collection of routines encompassing the logic of validating
  * different message types.
-*/
+ */
 
 public class ValUtils {
 
@@ -46,7 +46,7 @@ public class ValUtils {
     // algorithm, so they are confined here.
 
     public enum ResponseType {
-        UNTYPED, // not sub typed yet
+        UNTYPED, // not subtyped yet
         UNKNOWN, // not a recognized sub type
         POSITIVE, // a positive response (no CNAME/DNAME chain)
         CNAME, // a positive response with a CNAME/DNAME chain.
@@ -60,7 +60,7 @@ public class ValUtils {
     }
 
     /** A local copy of the verifier object. */
-    private DnsSecVerifier  mVerifier;
+    private DnsSecVerifier mVerifier;
 
     public ValUtils(DnsSecVerifier verifier) {
         mVerifier = verifier;
@@ -156,7 +156,6 @@ public class ValUtils {
         return null;
     }
 
-
     /**
      * Given a DNSKEY record, generate the DS record from it.
      * 
@@ -164,7 +163,7 @@ public class ValUtils {
      *            the DNSKEY record in question.
      * @param ds_alg
      *            The DS digest algorithm in use.
-     * @return the corresponding {@link org.xbill.DNS.DSRecord}
+     * @return the corresponding hash value.
      */
     public static byte[] calculateDSHash(DNSKEYRecord keyrec, int ds_alg) {
         DNSOutput os = new DNSOutput();
@@ -192,6 +191,14 @@ public class ValUtils {
         }
     }
 
+    /**
+     * Determine if the given DS digest algorithm is supported.
+     * 
+     * @param digest_id
+     *            The numeric digest identifier.
+     * 
+     * @return true if the algorithm id is supported.
+     */
     public static boolean supportsDigestID(int digest_id) {
         if (digest_id == DSRecord.SHA1_DIGEST_ID) return true;
         if (digest_id == DSRecord.SHA256_DIGEST_ID) return true;
@@ -243,15 +250,9 @@ public class ValUtils {
      * less) and all of the RRsets.
      * 
      * @param m
+     *            The message to set the security status of.
      * @param security
-     *            KeyEntry ke;
-     * 
-     *            SMessage m = response.getSMessage(); SRRset ans_rrset =
-     *            m.findAnswerRRset(qname, qtype, qclass);
-     * 
-     *            ke = verifySRRset(ans_rrset, key_rrset); if
-     *            (ans_rrset.getSecurityStatus() != SecurityStatus.SECURE) {
-     *            return; } key_rrset = ke.getRRset();
+     *            The status to upgrade to, if possible.
      */
     public static void setMessageSecurity(SMessage m, byte security) {
         if (m == null) return;
@@ -281,27 +282,15 @@ public class ValUtils {
      * @return The status (BOGUS or SECURE).
      */
     public byte verifySRRset(SRRset rrset, SRRset key_rrset) {
-//        String rrset_name = rrset.getName() + "/"
-//                            + Type.string(rrset.getType()) + "/"
-//                            + DClass.string(rrset.getDClass());
 
         if (rrset.getSecurityStatus() == SecurityStatus.SECURE) {
-            // log.trace("verifySRRset: rrset <" + rrset_name
-            // + "> previously found to be SECURE");
             return SecurityStatus.SECURE;
         }
 
         byte status = mVerifier.verify(rrset, key_rrset);
         if (status != SecurityStatus.SECURE) {
-            // log.debug("verifySRRset: rrset <" + rrset_name +
-            // "> found to be BAD");
             status = SecurityStatus.BOGUS;
         }
-        // else
-        // {
-        // log.trace("verifySRRset: rrset <" + rrset_name +
-        // "> found to be SECURE");
-        // }
 
         rrset.setSecurityStatus(status);
         return status;
@@ -335,8 +324,11 @@ public class ValUtils {
      * Finds the longest common name between two domain names.
      * 
      * @param domain1
+     *            The first domain.
      * @param domain2
-     * @return
+     *            The second domain.
+     * @return The longest common ancestor domain between the two domain names.
+     *         If there was no common ancestor, then the root name is returned.
      */
     public static Name longestCommonName(Name domain1, Name domain2) {
         if (domain1 == null || domain2 == null) return null;
@@ -358,6 +350,17 @@ public class ValUtils {
         return Name.root;
     }
 
+    /**
+     * Determine if the child name is strictly a subdomain of the parent -- the
+     * normal subdomain check will return true if the names are equal. This will
+     * not.
+     * 
+     * @param child
+     *            The purported child name.
+     * @param parent
+     *            The purported parent name.
+     * @return true if the child is below the parent.
+     */
     public static boolean strictSubdomain(Name child, Name parent) {
         int clabels = child.labels();
         int plabels = parent.labels();
@@ -411,6 +414,17 @@ public class ValUtils {
         return null;
     }
 
+    /**
+     * Calculate the "closest encloser" to a domain given its covering NSEC
+     * record. The closest encloser is the longest existing ancestor to the
+     * name.
+     * 
+     * @param domain
+     *            The domain in question.
+     * @param nsec
+     *            The covering NSEC record.
+     * @return the closest encloser.
+     */
     public static Name closestEncloser(Name domain, NSECRecord nsec) {
         Name n1 = longestCommonName(domain, nsec.getName());
         Name n2 = longestCommonName(domain, nsec.getNext());
@@ -418,6 +432,16 @@ public class ValUtils {
         return (n1.labels() > n2.labels()) ? n1 : n2;
     }
 
+    /**
+     * Calculate the possible wildcard that could have generated 'domain' based
+     * on the covering NSEC record.
+     * 
+     * @param domain
+     *            The domain in question.
+     * @param nsec
+     *            The covering NSEC record.
+     * @return A wildcard name that could have generated 'domain' if it existed.
+     */
     public static Name nsecWildcard(Name domain, NSECRecord nsec) {
         try {
             return new Name("*", closestEncloser(domain, nsec));
@@ -454,9 +478,9 @@ public class ValUtils {
         // If NSEC is a parent of qname, we need to check the type map
         // If the parent name has a DNAME or is a delegation point, then this
         // NSEC is being misused.
-        boolean hasBadType = typeMapHasType(nsec.getTypes(), Type.DNAME)
-                             || (typeMapHasType(nsec.getTypes(), Type.NS) && !typeMapHasType(nsec.getTypes(),
-                                                                                             Type.SOA));
+        int types[] = nsec.getTypes();
+        boolean hasBadType = typeMapHasType(types, Type.DNAME) || 
+            (typeMapHasType(types, Type.NS) && !typeMapHasType(types, Type.SOA));
         if (qname.subdomain(owner) && hasBadType) {
             return false;
         }
@@ -473,7 +497,7 @@ public class ValUtils {
      * could have produced qname.
      * 
      * @param nsec
-     *            The nsec to check.
+     *            The NSEC record to check.
      * @param qname
      *            The qname to check against.
      * @param signerName
@@ -516,70 +540,83 @@ public class ValUtils {
      */
     public static boolean nsecProvesNodata(NSECRecord nsec, Name qname,
                                            int qtype) {
+        int types[] = nsec.getTypes();
+
         if (!nsec.getName().equals(qname)) {
-            // wildcard checking.
+            // Wildcard Check
 
             // If this is a wildcard NSEC, make sure that a) it was possible to
-            // have
-            // generated qname from the wildcard and b) the type map does not
-            // contain qtype. Note that this does NOT prove that this wildcard
-            // was
-            // the applicable wildcard.
+            // have generated qname from the wildcard and b) the type map does
+            // not contain qtype. Note that this does NOT prove that this
+            // wildcard was the applicable wildcard.
             if (nsec.getName().isWild()) {
-                // the is the purported closest encloser.
+                // this is the purported closest encloser.
                 Name ce = new Name(nsec.getName(), 1);
 
                 // The qname must be a strict subdomain of the closest encloser,
-                // and
-                // the qtype must be absent from the type map.
-                if (!strictSubdomain(qname, ce)
-                    || typeMapHasType(nsec.getTypes(), qtype)) {
+                // and the qtype must be absent from the type map.
+                if (!strictSubdomain(qname, ce) || typeMapHasType(types, qtype)) {
                     return false;
                 }
                 return true;
             }
 
-            // empty-non-terminal checking.
+            // Empty-non-terminal Check
 
-            // If the nsec is proving that qname is an ENT, the nsec owner will
-            // be
-            // less than qname, and the next name will be a child domain of the
-            // qname.
+            // If the NSEC is proving that qname is an ENT, the NSEC owner will
+            // be less than qname, and the next name will be a child domain of
+            // the qname.
             if (strictSubdomain(nsec.getNext(), qname)
-                && qname.compareTo(nsec.getName()) > 0) {
+                    && qname.compareTo(nsec.getName()) > 0) {
                 return true;
             }
+
             // Otherwise, this NSEC does not prove ENT, so it does not prove
             // NODATA.
             return false;
         }
 
         // If the qtype exists, then we should have gotten it.
-        if (typeMapHasType(nsec.getTypes(), qtype)) {
+        if (typeMapHasType(types, qtype)) {
             return false;
         }
 
         // if the name is a CNAME node, then we should have gotten the CNAME
-        if (typeMapHasType(nsec.getTypes(), Type.CNAME)) {
+        if (typeMapHasType(types, Type.CNAME)) {
             return false;
         }
 
         // If an NS set exists at this name, and NOT a SOA (so this is a zone
-        // cut,
-        // not a zone apex), then we should have gotten a referral (or we just
-        // got
-        // the wrong NSEC).
-        if (typeMapHasType(nsec.getTypes(), Type.NS)
-            && !typeMapHasType(nsec.getTypes(), Type.SOA)) {
+        // cut, not a zone apex), then we should have gotten a referral (or we
+        // just got he wrong NSEC).
+        if (typeMapHasType(types, Type.NS) && !typeMapHasType(types, Type.SOA)) {
             return false;
         }
 
         return true;
     }
 
+    /**
+     * Determine if the NSEC record proves that there was no DS record present
+     * at the delegation point.
+     * 
+     * @param nsec
+     *            The NSEC record attempting to prove that there was no DS
+     *            present.
+     * @param qname
+     *            The query name.
+     * @return The security status determined by the proof: BOGUS if the NSEC
+     *         record doesn't prove no DS (either by proving that the DS exists,
+     *         or being the wrong NSEC record (from the wrong side of the zone
+     *         cut). INSECURE means that the NSEC record wasn't for a delegation
+     *         point, so it didn't prove anything either way. SECURE if the DS
+     *         was proven to not exist.
+     */
     public static byte nsecProvesNoDS(NSECRecord nsec, Name qname) {
-        // Could check to make sure the qname is a subdomain of nsec
+        // FIXME: Could check to make sure the qname is a subdomain of NSEC
+
         int[] types = nsec.getTypes();
+
         if (typeMapHasType(types, Type.SOA) || typeMapHasType(types, Type.DS)) {
             // SOA present means that this is the NSEC from the child, not the
             // parent (so it is the wrong one)