From d0036deaf7f376165bfe975d85e450985ee22889 Mon Sep 17 00:00:00 2001 From: Torsten Grote <t@grobox.de> Date: Wed, 13 Apr 2016 13:46:34 -0300 Subject: [PATCH] This addresses two types of introduction corner cases: * force decline when two of our own identities are introduced to each other * throw away introduction requests to the same identity (impossible to trigger from UI) Closes #284 --- .../res/layout/list_item_introduction_in.xml | 4 +-- briar-android/res/values/strings.xml | 1 + .../android/contact/ConversationAdapter.java | 26 ++++++++++++------- .../api/db/DatabaseComponent.java | 6 +++++ .../introduction/IntroductionConstants.java | 1 + .../api/introduction/IntroductionRequest.java | 8 ++++-- .../db/DatabaseComponentImpl.java | 7 +++++ .../introduction/IntroduceeEngine.java | 5 +++- .../introduction/IntroduceeManager.java | 12 +++++++++ .../introduction/IntroductionManagerImpl.java | 13 +++++++--- .../introduction/IntroduceeManagerTest.java | 6 +++++ 11 files changed, 72 insertions(+), 17 deletions(-) diff --git a/briar-android/res/layout/list_item_introduction_in.xml b/briar-android/res/layout/list_item_introduction_in.xml index 5daea06085..2f157fad66 100644 --- a/briar-android/res/layout/list_item_introduction_in.xml +++ b/briar-android/res/layout/list_item_introduction_in.xml @@ -36,7 +36,7 @@ android:layout_marginTop="@dimen/message_bubble_timestamp_margin" android:layout_alignEnd="@+id/introductionText" android:layout_alignRight="@+id/introductionText" - android:layout_below="@+id/acceptButton" + android:layout_below="@+id/declineButton" android:textColor="@color/private_message_date" android:textSize="@dimen/text_size_tiny" tools:text="Dec 24, 13:37"/> @@ -46,7 +46,6 @@ style="@style/BriarButtonFlat.Positive" android:layout_width="wrap_content" android:layout_height="wrap_content" - android:layout_marginBottom="-15dp" android:layout_alignEnd="@+id/introductionText" android:layout_alignRight="@+id/introductionText" android:layout_below="@+id/introductionText" @@ -57,6 +56,7 @@ style="@style/BriarButtonFlat.Negative" android:layout_width="wrap_content" android:layout_height="wrap_content" + android:layout_marginBottom="-15dp" android:layout_below="@+id/introductionText" android:layout_toLeftOf="@+id/acceptButton" android:layout_toStartOf="@+id/acceptButton" diff --git a/briar-android/res/values/strings.xml b/briar-android/res/values/strings.xml index bfd82ba9f3..ca1e962f4e 100644 --- a/briar-android/res/values/strings.xml +++ b/briar-android/res/values/strings.xml @@ -155,6 +155,7 @@ <string name="introduction_request_sent">You have asked to introduce %1$s to %2$s.</string> <string name="introduction_request_received">%1$s has asked to introduce you to %2$s. Do you want to add %2$s to your contact list?</string> <string name="introduction_request_exists_received">%1$s has asked to introduce you to %2$s, but %2$s is already in your contact list. Since %1$s might not know that, you can still respond:</string> + <string name="introduction_request_for_our_identity_received">%1$s has asked to introduce you to %2$s, but %2$s is one of your other identities. For that reason you cannot accept the introduction:</string> <string name="introduction_request_answered_received">%1$s has asked to introduce you to %2$s.</string> <string name="introduction_response_accepted_sent">You accepted the introduction to %1$s.</string> <string name="introduction_response_declined_sent">You declined the introduction to %1$s.</string> diff --git a/briar-android/src/org/briarproject/android/contact/ConversationAdapter.java b/briar-android/src/org/briarproject/android/contact/ConversationAdapter.java index 4e8267663b..3a90e4f994 100644 --- a/briar-android/src/org/briarproject/android/contact/ConversationAdapter.java +++ b/briar-android/src/org/briarproject/android/contact/ConversationAdapter.java @@ -261,15 +261,23 @@ class ConversationAdapter extends RecyclerView.Adapter { contactName, ir.getName())); } - ui.acceptButton.setVisibility(View.VISIBLE); - ui.acceptButton.setOnClickListener(new View.OnClickListener() { - @Override - public void onClick(View v) { - intro.respondToIntroduction(ir.getSessionId(), true); - item.setAnswered(true); - notifyItemChanged(position); - } - }); + if (item.getIntroductionRequest().doesIntroduceOtherIdentity()) { + // don't allow accept when one of our identities is introduced + ui.acceptButton.setVisibility(View.GONE); + ui.text.setText(ctx.getString( + R.string.introduction_request_for_our_identity_received, + contactName, ir.getName())); + } else { + ui.acceptButton.setVisibility(View.VISIBLE); + ui.acceptButton.setOnClickListener(new View.OnClickListener() { + @Override + public void onClick(View v) { + intro.respondToIntroduction(ir.getSessionId(), true); + item.setAnswered(true); + notifyItemChanged(position); + } + }); + } ui.declineButton.setVisibility(View.VISIBLE); ui.declineButton.setOnClickListener(new View.OnClickListener() { @Override diff --git a/briar-api/src/org/briarproject/api/db/DatabaseComponent.java b/briar-api/src/org/briarproject/api/db/DatabaseComponent.java index bf6b9f66cd..eca709bc3a 100644 --- a/briar-api/src/org/briarproject/api/db/DatabaseComponent.java +++ b/briar-api/src/org/briarproject/api/db/DatabaseComponent.java @@ -102,6 +102,12 @@ public interface DatabaseComponent { */ boolean containsGroup(Transaction txn, GroupId g) throws DbException; + /** + * Returns true if the database contains the given local author. + */ + boolean containsLocalAuthor(Transaction txn, AuthorId local) + throws DbException; + /** * Deletes the message with the given ID. The message ID and any other * associated data are not deleted. diff --git a/briar-api/src/org/briarproject/api/introduction/IntroductionConstants.java b/briar-api/src/org/briarproject/api/introduction/IntroductionConstants.java index 1ccf2d4805..c3376e36ee 100644 --- a/briar-api/src/org/briarproject/api/introduction/IntroductionConstants.java +++ b/briar-api/src/org/briarproject/api/introduction/IntroductionConstants.java @@ -57,6 +57,7 @@ public interface IntroductionConstants { String ADDED_CONTACT_ID = "addedContactId"; String NOT_OUR_RESPONSE = "notOurResponse"; String EXISTS = "contactExists"; + String REMOTE_AUTHOR_IS_US = "remoteAuthorIsUs"; String ANSWERED = "answered"; String TASK = "task"; diff --git a/briar-api/src/org/briarproject/api/introduction/IntroductionRequest.java b/briar-api/src/org/briarproject/api/introduction/IntroductionRequest.java index 227c0500ce..163eb57be5 100644 --- a/briar-api/src/org/briarproject/api/introduction/IntroductionRequest.java +++ b/briar-api/src/org/briarproject/api/introduction/IntroductionRequest.java @@ -6,12 +6,12 @@ import org.briarproject.api.sync.MessageId; public class IntroductionRequest extends IntroductionResponse { private final String message; - private final boolean answered, exists; + private final boolean answered, exists, introducesOtherIdentity; public IntroductionRequest(SessionId sessionId, MessageId messageId, long time, boolean local, boolean sent, boolean seen, boolean read, AuthorId authorId, String name, boolean accepted, String message, - boolean answered, boolean exists) { + boolean answered, boolean exists, boolean introducesOtherIdentity) { super(sessionId, messageId, time, local, sent, seen, read, authorId, name, accepted); @@ -19,6 +19,7 @@ public class IntroductionRequest extends IntroductionResponse { this.message = message; this.answered = answered; this.exists = exists; + this.introducesOtherIdentity = introducesOtherIdentity; } public String getMessage() { @@ -33,4 +34,7 @@ public class IntroductionRequest extends IntroductionResponse { return exists; } + public boolean doesIntroduceOtherIdentity() { + return introducesOtherIdentity; + } } diff --git a/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java b/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java index 45ef3b9606..45a9c4cbf4 100644 --- a/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java +++ b/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java @@ -243,6 +243,13 @@ class DatabaseComponentImpl<T> implements DatabaseComponent { return db.containsGroup(txn, g); } + @Override + public boolean containsLocalAuthor(Transaction transaction, AuthorId local) + throws DbException { + T txn = unbox(transaction); + return db.containsLocalAuthor(txn, local); + } + public void deleteMessage(Transaction transaction, MessageId m) throws DbException { if (transaction.isReadOnly()) throw new IllegalArgumentException(); diff --git a/briar-core/src/org/briarproject/introduction/IntroduceeEngine.java b/briar-core/src/org/briarproject/introduction/IntroduceeEngine.java index bc1e3d5f95..e713fb8bd8 100644 --- a/briar-core/src/org/briarproject/introduction/IntroduceeEngine.java +++ b/briar-core/src/org/briarproject/introduction/IntroduceeEngine.java @@ -47,6 +47,7 @@ import static org.briarproject.api.introduction.IntroductionConstants.OUR_PUBLIC import static org.briarproject.api.introduction.IntroductionConstants.OUR_TIME; import static org.briarproject.api.introduction.IntroductionConstants.PUBLIC_KEY; import static org.briarproject.api.introduction.IntroductionConstants.REMOTE_AUTHOR_ID; +import static org.briarproject.api.introduction.IntroductionConstants.REMOTE_AUTHOR_IS_US; import static org.briarproject.api.introduction.IntroductionConstants.SESSION_ID; import static org.briarproject.api.introduction.IntroductionConstants.STATE; import static org.briarproject.api.introduction.IntroductionConstants.TASK; @@ -328,10 +329,12 @@ public class IntroduceeEngine String name = msg.getString(NAME); String message = msg.getOptionalString(MSG); boolean exists = localState.getBoolean(EXISTS); + boolean introducesOtherIdentity = + localState.getBoolean(REMOTE_AUTHOR_IS_US); IntroductionRequest ir = new IntroductionRequest(sessionId, messageId, time, false, false, false, false, authorId, name, false, - message, false, exists); + message, false, exists, introducesOtherIdentity); return new IntroductionRequestReceivedEvent(contactId, ir); } diff --git a/briar-core/src/org/briarproject/introduction/IntroduceeManager.java b/briar-core/src/org/briarproject/introduction/IntroduceeManager.java index b25f299e02..b329a3abf1 100644 --- a/briar-core/src/org/briarproject/introduction/IntroduceeManager.java +++ b/briar-core/src/org/briarproject/introduction/IntroduceeManager.java @@ -59,6 +59,7 @@ import static org.briarproject.api.introduction.IntroductionConstants.OUR_PUBLIC import static org.briarproject.api.introduction.IntroductionConstants.OUR_TIME; import static org.briarproject.api.introduction.IntroductionConstants.PUBLIC_KEY; import static org.briarproject.api.introduction.IntroductionConstants.REMOTE_AUTHOR_ID; +import static org.briarproject.api.introduction.IntroductionConstants.REMOTE_AUTHOR_IS_US; import static org.briarproject.api.introduction.IntroductionConstants.ROLE; import static org.briarproject.api.introduction.IntroductionConstants.ROLE_INTRODUCEE; import static org.briarproject.api.introduction.IntroductionConstants.STATE; @@ -147,6 +148,17 @@ class IntroduceeManager { d.put(EXISTS, exists); d.put(REMOTE_AUTHOR_ID, remoteAuthorId); + // check if someone is trying to introduce us to ourselves + if(remoteAuthorId.equals(introducer.getLocalAuthorId())) { + LOG.warning("Received Introduction Request to Ourselves"); + throw new FormatException(); + } + + // check if remote author is actually one of our other identities + boolean introducesOtherIdentity = + db.containsLocalAuthor(txn, remoteAuthorId); + d.put(REMOTE_AUTHOR_IS_US, introducesOtherIdentity); + // save local state to database clientHelper.addLocalMessage(txn, localMsg, IntroductionManagerImpl.CLIENT_ID, d, false); diff --git a/briar-core/src/org/briarproject/introduction/IntroductionManagerImpl.java b/briar-core/src/org/briarproject/introduction/IntroductionManagerImpl.java index 3550d01489..24c89324e1 100644 --- a/briar-core/src/org/briarproject/introduction/IntroductionManagerImpl.java +++ b/briar-core/src/org/briarproject/introduction/IntroductionManagerImpl.java @@ -60,6 +60,7 @@ import static org.briarproject.api.introduction.IntroductionConstants.NAME; import static org.briarproject.api.introduction.IntroductionConstants.NOT_OUR_RESPONSE; import static org.briarproject.api.introduction.IntroductionConstants.READ; import static org.briarproject.api.introduction.IntroductionConstants.REMOTE_AUTHOR_ID; +import static org.briarproject.api.introduction.IntroductionConstants.REMOTE_AUTHOR_IS_US; import static org.briarproject.api.introduction.IntroductionConstants.RESPONSE_1; import static org.briarproject.api.introduction.IntroductionConstants.RESPONSE_2; import static org.briarproject.api.introduction.IntroductionConstants.ROLE; @@ -186,9 +187,11 @@ class IntroductionManagerImpl extends BdfIncomingMessageHook state = introduceeManager.initialize(txn, groupId, message); } catch (FormatException e) { if (LOG.isLoggable(WARNING)) { - LOG.warning("Could not initialize introducee state"); + LOG.warning( + "Could not initialize introducee state, deleting..."); LOG.log(WARNING, e.toString(), e); } + deleteMessage(txn, m.getId()); return; } try { @@ -361,7 +364,7 @@ class IntroductionManagerImpl extends BdfIncomingMessageHook list.add(ir); } else if (type == TYPE_REQUEST) { String message; - boolean answered, exists; + boolean answered, exists, introducesOtherIdentity; if (state.getLong(ROLE) == ROLE_INTRODUCER) { local = true; authorId = @@ -370,6 +373,7 @@ class IntroductionManagerImpl extends BdfIncomingMessageHook message = msg.getOptionalString(MSG); answered = false; exists = false; + introducesOtherIdentity = false; } else { local = false; authorId = new AuthorId( @@ -378,11 +382,14 @@ class IntroductionManagerImpl extends BdfIncomingMessageHook message = state.getOptionalString(MSG); answered = state.getBoolean(ANSWERED); exists = state.getBoolean(EXISTS); + introducesOtherIdentity = + state.getBoolean(REMOTE_AUTHOR_IS_US); } IntroductionRequest ir = new IntroductionRequest( sessionId, messageId, time, local, s.isSent(), s.isSeen(), read, authorId, name, accepted, - message, answered, exists); + message, answered, exists, + introducesOtherIdentity); list.add(ir); } } catch (FormatException e) { diff --git a/briar-tests/src/org/briarproject/introduction/IntroduceeManagerTest.java b/briar-tests/src/org/briarproject/introduction/IntroduceeManagerTest.java index 455c936032..a4d986d257 100644 --- a/briar-tests/src/org/briarproject/introduction/IntroduceeManagerTest.java +++ b/briar-tests/src/org/briarproject/introduction/IntroduceeManagerTest.java @@ -51,6 +51,7 @@ import static org.briarproject.api.introduction.IntroductionConstants.NAME; import static org.briarproject.api.introduction.IntroductionConstants.NOT_OUR_RESPONSE; import static org.briarproject.api.introduction.IntroductionConstants.PUBLIC_KEY; import static org.briarproject.api.introduction.IntroductionConstants.REMOTE_AUTHOR_ID; +import static org.briarproject.api.introduction.IntroductionConstants.REMOTE_AUTHOR_IS_US; import static org.briarproject.api.introduction.IntroductionConstants.ROLE; import static org.briarproject.api.introduction.IntroductionConstants.ROLE_INTRODUCEE; import static org.briarproject.api.introduction.IntroductionConstants.SESSION_ID; @@ -239,6 +240,7 @@ public class IntroduceeManagerTest extends BriarTestCase { state.put(ANSWERED, false); state.put(EXISTS, true); state.put(REMOTE_AUTHOR_ID, introducee2.getAuthor().getId()); + state.put(REMOTE_AUTHOR_IS_US, false); context.checking(new Expectations() {{ oneOf(clock).currentTimeMillis(); @@ -268,6 +270,10 @@ public class IntroduceeManagerTest extends BriarTestCase { introducer.getLocalAuthorId()); will(returnValue(contactExists)); + // checks if remote author is one of our identities + oneOf(db).containsLocalAuthor(txn, introducee2.getAuthor().getId()); + will(returnValue(false)); + // store session state oneOf(clientHelper) .addLocalMessage(txn, localStateMessage, clientId, state, -- GitLab