From 5ce8b1978d418b5ff6819d4045858d2645bc89d5 Mon Sep 17 00:00:00 2001
From: Torsten Grote <t@grobox.de>
Date: Thu, 27 Oct 2016 11:24:33 -0200
Subject: [PATCH] Remove forum content type and move bodies to string

Also removes support for anonymous forum posts.
Closes #698
---
 .../org/briarproject/ForumManagerTest.java    | 36 +++++----
 .../ForumSharingIntegrationTest.java          |  6 +-
 .../MessageSizeIntegrationTest.java           |  2 +-
 .../android/forum/ForumControllerImpl.java    |  3 +-
 .../briarproject/api/forum/ForumManager.java  |  2 +-
 .../api/forum/ForumPostFactory.java           | 11 ++-
 .../briarproject/forum/ForumManagerImpl.java  |  8 +-
 .../org/briarproject/forum/ForumModule.java   |  6 +-
 .../forum/ForumPostFactoryImpl.java           | 55 +++-----------
 .../forum/ForumPostValidator.java             | 76 ++++++++-----------
 10 files changed, 81 insertions(+), 124 deletions(-)

diff --git a/briar-android-tests/src/test/java/org/briarproject/ForumManagerTest.java b/briar-android-tests/src/test/java/org/briarproject/ForumManagerTest.java
index 64ad251f07..059f3912e6 100644
--- a/briar-android-tests/src/test/java/org/briarproject/ForumManagerTest.java
+++ b/briar-android-tests/src/test/java/org/briarproject/ForumManagerTest.java
@@ -7,6 +7,8 @@ import net.jodah.concurrentunit.Waiter;
 import org.briarproject.api.contact.Contact;
 import org.briarproject.api.contact.ContactId;
 import org.briarproject.api.contact.ContactManager;
+import org.briarproject.api.crypto.CryptoComponent;
+import org.briarproject.api.crypto.KeyPair;
 import org.briarproject.api.crypto.SecretKey;
 import org.briarproject.api.db.DbException;
 import org.briarproject.api.event.Event;
@@ -34,7 +36,7 @@ import org.briarproject.properties.PropertiesModule;
 import org.briarproject.sharing.SharingModule;
 import org.briarproject.sync.SyncModule;
 import org.briarproject.transport.TransportModule;
-import org.briarproject.util.StringUtils;
+import org.jetbrains.annotations.Nullable;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -53,7 +55,6 @@ import static junit.framework.Assert.assertEquals;
 import static junit.framework.Assert.assertNull;
 import static junit.framework.TestCase.assertFalse;
 import static org.briarproject.TestPluginsModule.MAX_LATENCY;
-import static org.briarproject.api.identity.AuthorConstants.MAX_PUBLIC_KEY_LENGTH;
 import static org.briarproject.api.sync.ValidationManager.State.DELIVERED;
 import static org.briarproject.api.sync.ValidationManager.State.INVALID;
 import static org.briarproject.api.sync.ValidationManager.State.PENDING;
