diff --git a/briar-api/src/main/java/org/briarproject/briar/api/attachment/AttachmentReader.java b/briar-api/src/main/java/org/briarproject/briar/api/attachment/AttachmentReader.java index 378c513c4..e2d5222f9 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/attachment/AttachmentReader.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/attachment/AttachmentReader.java @@ -2,6 +2,7 @@ package org.briarproject.briar.api.attachment; import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.db.NoSuchMessageException; +import org.briarproject.bramble.api.db.Transaction; public interface AttachmentReader { @@ -17,4 +18,17 @@ public interface AttachmentReader { */ Attachment getAttachment(AttachmentHeader h) throws DbException; + /** + * Returns the attachment with the given attachment header. + * + * @throws NoSuchMessageException If the header refers to a message in + * a different group from the one specified in the header, to a message + * that is not an attachment, or to an attachment that does not have the + * expected content type. This is meant to prevent social engineering + * attacks that use invalid attachment IDs to test whether messages exist + * in the victim's database + */ + Attachment getAttachment(Transaction txn, AttachmentHeader h) + throws DbException; + } diff --git a/briar-core/src/main/java/org/briarproject/briar/attachment/AttachmentReaderImpl.java b/briar-core/src/main/java/org/briarproject/briar/attachment/AttachmentReaderImpl.java index e61918cfe..69d5f963f 100644 --- a/briar-core/src/main/java/org/briarproject/briar/attachment/AttachmentReaderImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/attachment/AttachmentReaderImpl.java @@ -5,6 +5,8 @@ import org.briarproject.bramble.api.client.ClientHelper; import org.briarproject.bramble.api.data.BdfDictionary; import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.db.NoSuchMessageException; +import org.briarproject.bramble.api.db.Transaction; +import org.briarproject.bramble.api.db.TransactionManager; import org.briarproject.bramble.api.sync.Message; import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.briar.api.attachment.Attachment; @@ -21,18 +23,27 @@ import static org.briarproject.briar.api.attachment.MediaConstants.MSG_KEY_DESCR public class AttachmentReaderImpl implements AttachmentReader { + private final TransactionManager db; private final ClientHelper clientHelper; @Inject - public AttachmentReaderImpl(ClientHelper clientHelper) { + public AttachmentReaderImpl(TransactionManager db, + ClientHelper clientHelper) { + this.db = db; this.clientHelper = clientHelper; } @Override public Attachment getAttachment(AttachmentHeader h) throws DbException { + return db.transactionWithResult(true, txn -> getAttachment(txn, h)); + } + + @Override + public Attachment getAttachment(Transaction txn, AttachmentHeader h) + throws DbException { // TODO: Support large messages MessageId m = h.getMessageId(); - Message message = clientHelper.getMessage(m); + Message message = clientHelper.getMessage(txn, m); // Check that the message is in the expected group, to prevent it from // being loaded in the context of a different group if (!message.getGroupId().equals(h.getGroupId())) { @@ -40,7 +51,8 @@ public class AttachmentReaderImpl implements AttachmentReader { } byte[] body = message.getBody(); try { - BdfDictionary meta = clientHelper.getMessageMetadataAsDictionary(m); + BdfDictionary meta = + clientHelper.getMessageMetadataAsDictionary(txn, m); String contentType = meta.getString(MSG_KEY_CONTENT_TYPE); if (!contentType.equals(h.getContentType())) throw new NoSuchMessageException(); diff --git a/briar-core/src/test/java/org/briarproject/briar/attachment/AttachmentReaderImplTest.java b/briar-core/src/test/java/org/briarproject/briar/attachment/AttachmentReaderImplTest.java index 9d21221a0..b0c165313 100644 --- a/briar-core/src/test/java/org/briarproject/briar/attachment/AttachmentReaderImplTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/attachment/AttachmentReaderImplTest.java @@ -3,13 +3,16 @@ package org.briarproject.briar.attachment; import org.briarproject.bramble.api.client.ClientHelper; import org.briarproject.bramble.api.data.BdfDictionary; import org.briarproject.bramble.api.data.BdfEntry; +import org.briarproject.bramble.api.db.DatabaseComponent; import org.briarproject.bramble.api.db.NoSuchMessageException; +import org.briarproject.bramble.api.db.Transaction; +import org.briarproject.bramble.api.db.TransactionManager; import org.briarproject.bramble.api.sync.GroupId; import org.briarproject.bramble.api.sync.Message; import org.briarproject.bramble.test.BrambleMockTestCase; +import org.briarproject.bramble.test.DbExpectations; import org.briarproject.briar.api.attachment.Attachment; import org.briarproject.briar.api.attachment.AttachmentHeader; -import org.jmock.Expectations; import org.junit.Test; import java.io.ByteArrayOutputStream; @@ -25,6 +28,7 @@ import static org.junit.Assert.assertArrayEquals; public class AttachmentReaderImplTest extends BrambleMockTestCase { + private final TransactionManager db = context.mock(DatabaseComponent.class); private final ClientHelper clientHelper = context.mock(ClientHelper.class); private final GroupId groupId = new GroupId(getRandomId()); @@ -34,7 +38,7 @@ public class AttachmentReaderImplTest extends BrambleMockTestCase { message.getId(), contentType); private final AttachmentReaderImpl attachmentReader = - new AttachmentReaderImpl(clientHelper); + new AttachmentReaderImpl(db, clientHelper); @Test(expected = NoSuchMessageException.class) public void testWrongGroup() throws Exception { @@ -42,8 +46,11 @@ public class AttachmentReaderImplTest extends BrambleMockTestCase { AttachmentHeader wrongGroup = new AttachmentHeader(wrongGroupId, message.getId(), contentType); - context.checking(new Expectations() {{ - oneOf(clientHelper).getMessage(message.getId()); + Transaction txn = new Transaction(null, true); + + context.checking(new DbExpectations() {{ + oneOf(db).transactionWithResult(with(true), withDbCallable(txn)); + oneOf(clientHelper).getMessage(txn, message.getId()); will(returnValue(message)); }}); @@ -74,10 +81,14 @@ public class AttachmentReaderImplTest extends BrambleMockTestCase { } private void testInvalidMetadata(BdfDictionary meta) throws Exception { - context.checking(new Expectations() {{ - oneOf(clientHelper).getMessage(message.getId()); + Transaction txn = new Transaction(null, true); + + context.checking(new DbExpectations() {{ + oneOf(db).transactionWithResult(with(true), withDbCallable(txn)); + oneOf(clientHelper).getMessage(txn, message.getId()); will(returnValue(message)); - oneOf(clientHelper).getMessageMetadataAsDictionary(message.getId()); + oneOf(clientHelper) + .getMessageMetadataAsDictionary(txn, message.getId()); will(returnValue(meta)); }}); @@ -95,10 +106,14 @@ public class AttachmentReaderImplTest extends BrambleMockTestCase { byte[] expectedData = new byte[body.length - descriptorLength]; arraycopy(body, descriptorLength, expectedData, 0, expectedData.length); - context.checking(new Expectations() {{ - oneOf(clientHelper).getMessage(message.getId()); + Transaction txn = new Transaction(null, true); + + context.checking(new DbExpectations() {{ + oneOf(db).transactionWithResult(with(true), withDbCallable(txn)); + oneOf(clientHelper).getMessage(txn, message.getId()); will(returnValue(message)); - oneOf(clientHelper).getMessageMetadataAsDictionary(message.getId()); + oneOf(clientHelper) + .getMessageMetadataAsDictionary(txn, message.getId()); will(returnValue(meta)); }});