Support other algorithms for directory signatures than sha1.

 - Support more than one "directory-signature" line in a vote, which
   may become relevant when authorities start signing votes using more
   than one algorithm.
 - Provide directory signatures in consensuses and votes in a list
   rather than a map to support multiple signatures made using the
   same identity key digest but different algorithms.
 - Be more lenient about digest lengths in directory signatures which
   may be longer or shorter than 20 bytes.

Implements #18875.

While implementing this, make "sha1" constant and deprecate
RelayNetworkStatusVote.getSigningKeyDigest(), because it's remissible
and ambiguous.  Suggested or based on discussions with iwakeh.

CHANGELOG.md

@@ -2,6 +2,14 @@
 
  * Medium changes
    - Parse "package" lines in consensuses and votes.
+   - Support more than one "directory-signature" line in a vote, which
+     may become relevant when authorities start signing votes using
+     more than one algorithm.
+   - Provide directory signatures in consensuses and votes in a list
+     rather than a map to support multiple signatures made using the
+     same identity key digest but different algorithms.
+   - Be more lenient about digest lengths in directory signatures
+     which may be longer or shorter than 20 bytes.
 
 
 # Changes in version 1.2.0 - 2016-05-31

src/org/torproject/descriptor/RelayNetworkStatusConsensus.java

