commit 90d61b700454df4f418ec497191cfb435e55ee10 Author: Karsten Loesing karsten.loesing@gmx.net Date: Tue Jun 14 10:10:24 2016 +0200

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 | 8 +++ .../descriptor/RelayNetworkStatusConsensus.java | 11 ++++ .../descriptor/RelayNetworkStatusVote.java | 19 ++++++ .../descriptor/impl/DirectorySignatureImpl.java | 10 ++-- .../descriptor/impl/NetworkStatusImpl.java | 22 +++++-- .../torproject/descriptor/impl/ParseHelper.java | 17 +++++- .../impl/RelayNetworkStatusVoteImpl.java | 17 ++++-- .../impl/RelayNetworkStatusConsensusImplTest.java | 35 ++++++++--- .../impl/RelayNetworkStatusVoteImplTest.java | 69 ++++++++++++++++++++++ 9 files changed, 184 insertions(+), 24 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c243d8..160d405 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/org/torproject/descriptor/RelayNetworkStatusConsensus.java b/src/org/torproject/descriptor/RelayNetworkStatusConsensus.java index 90f96d7..15fdaca 100644 --- a/src/org/torproject/descriptor/RelayNetworkStatusConsensus.java +++ b/src/org/torproject/descriptor/RelayNetworkStatusConsensus.java @@ -186,11 +186,22 @@ 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 * and map values being weight values, or null if the consensus doesn't diff --git a/src/org/torproject/descriptor/RelayNetworkStatusVote.java b/src/org/torproject/descriptor/RelayNetworkStatusVote.java index 6e7e1b7..1f77db6 100644 --- a/src/org/torproject/descriptor/RelayNetworkStatusVote.java +++ b/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(); }

diff --git a/src/org/torproject/descriptor/impl/DirectorySignatureImpl.java b/src/org/torproject/descriptor/impl/DirectorySignatureImpl.java index 597ccce..a955f62 100644 --- a/src/org/torproject/descriptor/impl/DirectorySignatureImpl.java +++ b/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; diff --git a/src/org/torproject/descriptor/impl/NetworkStatusImpl.java b/src/org/torproject/descriptor/impl/NetworkStatusImpl.java index 9a950cf..5fa22c7 100644 --- a/src/org/torproject/descriptor/impl/NetworkStatusImpl.java +++ b/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; } }

diff --git a/src/org/torproject/descriptor/impl/ParseHelper.java b/src/org/torproject/descriptor/impl/ParseHelper.java index 1d349b4..82c0813 100644 --- a/src/org/torproject/descriptor/impl/ParseHelper.java +++ b/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 + "'."); } diff --git a/src/org/torproject/descriptor/impl/RelayNetworkStatusVoteImpl.java b/src/org/torproject/descriptor/impl/RelayNetworkStatusVoteImpl.java index d9bdd3c..384ad1f 100644 --- a/src/org/torproject/descriptor/impl/RelayNetworkStatusVoteImpl.java +++ b/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; } diff --git a/test/org/torproject/descriptor/impl/RelayNetworkStatusConsensusImplTest.java b/test/org/torproject/descriptor/impl/RelayNetworkStatusConsensusImplTest.java index 86e4e78..d864337 100644 --- a/test/org/torproject/descriptor/impl/RelayNetworkStatusConsensusImplTest.java +++ b/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"); } diff --git a/test/org/torproject/descriptor/impl/RelayNetworkStatusVoteImplTest.java b/test/org/torproject/descriptor/impl/RelayNetworkStatusVoteImplTest.java index f7ddd8c..1c840f5 100644 --- a/test/org/torproject/descriptor/impl/RelayNetworkStatusVoteImplTest.java +++ b/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 {