@@ -75,6 +76,8 @@ public class ForumManagerTest extends BriarIntegrationTest {
 	@Inject
 	AuthorFactory authorFactory;
 	@Inject
+	CryptoComponent crypto;
+	@Inject
 	ForumPostFactory forumPostFactory;
 
 	// objects accessed from background threads need to be volatile
@@ -127,16 +130,17 @@ public class ForumManagerTest extends BriarIntegrationTest {
 		deliveryWaiter = new Waiter();
 	}
 
-	private ForumPost createForumPost(GroupId groupId, ForumPost parent,
-			String body, long ms) throws Exception {
-		return forumPostFactory.createAnonymousPost(groupId, ms,
+	private ForumPost createForumPost(GroupId groupId,
+			@Nullable ForumPost parent, String body, long ms) throws Exception {
+		return forumPostFactory.createPost(groupId, ms,
 				parent == null ? null : parent.getMessage().getId(),
-				"text/plain", StringUtils.toUtf8(body));
+				author0, body);
 	}
 
 	@Test
 	public void testForumPost() throws Exception {
 		startLifecycles();
+		addDefaultIdentities();
 		Forum forum = forumManager0.addForum("TestForum");
 		assertEquals(1, forumManager0.getForums().size());
 		final long ms1 = clock.currentTimeMillis() - 1000L;
@@ -167,8 +171,7 @@ public class ForumManagerTest extends BriarIntegrationTest {
 				forumManager0.getPostHeaders(forum.getGroup().getId());
 		assertEquals(2, headers.size());
 		for (ForumPostHeader h : headers) {
-			final String hBody =
-					StringUtils.fromUtf8(forumManager0.getPostBody(h.getId()));
+			final String hBody = forumManager0.getPostBody(h.getId());
 
 			boolean isPost1 = h.getId().equals(post1.getMessage().getId());
 			boolean isPost2 = h.getId().equals(post2.getMessage().getId());
@@ -366,13 +369,18 @@ public class ForumManagerTest extends BriarIntegrationTest {
 	}
 
 	private void addDefaultIdentities() throws DbException {
-		author0 = authorFactory.createLocalAuthor(SHARER,
-				TestUtils.getRandomBytes(MAX_PUBLIC_KEY_LENGTH),
-				TestUtils.getRandomBytes(123));
+		KeyPair keyPair0 = crypto.generateSignatureKeyPair();
+		byte[] publicKey0 = keyPair0.getPublic().getEncoded();
+		byte[] privateKey0 = keyPair0.getPrivate().getEncoded();
+		author0 = authorFactory
+				.createLocalAuthor(SHARER, publicKey0, privateKey0);
 		identityManager0.addLocalAuthor(author0);
-		author1 = authorFactory.createLocalAuthor(INVITEE,
-				TestUtils.getRandomBytes(MAX_PUBLIC_KEY_LENGTH),
-				TestUtils.getRandomBytes(123));
+
+		KeyPair keyPair1 = crypto.generateSignatureKeyPair();
+		byte[] publicKey1 = keyPair1.getPublic().getEncoded();
+		byte[] privateKey1 = keyPair1.getPrivate().getEncoded();
+		author1 = authorFactory
+				.createLocalAuthor(INVITEE, publicKey1, privateKey1);
 		identityManager1.addLocalAuthor(author1);
 	}
 
diff --git a/briar-android-tests/src/test/java/org/briarproject/ForumSharingIntegrationTest.java b/briar-android-tests/src/test/java/org/briarproject/ForumSharingIntegrationTest.java
index 6d2184a6ae..a30be29ce8 100644
--- a/briar-android-tests/src/test/java/org/briarproject/ForumSharingIntegrationTest.java
+++ b/briar-android-tests/src/test/java/org/briarproject/ForumSharingIntegrationTest.java
@@ -820,7 +820,7 @@ public class ForumSharingIntegrationTest extends BriarTestCase {
 			long time = clock.currentTimeMillis();
 			String body = TestUtils.getRandomString(42);
 			ForumPost p = forumPostFactory
-					.createPseudonymousPost(forum0.getId(), time, null, author0,
+					.createPost(forum0.getId(), time, null, author0,
 							body);
 			forumManager0.addLocalPost(p);
 
@@ -839,7 +839,7 @@ public class ForumSharingIntegrationTest extends BriarTestCase {
 			time = clock.currentTimeMillis();
 			body = TestUtils.getRandomString(42);
 			p = forumPostFactory
-					.createPseudonymousPost(forum0.getId(), time, null, author1,
+					.createPost(forum0.getId(), time, null, author1,
 							body);
 			forumManager1.addLocalPost(p);
 
@@ -883,7 +883,7 @@ public class ForumSharingIntegrationTest extends BriarTestCase {
 			time = clock.currentTimeMillis();
 			body = TestUtils.getRandomString(42);
 			p = forumPostFactory
-					.createPseudonymousPost(forum0.getId(), time, null, author1,
+					.createPost(forum0.getId(), time, null, author1,
 							body);
 			forumManager1.addLocalPost(p);
 
diff --git a/briar-android-tests/src/test/java/org/briarproject/MessageSizeIntegrationTest.java b/briar-android-tests/src/test/java/org/briarproject/MessageSizeIntegrationTest.java
index 45fb9d7237..02849dfee8 100644
--- a/briar-android-tests/src/test/java/org/briarproject/MessageSizeIntegrationTest.java
+++ b/briar-android-tests/src/test/java/org/briarproject/MessageSizeIntegrationTest.java
@@ -77,7 +77,7 @@ public class MessageSizeIntegrationTest extends BriarTestCase {
 		long timestamp = Long.MAX_VALUE;
 		MessageId parent = new MessageId(TestUtils.getRandomId());
 		String body = TestUtils.getRandomString(MAX_FORUM_POST_BODY_LENGTH);
-		ForumPost post = forumPostFactory.createPseudonymousPost(groupId,
+		ForumPost post = forumPostFactory.createPost(groupId,
 				timestamp, parent, author, body);
 		// Check the size of the serialised message
 		int length = post.getMessage().getRaw().length;
diff --git a/briar-android/src/org/briarproject/android/forum/ForumControllerImpl.java b/briar-android/src/org/briarproject/android/forum/ForumControllerImpl.java
index 25d8328464..a3b7fde893 100644
--- a/briar-android/src/org/briarproject/android/forum/ForumControllerImpl.java
+++ b/briar-android/src/org/briarproject/android/forum/ForumControllerImpl.java
@@ -21,7 +21,6 @@ import org.briarproject.api.identity.LocalAuthor;
 import org.briarproject.api.lifecycle.LifecycleManager;
 import org.briarproject.api.sync.MessageId;
 import org.briarproject.api.system.Clock;
-import org.briarproject.util.StringUtils;
 
 import java.util.Collection;
 import java.util.concurrent.Executor;
@@ -89,7 +88,7 @@ public class ForumControllerImpl
 
 	@Override
 	protected String loadMessageBody(ForumPostHeader h) throws DbException {
-		return StringUtils.fromUtf8(forumManager.getPostBody(h.getId()));
+		return forumManager.getPostBody(h.getId());
 	}
 
 	@Override
diff --git a/briar-api/src/org/briarproject/api/forum/ForumManager.java b/briar-api/src/org/briarproject/api/forum/ForumManager.java
index 7561fc9636..51af10a1c3 100644
--- a/briar-api/src/org/briarproject/api/forum/ForumManager.java
+++ b/briar-api/src/org/briarproject/api/forum/ForumManager.java
@@ -41,7 +41,7 @@ public interface ForumManager extends MessageTracker {
 	Collection<Forum> getForums() throws DbException;
 
 	/** Returns the body of the forum post with the given ID. */
-	byte[] getPostBody(MessageId m) throws DbException;
+	String getPostBody(MessageId m) throws DbException;
 
 	/** Returns the headers of all posts in the given forum. */
 	Collection<ForumPostHeader> getPostHeaders(GroupId g) throws DbException;
diff --git a/briar-api/src/org/briarproject/api/forum/ForumPostFactory.java b/briar-api/src/org/briarproject/api/forum/ForumPostFactory.java
index 1d6aa9e76d..820a11acc2 100644
--- a/briar-api/src/org/briarproject/api/forum/ForumPostFactory.java
+++ b/briar-api/src/org/briarproject/api/forum/ForumPostFactory.java
@@ -1,6 +1,7 @@
 package org.briarproject.api.forum;
 
 import org.briarproject.api.FormatException;
+import org.briarproject.api.crypto.CryptoExecutor;
 import org.briarproject.api.identity.LocalAuthor;
 import org.briarproject.api.sync.GroupId;
 import org.briarproject.api.sync.MessageId;
@@ -9,11 +10,9 @@ import java.security.GeneralSecurityException;
 
 public interface ForumPostFactory {
 
-	ForumPost createAnonymousPost(GroupId groupId, long timestamp,
-			MessageId parent, String contentType, byte[] body)
-			throws FormatException;
-
-	ForumPost createPseudonymousPost(GroupId groupId, long timestamp,
-			MessageId parent, LocalAuthor author, String body)
+	@CryptoExecutor
+	ForumPost createPost(GroupId groupId, long timestamp, MessageId parent,
+			LocalAuthor author, String body)
 			throws FormatException, GeneralSecurityException;
+
 }
diff --git a/briar-core/src/org/briarproject/forum/ForumManagerImpl.java b/briar-core/src/org/briarproject/forum/ForumManagerImpl.java
index 8d1511f7d8..44f7659e1d 100644
--- a/briar-core/src/org/briarproject/forum/ForumManagerImpl.java
+++ b/briar-core/src/org/briarproject/forum/ForumManagerImpl.java
@@ -131,7 +131,7 @@ class ForumManagerImpl extends BdfIncomingMessageHook implements ForumManager {
 		ForumPost p;
 		try {
 			p = forumPostFactory
-					.createPseudonymousPost(groupId, timestamp, parentId,
+					.createPost(groupId, timestamp, parentId,
 							author, body);
 		} catch (GeneralSecurityException e) {
 			throw new RuntimeException(e);
@@ -213,11 +213,11 @@ class ForumManagerImpl extends BdfIncomingMessageHook implements ForumManager {
 	}
 
 	@Override
-	public byte[] getPostBody(MessageId m) throws DbException {
+	public String getPostBody(MessageId m) throws DbException {
 		try {
-			// Parent ID, author, content type, forum post body, signature
+			// Parent ID, author, forum post body, signature
 			BdfList message = clientHelper.getMessageAsList(m);
-			return message.getRaw(3);
+			return message.getString(2);
 		} catch (FormatException e) {
 			throw new DbException(e);
 		}
diff --git a/briar-core/src/org/briarproject/forum/ForumModule.java b/briar-core/src/org/briarproject/forum/ForumModule.java
index eeafb54dfc..4a2e6a7983 100644
--- a/briar-core/src/org/briarproject/forum/ForumModule.java
+++ b/briar-core/src/org/briarproject/forum/ForumModule.java
@@ -40,9 +40,9 @@ public class ForumModule {
 	}
 
 	@Provides
-	ForumPostFactory provideForumPostFactory(CryptoComponent crypto,
-			ClientHelper clientHelper) {
-		return new ForumPostFactoryImpl(crypto, clientHelper);
+	ForumPostFactory provideForumPostFactory(
+			ForumPostFactoryImpl forumPostFactory) {
+		return forumPostFactory;
 	}
 
 	@Provides
diff --git a/briar-core/src/org/briarproject/forum/ForumPostFactoryImpl.java b/briar-core/src/org/briarproject/forum/ForumPostFactoryImpl.java
index 7c0d1e46dd..56c3a61b28 100644
--- a/briar-core/src/org/briarproject/forum/ForumPostFactoryImpl.java
+++ b/briar-core/src/org/briarproject/forum/ForumPostFactoryImpl.java
@@ -2,10 +2,6 @@ package org.briarproject.forum;
 
 import org.briarproject.api.FormatException;
 import org.briarproject.api.clients.ClientHelper;
-import org.briarproject.api.crypto.CryptoComponent;
-import org.briarproject.api.crypto.KeyParser;
-import org.briarproject.api.crypto.PrivateKey;
-import org.briarproject.api.crypto.Signature;
 import org.briarproject.api.data.BdfList;
 import org.briarproject.api.forum.ForumPost;
 import org.briarproject.api.forum.ForumPostFactory;
@@ -19,64 +15,35 @@ import java.security.GeneralSecurityException;
 
 import javax.inject.Inject;
 
-import static org.briarproject.api.forum.ForumConstants.MAX_CONTENT_TYPE_LENGTH;
 import static org.briarproject.api.forum.ForumConstants.MAX_FORUM_POST_BODY_LENGTH;
 
 class ForumPostFactoryImpl implements ForumPostFactory {
 
-	private final CryptoComponent crypto;
 	private final ClientHelper clientHelper;
 
 	@Inject
-	ForumPostFactoryImpl(CryptoComponent crypto, ClientHelper clientHelper) {
-		this.crypto = crypto;
+	ForumPostFactoryImpl(ClientHelper clientHelper) {
 		this.clientHelper = clientHelper;
 	}
 
 	@Override
-	public ForumPost createAnonymousPost(GroupId groupId, long timestamp,
-			MessageId parent, String contentType, byte[] body)
-			throws FormatException {
-		// Validate the arguments
-		if (StringUtils.toUtf8(contentType).length > MAX_CONTENT_TYPE_LENGTH)
-			throw new IllegalArgumentException();
-		if (body.length > MAX_FORUM_POST_BODY_LENGTH)
-			throw new IllegalArgumentException();
-		// Serialise the message
-		BdfList message = BdfList.of(parent, null, contentType, body, null);
-		Message m = clientHelper.createMessage(groupId, timestamp, message);
-		return new ForumPost(m, parent, null);
-	}
-
-	@Override
-	public ForumPost createPseudonymousPost(GroupId groupId, long timestamp,
-			MessageId parent, LocalAuthor author, String bodyStr)
+	public ForumPost createPost(GroupId groupId, long timestamp,
+			MessageId parent, LocalAuthor author, String body)
 			throws FormatException, GeneralSecurityException {
-		String contentType = "text/plain";
-		byte[] body = StringUtils.toUtf8(bodyStr);
 		// Validate the arguments
-		if (StringUtils.toUtf8(contentType).length > MAX_CONTENT_TYPE_LENGTH)
-			throw new IllegalArgumentException();
-		if (body.length > MAX_FORUM_POST_BODY_LENGTH)
+		if (StringUtils.isTooLong(body, MAX_FORUM_POST_BODY_LENGTH))
 			throw new IllegalArgumentException();
 		// Serialise the data to be signed
-		BdfList authorList = BdfList.of(author.getName(),
-				author.getPublicKey());
+		BdfList authorList =
+				BdfList.of(author.getName(), author.getPublicKey());
 		BdfList signed = BdfList.of(groupId, timestamp, parent, authorList,
-				contentType, body);
-		// Get private key
-		KeyParser keyParser = crypto.getSignatureKeyParser();
-		byte[] k = author.getPrivateKey();
-		PrivateKey privateKey = keyParser.parsePrivateKey(k);
-		// Generate the signature
-		Signature signature = crypto.getSignature();
-		signature.initSign(privateKey);
-		signature.update(clientHelper.toByteArray(signed));
-		byte[] sig = signature.sign();
+				body);
+		// Sign the data
+		byte[] sig = clientHelper.sign(signed, author.getPrivateKey());
 		// Serialise the signed message
-		BdfList message = BdfList.of(parent, authorList, contentType, body,
-				sig);
+		BdfList message = BdfList.of(parent, authorList, body, sig);
 		Message m = clientHelper.createMessage(groupId, timestamp, message);
 		return new ForumPost(m, parent, author);
 	}
+
 }
diff --git a/briar-core/src/org/briarproject/forum/ForumPostValidator.java b/briar-core/src/org/briarproject/forum/ForumPostValidator.java
index ce4bddb436..a763b821e8 100644
--- a/briar-core/src/org/briarproject/forum/ForumPostValidator.java
+++ b/briar-core/src/org/briarproject/forum/ForumPostValidator.java
@@ -20,7 +20,6 @@ import java.security.GeneralSecurityException;
 import java.util.Collection;
 import java.util.Collections;
 
-import static org.briarproject.api.forum.ForumConstants.MAX_CONTENT_TYPE_LENGTH;
 import static org.briarproject.api.forum.ForumConstants.MAX_FORUM_POST_BODY_LENGTH;
 import static org.briarproject.api.identity.AuthorConstants.MAX_AUTHOR_NAME_LENGTH;
 import static org.briarproject.api.identity.AuthorConstants.MAX_PUBLIC_KEY_LENGTH;
@@ -40,50 +39,38 @@ class ForumPostValidator extends BdfMessageValidator {
 	protected BdfMessageContext validateMessage(Message m, Group g,
 			BdfList body) throws InvalidMessageException, FormatException {
 		// Parent ID, author, content type, forum post body, signature
-		checkSize(body, 5);
+		checkSize(body, 4);
+
 		// Parent ID is optional
 		byte[] parent = body.getOptionalRaw(0);
 		checkLength(parent, UniqueId.LENGTH);
-		// Author is optional
-		Author author = null;
-		BdfList authorList = body.getOptionalList(1);
-		if (authorList != null) {
-			// Name, public key
-			checkSize(authorList, 2);
-			String name = authorList.getString(0);
-			checkLength(name, 1, MAX_AUTHOR_NAME_LENGTH);
-			byte[] publicKey = authorList.getRaw(1);
-			checkLength(publicKey, 0, MAX_PUBLIC_KEY_LENGTH);
-			author = authorFactory.createAuthor(name, publicKey);
-		}
-		// Content type
-		String contentType = body.getString(2);
-		checkLength(contentType, 0, MAX_CONTENT_TYPE_LENGTH);
+
+		// Author
+		BdfList authorList = body.getList(1);
+		// Name, public key
+		checkSize(authorList, 2);
+		String name = authorList.getString(0);
+		checkLength(name, 1, MAX_AUTHOR_NAME_LENGTH);
+		byte[] publicKey = authorList.getRaw(1);
+		checkLength(publicKey, 0, MAX_PUBLIC_KEY_LENGTH);
+		Author author = authorFactory.createAuthor(name, publicKey);
+
 		// Forum post body
-		byte[] forumPostBody = body.getRaw(3);
+		String forumPostBody = body.getString(2);
 		checkLength(forumPostBody, 0, MAX_FORUM_POST_BODY_LENGTH);
-		// Signature is optional
-		byte[] sig = body.getOptionalRaw(4);
+
+		// Signature
+		byte[] sig = body.getRaw(3);
 		checkLength(sig, 0, MAX_SIGNATURE_LENGTH);
-		// If there's an author there must be a signature and vice versa
-		if (author != null && sig == null) {
-			throw new InvalidMessageException("Author without signature");
-		}
-		if (author == null && sig != null) {
-			throw new InvalidMessageException("Signature without author");
-		}
-		// Verify the signature, if any
-		if (author != null) {
-			// Serialise the data to be verified
-			BdfList signed = BdfList.of(g.getId(), m.getTimestamp(), parent,
-					authorList, contentType, forumPostBody);
-			try {
-				clientHelper
-						.verifySignature(sig, author.getPublicKey(), signed);
-			} catch (GeneralSecurityException e) {
-				throw new InvalidMessageException(e);
-			}
+		// Verify the signature
+		BdfList signed = BdfList.of(g.getId(), m.getTimestamp(), parent,
+				authorList, forumPostBody);
+		try {
+			clientHelper.verifySignature(sig, author.getPublicKey(), signed);
+		} catch (GeneralSecurityException e) {
+			throw new InvalidMessageException(e);
 		}
+
 		// Return the metadata and dependencies
 		BdfDictionary meta = new BdfDictionary();
 		Collection<MessageId> dependencies = Collections.emptyList();
@@ -92,14 +79,11 @@ class ForumPostValidator extends BdfMessageValidator {
 			meta.put("parent", parent);
 			dependencies = Collections.singletonList(new MessageId(parent));
 		}
-		if (author != null) {
-			BdfDictionary authorMeta = new BdfDictionary();
-			authorMeta.put("id", author.getId());
-			authorMeta.put("name", author.getName());
-			authorMeta.put("publicKey", author.getPublicKey());
-			meta.put("author", authorMeta);
-		}
-		meta.put("contentType", contentType);
+		BdfDictionary authorMeta = new BdfDictionary();
+		authorMeta.put("id", author.getId());
+		authorMeta.put("name", author.getName());
+		authorMeta.put("publicKey", author.getPublicKey());
+		meta.put("author", authorMeta);
 		meta.put("read", false);
 		return new BdfMessageContext(meta, dependencies);
 	}
-- 
GitLab