Permalink
Browse files

Patch in a bug fix to Signer from by Quan Nguyen, and update Verifier…

… to be backwards compatible with existing signatures even when runnning on stricter than usual JCE implementations that reject extra bytes at the end of signatures during DSA verification.
  • Loading branch information...
1 parent 36d7add commit fb019ba4c5ed7002b93e632e85c5bb95af860711 @asacamano asacamano committed Oct 12, 2016
@@ -20,14 +20,14 @@
import org.keyczar.exceptions.KeyczarException;
import org.keyczar.exceptions.NoPrimaryKeyException;
import org.keyczar.exceptions.ShortBufferException;
-import org.keyczar.i18n.Messages;
import org.keyczar.interfaces.KeyczarReader;
import org.keyczar.interfaces.SigningStream;
import org.keyczar.util.Base64Coder;
import org.keyczar.util.Util;
import java.io.UnsupportedEncodingException;
import java.nio.ByteBuffer;
+import java.util.Arrays;
/**
* Signers may both sign and verify data using sets of symmetric or private
@@ -230,7 +230,8 @@ public String sign(String input) throws KeyczarException {
// Attached signature format is:
// [Format number | 4 bytes of key hash | blob size | blob | raw signature]
byte[] signature =
- Util.cat(FORMAT_BYTES, signingKey.hash(), Util.lenPrefix(blob), output.array());
+ Util.cat(FORMAT_BYTES, signingKey.hash(), Util.lenPrefix(blob),
+ Arrays.copyOfRange(output.array(), 0, output.position()));
signingKey.addStreamToCacheForReuse(stream);
return signature;
}
@@ -16,13 +16,11 @@
package org.keyczar;
-
import org.keyczar.enums.KeyPurpose;
import org.keyczar.exceptions.BadVersionException;
import org.keyczar.exceptions.KeyNotFoundException;
import org.keyczar.exceptions.KeyczarException;
import org.keyczar.exceptions.ShortSignatureException;
-import org.keyczar.i18n.Messages;
import org.keyczar.interfaces.KeyczarReader;
import org.keyczar.interfaces.VerifyingStream;
import org.keyczar.util.Base64Coder;
@@ -31,7 +29,6 @@
import java.io.UnsupportedEncodingException;
import java.nio.ByteBuffer;
-
/**
* Verifiers are used strictly to verify signatures. Typically, Verifiers will
* read sets of public keys, although may also be instantiated with sets of
@@ -182,6 +179,15 @@ boolean rawVerify(KeyczarKey key, final ByteBuffer data, final ByteBuffer hidden
// The signed data is terminated with the current Keyczar format
stream.updateVerify(ByteBuffer.wrap(FORMAT_BYTES));
+ // There was a bug in Signer which allowed extra bytes to be included at the end of signatures
+ // in some cases. There are some crypto libraries which reject signatures with extra bytes at
+ // the end. So this change trims the signature to the expected length so that existing
+ // signatures will not break. Note that if you don't use one of the strict crypto libraries,
+ // then you won't have this problem - but there is no way to identify them.
+ int trimmedSignatureLength =
+ Math.min(stream.digestSize(), signature.limit() - signature.position());
+ ByteBuffer trimmedSignature = ByteBuffer.allocate(trimmedSignatureLength);
+ trimmedSignature.put(signature.array(), signature.position(), trimmedSignatureLength);
boolean result = stream.verify(signature);
key.addStreamToCacheForReuse(stream);
return result;
@@ -0,0 +1,48 @@
+package org.keyczar;
+
+import java.nio.ByteBuffer;
+import junit.framework.TestCase;
+import org.junit.Test;
+import org.keyczar.exceptions.KeyczarException;
+import org.keyczar.util.Base64Coder;
+
+/**
+ * This test makes sure that the verification of both proper length signatures and signatures with
+ * extra bytes appended can be verified.
+ */
+public class VerifierBackwardsCompatilityTest extends TestCase {
+ private static final String TEST_DATA = "./testdata";
+ private Signer privateKeySigner;
+ private Verifier publicKeyVerifier;
+ private byte[] data;
+
+ @Override
+ protected void setUp() throws Exception {
+ super.setUp();
+ privateKeySigner = new Signer(TEST_DATA + "/dsa");
+ publicKeyVerifier = new Verifier(TEST_DATA + "/dsa.public");
+ data = Base64Coder.decodeWebSafe(
+ "U3VjY2VzcyEgWW91J3ZlIG1hbmFnZWQgdG8gaW5maWx0cmF0ZSBDb21tYW5kZXIgTGFtYmRhJ3MgZXZpbCBvcmdhbm"
+ + "l6YXRpb24sIGFuZCBmaW5hbGx5IGVhcm5lZCB5b3Vyc2VsZiBhbiBlbnRyeS1sZXZlbCBwb3NpdGlvbiBhcyBhIE"
+ + "1pbmlvbiBvbiBoZXIgc3BhY2Ugc3RhdGlvbi4gRnJvbSBoZXJlLCB5b3UganVzdCBtaWdodCBiZSBhYmxlIHRvIH"
+ + "N1YnZlcnQgaGVyIHBsYW5zIHRvIHVzZSB0aGUgTEFNQkNIT1AgZG9vbXNkYXkgZGV2aWNlIHRvIGRlc3Ryb3kgQn"
+ + "VubnkgUGxhbmV0LiBQcm9ibGVtIGlzLCBNaW5pb25zIGFyZSB0aGUgbG93ZXN0IG9mIHRoZSBsb3cgaW4gdGhlIE"
+ + "xhbWJkYSBoaWVyYXJjaHkuIEJldHRlciBidWNrIHVwIGFuZCBnZXQgd29ya2luZywgb3IgeW91J2xsIG5ldmVyIG"
+ + "1ha2UgaXQgdG8gdGhlIHRvcC4uLg=="
+ );
+ }
+
+ @Test
+ public final void testVerifyWithExtraBytes() throws KeyczarException {
+ byte[] signature = privateKeySigner.sign(data);
+ for (int cruftSize = 0; cruftSize < 16; cruftSize++) {
+ ByteBuffer cruftySignature = ByteBuffer.allocate(signature.length + cruftSize);
+ cruftySignature.put(signature);
+ for (int i = 0; i < cruftSize; i++) {
+ cruftySignature.put((byte)i);
+ }
+ assertTrue("failed with " + cruftSize + " bytes of cruft",
+ publicKeyVerifier.verify(data, cruftySignature.array()));
+ }
+ }
+}

0 comments on commit fb019ba

Please sign in to comment.