Merge branch '1912-specify-group-id-when-loading-attachment' into '214-user-avatars'

Ensure that attachment has expected group ID when loading

See merge request briar/briar!1347

briar-android/src/androidTestOfficial/java/org/briarproject/briar/android/attachment/AttachmentRetrieverIntegrationTest.java

@@ -1,6 +1,7 @@
 package org.briarproject.briar.android.attachment;
 
 import org.briarproject.bramble.api.UniqueId;
+import org.briarproject.bramble.api.sync.GroupId;
 import org.briarproject.bramble.api.sync.MessageId;
 import org.briarproject.briar.android.attachment.media.ImageHelper;
 import org.briarproject.briar.android.attachment.media.ImageSizeCalculator;
@@ -28,6 +29,7 @@ public class AttachmentRetrieverIntegrationTest {
 	private final AttachmentDimensions dimensions = new AttachmentDimensions(
 			100, 50, 200, 75, 300
 	);
+	private final GroupId groupId = new GroupId(getRandomId());
 	private final MessageId msgId = new MessageId(getRandomId());
 
 	@Inject
@@ -48,7 +50,7 @@ public class AttachmentRetrieverIntegrationTest {
 
 	@Test
 	public void testSmallJpegImage() throws Exception {
-		AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg");
+		AttachmentHeader h = new AttachmentHeader(groupId, msgId, "image/jpeg");
 		InputStream is = getAssetInputStream("kitten_small.jpg");
 		Attachment a = new Attachment(h, is);
 		AttachmentItem item = retriever.createAttachmentItem(a, true);
@@ -64,7 +66,7 @@ public class AttachmentRetrieverIntegrationTest {
 
 	@Test
 	public void testBigJpegImage() throws Exception {
-		AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg");
+		AttachmentHeader h = new AttachmentHeader(groupId, msgId, "image/jpeg");
 		InputStream is = getAssetInputStream("kitten_original.jpg");
 		Attachment a = new Attachment(h, is);
 		AttachmentItem item = retriever.createAttachmentItem(a, true);
@@ -80,7 +82,7 @@ public class AttachmentRetrieverIntegrationTest {
 
 	@Test
 	public void testSmallPngImage() throws Exception {
-		AttachmentHeader h = new AttachmentHeader(msgId, "image/png");
+		AttachmentHeader h = new AttachmentHeader(groupId, msgId, "image/png");
 		InputStream is = getAssetInputStream("kitten.png");
 		Attachment a = new Attachment(h, is);
 		AttachmentItem item = retriever.createAttachmentItem(a, true);
@@ -96,7 +98,7 @@ public class AttachmentRetrieverIntegrationTest {
 
 	@Test
 	public void testUberGif() throws Exception {
-		AttachmentHeader h = new AttachmentHeader(msgId, "image/gif");
+		AttachmentHeader h = new AttachmentHeader(groupId, msgId, "image/gif");
 		InputStream is = getAssetInputStream("uber.gif");
 		Attachment a = new Attachment(h, is);
 		AttachmentItem item = retriever.createAttachmentItem(a, true);
@@ -111,7 +113,7 @@ public class AttachmentRetrieverIntegrationTest {
 
 	@Test
 	public void testLottaPixels() throws Exception {
-		AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg");
+		AttachmentHeader h = new AttachmentHeader(groupId, msgId, "image/jpeg");
 		InputStream is = getAssetInputStream("lottapixel.jpg");
 		Attachment a = new Attachment(h, is);
 		AttachmentItem item = retriever.createAttachmentItem(a, true);
@@ -126,7 +128,7 @@ public class AttachmentRetrieverIntegrationTest {
 
 	@Test
 	public void testImageIoCrash() throws Exception {
-		AttachmentHeader h = new AttachmentHeader(msgId, "image/png");
+		AttachmentHeader h = new AttachmentHeader(groupId, msgId, "image/png");
 		InputStream is = getAssetInputStream("image_io_crash.png");
 		Attachment a = new Attachment(h, is);
 		AttachmentItem item = retriever.createAttachmentItem(a, true);
@@ -141,7 +143,7 @@ public class AttachmentRetrieverIntegrationTest {
 
 	@Test
 	public void testGimpCrash() throws Exception {
-		AttachmentHeader h = new AttachmentHeader(msgId, "image/gif");
+		AttachmentHeader h = new AttachmentHeader(groupId, msgId, "image/gif");
 		InputStream is = getAssetInputStream("gimp_crash.gif");
 		Attachment a = new Attachment(h, is);
 		AttachmentItem item = retriever.createAttachmentItem(a, true);
@@ -156,7 +158,7 @@ public class AttachmentRetrieverIntegrationTest {
 
 	@Test
 	public void testOptiPngAfl() throws Exception {
-		AttachmentHeader h = new AttachmentHeader(msgId, "image/gif");
+		AttachmentHeader h = new AttachmentHeader(groupId, msgId, "image/gif");
 		InputStream is = getAssetInputStream("opti_png_afl.gif");
 		Attachment a = new Attachment(h, is);
 		AttachmentItem item = retriever.createAttachmentItem(a, true);
@@ -171,7 +173,7 @@ public class AttachmentRetrieverIntegrationTest {
 
 	@Test
 	public void testLibrawError() throws Exception {
-		AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg");
+		AttachmentHeader h = new AttachmentHeader(groupId, msgId, "image/jpeg");
 		InputStream is = getAssetInputStream("libraw_error.jpg");
 		Attachment a = new Attachment(h, is);
 		AttachmentItem item = retriever.createAttachmentItem(a, true);
@@ -180,7 +182,7 @@ public class AttachmentRetrieverIntegrationTest {
 
 	@Test
 	public void testSmallAnimatedGifMaxDimensions() throws Exception {
-		AttachmentHeader h = new AttachmentHeader(msgId, "image/gif");
+		AttachmentHeader h = new AttachmentHeader(groupId, msgId, "image/gif");
 		InputStream is = getAssetInputStream("animated.gif");
 		Attachment a = new Attachment(h, is);
 		AttachmentItem item = retriever.createAttachmentItem(a, true);
@@ -195,7 +197,7 @@ public class AttachmentRetrieverIntegrationTest {
 
 	@Test
 	public void testSmallAnimatedGifHugeDimensions() throws Exception {
-		AttachmentHeader h = new AttachmentHeader(msgId, "image/gif");
+		AttachmentHeader h = new AttachmentHeader(groupId, msgId, "image/gif");
 		InputStream is = getAssetInputStream("animated2.gif");
 		Attachment a = new Attachment(h, is);
 		AttachmentItem item = retriever.createAttachmentItem(a, true);
@@ -210,7 +212,7 @@ public class AttachmentRetrieverIntegrationTest {
 
 	@Test
 	public void testSmallGifLargeDimensions() throws Exception {
-		AttachmentHeader h = new AttachmentHeader(msgId, "image/gif");
+		AttachmentHeader h = new AttachmentHeader(groupId, msgId, "image/gif");
 		InputStream is = getAssetInputStream("error_large.gif");
 		Attachment a = new Attachment(h, is);
 		AttachmentItem item = retriever.createAttachmentItem(a, true);
@@ -225,7 +227,7 @@ public class AttachmentRetrieverIntegrationTest {
 
 	@Test
 	public void testHighError() throws Exception {
-		AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg");
+		AttachmentHeader h = new AttachmentHeader(groupId, msgId, "image/jpeg");
 		InputStream is = getAssetInputStream("error_high.jpg");
 		Attachment a = new Attachment(h, is);
 		AttachmentItem item = retriever.createAttachmentItem(a, true);
@@ -240,7 +242,7 @@ public class AttachmentRetrieverIntegrationTest {
 
 	@Test
 	public void testWideError() throws Exception {
-		AttachmentHeader h = new AttachmentHeader(msgId, "image/jpeg");
+		AttachmentHeader h = new AttachmentHeader(groupId, msgId, "image/jpeg");
 		InputStream is = getAssetInputStream("error_wide.jpg");
 		Attachment a = new Attachment(h, is);
 		AttachmentItem item = retriever.createAttachmentItem(a, true);

briar-android/src/main/java/org/briarproject/briar/android/attachment/AttachmentItem.java

@@ -4,6 +4,7 @@ import android.os.Parcel;
 import android.os.Parcelable;
 
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
+import org.briarproject.bramble.api.sync.GroupId;
 import org.briarproject.bramble.api.sync.MessageId;
 import org.briarproject.briar.api.attachment.AttachmentHeader;
 
@@ -78,6 +79,9 @@ public class AttachmentItem implements Parcelable {
 	}
 
 	protected AttachmentItem(Parcel in) {
+		byte[] groupIdByte = new byte[GroupId.LENGTH];
+		in.readByteArray(groupIdByte);
+		GroupId groupId = new GroupId(groupIdByte);
 		byte[] messageIdByte = new byte[MessageId.LENGTH];
 		in.readByteArray(messageIdByte);
 		MessageId messageId = new MessageId(messageIdByte);
@@ -88,7 +92,7 @@ public class AttachmentItem implements Parcelable {
 		thumbnailWidth = in.readInt();
 		thumbnailHeight = in.readInt();
 		state = State.valueOf(requireNonNull(in.readString()));
-		header = new AttachmentHeader(messageId, mimeType);
+		header = new AttachmentHeader(groupId, messageId, mimeType);
 	}
 
 	public AttachmentHeader getHeader() {
@@ -142,6 +146,7 @@ public class AttachmentItem implements Parcelable {
 
 	@Override
 	public void writeToParcel(Parcel dest, int flags) {
+		dest.writeByteArray(header.getGroupId().getBytes());
 		dest.writeByteArray(header.getMessageId().getBytes());
 		dest.writeInt(width);
 		dest.writeInt(height);

briar-android/src/test/java/org/briarproject/briar/android/attachment/AttachmentRetrieverTest.java

@@ -1,5 +1,6 @@
 package org.briarproject.briar.android.attachment;
 
+import org.briarproject.bramble.api.sync.GroupId;
 import org.briarproject.bramble.api.sync.MessageId;
 import org.briarproject.bramble.test.BrambleMockTestCase;
 import org.briarproject.bramble.test.ImmediateExecutor;
@@ -28,6 +29,7 @@ public class AttachmentRetrieverTest extends BrambleMockTestCase {
 	private final AttachmentDimensions dimensions = new AttachmentDimensions(
 			100, 50, 200, 75, 300
 	);
+	private final GroupId groupId = new GroupId(getRandomId());
 	private final MessageId msgId = new MessageId(getRandomId());
 	private final ImageHelper imageHelper = context.mock(ImageHelper.class);
 	private final ImageSizeCalculator imageSizeCalculator;
@@ -136,7 +138,8 @@ public class AttachmentRetrieverTest extends BrambleMockTestCase {
 	}
 
 	private Attachment getAttachment(String contentType) {
-		AttachmentHeader header = new AttachmentHeader(msgId, contentType);
+		AttachmentHeader header =
+				new AttachmentHeader(groupId, msgId, contentType);
 		InputStream in = new ByteArrayInputStream(getRandomBytes(42));
 		return new Attachment(header, in);
 	}

briar-api/src/main/java/org/briarproject/briar/api/attachment/AttachmentHeader.java

@@ -1,6 +1,7 @@
 package org.briarproject.briar.api.attachment;
 
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
+import org.briarproject.bramble.api.sync.GroupId;
 import org.briarproject.bramble.api.sync.MessageId;
 
 import javax.annotation.concurrent.Immutable;
@@ -9,14 +10,21 @@ import javax.annotation.concurrent.Immutable;
 @NotNullByDefault
 public class AttachmentHeader {
 
+	private final GroupId groupId;
 	private final MessageId messageId;
 	private final String contentType;
 
-	public AttachmentHeader(MessageId messageId, String contentType) {
+	public AttachmentHeader(GroupId groupId, MessageId messageId,
+			String contentType) {
+		this.groupId = groupId;
 		this.messageId = messageId;
 		this.contentType = contentType;
 	}
 
+	public GroupId getGroupId() {
+		return groupId;
+	}
+
 	public MessageId getMessageId() {
 		return messageId;
 	}
@@ -27,13 +35,15 @@ public class AttachmentHeader {
 
 	@Override
 	public boolean equals(Object o) {
-		return o instanceof AttachmentHeader &&
+		if (o instanceof AttachmentHeader) {
-				messageId.equals(((AttachmentHeader) o).messageId);
+			AttachmentHeader h = (AttachmentHeader) o;
+			return groupId.equals(h.groupId) && messageId.equals(h.messageId);
+		}
+		return false;
 	}
 
 	@Override
 	public int hashCode() {
 		return messageId.hashCode();
 	}
-
 }

briar-api/src/main/java/org/briarproject/briar/api/attachment/AttachmentReader.java

@@ -1,15 +1,19 @@
 package org.briarproject.briar.api.attachment;
 
 import org.briarproject.bramble.api.db.DbException;
+import org.briarproject.bramble.api.db.NoSuchMessageException;
 
 public interface AttachmentReader {
 
 	/**
 	 * Returns the attachment with the given attachment header.
 	 *
-	 * @throws InvalidAttachmentException If the header refers to a message
+	 * @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
+	 * 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(AttachmentHeader h) throws DbException;
 

briar-api/src/main/java/org/briarproject/briar/api/attachment/InvalidAttachmentException.java

@@ -1,21 +0,0 @@
-package org.briarproject.briar.api.attachment;
-
-import org.briarproject.bramble.api.db.DbException;
-import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
-
-/**
- * An exception that is thrown when an {@link AttachmentHeader} is used to
- * load an {@link Attachment}, and the header refers to a message that is not
- * an attachment, or to an attachment that does not have the expected content
- * type.
- */
-@NotNullByDefault
-public class InvalidAttachmentException extends DbException {
-	public InvalidAttachmentException() {
-		super();
-	}
-
-	public InvalidAttachmentException(Throwable t) {
-		super(t);
-	}
-}

briar-api/src/test/java/org/briarproject/briar/api/identity/AuthorInfoTest.java

@@ -1,5 +1,6 @@
 package org.briarproject.briar.api.identity;
 
+import org.briarproject.bramble.api.sync.GroupId;
 import org.briarproject.bramble.api.sync.MessageId;
 import org.briarproject.bramble.test.BrambleTestCase;
 import org.briarproject.briar.api.attachment.AttachmentHeader;
@@ -17,7 +18,8 @@ public class AuthorInfoTest extends BrambleTestCase {
 
 	private final String contentType = getRandomString(MAX_CONTENT_TYPE_BYTES);
 	private final AttachmentHeader avatarHeader =
-			new AttachmentHeader(new MessageId(getRandomId()), contentType);
+			new AttachmentHeader(new GroupId(getRandomId()),
+					new MessageId(getRandomId()), contentType);
 
 	@Test
 	public void testEquals() {

briar-core/src/main/java/org/briarproject/briar/attachment/AttachmentReaderImpl.java

@@ -4,11 +4,12 @@ import org.briarproject.bramble.api.FormatException;
 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.sync.Message;
 import org.briarproject.bramble.api.sync.MessageId;
 import org.briarproject.briar.api.attachment.Attachment;
 import org.briarproject.briar.api.attachment.AttachmentHeader;
 import org.briarproject.briar.api.attachment.AttachmentReader;
-import org.briarproject.briar.api.attachment.InvalidAttachmentException;
 
 import java.io.ByteArrayInputStream;
 import java.io.InputStream;
@@ -31,18 +32,24 @@ public class AttachmentReaderImpl implements AttachmentReader {
 	public Attachment getAttachment(AttachmentHeader h) throws DbException {
 		// TODO: Support large messages
 		MessageId m = h.getMessageId();
-		byte[] body = clientHelper.getMessage(m).getBody();
+		Message message = clientHelper.getMessage(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())) {
+			throw new NoSuchMessageException();
+		}
+		byte[] body = message.getBody();
 		try {
 			BdfDictionary meta = clientHelper.getMessageMetadataAsDictionary(m);
 			String contentType = meta.getString(MSG_KEY_CONTENT_TYPE);
 			if (!contentType.equals(h.getContentType()))
-				throw new InvalidAttachmentException();
+				throw new NoSuchMessageException();
 			int offset = meta.getLong(MSG_KEY_DESCRIPTOR_LENGTH).intValue();
 			InputStream stream = new ByteArrayInputStream(body, offset,
 					body.length - offset);
 			return new Attachment(h, stream);
 		} catch (FormatException e) {
-			throw new InvalidAttachmentException(e);
+			throw new NoSuchMessageException();
 		}
 	}
 

briar-core/src/main/java/org/briarproject/briar/avatar/AvatarManagerImpl.java

@@ -149,7 +149,8 @@ class AvatarManagerImpl implements AvatarManager, OpenDatabaseHook, ContactHook,
 			}
 			ContactId contactId = getContactId(txn, m.getGroupId());
 			String contentType = d.getString(MSG_KEY_CONTENT_TYPE);
-			AttachmentHeader a = new AttachmentHeader(m.getId(), contentType);
+			AttachmentHeader a = new AttachmentHeader(m.getGroupId(), m.getId(),
+					contentType);
 			txn.attach(new AvatarUpdatedEvent(contactId, a));
 		} catch (FormatException e) {
 			throw new InvalidMessageException(e);
@@ -184,7 +185,7 @@ class AvatarManagerImpl implements AvatarManager, OpenDatabaseHook, ContactHook,
 			if (newLatest != null && newLatest.version > version) {
 				// latest update is newer than our own
 				// no need to store or delete anything, just return latest
-				return new AttachmentHeader(newLatest.messageId,
+				return new AttachmentHeader(groupId, newLatest.messageId,
 						newLatest.contentType);
 			} else if (newLatest != null) {
 				// delete latest update if it has the same or lower version
@@ -192,7 +193,7 @@ class AvatarManagerImpl implements AvatarManager, OpenDatabaseHook, ContactHook,
 				db.deleteMessageMetadata(txn2, newLatest.messageId);
 			}
 			clientHelper.addLocalMessage(txn2, m, meta, true, false);
-			return new AttachmentHeader(m.getId(), contentType);
+			return new AttachmentHeader(groupId, m.getId(), contentType);
 		});
 	}
 
@@ -225,7 +226,8 @@ class AvatarManagerImpl implements AvatarManager, OpenDatabaseHook, ContactHook,
 			throws DbException, FormatException {
 		LatestUpdate latest = findLatest(txn, groupId);
 		if (latest == null) return null;
-		return new AttachmentHeader(latest.messageId, latest.contentType);
+		return new AttachmentHeader(groupId, latest.messageId,
+				latest.contentType);
 	}
 
 	@Nullable

briar-core/src/main/java/org/briarproject/briar/messaging/MessagingManagerImpl.java

@@ -172,7 +172,7 @@ class MessagingManagerImpl implements MessagingManager, IncomingMessageHook,
 			} else if (messageType == PRIVATE_MESSAGE) {
 				boolean hasText = metaDict.getBoolean(MSG_KEY_HAS_TEXT);
 				List<AttachmentHeader> headers =
-						parseAttachmentHeaders(metaDict);
+						parseAttachmentHeaders(m.getGroupId(), metaDict);
 				incomingPrivateMessage(txn, m, metaDict, hasText, headers);
 			} else if (messageType == ATTACHMENT) {
 				incomingAttachment(txn, m);
@@ -203,16 +203,17 @@ class MessagingManagerImpl implements MessagingManager, IncomingMessageHook,
 		messageTracker.trackIncomingMessage(txn, m);
 	}
 
-	private List<AttachmentHeader> parseAttachmentHeaders(BdfDictionary meta)
+	private List<AttachmentHeader> parseAttachmentHeaders(GroupId g,
+			BdfDictionary meta)
 			throws FormatException {
 		BdfList attachmentHeaders = meta.getList(MSG_KEY_ATTACHMENT_HEADERS);
 		int length = attachmentHeaders.size();
 		List<AttachmentHeader> headers = new ArrayList<>(length);
 		for (int i = 0; i < length; i++) {
 			BdfList header = attachmentHeaders.getList(i);
-			MessageId id = new MessageId(header.getRaw(0));
+			MessageId m = new MessageId(header.getRaw(0));
 			String contentType = header.getString(1);
-			headers.add(new AttachmentHeader(id, contentType));
+			headers.add(new AttachmentHeader(g, m, contentType));
 		}
 		return headers;
 	}
@@ -280,7 +281,7 @@ class MessagingManagerImpl implements MessagingManager, IncomingMessageHook,
 		// Mark attachments as temporary, not shared until we're ready to send
 		db.transaction(false, txn ->
 				clientHelper.addLocalMessage(txn, m, meta, false, true));
-		return new AttachmentHeader(m.getId(), contentType);
+		return new AttachmentHeader(groupId, m.getId(), contentType);
 	}
 
 	@Override
@@ -357,7 +358,7 @@ class MessagingManagerImpl implements MessagingManager, IncomingMessageHook,
 					boolean hasText = meta.getBoolean(MSG_KEY_HAS_TEXT);
 					headers.add(new PrivateMessageHeader(id, g, timestamp,
 							local, read, s.isSent(), s.isSeen(), hasText,
-							parseAttachmentHeaders(meta)));
+							parseAttachmentHeaders(g, meta)));
 				}
 			} catch (FormatException e) {
 				throw new DbException(e);
@@ -424,6 +425,7 @@ class MessagingManagerImpl implements MessagingManager, IncomingMessageHook,
 	public DeletionResult deleteMessages(Transaction txn, ContactId c,
 			Set<MessageId> messageIds) throws DbException {
 		DeletionResult result = new DeletionResult();
+		GroupId g = getContactGroup(db.getContact(txn, c)).getId();
 		for (MessageId m : messageIds) {
 			// get attachment headers
 			List<AttachmentHeader> headers;
@@ -434,7 +436,7 @@ class MessagingManagerImpl implements MessagingManager, IncomingMessageHook,
 				if (messageType != null && messageType != PRIVATE_MESSAGE)
 					throw new AssertionError("not supported");
 				headers = messageType == null ? emptyList() :
-						parseAttachmentHeaders(meta);
+						parseAttachmentHeaders(g, meta);
 			} catch (FormatException e) {
 				throw new DbException(e);
 			}
@@ -460,7 +462,6 @@ class MessagingManagerImpl implements MessagingManager, IncomingMessageHook,
 				result.addNotFullyDownloaded();
 			}
 		}
-		GroupId g = getContactGroup(db.getContact(txn, c)).getId();
 		recalculateGroupCount(txn, g);
 		return result;
 	}

briar-core/src/test/java/org/briarproject/briar/attachment/AttachmentReaderImplTest.java

@@ -0,0 +1,113 @@
+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.NoSuchMessageException;
+import org.briarproject.bramble.api.sync.GroupId;
+import org.briarproject.bramble.api.sync.Message;
+import org.briarproject.bramble.test.BrambleMockTestCase;
+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;
+import java.io.InputStream;
+
+import static java.lang.System.arraycopy;
+import static org.briarproject.bramble.test.TestUtils.getMessage;
+import static org.briarproject.bramble.test.TestUtils.getRandomId;
+import static org.briarproject.bramble.util.IoUtils.copyAndClose;
+import static org.briarproject.briar.api.attachment.MediaConstants.MSG_KEY_CONTENT_TYPE;
+import static org.briarproject.briar.api.attachment.MediaConstants.MSG_KEY_DESCRIPTOR_LENGTH;
+import static org.junit.Assert.assertArrayEquals;
+
+public class AttachmentReaderImplTest extends BrambleMockTestCase {
+
+	private final ClientHelper clientHelper = context.mock(ClientHelper.class);
+
+	private final GroupId groupId = new GroupId(getRandomId());
+	private final Message message = getMessage(groupId, 1234);
+	private final String contentType = "image/jpeg";
+	private final AttachmentHeader header = new AttachmentHeader(groupId,
+			message.getId(), contentType);
+
+	private final AttachmentReaderImpl attachmentReader =
+			new AttachmentReaderImpl(clientHelper);
+
+	@Test(expected = NoSuchMessageException.class)
+	public void testWrongGroup() throws Exception {
+		GroupId wrongGroupId = new GroupId(getRandomId());
+		AttachmentHeader wrongGroup = new AttachmentHeader(wrongGroupId,
+				message.getId(), contentType);
+
+		context.checking(new Expectations() {{
+			oneOf(clientHelper).getMessage(message.getId());
+			will(returnValue(message));
+		}});
+
+		attachmentReader.getAttachment(wrongGroup);
+	}
+
+	@Test(expected = NoSuchMessageException.class)
+	public void testMissingContentType() throws Exception {
+		BdfDictionary meta = new BdfDictionary();
+
+		testInvalidMetadata(meta);
+	}
+
+	@Test(expected = NoSuchMessageException.class)
+	public void testWrongContentType() throws Exception {
+		BdfDictionary meta = BdfDictionary.of(
+				new BdfEntry(MSG_KEY_CONTENT_TYPE, "image/png"));
+
+		testInvalidMetadata(meta);
+	}
+
+	@Test(expected = NoSuchMessageException.class)
+	public void testMissingDescriptorLength() throws Exception {
+		BdfDictionary meta = BdfDictionary.of(
+				new BdfEntry(MSG_KEY_CONTENT_TYPE, contentType));
+
+		testInvalidMetadata(meta);
+	}
+
+	private void testInvalidMetadata(BdfDictionary meta) throws Exception {
+		context.checking(new Expectations() {{
+			oneOf(clientHelper).getMessage(message.getId());
+			will(returnValue(message));
+			oneOf(clientHelper).getMessageMetadataAsDictionary(message.getId());
+			will(returnValue(meta));
+		}});
+
+		attachmentReader.getAttachment(header);
+	}
+
+	@Test
+	public void testSkipsDescriptor() throws Exception {
+		int descriptorLength = 123;
+		BdfDictionary meta = BdfDictionary.of(
+				new BdfEntry(MSG_KEY_CONTENT_TYPE, contentType),
+				new BdfEntry(MSG_KEY_DESCRIPTOR_LENGTH, descriptorLength));
+
+		byte[] body = message.getBody();
+		byte[] expectedData = new byte[body.length - descriptorLength];
+		arraycopy(body, descriptorLength, expectedData, 0, expectedData.length);
+
+		context.checking(new Expectations() {{
+			oneOf(clientHelper).getMessage(message.getId());
+			will(returnValue(message));
+			oneOf(clientHelper).getMessageMetadataAsDictionary(message.getId());
+			will(returnValue(meta));
+		}});
+
+		Attachment attachment = attachmentReader.getAttachment(header);
+		InputStream in = attachment.getStream();
+		ByteArrayOutputStream out = new ByteArrayOutputStream();
+		copyAndClose(in, out);
+		byte[] data = out.toByteArray();
+
+		assertArrayEquals(expectedData, data);
+	}
+}

briar-core/src/test/java/org/briarproject/briar/identity/AuthorManagerImplTest.java

@@ -8,6 +8,7 @@ import org.briarproject.bramble.api.identity.Author;
 import org.briarproject.bramble.api.identity.AuthorId;
 import org.briarproject.bramble.api.identity.IdentityManager;
 import org.briarproject.bramble.api.identity.LocalAuthor;
+import org.briarproject.bramble.api.sync.GroupId;
 import org.briarproject.bramble.api.sync.MessageId;
 import org.briarproject.bramble.test.BrambleMockTestCase;
 import org.briarproject.bramble.test.DbExpectations;
@@ -49,7 +50,8 @@ public class AuthorManagerImplTest extends BrambleMockTestCase {
 	private final Contact contact = getContact(remote, local, verified);
 	private final String contentType = getRandomString(MAX_CONTENT_TYPE_BYTES);
 	private final AttachmentHeader avatarHeader =
-			new AttachmentHeader(new MessageId(getRandomId()), contentType);
+			new AttachmentHeader(new GroupId(getRandomId()),
+					new MessageId(getRandomId()), contentType);
 
 	private final AuthorManagerImpl authorManager =
 			new AuthorManagerImpl(db, identityManager, avatarManager);

briar-core/src/test/java/org/briarproject/briar/messaging/MessageSizeIntegrationTest.java

@@ -73,7 +73,8 @@ public class MessageSizeIntegrationTest extends BriarTestCase {
 		// Create the maximum number of maximum-length attachment headers
 		List<AttachmentHeader> headers = new ArrayList<>();
 		for (int i = 0; i < MAX_ATTACHMENTS_PER_MESSAGE; i++) {
-			headers.add(new AttachmentHeader(new MessageId(getRandomId()),
+			headers.add(new AttachmentHeader(groupId,
+					new MessageId(getRandomId()),
 					getRandomString(MAX_CONTENT_TYPE_BYTES)));
 		}
 		PrivateMessage message = privateMessageFactory.createPrivateMessage(