@@ -186,10 +186,21 @@ public interface RelayNetworkStatusConsensus extends Descriptor {
    * SHA-1 digests of the authorities' identity keys in the version 3
    * directory protocol, encoded as 40 upper-case hexadecimal characters.
    *
+   * @deprecated Replaced by {@link #getSignatures()} which permits an
+   * arbitrary number of signatures made by an authority using the same
+   * identity key digest and different algorithms.
+   *
    * @since 1.0.0
    */
   public SortedMap<String, DirectorySignature> getDirectorySignatures();
 
+  /**
+   * Return the list of signatures contained in this consensus.
+   *
+   * @since 1.3.0
+   */
+  public List<DirectorySignature> getSignatures();
+
   /**
    * Return optional weights to be applied to router bandwidths during
    * path selection with map keys being case-sensitive weight identifiers

src/org/torproject/descriptor/RelayNetworkStatusVote.java

@@ -325,6 +325,12 @@ public interface RelayNetworkStatusVote extends Descriptor {
    * 40 upper-case hexadecimal characters, or null if this digest cannot
    * be obtained from the directory signature.
    *
+   * @deprecated Removed in order to be more explicit that authorities may
+   *     use different digest algorithms than "sha1"; see
+   *     {@link #getSignatures()} and
+   *     {@link DirectorySignature#getSigningKeyDigest()} for
+   *     alternatives.
+   *
    * @since 1.0.0
    */
   public String getSigningKeyDigest();
@@ -382,8 +388,21 @@ public interface RelayNetworkStatusVote extends Descriptor {
    * 3 directory protocol, encoded as 40 upper-case hexadecimal
    * characters.
    *
+   * @deprecated Replaced by {@link #getSignatures()} which permits an
+   * arbitrary number of signatures made by the authority using the same
+   * identity key digest and different algorithms.
+   *
    * @since 1.0.0
    */
   public SortedMap<String, DirectorySignature> getDirectorySignatures();
+
+  /**
+   * Return a list of signatures contained in this vote, which is
+   * typically a single signature made by the authority but which may also
+   * be more than one signature made with different keys or algorithms.
+   *
+   * @since 1.3.0
+   */
+  public List<DirectorySignature> getSignatures();
 }
 

src/org/torproject/descriptor/impl/DirectorySignatureImpl.java

@@ -53,9 +53,9 @@ public class DirectorySignatureImpl implements DirectorySignature {
           throw new DescriptorParseException("Illegal line '" + line
               + "'.");
         }
-        this.identity = ParseHelper.parseTwentyByteHexString(line,
+        this.identity = ParseHelper.parseHexString(line,
             parts[1 + algorithmOffset]);
-        this.signingKeyDigest = ParseHelper.parseTwentyByteHexString(
+        this.signingKeyDigest = ParseHelper.parseHexString(
             line, parts[2 + algorithmOffset]);
         break;
       case "-----BEGIN":
@@ -86,10 +86,12 @@ public class DirectorySignatureImpl implements DirectorySignature {
     }
   }
 
-  private String algorithm = "sha1";
+  static final String DEFAULT_ALGORITHM = "sha1";
+
+  private String algorithm;
   @Override
   public String getAlgorithm() {
-    return this.algorithm;
+    return this.algorithm == null ? DEFAULT_ALGORITHM : this.algorithm;
   }
 
   private String identity;

src/org/torproject/descriptor/impl/NetworkStatusImpl.java

@@ -217,12 +217,12 @@ public abstract class NetworkStatusImpl extends DescriptorImpl {
 
   protected void parseDirectorySignature(byte[] directorySignatureBytes)
       throws DescriptorParseException {
-    if (this.directorySignatures == null) {
-      this.directorySignatures = new TreeMap<>();
+    if (this.signatures == null) {
+      this.signatures = new ArrayList<>();
     }
     DirectorySignatureImpl signature = new DirectorySignatureImpl(
         directorySignatureBytes, failUnrecognizedDescriptorLines);
-    this.directorySignatures.put(signature.getIdentity(), signature);
+    this.signatures.add(signature);
     List<String> unrecognizedStatusEntryLines = signature.
         getAndClearUnrecognizedLines();
     if (unrecognizedStatusEntryLines != null) {
@@ -251,10 +251,20 @@ public abstract class NetworkStatusImpl extends DescriptorImpl {
     return this.statusEntries.get(fingerprint);
   }
 
-  protected SortedMap<String, DirectorySignature> directorySignatures;
+  protected List<DirectorySignature> signatures;
+  public List<DirectorySignature> getSignatures() {
+    return this.signatures == null ? null
+        : new ArrayList<>(this.signatures);
+  }
   public SortedMap<String, DirectorySignature> getDirectorySignatures() {
-    return this.directorySignatures == null ? null :
-        new TreeMap<>(this.directorySignatures);
+    SortedMap<String, DirectorySignature> directorySignatures = null;
+    if (this.signatures != null) {
+      directorySignatures = new TreeMap<>();
+      for (DirectorySignature signature : this.signatures) {
+        directorySignatures.put(signature.getIdentity(), signature);
+      }
+    }
+    return directorySignatures;
   }
 }
 

src/org/torproject/descriptor/impl/ParseHelper.java

@@ -207,11 +207,22 @@ public class ParseHelper {
     return result;
   }
 
-  private static Pattern twentyByteHexPattern =
-      Pattern.compile("^[0-9a-fA-F]{40}$");
   protected static String parseTwentyByteHexString(String line,
       String hexString) throws DescriptorParseException {
-    if (!twentyByteHexPattern.matcher(hexString).matches()) {
+    return parseHexString(line, hexString, 40);
+  }
+
+  protected static String parseHexString(String line, String hexString)
+      throws DescriptorParseException {
+    return parseHexString(line, hexString, -1);
+  }
+
+  private static Pattern hexPattern = Pattern.compile("^[0-9a-fA-F]*$");
+  private static String parseHexString(String line, String hexString,
+      int expectedLength) throws DescriptorParseException {
+    if (!hexPattern.matcher(hexString).matches() ||
+        hexString.length() % 2 != 0 ||
+        (expectedLength >= 0 && hexString.length() != expectedLength)) {
       throw new DescriptorParseException("Illegal hex string in line '"
           + line + "'.");
     }

src/org/torproject/descriptor/impl/RelayNetworkStatusVoteImpl.java

@@ -3,6 +3,7 @@
 package org.torproject.descriptor.impl;
 
 import org.torproject.descriptor.DescriptorParseException;
+import org.torproject.descriptor.DirectorySignature;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -47,7 +48,7 @@ public class RelayNetworkStatusVoteImpl extends NetworkStatusImpl
         + "valid-until,voting-delay,known-flags,dir-source,"
         + "dir-key-certificate-version,fingerprint,dir-key-published,"
         + "dir-key-expires,dir-identity-key,dir-signing-key,"
-        + "dir-key-certification,directory-signature").split(",")));
+        + "dir-key-certification").split(",")));
     this.checkExactlyOnceKeywords(exactlyOnceKeywords);
     Set<String> atMostOnceKeywords = new HashSet<>(Arrays.asList((
         "consensus-methods,client-versions,server-versions,"
@@ -55,6 +56,9 @@ public class RelayNetworkStatusVoteImpl extends NetworkStatusImpl
         + "legacy-key,dir-key-crosscert,dir-address,directory-footer").
         split(",")));
     this.checkAtMostOnceKeywords(atMostOnceKeywords);
+    Set<String> atLeastOnceKeywords = new HashSet<>(Arrays.asList(
+        "directory-signature"));
+    this.checkAtLeastOnceKeywords(atLeastOnceKeywords);
     this.checkFirstKeyword("network-status-version");
     this.clearParsedKeywords();
   }
@@ -605,9 +609,14 @@ public class RelayNetworkStatusVoteImpl extends NetworkStatusImpl
   @Override
   public String getSigningKeyDigest() {
     String signingKeyDigest = null;
-    if (!this.directorySignatures.isEmpty()) {
-      signingKeyDigest = this.directorySignatures.get(
-          this.directorySignatures.firstKey()).getSigningKeyDigest();
+    if (this.signatures != null && !this.signatures.isEmpty()) {
+      for (DirectorySignature signature : this.signatures) {
+        if (DirectorySignatureImpl.DEFAULT_ALGORITHM.equals(
+            signature.getAlgorithm())) {
+          signingKeyDigest = signature.getSigningKeyDigest();
+          break;
+        }
+      }
     }
     return signingKeyDigest;
   }

test/org/torproject/descriptor/impl/RelayNetworkStatusConsensusImplTest.java

@@ -3,6 +3,8 @@
 package org.torproject.descriptor.impl;
 
 import org.torproject.descriptor.DescriptorParseException;
+import org.torproject.descriptor.DirectorySignature;
+
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
@@ -308,10 +310,13 @@ public class RelayNetworkStatusConsensusImplTest {
         "00795A6E8D91C270FC23B30F388A495553E01894"));
     assertEquals("188.177.149.216", consensus.getStatusEntry(
         "00795A6E8D91C270FC23B30F388A495553E01894").getAddress());
-    assertEquals("3509BA5A624403A905C74DA5C8A0CEC9E0D3AF86",
-        consensus.getDirectorySignatures().get(
-        "14C131DFC5C6F93646BE72FA1401C02A8DF2E8B4").
-        getSigningKeyDigest());
+    for (DirectorySignature signature : consensus.getSignatures()) {
+      if ("14C131DFC5C6F93646BE72FA1401C02A8DF2E8B4".equals(
+          signature.getIdentity())) {
+        assertEquals("3509BA5A624403A905C74DA5C8A0CEC9E0D3AF86",
+            signature.getSigningKeyDigest());
+      }
+    }
     assertEquals(285, (int) consensus.getBandwidthWeights().get("Wbd"));
     assertTrue(consensus.getUnrecognizedLines().isEmpty());
   }
@@ -1114,22 +1119,38 @@ public class RelayNetworkStatusConsensusImplTest {
     DirectorySignatureBuilder.createWithIdentity("ED03BB616EB2F60");
   }
 
-  @Test(expected = DescriptorParseException.class)
+  @Test()
   public void testDirectorySignatureIdentityTooLong()
       throws DescriptorParseException {
+    /* This hex string has an unusual length of 58 hex characters, but
+     * dir-spec.txt only requires a hex string, and we can't know all hex
+     * string lengths for all future digest algorithms, so let's just
+     * accept this. */
     DirectorySignatureBuilder.createWithIdentity(
         "ED03BB616EB2F60BEC80151114BB25CEF515B226ED03BB616EB2F60BEC");
   }
 
-  @Test(expected = DescriptorParseException.class)
+  @Test()
   public void testDirectorySignatureSigningKeyTooShort()
       throws DescriptorParseException {
-    DirectorySignatureBuilder.createWithSigningKey("845CF1D0B370CA4");
+    /* See above, we accept this hex string even though it's unusually
+     * short. */
+    DirectorySignatureBuilder.createWithSigningKey("845CF1D0B370CA");
   }
 
   @Test(expected = DescriptorParseException.class)
+  public void testDirectorySignatureSigningKeyTooShortOddNumber()
+      throws DescriptorParseException {
+    /* We don't accept this hex string, because it contains an odd number
+     * of hex characters. */
+    DirectorySignatureBuilder.createWithSigningKey("845");
+  }
+
+  @Test()
   public void testDirectorySignatureSigningKeyTooLong()
       throws DescriptorParseException {
+    /* See above, we accept this hex string even though it's unusually
+     * long. */
     DirectorySignatureBuilder.createWithSigningKey(
         "845CF1D0B370CA443A8579D18E7987E7E532F639845CF1D0B370CA443A");
   }

test/org/torproject/descriptor/impl/RelayNetworkStatusVoteImplTest.java

@@ -3,6 +3,8 @@
 package org.torproject.descriptor.impl;
 
 import org.torproject.descriptor.DescriptorParseException;
+import org.torproject.descriptor.DirectorySignature;
+
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
@@ -532,6 +534,18 @@ public class RelayNetworkStatusVoteImplTest {
         vote.getDirKeyCrosscert().split("\n")[0]);
     assertEquals("-----BEGIN SIGNATURE-----",
         vote.getDirKeyCertification().split("\n")[0]);
+    assertEquals(1, vote.getSignatures().size());
+    DirectorySignature signature = vote.getSignatures().get(0);
+    assertEquals("sha1", signature.getAlgorithm());
+    assertEquals("80550987E1D626E3EBA5E5E75A458DE0626D088C",
+        signature.getIdentity());
+    assertEquals("EEB9299D295C1C815E289FBF2F2BBEA5F52FDD19",
+        signature.getSigningKeyDigest());
+    assertEquals("-----BEGIN SIGNATURE-----\n"
+        + "iHEU3Iidya5RIrjyYgv8tlU0R+rF56/3/MmaaZi0a67e7ZkISfQ4dghScHxn"
+        + "F3Yh\nrXVaaoP07r6Ta+s0g1Zijm3lms50Nk/4tV2p8Y63c3F4Q3DAnK40Oi"
+        + "kfOIwEj+Ny\n+zBRQssP3hPhTPOj/A7o3mZZwtL6x1sxpeu/nME1l5E=\n"
+        + "-----END SIGNATURE-----\n", signature.getSignature());
     assertTrue(vote.getUnrecognizedLines().isEmpty());
   }
 
