From f459115b19b8f36ca49a013392950c0ec59937a3 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 15 Mar 2019 17:31:56 +0000 Subject: [PATCH 1/7] Run contact exchange task on IO executor. --- .../api/contact/ContactExchangeTask.java | 7 ++- .../contact/ContactExchangeTaskImpl.java | 47 ++++++------------- .../keyagreement/ContactExchangeActivity.java | 13 +++-- 3 files changed, 29 insertions(+), 38 deletions(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeTask.java b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeTask.java index c72b0fa7e..fd8dd20eb 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeTask.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeTask.java @@ -38,10 +38,15 @@ public interface ContactExchangeTask { */ String BOB_NONCE_LABEL = "org.briarproject.bramble.contact/BOB_NONCE"; + /** + * Label for signing key binding nonces. + */ + String SIGNING_LABEL = "org.briarproject.briar.contact/EXCHANGE"; + /** * Exchanges contact information with a remote peer. */ - void startExchange(LocalAuthor localAuthor, SecretKey masterKey, + void exchangeContacts(LocalAuthor localAuthor, SecretKey masterKey, DuplexTransportConnection conn, TransportId transportId, boolean alice); } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeTaskImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeTaskImpl.java index a6c90699e..3b2668a99 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeTaskImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeTaskImpl.java @@ -46,6 +46,7 @@ import java.util.logging.Logger; import javax.inject.Inject; import static java.util.logging.Level.WARNING; +import static java.util.logging.Logger.getLogger; import static org.briarproject.bramble.api.contact.RecordTypes.CONTACT_INFO; import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_SIGNATURE_LENGTH; import static org.briarproject.bramble.util.LogUtils.logException; @@ -54,13 +55,10 @@ import static org.briarproject.bramble.util.ValidationUtils.checkSize; @MethodsNotNullByDefault @ParametersNotNullByDefault -class ContactExchangeTaskImpl extends Thread implements ContactExchangeTask { +class ContactExchangeTaskImpl implements ContactExchangeTask { private static final Logger LOG = - Logger.getLogger(ContactExchangeTaskImpl.class.getName()); - - private static final String SIGNING_LABEL_EXCHANGE = - "org.briarproject.briar.contact/EXCHANGE"; + getLogger(ContactExchangeTaskImpl.class.getName()); // Accept records with current protocol version, known record type private static final Predicate ACCEPT = r -> @@ -89,12 +87,6 @@ class ContactExchangeTaskImpl extends Thread implements ContactExchangeTask { private final StreamReaderFactory streamReaderFactory; private final StreamWriterFactory streamWriterFactory; - private volatile LocalAuthor localAuthor; - private volatile DuplexTransportConnection conn; - private volatile TransportId transportId; - private volatile SecretKey masterKey; - private volatile boolean alice; - @Inject ContactExchangeTaskImpl(DatabaseComponent db, ClientHelper clientHelper, RecordReaderFactory recordReaderFactory, @@ -119,19 +111,9 @@ class ContactExchangeTaskImpl extends Thread implements ContactExchangeTask { } @Override - public void startExchange(LocalAuthor localAuthor, SecretKey masterKey, - DuplexTransportConnection conn, TransportId transportId, - boolean alice) { - this.localAuthor = localAuthor; - this.conn = conn; - this.transportId = transportId; - this.masterKey = masterKey; - this.alice = alice; - start(); - } - - @Override - public void run() { + public void exchangeContacts(LocalAuthor localAuthor, + SecretKey masterKey, DuplexTransportConnection conn, + TransportId transportId, boolean alice) { // Get the transport connection's input and output streams InputStream in; OutputStream out; @@ -195,13 +177,11 @@ class ContactExchangeTaskImpl extends Thread implements ContactExchangeTask { if (alice) { sendContactInfo(recordWriter, localAuthor, localProperties, localSignature, localTimestamp); - recordWriter.flush(); remoteInfo = receiveContactInfo(recordReader); } else { remoteInfo = receiveContactInfo(recordReader); sendContactInfo(recordWriter, localAuthor, localProperties, localSignature, localTimestamp); - recordWriter.flush(); } // Send EOF on the outgoing stream streamWriter.sendEndOfStream(); @@ -227,8 +207,8 @@ class ContactExchangeTaskImpl extends Thread implements ContactExchangeTask { try { // Add the contact - ContactId contactId = addContact(remoteInfo.author, timestamp, - remoteInfo.properties); + ContactId contactId = addContact(remoteInfo.author, localAuthor, + masterKey, timestamp, alice, remoteInfo.properties); // Reuse the connection as a transport connection connectionManager.manageOutgoingConnection(contactId, transportId, conn); @@ -250,8 +230,7 @@ class ContactExchangeTaskImpl extends Thread implements ContactExchangeTask { private byte[] sign(LocalAuthor author, byte[] nonce) { try { - return crypto.sign(SIGNING_LABEL_EXCHANGE, nonce, - author.getPrivateKey()); + return crypto.sign(SIGNING_LABEL, nonce, author.getPrivateKey()); } catch (GeneralSecurityException e) { throw new AssertionError(); } @@ -259,8 +238,8 @@ class ContactExchangeTaskImpl extends Thread implements ContactExchangeTask { private boolean verify(Author author, byte[] nonce, byte[] signature) { try { - return crypto.verifySignature(signature, SIGNING_LABEL_EXCHANGE, - nonce, author.getPublicKey()); + return crypto.verifySignature(signature, SIGNING_LABEL, nonce, + author.getPublicKey()); } catch (GeneralSecurityException e) { return false; } @@ -274,6 +253,7 @@ class ContactExchangeTaskImpl extends Thread implements ContactExchangeTask { BdfList payload = BdfList.of(authorList, props, signature, timestamp); recordWriter.writeRecord(new Record(PROTOCOL_VERSION, CONTACT_INFO, clientHelper.toByteArray(payload))); + recordWriter.flush(); LOG.info("Sent contact info"); } @@ -295,7 +275,8 @@ class ContactExchangeTaskImpl extends Thread implements ContactExchangeTask { return new ContactInfo(author, properties, signature, timestamp); } - private ContactId addContact(Author remoteAuthor, long timestamp, + private ContactId addContact(Author remoteAuthor, LocalAuthor localAuthor, + SecretKey masterKey, long timestamp, boolean alice, Map remoteProperties) throws DbException { return db.transactionWithResult(false, txn -> { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeActivity.java index cf4329518..a99ce9aaa 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeActivity.java @@ -14,11 +14,13 @@ import org.briarproject.bramble.api.identity.Author; import org.briarproject.bramble.api.identity.IdentityManager; import org.briarproject.bramble.api.identity.LocalAuthor; import org.briarproject.bramble.api.keyagreement.KeyAgreementResult; +import org.briarproject.bramble.api.lifecycle.IoExecutor; import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault; import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault; import org.briarproject.briar.R; import org.briarproject.briar.android.activity.ActivityComponent; +import java.util.concurrent.Executor; import java.util.logging.Logger; import javax.annotation.Nullable; @@ -36,6 +38,10 @@ public class ContactExchangeActivity extends KeyAgreementActivity implements private static final Logger LOG = Logger.getLogger(ContactExchangeActivity.class.getName()); + @Inject + @IoExecutor + Executor ioExecutor; + // Fields that are accessed from background threads must be volatile @Inject volatile ContactExchangeTask contactExchangeTask; @@ -63,12 +69,12 @@ public class ContactExchangeActivity extends KeyAgreementActivity implements @Override protected void onStop() { super.onStop(); - // Stop listen to updates from contactExchangeTask + // Stop listening to updates from contactExchangeTask eventBus.addListener(this); } private void startContactExchange(KeyAgreementResult result) { - runOnDbThread(() -> { + ioExecutor.execute(() -> { LocalAuthor localAuthor; // Load the local pseudonym try { @@ -78,9 +84,8 @@ public class ContactExchangeActivity extends KeyAgreementActivity implements contactExchangeFailed(); return; } - // Exchange contact details - contactExchangeTask.startExchange(localAuthor, + contactExchangeTask.exchangeContacts(localAuthor, result.getMasterKey(), result.getConnection(), result.getTransportId(), result.wasAlice()); }); From da54712ae14b96dc94d294949e0ce3eba88e74b0 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 9 May 2019 17:03:50 +0100 Subject: [PATCH 2/7] Refactor ContactExchangeTask into reusable manager. --- .../api/contact/ContactExchangeManager.java | 16 ++++ .../contact/ContactExchangeConstants.java | 21 +---- ...l.java => ContactExchangeManagerImpl.java} | 31 +++++--- .../bramble/contact/ContactModule.java | 10 +-- .../briar/android/AndroidComponent.java | 4 +- .../keyagreement/ContactExchangeActivity.java | 78 +++++++------------ 6 files changed, 72 insertions(+), 88 deletions(-) create mode 100644 bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeManager.java rename bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeTask.java => bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeConstants.java (53%) rename bramble-core/src/main/java/org/briarproject/bramble/contact/{ContactExchangeTaskImpl.java => ContactExchangeManagerImpl.java} (89%) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeManager.java b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeManager.java new file mode 100644 index 000000000..67e72bab6 --- /dev/null +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeManager.java @@ -0,0 +1,16 @@ +package org.briarproject.bramble.api.contact; + +import org.briarproject.bramble.api.crypto.SecretKey; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import org.briarproject.bramble.api.plugin.TransportId; +import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; + +@NotNullByDefault +public interface ContactExchangeManager { + + /** + * Exchanges contact information with a remote peer. + */ + void exchangeContacts(TransportId t, DuplexTransportConnection conn, + SecretKey masterKey, boolean alice); +} diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeTask.java b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeConstants.java similarity index 53% rename from bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeTask.java rename to bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeConstants.java index fd8dd20eb..9f39d3470 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeTask.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeConstants.java @@ -1,16 +1,6 @@ -package org.briarproject.bramble.api.contact; +package org.briarproject.bramble.contact; -import org.briarproject.bramble.api.crypto.SecretKey; -import org.briarproject.bramble.api.identity.LocalAuthor; -import org.briarproject.bramble.api.nullsafety.NotNullByDefault; -import org.briarproject.bramble.api.plugin.TransportId; -import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; - -/** - * A task for conducting a contact information exchange with a remote peer. - */ -@NotNullByDefault -public interface ContactExchangeTask { +interface ContactExchangeConstants { /** * The current version of the contact exchange protocol. @@ -42,11 +32,4 @@ public interface ContactExchangeTask { * Label for signing key binding nonces. */ String SIGNING_LABEL = "org.briarproject.briar.contact/EXCHANGE"; - - /** - * Exchanges contact information with a remote peer. - */ - void exchangeContacts(LocalAuthor localAuthor, SecretKey masterKey, - DuplexTransportConnection conn, TransportId transportId, - boolean alice); } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeTaskImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java similarity index 89% rename from bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeTaskImpl.java rename to bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java index 3b2668a99..f68ab538b 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeTaskImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java @@ -3,7 +3,7 @@ package org.briarproject.bramble.contact; import org.briarproject.bramble.api.FormatException; import org.briarproject.bramble.api.Predicate; import org.briarproject.bramble.api.client.ClientHelper; -import org.briarproject.bramble.api.contact.ContactExchangeTask; +import org.briarproject.bramble.api.contact.ContactExchangeManager; import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.contact.ContactManager; import org.briarproject.bramble.api.contact.event.ContactExchangeFailedEvent; @@ -17,6 +17,7 @@ import org.briarproject.bramble.api.db.DatabaseComponent; import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.event.EventBus; import org.briarproject.bramble.api.identity.Author; +import org.briarproject.bramble.api.identity.IdentityManager; import org.briarproject.bramble.api.identity.LocalAuthor; import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault; import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault; @@ -49,16 +50,22 @@ import static java.util.logging.Level.WARNING; import static java.util.logging.Logger.getLogger; import static org.briarproject.bramble.api.contact.RecordTypes.CONTACT_INFO; import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_SIGNATURE_LENGTH; +import static org.briarproject.bramble.contact.ContactExchangeConstants.ALICE_KEY_LABEL; +import static org.briarproject.bramble.contact.ContactExchangeConstants.ALICE_NONCE_LABEL; +import static org.briarproject.bramble.contact.ContactExchangeConstants.BOB_KEY_LABEL; +import static org.briarproject.bramble.contact.ContactExchangeConstants.BOB_NONCE_LABEL; +import static org.briarproject.bramble.contact.ContactExchangeConstants.PROTOCOL_VERSION; +import static org.briarproject.bramble.contact.ContactExchangeConstants.SIGNING_LABEL; import static org.briarproject.bramble.util.LogUtils.logException; import static org.briarproject.bramble.util.ValidationUtils.checkLength; import static org.briarproject.bramble.util.ValidationUtils.checkSize; @MethodsNotNullByDefault @ParametersNotNullByDefault -class ContactExchangeTaskImpl implements ContactExchangeTask { +class ContactExchangeManagerImpl implements ContactExchangeManager { private static final Logger LOG = - getLogger(ContactExchangeTaskImpl.class.getName()); + getLogger(ContactExchangeManagerImpl.class.getName()); // Accept records with current protocol version, known record type private static final Predicate ACCEPT = r -> @@ -82,17 +89,18 @@ class ContactExchangeTaskImpl implements ContactExchangeTask { private final Clock clock; private final ConnectionManager connectionManager; private final ContactManager contactManager; + private final IdentityManager identityManager; private final TransportPropertyManager transportPropertyManager; private final CryptoComponent crypto; private final StreamReaderFactory streamReaderFactory; private final StreamWriterFactory streamWriterFactory; @Inject - ContactExchangeTaskImpl(DatabaseComponent db, ClientHelper clientHelper, + ContactExchangeManagerImpl(DatabaseComponent db, ClientHelper clientHelper, RecordReaderFactory recordReaderFactory, RecordWriterFactory recordWriterFactory, EventBus eventBus, Clock clock, ConnectionManager connectionManager, - ContactManager contactManager, + ContactManager contactManager, IdentityManager identityManager, TransportPropertyManager transportPropertyManager, CryptoComponent crypto, StreamReaderFactory streamReaderFactory, StreamWriterFactory streamWriterFactory) { @@ -104,6 +112,7 @@ class ContactExchangeTaskImpl implements ContactExchangeTask { this.clock = clock; this.connectionManager = connectionManager; this.contactManager = contactManager; + this.identityManager = identityManager; this.transportPropertyManager = transportPropertyManager; this.crypto = crypto; this.streamReaderFactory = streamReaderFactory; @@ -111,9 +120,8 @@ class ContactExchangeTaskImpl implements ContactExchangeTask { } @Override - public void exchangeContacts(LocalAuthor localAuthor, - SecretKey masterKey, DuplexTransportConnection conn, - TransportId transportId, boolean alice) { + public void exchangeContacts(TransportId t, DuplexTransportConnection conn, + SecretKey masterKey, boolean alice) { // Get the transport connection's input and output streams InputStream in; OutputStream out; @@ -127,9 +135,11 @@ class ContactExchangeTaskImpl implements ContactExchangeTask { return; } - // Get the local transport properties + // Get the local author and transport properties + LocalAuthor localAuthor; Map localProperties; try { + localAuthor = identityManager.getLocalAuthor(); localProperties = transportPropertyManager.getLocalProperties(); } catch (DbException e) { logException(LOG, WARNING, e); @@ -210,8 +220,7 @@ class ContactExchangeTaskImpl implements ContactExchangeTask { ContactId contactId = addContact(remoteInfo.author, localAuthor, masterKey, timestamp, alice, remoteInfo.properties); // Reuse the connection as a transport connection - connectionManager.manageOutgoingConnection(contactId, transportId, - conn); + connectionManager.manageOutgoingConnection(contactId, t, conn); // Pseudonym exchange succeeded LOG.info("Pseudonym exchange succeeded"); eventBus.broadcast( diff --git a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactModule.java b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactModule.java index f31ce108a..614edd4ea 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactModule.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactModule.java @@ -1,6 +1,6 @@ package org.briarproject.bramble.contact; -import org.briarproject.bramble.api.contact.ContactExchangeTask; +import org.briarproject.bramble.api.contact.ContactExchangeManager; import org.briarproject.bramble.api.contact.ContactManager; import javax.inject.Inject; @@ -19,14 +19,14 @@ public class ContactModule { @Provides @Singleton - ContactManager getContactManager(ContactManagerImpl contactManager) { + ContactManager provideContactManager(ContactManagerImpl contactManager) { return contactManager; } @Provides - ContactExchangeTask provideContactExchangeTask( - ContactExchangeTaskImpl contactExchangeTask) { - return contactExchangeTask; + ContactExchangeManager provideContactExchangeManager( + ContactExchangeManagerImpl contactExchangeManager) { + return contactExchangeManager; } @Provides diff --git a/briar-android/src/main/java/org/briarproject/briar/android/AndroidComponent.java b/briar-android/src/main/java/org/briarproject/briar/android/AndroidComponent.java index 90a76e6da..e2473e846 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/AndroidComponent.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/AndroidComponent.java @@ -8,7 +8,7 @@ import org.briarproject.bramble.BrambleCoreEagerSingletons; import org.briarproject.bramble.BrambleCoreModule; import org.briarproject.bramble.account.BriarAccountModule; import org.briarproject.bramble.api.account.AccountManager; -import org.briarproject.bramble.api.contact.ContactExchangeTask; +import org.briarproject.bramble.api.contact.ContactExchangeManager; import org.briarproject.bramble.api.contact.ContactManager; import org.briarproject.bramble.api.crypto.CryptoExecutor; import org.briarproject.bramble.api.crypto.PasswordStrengthEstimator; @@ -128,7 +128,7 @@ public interface AndroidComponent SettingsManager settingsManager(); - ContactExchangeTask contactExchangeTask(); + ContactExchangeManager contactExchangeManager(); KeyAgreementTask keyAgreementTask(); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeActivity.java index a99ce9aaa..9d8f661e2 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeActivity.java @@ -4,15 +4,12 @@ import android.os.Bundle; import android.support.annotation.UiThread; import android.widget.Toast; -import org.briarproject.bramble.api.contact.ContactExchangeTask; +import org.briarproject.bramble.api.contact.ContactExchangeManager; import org.briarproject.bramble.api.contact.event.ContactExchangeFailedEvent; import org.briarproject.bramble.api.contact.event.ContactExchangeSucceededEvent; -import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.event.Event; import org.briarproject.bramble.api.event.EventListener; import org.briarproject.bramble.api.identity.Author; -import org.briarproject.bramble.api.identity.IdentityManager; -import org.briarproject.bramble.api.identity.LocalAuthor; import org.briarproject.bramble.api.keyagreement.KeyAgreementResult; import org.briarproject.bramble.api.lifecycle.IoExecutor; import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault; @@ -21,32 +18,25 @@ import org.briarproject.briar.R; import org.briarproject.briar.android.activity.ActivityComponent; import java.util.concurrent.Executor; -import java.util.logging.Logger; import javax.annotation.Nullable; import javax.inject.Inject; import static android.widget.Toast.LENGTH_LONG; -import static java.util.logging.Level.WARNING; -import static org.briarproject.bramble.util.LogUtils.logException; +import static java.util.Objects.requireNonNull; @MethodsNotNullByDefault @ParametersNotNullByDefault public class ContactExchangeActivity extends KeyAgreementActivity implements EventListener { - private static final Logger LOG = - Logger.getLogger(ContactExchangeActivity.class.getName()); - @Inject @IoExecutor Executor ioExecutor; // Fields that are accessed from background threads must be volatile @Inject - volatile ContactExchangeTask contactExchangeTask; - @Inject - volatile IdentityManager identityManager; + volatile ContactExchangeManager contactExchangeManager; @Override public void injectActivity(ActivityComponent component) { @@ -62,76 +52,62 @@ public class ContactExchangeActivity extends KeyAgreementActivity implements @Override public void onStart() { super.onStart(); - // Listen to updates from contactExchangeTask + // Listen to updates from ContactExchangeManager eventBus.addListener(this); } @Override protected void onStop() { super.onStop(); - // Stop listening to updates from contactExchangeTask + // Stop listening to updates from ContactExchangeManager eventBus.addListener(this); } private void startContactExchange(KeyAgreementResult result) { ioExecutor.execute(() -> { - LocalAuthor localAuthor; - // Load the local pseudonym - try { - localAuthor = identityManager.getLocalAuthor(); - } catch (DbException e) { - logException(LOG, WARNING, e); - contactExchangeFailed(); - return; - } // Exchange contact details - contactExchangeTask.exchangeContacts(localAuthor, - result.getMasterKey(), result.getConnection(), - result.getTransportId(), result.wasAlice()); + contactExchangeManager.exchangeContacts(result.getTransportId(), + result.getConnection(), result.getMasterKey(), + result.wasAlice()); }); } @Override public void eventOccurred(Event e) { if (e instanceof ContactExchangeSucceededEvent) { - contactExchangeSucceeded( - ((ContactExchangeSucceededEvent) e).getRemoteAuthor()); + ContactExchangeSucceededEvent c = (ContactExchangeSucceededEvent) e; + contactExchangeSucceeded(c.getRemoteAuthor()); } else if (e instanceof ContactExchangeFailedEvent) { - ContactExchangeFailedEvent fe = (ContactExchangeFailedEvent) e; - if (fe.wasDuplicateContact()) { - duplicateContact(fe.getDuplicateRemoteAuthor()); + ContactExchangeFailedEvent c = (ContactExchangeFailedEvent) e; + if (c.wasDuplicateContact()) { + duplicateContact(requireNonNull(c.getDuplicateRemoteAuthor())); } else { contactExchangeFailed(); } } } + @UiThread private void contactExchangeSucceeded(Author remoteAuthor) { - runOnUiThreadUnlessDestroyed(() -> { - String contactName = remoteAuthor.getName(); - String format = getString(R.string.contact_added_toast); - String text = String.format(format, contactName); - Toast.makeText(ContactExchangeActivity.this, text, LENGTH_LONG) - .show(); - supportFinishAfterTransition(); - }); + String contactName = remoteAuthor.getName(); + String format = getString(R.string.contact_added_toast); + String text = String.format(format, contactName); + Toast.makeText(this, text, LENGTH_LONG).show(); + supportFinishAfterTransition(); } + @UiThread private void duplicateContact(Author remoteAuthor) { - runOnUiThreadUnlessDestroyed(() -> { - String contactName = remoteAuthor.getName(); - String format = getString(R.string.contact_already_exists); - String text = String.format(format, contactName); - Toast.makeText(ContactExchangeActivity.this, text, LENGTH_LONG) - .show(); - finish(); - }); + String contactName = remoteAuthor.getName(); + String format = getString(R.string.contact_already_exists); + String text = String.format(format, contactName); + Toast.makeText(this, text, LENGTH_LONG).show(); + finish(); } + @UiThread private void contactExchangeFailed() { - runOnUiThreadUnlessDestroyed(() -> { - showErrorFragment(R.string.connection_error_explanation); - }); + showErrorFragment(R.string.connection_error_explanation); } @UiThread From 9ea91cbb3e95e7134dedac8cf4b166801b78427e Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 9 May 2019 17:41:41 +0100 Subject: [PATCH 3/7] Move background work into view model. --- .../briarproject/briar/android/AppModule.java | 3 +- .../keyagreement/ContactExchangeActivity.java | 70 +++++--------- .../keyagreement/ContactExchangeModule.java | 20 ++++ .../ContactExchangeViewModel.java | 93 +++++++++++++++++++ .../briar/android/viewmodel/ViewModelKey.java | 2 +- 5 files changed, 137 insertions(+), 51 deletions(-) create mode 100644 briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeModule.java create mode 100644 briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeViewModel.java diff --git a/briar-android/src/main/java/org/briarproject/briar/android/AppModule.java b/briar-android/src/main/java/org/briarproject/briar/android/AppModule.java index c3ccaa9cc..55e966549 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/AppModule.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/AppModule.java @@ -33,6 +33,7 @@ import org.briarproject.bramble.plugin.tor.CircumventionProvider; import org.briarproject.bramble.util.AndroidUtils; import org.briarproject.bramble.util.StringUtils; import org.briarproject.briar.android.account.LockManagerImpl; +import org.briarproject.briar.android.keyagreement.ContactExchangeModule; import org.briarproject.briar.android.viewmodel.ViewModelModule; import org.briarproject.briar.api.android.AndroidNotificationManager; import org.briarproject.briar.api.android.DozeWatchdog; @@ -59,7 +60,7 @@ import static java.util.Collections.emptyList; import static org.briarproject.bramble.api.reporting.ReportingConstants.DEV_ONION_ADDRESS; import static org.briarproject.bramble.api.reporting.ReportingConstants.DEV_PUBLIC_KEY_HEX; -@Module(includes = ViewModelModule.class) +@Module(includes = {ContactExchangeModule.class, ViewModelModule.class}) public class AppModule { static class EagerSingletons { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeActivity.java index 9d8f661e2..7372ae2ae 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeActivity.java @@ -1,24 +1,18 @@ package org.briarproject.briar.android.keyagreement; +import android.arch.lifecycle.ViewModelProvider; +import android.arch.lifecycle.ViewModelProviders; import android.os.Bundle; import android.support.annotation.UiThread; import android.widget.Toast; -import org.briarproject.bramble.api.contact.ContactExchangeManager; -import org.briarproject.bramble.api.contact.event.ContactExchangeFailedEvent; -import org.briarproject.bramble.api.contact.event.ContactExchangeSucceededEvent; -import org.briarproject.bramble.api.event.Event; -import org.briarproject.bramble.api.event.EventListener; import org.briarproject.bramble.api.identity.Author; import org.briarproject.bramble.api.keyagreement.KeyAgreementResult; -import org.briarproject.bramble.api.lifecycle.IoExecutor; import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault; import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault; import org.briarproject.briar.R; import org.briarproject.briar.android.activity.ActivityComponent; -import java.util.concurrent.Executor; - import javax.annotation.Nullable; import javax.inject.Inject; @@ -27,16 +21,12 @@ import static java.util.Objects.requireNonNull; @MethodsNotNullByDefault @ParametersNotNullByDefault -public class ContactExchangeActivity extends KeyAgreementActivity implements - EventListener { +public class ContactExchangeActivity extends KeyAgreementActivity { @Inject - @IoExecutor - Executor ioExecutor; + ViewModelProvider.Factory viewModelFactory; - // Fields that are accessed from background threads must be volatile - @Inject - volatile ContactExchangeManager contactExchangeManager; + private ContactExchangeViewModel viewModel; @Override public void injectActivity(ActivityComponent component) { @@ -46,45 +36,27 @@ public class ContactExchangeActivity extends KeyAgreementActivity implements @Override public void onCreate(@Nullable Bundle state) { super.onCreate(state); - getSupportActionBar().setTitle(R.string.add_contact_title); - } - - @Override - public void onStart() { - super.onStart(); - // Listen to updates from ContactExchangeManager - eventBus.addListener(this); - } - - @Override - protected void onStop() { - super.onStop(); - // Stop listening to updates from ContactExchangeManager - eventBus.addListener(this); + requireNonNull(getSupportActionBar()) + .setTitle(R.string.add_contact_title); + viewModel = ViewModelProviders.of(this, viewModelFactory) + .get(ContactExchangeViewModel.class); } private void startContactExchange(KeyAgreementResult result) { - ioExecutor.execute(() -> { - // Exchange contact details - contactExchangeManager.exchangeContacts(result.getTransportId(), - result.getConnection(), result.getMasterKey(), - result.wasAlice()); - }); - } - - @Override - public void eventOccurred(Event e) { - if (e instanceof ContactExchangeSucceededEvent) { - ContactExchangeSucceededEvent c = (ContactExchangeSucceededEvent) e; - contactExchangeSucceeded(c.getRemoteAuthor()); - } else if (e instanceof ContactExchangeFailedEvent) { - ContactExchangeFailedEvent c = (ContactExchangeFailedEvent) e; - if (c.wasDuplicateContact()) { - duplicateContact(requireNonNull(c.getDuplicateRemoteAuthor())); + viewModel.getSucceeded().observe(this, succeeded -> { + if (succeeded == null) return; + if (succeeded) { + Author remote = requireNonNull(viewModel.getRemoteAuthor()); + contactExchangeSucceeded(remote); } else { - contactExchangeFailed(); + Author duplicate = viewModel.getDuplicateAuthor(); + if (duplicate == null) contactExchangeFailed(); + else duplicateContact(duplicate); } - } + }); + viewModel.startContactExchange(result.getTransportId(), + result.getConnection(), result.getMasterKey(), + result.wasAlice()); } @UiThread diff --git a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeModule.java b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeModule.java new file mode 100644 index 000000000..65baf1e70 --- /dev/null +++ b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeModule.java @@ -0,0 +1,20 @@ +package org.briarproject.briar.android.keyagreement; + +import android.arch.lifecycle.ViewModel; + +import org.briarproject.briar.android.viewmodel.ViewModelKey; + +import dagger.Binds; +import dagger.Module; +import dagger.multibindings.IntoMap; + +@Module +public abstract class ContactExchangeModule { + + @Binds + @IntoMap + @ViewModelKey(ContactExchangeViewModel.class) + abstract ViewModel bindContactExchangeViewModel( + ContactExchangeViewModel contactExchangeViewModel); + +} diff --git a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeViewModel.java new file mode 100644 index 000000000..5b990f2d8 --- /dev/null +++ b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeViewModel.java @@ -0,0 +1,93 @@ +package org.briarproject.briar.android.keyagreement; + +import android.app.Application; +import android.arch.lifecycle.AndroidViewModel; +import android.arch.lifecycle.LiveData; +import android.arch.lifecycle.MutableLiveData; +import android.support.annotation.UiThread; + +import org.briarproject.bramble.api.contact.ContactExchangeManager; +import org.briarproject.bramble.api.contact.event.ContactExchangeFailedEvent; +import org.briarproject.bramble.api.contact.event.ContactExchangeSucceededEvent; +import org.briarproject.bramble.api.crypto.SecretKey; +import org.briarproject.bramble.api.event.Event; +import org.briarproject.bramble.api.event.EventBus; +import org.briarproject.bramble.api.event.EventListener; +import org.briarproject.bramble.api.identity.Author; +import org.briarproject.bramble.api.lifecycle.IoExecutor; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import org.briarproject.bramble.api.plugin.TransportId; +import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; + +import java.util.concurrent.Executor; + +import javax.annotation.Nullable; +import javax.inject.Inject; + +import static java.util.Objects.requireNonNull; + +@NotNullByDefault +class ContactExchangeViewModel extends AndroidViewModel + implements EventListener { + + private final Executor ioExecutor; + private final ContactExchangeManager contactExchangeManager; + private final EventBus eventBus; + private final MutableLiveData succeeded = new MutableLiveData<>(); + + @Nullable + private Author remoteAuthor, duplicateAuthor; + + @Inject + ContactExchangeViewModel(Application app, @IoExecutor Executor ioExecutor, + ContactExchangeManager contactExchangeManager, EventBus eventBus) { + super(app); + this.ioExecutor = ioExecutor; + this.contactExchangeManager = contactExchangeManager; + this.eventBus = eventBus; + eventBus.addListener(this); + } + + @Override + protected void onCleared() { + super.onCleared(); + eventBus.removeListener(this); + } + + @UiThread + void startContactExchange(TransportId t, DuplexTransportConnection conn, + SecretKey masterKey, boolean alice) { + ioExecutor.execute(() -> contactExchangeManager.exchangeContacts(t, + conn, masterKey, alice)); + } + + @UiThread + @Nullable + Author getRemoteAuthor() { + return remoteAuthor; + } + + @UiThread + @Nullable + Author getDuplicateAuthor() { + return duplicateAuthor; + } + + LiveData getSucceeded() { + return succeeded; + } + + @Override + public void eventOccurred(Event e) { + if (e instanceof ContactExchangeSucceededEvent) { + ContactExchangeSucceededEvent c = (ContactExchangeSucceededEvent) e; + remoteAuthor = c.getRemoteAuthor(); + succeeded.setValue(true); + } else if (e instanceof ContactExchangeFailedEvent) { + ContactExchangeFailedEvent c = (ContactExchangeFailedEvent) e; + if (c.wasDuplicateContact()) + duplicateAuthor = requireNonNull(c.getDuplicateRemoteAuthor()); + succeeded.setValue(false); + } + } +} diff --git a/briar-android/src/main/java/org/briarproject/briar/android/viewmodel/ViewModelKey.java b/briar-android/src/main/java/org/briarproject/briar/android/viewmodel/ViewModelKey.java index bd4f3d650..66288e9bb 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/viewmodel/ViewModelKey.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/viewmodel/ViewModelKey.java @@ -14,6 +14,6 @@ import dagger.MapKey; @Target({ElementType.METHOD}) @Retention(RetentionPolicy.RUNTIME) @MapKey -@interface ViewModelKey { +public @interface ViewModelKey { Class value(); } From bcc899eebfc7d1c2d57dc8c1e75167ba3633e5c7 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 9 May 2019 17:46:45 +0100 Subject: [PATCH 4/7] Attach information to ContactExistsException. --- .../api/db/ContactExistsException.java | 19 +++++++++++++++++++ .../contact/ContactExchangeManagerImpl.java | 2 +- .../bramble/db/DatabaseComponentImpl.java | 7 +++---- .../bramble/db/DatabaseComponentImplTest.java | 6 ++++-- 4 files changed, 27 insertions(+), 7 deletions(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/db/ContactExistsException.java b/bramble-api/src/main/java/org/briarproject/bramble/api/db/ContactExistsException.java index 6b51ce5b6..05b5821ef 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/db/ContactExistsException.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/db/ContactExistsException.java @@ -1,8 +1,27 @@ package org.briarproject.bramble.api.db; +import org.briarproject.bramble.api.identity.Author; +import org.briarproject.bramble.api.identity.AuthorId; + /** * Thrown when a duplicate contact is added to the database. This exception may * occur due to concurrent updates and does not indicate a database error. */ public class ContactExistsException extends DbException { + + private final AuthorId local; + private final Author remote; + + public ContactExistsException(AuthorId local, Author remote) { + this.local = local; + this.remote = remote; + } + + public AuthorId getLocalAuthorId() { + return local; + } + + public Author getRemoteAuthor() { + return remote; + } } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java index f68ab538b..bebe3c407 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java @@ -229,7 +229,7 @@ class ContactExchangeManagerImpl implements ContactExchangeManager { logException(LOG, WARNING, e); tryToClose(conn); eventBus.broadcast( - new ContactExchangeFailedEvent(remoteInfo.author)); + new ContactExchangeFailedEvent(e.getRemoteAuthor())); } catch (DbException e) { logException(LOG, WARNING, e); tryToClose(conn); diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java index c4aeb15c5..275d04147 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java @@ -234,16 +234,15 @@ class DatabaseComponentImpl implements DatabaseComponent { @Override public ContactId addContact(Transaction transaction, Author remote, - AuthorId local, boolean verified) - throws DbException { + AuthorId local, boolean verified) throws DbException { if (transaction.isReadOnly()) throw new IllegalArgumentException(); T txn = unbox(transaction); if (!db.containsIdentity(txn, local)) throw new NoSuchIdentityException(); if (db.containsIdentity(txn, remote.getId())) - throw new ContactExistsException(); + throw new ContactExistsException(local, remote); if (db.containsContact(txn, remote.getId(), local)) - throw new ContactExistsException(); + throw new ContactExistsException(local, remote); ContactId c = db.addContact(txn, remote, local, verified); transaction.attach(new ContactAddedEvent(c)); return c; diff --git a/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java index a77acbdd6..24c363e22 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java @@ -1459,7 +1459,8 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { true)); fail(); } catch (ContactExistsException expected) { - // Expected + assertEquals(localAuthor.getId(), expected.getLocalAuthorId()); + assertEquals(author, expected.getRemoteAuthor()); } } @@ -1488,7 +1489,8 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { true)); fail(); } catch (ContactExistsException expected) { - // Expected + assertEquals(localAuthor.getId(), expected.getLocalAuthorId()); + assertEquals(author, expected.getRemoteAuthor()); } } From 5be0e928c4918d4ab9a62686409137c5f4ec94c7 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 24 May 2019 10:50:03 +0100 Subject: [PATCH 5/7] Replace events with return value and exceptions. --- .../api/contact/ContactExchangeManager.java | 12 +- .../event/ContactExchangeFailedEvent.java | 32 ----- .../event/ContactExchangeSucceededEvent.java | 20 --- .../contact/ContactExchangeManagerImpl.java | 115 +++++------------- .../ContactExchangeViewModel.java | 61 ++++------ 5 files changed, 66 insertions(+), 174 deletions(-) delete mode 100644 bramble-api/src/main/java/org/briarproject/bramble/api/contact/event/ContactExchangeFailedEvent.java delete mode 100644 bramble-api/src/main/java/org/briarproject/bramble/api/contact/event/ContactExchangeSucceededEvent.java diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeManager.java b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeManager.java index 67e72bab6..ef7442014 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeManager.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeManager.java @@ -1,16 +1,24 @@ package org.briarproject.bramble.api.contact; import org.briarproject.bramble.api.crypto.SecretKey; +import org.briarproject.bramble.api.db.ContactExistsException; +import org.briarproject.bramble.api.db.DbException; +import org.briarproject.bramble.api.identity.Author; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.plugin.TransportId; import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; +import java.io.IOException; + @NotNullByDefault public interface ContactExchangeManager { /** * Exchanges contact information with a remote peer. + * + * @return The contact's pseudonym + * @throws ContactExistsException If the contact already exists */ - void exchangeContacts(TransportId t, DuplexTransportConnection conn, - SecretKey masterKey, boolean alice); + Author exchangeContacts(TransportId t, DuplexTransportConnection conn, + SecretKey masterKey, boolean alice) throws IOException, DbException; } diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/event/ContactExchangeFailedEvent.java b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/event/ContactExchangeFailedEvent.java deleted file mode 100644 index 0d300f72f..000000000 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/event/ContactExchangeFailedEvent.java +++ /dev/null @@ -1,32 +0,0 @@ -package org.briarproject.bramble.api.contact.event; - -import org.briarproject.bramble.api.event.Event; -import org.briarproject.bramble.api.identity.Author; -import org.briarproject.bramble.api.nullsafety.NotNullByDefault; - -import javax.annotation.Nullable; - -@NotNullByDefault -public class ContactExchangeFailedEvent extends Event { - - @Nullable - private final Author duplicateRemoteAuthor; - - public ContactExchangeFailedEvent(@Nullable Author duplicateRemoteAuthor) { - this.duplicateRemoteAuthor = duplicateRemoteAuthor; - } - - public ContactExchangeFailedEvent() { - this(null); - } - - @Nullable - public Author getDuplicateRemoteAuthor() { - return duplicateRemoteAuthor; - } - - public boolean wasDuplicateContact() { - return duplicateRemoteAuthor != null; - } - -} diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/event/ContactExchangeSucceededEvent.java b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/event/ContactExchangeSucceededEvent.java deleted file mode 100644 index 941a212af..000000000 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/event/ContactExchangeSucceededEvent.java +++ /dev/null @@ -1,20 +0,0 @@ -package org.briarproject.bramble.api.contact.event; - -import org.briarproject.bramble.api.event.Event; -import org.briarproject.bramble.api.identity.Author; -import org.briarproject.bramble.api.nullsafety.NotNullByDefault; - -@NotNullByDefault -public class ContactExchangeSucceededEvent extends Event { - - private final Author remoteAuthor; - - public ContactExchangeSucceededEvent(Author remoteAuthor) { - this.remoteAuthor = remoteAuthor; - } - - public Author getRemoteAuthor() { - return remoteAuthor; - } - -} diff --git a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java index bebe3c407..7c6ba360f 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java @@ -6,16 +6,12 @@ import org.briarproject.bramble.api.client.ClientHelper; import org.briarproject.bramble.api.contact.ContactExchangeManager; import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.contact.ContactManager; -import org.briarproject.bramble.api.contact.event.ContactExchangeFailedEvent; -import org.briarproject.bramble.api.contact.event.ContactExchangeSucceededEvent; import org.briarproject.bramble.api.crypto.CryptoComponent; import org.briarproject.bramble.api.crypto.SecretKey; import org.briarproject.bramble.api.data.BdfDictionary; import org.briarproject.bramble.api.data.BdfList; -import org.briarproject.bramble.api.db.ContactExistsException; import org.briarproject.bramble.api.db.DatabaseComponent; import org.briarproject.bramble.api.db.DbException; -import org.briarproject.bramble.api.event.EventBus; import org.briarproject.bramble.api.identity.Author; import org.briarproject.bramble.api.identity.IdentityManager; import org.briarproject.bramble.api.identity.LocalAuthor; @@ -46,7 +42,6 @@ import java.util.logging.Logger; import javax.inject.Inject; -import static java.util.logging.Level.WARNING; import static java.util.logging.Logger.getLogger; import static org.briarproject.bramble.api.contact.RecordTypes.CONTACT_INFO; import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_SIGNATURE_LENGTH; @@ -56,7 +51,6 @@ import static org.briarproject.bramble.contact.ContactExchangeConstants.BOB_KEY_ import static org.briarproject.bramble.contact.ContactExchangeConstants.BOB_NONCE_LABEL; import static org.briarproject.bramble.contact.ContactExchangeConstants.PROTOCOL_VERSION; import static org.briarproject.bramble.contact.ContactExchangeConstants.SIGNING_LABEL; -import static org.briarproject.bramble.util.LogUtils.logException; import static org.briarproject.bramble.util.ValidationUtils.checkLength; import static org.briarproject.bramble.util.ValidationUtils.checkSize; @@ -85,7 +79,6 @@ class ContactExchangeManagerImpl implements ContactExchangeManager { private final ClientHelper clientHelper; private final RecordReaderFactory recordReaderFactory; private final RecordWriterFactory recordWriterFactory; - private final EventBus eventBus; private final Clock clock; private final ConnectionManager connectionManager; private final ContactManager contactManager; @@ -98,7 +91,7 @@ class ContactExchangeManagerImpl implements ContactExchangeManager { @Inject ContactExchangeManagerImpl(DatabaseComponent db, ClientHelper clientHelper, RecordReaderFactory recordReaderFactory, - RecordWriterFactory recordWriterFactory, EventBus eventBus, + RecordWriterFactory recordWriterFactory, Clock clock, ConnectionManager connectionManager, ContactManager contactManager, IdentityManager identityManager, TransportPropertyManager transportPropertyManager, @@ -108,7 +101,6 @@ class ContactExchangeManagerImpl implements ContactExchangeManager { this.clientHelper = clientHelper; this.recordReaderFactory = recordReaderFactory; this.recordWriterFactory = recordWriterFactory; - this.eventBus = eventBus; this.clock = clock; this.connectionManager = connectionManager; this.contactManager = contactManager; @@ -120,33 +112,17 @@ class ContactExchangeManagerImpl implements ContactExchangeManager { } @Override - public void exchangeContacts(TransportId t, DuplexTransportConnection conn, - SecretKey masterKey, boolean alice) { + public Author exchangeContacts(TransportId t, + DuplexTransportConnection conn, SecretKey masterKey, boolean alice) + throws IOException, DbException { // Get the transport connection's input and output streams - InputStream in; - OutputStream out; - try { - in = conn.getReader().getInputStream(); - out = conn.getWriter().getOutputStream(); - } catch (IOException e) { - logException(LOG, WARNING, e); - tryToClose(conn); - eventBus.broadcast(new ContactExchangeFailedEvent()); - return; - } + InputStream in = conn.getReader().getInputStream(); + OutputStream out = conn.getWriter().getOutputStream(); // Get the local author and transport properties - LocalAuthor localAuthor; - Map localProperties; - try { - localAuthor = identityManager.getLocalAuthor(); - localProperties = transportPropertyManager.getLocalProperties(); - } catch (DbException e) { - logException(LOG, WARNING, e); - eventBus.broadcast(new ContactExchangeFailedEvent()); - tryToClose(conn); - return; - } + LocalAuthor localAuthor = identityManager.getLocalAuthor(); + Map localProperties = + transportPropertyManager.getLocalProperties(); // Derive the header keys for the transport streams SecretKey aliceHeaderKey = crypto.deriveKey(ALICE_KEY_LABEL, masterKey, @@ -183,58 +159,37 @@ class ContactExchangeManagerImpl implements ContactExchangeManager { // Exchange contact info long localTimestamp = clock.currentTimeMillis(); ContactInfo remoteInfo; - try { - if (alice) { - sendContactInfo(recordWriter, localAuthor, localProperties, - localSignature, localTimestamp); - remoteInfo = receiveContactInfo(recordReader); - } else { - remoteInfo = receiveContactInfo(recordReader); - sendContactInfo(recordWriter, localAuthor, localProperties, - localSignature, localTimestamp); - } - // Send EOF on the outgoing stream - streamWriter.sendEndOfStream(); - // Skip any remaining records from the incoming stream - recordReader.readRecord(r -> false, IGNORE); - } catch (IOException e) { - logException(LOG, WARNING, e); - eventBus.broadcast(new ContactExchangeFailedEvent()); - tryToClose(conn); - return; + if (alice) { + sendContactInfo(recordWriter, localAuthor, localProperties, + localSignature, localTimestamp); + remoteInfo = receiveContactInfo(recordReader); + } else { + remoteInfo = receiveContactInfo(recordReader); + sendContactInfo(recordWriter, localAuthor, localProperties, + localSignature, localTimestamp); } + // Send EOF on the outgoing stream + streamWriter.sendEndOfStream(); + // Skip any remaining records from the incoming stream + recordReader.readRecord(r -> false, IGNORE); // Verify the contact's signature if (!verify(remoteInfo.author, remoteNonce, remoteInfo.signature)) { LOG.warning("Invalid signature"); - eventBus.broadcast(new ContactExchangeFailedEvent()); - tryToClose(conn); - return; + throw new FormatException(); } // The agreed timestamp is the minimum of the peers' timestamps long timestamp = Math.min(localTimestamp, remoteInfo.timestamp); - try { - // Add the contact - ContactId contactId = addContact(remoteInfo.author, localAuthor, - masterKey, timestamp, alice, remoteInfo.properties); - // Reuse the connection as a transport connection - connectionManager.manageOutgoingConnection(contactId, t, conn); - // Pseudonym exchange succeeded - LOG.info("Pseudonym exchange succeeded"); - eventBus.broadcast( - new ContactExchangeSucceededEvent(remoteInfo.author)); - } catch (ContactExistsException e) { - logException(LOG, WARNING, e); - tryToClose(conn); - eventBus.broadcast( - new ContactExchangeFailedEvent(e.getRemoteAuthor())); - } catch (DbException e) { - logException(LOG, WARNING, e); - tryToClose(conn); - eventBus.broadcast(new ContactExchangeFailedEvent()); - } + // Add the contact + ContactId contactId = addContact(remoteInfo.author, localAuthor, + masterKey, timestamp, alice, remoteInfo.properties); + // Reuse the connection as a transport connection + connectionManager.manageOutgoingConnection(contactId, t, conn); + // Pseudonym exchange succeeded + LOG.info("Pseudonym exchange succeeded"); + return remoteInfo.author; } private byte[] sign(LocalAuthor author, byte[] nonce) { @@ -298,16 +253,6 @@ class ContactExchangeManagerImpl implements ContactExchangeManager { }); } - private void tryToClose(DuplexTransportConnection conn) { - try { - LOG.info("Closing connection"); - conn.getReader().dispose(true, true); - conn.getWriter().dispose(true); - } catch (IOException e) { - logException(LOG, WARNING, e); - } - } - private static class ContactInfo { private final Author author; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeViewModel.java index 5b990f2d8..149f92b8d 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeViewModel.java @@ -7,58 +7,63 @@ import android.arch.lifecycle.MutableLiveData; import android.support.annotation.UiThread; import org.briarproject.bramble.api.contact.ContactExchangeManager; -import org.briarproject.bramble.api.contact.event.ContactExchangeFailedEvent; -import org.briarproject.bramble.api.contact.event.ContactExchangeSucceededEvent; import org.briarproject.bramble.api.crypto.SecretKey; -import org.briarproject.bramble.api.event.Event; -import org.briarproject.bramble.api.event.EventBus; -import org.briarproject.bramble.api.event.EventListener; +import org.briarproject.bramble.api.db.ContactExistsException; +import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.identity.Author; import org.briarproject.bramble.api.lifecycle.IoExecutor; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.plugin.TransportId; import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; +import java.io.IOException; import java.util.concurrent.Executor; +import java.util.logging.Logger; import javax.annotation.Nullable; import javax.inject.Inject; -import static java.util.Objects.requireNonNull; +import static java.util.logging.Level.WARNING; +import static java.util.logging.Logger.getLogger; +import static org.briarproject.bramble.util.LogUtils.logException; @NotNullByDefault -class ContactExchangeViewModel extends AndroidViewModel - implements EventListener { +class ContactExchangeViewModel extends AndroidViewModel { + + private static final Logger LOG = + getLogger(ContactExchangeViewModel.class.getName()); private final Executor ioExecutor; private final ContactExchangeManager contactExchangeManager; - private final EventBus eventBus; private final MutableLiveData succeeded = new MutableLiveData<>(); @Nullable - private Author remoteAuthor, duplicateAuthor; + private volatile Author remoteAuthor, duplicateAuthor; @Inject ContactExchangeViewModel(Application app, @IoExecutor Executor ioExecutor, - ContactExchangeManager contactExchangeManager, EventBus eventBus) { + ContactExchangeManager contactExchangeManager) { super(app); this.ioExecutor = ioExecutor; this.contactExchangeManager = contactExchangeManager; - this.eventBus = eventBus; - eventBus.addListener(this); - } - - @Override - protected void onCleared() { - super.onCleared(); - eventBus.removeListener(this); } @UiThread void startContactExchange(TransportId t, DuplexTransportConnection conn, SecretKey masterKey, boolean alice) { - ioExecutor.execute(() -> contactExchangeManager.exchangeContacts(t, - conn, masterKey, alice)); + ioExecutor.execute(() -> { + try { + remoteAuthor = contactExchangeManager.exchangeContacts(t, conn, + masterKey, alice); + succeeded.postValue(true); + } catch (ContactExistsException e) { + duplicateAuthor = e.getRemoteAuthor(); + succeeded.postValue(false); + } catch (DbException | IOException e) { + logException(LOG, WARNING, e); + succeeded.postValue(false); + } + }); } @UiThread @@ -76,18 +81,4 @@ class ContactExchangeViewModel extends AndroidViewModel LiveData getSucceeded() { return succeeded; } - - @Override - public void eventOccurred(Event e) { - if (e instanceof ContactExchangeSucceededEvent) { - ContactExchangeSucceededEvent c = (ContactExchangeSucceededEvent) e; - remoteAuthor = c.getRemoteAuthor(); - succeeded.setValue(true); - } else if (e instanceof ContactExchangeFailedEvent) { - ContactExchangeFailedEvent c = (ContactExchangeFailedEvent) e; - if (c.wasDuplicateContact()) - duplicateAuthor = requireNonNull(c.getDuplicateRemoteAuthor()); - succeeded.setValue(false); - } - } } From f1e5c2dd6635d5fffcabdfbcb745995d86af818b Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 24 May 2019 11:40:12 +0100 Subject: [PATCH 6/7] Return a contact, encapsulate contact exchange crypto. --- .../api/contact/ContactExchangeManager.java | 6 +- .../bramble/api/contact/ContactManager.java | 5 + .../contact/ContactExchangeCrypto.java | 35 +++++++ .../contact/ContactExchangeCryptoImpl.java | 66 +++++++++++++ .../contact/ContactExchangeManagerImpl.java | 96 +++++++------------ .../bramble/contact/ContactManagerImpl.java | 5 + .../bramble/contact/ContactModule.java | 6 ++ .../ContactExchangeViewModel.java | 15 ++- 8 files changed, 164 insertions(+), 70 deletions(-) create mode 100644 bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeCrypto.java create mode 100644 bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeCryptoImpl.java diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeManager.java b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeManager.java index ef7442014..0981158f0 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeManager.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactExchangeManager.java @@ -3,7 +3,6 @@ package org.briarproject.bramble.api.contact; import org.briarproject.bramble.api.crypto.SecretKey; import org.briarproject.bramble.api.db.ContactExistsException; import org.briarproject.bramble.api.db.DbException; -import org.briarproject.bramble.api.identity.Author; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.plugin.TransportId; import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; @@ -16,9 +15,10 @@ public interface ContactExchangeManager { /** * Exchanges contact information with a remote peer. * - * @return The contact's pseudonym + * @param alice Whether the local peer takes the role of Alice + * @return The newly added contact * @throws ContactExistsException If the contact already exists */ - Author exchangeContacts(TransportId t, DuplexTransportConnection conn, + Contact exchangeContacts(TransportId t, DuplexTransportConnection conn, SecretKey masterKey, boolean alice) throws IOException, DbException; } diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactManager.java b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactManager.java index 99988094d..2b18a17a5 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactManager.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactManager.java @@ -93,6 +93,11 @@ public interface ContactManager { */ Contact getContact(ContactId c) throws DbException; + /** + * Returns the contact with the given ID. + */ + Contact getContact(Transaction txn, ContactId c) throws DbException; + /** * Returns the contact with the given remoteAuthorId * that was added by the LocalAuthor with the given localAuthorId diff --git a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeCrypto.java b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeCrypto.java new file mode 100644 index 000000000..5c21eb23a --- /dev/null +++ b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeCrypto.java @@ -0,0 +1,35 @@ +package org.briarproject.bramble.contact; + +import org.briarproject.bramble.api.crypto.PrivateKey; +import org.briarproject.bramble.api.crypto.PublicKey; +import org.briarproject.bramble.api.crypto.SecretKey; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + +@NotNullByDefault +interface ContactExchangeCrypto { + + /** + * Derives the header key for a contact exchange stream from the master key. + * + * @param alice Whether the header key is for the stream sent by Alice + */ + SecretKey deriveHeaderKey(SecretKey masterKey, boolean alice); + + /** + * Creates and returns a signature that proves ownership of a pseudonym. + * + * @param privateKey The pseudonym's signature private key + * @param alice Whether the pseudonym belongs to Alice + */ + byte[] sign(PrivateKey privateKey, SecretKey masterKey, boolean alice); + + /** + * Verifies a signature that proves ownership of a pseudonym. + * + * @param publicKey The pseudonym's signature public key + * @param alice Whether the pseudonym belongs to Alice + * @return True if the signature is valid + */ + boolean verify(PublicKey publicKey, SecretKey masterKey, boolean alice, + byte[] signature); +} diff --git a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeCryptoImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeCryptoImpl.java new file mode 100644 index 000000000..6a3a3c44a --- /dev/null +++ b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeCryptoImpl.java @@ -0,0 +1,66 @@ +package org.briarproject.bramble.contact; + +import org.briarproject.bramble.api.crypto.CryptoComponent; +import org.briarproject.bramble.api.crypto.PrivateKey; +import org.briarproject.bramble.api.crypto.PublicKey; +import org.briarproject.bramble.api.crypto.SecretKey; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + +import java.security.GeneralSecurityException; + +import javax.inject.Inject; + +import static org.briarproject.bramble.contact.ContactExchangeConstants.ALICE_KEY_LABEL; +import static org.briarproject.bramble.contact.ContactExchangeConstants.ALICE_NONCE_LABEL; +import static org.briarproject.bramble.contact.ContactExchangeConstants.BOB_KEY_LABEL; +import static org.briarproject.bramble.contact.ContactExchangeConstants.BOB_NONCE_LABEL; +import static org.briarproject.bramble.contact.ContactExchangeConstants.PROTOCOL_VERSION; +import static org.briarproject.bramble.contact.ContactExchangeConstants.SIGNING_LABEL; + +@NotNullByDefault +class ContactExchangeCryptoImpl implements ContactExchangeCrypto { + + private static final byte[] PROTOCOL_VERSION_BYTES = + new byte[] {PROTOCOL_VERSION}; + + private final CryptoComponent crypto; + + @Inject + ContactExchangeCryptoImpl(CryptoComponent crypto) { + this.crypto = crypto; + } + + @Override + public SecretKey deriveHeaderKey(SecretKey masterKey, boolean alice) { + String label = alice ? ALICE_KEY_LABEL : BOB_KEY_LABEL; + return crypto.deriveKey(label, masterKey, PROTOCOL_VERSION_BYTES); + } + + @Override + public byte[] sign(PrivateKey privateKey, SecretKey masterKey, + boolean alice) { + byte[] nonce = deriveNonce(masterKey, alice); + try { + return crypto.sign(SIGNING_LABEL, nonce, privateKey); + } catch (GeneralSecurityException e) { + throw new AssertionError(); + } + } + + @Override + public boolean verify(PublicKey publicKey, + SecretKey masterKey, boolean alice, byte[] signature) { + byte[] nonce = deriveNonce(masterKey, alice); + try { + return crypto.verifySignature(signature, SIGNING_LABEL, nonce, + publicKey); + } catch (GeneralSecurityException e) { + return false; + } + } + + private byte[] deriveNonce(SecretKey masterKey, boolean alice) { + String label = alice ? ALICE_NONCE_LABEL : BOB_NONCE_LABEL; + return crypto.mac(label, masterKey, PROTOCOL_VERSION_BYTES); + } +} diff --git a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java index 7c6ba360f..49ed255f2 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactExchangeManagerImpl.java @@ -3,10 +3,11 @@ package org.briarproject.bramble.contact; import org.briarproject.bramble.api.FormatException; import org.briarproject.bramble.api.Predicate; import org.briarproject.bramble.api.client.ClientHelper; +import org.briarproject.bramble.api.contact.Contact; import org.briarproject.bramble.api.contact.ContactExchangeManager; import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.contact.ContactManager; -import org.briarproject.bramble.api.crypto.CryptoComponent; +import org.briarproject.bramble.api.crypto.PublicKey; import org.briarproject.bramble.api.crypto.SecretKey; import org.briarproject.bramble.api.data.BdfDictionary; import org.briarproject.bramble.api.data.BdfList; @@ -17,7 +18,6 @@ import org.briarproject.bramble.api.identity.IdentityManager; import org.briarproject.bramble.api.identity.LocalAuthor; import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault; import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault; -import org.briarproject.bramble.api.plugin.ConnectionManager; import org.briarproject.bramble.api.plugin.TransportId; import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; import org.briarproject.bramble.api.properties.TransportProperties; @@ -36,7 +36,6 @@ import java.io.EOFException; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.security.GeneralSecurityException; import java.util.Map; import java.util.logging.Logger; @@ -45,12 +44,7 @@ import javax.inject.Inject; import static java.util.logging.Logger.getLogger; import static org.briarproject.bramble.api.contact.RecordTypes.CONTACT_INFO; import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_SIGNATURE_LENGTH; -import static org.briarproject.bramble.contact.ContactExchangeConstants.ALICE_KEY_LABEL; -import static org.briarproject.bramble.contact.ContactExchangeConstants.ALICE_NONCE_LABEL; -import static org.briarproject.bramble.contact.ContactExchangeConstants.BOB_KEY_LABEL; -import static org.briarproject.bramble.contact.ContactExchangeConstants.BOB_NONCE_LABEL; import static org.briarproject.bramble.contact.ContactExchangeConstants.PROTOCOL_VERSION; -import static org.briarproject.bramble.contact.ContactExchangeConstants.SIGNING_LABEL; import static org.briarproject.bramble.util.ValidationUtils.checkLength; import static org.briarproject.bramble.util.ValidationUtils.checkSize; @@ -80,39 +74,37 @@ class ContactExchangeManagerImpl implements ContactExchangeManager { private final RecordReaderFactory recordReaderFactory; private final RecordWriterFactory recordWriterFactory; private final Clock clock; - private final ConnectionManager connectionManager; private final ContactManager contactManager; private final IdentityManager identityManager; private final TransportPropertyManager transportPropertyManager; - private final CryptoComponent crypto; + private final ContactExchangeCrypto contactExchangeCrypto; private final StreamReaderFactory streamReaderFactory; private final StreamWriterFactory streamWriterFactory; @Inject ContactExchangeManagerImpl(DatabaseComponent db, ClientHelper clientHelper, RecordReaderFactory recordReaderFactory, - RecordWriterFactory recordWriterFactory, - Clock clock, ConnectionManager connectionManager, + RecordWriterFactory recordWriterFactory, Clock clock, ContactManager contactManager, IdentityManager identityManager, TransportPropertyManager transportPropertyManager, - CryptoComponent crypto, StreamReaderFactory streamReaderFactory, + ContactExchangeCrypto contactExchangeCrypto, + StreamReaderFactory streamReaderFactory, StreamWriterFactory streamWriterFactory) { this.db = db; this.clientHelper = clientHelper; this.recordReaderFactory = recordReaderFactory; this.recordWriterFactory = recordWriterFactory; this.clock = clock; - this.connectionManager = connectionManager; this.contactManager = contactManager; this.identityManager = identityManager; this.transportPropertyManager = transportPropertyManager; - this.crypto = crypto; + this.contactExchangeCrypto = contactExchangeCrypto; this.streamReaderFactory = streamReaderFactory; this.streamWriterFactory = streamWriterFactory; } @Override - public Author exchangeContacts(TransportId t, + public Contact exchangeContacts(TransportId t, DuplexTransportConnection conn, SecretKey masterKey, boolean alice) throws IOException, DbException { // Get the transport connection's input and output streams @@ -125,36 +117,26 @@ class ContactExchangeManagerImpl implements ContactExchangeManager { transportPropertyManager.getLocalProperties(); // Derive the header keys for the transport streams - SecretKey aliceHeaderKey = crypto.deriveKey(ALICE_KEY_LABEL, masterKey, - new byte[] {PROTOCOL_VERSION}); - SecretKey bobHeaderKey = crypto.deriveKey(BOB_KEY_LABEL, masterKey, - new byte[] {PROTOCOL_VERSION}); + SecretKey localHeaderKey = + contactExchangeCrypto.deriveHeaderKey(masterKey, alice); + SecretKey remoteHeaderKey = + contactExchangeCrypto.deriveHeaderKey(masterKey, !alice); // Create the readers - InputStream streamReader = - streamReaderFactory.createContactExchangeStreamReader(in, - alice ? bobHeaderKey : aliceHeaderKey); + InputStream streamReader = streamReaderFactory + .createContactExchangeStreamReader(in, remoteHeaderKey); RecordReader recordReader = recordReaderFactory.createRecordReader(streamReader); // Create the writers - StreamWriter streamWriter = - streamWriterFactory.createContactExchangeStreamWriter(out, - alice ? aliceHeaderKey : bobHeaderKey); - RecordWriter recordWriter = - recordWriterFactory - .createRecordWriter(streamWriter.getOutputStream()); + StreamWriter streamWriter = streamWriterFactory + .createContactExchangeStreamWriter(out, localHeaderKey); + RecordWriter recordWriter = recordWriterFactory + .createRecordWriter(streamWriter.getOutputStream()); - // Derive the nonces to be signed - byte[] aliceNonce = crypto.mac(ALICE_NONCE_LABEL, masterKey, - new byte[] {PROTOCOL_VERSION}); - byte[] bobNonce = crypto.mac(BOB_NONCE_LABEL, masterKey, - new byte[] {PROTOCOL_VERSION}); - byte[] localNonce = alice ? aliceNonce : bobNonce; - byte[] remoteNonce = alice ? bobNonce : aliceNonce; - - // Sign the nonce - byte[] localSignature = sign(localAuthor, localNonce); + // Create our signature + byte[] localSignature = contactExchangeCrypto + .sign(localAuthor.getPrivateKey(), masterKey, alice); // Exchange contact info long localTimestamp = clock.currentTimeMillis(); @@ -168,13 +150,17 @@ class ContactExchangeManagerImpl implements ContactExchangeManager { sendContactInfo(recordWriter, localAuthor, localProperties, localSignature, localTimestamp); } + // Send EOF on the outgoing stream streamWriter.sendEndOfStream(); + // Skip any remaining records from the incoming stream recordReader.readRecord(r -> false, IGNORE); // Verify the contact's signature - if (!verify(remoteInfo.author, remoteNonce, remoteInfo.signature)) { + PublicKey remotePublicKey = remoteInfo.author.getPublicKey(); + if (!contactExchangeCrypto.verify(remotePublicKey, + masterKey, !alice, remoteInfo.signature)) { LOG.warning("Invalid signature"); throw new FormatException(); } @@ -183,30 +169,12 @@ class ContactExchangeManagerImpl implements ContactExchangeManager { long timestamp = Math.min(localTimestamp, remoteInfo.timestamp); // Add the contact - ContactId contactId = addContact(remoteInfo.author, localAuthor, + Contact contact = addContact(remoteInfo.author, localAuthor, masterKey, timestamp, alice, remoteInfo.properties); - // Reuse the connection as a transport connection - connectionManager.manageOutgoingConnection(contactId, t, conn); - // Pseudonym exchange succeeded - LOG.info("Pseudonym exchange succeeded"); - return remoteInfo.author; - } - private byte[] sign(LocalAuthor author, byte[] nonce) { - try { - return crypto.sign(SIGNING_LABEL, nonce, author.getPrivateKey()); - } catch (GeneralSecurityException e) { - throw new AssertionError(); - } - } - - private boolean verify(Author author, byte[] nonce, byte[] signature) { - try { - return crypto.verifySignature(signature, SIGNING_LABEL, nonce, - author.getPublicKey()); - } catch (GeneralSecurityException e) { - return false; - } + // Contact exchange succeeded + LOG.info("Contact exchange succeeded"); + return contact; } private void sendContactInfo(RecordWriter recordWriter, Author author, @@ -239,7 +207,7 @@ class ContactExchangeManagerImpl implements ContactExchangeManager { return new ContactInfo(author, properties, signature, timestamp); } - private ContactId addContact(Author remoteAuthor, LocalAuthor localAuthor, + private Contact addContact(Author remoteAuthor, LocalAuthor localAuthor, SecretKey masterKey, long timestamp, boolean alice, Map remoteProperties) throws DbException { @@ -249,7 +217,7 @@ class ContactExchangeManagerImpl implements ContactExchangeManager { true, true); transportPropertyManager.addRemoteProperties(txn, contactId, remoteProperties); - return contactId; + return contactManager.getContact(txn, contactId); }); } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactManagerImpl.java index d6e2d0782..510be77a0 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactManagerImpl.java @@ -137,6 +137,11 @@ class ContactManagerImpl implements ContactManager { return db.transactionWithResult(true, txn -> db.getContact(txn, c)); } + @Override + public Contact getContact(Transaction txn, ContactId c) throws DbException { + return db.getContact(txn, c); + } + @Override public Contact getContact(AuthorId remoteAuthorId, AuthorId localAuthorId) throws DbException { diff --git a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactModule.java b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactModule.java index 614edd4ea..97af570b6 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactModule.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactModule.java @@ -34,4 +34,10 @@ public class ContactModule { PendingContactFactoryImpl pendingContactFactory) { return pendingContactFactory; } + + @Provides + ContactExchangeCrypto provideContactExchangeCrypto( + ContactExchangeCryptoImpl contactExchangeCrypto) { + return contactExchangeCrypto; + } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeViewModel.java index 149f92b8d..259e43f68 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeViewModel.java @@ -6,6 +6,7 @@ import android.arch.lifecycle.LiveData; import android.arch.lifecycle.MutableLiveData; import android.support.annotation.UiThread; +import org.briarproject.bramble.api.contact.Contact; import org.briarproject.bramble.api.contact.ContactExchangeManager; import org.briarproject.bramble.api.crypto.SecretKey; import org.briarproject.bramble.api.db.ContactExistsException; @@ -13,6 +14,7 @@ import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.identity.Author; import org.briarproject.bramble.api.lifecycle.IoExecutor; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import org.briarproject.bramble.api.plugin.ConnectionManager; import org.briarproject.bramble.api.plugin.TransportId; import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; @@ -35,6 +37,7 @@ class ContactExchangeViewModel extends AndroidViewModel { private final Executor ioExecutor; private final ContactExchangeManager contactExchangeManager; + private final ConnectionManager connectionManager; private final MutableLiveData succeeded = new MutableLiveData<>(); @Nullable @@ -42,10 +45,12 @@ class ContactExchangeViewModel extends AndroidViewModel { @Inject ContactExchangeViewModel(Application app, @IoExecutor Executor ioExecutor, - ContactExchangeManager contactExchangeManager) { + ContactExchangeManager contactExchangeManager, + ConnectionManager connectionManager) { super(app); this.ioExecutor = ioExecutor; this.contactExchangeManager = contactExchangeManager; + this.connectionManager = connectionManager; } @UiThread @@ -53,8 +58,12 @@ class ContactExchangeViewModel extends AndroidViewModel { SecretKey masterKey, boolean alice) { ioExecutor.execute(() -> { try { - remoteAuthor = contactExchangeManager.exchangeContacts(t, conn, - masterKey, alice); + Contact contact = contactExchangeManager.exchangeContacts(t, + conn, masterKey, alice); + // Reuse the connection as a transport connection + connectionManager.manageOutgoingConnection(contact.getId(), + t, conn); + remoteAuthor = contact.getAuthor(); succeeded.postValue(true); } catch (ContactExistsException e) { duplicateAuthor = e.getRemoteAuthor(); From d80c77f4665f818b75bdf674211da0c9615d195b Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 24 May 2019 13:14:53 +0100 Subject: [PATCH 7/7] Try to close connection if contact exchange fails. --- .../keyagreement/ContactExchangeViewModel.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeViewModel.java index 259e43f68..a7ada2cbe 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/ContactExchangeViewModel.java @@ -66,15 +66,26 @@ class ContactExchangeViewModel extends AndroidViewModel { remoteAuthor = contact.getAuthor(); succeeded.postValue(true); } catch (ContactExistsException e) { + tryToClose(conn); duplicateAuthor = e.getRemoteAuthor(); succeeded.postValue(false); } catch (DbException | IOException e) { + tryToClose(conn); logException(LOG, WARNING, e); succeeded.postValue(false); } }); } + private void tryToClose(DuplexTransportConnection conn) { + try { + conn.getReader().dispose(true, true); + conn.getWriter().dispose(true); + } catch (IOException e) { + logException(LOG, WARNING, e); + } + } + @UiThread @Nullable Author getRemoteAuthor() {