From 1d89c6cebc5fd36c90317447fac4ec3f4a4c59b7 Mon Sep 17 00:00:00 2001
From: akwizgran <akwizgran@users.sourceforge.net>
Date: Fri, 26 Feb 2016 10:57:32 +0000
Subject: [PATCH] Add contact and transport keys in the same transaction.
This avoids a potential problem where the app crashes after adding the contact but before adding the transport keys, leaving the contact unusable.
---
.../api/contact/ContactManager.java | 5 ++--
.../api/transport/KeyManager.java | 6 ++--
.../contact/ContactManagerImpl.java | 10 +++++--
.../briarproject/invitation/Connector.java | 4 +--
.../transport/KeyManagerImpl.java | 6 ++--
.../transport/TransportKeyManager.java | 14 ++-------
.../org/briarproject/sync/ConstantsTest.java | 4 ++-
.../sync/SimplexMessagingIntegrationTest.java | 8 ++---
.../transport/TransportKeyManagerTest.java | 30 +++++++------------
9 files changed, 37 insertions(+), 50 deletions(-)
diff --git a/briar-api/src/org/briarproject/api/contact/ContactManager.java b/briar-api/src/org/briarproject/api/contact/ContactManager.java
index 47095d2ba2..ba1d2c6630 100644
--- a/briar-api/src/org/briarproject/api/contact/ContactManager.java
+++ b/briar-api/src/org/briarproject/api/contact/ContactManager.java
@@ -1,5 +1,6 @@
package org.briarproject.api.contact;
+import org.briarproject.api.crypto.SecretKey;
import org.briarproject.api.db.DbException;
import org.briarproject.api.db.Transaction;
import org.briarproject.api.identity.Author;
@@ -19,8 +20,8 @@ public interface ContactManager {
* Stores a contact associated with the given local and remote pseudonyms,
* and returns an ID for the contact.
*/
- ContactId addContact(Author remote, AuthorId local, boolean active)
- throws DbException;
+ ContactId addContact(Author remote, AuthorId local, SecretKey master,
+ long timestamp, boolean alice, boolean active) throws DbException;
/** Returns the contact with the given ID. */
Contact getContact(ContactId c) throws DbException;
diff --git a/briar-api/src/org/briarproject/api/transport/KeyManager.java b/briar-api/src/org/briarproject/api/transport/KeyManager.java
index 1d6ece09af..7cfd4d1eb9 100644
--- a/briar-api/src/org/briarproject/api/transport/KeyManager.java
+++ b/briar-api/src/org/briarproject/api/transport/KeyManager.java
@@ -3,6 +3,8 @@ package org.briarproject.api.transport;
import org.briarproject.api.TransportId;
import org.briarproject.api.contact.ContactId;
import org.briarproject.api.crypto.SecretKey;
+import org.briarproject.api.db.DbException;
+import org.briarproject.api.db.Transaction;
/**
* Responsible for managing transport keys and recognising the pseudo-random
@@ -16,8 +18,8 @@ public interface KeyManager {
* {@link StreamContext StreamContexts} for the contact can be created
* after this method has returned.
*/
- void addContact(ContactId c, SecretKey master, long timestamp,
- boolean alice);
+ void addContact(Transaction txn, ContactId c, SecretKey master,
+ long timestamp, boolean alice) throws DbException;
/**
* Returns a {@link StreamContext} for sending a stream to the given
diff --git a/briar-core/src/org/briarproject/contact/ContactManagerImpl.java b/briar-core/src/org/briarproject/contact/ContactManagerImpl.java
index 03a4eae724..4f5698acfa 100644
--- a/briar-core/src/org/briarproject/contact/ContactManagerImpl.java
+++ b/briar-core/src/org/briarproject/contact/ContactManagerImpl.java
@@ -5,6 +5,7 @@ import com.google.inject.Inject;
import org.briarproject.api.contact.Contact;
import org.briarproject.api.contact.ContactId;
import org.briarproject.api.contact.ContactManager;
+import org.briarproject.api.crypto.SecretKey;
import org.briarproject.api.db.DatabaseComponent;
import org.briarproject.api.db.DbException;
import org.briarproject.api.db.Transaction;
@@ -12,6 +13,7 @@ import org.briarproject.api.identity.Author;
import org.briarproject.api.identity.AuthorId;
import org.briarproject.api.identity.IdentityManager.RemoveIdentityHook;
import org.briarproject.api.identity.LocalAuthor;
+import org.briarproject.api.transport.KeyManager;
import java.util.ArrayList;
import java.util.Collection;
@@ -22,12 +24,14 @@ import java.util.concurrent.CopyOnWriteArrayList;
class ContactManagerImpl implements ContactManager, RemoveIdentityHook {
private final DatabaseComponent db;
+ private final KeyManager keyManager;
private final List<AddContactHook> addHooks;
private final List<RemoveContactHook> removeHooks;
@Inject
- ContactManagerImpl(DatabaseComponent db) {
+ ContactManagerImpl(DatabaseComponent db, KeyManager keyManager) {
this.db = db;
+ this.keyManager = keyManager;
addHooks = new CopyOnWriteArrayList<AddContactHook>();
removeHooks = new CopyOnWriteArrayList<RemoveContactHook>();
}
@@ -43,12 +47,14 @@ class ContactManagerImpl implements ContactManager, RemoveIdentityHook {
}
@Override
- public ContactId addContact(Author remote, AuthorId local, boolean active)
+ public ContactId addContact(Author remote, AuthorId local, SecretKey master,
+ long timestamp, boolean alice, boolean active)
throws DbException {
ContactId c;
Transaction txn = db.startTransaction();
try {
c = db.addContact(txn, remote, local, active);
+ keyManager.addContact(txn, c, master, timestamp, alice);
Contact contact = db.getContact(txn, c);
for (AddContactHook hook : addHooks)
hook.addingContact(txn, contact);
diff --git a/briar-core/src/org/briarproject/invitation/Connector.java b/briar-core/src/org/briarproject/invitation/Connector.java
index 4bd7fc2ea6..702671364f 100644
--- a/briar-core/src/org/briarproject/invitation/Connector.java
+++ b/briar-core/src/org/briarproject/invitation/Connector.java
@@ -219,9 +219,7 @@ abstract class Connector extends Thread {
long timestamp, boolean alice) throws DbException {
// Add the contact to the database
contactId = contactManager.addContact(remoteAuthor,
- localAuthor.getId(), true);
- // Derive transport keys
- keyManager.addContact(contactId, master, timestamp, alice);
+ localAuthor.getId(), master, timestamp, alice, true);
return contactId;
}
diff --git a/briar-core/src/org/briarproject/transport/KeyManagerImpl.java b/briar-core/src/org/briarproject/transport/KeyManagerImpl.java
index a4aede92e3..3cbfef824f 100644
--- a/briar-core/src/org/briarproject/transport/KeyManagerImpl.java
+++ b/briar-core/src/org/briarproject/transport/KeyManagerImpl.java
@@ -88,10 +88,10 @@ class KeyManagerImpl implements KeyManager, Service, EventListener {
return true;
}
- public void addContact(ContactId c, SecretKey master, long timestamp,
- boolean alice) {
+ public void addContact(Transaction txn, ContactId c, SecretKey master,
+ long timestamp, boolean alice) throws DbException {
for (TransportKeyManager m : managers.values())
- m.addContact(c, master, timestamp, alice);
+ m.addContact(txn, c, master, timestamp, alice);
}
public StreamContext getStreamContext(ContactId c, TransportId t) {
diff --git a/briar-core/src/org/briarproject/transport/TransportKeyManager.java b/briar-core/src/org/briarproject/transport/TransportKeyManager.java
index ffc8c3bbfc..b8bcdfb3f9 100644
--- a/briar-core/src/org/briarproject/transport/TransportKeyManager.java
+++ b/briar-core/src/org/briarproject/transport/TransportKeyManager.java
@@ -149,8 +149,8 @@ class TransportKeyManager {
timer.schedule(task, delay);
}
- void addContact(ContactId c, SecretKey master, long timestamp,
- boolean alice) {
+ void addContact(Transaction txn, ContactId c, SecretKey master,
+ long timestamp, boolean alice) throws DbException {
lock.lock();
try {
// Work out what rotation period the timestamp belongs to
@@ -164,15 +164,7 @@ class TransportKeyManager {
// Initialise mutable state for the contact
addKeys(c, new MutableTransportKeys(k));
// Write the keys back to the DB
- Transaction txn = db.startTransaction();
- try {
- db.addTransportKeys(txn, c, k);
- txn.setComplete();
- } finally {
- db.endTransaction(txn);
- }
- } catch (DbException e) {
- if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
+ db.addTransportKeys(txn, c, k);
} finally {
lock.unlock();
}
diff --git a/briar-tests/src/org/briarproject/sync/ConstantsTest.java b/briar-tests/src/org/briarproject/sync/ConstantsTest.java
index 9f31dce47d..f3af582b87 100644
--- a/briar-tests/src/org/briarproject/sync/ConstantsTest.java
+++ b/briar-tests/src/org/briarproject/sync/ConstantsTest.java
@@ -31,6 +31,7 @@ import org.briarproject.event.EventModule;
import org.briarproject.forum.ForumModule;
import org.briarproject.identity.IdentityModule;
import org.briarproject.messaging.MessagingModule;
+import org.briarproject.transport.TransportModule;
import org.junit.Test;
import java.util.Random;
@@ -57,7 +58,8 @@ public class ConstantsTest extends BriarTestCase {
new TestLifecycleModule(), new TestSystemModule(),
new ContactModule(), new CryptoModule(), new DatabaseModule(),
new DataModule(), new EventModule(), new ForumModule(),
- new IdentityModule(), new MessagingModule(), new SyncModule());
+ new IdentityModule(), new MessagingModule(), new SyncModule(),
+ new TransportModule());
crypto = i.getInstance(CryptoComponent.class);
authorFactory = i.getInstance(AuthorFactory.class);
privateMessageFactory = i.getInstance(PrivateMessageFactory.class);
diff --git a/briar-tests/src/org/briarproject/sync/SimplexMessagingIntegrationTest.java b/briar-tests/src/org/briarproject/sync/SimplexMessagingIntegrationTest.java
index 62e9a613b7..87abf08f1d 100644
--- a/briar-tests/src/org/briarproject/sync/SimplexMessagingIntegrationTest.java
+++ b/briar-tests/src/org/briarproject/sync/SimplexMessagingIntegrationTest.java
@@ -135,9 +135,7 @@ public class SimplexMessagingIntegrationTest extends BriarTestCase {
Author bobAuthor = new Author(bobId, "Bob",
new byte[MAX_PUBLIC_KEY_LENGTH]);
ContactId contactId = contactManager.addContact(bobAuthor, aliceId,
- true);
- // Derive and store the transport keys
- keyManager.addContact(contactId, master, timestamp, true);
+ master, timestamp, true, true);
// Send Bob a message
GroupId groupId = messagingManager.getConversationId(contactId);
@@ -206,9 +204,7 @@ public class SimplexMessagingIntegrationTest extends BriarTestCase {
Author aliceAuthor = new Author(aliceId, "Alice",
new byte[MAX_PUBLIC_KEY_LENGTH]);
ContactId contactId = contactManager.addContact(aliceAuthor, bobId,
- true);
- // Derive and store the transport keys
- keyManager.addContact(contactId, master, timestamp, false);
+ master, timestamp, false, true);
// Set up an event listener
MessageListener listener = new MessageListener();
diff --git a/briar-tests/src/org/briarproject/transport/TransportKeyManagerTest.java b/briar-tests/src/org/briarproject/transport/TransportKeyManagerTest.java
index 2422bb45e3..148262b275 100644
--- a/briar-tests/src/org/briarproject/transport/TransportKeyManagerTest.java
+++ b/briar-tests/src/org/briarproject/transport/TransportKeyManagerTest.java
@@ -133,17 +133,15 @@ public class TransportKeyManagerTest extends BriarTestCase {
will(new EncodeTagAction());
}
// Save the keys
- oneOf(db).startTransaction();
- will(returnValue(txn));
oneOf(db).addTransportKeys(txn, contactId, rotated);
- oneOf(db).endTransaction(txn);
}});
TransportKeyManager transportKeyManager = new TransportKeyManager(db,
crypto, timer, clock, transportId, maxLatency);
// The timestamp is 1 ms before the start of rotation period 1000
long timestamp = rotationPeriodLength * 1000 - 1;
- transportKeyManager.addContact(contactId, masterKey, timestamp, alice);
+ transportKeyManager.addContact(txn, contactId, masterKey, timestamp,
+ alice);
context.assertIsSatisfied();
}
@@ -194,17 +192,15 @@ public class TransportKeyManagerTest extends BriarTestCase {
oneOf(crypto).rotateTransportKeys(transportKeys, 1000);
will(returnValue(transportKeys));
// Save the keys
- oneOf(db).startTransaction();
- will(returnValue(txn));
oneOf(db).addTransportKeys(txn, contactId, transportKeys);
- oneOf(db).endTransaction(txn);
}});
TransportKeyManager transportKeyManager = new TransportKeyManager(db,
crypto, timer, clock, transportId, maxLatency);
// The timestamp is at the start of rotation period 1000
long timestamp = rotationPeriodLength * 1000;
- transportKeyManager.addContact(contactId, masterKey, timestamp, alice);
+ transportKeyManager.addContact(txn, contactId, masterKey, timestamp,
+ alice);
assertNull(transportKeyManager.getStreamContext(contactId));
context.assertIsSatisfied();
@@ -240,10 +236,7 @@ public class TransportKeyManagerTest extends BriarTestCase {
oneOf(crypto).rotateTransportKeys(transportKeys, 1000);
will(returnValue(transportKeys));
// Save the keys
- oneOf(db).startTransaction();
- will(returnValue(txn));
oneOf(db).addTransportKeys(txn, contactId, transportKeys);
- oneOf(db).endTransaction(txn);
// Increment the stream counter
oneOf(db).startTransaction();
will(returnValue(txn1));
@@ -256,7 +249,8 @@ public class TransportKeyManagerTest extends BriarTestCase {
crypto, timer, clock, transportId, maxLatency);
// The timestamp is at the start of rotation period 1000
long timestamp = rotationPeriodLength * 1000;
- transportKeyManager.addContact(contactId, masterKey, timestamp, alice);
+ transportKeyManager.addContact(txn, contactId, masterKey, timestamp,
+ alice);
// The first request should return a stream context
StreamContext ctx = transportKeyManager.getStreamContext(contactId);
assertNotNull(ctx);
@@ -299,17 +293,15 @@ public class TransportKeyManagerTest extends BriarTestCase {
oneOf(crypto).rotateTransportKeys(transportKeys, 1000);
will(returnValue(transportKeys));
// Save the keys
- oneOf(db).startTransaction();
- will(returnValue(txn));
oneOf(db).addTransportKeys(txn, contactId, transportKeys);
- oneOf(db).endTransaction(txn);
}});
TransportKeyManager transportKeyManager = new TransportKeyManager(db,
crypto, timer, clock, transportId, maxLatency);
// The timestamp is at the start of rotation period 1000
long timestamp = rotationPeriodLength * 1000;
- transportKeyManager.addContact(contactId, masterKey, timestamp, alice);
+ transportKeyManager.addContact(txn, contactId, masterKey, timestamp,
+ alice);
assertNull(transportKeyManager.getStreamContext(new byte[TAG_LENGTH]));
context.assertIsSatisfied();
@@ -345,10 +337,7 @@ public class TransportKeyManagerTest extends BriarTestCase {
oneOf(crypto).rotateTransportKeys(transportKeys, 1000);
will(returnValue(transportKeys));
// Save the keys
- oneOf(db).startTransaction();
- will(returnValue(txn));
oneOf(db).addTransportKeys(txn, contactId, transportKeys);
- oneOf(db).endTransaction(txn);
// Encode a new tag after sliding the window
oneOf(crypto).encodeTag(with(any(byte[].class)),
with(tagKey), with((long) REORDERING_WINDOW_SIZE));
@@ -365,7 +354,8 @@ public class TransportKeyManagerTest extends BriarTestCase {
crypto, timer, clock, transportId, maxLatency);
// The timestamp is at the start of rotation period 1000
long timestamp = rotationPeriodLength * 1000;
- transportKeyManager.addContact(contactId, masterKey, timestamp, alice);
+ transportKeyManager.addContact(txn, contactId, masterKey, timestamp,
+ alice);
// Use the first tag (previous rotation period, stream number 0)
assertEquals(REORDERING_WINDOW_SIZE * 3, tags.size());
byte[] tag = tags.get(0);
--
GitLab