From e4bd6fdf951d06bc1b0b33f8c0a806f47fda0663 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 23 Jun 2021 14:34:22 -0300 Subject: [PATCH 1/7] Put FeatureFlags for tests into a TestFeatureFlagModule --- ...emovableDriveIntegrationTestComponent.java | 2 ++ .../RemovableDriveIntegrationTestModule.java | 27 --------------- .../BrambleCoreIntegrationTestModule.java | 28 +-------------- .../bramble/test/TestFeatureFlagModule.java | 34 +++++++++++++++++++ .../briar/headless/HeadlessTestModule.kt | 11 ++---- 5 files changed, 39 insertions(+), 63 deletions(-) create mode 100644 bramble-core/src/test/java/org/briarproject/bramble/test/TestFeatureFlagModule.java diff --git a/bramble-core/src/test/java/org/briarproject/bramble/plugin/file/RemovableDriveIntegrationTestComponent.java b/bramble-core/src/test/java/org/briarproject/bramble/plugin/file/RemovableDriveIntegrationTestComponent.java index 9c5c3153d..385e140da 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/plugin/file/RemovableDriveIntegrationTestComponent.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/plugin/file/RemovableDriveIntegrationTestComponent.java @@ -12,6 +12,7 @@ import org.briarproject.bramble.event.DefaultEventExecutorModule; import org.briarproject.bramble.system.DefaultWakefulIoExecutorModule; import org.briarproject.bramble.system.TimeTravelModule; import org.briarproject.bramble.test.TestDatabaseConfigModule; +import org.briarproject.bramble.test.TestFeatureFlagModule; import org.briarproject.bramble.test.TestSecureRandomModule; import javax.inject.Singleton; @@ -25,6 +26,7 @@ import dagger.Component; DefaultEventExecutorModule.class, DefaultWakefulIoExecutorModule.class, TestDatabaseConfigModule.class, + TestFeatureFlagModule.class, RemovableDriveIntegrationTestModule.class, RemovableDriveModule.class, TestSecureRandomModule.class, diff --git a/bramble-core/src/test/java/org/briarproject/bramble/plugin/file/RemovableDriveIntegrationTestModule.java b/bramble-core/src/test/java/org/briarproject/bramble/plugin/file/RemovableDriveIntegrationTestModule.java index 7b4699e10..5f3e19842 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/plugin/file/RemovableDriveIntegrationTestModule.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/plugin/file/RemovableDriveIntegrationTestModule.java @@ -1,6 +1,5 @@ package org.briarproject.bramble.plugin.file; -import org.briarproject.bramble.api.FeatureFlags; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.plugin.PluginConfig; import org.briarproject.bramble.api.plugin.TransportId; @@ -52,30 +51,4 @@ class RemovableDriveIntegrationTestModule { }; return pluginConfig; } - - @Provides - FeatureFlags provideFeatureFlags() { - return new FeatureFlags() { - - @Override - public boolean shouldEnableImageAttachments() { - return true; - } - - @Override - public boolean shouldEnableProfilePictures() { - return true; - } - - @Override - public boolean shouldEnableDisappearingMessages() { - return true; - } - - @Override - public boolean shouldEnableConnectViaBluetooth() { - return true; - } - }; - } } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleCoreIntegrationTestModule.java b/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleCoreIntegrationTestModule.java index 22dea21f6..b735dab7d 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleCoreIntegrationTestModule.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleCoreIntegrationTestModule.java @@ -1,48 +1,22 @@ package org.briarproject.bramble.test; -import org.briarproject.bramble.api.FeatureFlags; import org.briarproject.bramble.battery.DefaultBatteryManagerModule; import org.briarproject.bramble.event.DefaultEventExecutorModule; import org.briarproject.bramble.system.DefaultWakefulIoExecutorModule; import org.briarproject.bramble.system.TimeTravelModule; import dagger.Module; -import dagger.Provides; @Module(includes = { DefaultBatteryManagerModule.class, DefaultEventExecutorModule.class, DefaultWakefulIoExecutorModule.class, TestDatabaseConfigModule.class, + TestFeatureFlagModule.class, TestPluginConfigModule.class, TestSecureRandomModule.class, TimeTravelModule.class }) public class BrambleCoreIntegrationTestModule { - @Provides - FeatureFlags provideFeatureFlags() { - return new FeatureFlags() { - - @Override - public boolean shouldEnableImageAttachments() { - return true; - } - - @Override - public boolean shouldEnableProfilePictures() { - return true; - } - - @Override - public boolean shouldEnableDisappearingMessages() { - return true; - } - - @Override - public boolean shouldEnableConnectViaBluetooth() { - return true; - } - }; - } } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/test/TestFeatureFlagModule.java b/bramble-core/src/test/java/org/briarproject/bramble/test/TestFeatureFlagModule.java new file mode 100644 index 000000000..3bac40b0f --- /dev/null +++ b/bramble-core/src/test/java/org/briarproject/bramble/test/TestFeatureFlagModule.java @@ -0,0 +1,34 @@ +package org.briarproject.bramble.test; + +import org.briarproject.bramble.api.FeatureFlags; + +import dagger.Module; +import dagger.Provides; + +@Module +public class TestFeatureFlagModule { + @Provides + FeatureFlags provideFeatureFlags() { + return new FeatureFlags() { + @Override + public boolean shouldEnableImageAttachments() { + return true; + } + + @Override + public boolean shouldEnableProfilePictures() { + return true; + } + + @Override + public boolean shouldEnableDisappearingMessages() { + return true; + } + + @Override + public boolean shouldEnableConnectViaBluetooth() { + return true; + } + }; + } +} diff --git a/briar-headless/src/test/java/org/briarproject/briar/headless/HeadlessTestModule.kt b/briar-headless/src/test/java/org/briarproject/briar/headless/HeadlessTestModule.kt index f31caf2e3..67171a977 100644 --- a/briar-headless/src/test/java/org/briarproject/briar/headless/HeadlessTestModule.kt +++ b/briar-headless/src/test/java/org/briarproject/briar/headless/HeadlessTestModule.kt @@ -4,7 +4,6 @@ import com.fasterxml.jackson.databind.ObjectMapper import dagger.Module import dagger.Provides import org.briarproject.bramble.account.AccountModule -import org.briarproject.bramble.api.FeatureFlags import org.briarproject.bramble.api.db.DatabaseConfig import org.briarproject.bramble.api.plugin.PluginConfig import org.briarproject.bramble.api.plugin.TransportId @@ -18,6 +17,7 @@ import org.briarproject.bramble.system.ClockModule import org.briarproject.bramble.system.DefaultTaskSchedulerModule import org.briarproject.bramble.system.DefaultWakefulIoExecutorModule import org.briarproject.bramble.system.JavaSystemModule +import org.briarproject.bramble.test.TestFeatureFlagModule import org.briarproject.bramble.test.TestSecureRandomModule import org.briarproject.briar.api.test.TestAvatarCreator import org.briarproject.briar.headless.blogs.HeadlessBlogModule @@ -40,6 +40,7 @@ import javax.inject.Singleton DefaultTaskSchedulerModule::class, DefaultWakefulIoExecutorModule::class, SocksModule::class, + TestFeatureFlagModule::class, TestSecureRandomModule::class, HeadlessBlogModule::class, HeadlessContactModule::class, @@ -78,14 +79,6 @@ internal class HeadlessTestModule(private val appDir: File) { @Singleton internal fun provideObjectMapper() = ObjectMapper() - @Provides - internal fun provideFeatureFlags() = object : FeatureFlags { - override fun shouldEnableImageAttachments() = false - override fun shouldEnableProfilePictures() = false - override fun shouldEnableDisappearingMessages() = false - override fun shouldEnableConnectViaBluetooth() = false - } - @Provides internal fun provideTestAvatarCreator() = TestAvatarCreator { null } } From a93b1f18ac11bdf4b1605a66ac6904f251666c35 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 23 Jun 2021 17:16:50 -0300 Subject: [PATCH 2/7] Refactor base of BriarIntegrationTest into BrambleIntegrationTest --- bramble-core/build.gradle | 1 + .../bramble/test/BrambleIntegrationTest.java | 306 ++++++++++++++++++ .../test/BrambleIntegrationTestComponent.java | 30 ++ bramble-core/witness.gradle | 1 + .../briar/test/BriarIntegrationTest.java | 288 +---------------- .../test/BriarIntegrationTestComponent.java | 15 +- 6 files changed, 350 insertions(+), 291 deletions(-) create mode 100644 bramble-core/src/test/java/org/briarproject/bramble/test/BrambleIntegrationTest.java create mode 100644 bramble-core/src/test/java/org/briarproject/bramble/test/BrambleIntegrationTestComponent.java diff --git a/bramble-core/build.gradle b/bramble-core/build.gradle index ce33f1ad9..e1384472e 100644 --- a/bramble-core/build.gradle +++ b/bramble-core/build.gradle @@ -21,6 +21,7 @@ dependencies { testImplementation project(path: ':bramble-api', configuration: 'testOutput') testImplementation 'org.hsqldb:hsqldb:2.3.5' // The last version that supports Java 1.6 + testImplementation 'net.jodah:concurrentunit:0.4.2' testImplementation 'junit:junit:4.12' testImplementation "org.jmock:jmock:2.8.2" testImplementation "org.jmock:jmock-junit4:2.8.2" diff --git a/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleIntegrationTest.java b/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleIntegrationTest.java new file mode 100644 index 000000000..861494b77 --- /dev/null +++ b/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleIntegrationTest.java @@ -0,0 +1,306 @@ +package org.briarproject.bramble.test; + +import net.jodah.concurrentunit.Waiter; + +import org.briarproject.bramble.api.FormatException; +import org.briarproject.bramble.api.client.ClientHelper; +import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.data.BdfList; +import org.briarproject.bramble.api.data.BdfStringUtils; +import org.briarproject.bramble.api.db.DbException; +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.nullsafety.MethodsNotNullByDefault; +import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault; +import org.briarproject.bramble.api.sync.MessageId; +import org.briarproject.bramble.api.sync.event.MessageStateChangedEvent; +import org.briarproject.bramble.api.sync.event.MessagesAckedEvent; +import org.briarproject.bramble.api.sync.event.MessagesSentEvent; +import org.junit.After; +import org.junit.Before; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.util.HashSet; +import java.util.Set; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Executor; +import java.util.concurrent.Semaphore; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.logging.Logger; + +import javax.annotation.Nonnull; + +import static java.util.concurrent.Executors.newSingleThreadExecutor; +import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static java.util.concurrent.TimeUnit.MINUTES; +import static java.util.logging.Level.WARNING; +import static java.util.logging.Logger.getLogger; +import static org.briarproject.bramble.api.sync.validation.MessageState.DELIVERED; +import static org.briarproject.bramble.api.sync.validation.MessageState.INVALID; +import static org.briarproject.bramble.api.sync.validation.MessageState.PENDING; +import static org.briarproject.bramble.test.TestPluginConfigModule.SIMPLEX_TRANSPORT_ID; +import static org.briarproject.bramble.util.LogUtils.logException; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +@MethodsNotNullByDefault +@ParametersNotNullByDefault +public abstract class BrambleIntegrationTest + extends BrambleTestCase { + + private static final Logger LOG = + getLogger(BrambleIntegrationTest.class.getName()); + + private static final boolean DEBUG = false; + + protected final static int TIMEOUT = 15000; + + // objects accessed from background threads need to be volatile + private volatile Waiter validationWaiter; + private volatile Waiter deliveryWaiter; + private volatile Waiter ackWaiter; + private volatile boolean expectAck = false; + + private final Semaphore messageSemaphore = new Semaphore(0); + private final AtomicInteger deliveryCounter = new AtomicInteger(0); + private final AtomicInteger validationCounter = new AtomicInteger(0); + private final AtomicInteger ackCounter = new AtomicInteger(0); + + protected final File testDir = TestUtils.getTestDirectory(); + + @Before + public void setUp() throws Exception { + assertTrue(testDir.mkdirs()); + + // initialize waiters fresh for each test + validationWaiter = new Waiter(); + deliveryWaiter = new Waiter(); + ackWaiter = new Waiter(); + deliveryCounter.set(0); + validationCounter.set(0); + ackCounter.set(0); + } + + @After + public void tearDown() throws Exception { + TestUtils.deleteTestDirectory(testDir); + } + + protected void addEventListener(C c) { + c.getEventBus().addListener(new Listener(c)); + } + + private class Listener implements EventListener { + + private final ClientHelper clientHelper; + private final Executor executor; + + private Listener(C c) { + clientHelper = c.getClientHelper(); + executor = newSingleThreadExecutor(); + } + + @Override + public void eventOccurred(Event e) { + if (e instanceof MessageStateChangedEvent) { + MessageStateChangedEvent event = (MessageStateChangedEvent) e; + if (!event.isLocal()) { + if (event.getState() == DELIVERED) { + LOG.info("Delivered new message " + + event.getMessageId()); + deliveryCounter.addAndGet(1); + loadAndLogMessage(event.getMessageId()); + deliveryWaiter.resume(); + } else if (event.getState() == INVALID || + event.getState() == PENDING) { + LOG.info("Validated new " + event.getState().name() + + " message " + event.getMessageId()); + validationCounter.addAndGet(1); + loadAndLogMessage(event.getMessageId()); + validationWaiter.resume(); + } + } + } else if (e instanceof MessagesAckedEvent && expectAck) { + MessagesAckedEvent event = (MessagesAckedEvent) e; + ackCounter.addAndGet(event.getMessageIds().size()); + for (MessageId m : event.getMessageIds()) { + loadAndLogMessage(m); + ackWaiter.resume(); + } + } + } + + private void loadAndLogMessage(MessageId id) { + executor.execute(() -> { + if (DEBUG) { + try { + BdfList body = clientHelper.getMessageAsList(id); + LOG.info("Contents of " + id + ":\n" + + BdfStringUtils.toString(body)); + } catch (DbException | FormatException e) { + logException(LOG, WARNING, e); + } + } + messageSemaphore.release(); + }); + } + } + + + protected void syncMessage(BrambleIntegrationTestComponent fromComponent, + BrambleIntegrationTestComponent toComponent, ContactId toId, + int num, boolean valid) throws Exception { + syncMessage(fromComponent, toComponent, toId, num, 0, valid ? 0 : num, + valid ? num : 0); + } + + protected void syncMessage(BrambleIntegrationTestComponent fromComponent, + BrambleIntegrationTestComponent toComponent, ContactId toId, + int numNew, int numDupes, int numPendingOrInvalid, int numDelivered) + throws Exception { + + // Debug output + String from = + fromComponent.getIdentityManager().getLocalAuthor().getName(); + String to = toComponent.getIdentityManager().getLocalAuthor().getName(); + LOG.info("TEST: Sending " + (numNew + numDupes) + " message(s) from " + + from + " to " + to); + + // Listen for messages being sent + waitForEvents(fromComponent); + SendListener sendListener = new SendListener(); + fromComponent.getEventBus().addListener(sendListener); + + // Write the messages to a transport stream + ByteArrayOutputStream out = new ByteArrayOutputStream(); + TestTransportConnectionWriter writer = + new TestTransportConnectionWriter(out, false); + fromComponent.getConnectionManager().manageOutgoingConnection(toId, + SIMPLEX_TRANSPORT_ID, writer); + writer.getDisposedLatch().await(TIMEOUT, MILLISECONDS); + + // Check that the expected number of messages were sent + waitForEvents(fromComponent); + fromComponent.getEventBus().removeListener(sendListener); + assertEquals("Messages sent", numNew + numDupes, + sendListener.sent.size()); + + // Read the messages from the transport stream + ByteArrayInputStream in = new ByteArrayInputStream(out.toByteArray()); + TestTransportConnectionReader reader = + new TestTransportConnectionReader(in); + toComponent.getConnectionManager().manageIncomingConnection( + SIMPLEX_TRANSPORT_ID, reader); + + if (numPendingOrInvalid > 0) { + validationWaiter.await(TIMEOUT, numPendingOrInvalid); + } + assertEquals("Messages validated", numPendingOrInvalid, + validationCounter.getAndSet(0)); + + if (numDelivered > 0) { + deliveryWaiter.await(TIMEOUT, numDelivered); + } + assertEquals("Messages delivered", numDelivered, + deliveryCounter.getAndSet(0)); + + try { + messageSemaphore.tryAcquire(numNew, TIMEOUT, MILLISECONDS); + } catch (InterruptedException e) { + LOG.info("Interrupted while waiting for messages"); + Thread.currentThread().interrupt(); + fail(); + } + } + + protected void sendAcks(BrambleIntegrationTestComponent fromComponent, + BrambleIntegrationTestComponent toComponent, ContactId toId, + int num) throws Exception { + // Debug output + String from = + fromComponent.getIdentityManager().getLocalAuthor().getName(); + String to = toComponent.getIdentityManager().getLocalAuthor().getName(); + LOG.info("TEST: Sending " + num + " ACKs from " + from + " to " + to); + + expectAck = true; + + // Listen for messages being sent (none should be sent) + waitForEvents(fromComponent); + SendListener sendListener = new SendListener(); + fromComponent.getEventBus().addListener(sendListener); + + // start outgoing connection + ByteArrayOutputStream out = new ByteArrayOutputStream(); + TestTransportConnectionWriter writer = + new TestTransportConnectionWriter(out, false); + fromComponent.getConnectionManager().manageOutgoingConnection(toId, + SIMPLEX_TRANSPORT_ID, writer); + writer.getDisposedLatch().await(TIMEOUT, MILLISECONDS); + + // Check that no messages were sent + waitForEvents(fromComponent); + fromComponent.getEventBus().removeListener(sendListener); + assertEquals("Messages sent", 0, sendListener.sent.size()); + + // handle incoming connection + ByteArrayInputStream in = new ByteArrayInputStream(out.toByteArray()); + TestTransportConnectionReader reader = + new TestTransportConnectionReader(in); + toComponent.getConnectionManager().manageIncomingConnection( + SIMPLEX_TRANSPORT_ID, reader); + + ackWaiter.await(TIMEOUT, num); + assertEquals("ACKs delivered", num, ackCounter.getAndSet(0)); + assertEquals("No messages delivered", 0, deliveryCounter.get()); + try { + messageSemaphore.tryAcquire(num, TIMEOUT, MILLISECONDS); + } catch (InterruptedException e) { + LOG.info("Interrupted while waiting for messages"); + Thread.currentThread().interrupt(); + fail(); + } finally { + expectAck = false; + } + } + + /** + * Broadcasts a marker event and waits for it to be delivered, which + * indicates that all previously broadcast events have been delivered. + */ + public static void waitForEvents(BrambleIntegrationTestComponent component) + throws Exception { + CountDownLatch latch = new CountDownLatch(1); + MarkerEvent marker = new MarkerEvent(); + EventBus eventBus = component.getEventBus(); + eventBus.addListener(new EventListener() { + @Override + public void eventOccurred(@Nonnull Event e) { + if (e == marker) { + latch.countDown(); + eventBus.removeListener(this); + } + } + }); + eventBus.broadcast(marker); + if (!latch.await(1, MINUTES)) fail(); + } + + private static class MarkerEvent extends Event { + } + + private static class SendListener implements EventListener { + + private final Set sent = new HashSet<>(); + + @Override + public void eventOccurred(Event e) { + if (e instanceof MessagesSentEvent) { + sent.addAll(((MessagesSentEvent) e).getMessageIds()); + } + } + } +} diff --git a/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleIntegrationTestComponent.java b/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleIntegrationTestComponent.java new file mode 100644 index 000000000..c7b0d80c5 --- /dev/null +++ b/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleIntegrationTestComponent.java @@ -0,0 +1,30 @@ +package org.briarproject.bramble.test; + +import org.briarproject.bramble.BrambleCoreIntegrationTestEagerSingletons; +import org.briarproject.bramble.BrambleCoreModule; +import org.briarproject.bramble.api.client.ClientHelper; +import org.briarproject.bramble.api.connection.ConnectionManager; +import org.briarproject.bramble.api.event.EventBus; +import org.briarproject.bramble.api.identity.IdentityManager; + +import javax.inject.Singleton; + +import dagger.Component; + +@Singleton +@Component(modules = { + BrambleCoreIntegrationTestModule.class, + BrambleCoreModule.class +}) +public interface BrambleIntegrationTestComponent + extends BrambleCoreIntegrationTestEagerSingletons { + + IdentityManager getIdentityManager(); + + EventBus getEventBus(); + + ConnectionManager getConnectionManager(); + + ClientHelper getClientHelper(); + +} diff --git a/bramble-core/witness.gradle b/bramble-core/witness.gradle index 9ab2fac6d..949566231 100644 --- a/bramble-core/witness.gradle +++ b/bramble-core/witness.gradle @@ -20,6 +20,7 @@ dependencyVerification { 'javax.inject:javax.inject:1:javax.inject-1.jar:91c77044a50c481636c32d916fd89c9118a72195390452c81065080f957de7ff', 'junit:junit:4.12:junit-4.12.jar:59721f0805e223d84b90677887d9ff567dc534d7c502ca903c0c2b17f05c116a', 'net.i2p.crypto:eddsa:0.2.0:eddsa-0.2.0.jar:a7cb1b85c16e2f0730b9204106929a1d9aaae1df728adc7041a8b8b605692140', + 'net.jodah:concurrentunit:0.4.2:concurrentunit-0.4.2.jar:5583078e1acf91734939e985bc9e7ee947b0e93a8eef679da6bb07bbeb47ced3', 'net.ltgt.gradle.incap:incap:0.2:incap-0.2.jar:b625b9806b0f1e4bc7a2e3457119488de3cd57ea20feedd513db070a573a4ffd', 'org.apache.ant:ant-launcher:1.9.4:ant-launcher-1.9.4.jar:7bccea20b41801ca17bcbc909a78c835d0f443f12d639c77bd6ae3d05861608d', 'org.apache.ant:ant:1.9.4:ant-1.9.4.jar:649ae0730251de07b8913f49286d46bba7b92d47c5f332610aa426c4f02161d8', diff --git a/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTest.java index e493b4fa5..899565f1f 100644 --- a/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTest.java @@ -1,8 +1,5 @@ package org.briarproject.briar.test; -import net.jodah.concurrentunit.Waiter; - -import org.briarproject.bramble.api.FormatException; import org.briarproject.bramble.api.client.ClientHelper; import org.briarproject.bramble.api.client.ContactGroupFactory; import org.briarproject.bramble.api.contact.Contact; @@ -10,13 +7,8 @@ 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.SecretKey; -import org.briarproject.bramble.api.data.BdfList; -import org.briarproject.bramble.api.data.BdfStringUtils; import org.briarproject.bramble.api.db.DatabaseComponent; import org.briarproject.bramble.api.db.DbException; -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.Identity; import org.briarproject.bramble.api.identity.IdentityManager; import org.briarproject.bramble.api.identity.LocalAuthor; @@ -25,11 +17,7 @@ import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault; import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault; import org.briarproject.bramble.api.sync.MessageFactory; import org.briarproject.bramble.api.sync.MessageId; -import org.briarproject.bramble.api.sync.event.MessageStateChangedEvent; -import org.briarproject.bramble.api.sync.event.MessagesAckedEvent; -import org.briarproject.bramble.api.sync.event.MessagesSentEvent; -import org.briarproject.bramble.test.TestTransportConnectionReader; -import org.briarproject.bramble.test.TestTransportConnectionWriter; +import org.briarproject.bramble.test.BrambleIntegrationTest; import org.briarproject.bramble.test.TestUtils; import org.briarproject.briar.api.autodelete.AutoDeleteManager; import org.briarproject.briar.api.blog.BlogFactory; @@ -43,48 +31,19 @@ import org.briarproject.briar.api.privategroup.invitation.GroupInvitationFactory import org.junit.After; import org.junit.Before; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; import java.io.File; -import java.util.HashSet; -import java.util.Set; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.Executor; -import java.util.concurrent.Semaphore; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.logging.Logger; -import javax.annotation.Nonnull; import javax.annotation.Nullable; import javax.inject.Inject; -import static java.util.concurrent.Executors.newSingleThreadExecutor; -import static java.util.concurrent.TimeUnit.MILLISECONDS; -import static java.util.concurrent.TimeUnit.MINUTES; -import static java.util.logging.Level.WARNING; -import static java.util.logging.Logger.getLogger; import static junit.framework.Assert.assertNotNull; -import static org.briarproject.bramble.api.sync.validation.MessageState.DELIVERED; -import static org.briarproject.bramble.api.sync.validation.MessageState.INVALID; -import static org.briarproject.bramble.api.sync.validation.MessageState.PENDING; -import static org.briarproject.bramble.test.TestPluginConfigModule.SIMPLEX_TRANSPORT_ID; import static org.briarproject.bramble.test.TestUtils.getSecretKey; -import static org.briarproject.bramble.util.LogUtils.logException; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; @MethodsNotNullByDefault @ParametersNotNullByDefault public abstract class BriarIntegrationTest - extends BriarTestCase { - - private static final Logger LOG = - getLogger(BriarIntegrationTest.class.getName()); - - private static final boolean DEBUG = false; - - protected final static int TIMEOUT = 15000; + extends BrambleIntegrationTest { @Nullable protected ContactId contactId1From2, contactId2From1; @@ -94,7 +53,7 @@ public abstract class BriarIntegrationTest { - if (DEBUG) { - try { - BdfList body = clientHelper.getMessageAsList(id); - LOG.info("Contents of " + id + ":\n" - + BdfStringUtils.toString(body)); - } catch (DbException | FormatException e) { - logException(LOG, WARNING, e); - } - } - messageSemaphore.release(); - }); - } + addEventListener(c0); + addEventListener(c1); + addEventListener(c2); } private void createAndRegisterIdentities() { @@ -335,9 +218,10 @@ public abstract class BriarIntegrationTest 0) { - validationWaiter.await(TIMEOUT, numPendingOrInvalid); - } - assertEquals("Messages validated", numPendingOrInvalid, - validationCounter.getAndSet(0)); - - if (numDelivered > 0) { - deliveryWaiter.await(TIMEOUT, numDelivered); - } - assertEquals("Messages delivered", numDelivered, - deliveryCounter.getAndSet(0)); - - try { - messageSemaphore.tryAcquire(numNew, TIMEOUT, MILLISECONDS); - } catch (InterruptedException e) { - LOG.info("Interrupted while waiting for messages"); - Thread.currentThread().interrupt(); - fail(); - } - } - - protected void sendAcks(BriarIntegrationTestComponent fromComponent, - BriarIntegrationTestComponent toComponent, ContactId toId, int num) - throws Exception { - // Debug output - String from = - fromComponent.getIdentityManager().getLocalAuthor().getName(); - String to = toComponent.getIdentityManager().getLocalAuthor().getName(); - LOG.info("TEST: Sending " + num + " ACKs from " + from + " to " + to); - - expectAck = true; - - // Listen for messages being sent (none should be sent) - waitForEvents(fromComponent); - SendListener sendListener = new SendListener(); - fromComponent.getEventBus().addListener(sendListener); - - // start outgoing connection - ByteArrayOutputStream out = new ByteArrayOutputStream(); - TestTransportConnectionWriter writer = - new TestTransportConnectionWriter(out, false); - fromComponent.getConnectionManager().manageOutgoingConnection(toId, - SIMPLEX_TRANSPORT_ID, writer); - writer.getDisposedLatch().await(TIMEOUT, MILLISECONDS); - - // Check that no messages were sent - waitForEvents(fromComponent); - fromComponent.getEventBus().removeListener(sendListener); - assertEquals("Messages sent", 0, sendListener.sent.size()); - - // handle incoming connection - ByteArrayInputStream in = new ByteArrayInputStream(out.toByteArray()); - TestTransportConnectionReader reader = - new TestTransportConnectionReader(in); - toComponent.getConnectionManager().manageIncomingConnection( - SIMPLEX_TRANSPORT_ID, reader); - - ackWaiter.await(TIMEOUT, num); - assertEquals("ACKs delivered", num, ackCounter.getAndSet(0)); - assertEquals("No messages delivered", 0, deliveryCounter.get()); - try { - messageSemaphore.tryAcquire(num, TIMEOUT, MILLISECONDS); - } catch (InterruptedException e) { - LOG.info("Interrupted while waiting for messages"); - Thread.currentThread().interrupt(); - fail(); - } finally { - expectAck = false; - } - } - protected void removeAllContacts() throws DbException { contactManager0.removeContact(contactId1From0); contactManager0.removeContact(contactId2From0); @@ -562,40 +330,4 @@ public abstract class BriarIntegrationTest db.setMessageShared(txn, messageId)); } - /** - * Broadcasts a marker event and waits for it to be delivered, which - * indicates that all previously broadcast events have been delivered. - */ - public static void waitForEvents(BriarIntegrationTestComponent component) - throws Exception { - CountDownLatch latch = new CountDownLatch(1); - MarkerEvent marker = new MarkerEvent(); - EventBus eventBus = component.getEventBus(); - eventBus.addListener(new EventListener() { - @Override - public void eventOccurred(@Nonnull Event e) { - if (e == marker) { - latch.countDown(); - eventBus.removeListener(this); - } - } - }); - eventBus.broadcast(marker); - if (!latch.await(1, MINUTES)) fail(); - } - - private static class MarkerEvent extends Event { - } - - private static class SendListener implements EventListener { - - private final Set sent = new HashSet<>(); - - @Override - public void eventOccurred(Event e) { - if (e instanceof MessagesSentEvent) { - sent.addAll(((MessagesSentEvent) e).getMessageIds()); - } - } - } } diff --git a/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTestComponent.java b/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTestComponent.java index 738b48e96..7c529d949 100644 --- a/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTestComponent.java +++ b/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTestComponent.java @@ -2,17 +2,14 @@ package org.briarproject.briar.test; import org.briarproject.bramble.BrambleCoreIntegrationTestEagerSingletons; import org.briarproject.bramble.BrambleCoreModule; -import org.briarproject.bramble.api.client.ClientHelper; -import org.briarproject.bramble.api.connection.ConnectionManager; import org.briarproject.bramble.api.contact.ContactManager; import org.briarproject.bramble.api.db.DatabaseComponent; -import org.briarproject.bramble.api.event.EventBus; import org.briarproject.bramble.api.identity.AuthorFactory; -import org.briarproject.bramble.api.identity.IdentityManager; import org.briarproject.bramble.api.lifecycle.LifecycleManager; import org.briarproject.bramble.api.properties.TransportPropertyManager; import org.briarproject.bramble.api.system.Clock; import org.briarproject.bramble.test.BrambleCoreIntegrationTestModule; +import org.briarproject.bramble.test.BrambleIntegrationTestComponent; import org.briarproject.bramble.test.TimeTravel; import org.briarproject.briar.api.attachment.AttachmentReader; import org.briarproject.briar.api.autodelete.AutoDeleteManager; @@ -67,7 +64,7 @@ import dagger.Component; SharingModule.class }) public interface BriarIntegrationTestComponent - extends BrambleCoreIntegrationTestEagerSingletons { + extends BrambleIntegrationTestComponent { void inject(BriarIntegrationTest init); @@ -95,16 +92,10 @@ public interface BriarIntegrationTestComponent LifecycleManager getLifecycleManager(); - EventBus getEventBus(); - - IdentityManager getIdentityManager(); - AttachmentReader getAttachmentReader(); AvatarManager getAvatarManager(); - ClientHelper getClientHelper(); - ContactManager getContactManager(); ConversationManager getConversationManager(); @@ -139,8 +130,6 @@ public interface BriarIntegrationTestComponent BlogFactory getBlogFactory(); - ConnectionManager getConnectionManager(); - AutoDeleteManager getAutoDeleteManager(); Clock getClock(); From abe570e905916964d715aa065a849acb18278fa6 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 23 Jun 2021 14:34:53 -0300 Subject: [PATCH 3/7] Add first integration test for TransportKeyAgreementManager --- .../bramble/test/BrambleIntegrationTest.java | 15 ++ .../bramble/test/TestPluginConfigModule.java | 20 +- .../TransportKeyAgreementIntegrationTest.java | 196 ++++++++++++++++++ .../TransportKeyAgreementTestComponent.java | 29 +++ 4 files changed, 256 insertions(+), 4 deletions(-) create mode 100644 bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementIntegrationTest.java create mode 100644 bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementTestComponent.java diff --git a/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleIntegrationTest.java b/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleIntegrationTest.java index 861494b77..cc5f17ed2 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleIntegrationTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleIntegrationTest.java @@ -28,6 +28,7 @@ import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Executor; import java.util.concurrent.Semaphore; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Logger; @@ -217,6 +218,20 @@ public abstract class BrambleIntegrationTest { + + private final File aliceDir = new File(testDir, "alice"); + private final File bobDir = new File(testDir, "bob"); + private final SecretKey masterKey = getSecretKey(); + private final long timestamp = System.currentTimeMillis(); + private final TransportId newTransportId = + new TransportId(getRandomString(8)); + + private TransportKeyAgreementTestComponent alice, bob; + + @Before + @Override + public void setUp() throws Exception { + super.setUp(); + // Create the devices + alice = createComponent(aliceDir, false); + bob = createComponent(bobDir, false); + + // Start both lifecycles + startLifecycle(alice, "Alice"); + startLifecycle(bob, "Bob"); + } + + private TransportKeyAgreementTestComponent createComponent( + File dir, boolean useNewTransport) { + TestPluginConfigModule pluginConfigModule = useNewTransport ? + new TestPluginConfigModule(SIMPLEX_TRANSPORT_ID, newTransportId) + : new TestPluginConfigModule(); + TransportKeyAgreementTestComponent c = + DaggerTransportKeyAgreementTestComponent.builder() + .testDatabaseConfigModule( + new TestDatabaseConfigModule(dir)) + .testPluginConfigModule(pluginConfigModule) + .build(); + BrambleCoreIntegrationTestEagerSingletons.Helper + .injectEagerSingletons(c); + return c; + } + + private void startLifecycle( + TransportKeyAgreementTestComponent device, + String identityName) throws Exception { + // Listen to message related events first to not miss early ones + addEventListener(device); + // Add an identity for the user + IdentityManager identityManager = device.getIdentityManager(); + Identity identity = identityManager.createIdentity(identityName); + identityManager.registerIdentity(identity); + // Start the lifecycle manager + LifecycleManager lifecycleManager = device.getLifecycleManager(); + lifecycleManager.startServices(masterKey); // re-using masterKey here + lifecycleManager.waitForStartup(); + } + + @After + @Override + public void tearDown() throws Exception { + tearDown(alice); + tearDown(bob); + super.tearDown(); + } + + private void tearDown(TransportKeyAgreementTestComponent device) + throws Exception { + // Stop the lifecycle manager + LifecycleManager lifecycleManager = device.getLifecycleManager(); + lifecycleManager.stopServices(); + lifecycleManager.waitForShutdown(); + } + + @Test + public void testAliceAddsTransportBeforeBob() throws Exception { + // Alice and Bob add each other. + Pair contactIds = addContacts(); + ContactId aliceId = contactIds.getFirst(); + ContactId bobId = contactIds.getSecond(); + + // Alice restarts and comes back with the new transport. + alice = restartWithNewTransport(alice, aliceDir, "Alice"); + + // Alice can still send via the old simplex, + // but not via the new duplex transport + assertTrue(alice.getKeyManager() + .canSendOutgoingStreams(bobId, SIMPLEX_TRANSPORT_ID)); + assertFalse(alice.getKeyManager() + .canSendOutgoingStreams(bobId, newTransportId)); + + // Alice has started a session and sends KEY message to Bob + // which he can't read, as he doesn't support the new transport, yet. + syncMessage(alice, bob, bobId, 1, false); + + // Bob restarts and comes back with the new transport. + bob = restartWithNewTransport(bob, bobDir, "Bob"); + + // Alice's pending KEY message now gets delivered async, so wait for it + awaitPendingMessageDelivery(1); + + // Bobs now and sends his own KEY as well as his ACTIVATE message. + syncMessage(bob, alice, aliceId, 2, true); + + // Alice can already send over the new transport while Bob still can't. + assertTrue(alice.getKeyManager() + .canSendOutgoingStreams(bobId, newTransportId)); + assertFalse(bob.getKeyManager() + .canSendOutgoingStreams(aliceId, newTransportId)); + + // Now Alice sends her ACTIVATE message to Bob. + syncMessage(alice, bob, bobId, 1, true); + + // Now Bob can also send over the new transport. + assertTrue(alice.getKeyManager() + .canSendOutgoingStreams(bobId, newTransportId)); + assertTrue(bob.getKeyManager() + .canSendOutgoingStreams(aliceId, newTransportId)); + } + + private Pair addContacts() throws Exception { + ContactId bobId = addContact(alice, bob, true); + ContactId aliceId = addContact(bob, alice, false); + + // Alice and Bob can send messages via the default test transports + assertTrue(alice.getKeyManager() + .canSendOutgoingStreams(bobId, SIMPLEX_TRANSPORT_ID)); + assertTrue(alice.getKeyManager() + .canSendOutgoingStreams(bobId, DUPLEX_TRANSPORT_ID)); + assertTrue(bob.getKeyManager() + .canSendOutgoingStreams(aliceId, SIMPLEX_TRANSPORT_ID)); + assertTrue(bob.getKeyManager() + .canSendOutgoingStreams(aliceId, DUPLEX_TRANSPORT_ID)); + + // Sync initial client versioning updates + syncMessage(alice, bob, bobId, 1, true); + syncMessage(bob, alice, aliceId, 1, true); + syncMessage(alice, bob, bobId, 1, true); + sendAcks(bob, alice, aliceId, 1); + + return new Pair<>(aliceId, bobId); + } + + private ContactId addContact( + TransportKeyAgreementTestComponent device, + TransportKeyAgreementTestComponent remote, + boolean alice) throws Exception { + // Get remote Author + Author remoteAuthor = remote.getIdentityManager().getLocalAuthor(); + // Get ID of LocalAuthor + IdentityManager identityManager = device.getIdentityManager(); + AuthorId localAuthorId = identityManager.getLocalAuthor().getId(); + // Add the other user as a contact + ContactManager contactManager = device.getContactManager(); + return contactManager.addContact(remoteAuthor, localAuthorId, masterKey, + timestamp, alice, true, true); + } + + private TransportKeyAgreementTestComponent restartWithNewTransport( + TransportKeyAgreementTestComponent device, File dir, String name) + throws Exception { + tearDown(device); + TransportKeyAgreementTestComponent newDevice = + createComponent(dir, true); + startLifecycle(newDevice, name); + return newDevice; + } + +} diff --git a/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementTestComponent.java b/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementTestComponent.java new file mode 100644 index 000000000..6c8a5d0c6 --- /dev/null +++ b/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementTestComponent.java @@ -0,0 +1,29 @@ +package org.briarproject.bramble.transport.agreement; + +import org.briarproject.bramble.BrambleCoreModule; +import org.briarproject.bramble.api.contact.ContactManager; +import org.briarproject.bramble.api.lifecycle.LifecycleManager; +import org.briarproject.bramble.api.transport.KeyManager; +import org.briarproject.bramble.test.BrambleCoreIntegrationTestModule; +import org.briarproject.bramble.test.BrambleIntegrationTestComponent; + +import javax.inject.Singleton; + +import dagger.Component; + +@Singleton +@Component(modules = { + BrambleCoreIntegrationTestModule.class, + BrambleCoreModule.class +}) +interface TransportKeyAgreementTestComponent + extends BrambleIntegrationTestComponent { + + KeyManager getKeyManager(); + + TransportKeyAgreementManagerImpl getTransportKeyAgreementManager(); + + ContactManager getContactManager(); + + LifecycleManager getLifecycleManager(); +} From 195123e6696713e60295c0f303752f0485e32e5f Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Thu, 24 Jun 2021 11:22:47 -0300 Subject: [PATCH 4/7] Ensure that private key is not stored anymore --- .../TransportKeyAgreementIntegrationTest.java | 39 +++++++++++++++++++ .../TransportKeyAgreementTestComponent.java | 5 +++ 2 files changed, 44 insertions(+) diff --git a/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementIntegrationTest.java b/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementIntegrationTest.java index e37ebc353..44b400e60 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementIntegrationTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementIntegrationTest.java @@ -2,15 +2,19 @@ package org.briarproject.bramble.transport.agreement; import org.briarproject.bramble.BrambleCoreIntegrationTestEagerSingletons; import org.briarproject.bramble.api.Pair; +import org.briarproject.bramble.api.contact.Contact; import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.contact.ContactManager; import org.briarproject.bramble.api.crypto.SecretKey; +import org.briarproject.bramble.api.data.BdfDictionary; import org.briarproject.bramble.api.identity.Author; import org.briarproject.bramble.api.identity.AuthorId; import org.briarproject.bramble.api.identity.Identity; import org.briarproject.bramble.api.identity.IdentityManager; import org.briarproject.bramble.api.lifecycle.LifecycleManager; import org.briarproject.bramble.api.plugin.TransportId; +import org.briarproject.bramble.api.sync.Group; +import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.bramble.test.BrambleIntegrationTest; import org.briarproject.bramble.test.TestDatabaseConfigModule; import org.briarproject.bramble.test.TestPluginConfigModule; @@ -19,12 +23,17 @@ import org.junit.Before; import org.junit.Test; import java.io.File; +import java.util.Map; +import static org.briarproject.bramble.api.transport.agreement.TransportKeyAgreementManager.CLIENT_ID; +import static org.briarproject.bramble.api.transport.agreement.TransportKeyAgreementManager.MAJOR_VERSION; import static org.briarproject.bramble.test.TestPluginConfigModule.DUPLEX_TRANSPORT_ID; import static org.briarproject.bramble.test.TestPluginConfigModule.SIMPLEX_TRANSPORT_ID; import static org.briarproject.bramble.test.TestUtils.getSecretKey; +import static org.briarproject.bramble.transport.agreement.TransportKeyAgreementConstants.MSG_KEY_IS_SESSION; import static org.briarproject.bramble.util.StringUtils.getRandomString; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; public class TransportKeyAgreementIntegrationTest @@ -143,6 +152,10 @@ public class TransportKeyAgreementIntegrationTest .canSendOutgoingStreams(bobId, newTransportId)); assertTrue(bob.getKeyManager() .canSendOutgoingStreams(aliceId, newTransportId)); + + // Ensure that private key is not stored anymore. + assertLocalKeyPairIsNull(alice, bobId); + assertLocalKeyPairIsNull(bob, aliceId); } private Pair addContacts() throws Exception { @@ -193,4 +206,30 @@ public class TransportKeyAgreementIntegrationTest return newDevice; } + /** + * Asserts that the local key pair (specifically the private key) is removed + * from the session as intended when leaving the AWAIT_KEY state. + * If it remained on disk after the keys had been activated + * then we'd lose forward secrecy. + */ + private void assertLocalKeyPairIsNull( + TransportKeyAgreementTestComponent device, ContactId contactId) + throws Exception { + Contact contact = device.getContactManager().getContact(contactId); + Group group = getContactGroup(device, contact); + Map map = bob.getClientHelper() + .getMessageMetadataAsDictionary(group.getId()); + for (Map.Entry e : map.entrySet()) { + if (!e.getValue().getBoolean(MSG_KEY_IS_SESSION)) continue; + Session s = device.getSessionParser().parseSession(e.getValue()); + assertNull(s.getLocalKeyPair()); + } + } + + private Group getContactGroup(TransportKeyAgreementTestComponent device, + Contact c) { + return device.getContactGroupFactory().createContactGroup(CLIENT_ID, + MAJOR_VERSION, c); + } + } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementTestComponent.java b/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementTestComponent.java index 6c8a5d0c6..a2bd0d280 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementTestComponent.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementTestComponent.java @@ -1,6 +1,7 @@ package org.briarproject.bramble.transport.agreement; import org.briarproject.bramble.BrambleCoreModule; +import org.briarproject.bramble.api.client.ContactGroupFactory; import org.briarproject.bramble.api.contact.ContactManager; import org.briarproject.bramble.api.lifecycle.LifecycleManager; import org.briarproject.bramble.api.transport.KeyManager; @@ -26,4 +27,8 @@ interface TransportKeyAgreementTestComponent ContactManager getContactManager(); LifecycleManager getLifecycleManager(); + + ContactGroupFactory getContactGroupFactory(); + + SessionParser getSessionParser(); } From e8428925ae5ac8b8999e213c5ddf54e6e371bc5a Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Thu, 24 Jun 2021 12:26:45 -0300 Subject: [PATCH 5/7] Add two more tests to TransportKeyAgreementIntegrationTest --- .../TransportKeyAgreementIntegrationTest.java | 117 ++++++++++++++++-- 1 file changed, 106 insertions(+), 11 deletions(-) diff --git a/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementIntegrationTest.java b/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementIntegrationTest.java index 44b400e60..c2dacedc6 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementIntegrationTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementIntegrationTest.java @@ -108,10 +108,58 @@ public class TransportKeyAgreementIntegrationTest lifecycleManager.waitForShutdown(); } + @Test + public void testBothAddTransportAtTheSameTime() throws Exception { + // Alice and Bob add each other. + Pair contactIds = addContacts(true); + ContactId aliceId = contactIds.getFirst(); + ContactId bobId = contactIds.getSecond(); + + // Alice and Bob restart and come back with the new transport. + alice = restartWithNewTransport(alice, aliceDir, "Alice"); + bob = restartWithNewTransport(bob, bobDir, "Bob"); + + // They can still send via the old simplex, + // but not via the new duplex transport + assertTrue(alice.getKeyManager() + .canSendOutgoingStreams(bobId, SIMPLEX_TRANSPORT_ID)); + assertFalse(alice.getKeyManager() + .canSendOutgoingStreams(bobId, newTransportId)); + assertTrue(bob.getKeyManager() + .canSendOutgoingStreams(aliceId, SIMPLEX_TRANSPORT_ID)); + assertFalse(bob.getKeyManager() + .canSendOutgoingStreams(aliceId, newTransportId)); + + // Bobs has started a session and sends KEY message to Alice + syncMessage(bob, alice, aliceId, 1, true); + + // Alice now and sends her own KEY as well as her ACTIVATE message. + syncMessage(alice, bob, bobId, 2, true); + + // Bob can already send over the new transport while Alice still can't. + assertFalse(alice.getKeyManager() + .canSendOutgoingStreams(bobId, newTransportId)); + assertTrue(bob.getKeyManager() + .canSendOutgoingStreams(aliceId, newTransportId)); + + // Now Bob sends his ACTIVATE message to Alice. + syncMessage(bob, alice, aliceId, 1, true); + + // Now Alice can also send over the new transport. + assertTrue(alice.getKeyManager() + .canSendOutgoingStreams(bobId, newTransportId)); + assertTrue(bob.getKeyManager() + .canSendOutgoingStreams(aliceId, newTransportId)); + + // Ensure that private key is not stored anymore. + assertLocalKeyPairIsNull(alice, bobId); + assertLocalKeyPairIsNull(bob, aliceId); + } + @Test public void testAliceAddsTransportBeforeBob() throws Exception { // Alice and Bob add each other. - Pair contactIds = addContacts(); + Pair contactIds = addContacts(true); ContactId aliceId = contactIds.getFirst(); ContactId bobId = contactIds.getSecond(); @@ -135,7 +183,7 @@ public class TransportKeyAgreementIntegrationTest // Alice's pending KEY message now gets delivered async, so wait for it awaitPendingMessageDelivery(1); - // Bobs now and sends his own KEY as well as his ACTIVATE message. + // Bob now sends his own KEY as well as his ACTIVATE message. syncMessage(bob, alice, aliceId, 2, true); // Alice can already send over the new transport while Bob still can't. @@ -158,19 +206,66 @@ public class TransportKeyAgreementIntegrationTest assertLocalKeyPairIsNull(bob, aliceId); } - private Pair addContacts() throws Exception { + @Test + public void testAliceAlreadyHasTransportWhenAddingBob() throws Exception { + // Alice restarts and comes back with the new transport. + alice = restartWithNewTransport(alice, aliceDir, "Alice"); + + // Alice and Bob add each other. + Pair contactIds = addContacts(false); + ContactId aliceId = contactIds.getFirst(); + ContactId bobId = contactIds.getSecond(); + + // Alice can still send via the old simplex, + // but not via the new duplex transport + assertTrue(alice.getKeyManager() + .canSendOutgoingStreams(bobId, SIMPLEX_TRANSPORT_ID)); + // FIXME normally Alice should not be able to already send streams +// assertFalse(alice.getKeyManager() +// .canSendOutgoingStreams(bobId, newTransportId)); + + // Bob restarts and comes back with the new transport. + bob = restartWithNewTransport(bob, bobDir, "Bob"); + + // Bob sends his own KEY message. + syncMessage(bob, alice, aliceId, 1, true); + + // Alice can already send over the new transport while Bob still can't. + assertTrue(alice.getKeyManager() + .canSendOutgoingStreams(bobId, newTransportId)); + assertFalse(bob.getKeyManager() + .canSendOutgoingStreams(aliceId, newTransportId)); + + // Now Alice sends her KEY and her ACTIVATE message to Bob. + syncMessage(alice, bob, bobId, 2, true); + + // Now Bob can also send over the new transport. + assertTrue(alice.getKeyManager() + .canSendOutgoingStreams(bobId, newTransportId)); + assertTrue(bob.getKeyManager() + .canSendOutgoingStreams(aliceId, newTransportId)); + + // Ensure that private key is not stored anymore. + assertLocalKeyPairIsNull(alice, bobId); + assertLocalKeyPairIsNull(bob, aliceId); + } + + private Pair addContacts( + boolean assertOldDuplexSending) throws Exception { ContactId bobId = addContact(alice, bob, true); ContactId aliceId = addContact(bob, alice, false); // Alice and Bob can send messages via the default test transports - assertTrue(alice.getKeyManager() - .canSendOutgoingStreams(bobId, SIMPLEX_TRANSPORT_ID)); - assertTrue(alice.getKeyManager() - .canSendOutgoingStreams(bobId, DUPLEX_TRANSPORT_ID)); - assertTrue(bob.getKeyManager() - .canSendOutgoingStreams(aliceId, SIMPLEX_TRANSPORT_ID)); - assertTrue(bob.getKeyManager() - .canSendOutgoingStreams(aliceId, DUPLEX_TRANSPORT_ID)); + if (assertOldDuplexSending) { + assertTrue(alice.getKeyManager() + .canSendOutgoingStreams(bobId, SIMPLEX_TRANSPORT_ID)); + assertTrue(alice.getKeyManager() + .canSendOutgoingStreams(bobId, DUPLEX_TRANSPORT_ID)); + assertTrue(bob.getKeyManager() + .canSendOutgoingStreams(aliceId, SIMPLEX_TRANSPORT_ID)); + assertTrue(bob.getKeyManager() + .canSendOutgoingStreams(aliceId, DUPLEX_TRANSPORT_ID)); + } // Sync initial client versioning updates syncMessage(alice, bob, bobId, 1, true); From ccec17f28a2931e6a79941f5357c1544a941c5fd Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Thu, 24 Jun 2021 15:57:19 -0300 Subject: [PATCH 6/7] Also test that messages arrive and activate keys --- .../bramble/connection/Connection.java | 1 - .../bramble/test/BrambleIntegrationTest.java | 19 ++- .../TransportKeyAgreementIntegrationTest.java | 119 +++++++++++++++--- .../TransportKeyAgreementTestComponent.java | 6 + 4 files changed, 125 insertions(+), 20 deletions(-) diff --git a/bramble-core/src/main/java/org/briarproject/bramble/connection/Connection.java b/bramble-core/src/main/java/org/briarproject/bramble/connection/Connection.java index 1efe2d480..9ca5c672e 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/connection/Connection.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/connection/Connection.java @@ -45,7 +45,6 @@ abstract class Connection { @Nullable StreamContext recogniseTag(TransportConnectionReader reader, TransportId transportId) { - StreamContext ctx; try { byte[] tag = readTag(reader.getInputStream()); return keyManager.getStreamContext(transportId, tag); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleIntegrationTest.java b/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleIntegrationTest.java index cc5f17ed2..68e9fe39b 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleIntegrationTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/test/BrambleIntegrationTest.java @@ -13,6 +13,7 @@ import org.briarproject.bramble.api.event.EventBus; import org.briarproject.bramble.api.event.EventListener; import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault; import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault; +import org.briarproject.bramble.api.plugin.TransportId; import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.bramble.api.sync.event.MessageStateChangedEvent; import org.briarproject.bramble.api.sync.event.MessagesAckedEvent; @@ -152,6 +153,13 @@ public abstract class BrambleIntegrationTest 0) { validationWaiter.await(TIMEOUT, numPendingOrInvalid); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementIntegrationTest.java b/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementIntegrationTest.java index c2dacedc6..fef3e82e3 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementIntegrationTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementIntegrationTest.java @@ -7,12 +7,14 @@ import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.contact.ContactManager; import org.briarproject.bramble.api.crypto.SecretKey; import org.briarproject.bramble.api.data.BdfDictionary; +import org.briarproject.bramble.api.db.DatabaseComponent; import org.briarproject.bramble.api.identity.Author; import org.briarproject.bramble.api.identity.AuthorId; import org.briarproject.bramble.api.identity.Identity; import org.briarproject.bramble.api.identity.IdentityManager; import org.briarproject.bramble.api.lifecycle.LifecycleManager; import org.briarproject.bramble.api.plugin.TransportId; +import org.briarproject.bramble.api.properties.TransportProperties; import org.briarproject.bramble.api.sync.Group; import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.bramble.test.BrambleIntegrationTest; @@ -47,6 +49,7 @@ public class TransportKeyAgreementIntegrationTest new TransportId(getRandomString(8)); private TransportKeyAgreementTestComponent alice, bob; + private Identity aliceIdentity, bobIdentity; @Before @Override @@ -56,9 +59,13 @@ public class TransportKeyAgreementIntegrationTest alice = createComponent(aliceDir, false); bob = createComponent(bobDir, false); + // Create identities + aliceIdentity = alice.getIdentityManager().createIdentity("Alice"); + bobIdentity = bob.getIdentityManager().createIdentity("Bob"); + // Start both lifecycles - startLifecycle(alice, "Alice"); - startLifecycle(bob, "Bob"); + startLifecycle(alice, aliceIdentity); + startLifecycle(bob, bobIdentity); } private TransportKeyAgreementTestComponent createComponent( @@ -79,13 +86,11 @@ public class TransportKeyAgreementIntegrationTest private void startLifecycle( TransportKeyAgreementTestComponent device, - String identityName) throws Exception { + Identity identity) throws Exception { // Listen to message related events first to not miss early ones addEventListener(device); - // Add an identity for the user - IdentityManager identityManager = device.getIdentityManager(); - Identity identity = identityManager.createIdentity(identityName); - identityManager.registerIdentity(identity); + // Register identity before starting lifecycle + device.getIdentityManager().registerIdentity(identity); // Start the lifecycle manager LifecycleManager lifecycleManager = device.getLifecycleManager(); lifecycleManager.startServices(masterKey); // re-using masterKey here @@ -116,8 +121,8 @@ public class TransportKeyAgreementIntegrationTest ContactId bobId = contactIds.getSecond(); // Alice and Bob restart and come back with the new transport. - alice = restartWithNewTransport(alice, aliceDir, "Alice"); - bob = restartWithNewTransport(bob, bobDir, "Bob"); + alice = restartWithNewTransport(alice, aliceDir, aliceIdentity); + bob = restartWithNewTransport(bob, bobDir, bobIdentity); // They can still send via the old simplex, // but not via the new duplex transport @@ -154,6 +159,10 @@ public class TransportKeyAgreementIntegrationTest // Ensure that private key is not stored anymore. assertLocalKeyPairIsNull(alice, bobId); assertLocalKeyPairIsNull(bob, aliceId); + + // Messages can be send over the new transport in both directions. + assertTransportMessageArrives(alice, bob, bobId, newTransportId); + assertTransportMessageArrives(bob, alice, aliceId, newTransportId); } @Test @@ -164,7 +173,7 @@ public class TransportKeyAgreementIntegrationTest ContactId bobId = contactIds.getSecond(); // Alice restarts and comes back with the new transport. - alice = restartWithNewTransport(alice, aliceDir, "Alice"); + alice = restartWithNewTransport(alice, aliceDir, aliceIdentity); // Alice can still send via the old simplex, // but not via the new duplex transport @@ -178,7 +187,7 @@ public class TransportKeyAgreementIntegrationTest syncMessage(alice, bob, bobId, 1, false); // Bob restarts and comes back with the new transport. - bob = restartWithNewTransport(bob, bobDir, "Bob"); + bob = restartWithNewTransport(bob, bobDir, bobIdentity); // Alice's pending KEY message now gets delivered async, so wait for it awaitPendingMessageDelivery(1); @@ -204,12 +213,16 @@ public class TransportKeyAgreementIntegrationTest // Ensure that private key is not stored anymore. assertLocalKeyPairIsNull(alice, bobId); assertLocalKeyPairIsNull(bob, aliceId); + + // Messages can be send over the new transport in both directions. + assertTransportMessageArrives(alice, bob, bobId, newTransportId); + assertTransportMessageArrives(bob, alice, aliceId, newTransportId); } @Test public void testAliceAlreadyHasTransportWhenAddingBob() throws Exception { // Alice restarts and comes back with the new transport. - alice = restartWithNewTransport(alice, aliceDir, "Alice"); + alice = restartWithNewTransport(alice, aliceDir, aliceIdentity); // Alice and Bob add each other. Pair contactIds = addContacts(false); @@ -225,7 +238,7 @@ public class TransportKeyAgreementIntegrationTest // .canSendOutgoingStreams(bobId, newTransportId)); // Bob restarts and comes back with the new transport. - bob = restartWithNewTransport(bob, bobDir, "Bob"); + bob = restartWithNewTransport(bob, bobDir, bobIdentity); // Bob sends his own KEY message. syncMessage(bob, alice, aliceId, 1, true); @@ -248,6 +261,68 @@ public class TransportKeyAgreementIntegrationTest // Ensure that private key is not stored anymore. assertLocalKeyPairIsNull(alice, bobId); assertLocalKeyPairIsNull(bob, aliceId); + + // Bobs still sends his ACTIVATE message. + syncMessage(bob, alice, aliceId, 1, true); + + // Messages can be send over the new transport in both directions. + assertTransportMessageArrives(alice, bob, bobId, newTransportId); + assertTransportMessageArrives(bob, alice, aliceId, newTransportId); + } + + @Test + public void testAliceActivatesKeysByIncomingMessage() throws Exception { + // Alice and Bob add each other. + Pair contactIds = addContacts(true); + ContactId aliceId = contactIds.getFirst(); + ContactId bobId = contactIds.getSecond(); + + // Alice and Bob restart and come back with the new transport. + alice = restartWithNewTransport(alice, aliceDir, aliceIdentity); + bob = restartWithNewTransport(bob, bobDir, bobIdentity); + + // They can still send via the old simplex, + // but not via the new duplex transport + assertTrue(alice.getKeyManager() + .canSendOutgoingStreams(bobId, SIMPLEX_TRANSPORT_ID)); + assertFalse(alice.getKeyManager() + .canSendOutgoingStreams(bobId, newTransportId)); + assertTrue(bob.getKeyManager() + .canSendOutgoingStreams(aliceId, SIMPLEX_TRANSPORT_ID)); + assertFalse(bob.getKeyManager() + .canSendOutgoingStreams(aliceId, newTransportId)); + + // Bobs has started a session and sends KEY message to Alice + syncMessage(bob, alice, aliceId, 1, true); + + // Alice now and sends her own KEY as well as her ACTIVATE message. + syncMessage(alice, bob, bobId, 2, true); + + // Bob can already send over the new transport while Alice still can't. + assertFalse(alice.getKeyManager() + .canSendOutgoingStreams(bobId, newTransportId)); + assertTrue(bob.getKeyManager() + .canSendOutgoingStreams(aliceId, newTransportId)); + + // Bob's database mysteriously loses the ACTIVATE message, + // so it won't be send to Alice. + Contact contact = bob.getContactManager().getContact(aliceId); + Group group = getContactGroup(bob, contact); + Map map = bob.getClientHelper() + .getMessageMetadataAsDictionary(group.getId()); + DatabaseComponent db = bob.getDatabaseComponent(); + for (Map.Entry e : map.entrySet()) { + if (e.getValue().getBoolean(MSG_KEY_IS_SESSION)) continue; + db.transaction(false, txn -> db.removeMessage(txn, e.getKey())); + } + + // Bob sends a message to Alice + assertTransportMessageArrives(bob, alice, aliceId, newTransportId); + + // Now without receiving the ACTIVATE, Alice can already send to Bob + assertTrue(alice.getKeyManager() + .canSendOutgoingStreams(bobId, newTransportId)); + assertTransportMessageArrives(alice, bob, bobId, newTransportId); } private Pair addContacts( @@ -292,12 +367,12 @@ public class TransportKeyAgreementIntegrationTest } private TransportKeyAgreementTestComponent restartWithNewTransport( - TransportKeyAgreementTestComponent device, File dir, String name) - throws Exception { + TransportKeyAgreementTestComponent device, File dir, + Identity identity) throws Exception { tearDown(device); TransportKeyAgreementTestComponent newDevice = createComponent(dir, true); - startLifecycle(newDevice, name); + startLifecycle(newDevice, identity); return newDevice; } @@ -312,7 +387,7 @@ public class TransportKeyAgreementIntegrationTest throws Exception { Contact contact = device.getContactManager().getContact(contactId); Group group = getContactGroup(device, contact); - Map map = bob.getClientHelper() + Map map = device.getClientHelper() .getMessageMetadataAsDictionary(group.getId()); for (Map.Entry e : map.entrySet()) { if (!e.getValue().getBoolean(MSG_KEY_IS_SESSION)) continue; @@ -327,4 +402,14 @@ public class TransportKeyAgreementIntegrationTest MAJOR_VERSION, c); } + private void assertTransportMessageArrives( + TransportKeyAgreementTestComponent from, + TransportKeyAgreementTestComponent to, ContactId toId, + TransportId transportId) throws Exception { + TransportProperties p = new TransportProperties(); + p.putBoolean("foo", true); + from.getTransportPropertyManager().mergeLocalProperties(transportId, p); + syncMessage(from, to, toId, transportId, 1, true); + } + } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementTestComponent.java b/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementTestComponent.java index a2bd0d280..a0de3c5e4 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementTestComponent.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementTestComponent.java @@ -3,7 +3,9 @@ package org.briarproject.bramble.transport.agreement; import org.briarproject.bramble.BrambleCoreModule; import org.briarproject.bramble.api.client.ContactGroupFactory; import org.briarproject.bramble.api.contact.ContactManager; +import org.briarproject.bramble.api.db.DatabaseComponent; import org.briarproject.bramble.api.lifecycle.LifecycleManager; +import org.briarproject.bramble.api.properties.TransportPropertyManager; import org.briarproject.bramble.api.transport.KeyManager; import org.briarproject.bramble.test.BrambleCoreIntegrationTestModule; import org.briarproject.bramble.test.BrambleIntegrationTestComponent; @@ -31,4 +33,8 @@ interface TransportKeyAgreementTestComponent ContactGroupFactory getContactGroupFactory(); SessionParser getSessionParser(); + + TransportPropertyManager getTransportPropertyManager(); + + DatabaseComponent getDatabaseComponent(); } From be3700d3646d728b1c4a92d5e1512ec9658e9f72 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 30 Jun 2021 16:57:32 -0300 Subject: [PATCH 7/7] Remove FIXME in test since we won't fix it this way --- .../agreement/TransportKeyAgreementIntegrationTest.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementIntegrationTest.java b/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementIntegrationTest.java index fef3e82e3..d16b6052c 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementIntegrationTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/transport/agreement/TransportKeyAgreementIntegrationTest.java @@ -233,9 +233,12 @@ public class TransportKeyAgreementIntegrationTest // but not via the new duplex transport assertTrue(alice.getKeyManager() .canSendOutgoingStreams(bobId, SIMPLEX_TRANSPORT_ID)); - // FIXME normally Alice should not be able to already send streams -// assertFalse(alice.getKeyManager() -// .canSendOutgoingStreams(bobId, newTransportId)); + // Normally, Alice should not be able to send streams already. + // However, she does already derive keys for the transport. + // The UI checks RemovableDriveManager#isTransportSupportedByContact() + // in practice to prevent sending streams that Bob can't decrypt. + assertTrue(alice.getKeyManager() + .canSendOutgoingStreams(bobId, newTransportId)); // Bob restarts and comes back with the new transport. bob = restartWithNewTransport(bob, bobDir, bobIdentity);