From 8dc529cc3f5b987021422c2930f1e966bb49ed4d Mon Sep 17 00:00:00 2001 From: Torsten Grote <t@grobox.de> Date: Tue, 18 Oct 2016 13:35:27 -0200 Subject: [PATCH] Move validator's signature verification into ClientHelper --- .../api/clients/ClientHelper.java | 5 ++ .../briarproject/blogs/BlogPostValidator.java | 38 ++----------- .../org/briarproject/blogs/BlogsModule.java | 15 +++-- .../clients/ClientHelperImpl.java | 24 ++++++++ .../org/briarproject/forum/ForumModule.java | 10 ++-- .../forum/ForumPostValidator.java | 33 ++--------- .../blogs/BlogPostValidatorTest.java | 55 ++++--------------- 7 files changed, 64 insertions(+), 116 deletions(-) diff --git a/briar-api/src/org/briarproject/api/clients/ClientHelper.java b/briar-api/src/org/briarproject/api/clients/ClientHelper.java index 5aeffaa21e..4622431d40 100644 --- a/briar-api/src/org/briarproject/api/clients/ClientHelper.java +++ b/briar-api/src/org/briarproject/api/clients/ClientHelper.java @@ -6,6 +6,7 @@ import org.briarproject.api.data.BdfList; import org.briarproject.api.db.DbException; 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.MessageId; @@ -81,4 +82,8 @@ public interface ClientHelper { byte[] sign(BdfList toSign, byte[] privateKey) throws FormatException, GeneralSecurityException; + + void verifySignature(byte[] sig, byte[] publicKey, BdfList signed) + throws InvalidMessageException; + } diff --git a/briar-core/src/org/briarproject/blogs/BlogPostValidator.java b/briar-core/src/org/briarproject/blogs/BlogPostValidator.java index 29908c6d3d..75b4797d0a 100644 --- a/briar-core/src/org/briarproject/blogs/BlogPostValidator.java +++ b/briar-core/src/org/briarproject/blogs/BlogPostValidator.java @@ -6,10 +6,6 @@ import org.briarproject.api.blogs.BlogFactory; import org.briarproject.api.blogs.MessageType; import org.briarproject.api.clients.BdfMessageContext; import org.briarproject.api.clients.ClientHelper; -import org.briarproject.api.crypto.CryptoComponent; -import org.briarproject.api.crypto.KeyParser; -import org.briarproject.api.crypto.PublicKey; -import org.briarproject.api.crypto.Signature; import org.briarproject.api.data.BdfDictionary; import org.briarproject.api.data.BdfEntry; import org.briarproject.api.data.BdfList; @@ -24,7 +20,6 @@ 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; @@ -48,18 +43,15 @@ import static org.briarproject.api.identity.AuthorConstants.MAX_SIGNATURE_LENGTH class BlogPostValidator extends BdfMessageValidator { - private final CryptoComponent crypto; private final GroupFactory groupFactory; private final MessageFactory messageFactory; private final BlogFactory blogFactory; - BlogPostValidator(CryptoComponent crypto, GroupFactory groupFactory, - MessageFactory messageFactory, BlogFactory blogFactory, - ClientHelper clientHelper, MetadataEncoder metadataEncoder, - Clock clock) { + BlogPostValidator(GroupFactory groupFactory, MessageFactory messageFactory, + BlogFactory blogFactory, ClientHelper clientHelper, + MetadataEncoder metadataEncoder, Clock clock) { super(clientHelper, metadataEncoder, clock); - this.crypto = crypto; this.groupFactory = groupFactory; this.messageFactory = messageFactory; this.blogFactory = blogFactory; @@ -109,7 +101,7 @@ 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(); - verifySignature(sig, a.getPublicKey(), signed); + clientHelper.verifySignature(sig, a.getPublicKey(), signed); // Return the metadata and dependencies BdfDictionary meta = new BdfDictionary(); @@ -150,7 +142,7 @@ class BlogPostValidator extends BdfMessageValidator { currentId); Blog b = blogFactory.parseBlog(g, ""); // description doesn't matter Author a = b.getAuthor(); - verifySignature(sig, a.getPublicKey(), signed); + clientHelper.verifySignature(sig, a.getPublicKey(), signed); // Return the metadata and dependencies BdfDictionary meta = new BdfDictionary(); @@ -267,26 +259,6 @@ class BlogPostValidator extends BdfMessageValidator { return new BdfMessageContext(meta, dependencies); } - private void verifySignature(byte[] sig, byte[] publicKey, BdfList signed) - throws InvalidMessageException { - try { - // Parse the public key - KeyParser keyParser = crypto.getSignatureKeyParser(); - PublicKey key = keyParser.parsePublicKey(publicKey); - // Verify the signature - Signature signature = crypto.getSignature(); - signature.initVerify(key); - signature.update(clientHelper.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); - } - } - static BdfDictionary authorToBdfDictionary(Author a) { return BdfDictionary.of( new BdfEntry(KEY_AUTHOR_ID, a.getId()), diff --git a/briar-core/src/org/briarproject/blogs/BlogsModule.java b/briar-core/src/org/briarproject/blogs/BlogsModule.java index 91cc68c7c5..e8927d5afc 100644 --- a/briar-core/src/org/briarproject/blogs/BlogsModule.java +++ b/briar-core/src/org/briarproject/blogs/BlogsModule.java @@ -5,7 +5,6 @@ import org.briarproject.api.blogs.BlogManager; import org.briarproject.api.blogs.BlogPostFactory; import org.briarproject.api.clients.ClientHelper; import org.briarproject.api.contact.ContactManager; -import org.briarproject.api.crypto.CryptoComponent; import org.briarproject.api.data.MetadataEncoder; import org.briarproject.api.identity.AuthorFactory; import org.briarproject.api.identity.IdentityManager; @@ -64,14 +63,14 @@ public class BlogsModule { @Provides @Singleton BlogPostValidator provideBlogPostValidator( - ValidationManager validationManager, CryptoComponent crypto, - GroupFactory groupFactory, MessageFactory messageFactory, - BlogFactory blogFactory, ClientHelper clientHelper, - MetadataEncoder metadataEncoder, Clock clock) { + ValidationManager validationManager, GroupFactory groupFactory, + MessageFactory messageFactory, BlogFactory blogFactory, + ClientHelper clientHelper, MetadataEncoder metadataEncoder, + Clock clock) { - BlogPostValidator validator = new BlogPostValidator(crypto, - groupFactory, messageFactory, blogFactory, clientHelper, - metadataEncoder, clock); + BlogPostValidator validator = new BlogPostValidator(groupFactory, + messageFactory, blogFactory, clientHelper, metadataEncoder, + clock); validationManager.registerMessageValidator(CLIENT_ID, validator); return validator; diff --git a/briar-core/src/org/briarproject/clients/ClientHelperImpl.java b/briar-core/src/org/briarproject/clients/ClientHelperImpl.java index d7a983b0ae..612f80e815 100644 --- a/briar-core/src/org/briarproject/clients/ClientHelperImpl.java +++ b/briar-core/src/org/briarproject/clients/ClientHelperImpl.java @@ -5,6 +5,7 @@ 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.PublicKey; import org.briarproject.api.crypto.Signature; import org.briarproject.api.data.BdfDictionary; import org.briarproject.api.data.BdfList; @@ -19,6 +20,7 @@ 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; @@ -320,4 +322,26 @@ class ClientHelperImpl implements ClientHelper { signature.update(toByteArray(toSign)); return signature.sign(); } + + @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); + } + } + } diff --git a/briar-core/src/org/briarproject/forum/ForumModule.java b/briar-core/src/org/briarproject/forum/ForumModule.java index 40bfd01381..eeafb54dfc 100644 --- a/briar-core/src/org/briarproject/forum/ForumModule.java +++ b/briar-core/src/org/briarproject/forum/ForumModule.java @@ -54,11 +54,11 @@ public class ForumModule { @Provides @Singleton ForumPostValidator provideForumPostValidator( - ValidationManager validationManager, CryptoComponent crypto, - AuthorFactory authorFactory, ClientHelper clientHelper, - MetadataEncoder metadataEncoder, Clock clock) { - ForumPostValidator validator = new ForumPostValidator(crypto, - authorFactory, clientHelper, metadataEncoder, clock); + ValidationManager validationManager, AuthorFactory authorFactory, + ClientHelper clientHelper, MetadataEncoder metadataEncoder, + Clock clock) { + ForumPostValidator validator = new ForumPostValidator(authorFactory, + clientHelper, metadataEncoder, clock); validationManager.registerMessageValidator( ForumManagerImpl.CLIENT_ID, validator); return validator; diff --git a/briar-core/src/org/briarproject/forum/ForumPostValidator.java b/briar-core/src/org/briarproject/forum/ForumPostValidator.java index 258ac9bb5f..f25181f1ce 100644 --- a/briar-core/src/org/briarproject/forum/ForumPostValidator.java +++ b/briar-core/src/org/briarproject/forum/ForumPostValidator.java @@ -4,10 +4,6 @@ import org.briarproject.api.FormatException; import org.briarproject.api.UniqueId; import org.briarproject.api.clients.BdfMessageContext; import org.briarproject.api.clients.ClientHelper; -import org.briarproject.api.crypto.CryptoComponent; -import org.briarproject.api.crypto.KeyParser; -import org.briarproject.api.crypto.PublicKey; -import org.briarproject.api.crypto.Signature; import org.briarproject.api.data.BdfDictionary; import org.briarproject.api.data.BdfList; import org.briarproject.api.data.MetadataEncoder; @@ -20,7 +16,6 @@ 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; @@ -32,14 +27,11 @@ import static org.briarproject.api.identity.AuthorConstants.MAX_SIGNATURE_LENGTH class ForumPostValidator extends BdfMessageValidator { - private final CryptoComponent crypto; private final AuthorFactory authorFactory; - ForumPostValidator(CryptoComponent crypto, AuthorFactory authorFactory, - ClientHelper clientHelper, MetadataEncoder metadataEncoder, - Clock clock) { + ForumPostValidator(AuthorFactory authorFactory, ClientHelper clientHelper, + MetadataEncoder metadataEncoder, Clock clock) { super(clientHelper, metadataEncoder, clock); - this.crypto = crypto; this.authorFactory = authorFactory; } @@ -81,23 +73,10 @@ class ForumPostValidator extends BdfMessageValidator { } // Verify the signature, if any if (author != null) { - try { - // Parse the public key - KeyParser keyParser = crypto.getSignatureKeyParser(); - PublicKey key = keyParser.parsePublicKey(author.getPublicKey()); - // Serialise the data to be signed - BdfList signed = BdfList.of(g.getId(), m.getTimestamp(), parent, - authorList, contentType, forumPostBody); - // Verify the signature - Signature signature = crypto.getSignature(); - signature.initVerify(key); - signature.update(clientHelper.toByteArray(signed)); - if (!signature.verify(sig)) { - throw new InvalidMessageException("Invalid signature"); - } - } catch (GeneralSecurityException e) { - throw new InvalidMessageException("Invalid public key"); - } + // Serialise the data to be signed + BdfList signed = BdfList.of(g.getId(), m.getTimestamp(), parent, + authorList, contentType, forumPostBody); + clientHelper.verifySignature(sig, author.getPublicKey(), signed); } // Return the metadata and dependencies BdfDictionary meta = new BdfDictionary(); diff --git a/briar-tests/src/org/briarproject/blogs/BlogPostValidatorTest.java b/briar-tests/src/org/briarproject/blogs/BlogPostValidatorTest.java index bdbcc2398c..afb1b90b9f 100644 --- a/briar-tests/src/org/briarproject/blogs/BlogPostValidatorTest.java +++ b/briar-tests/src/org/briarproject/blogs/BlogPostValidatorTest.java @@ -7,9 +7,6 @@ import org.briarproject.api.blogs.Blog; import org.briarproject.api.blogs.BlogFactory; import org.briarproject.api.clients.ClientHelper; import org.briarproject.api.crypto.CryptoComponent; -import org.briarproject.api.crypto.KeyParser; -import org.briarproject.api.crypto.PublicKey; -import org.briarproject.api.crypto.Signature; import org.briarproject.api.data.BdfDictionary; import org.briarproject.api.data.BdfEntry; import org.briarproject.api.data.BdfList; @@ -20,7 +17,6 @@ import org.briarproject.api.sync.ClientId; import org.briarproject.api.sync.Group; import org.briarproject.api.sync.GroupFactory; 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; @@ -37,9 +33,9 @@ import static org.briarproject.api.blogs.BlogConstants.KEY_AUTHOR; import static org.briarproject.api.blogs.BlogConstants.KEY_AUTHOR_ID; import static org.briarproject.api.blogs.BlogConstants.KEY_AUTHOR_NAME; import static org.briarproject.api.blogs.BlogConstants.KEY_COMMENT; +import static org.briarproject.api.blogs.BlogConstants.KEY_ORIGINAL_MSG_ID; import static org.briarproject.api.blogs.BlogConstants.KEY_ORIGINAL_PARENT_MSG_ID; import static org.briarproject.api.blogs.BlogConstants.KEY_PARENT_MSG_ID; -import static org.briarproject.api.blogs.BlogConstants.KEY_ORIGINAL_MSG_ID; import static org.briarproject.api.blogs.BlogConstants.KEY_PUBLIC_KEY; import static org.briarproject.api.blogs.BlogConstants.KEY_READ; import static org.briarproject.api.blogs.MessageType.COMMENT; @@ -94,9 +90,8 @@ public class BlogPostValidatorTest extends BriarTestCase { message = new Message(messageId, group.getId(), timestamp, raw); MetadataEncoder metadataEncoder = context.mock(MetadataEncoder.class); - validator = new BlogPostValidator(cryptoComponent, groupFactory, - messageFactory, blogFactory, clientHelper, metadataEncoder, - clock); + validator = new BlogPostValidator(groupFactory, messageFactory, + blogFactory, clientHelper, metadataEncoder, clock); context.assertIsSatisfied(); } @@ -108,7 +103,7 @@ public class BlogPostValidatorTest extends BriarTestCase { BdfList signed = BdfList.of(blog.getId(), message.getTimestamp(), body); - expectCrypto(signed, sigBytes, true); + expectCrypto(signed, sigBytes); final BdfDictionary result = validator.validateMessage(message, group, m).getDictionary(); @@ -135,18 +130,6 @@ public class BlogPostValidatorTest extends BriarTestCase { validator.validateMessage(message, group, m).getDictionary(); } - @Test(expected = InvalidMessageException.class) - public void testValidateBlogPostWithBadSignature() - throws IOException, GeneralSecurityException { - final byte[] sigBytes = TestUtils.getRandomBytes(42); - BdfList m = BdfList.of(POST.getInt(), body, sigBytes); - - BdfList signed = - BdfList.of(blog.getId(), message.getTimestamp(), body); - expectCrypto(signed, sigBytes, false); - validator.validateMessage(message, group, m).getDictionary(); - } - @Test public void testValidateProperBlogComment() throws IOException, GeneralSecurityException { @@ -162,7 +145,7 @@ public class BlogPostValidatorTest extends BriarTestCase { BdfList signed = BdfList.of(blog.getId(), message.getTimestamp(), comment, pOriginalId, currentId); - expectCrypto(signed, sigBytes, true); + expectCrypto(signed, sigBytes); final BdfDictionary result = validator.validateMessage(message, group, m).getDictionary(); @@ -189,7 +172,7 @@ public class BlogPostValidatorTest extends BriarTestCase { BdfList signed = BdfList.of(blog.getId(), message.getTimestamp(), null, originalId, currentId); - expectCrypto(signed, sigBytes, true); + expectCrypto(signed, sigBytes); final BdfDictionary result = validator.validateMessage(message, group, m).getDictionary(); @@ -208,7 +191,7 @@ public class BlogPostValidatorTest extends BriarTestCase { BdfList signed = BdfList.of(blog.getId(), message.getTimestamp(), body); - expectCrypto(signed, sigBytes, true); + expectCrypto(signed, sigBytes); final BdfList originalList = BdfList.of(POST.getInt(), body, sigBytes); final byte[] originalBody = TestUtils.getRandomBytes(42); @@ -247,7 +230,7 @@ public class BlogPostValidatorTest extends BriarTestCase { BdfList signed = BdfList.of(blog.getId(), message.getTimestamp(), comment, originalId, oldId); - expectCrypto(signed, sigBytes, true); + expectCrypto(signed, sigBytes); final BdfList originalList = BdfList.of(COMMENT.getInt(), comment, originalId, oldId, sigBytes); @@ -275,27 +258,13 @@ public class BlogPostValidatorTest extends BriarTestCase { context.assertIsSatisfied(); } - private void expectCrypto(final BdfList signed, final byte[] sig, - final boolean pass) throws IOException, GeneralSecurityException { - final Signature signature = context.mock(Signature.class); - final KeyParser keyParser = context.mock(KeyParser.class); - final PublicKey publicKey = context.mock(PublicKey.class); - + private void expectCrypto(final BdfList signed, final byte[] sig) + throws IOException, GeneralSecurityException { context.checking(new Expectations() {{ oneOf(blogFactory).parseBlog(group, ""); will(returnValue(blog)); - oneOf(cryptoComponent).getSignatureKeyParser(); - will(returnValue(keyParser)); - oneOf(keyParser).parsePublicKey(blog.getAuthor().getPublicKey()); - will(returnValue(publicKey)); - oneOf(cryptoComponent).getSignature(); - will(returnValue(signature)); - oneOf(signature).initVerify(publicKey); - oneOf(clientHelper).toByteArray(signed); - will(returnValue(sig)); - oneOf(signature).update(sig); - oneOf(signature).verify(sig); - will(returnValue(pass)); + oneOf(clientHelper) + .verifySignature(sig, author.getPublicKey(), signed); }}); } -- GitLab