Fix issue 14 (#15)

handle duplicate key tags, gen duplicate key tags, other minor cleanup

ChangeLog

@@ -1,3 +1,22 @@
+2024-03-25  David Blacka <davidb@verisign.com>
+
+        * Released version 0.19
+        * Handle duplicate key tags
+        * jdnssec-keygen can now attempt to generate keys with specified key tags
+
+2023-07-24  David Blacka  <davidb@verisign.com>
+
+        * Released version 0.17.1
+        * Add a `-t` option to jdnssec-verifyzone: verify using specified time.
+
+2022-09-21  David Blacka  <davidb@verisign.com>
+
+        * Released version 0.17
+        * Updated to dnsjava 3.5.1
+        * Formatting and linter suggestions
+        * Use slf4j instead of log4j.
+        * jdnssec-dstool can now generate CDS records
+
 2019-07-23  David Blacka  <davidb@verisign.com>
 
         * Released version 0.16

README.md

@@ -2,7 +2,7 @@
 
 * <https://github.com/dblacka/jdnssec-tools/wiki>
 
-Author: David Blacka (davidb@verisign.com)
+Author: David Blacka (<davidb@verisign.com>)
 
 This is a collection of DNSSEC tools written in Java.  They are intended to be an addition or replacement for the DNSSEC tools that are part of BIND 9.
 
@@ -45,4 +45,4 @@ The source for this project is available in git on github: <https://github.com/d
 
 ---
 
-Questions or comments may be directed to the author (mailto:davidb@verisign.com), or by creating issues in the [github issue tracker](https://github.com/dblacka/jdnssec-tools/issues).
+Questions or comments may be directed to the author (<mailto:davidb@verisign.com>), or by creating issues in the [github issue tracker](https://github.com/dblacka/jdnssec-tools/issues).

VERSION

@@ -1 +1 @@
-version=0.17
+version=0.19

build.gradle

@@ -10,7 +10,7 @@ apply plugin: 'idea'
 
 jar {
     baseName = 'jdnssec-tools'
-    version =  '0.17'
+    version =  '0.17.1'
 }
 
 repositories {

build.xml

@@ -47,8 +47,8 @@
            deprecation="true"
            includeantruntime="false"
            includes="com/verisignlabs/dnssec/"
-           source="8"
+           source="17"
-           target="8" />
+           target="17" />
   </target>
 
   <target name="sectools-jar" depends="usage,sectools">

src/main/java/com/verisignlabs/dnssec/cl/CLBase.java

@@ -101,8 +101,8 @@ public abstract class CLBase {
       opts.addOption("m", "multiline", false,
           "Output DNS records using 'multiline' format");
 
-      opts.addOption(Option.builder("v").longOpt("verbose").argName("level").optionalArg(true).desc(
+      opts.addOption(Option.builder("v").longOpt("verbose").argName("level").hasArg().desc(
-          "verbosity level -- 0 is silence, 3 is info, 5 is debug information, 6 is trace information. default is level 2 (warning)")
+          "verbosity level -- 0: silence, 1: error, 2: warning, 3: info, 4/5: fine, 6: finest; default: 2 (warning)")
           .build());
 
       opts.addOption(Option.builder("A").hasArg().argName("alias:original:mnemonic").longOpt("alg-alias")

src/main/java/com/verisignlabs/dnssec/cl/KeyGen.java

@@ -53,6 +53,7 @@ public class KeyGen extends CLBase {
     public boolean kskFlag = false;
     public String owner = null;
     public long ttl = 86400;
+    public int givenKeyTag = -1;
 
     public CLIState() {
       super("jdnssec-keygen [..options..] name");
@@ -87,6 +88,8 @@ public class KeyGen extends CLBase {
           .desc("generated keyfiles are written to this directory").build());
       opts.addOption(Option.builder("T").hasArg().argName("ttl").longOpt("ttl")
           .desc("use this TTL for the generated DNSKEY records (default: 86400").build());
+      opts.addOption(Option.builder().hasArg().argName("tag").longOpt("with-tag")
+          .desc("Generate keys until tag is the given value.").build());
 
     }
 
@@ -133,6 +136,10 @@ public class KeyGen extends CLBase {
         ttl = parseInt(optstr, 86400);
       }
 
+      if ((optstr = cli.getOptionValue("with-tag")) != null) {
+        givenKeyTag = parseInt(optstr, -1);
+      }
+
       String[] args = cli.getArgs();
 
       if (args.length < 1) {
@@ -169,11 +176,12 @@ public class KeyGen extends CLBase {
 
     // Calculate our flags
     int flags = 0;
-    if (state.zoneKey)
+    if (state.zoneKey) {
       flags |= DNSKEYRecord.Flags.ZONE_KEY;
-    if (state.kskFlag)
+    }
+    if (state.kskFlag) {
       flags |= DNSKEYRecord.Flags.SEP_KEY;
-
+    }
     log.fine("create key pair with (name = " + ownerName + ", ttl = " + state.ttl
         + ", alg = " + state.algorithm + ", flags = " + flags + ", length = "
         + state.keylength + ")");
@@ -182,6 +190,12 @@ public class KeyGen extends CLBase {
         state.algorithm, flags, state.keylength,
         state.useLargeE);
 
+    // If we were asked to generate a duplicate keytag, keep trying until we get one
+    while (state.givenKeyTag >= 0 && pair.getDNSKEYFootprint() != state.givenKeyTag) {
+      pair = signer.generateKey(ownerName, state.ttl, DClass.IN, state.algorithm, flags, state.keylength,
+          state.useLargeE);
+    }
+
     if (state.outputfile != null) {
       BINDKeyUtils.writeKeyFiles(state.outputfile, pair, state.keydir);
     } else {

src/main/java/com/verisignlabs/dnssec/cl/SignKeyset.java

@@ -22,7 +22,6 @@ import java.io.FileFilter;
 import java.io.IOException;
 import java.time.Instant;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 
 import org.apache.commons.cli.CommandLine;
@@ -184,11 +183,11 @@ public class SignKeyset extends CLBase {
   private static List<DnsKeyPair> getKeys(String[] keyfiles, int startIndex,
       File inDirectory) throws IOException {
     if (keyfiles == null)
-      return Collections.emptyList();
+      return new ArrayList<>();
 
     int len = keyfiles.length - startIndex;
     if (len <= 0)
-      return Collections.emptyList();
+      return new ArrayList<>();
 
     ArrayList<DnsKeyPair> keys = new ArrayList<>(len);
 

src/main/java/com/verisignlabs/dnssec/cl/SignRRset.java

@@ -21,7 +21,6 @@ import java.io.File;
 import java.io.IOException;
 import java.time.Instant;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 
 import org.apache.commons.cli.CommandLine;
@@ -191,11 +190,11 @@ public class SignRRset extends CLBase {
   private static List<DnsKeyPair> getKeys(String[] keyfiles, int startIndex,
       File inDirectory) throws IOException {
     if (keyfiles == null)
-      return Collections.emptyList();
+      return new ArrayList<>();
 
     int len = keyfiles.length - startIndex;
     if (len <= 0)
-      return Collections.emptyList();
+      return new ArrayList<>();
 
     ArrayList<DnsKeyPair> keys = new ArrayList<>(len);
 

src/main/java/com/verisignlabs/dnssec/cl/SignZone.java

@@ -24,7 +24,6 @@ import java.io.FileReader;
 import java.io.IOException;
 import java.time.Instant;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Random;
@@ -331,11 +330,11 @@ public class SignZone extends CLBase {
   private static List<DnsKeyPair> getKeys(String[] keyfiles, int startIndex,
       File inDirectory) throws IOException {
     if (keyfiles == null)
-      return Collections.emptyList();
+      return new ArrayList<>();
 
     int len = keyfiles.length - startIndex;
     if (len <= 0)
-      return Collections.emptyList();
+      return new ArrayList<>();
 
     ArrayList<DnsKeyPair> keys = new ArrayList<>(len);
 

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

@@ -17,11 +17,13 @@
 
 package com.verisignlabs.dnssec.cl;
 
+import java.time.Instant;
 import java.util.List;
 
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.Options;
 import org.apache.commons.cli.Option;
+import org.apache.commons.cli.ParseException;
 import org.xbill.DNS.Record;
 
 import com.verisignlabs.dnssec.security.ZoneUtils;
@@ -47,6 +49,7 @@ public class VerifyZone extends CLBase {
     public int expirefudge = 0;
     public boolean ignoreTime = false;
     public boolean ignoreDups = false;
+    public Instant currentTime = null;
 
     public CLIState() {
       super("jdnssec-verifyzone [..options..] zonefile");
@@ -54,10 +57,13 @@ public class VerifyZone extends CLBase {
 
     @Override
     protected void setupOptions(Options opts) {
-      opts.addOption(Option.builder("S").optionalArg(true).argName("seconds").longOpt("sig-start-fudge")
+      opts.addOption(Option.builder("S").hasArg().argName("seconds").longOpt("sig-start-fudge")
           .desc("'fudge' RRSIG inception ties by 'seconds'").build());
-      opts.addOption(Option.builder("E").optionalArg(true).argName("seconds").longOpt("sig-expire-fudge")
+      opts.addOption(Option.builder("E").hasArg().argName("seconds").longOpt("sig-expire-fudge")
           .desc("'fudge' RRSIG expiration times by 'seconds'").build());
+      opts.addOption(Option.builder("t").hasArg().argName("time").longOpt("use-time")
+          .desc("Use 'time' as the time for verification purposes.").build());
+
       opts.addOption(
           Option.builder().longOpt("ignore-time").desc("Ignore RRSIG inception and expiration time errors.").build());
       opts.addOption(Option.builder().longOpt("ignore-duplicate-rrs").desc("Ignore duplicate record errors.").build());
@@ -82,6 +88,15 @@ public class VerifyZone extends CLBase {
         expirefudge = parseInt(optstr, 0);
       }
 
+      if ((optstr = cli.getOptionValue('t')) != null) {
+        try {
+          currentTime = convertDuration(null, optstr);
+        } catch (ParseException e) {
+          System.err.println("error: could not parse timespec");
+          usage();
+        }
+      }
+
       String[] optstrs = null;
       if ((optstrs = cli.getOptionValues('A')) != null) {
         for (int i = 0; i < optstrs.length; i++) {
@@ -110,6 +125,7 @@ public class VerifyZone extends CLBase {
     zoneverifier.getVerifier().setStartFudge(state.startfudge);
     zoneverifier.getVerifier().setExpireFudge(state.expirefudge);
     zoneverifier.getVerifier().setIgnoreTime(state.ignoreTime);
+    zoneverifier.getVerifier().setCurrentTime(state.currentTime);
     zoneverifier.setIgnoreDuplicateRRs(state.ignoreDups);
 
     List<Record> records = ZoneUtils.readZoneFile(state.zonefile, null);

src/main/java/com/verisignlabs/dnssec/security/DnsKeyAlgorithm.java

@@ -147,19 +147,18 @@ public class DnsKeyAlgorithm {
     // This is so we can use this provider if it is available, but not require
     // the user to add it as one of the java.security providers.
     try {
-      Class<?> bc_provider_class = Class.forName("org.bouncycastle.jce.provider.BouncyCastleProvider");
+      Class<?> bcProviderClass = Class.forName("org.bouncycastle.jce.provider.BouncyCastleProvider");
-      // Provider bc_provider = (Provider) bc_provider_class.newInstance();
+      Provider bcProvider = (Provider) bcProviderClass.getDeclaredConstructor().newInstance();
-      Provider bc_provider = (Provider) bc_provider_class.getDeclaredConstructor().newInstance();
+      Security.addProvider(bcProvider);
-      Security.addProvider(bc_provider);
     } catch (ReflectiveOperationException e) {
+      log.info("Unable to load BC provider");
     }
 
     // Attempt to add the EdDSA-Java provider.
     try {
-      Class<?> eddsa_provider_class = Class.forName("net.i2p.crypto.eddsa.EdDSASecurityProvider");
+      Class<?> eddsaProviderClass = Class.forName("net.i2p.crypto.eddsa.EdDSASecurityProvider");
-      // Provider eddsa_provider = (Provider) eddsa_provider_class.newInstance();
+      Provider eddsaProvider = (Provider) eddsaProviderClass.getDeclaredConstructor().newInstance();
-      Provider eddsa_provider = (Provider) eddsa_provider_class.getDeclaredConstructor().newInstance();
+      Security.addProvider(eddsaProvider);
-      Security.addProvider(eddsa_provider);
     } catch (ReflectiveOperationException e) {
       log.warning("Unable to load EdDSA provider");
     }
@@ -304,8 +303,7 @@ public class DnsKeyAlgorithm {
   // name, we can construct the parameters here. For now, we only do this for
   // the ECC-GOST curve.
   private ECParameterSpec ECSpecFromAlgorithm(int algorithm) {
-    switch (algorithm) {
+    if (algorithm == DNSSEC.Algorithm.ECC_GOST) {
-      case DNSSEC.Algorithm.ECC_GOST: {
         // From RFC 4357 Section 11.4
         BigInteger p = new BigInteger("FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFD97", 16);
         BigInteger a = new BigInteger("FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFD94", 16);
@@ -316,10 +314,8 @@ public class DnsKeyAlgorithm {
 
         EllipticCurve curve = new EllipticCurve(new ECFieldFp(p), a, b);
         return new ECParameterSpec(curve, new ECPoint(gx, gy), n, 1);
-      }
-      default:
-        return null;
     }
+    return null;
   }
 
   // Fetch the curve parameters from a named ECDSA curve.
@@ -455,9 +451,7 @@ public class DnsKeyAlgorithm {
    *         alias.
    */
   public boolean supportedAlgorithm(int algorithm) {
-    if (mAlgorithmMap.containsKey(algorithm))
+    return mAlgorithmMap.containsKey(algorithm);
-      return true;
-    return false;
   }
 
   /**

src/main/java/com/verisignlabs/dnssec/security/DnsKeyPair.java

@@ -16,6 +16,7 @@
 
 package com.verisignlabs.dnssec.security;
 
+import java.nio.charset.StandardCharsets;
 import java.security.InvalidKeyException;
 import java.security.NoSuchAlgorithmException;
 import java.security.PrivateKey;
@@ -127,6 +128,12 @@ public class DnsKeyPair {
     setPrivateKeyString(pair.getPrivateKeyString());
   }
 
+  public String toString() {
+
+    return this.getDNSKEYName() + "/" + this.getDNSKEYAlgorithm() + "/" + this.getDNSKEYFootprint() + "/"
+        + this.getDNSKEYPublicPrefix(6);
+  }
+
   /** @return cached DnsKeyConverter object. */
   protected DnsKeyConverter getKeyConverter() {
     if (mKeyConverter == null) {
@@ -319,4 +326,26 @@ public class DnsKeyPair {
       return kr.getFootprint();
     return -1;
   }
+
+  // This is from a StackOverflow answer.  There are number of bytes-to-hex
+  // converters in the ecosystem, but this avoid extra dependencies
+  private static final byte[] HEX_ARRAY = "0123456789ABCDEF".getBytes(StandardCharsets.US_ASCII);
+  public static String toHex(byte[] bytes) {
+    byte[] hexChars = new byte[bytes.length * 2];
+    for (int j = 0; j < bytes.length; j++) {
+        int v = bytes[j] & 0xFF;
+        hexChars[j * 2] = HEX_ARRAY[v >>> 4];
+        hexChars[j * 2 + 1] = HEX_ARRAY[v & 0x0F];
+    }
+    return new String(hexChars, StandardCharsets.UTF_8);
+  }
+
+  public String getDNSKEYPublicPrefix(int length) {
+    DNSKEYRecord kr = getDNSKEYRecord();
+    if (kr == null) {
+      return "";
+    }
+    String hexKey = toHex(kr.getKey());
+    return hexKey.substring(0, length);
+  }
 }

src/main/java/com/verisignlabs/dnssec/security/DnsSecVerifier.java

@@ -70,20 +70,19 @@ public class DnsSecVerifier {
       add(pair);
     }
 
-    public DnsKeyPair find(Name name, int algorithm, int keyid) {
+    public List<DnsKeyPair> find(Name name, int algorithm, int keyid) {
       String n = name.toString().toLowerCase();
       List<DnsKeyPair> l = mKeyMap.get(n);
+      List<DnsKeyPair> result = new ArrayList<>();
       if (l == null)
-        return null;
+        return result;
 
-      // FIXME: this algorithm assumes that name+alg+footprint is
-      // unique, which isn't necessarily true.
       for (DnsKeyPair p : l) {
         if (p.getDNSKEYAlgorithm() == algorithm && p.getDNSKEYFootprint() == keyid) {
-          return p;
+          result.add(p);
         }
       }
-      return null;
+      return result;
     }
   }
 
@@ -92,6 +91,7 @@ public class DnsSecVerifier {
   private int mExpireFudge = 0;
   private boolean mVerifyAllSigs = false;
   private boolean mIgnoreTime = false;
+  private Instant mCurrentTime = null;
 
   private Logger log;
 
@@ -133,7 +133,11 @@ public class DnsSecVerifier {
     mIgnoreTime = v;
   }
 
-  private DnsKeyPair findKey(Name name, int algorithm, int footprint) {
+  public void setCurrentTime(Instant time) {
+    mCurrentTime = time;
+  }
+
+  private List<DnsKeyPair> findKey(Name name, int algorithm, int footprint) {
     return mKeyStore.find(name, algorithm, footprint);
   }
 
@@ -155,7 +159,13 @@ public class DnsSecVerifier {
     if (mIgnoreTime)
       return true;
 
-    Instant now = Instant.now();
+    Instant now;
+    if (mCurrentTime != null) {
+      now = mCurrentTime;
+    } else {
+      now = Instant.now();
+    }
+
     Instant start = sigrec.getTimeSigned();
     Instant expire = sigrec.getExpire();
 
@@ -208,43 +218,51 @@ public class DnsSecVerifier {
     if (!result)
       return result;
 
-    DnsKeyPair keypair = findKey(sigrec.getSigner(), sigrec.getAlgorithm(),
+    List<DnsKeyPair> keypairs = findKey(sigrec.getSigner(), sigrec.getAlgorithm(),
         sigrec.getFootprint());
 
-    if (keypair == null) {
+    if (keypairs.isEmpty()) {
       if (reasons != null)
         reasons.add("Could not find matching trusted key");
       log.fine("could not find matching trusted key");
       return false;
     }
-
     try {
       byte[] data = SignUtils.generateSigData(rrset, sigrec);
 
       DnsKeyAlgorithm algs = DnsKeyAlgorithm.getInstance();
 
-      Signature signer = keypair.getVerifier();
+      // Tolerate duplicate keytags, so we can have more than one DnsKeyPair
-      signer.update(data);
+      List<String> localReasons = new ArrayList<>();
-
+      boolean validated = false;
-      byte[] sig = sigrec.getSignature();
+      for (DnsKeyPair keypair : keypairs) {
-
+        Signature signer = keypair.getVerifier();
-      if (algs.baseType(sigrec.getAlgorithm()) == DnsKeyAlgorithm.DSA) {
+        signer.update(data);
-        sig = SignUtils.convertDSASignature(sig);
+  
+        byte[] sig = sigrec.getSignature();
+  
+        if (algs.baseType(sigrec.getAlgorithm()) == DnsKeyAlgorithm.DSA) {
+          sig = SignUtils.convertDSASignature(sig);
+        }
+  
+        if (sigrec.getAlgorithm() == DNSSEC.Algorithm.ECDSAP256SHA256 ||
+            sigrec.getAlgorithm() == DNSSEC.Algorithm.ECDSAP384SHA384) {
+          sig = SignUtils.convertECDSASignature(sig);
+        }
+        if (signer.verify(sig)) {
+          validated = true;
+          break;
+        }
+        log.fine("Signature failed to validate cryptographically with " + keypair);
+        if (localReasons != null) {
+          localReasons.add("Signature failed to verify cryptographically with " + keypair);
+        }
       }
 
-      if (sigrec.getAlgorithm() == DNSSEC.Algorithm.ECDSAP256SHA256 ||
+      if (!validated) {
-          sigrec.getAlgorithm() == DNSSEC.Algorithm.ECDSAP384SHA384) {
+        reasons.addAll(localReasons);
-        sig = SignUtils.convertECDSASignature(sig);
       }
-
+      return validated;
-      if (!signer.verify(sig)) {
-        if (reasons != null)
-          reasons.add("Signature failed to verify cryptographically");
-        log.fine("Signature failed to verify cryptographically");
-        return false;
-      }
-
-      return true;
     } catch (IOException e) {
       log.severe("I/O error: " + e);
     } catch (GeneralSecurityException e) {
@@ -262,7 +280,7 @@ public class DnsSecVerifier {
    * @return true if the set verified, false if it did not.
    */
   public boolean verify(RRset rrset) {
-    boolean result = mVerifyAllSigs ? true : false;
+    boolean result = mVerifyAllSigs;
 
     if (rrset.sigs().isEmpty()) {
       log.fine("RRset failed to verify due to lack of signatures");

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

@@ -84,6 +84,14 @@ public class ZoneVerifier {
     private static final long serialVersionUID = 1L;
     private boolean mIsMarked = false;
 
+    @Override
+    public boolean equals(Object o) {
+      return super.equals(o);
+    }
+    @Override
+    public int hashCode() {
+      return super.hashCode();
+    }
     boolean getMark() {
       return mIsMarked;
     }
@@ -140,7 +148,7 @@ public class ZoneVerifier {
   /**
    * Add a record to the various maps.
    *
-   * @return TODO
+   * @return true if the RR was added, false if it wasn't (because it was a duplicate)
    */
   private boolean addRR(Record r) {
     Name n = r.getName();
@@ -161,7 +169,7 @@ public class ZoneVerifier {
       if (mNSEC3Map == null) {
         mNSEC3Map = new TreeMap<>();
       }
-      MarkRRset rrset = mNSECMap.computeIfAbsent(n, k -> new MarkRRset());
+      MarkRRset rrset = mNSEC3Map.computeIfAbsent(n, k -> new MarkRRset());
 
       return addRRtoRRset(rrset, r);
     }
@@ -199,7 +207,7 @@ public class ZoneVerifier {
    * determine the NSEC3 parameters and signing type.
    *
    * @param records
-   * @return TODO
+   * @return the number of errors encountered.
    */
   private int calculateNodes(List<Record> records) {
     mNodeMap = new TreeMap<>();
@@ -326,6 +334,8 @@ public class ZoneVerifier {
       }
 
       switch (mDNSSECType) {
+        case UNSIGNED:
+          throw new IllegalArgumentException("Cannot process Unsigned zone");
         case NSEC:
           // all nodes with NSEC records have NSEC and RRSIG types
           typeset.add(Type.NSEC);