output changes for VerifyZone, some code cleanup and bug fixes for ZoneVerifier

git-svn-id: https://svn.verisignlabs.com/jdnssec/tools/trunk@220 4cbd57fe-54e5-0310-bd9a-f30fe5ea5e6e

src/com/verisignlabs/dnssec/cl/VerifyZone.java

@@ -19,16 +19,25 @@
 
 package com.verisignlabs.dnssec.cl;
 
-//import java.io.File;
-//import java.io.IOException;
 import java.io.PrintWriter;
 import java.util.List;
+import java.util.logging.Handler;
 import java.util.logging.Level;
+import java.util.logging.LogRecord;
 import java.util.logging.Logger;
 
-import org.apache.commons.cli.*;
+import org.apache.commons.cli.AlreadySelectedException;
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.CommandLineParser;
+import org.apache.commons.cli.HelpFormatter;
+import org.apache.commons.cli.OptionBuilder;
 import org.apache.commons.cli.Options;
-import com.verisignlabs.dnssec.security.*;
+import org.apache.commons.cli.PosixParser;
+import org.apache.commons.cli.UnrecognizedOptionException;
+
+import com.verisignlabs.dnssec.security.DnsKeyAlgorithm;
+import com.verisignlabs.dnssec.security.ZoneUtils;
+import com.verisignlabs.dnssec.security.ZoneVerifier;
 
 /**
  * This class forms the command line implementation of a DNSSEC zone validator.
@@ -41,6 +50,24 @@ public class VerifyZone
 {
   private static Logger log;
 
+  // A log formatter that strips away all of the noise that the default SimpleFormatter has
+  private static class MyLogFormatter extends java.util.logging.Formatter
+  {
+    @Override
+    public String format(LogRecord arg0)
+    {
+      StringBuilder out = new StringBuilder();
+      String lvl = arg0.getLevel().getName();
+      
+      out.append(lvl);
+      out.append(": ");
+      out.append(arg0.getMessage());
+      out.append("\n");
+
+      return out.toString();
+    }
+  }
+
   /**
    * This is a small inner class used to hold all of the command line option
    * state.
@@ -48,8 +75,8 @@ public class VerifyZone
   private static class CLIState
   {
     private Options opts;
-//    public boolean  strict   = false;
+    //    public boolean  strict   = false;
-//    public File     keydir   = null;
+    //    public File     keydir   = null;
     public String   zonefile = null;
     public String[] keyfiles = null;
 
@@ -69,25 +96,13 @@ public class VerifyZone
 
       // boolean options
       opts.addOption("h", "help", false, "Print this message.");
-//      opts.addOption("s", "strict", false,
+      opts.addOption("m", "multiline", false, "log DNS records using 'multiline' format");
-//          "Zone will only be considered valid if all "
-//              + "signatures could be cryptographically verified");
-      opts.addOption("m", "multiline", false,
-          "log DNS records using 'multiline' format");
-
-      // Argument options
-//      OptionBuilder.hasArg();
-//      OptionBuilder.withLongOpt("keydir");
-//      OptionBuilder.withArgName("dir");
-//      OptionBuilder.withDescription("directory to find " + "trusted key files");
-//      opts.addOption(OptionBuilder.create('d'));
 
       OptionBuilder.hasOptionalArg();
       OptionBuilder.withLongOpt("verbose");
       OptionBuilder.withArgName("level");
       OptionBuilder.withDescription("verbosity level -- 0 is silence, "
-          + "5 is debug information, 6 is trace information.\n"
+          + "5 is debug information, 6 is trace information.\n" + "default is level 5.");
-          + "default is level 5.");
       opts.addOption(OptionBuilder.create('v'));
 
       OptionBuilder.hasArg();
@@ -103,14 +118,12 @@ public class VerifyZone
       CommandLineParser cli_parser = new PosixParser();
       CommandLine cli = cli_parser.parse(opts, args);
 
-//      String optstr = null;
-
       if (cli.hasOption('h')) usage();
 
+      Logger rootLogger = Logger.getLogger("");
       if (cli.hasOption('v'))
       {
         int value = parseInt(cli.getOptionValue('v'), 1);
-        Logger rootLogger = Logger.getLogger("");
         switch (value)
         {
           case 0:
@@ -128,19 +141,18 @@ public class VerifyZone
             break;
         }
       }
-
+      // I hate java.util.logging, btw.
-//      if (cli.hasOption('s')) strict = true;
+      for (Handler h : rootLogger.getHandlers())
+      {
+        h.setLevel(rootLogger.getLevel());
+        h.setFormatter(new MyLogFormatter());
+      }
 
       if (cli.hasOption('m'))
       {
         org.xbill.DNS.Options.set("multiline");
       }
 
-//      if ((optstr = cli.getOptionValue('d')) != null)
-//      {
-//        keydir = new File(optstr);
-//      }
-
       String[] optstrs = null;
       if ((optstrs = cli.getOptionValues('A')) != null)
       {
@@ -195,8 +207,8 @@ public class VerifyZone
 
       // print our own usage statement:
       f.printHelp(out, 75, "jdnssec-verifyzone [..options..] zonefile "
-          + "[keyfile [keyfile...]]", null, opts,
+          + "[keyfile [keyfile...]]", null, opts, HelpFormatter.DEFAULT_LEFT_PAD,
-          HelpFormatter.DEFAULT_LEFT_PAD, HelpFormatter.DEFAULT_DESC_PAD, null);
+                  HelpFormatter.DEFAULT_DESC_PAD, null);
 
       out.flush();
       System.exit(64);
@@ -259,8 +271,7 @@ public class VerifyZone
     }
     catch (UnrecognizedOptionException e)
     {
-      System.err
+      System.err.println("error: unknown option encountered: " + e.getMessage());
-          .println("error: unknown option encountered: " + e.getMessage());
       state.usage();
     }
     catch (AlreadySelectedException e)

src/com/verisignlabs/dnssec/security/BINDKeyUtils.java

@@ -336,8 +336,6 @@ public class BINDKeyUtils
   {
     if (priv != null)
     {
-      // debug
-      // System.out.println("converting from privatekey to bind9 string");
       DnsKeyConverter keyconv = new DnsKeyConverter();
       String priv_string = keyconv.generatePrivateKeyString(priv, pub, alg);
 

src/com/verisignlabs/dnssec/security/ByteArrayComparator.java

@@ -19,6 +19,7 @@
 package com.verisignlabs.dnssec.security;
 
 import java.util.Comparator;
+import java.util.logging.Logger;
 
 /**
  * This class implements a basic comparator for byte arrays. It is primarily
@@ -33,6 +34,7 @@ public class ByteArrayComparator implements Comparator
 {
   private int     mOffset = 0;
   private boolean mDebug  = false;
+  private Logger log;
 
   public ByteArrayComparator()
   {
@@ -55,7 +57,7 @@ public class ByteArrayComparator implements Comparator
       {
         if (mDebug)
         {
-          System.out.println("offset " + i + " differs (this is "
+          log.info("offset " + i + " differs (this is "
               + (i - mOffset) + " bytes in from our offset.)");
         }
         return (b1[i] & 0xFF) - (b2[i] & 0xFF);

src/com/verisignlabs/dnssec/security/JCEDnsSecSigner.java

@@ -31,6 +31,7 @@ import java.util.Date;
 import java.util.Iterator;
 import java.util.List;
 import java.util.ListIterator;
+import java.util.logging.Logger;
 
 import org.xbill.DNS.DNSKEYRecord;
 import org.xbill.DNS.Name;
@@ -57,6 +58,8 @@ public class JCEDnsSecSigner
   private DnsKeyConverter mKeyConverter;
   private boolean         mVerboseSigning = false;
   
+  private Logger log;
+
   public JCEDnsSecSigner()
   {
     this.mKeyConverter = null;
@@ -139,8 +142,8 @@ public class JCEDnsSecSigner
 
     if (mVerboseSigning)
     {
-      System.out.println("Signing RRset:");
+      log.info("Signing RRset:");
-      System.out.println(ZoneUtils.rrsetToString(rrset, false));
+      log.info(ZoneUtils.rrsetToString(rrset, false));
     }
 
     // first, pre-calculate the RRset bytes.
@@ -161,9 +164,9 @@ public class JCEDnsSecSigner
 
       if (mVerboseSigning)
       {
-        System.out.println("Canonical pre-signature data to sign with key " + keyrec.getName().toString() + "/"
+        log.info("Canonical pre-signature data to sign with key " + keyrec.getName().toString() + "/"
             + keyrec.getAlgorithm() + "/" + keyrec.getFootprint() + ":");
-        System.out.println(hexdump.dump(null, sign_data));
+        log.info(hexdump.dump(null, sign_data));
       }
 
       Signature signer = pair.getSigner();
@@ -171,7 +174,7 @@ public class JCEDnsSecSigner
       if (signer == null)
       {
         // debug
-        System.out.println("missing private key that goes with:\n"
+        log.fine("missing private key that goes with:\n"
             + pair.getDNSKEYRecord());
         throw new GeneralSecurityException("cannot sign without a valid Signer "
             + "(probably missing private key)");
@@ -183,8 +186,8 @@ public class JCEDnsSecSigner
 
       if (mVerboseSigning)
       {
-        System.out.println("Raw Signature:");
+        log.info("Raw Signature:");
-        System.out.println(hexdump.dump(null, sig));
+        log.info(hexdump.dump(null, sig));
       }
 
       DnsKeyAlgorithm algs = DnsKeyAlgorithm.getInstance();
@@ -197,8 +200,7 @@ public class JCEDnsSecSigner
       RRSIGRecord sigrec = SignUtils.generateRRSIG(sig, presig);
       if (mVerboseSigning)
       {
-        System.out.println("RRSIG:\n" + sigrec);
+        log.info("RRSIG:\n" + sigrec);
-        System.out.println();
       }
       sigs.add(sigrec);
     }

src/com/verisignlabs/dnssec/security/ZoneUtils.java

@@ -173,7 +173,7 @@ public class ZoneUtils
   /** This is an alternate way to format an RRset into a string */
   public static String rrsetToString(RRset rrset, boolean includeSigs)
   {
-    StringBuffer out = new StringBuffer();
+    StringBuilder out = new StringBuilder();
 
     for (Iterator i = rrset.rrs(false); i.hasNext();)
     {

src/com/verisignlabs/dnssec/security/ZoneVerifier.java

@@ -20,8 +20,8 @@
 package com.verisignlabs.dnssec.security;
 
 import java.security.NoSuchAlgorithmException;
+import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -30,6 +30,7 @@ import java.util.Map;
 import java.util.Set;
 import java.util.SortedMap;
 import java.util.TreeMap;
+import java.util.logging.Logger;
 
 import org.xbill.DNS.DNSKEYRecord;
 import org.xbill.DNS.DNSSEC;
@@ -56,7 +57,7 @@ import org.xbill.DNS.utils.base32;
 public class ZoneVerifier
 {
 
-  private SortedMap<Name, Set>       mNodeMap;
+  private SortedMap<Name, Set<Integer>> mNodeMap;
   private HashMap<String, RRset>        mRRsetMap;
   private SortedMap<Name, MarkRRset>    mNSECMap;
   private SortedMap<Name, MarkRRset>    mNSEC3Map;
@@ -68,18 +69,22 @@ public class ZoneVerifier
   private base32                        mBase32;
   private ByteArrayComparator           mBAcmp;
 
+  private Logger                        log = Logger.getLogger("ZoneVerifier");
+
+  // The various types of signed zones.
   enum DNSSECType
   {
     UNSIGNED, NSEC, NSEC3, NSEC3_OPTOUT;
   }
 
+  // The types of nodes (a node consists of all RRs with the same name).
   enum NodeType
   {
     NORMAL, DELEGATION, GLUE;
   }
 
   /**
-   * This is a subclass of org.xbill.DNS.RRset that adds a "mark".
+   * This is a subclass of {@link org.xbill.DNS.RRset} that adds a "mark".
    */
   private class MarkRRset extends RRset
   {
@@ -98,16 +103,6 @@ public class ZoneVerifier
     }
   }
 
-  private class NameComparator implements Comparator
-  {
-    public int compare(Object o1, Object o2) throws ClassCastException
-    {
-      Name n1 = (Name) o1;
-
-      return n1.compareTo(o2);
-    }
-  }
-
   public ZoneVerifier()
   {
     mVerifier = new DnsSecVerifier();
@@ -120,6 +115,9 @@ public class ZoneVerifier
     return n.toString() + ':' + type;
   }
 
+  /**
+   * Add a record to the various maps.
+   */
   private void addRR(Record r)
   {
     Name r_name = r.getName();
@@ -127,12 +125,9 @@ public class ZoneVerifier
     if (r_type == Type.RRSIG) r_type = ((RRSIGRecord) r).getTypeCovered();
 
     // Add NSEC and NSEC3 RRs to their respective maps
-    if (r_type == Type.NSEC || r_type == Type.NSEC3)
+    if (r_type == Type.NSEC)
     {
-      if (mNSECMap == null)
+      if (mNSECMap == null) mNSECMap = new TreeMap<Name, MarkRRset>();
-      {
-        mNSECMap = new TreeMap(new NameComparator());
-      }
       MarkRRset rrset = mNSECMap.get(r_name);
       if (rrset == null)
       {
@@ -145,10 +140,7 @@ public class ZoneVerifier
 
     if (r_type == Type.NSEC3)
     {
-      if (mNSEC3Map == null)
+      if (mNSEC3Map == null) mNSEC3Map = new TreeMap<Name, MarkRRset>();
-      {
-        mNSEC3Map = new TreeMap(new NameComparator());
-      }
       MarkRRset rrset = mNSEC3Map.get(r_name);
       if (rrset == null)
       {
@@ -160,10 +152,10 @@ public class ZoneVerifier
     }
 
     // Add the name and type to the node map
-    Set typeset = mNodeMap.get(r_name);
+    Set<Integer> typeset = mNodeMap.get(r_name);
     if (typeset == null)
     {
-      typeset = new HashSet();
+      typeset = new HashSet<Integer>();
       mNodeMap.put(r_name, typeset);
     }
     typeset.add(r.getType()); // add the original type
@@ -179,6 +171,26 @@ public class ZoneVerifier
     rrset.addRR(r);
   }
 
+  /**
+   * Given a record, determine the DNSSEC signing type. If the record doesn't
+   * determine that, DNSSECType.UNSIGNED is returned
+   */
+  private DNSSECType determineDNSSECType(Record r)
+  {
+    if (r.getType() == Type.NSEC) return DNSSECType.NSEC;
+    if (r.getType() == Type.NSEC3)
+    {
+      NSEC3Record nsec3 = (NSEC3Record) r;
+      if ((nsec3.getFlags() & NSEC3Record.Flags.OPT_OUT) == NSEC3Record.Flags.OPT_OUT)
+      {
+        return DNSSECType.NSEC3_OPTOUT;
+      }
+      return DNSSECType.NSEC3;
+    }
+
+    return DNSSECType.UNSIGNED;
+  }
+
   /**
    * Given an unsorted list of records, load the node and rrset maps, as well as
    * determine the NSEC3 parameters and signing type.
@@ -187,11 +199,10 @@ public class ZoneVerifier
    */
   private void calculateNodes(List<Record> records)
   {
-    Comparator comparator = new NameComparator();
+    mNodeMap = new TreeMap<Name, Set<Integer>>();
-
-    mNodeMap = new TreeMap<Name, Set>(comparator);
     mRRsetMap = new HashMap<String, RRset>();
 
+    // The zone is unsigned until we get a clue otherwise.
     mDNSSECType = DNSSECType.UNSIGNED;
 
     for (Record r : records)
@@ -203,41 +214,19 @@ public class ZoneVerifier
       addRR(r);
 
       // Learn some things about the zone as we do this pass.
-      if (r_type == Type.SOA)
+      if (r_type == Type.SOA) mZoneName = r_name;
-      {
+      if (r_type == Type.NSEC3PARAM) mNSEC3params = (NSEC3PARAMRecord) r;
-        mZoneName = r_name;
+      if (r_type == Type.DNSKEY) mVerifier.addTrustedKey((DNSKEYRecord) r);
-      }
 
-      if (r_type == Type.NSEC3PARAM)
+      if (mDNSSECType == DNSSECType.UNSIGNED) mDNSSECType = determineDNSSECType(r);
-      {
-        mNSEC3params = (NSEC3PARAMRecord) r;
-      }
-
-      if (r_type == Type.DNSKEY)
-      {
-        mVerifier.addTrustedKey((DNSKEYRecord) r);
-      }
-
-      if (mDNSSECType == DNSSECType.UNSIGNED)
-      {
-        if (r_type == Type.NSEC) mDNSSECType = DNSSECType.NSEC;
-        if (r_type == Type.NSEC3)
-        {
-          NSEC3Record nsec3 = (NSEC3Record) r;
-          if ((nsec3.getFlags() & NSEC3Record.Flags.OPT_OUT) == NSEC3Record.Flags.OPT_OUT)
-          {
-            mDNSSECType = DNSSECType.NSEC3_OPTOUT;
-          }
-          else
-          {
-            mDNSSECType = DNSSECType.NSEC3;
-          }
-        }
-      }
     }
   }
 
-  private NodeType determineNodeType(Name n, Set typeset, Name last_cut)
+  /**
+   * Given a name, typeset, and name of the last zone cut, determine the node
+   * type.
+   */
+  private NodeType determineNodeType(Name n, Set<Integer> typeset, Name last_cut)
   {
     // All RRs at the zone apex are normal
     if (n.equals(mZoneName)) return NodeType.NORMAL;
@@ -254,17 +243,33 @@ public class ZoneVerifier
     return NodeType.NORMAL;
   }
 
+  private Set<Integer> cleanupDelegationTypeset(Set<Integer> typeset)
+  {
+    Set<Integer> t = new HashSet<Integer>();
+    if (typeset.contains(Type.NS)) t.add(Type.NS);
+    if (typeset.contains(Type.DS)) t.add(Type.DS);
+    if (typeset.contains(Type.RRSIG)) t.add(Type.RRSIG);
+
+    if (!typeset.equals(t)) return t;
+    return typeset;
+  }
+
+  /**
+   * For each node, determine which RRsets should be signed, verify those, and
+   * determine which nodes get NSEC or NSEC3 RRs and verify those.
+   */
   private int processNodes() throws NoSuchAlgorithmException, TextParseException
   {
     int errors = 0;
     Name last_cut = null;
 
-    for (Map.Entry<Name, Set> entry : mNodeMap.entrySet())
+    for (Map.Entry<Name, Set<Integer>> entry : mNodeMap.entrySet())
     {
       Name n = entry.getKey();
-      Set typeset = entry.getValue();
+      Set<Integer> typeset = entry.getValue();
 
       NodeType ntype = determineNodeType(n, typeset, last_cut);
+      log.finest("Node " + n + " is type " + ntype);
 
       // we can ignore glue/invalid RRs.
       if (ntype == NodeType.GLUE) continue;
@@ -276,9 +281,8 @@ public class ZoneVerifier
       }
 
       // check all of the RRset that should be signed
-      for (Object o : typeset)
+      for (int type : typeset)
       {
-        int type = ((Integer) o).intValue();
         if (type == Type.RRSIG) continue;
         // at delegation points, only DS RRs are signed (and NSEC, but those are checked separately)
         if (ntype == NodeType.DELEGATION && type != Type.DS) continue;
@@ -293,15 +297,7 @@ public class ZoneVerifier
       // the only types that should be there are NS, DS and RRSIG.
       if (ntype == NodeType.DELEGATION)
       {
-        Set newtypeset = new HashSet();
+        typeset = cleanupDelegationTypeset(typeset);
-        if (typeset.contains(Type.NS)) newtypeset.add(Type.NS);
-        if (typeset.contains(Type.DS)) newtypeset.add(Type.DS);
-        if (typeset.contains(Type.RRSIG)) newtypeset.add(Type.RRSIG);
-
-        if (!typeset.equals(newtypeset))
-        {
-          typeset = newtypeset;
-        }
       }
 
       switch (mDNSSECType)
@@ -316,7 +312,8 @@ public class ZoneVerifier
           errors += processNSEC3(n, typeset, ntype);
           break;
         case NSEC3_OPTOUT:
-          if (typeset.contains(Type.DS))
+          if (ntype == NodeType.NORMAL
+              || (ntype == NodeType.DELEGATION && typeset.contains(Type.DS)))
           {
             errors += processNSEC3(n, typeset, ntype);
           }
@@ -328,30 +325,59 @@ public class ZoneVerifier
     return errors;
   }
 
+  private static String reasonListToString(List reasons)
+  {
+    if (reasons == null) return "";
+    StringBuffer out = new StringBuffer();
+    for (Iterator i = reasons.iterator(); i.hasNext();)
+    {
+      out.append("Reason: ");
+      out.append((String) i.next());
+      if (i.hasNext()) out.append("\n");
+    }
+    return out.toString();
+  }
+
   private int processRRset(RRset rrset)
   {
-    // FIXME: use the slightly lower level verifySignature to get the list of reasons.
+    List reasons = new ArrayList();
-    int res = mVerifier.verify(rrset, null);
+    int result = DNSSEC.Failed;
+
+    for (Iterator i = rrset.sigs(); i.hasNext();)
+    {
+      RRSIGRecord sigrec = (RRSIGRecord) i.next();
+      byte res = mVerifier.verifySignature(rrset, sigrec, null, reasons);
+      if (res != DNSSEC.Secure)
+      {
+        log.warning("Signature failed to verify RRset:\n  rr:  "
+            + ZoneUtils.rrsetToString(rrset, false) + "\n  sig: " + sigrec + "\n"
+            + reasonListToString(reasons));
+      }
+
+      if (res > result) result = res;
+    }
 
     String rrsetname = rrset.getName() + "/" + Type.string(rrset.getType());
-    if (res == DNSSEC.Secure)
+    if (result == DNSSEC.Secure)
     {
-      System.out.println("RRset " + rrsetname + " verified.");
+      log.fine("RRset " + rrsetname + " verified.");
     }
     else
     {
-      System.out.println("RRset " + rrsetname + " did not verify.");
+      log.warning("RRset " + rrsetname + " did not verify.");
-    }
-    return res == DNSSEC.Secure ? 0 : 1;
     }
 
-  private String typesetToString(Set typeset)
+    return result == DNSSEC.Secure ? 0 : 1;
+  }
+
+  private String typesetToString(Set<Integer> typeset)
   {
+    if (typeset == null) return "";
+
     StringBuilder sb = new StringBuilder();
     boolean first = true;
-    for (Iterator i = typeset.iterator(); i.hasNext();)
+    for (int type : typeset)
     {
-      int type = ((Integer) i.next()).intValue();
       if (!first) sb.append(' ');
       sb.append(Type.string(type));
       first = false;
@@ -374,7 +400,7 @@ public class ZoneVerifier
     return sb.toString();
   }
 
-  private boolean checkTypeMap(Set typeset, int[] types)
+  private boolean checkTypeMap(Set<Integer> typeset, int[] types)
   {
     // a null typeset means that we are expecting the typemap of an ENT, which should be empty.
     if (typeset == null) return types.length == 0;
@@ -388,12 +414,12 @@ public class ZoneVerifier
     return typeset.equals(compareTypeset);
   }
 
-  private int processNSEC(Name n, Set typeset)
+  private int processNSEC(Name n, Set<Integer> typeset)
   {
     MarkRRset rrset = mNSECMap.get(n);
     if (n == null)
     {
-      System.out.println("Missing NSEC for " + n);
+      log.warning("Missing NSEC for " + n);
       return 1;
     }
 
@@ -406,7 +432,7 @@ public class ZoneVerifier
     // check typemap
     if (!checkTypeMap(typeset, nsec.getTypes()))
     {
-      System.out.println("Typemap for NSEC RR " + n
+      log.warning("Typemap for NSEC RR " + n
           + " did not match what was expected. Expected '" + typesetToString(typeset)
           + "', got '" + typesToString(nsec.getTypes()));
       errors++;
@@ -418,7 +444,7 @@ public class ZoneVerifier
     return errors;
   }
 
-  private boolean shouldCheckENTs(Name n, Set typeset, NodeType ntype)
+  private boolean shouldCheckENTs(Name n, Set<Integer> typeset, NodeType ntype)
   {
     // if we are just one (or zero) labels longer than the zonename, the node can't create a ENT
     if (n.labels() <= mZoneName.labels() + 1) return false;
@@ -439,22 +465,19 @@ public class ZoneVerifier
     return true;
   }
 
-  private int processNSEC3(Name n, Set typeset, NodeType ntype)
+  private int processNSEC3(Name n, Set<Integer> typeset, NodeType ntype)
       throws NoSuchAlgorithmException, TextParseException
   {
     // calculate the NSEC3 RR name
     byte[] hash = mNSEC3params.hashName(n);
-    if (mBase32 == null)
-    {
 
-    }
     String hashstr = mBase32.toString(hash);
     Name hashname = new Name(hashstr, mZoneName);
 
     MarkRRset rrset = mNSEC3Map.get(hashname);
     if (rrset == null)
     {
-      System.out.println("Missing NSEC3 for " + hashname + " corresponding to " + n);
+      log.warning("Missing NSEC3 for " + hashname + " corresponding to " + n);
       return 1;
     }
 
@@ -467,6 +490,9 @@ public class ZoneVerifier
     // check typemap
     if (!checkTypeMap(typeset, nsec3.getTypes()))
     {
+      log.warning("Typemap for NSEC3 RR " + hashname + " for " + n
+          + " did not match what was expected. Expected '" + typesetToString(typeset)
+          + "', got '" + typesToString(nsec3.getTypes()));
       errors++;
     }
 
@@ -478,7 +504,7 @@ public class ZoneVerifier
     if (shouldCheckENTs(n, typeset, ntype))
     {
       Name ent = new Name(n, 1);
-      if (!mNodeMap.containsKey(ent))
+      if (mNodeMap.get(ent) == null)
       {
         errors += processNSEC3(ent, null, NodeType.NORMAL);
       }
@@ -500,7 +526,7 @@ public class ZoneVerifier
       {
         if (lastNSEC.getName().compareTo(lastNSEC.getNext()) >= 0)
         {
-          System.out.println("NSEC for " + lastNSEC.getName()
+          log.warning("NSEC for " + lastNSEC.getName()
               + " has next name >= owner but is not the last NSEC in the chain.");
           errors++;
         }
@@ -513,7 +539,7 @@ public class ZoneVerifier
       // check to see if the NSEC is marked.  If not, it was not correlated to a signed node.
       if (!rrset.getMark())
       {
-        System.out.println("NSEC RR for " + n + " appears to be extra.");
+        log.warning("NSEC RR for " + n + " appears to be extra.");
         errors++;
       }
 
@@ -523,15 +549,14 @@ public class ZoneVerifier
       // nsec map incorrectly.
       if (!n.equals(nsec.getName()))
       {
-        System.out.println("The NSEC in the map for name " + n + " has name "
+        log.warning("The NSEC in the map for name " + n + " has name " + nsec.getName());
-            + nsec.getName());
         errors++;
       }
 
       // If this is the first row, ensure that the owner name equals the zone name
       if (lastNSEC == null && !n.equals(mZoneName))
       {
-        System.out.println("The first NSEC in the chain does not match the zone name: name = "
+        log.warning("The first NSEC in the chain does not match the zone name: name = "
             + n + " zonename = " + mZoneName);
         errors++;
       }
@@ -541,7 +566,7 @@ public class ZoneVerifier
       {
         if (!lastNSEC.getNext().equals(nsec.getName()))
         {
-          System.out.println("NSEC for " + lastNSEC.getName()
+          log.warning("NSEC for " + lastNSEC.getName()
               + " does not point to the next NSEC in the chain: " + n);
           errors++;
         }
@@ -554,7 +579,7 @@ public class ZoneVerifier
     // the ownername should be >= next name.
     if (lastNSEC.getName().compareTo(lastNSEC.getNext()) < 0)
     {
-      System.out.println("The last NSEC RR in the chain did not have an owner >= next: owner = "
+      log.warning("The last NSEC RR in the chain did not have an owner >= next: owner = "
           + lastNSEC.getName() + " next = " + lastNSEC.getNext());
       errors++;
     }
@@ -562,7 +587,7 @@ public class ZoneVerifier
     // check to make sure it links to the first NSEC in the chain
     if (!lastNSEC.getNext().equals(mZoneName))
     {
-      System.out.println("The last NSEC RR in the chain did not link to the first NSEC");
+      log.warning("The last NSEC RR in the chain did not link to the first NSEC");
       errors++;
     }
 
@@ -592,7 +617,7 @@ public class ZoneVerifier
       {
         if (compareNSEC3Hashes(lastNSEC3.getName(), lastNSEC3.getNext()) >= 0)
         {
-          System.out.println("NSEC3 for " + lastNSEC3.getName()
+          log.warning("NSEC3 for " + lastNSEC3.getName()
               + " has next name >= owner but is not the last NSEC3 in the chain.");
           errors++;
         }
@@ -605,34 +630,34 @@ public class ZoneVerifier
       // check to see if the NSEC is marked.  If not, it was not correlated to a signed node.
       if (!rrset.getMark())
       {
-        System.out.println("NSEC3 RR for " + n + " appears to be extra.");
+        log.warning("NSEC3 RR for " + n + " appears to be extra.");
         errors++;
       }
 
       NSEC3Record nsec3 = (NSEC3Record) rrset.first();
 
       // This is just a sanity check.  If this isn't true, we are constructing the
-      // nsec map incorrectly.
+      // nsec3 map incorrectly.
       if (!n.equals(nsec3.getName()))
       {
-        System.out.println("The NSEC3 in the map for name " + n + " has name "
+        log.severe("The NSEC3 in the map for name " + n + " has name " + nsec3.getName());
-            + nsec3.getName());
         errors++;
       }
 
-      // If this is the first row, ensure that the owner name equals the zone name
+      // note the first NSEC3 in the chain.
       if (lastNSEC3 == null)
       {
         firstNSEC3 = nsec3;
       }
-
+      else
-      // Check that the prior NSEC's next name equals this rows owner name.
+      // Check that the prior NSEC3's next hashed name equals this row's hashed owner name.
-      if (lastNSEC3 != null)
       {
-        if (compareNSEC3Hashes(nsec3.getName(), lastNSEC3.getNext()) == 0)
+        if (compareNSEC3Hashes(nsec3.getName(), lastNSEC3.getNext()) != 0)
         {
-          System.out.println("NSEC3 for " + lastNSEC3.getName()
+          String nextstr = mBase32.toString(lastNSEC3.getNext());
-              + " does not point to the next NSEC3 in the chain: " + n);
+          log.warning("NSEC3 for " + lastNSEC3.getName()
+              + " does not point to the next NSEC3 in the chain: " + nsec3.getName()
+              + ", instead points to: " + nextstr);
           errors++;
         }
       }
@@ -645,7 +670,7 @@ public class ZoneVerifier
     if (compareNSEC3Hashes(lastNSEC3.getName(), lastNSEC3.getNext()) < 0)
     {
       String nextstr = mBase32.toString(lastNSEC3.getNext());
-      System.out.println("The last NSEC3 RR in the chain did not have an owner >= next: owner = "
+      log.warning("The last NSEC3 RR in the chain did not have an owner >= next: owner = "
           + lastNSEC3.getName() + " next = " + nextstr);
       errors++;
     }
@@ -653,7 +678,7 @@ public class ZoneVerifier
     // check to make sure it links to the first NSEC in the chain
     if (compareNSEC3Hashes(firstNSEC3.getName(), lastNSEC3.getNext()) != 0)
     {
-      System.out.println("The last NSEC3 RR in the chain did not link to the first NSEC3");
+      log.warning("The last NSEC3 RR in the chain did not link to the first NSEC3");
       errors++;
     }
 
@@ -677,8 +702,14 @@ public class ZoneVerifier
       errors += processNSEC3Chain();
     }
 
-    System.out.println("Zone " + mZoneName + " verified with " + errors
+    if (errors > 0)
-        + ((errors == 1) ? " error" : " errors"));
+    {
+      log.info("Zone " + mZoneName + " failed verification with " + errors + " errors");
+    }
+    else
+    {
+      log.info("Zone " + mZoneName + " verified with 0 errors");
+    }
 
     return errors;
   }