Address review comments

This commit is contained in:
Torsten Grote
2016-10-28 16:07:17 -02:00
parent c79ce61f6d
commit c0aa255bb6
22 changed files with 226 additions and 132 deletions

View File

@@ -20,6 +20,7 @@ import org.briarproject.api.sync.MessageId;
import org.briarproject.api.system.Clock;
import org.briarproject.clients.BdfMessageValidator;
import java.security.GeneralSecurityException;
import java.util.Collection;
import java.util.Collections;
@@ -101,7 +102,11 @@ class BlogPostValidator extends BdfMessageValidator {
BdfList signed = BdfList.of(g.getId(), m.getTimestamp(), postBody);
Blog b = blogFactory.parseBlog(g, ""); // description doesn't matter
Author a = b.getAuthor();
clientHelper.verifySignature(sig, a.getPublicKey(), signed);
try {
clientHelper.verifySignature(sig, a.getPublicKey(), signed);
} catch (GeneralSecurityException e) {
throw new InvalidMessageException(e);
}
// Return the metadata and dependencies
BdfDictionary meta = new BdfDictionary();
@@ -142,7 +147,11 @@ class BlogPostValidator extends BdfMessageValidator {
currentId);
Blog b = blogFactory.parseBlog(g, ""); // description doesn't matter
Author a = b.getAuthor();
clientHelper.verifySignature(sig, a.getPublicKey(), signed);
try {
clientHelper.verifySignature(sig, a.getPublicKey(), signed);
} catch (GeneralSecurityException e) {
throw new InvalidMessageException(e);
}
// Return the metadata and dependencies
BdfDictionary meta = new BdfDictionary();

View File

@@ -20,7 +20,6 @@ import org.briarproject.api.db.DbException;
import org.briarproject.api.db.Metadata;
import org.briarproject.api.db.Transaction;
import org.briarproject.api.sync.GroupId;
import org.briarproject.api.sync.InvalidMessageException;
import org.briarproject.api.sync.Message;
import org.briarproject.api.sync.MessageFactory;
import org.briarproject.api.sync.MessageId;
@@ -325,22 +324,16 @@ class ClientHelperImpl implements ClientHelper {
@Override
public void verifySignature(byte[] sig, byte[] publicKey, BdfList signed)
throws InvalidMessageException {
try {
// Parse the public key
KeyParser keyParser = cryptoComponent.getSignatureKeyParser();
PublicKey key = keyParser.parsePublicKey(publicKey);
// Verify the signature
Signature signature = cryptoComponent.getSignature();
signature.initVerify(key);
signature.update(toByteArray(signed));
if (!signature.verify(sig)) {
throw new InvalidMessageException("Invalid signature");
}
} catch (GeneralSecurityException e) {
throw new InvalidMessageException("Invalid public key");
} catch (FormatException e) {
throw new InvalidMessageException(e);
throws FormatException, GeneralSecurityException {
// Parse the public key
KeyParser keyParser = cryptoComponent.getSignatureKeyParser();
PublicKey key = keyParser.parsePublicKey(publicKey);
// Verify the signature
Signature signature = cryptoComponent.getSignature();
signature.initVerify(key);
signature.update(toByteArray(signed));
if (!signature.verify(sig)) {
throw new GeneralSecurityException("Invalid signature");
}
}

View File

@@ -16,6 +16,7 @@ import org.briarproject.api.sync.MessageId;
import org.briarproject.api.system.Clock;
import org.briarproject.clients.BdfMessageValidator;
import java.security.GeneralSecurityException;
import java.util.Collection;
import java.util.Collections;
@@ -73,10 +74,15 @@ class ForumPostValidator extends BdfMessageValidator {
}
// Verify the signature, if any
if (author != null) {
// Serialise the data to be signed
// Serialise the data to be verified
BdfList signed = BdfList.of(g.getId(), m.getTimestamp(), parent,
authorList, contentType, forumPostBody);
clientHelper.verifySignature(sig, author.getPublicKey(), signed);
try {
clientHelper
.verifySignature(sig, author.getPublicKey(), signed);
} catch (GeneralSecurityException e) {
throw new InvalidMessageException(e);
}
}
// Return the metadata and dependencies
BdfDictionary meta = new BdfDictionary();

View File

@@ -11,8 +11,8 @@ interface Constants {
String KEY_PARENT_MSG_ID = "parentMsgId";
String KEY_NEW_MEMBER_MSG_ID = "newMemberMsgId";
String KEY_PREVIOUS_MSG_ID = "previousMsgId";
String KEY_AUTHOR_ID = "authorId";
String KEY_AUTHOR_NAME = "authorName";
String KEY_AUTHOR_PUBLIC_KEY = "authorPublicKey";
String KEY_MEMBER_ID = "memberId";
String KEY_MEMBER_NAME = "memberName";
String KEY_MEMBER_PUBLIC_KEY = "memberPublicKey";
}

View File

@@ -36,14 +36,15 @@ class GroupMessageFactoryImpl implements GroupMessageFactory {
LocalAuthor creator, Author member) {
try {
// Generate the signature
BdfList toSign = BdfList.of(groupId, timestamp, member.getName(),
member.getPublicKey());
int type = NEW_MEMBER.getInt();
BdfList toSign = BdfList.of(groupId, timestamp, type,
member.getName(), member.getPublicKey());
byte[] signature =
clientHelper.sign(toSign, creator.getPrivateKey());
// Compose the message
BdfList body =
BdfList.of(NEW_MEMBER.getInt(), member.getName(),
BdfList.of(type, member.getName(),
member.getPublicKey(), signature);
Message m = clientHelper.createMessage(groupId, timestamp, body);
@@ -60,14 +61,15 @@ class GroupMessageFactoryImpl implements GroupMessageFactory {
LocalAuthor member, MessageId newMemberId) {
try {
// Generate the signature
BdfList toSign = BdfList.of(groupId, timestamp, member.getName(),
member.getPublicKey(), newMemberId);
int type = JOIN.getInt();
BdfList toSign = BdfList.of(groupId, timestamp, type,
member.getName(), member.getPublicKey(), newMemberId);
byte[] signature =
clientHelper.sign(toSign, member.getPrivateKey());
// Compose the message
BdfList body =
BdfList.of(JOIN.getInt(), member.getName(),
BdfList.of(type, member.getName(),
member.getPublicKey(), newMemberId, signature);
Message m = clientHelper.createMessage(groupId, timestamp, body);
@@ -85,14 +87,16 @@ class GroupMessageFactoryImpl implements GroupMessageFactory {
MessageId previousMsgId) {
try {
// Generate the signature
BdfList toSign = BdfList.of(groupId, timestamp, author.getName(),
author.getPublicKey(), parentId, previousMsgId, content);
int type = POST.getInt();
BdfList toSign = BdfList.of(groupId, timestamp, type,
author.getName(), author.getPublicKey(), parentId,
previousMsgId, content);
byte[] signature =
clientHelper.sign(toSign, author.getPrivateKey());
// Compose the message
BdfList body =
BdfList.of(POST.getInt(), author.getName(),
BdfList.of(type, author.getName(),
author.getPublicKey(), parentId, previousMsgId,
content, signature);
Message m = clientHelper.createMessage(groupId, timestamp, body);

View File

@@ -18,6 +18,7 @@ import org.briarproject.api.sync.MessageId;
import org.briarproject.api.system.Clock;
import org.briarproject.clients.BdfMessageValidator;
import java.security.GeneralSecurityException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
@@ -25,10 +26,13 @@ import java.util.Collections;
import static org.briarproject.api.identity.AuthorConstants.MAX_AUTHOR_NAME_LENGTH;
import static org.briarproject.api.identity.AuthorConstants.MAX_PUBLIC_KEY_LENGTH;
import static org.briarproject.api.identity.AuthorConstants.MAX_SIGNATURE_LENGTH;
import static org.briarproject.api.privategroup.MessageType.JOIN;
import static org.briarproject.api.privategroup.MessageType.NEW_MEMBER;
import static org.briarproject.api.privategroup.MessageType.POST;
import static org.briarproject.api.privategroup.PrivateGroupConstants.MAX_GROUP_POST_BODY_LENGTH;
import static org.briarproject.privategroup.Constants.KEY_AUTHOR_ID;
import static org.briarproject.privategroup.Constants.KEY_AUTHOR_NAME;
import static org.briarproject.privategroup.Constants.KEY_AUTHOR_PUBLIC_KEY;
import static org.briarproject.privategroup.Constants.KEY_MEMBER_ID;
import static org.briarproject.privategroup.Constants.KEY_MEMBER_NAME;
import static org.briarproject.privategroup.Constants.KEY_MEMBER_PUBLIC_KEY;
import static org.briarproject.privategroup.Constants.KEY_NEW_MEMBER_MSG_ID;
import static org.briarproject.privategroup.Constants.KEY_PARENT_MSG_ID;
import static org.briarproject.privategroup.Constants.KEY_PREVIOUS_MSG_ID;
@@ -60,29 +64,29 @@ class GroupMessageValidator extends BdfMessageValidator {
body.removeElementAt(0);
// member_name (string)
String member_name = body.getString(0);
checkLength(member_name, 1, MAX_AUTHOR_NAME_LENGTH);
String memberName = body.getString(0);
checkLength(memberName, 1, MAX_AUTHOR_NAME_LENGTH);
// member_public_key (raw)
byte[] member_public_key = body.getRaw(1);
checkLength(member_public_key, 1, MAX_PUBLIC_KEY_LENGTH);
byte[] memberPublicKey = body.getRaw(1);
checkLength(memberPublicKey, 1, MAX_PUBLIC_KEY_LENGTH);
BdfMessageContext c;
switch (MessageType.valueOf(type)) {
case NEW_MEMBER:
c = validateNewMember(m, g, body, member_name,
member_public_key);
addMessageMetadata(c, member_name, member_public_key,
c = validateNewMember(m, g, body, memberName,
memberPublicKey);
addMessageMetadata(c, memberName, memberPublicKey,
m.getTimestamp());
break;
case JOIN:
c = validateJoin(m, g, body, member_name, member_public_key);
addMessageMetadata(c, member_name, member_public_key,
c = validateJoin(m, g, body, memberName, memberPublicKey);
addMessageMetadata(c, memberName, memberPublicKey,
m.getTimestamp());
break;
case POST:
c = validatePost(m, g, body, member_name, member_public_key);
addMessageMetadata(c, member_name, member_public_key,
c = validatePost(m, g, body, memberName, memberPublicKey);
addMessageMetadata(c, memberName, memberPublicKey,
m.getTimestamp());
break;
default:
@@ -93,7 +97,7 @@ class GroupMessageValidator extends BdfMessageValidator {
}
private BdfMessageContext validateNewMember(Message m, Group g,
BdfList body, String member_name, byte[] member_public_key)
BdfList body, String memberName, byte[] memberPublicKey)
throws InvalidMessageException, FormatException {
// The content is a BDF list with three elements
@@ -105,11 +109,16 @@ class GroupMessageValidator extends BdfMessageValidator {
checkLength(signature, 1, MAX_SIGNATURE_LENGTH);
// Verify Signature
BdfList signed = BdfList.of(g.getId(), m.getTimestamp(), member_name,
member_public_key);
BdfList signed =
BdfList.of(g.getId(), m.getTimestamp(), NEW_MEMBER.getInt(),
memberName, memberPublicKey);
PrivateGroup group = groupFactory.parsePrivateGroup(g);
byte[] creatorPublicKey = group.getAuthor().getPublicKey();
clientHelper.verifySignature(signature, creatorPublicKey, signed);
try {
clientHelper.verifySignature(signature, creatorPublicKey, signed);
} catch (GeneralSecurityException e) {
throw new InvalidMessageException(e);
}
// Return the metadata and no dependencies
BdfDictionary meta = new BdfDictionary();
@@ -117,7 +126,7 @@ class GroupMessageValidator extends BdfMessageValidator {
}
private BdfMessageContext validateJoin(Message m, Group g, BdfList body,
String member_name, byte[] member_public_key)
String memberName, byte[] memberPublicKey)
throws InvalidMessageException, FormatException {
// The content is a BDF list with four elements
@@ -126,8 +135,8 @@ class GroupMessageValidator extends BdfMessageValidator {
// new_member_id (raw)
// the identifier of a new member message
// with the same member_name and member_public_key
byte[] new_member_id = body.getRaw(2);
checkLength(new_member_id, MessageId.LENGTH);
byte[] newMemberId = body.getRaw(2);
checkLength(newMemberId, MessageId.LENGTH);
// signature (raw)
// a signature with the member's private key over a list with 5 elements
@@ -135,22 +144,26 @@ class GroupMessageValidator extends BdfMessageValidator {
checkLength(signature, 1, MAX_SIGNATURE_LENGTH);
// Verify Signature
BdfList signed = BdfList.of(g.getId(), m.getTimestamp(), member_name,
member_public_key, new_member_id);
clientHelper.verifySignature(signature, member_public_key, signed);
BdfList signed = BdfList.of(g.getId(), m.getTimestamp(), JOIN.getInt(),
memberName, memberPublicKey, newMemberId);
try {
clientHelper.verifySignature(signature, memberPublicKey, signed);
} catch (GeneralSecurityException e) {
throw new InvalidMessageException(e);
}
// The new member message is a dependency
Collection<MessageId> dependencies =
Collections.singleton(new MessageId(new_member_id));
Collections.singleton(new MessageId(newMemberId));
// Return the metadata and dependencies
BdfDictionary meta = new BdfDictionary();
meta.put(KEY_NEW_MEMBER_MSG_ID, new_member_id);
meta.put(KEY_NEW_MEMBER_MSG_ID, newMemberId);
return new BdfMessageContext(meta, dependencies);
}
private BdfMessageContext validatePost(Message m, Group g, BdfList body,
String member_name, byte[] member_public_key)
String memberName, byte[] memberPublicKey)
throws InvalidMessageException, FormatException {
// The content is a BDF list with six elements
@@ -158,15 +171,13 @@ class GroupMessageValidator extends BdfMessageValidator {
// parent_id (raw or null)
// the identifier of the post to which this is a reply, if any
byte[] parent_id = body.getOptionalRaw(2);
if (parent_id != null) {
checkLength(parent_id, MessageId.LENGTH);
}
byte[] parentId = body.getOptionalRaw(2);
checkLength(parentId, MessageId.LENGTH);
// previous_message_id (raw)
// the identifier of the member's previous post or join message
byte[] previous_message_id = body.getRaw(3);
checkLength(previous_message_id, MessageId.LENGTH);
byte[] previousMessageId = body.getRaw(3);
checkLength(previousMessageId, MessageId.LENGTH);
// content (string)
String content = body.getString(4);
@@ -178,20 +189,25 @@ class GroupMessageValidator extends BdfMessageValidator {
checkLength(signature, 1, MAX_SIGNATURE_LENGTH);
// Verify Signature
BdfList signed = BdfList.of(g.getId(), m.getTimestamp(), member_name,
member_public_key, parent_id, previous_message_id, content);
clientHelper.verifySignature(signature, member_public_key, signed);
BdfList signed = BdfList.of(g.getId(), m.getTimestamp(), POST.getInt(),
memberName, memberPublicKey, parentId, previousMessageId,
content);
try {
clientHelper.verifySignature(signature, memberPublicKey, signed);
} catch (GeneralSecurityException e) {
throw new InvalidMessageException(e);
}
// The parent post, if any,
// and the member's previous message are dependencies
Collection<MessageId> dependencies = new ArrayList<MessageId>();
if (parent_id != null) dependencies.add(new MessageId(parent_id));
dependencies.add(new MessageId(previous_message_id));
if (parentId != null) dependencies.add(new MessageId(parentId));
dependencies.add(new MessageId(previousMessageId));
// Return the metadata and dependencies
BdfDictionary meta = new BdfDictionary();
if (parent_id != null) meta.put(KEY_PARENT_MSG_ID, parent_id);
meta.put(KEY_PREVIOUS_MSG_ID, previous_message_id);
if (parentId != null) meta.put(KEY_PARENT_MSG_ID, parentId);
meta.put(KEY_PREVIOUS_MSG_ID, previousMessageId);
return new BdfMessageContext(meta, dependencies);
}
@@ -200,9 +216,9 @@ class GroupMessageValidator extends BdfMessageValidator {
c.getDictionary().put(KEY_TIMESTAMP, time);
c.getDictionary().put(KEY_READ, false);
Author a = authorFactory.createAuthor(authorName, pubKey);
c.getDictionary().put(KEY_AUTHOR_ID, a.getId());
c.getDictionary().put(KEY_AUTHOR_NAME, authorName);
c.getDictionary().put(KEY_AUTHOR_PUBLIC_KEY, pubKey);
c.getDictionary().put(KEY_MEMBER_ID, a.getId());
c.getDictionary().put(KEY_MEMBER_NAME, authorName);
c.getDictionary().put(KEY_MEMBER_PUBLIC_KEY, pubKey);
}
}

View File

@@ -44,9 +44,9 @@ import static org.briarproject.api.identity.Author.Status.OURSELVES;
import static org.briarproject.api.privategroup.MessageType.JOIN;
import static org.briarproject.api.privategroup.MessageType.NEW_MEMBER;
import static org.briarproject.api.privategroup.MessageType.POST;
import static org.briarproject.privategroup.Constants.KEY_AUTHOR_ID;
import static org.briarproject.privategroup.Constants.KEY_AUTHOR_NAME;
import static org.briarproject.privategroup.Constants.KEY_AUTHOR_PUBLIC_KEY;
import static org.briarproject.privategroup.Constants.KEY_MEMBER_ID;
import static org.briarproject.privategroup.Constants.KEY_MEMBER_NAME;
import static org.briarproject.privategroup.Constants.KEY_MEMBER_PUBLIC_KEY;
import static org.briarproject.privategroup.Constants.KEY_NEW_MEMBER_MSG_ID;
import static org.briarproject.privategroup.Constants.KEY_PARENT_MSG_ID;
import static org.briarproject.privategroup.Constants.KEY_PREVIOUS_MSG_ID;
@@ -141,8 +141,7 @@ public class PrivateGroupManagerImpl extends BdfIncomingMessageHook implements
private MessageId getPreviousMsgId(Transaction txn, GroupId g)
throws DbException, FormatException {
BdfDictionary d = clientHelper.getGroupMetadataAsDictionary(txn, g);
byte[] previousMsgIdBytes = d.getOptionalRaw(KEY_PREVIOUS_MSG_ID);
if (previousMsgIdBytes == null) throw new DbException();
byte[] previousMsgIdBytes = d.getRaw(KEY_PREVIOUS_MSG_ID);
return new MessageId(previousMsgIdBytes);
}
@@ -191,9 +190,9 @@ public class PrivateGroupManagerImpl extends BdfIncomingMessageHook implements
boolean read) {
meta.put(KEY_TIMESTAMP, m.getMessage().getTimestamp());
meta.put(KEY_READ, read);
meta.put(KEY_AUTHOR_ID, m.getMember().getId());
meta.put(KEY_AUTHOR_NAME, m.getMember().getName());
meta.put(KEY_AUTHOR_PUBLIC_KEY, m.getMember().getPublicKey());
meta.put(KEY_MEMBER_ID, m.getMember().getId());
meta.put(KEY_MEMBER_NAME, m.getMember().getName());
meta.put(KEY_MEMBER_PUBLIC_KEY, m.getMember().getPublicKey());
}
@Override
@@ -269,11 +268,10 @@ public class PrivateGroupManagerImpl extends BdfIncomingMessageHook implements
clientHelper.getMessageMetadataAsDictionary(txn, g);
// get all authors we need to get the status for
Set<AuthorId> authors = new HashSet<AuthorId>();
for (Entry<MessageId, BdfDictionary> entry : metadata.entrySet()) {
BdfDictionary meta = entry.getValue();
for (BdfDictionary meta : metadata.values()) {
if (meta.getLong(KEY_TYPE) == NEW_MEMBER.getInt())
continue;
byte[] idBytes = meta.getRaw(KEY_AUTHOR_ID);
byte[] idBytes = meta.getRaw(KEY_MEMBER_ID);
authors.add(new AuthorId(idBytes));
}
// get statuses for all authors
@@ -308,9 +306,9 @@ public class PrivateGroupManagerImpl extends BdfIncomingMessageHook implements
}
long timestamp = meta.getLong(KEY_TIMESTAMP);
AuthorId authorId = new AuthorId(meta.getRaw(KEY_AUTHOR_ID));
String name = meta.getString(KEY_AUTHOR_NAME);
byte[] publicKey = meta.getRaw(KEY_AUTHOR_PUBLIC_KEY);
AuthorId authorId = new AuthorId(meta.getRaw(KEY_MEMBER_ID));
String name = meta.getString(KEY_MEMBER_NAME);
byte[] publicKey = meta.getRaw(KEY_MEMBER_PUBLIC_KEY);
Author author = new Author(authorId, name, publicKey);
Status status;
@@ -361,8 +359,8 @@ public class PrivateGroupManagerImpl extends BdfIncomingMessageHook implements
return false;
}
// NEW_MEMBER must have same member_name and member_public_key
if (!Arrays.equals(meta.getRaw(KEY_AUTHOR_ID),
newMemberMeta.getRaw(KEY_AUTHOR_ID))) {
if (!Arrays.equals(meta.getRaw(KEY_MEMBER_ID),
newMemberMeta.getRaw(KEY_MEMBER_ID))) {
// FIXME throw new InvalidMessageException() (#643)
db.deleteMessage(txn, m.getId());
return false;
@@ -401,8 +399,16 @@ public class PrivateGroupManagerImpl extends BdfIncomingMessageHook implements
return false;
}
// previous message must be from same member
if (!Arrays.equals(meta.getRaw(KEY_AUTHOR_ID),
previousMeta.getRaw(KEY_AUTHOR_ID))) {
if (!Arrays.equals(meta.getRaw(KEY_MEMBER_ID),
previousMeta.getRaw(KEY_MEMBER_ID))) {
// FIXME throw new InvalidMessageException() (#643)
db.deleteMessage(txn, m.getId());
return false;
}
// previous message must be a POST or JOIN
MessageType previousType = MessageType
.valueOf(previousMeta.getLong(KEY_TYPE).intValue());
if (previousType != JOIN && previousType != POST) {
// FIXME throw new InvalidMessageException() (#643)
db.deleteMessage(txn, m.getId());
return false;