@@ -1204,6 +1218,61 @@ public class RelayNetworkStatusVoteImplTest {
     VoteBuilder.createWithDirectorySignatureLines(null);
   }
 
+  @Test()
+  public void testDirectorySignaturesLinesTwoAlgorithms()
+      throws DescriptorParseException {
+    String identitySha256 = "32519E5CB7254AB5A94CC9925EC7676E53D5D52EEAB7"
+        + "914BD3ED751E537CAFCC";
+    String signingKeyDigestSha256 = "5A59D99C17831B9254422B6C5AA10CC59381"
+        + "6CAA5241E22ECAE8BBB4E8E9D1FC";
+    String signatureSha256 = "-----BEGIN SIGNATURE-----\n"
+        + "x57Alc424/zHS73SHokghGtNBVrBjtUz+gSL5w9AHGKUQcMyfw4Z9aDlKpTbFc"
+        + "5W\nnyIvFmM9C2OAH0S1+a647HHIxhE0zKf4+yKSwzqSyL6sbKQygVlJsRHNRr"
+        + "cFg8lp\nqBxEwvxQoA4xEDqnerR92pbK9l42nNLiKOcoReUqbbQ=\n"
+        + "-----END SIGNATURE-----";
+    String identitySha1 = "80550987E1D626E3EBA5E5E75A458DE0626D088C";
+    String signingKeyDigestSha1 =
+        "EEB9299D295C1C815E289FBF2F2BBEA5F52FDD19";
+    String signatureSha1 = "-----BEGIN SIGNATURE-----\n"
+        + "iHEU3Iidya5RIrjyYgv8tlU0R+rF56/3/MmaaZi0a67e7ZkISfQ4dghScHxnF3"
+        + "Yh\nrXVaaoP07r6Ta+s0g1Zijm3lms50Nk/4tV2p8Y63c3F4Q3DAnK40OikfOI"
+        + "wEj+Ny\n+zBRQssP3hPhTPOj/A7o3mZZwtL6x1sxpeu/nME1l5E=\n"
+        + "-----END SIGNATURE-----";
+    String signaturesLines = String.format(
+        "directory-signature sha256 %s %s\n%s\n"
+        + "directory-signature %s %s\n%s", identitySha256,
+        signingKeyDigestSha256, signatureSha256, identitySha1,
+        signingKeyDigestSha1, signatureSha1);
+    RelayNetworkStatusVote vote =
+        VoteBuilder.createWithDirectorySignatureLines(signaturesLines);
+    assertEquals(2, vote.getSignatures().size());
+    DirectorySignature firstSignature = vote.getSignatures().get(0);
+    assertEquals("sha256", firstSignature.getAlgorithm());
+    assertEquals(identitySha256, firstSignature.getIdentity());
+    assertEquals(signingKeyDigestSha256,
+        firstSignature.getSigningKeyDigest());
+    assertEquals(signatureSha256 + "\n", firstSignature.getSignature());
+    DirectorySignature secondSignature = vote.getSignatures().get(1);
+    assertEquals("sha1", secondSignature.getAlgorithm());
+    assertEquals(identitySha1, secondSignature.getIdentity());
+    assertEquals(signingKeyDigestSha1,
+        secondSignature.getSigningKeyDigest());
+    assertEquals(signatureSha1 + "\n", secondSignature.getSignature());
+    assertEquals(signingKeyDigestSha1, vote.getSigningKeyDigest());
+  }
+
+  @Test()
+  public void testDirectorySignaturesLinesTwoAlgorithmsSameDigests()
+      throws DescriptorParseException {
+    String signaturesLines = "directory-signature 00 00\n"
+        + "-----BEGIN SIGNATURE-----\n00\n-----END SIGNATURE-----\n"
+        + "directory-signature sha256 00 00\n"
+        + "-----BEGIN SIGNATURE-----\n00\n-----END SIGNATURE-----";
+    RelayNetworkStatusVote vote =
+        VoteBuilder.createWithDirectorySignatureLines(signaturesLines);
+    assertEquals(2, vote.getSignatures().size());
+  }
+
   @Test(expected = DescriptorParseException.class)
   public void testUnrecognizedHeaderLineFail()
       throws DescriptorParseException {