From 2af6f19476423a71318eae735ddbf200964b13a8 Mon Sep 17 00:00:00 2001
From: akwizgran <akwizgran@users.sourceforge.net>
Date: Tue, 12 Jul 2011 17:08:31 +0100
Subject: [PATCH] Check the return value from Signature.verify(). *cough*
---
.../sf/briar/api/protocol/BatchBuilder.java | 5 +-
.../sf/briar/api/protocol/HeaderBuilder.java | 5 +-
.../briar/protocol/IncomingBatchBuilder.java | 7 ++-
.../briar/protocol/IncomingHeaderBuilder.java | 7 ++-
.../briar/protocol/OutgoingBatchBuilder.java | 6 +--
.../briar/protocol/OutgoingHeaderBuilder.java | 6 +--
.../briar/protocol/BundleReadWriteTest.java | 51 +++++++++++++++++++
7 files changed, 65 insertions(+), 22 deletions(-)
diff --git a/api/net/sf/briar/api/protocol/BatchBuilder.java b/api/net/sf/briar/api/protocol/BatchBuilder.java
index fd17670c0d..ca7f867940 100644
--- a/api/net/sf/briar/api/protocol/BatchBuilder.java
+++ b/api/net/sf/briar/api/protocol/BatchBuilder.java
@@ -1,8 +1,7 @@
package net.sf.briar.api.protocol;
import java.io.IOException;
-import java.security.InvalidKeyException;
-import java.security.SignatureException;
+import java.security.GeneralSecurityException;
public interface BatchBuilder {
@@ -13,5 +12,5 @@ public interface BatchBuilder {
void setSignature(byte[] sig);
/** Builds and returns the batch. */
- Batch build() throws IOException, SignatureException, InvalidKeyException;
+ Batch build() throws IOException, GeneralSecurityException;
}
diff --git a/api/net/sf/briar/api/protocol/HeaderBuilder.java b/api/net/sf/briar/api/protocol/HeaderBuilder.java
index 9c13b889f3..c22ed3a361 100644
--- a/api/net/sf/briar/api/protocol/HeaderBuilder.java
+++ b/api/net/sf/briar/api/protocol/HeaderBuilder.java
@@ -1,8 +1,7 @@
package net.sf.briar.api.protocol;
import java.io.IOException;
-import java.security.InvalidKeyException;
-import java.security.SignatureException;
+import java.security.GeneralSecurityException;
import java.util.Map;
public interface HeaderBuilder {
@@ -20,5 +19,5 @@ public interface HeaderBuilder {
void setSignature(byte[] sig);
/** Builds and returns the header. */
- Header build() throws IOException, SignatureException, InvalidKeyException;
+ Header build() throws IOException, GeneralSecurityException;
}
diff --git a/components/net/sf/briar/protocol/IncomingBatchBuilder.java b/components/net/sf/briar/protocol/IncomingBatchBuilder.java
index 5cce8fa658..a07d17bb69 100644
--- a/components/net/sf/briar/protocol/IncomingBatchBuilder.java
+++ b/components/net/sf/briar/protocol/IncomingBatchBuilder.java
@@ -1,7 +1,7 @@
package net.sf.briar.protocol;
import java.io.IOException;
-import java.security.InvalidKeyException;
+import java.security.GeneralSecurityException;
import java.security.KeyPair;
import java.security.MessageDigest;
import java.security.Signature;
@@ -24,13 +24,12 @@ public class IncomingBatchBuilder extends BatchBuilderImpl {
this.sig = sig;
}
- public Batch build() throws IOException, SignatureException,
- InvalidKeyException {
+ public Batch build() throws IOException, GeneralSecurityException {
if(sig == null) throw new IllegalStateException();
byte[] raw = getSignableRepresentation();
signature.initVerify(keyPair.getPublic());
signature.update(raw);
- signature.verify(sig);
+ if(!signature.verify(sig)) throw new SignatureException();
messageDigest.reset();
messageDigest.update(raw);
messageDigest.update(sig);
diff --git a/components/net/sf/briar/protocol/IncomingHeaderBuilder.java b/components/net/sf/briar/protocol/IncomingHeaderBuilder.java
index 04371f8497..0c60a82f1c 100644
--- a/components/net/sf/briar/protocol/IncomingHeaderBuilder.java
+++ b/components/net/sf/briar/protocol/IncomingHeaderBuilder.java
@@ -1,7 +1,7 @@
package net.sf.briar.protocol;
import java.io.IOException;
-import java.security.InvalidKeyException;
+import java.security.GeneralSecurityException;
import java.security.KeyPair;
import java.security.MessageDigest;
import java.security.Signature;
@@ -28,13 +28,12 @@ class IncomingHeaderBuilder extends HeaderBuilderImpl {
this.sig = sig;
}
- public Header build() throws IOException, SignatureException,
- InvalidKeyException {
+ public Header build() throws IOException, GeneralSecurityException {
if(sig == null) throw new IllegalStateException();
byte[] raw = getSignableRepresentation();
signature.initVerify(keyPair.getPublic());
signature.update(raw);
- signature.verify(sig);
+ if(!signature.verify(sig)) throw new SignatureException();
messageDigest.reset();
messageDigest.update(raw);
messageDigest.update(sig);
diff --git a/components/net/sf/briar/protocol/OutgoingBatchBuilder.java b/components/net/sf/briar/protocol/OutgoingBatchBuilder.java
index 1f06d42539..bc0d673f3f 100644
--- a/components/net/sf/briar/protocol/OutgoingBatchBuilder.java
+++ b/components/net/sf/briar/protocol/OutgoingBatchBuilder.java
@@ -1,11 +1,10 @@
package net.sf.briar.protocol;
import java.io.IOException;
-import java.security.InvalidKeyException;
+import java.security.GeneralSecurityException;
import java.security.KeyPair;
import java.security.MessageDigest;
import java.security.Signature;
-import java.security.SignatureException;
import net.sf.briar.api.protocol.Batch;
import net.sf.briar.api.protocol.BatchId;
@@ -22,8 +21,7 @@ public class OutgoingBatchBuilder extends BatchBuilderImpl {
throw new UnsupportedOperationException();
}
- public Batch build() throws IOException, SignatureException,
- InvalidKeyException {
+ public Batch build() throws IOException, GeneralSecurityException {
byte[] raw = getSignableRepresentation();
signature.initSign(keyPair.getPrivate());
signature.update(raw);
diff --git a/components/net/sf/briar/protocol/OutgoingHeaderBuilder.java b/components/net/sf/briar/protocol/OutgoingHeaderBuilder.java
index 268ec5cc69..df735ddaa0 100644
--- a/components/net/sf/briar/protocol/OutgoingHeaderBuilder.java
+++ b/components/net/sf/briar/protocol/OutgoingHeaderBuilder.java
@@ -1,11 +1,10 @@
package net.sf.briar.protocol;
import java.io.IOException;
-import java.security.InvalidKeyException;
+import java.security.GeneralSecurityException;
import java.security.KeyPair;
import java.security.MessageDigest;
import java.security.Signature;
-import java.security.SignatureException;
import java.util.HashSet;
import java.util.Set;
@@ -26,8 +25,7 @@ public class OutgoingHeaderBuilder extends HeaderBuilderImpl {
throw new UnsupportedOperationException();
}
- public Header build() throws IOException, SignatureException,
- InvalidKeyException {
+ public Header build() throws IOException, GeneralSecurityException {
byte[] raw = getSignableRepresentation();
signature.initSign(keyPair.getPrivate());
signature.update(raw);
diff --git a/test/net/sf/briar/protocol/BundleReadWriteTest.java b/test/net/sf/briar/protocol/BundleReadWriteTest.java
index 52b4824133..69ddd12415 100644
--- a/test/net/sf/briar/protocol/BundleReadWriteTest.java
+++ b/test/net/sf/briar/protocol/BundleReadWriteTest.java
@@ -3,6 +3,8 @@ package net.sf.briar.protocol;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
+import java.io.RandomAccessFile;
+import java.security.GeneralSecurityException;
import java.security.KeyPair;
import java.security.KeyPairGenerator;
import java.security.MessageDigest;
@@ -149,6 +151,55 @@ public class BundleReadWriteTest extends TestCase {
r.close();
}
+
+ @Test
+ public void testModifyingBundleBreaksSignature() throws Exception {
+
+ testWriteBundle();
+
+ RandomAccessFile f = new RandomAccessFile(bundle, "rw");
+ f.seek(bundle.length() - 50);
+ byte b = f.readByte();
+ f.seek(bundle.length() - 50);
+ f.writeByte(b + 1);
+ f.close();
+
+ MessageParser messageParser = new MessageParser() {
+ public Message parseMessage(byte[] body) throws FormatException,
+ SignatureException {
+ // FIXME: Really parse the message
+ return message;
+ }
+ };
+ Provider<HeaderBuilder> headerBuilderProvider =
+ new Provider<HeaderBuilder>() {
+ public HeaderBuilder get() {
+ return new IncomingHeaderBuilder(keyPair, sig, digest, wf);
+ }
+ };
+ Provider<BatchBuilder> batchBuilderProvider =
+ new Provider<BatchBuilder>() {
+ public BatchBuilder get() {
+ return new IncomingBatchBuilder(keyPair, sig, digest, wf);
+ }
+ };
+
+ FileInputStream in = new FileInputStream(bundle);
+ Reader reader = new ReaderFactoryImpl().createReader(in);
+ BundleReader r = new BundleReaderImpl(reader, bundle.length(),
+ messageParser, headerBuilderProvider, batchBuilderProvider);
+
+ Header h = r.getHeader();
+ assertEquals(acks, h.getAcks());
+ assertEquals(subs, h.getSubscriptions());
+ assertEquals(transports, h.getTransports());
+ try {
+ r.getNextBatch();
+ assertTrue(false);
+ } catch(GeneralSecurityException expected) {}
+ r.close();
+ }
+
@After
public void tearDown() {
TestUtils.deleteTestDirectory(testDir);
--
GitLab