Transactional version of AttachmentReader#getAttachment()

This commit is contained in:
Sebastian Kürten
2021-12-17 18:10:48 +01:00
parent 20b52804bf
commit d2a39da3e0
3 changed files with 54 additions and 13 deletions

View File

@@ -2,6 +2,7 @@ package org.briarproject.briar.api.attachment;
import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.db.DbException;
import org.briarproject.bramble.api.db.NoSuchMessageException; import org.briarproject.bramble.api.db.NoSuchMessageException;
import org.briarproject.bramble.api.db.Transaction;
public interface AttachmentReader { public interface AttachmentReader {
@@ -17,4 +18,17 @@ public interface AttachmentReader {
*/ */
Attachment getAttachment(AttachmentHeader h) throws DbException; 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;
} }

View File

@@ -5,6 +5,8 @@ import org.briarproject.bramble.api.client.ClientHelper;
import org.briarproject.bramble.api.data.BdfDictionary; import org.briarproject.bramble.api.data.BdfDictionary;
import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.db.DbException;
import org.briarproject.bramble.api.db.NoSuchMessageException; 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.Message;
import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.bramble.api.sync.MessageId;
import org.briarproject.briar.api.attachment.Attachment; 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 { public class AttachmentReaderImpl implements AttachmentReader {
private final TransactionManager db;
private final ClientHelper clientHelper; private final ClientHelper clientHelper;
@Inject @Inject
public AttachmentReaderImpl(ClientHelper clientHelper) { public AttachmentReaderImpl(TransactionManager db,
ClientHelper clientHelper) {
this.db = db;
this.clientHelper = clientHelper; this.clientHelper = clientHelper;
} }
@Override @Override
public Attachment getAttachment(AttachmentHeader h) throws DbException { 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 // TODO: Support large messages
MessageId m = h.getMessageId(); 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 // Check that the message is in the expected group, to prevent it from
// being loaded in the context of a different group // being loaded in the context of a different group
if (!message.getGroupId().equals(h.getGroupId())) { if (!message.getGroupId().equals(h.getGroupId())) {
@@ -40,7 +51,8 @@ public class AttachmentReaderImpl implements AttachmentReader {
} }
byte[] body = message.getBody(); byte[] body = message.getBody();
try { try {
BdfDictionary meta = clientHelper.getMessageMetadataAsDictionary(m); BdfDictionary meta =
clientHelper.getMessageMetadataAsDictionary(txn, m);
String contentType = meta.getString(MSG_KEY_CONTENT_TYPE); String contentType = meta.getString(MSG_KEY_CONTENT_TYPE);
if (!contentType.equals(h.getContentType())) if (!contentType.equals(h.getContentType()))
throw new NoSuchMessageException(); throw new NoSuchMessageException();

View File

@@ -3,13 +3,16 @@ package org.briarproject.briar.attachment;
import org.briarproject.bramble.api.client.ClientHelper; import org.briarproject.bramble.api.client.ClientHelper;
import org.briarproject.bramble.api.data.BdfDictionary; import org.briarproject.bramble.api.data.BdfDictionary;
import org.briarproject.bramble.api.data.BdfEntry; 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.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.GroupId;
import org.briarproject.bramble.api.sync.Message; import org.briarproject.bramble.api.sync.Message;
import org.briarproject.bramble.test.BrambleMockTestCase; 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.Attachment;
import org.briarproject.briar.api.attachment.AttachmentHeader; import org.briarproject.briar.api.attachment.AttachmentHeader;
import org.jmock.Expectations;
import org.junit.Test; import org.junit.Test;
import java.io.ByteArrayOutputStream; import java.io.ByteArrayOutputStream;
@@ -25,6 +28,7 @@ import static org.junit.Assert.assertArrayEquals;
public class AttachmentReaderImplTest extends BrambleMockTestCase { public class AttachmentReaderImplTest extends BrambleMockTestCase {
private final TransactionManager db = context.mock(DatabaseComponent.class);
private final ClientHelper clientHelper = context.mock(ClientHelper.class); private final ClientHelper clientHelper = context.mock(ClientHelper.class);
private final GroupId groupId = new GroupId(getRandomId()); private final GroupId groupId = new GroupId(getRandomId());
@@ -34,7 +38,7 @@ public class AttachmentReaderImplTest extends BrambleMockTestCase {
message.getId(), contentType); message.getId(), contentType);
private final AttachmentReaderImpl attachmentReader = private final AttachmentReaderImpl attachmentReader =
new AttachmentReaderImpl(clientHelper); new AttachmentReaderImpl(db, clientHelper);
@Test(expected = NoSuchMessageException.class) @Test(expected = NoSuchMessageException.class)
public void testWrongGroup() throws Exception { public void testWrongGroup() throws Exception {
@@ -42,8 +46,11 @@ public class AttachmentReaderImplTest extends BrambleMockTestCase {
AttachmentHeader wrongGroup = new AttachmentHeader(wrongGroupId, AttachmentHeader wrongGroup = new AttachmentHeader(wrongGroupId,
message.getId(), contentType); message.getId(), contentType);
context.checking(new Expectations() {{ Transaction txn = new Transaction(null, true);
oneOf(clientHelper).getMessage(message.getId());
context.checking(new DbExpectations() {{
oneOf(db).transactionWithResult(with(true), withDbCallable(txn));
oneOf(clientHelper).getMessage(txn, message.getId());
will(returnValue(message)); will(returnValue(message));
}}); }});
@@ -74,10 +81,14 @@ public class AttachmentReaderImplTest extends BrambleMockTestCase {
} }
private void testInvalidMetadata(BdfDictionary meta) throws Exception { private void testInvalidMetadata(BdfDictionary meta) throws Exception {
context.checking(new Expectations() {{ Transaction txn = new Transaction(null, true);
oneOf(clientHelper).getMessage(message.getId());
context.checking(new DbExpectations() {{
oneOf(db).transactionWithResult(with(true), withDbCallable(txn));
oneOf(clientHelper).getMessage(txn, message.getId());
will(returnValue(message)); will(returnValue(message));
oneOf(clientHelper).getMessageMetadataAsDictionary(message.getId()); oneOf(clientHelper)
.getMessageMetadataAsDictionary(txn, message.getId());
will(returnValue(meta)); will(returnValue(meta));
}}); }});
@@ -95,10 +106,14 @@ public class AttachmentReaderImplTest extends BrambleMockTestCase {
byte[] expectedData = new byte[body.length - descriptorLength]; byte[] expectedData = new byte[body.length - descriptorLength];
arraycopy(body, descriptorLength, expectedData, 0, expectedData.length); arraycopy(body, descriptorLength, expectedData, 0, expectedData.length);
context.checking(new Expectations() {{ Transaction txn = new Transaction(null, true);
oneOf(clientHelper).getMessage(message.getId());
context.checking(new DbExpectations() {{
oneOf(db).transactionWithResult(with(true), withDbCallable(txn));
oneOf(clientHelper).getMessage(txn, message.getId());
will(returnValue(message)); will(returnValue(message));
oneOf(clientHelper).getMessageMetadataAsDictionary(message.getId()); oneOf(clientHelper)
.getMessageMetadataAsDictionary(txn, message.getId());
will(returnValue(meta)); will(returnValue(meta));
}}); }});