Merge branch '695-introduction-failure' into 'master'

Fix regression in IntroduceeManager

This was happening when the remote response arrives before the local
response is made and thus the local response needs to be send with the
ACK following. The problem was that we ACK was sent before the response
which is not allowed and resulted in the session being aborted by the
introducee. This was happening, because recursion is hard ;)

The fix is only restarting another protocol engine to send the ACK
after the first run has been completed.

An integration test was added to prevent such regression in the future
and to test this code path.

Closes #695

See merge request !338
This commit is contained in:
akwizgran
2016-10-04 21:38:48 +00:00
2 changed files with 70 additions and 11 deletions

View File

@@ -442,6 +442,54 @@ public class IntroductionIntegrationTest extends BriarTestCase {
}
}
@Test
public void testResponseAndAckInOneSession() throws Exception {
startLifecycles();
addDefaultIdentities();
addDefaultContacts();
addListeners(true, true);
addTransportProperties();
// make introduction
long time = clock.currentTimeMillis();
Contact introducee1 = contactManager0.getContact(contactId1);
Contact introducee2 = contactManager0.getContact(contactId2);
introductionManager0
.makeIntroduction(introducee1, introducee2, "Hi!", time);
// sync first request message
deliverMessage(sync0, contactId0, sync1, contactId1, "0 to 1");
eventWaiter.await(TIMEOUT, 1);
assertTrue(listener1.requestReceived);
// sync first response
deliverMessage(sync1, contactId1, sync0, contactId0, "1 to 0");
eventWaiter.await(TIMEOUT, 1);
assertTrue(listener0.response1Received);
// don't let 2 answer the request right away
// to have the response arrive first
listener2.answerRequests = false;
// sync second request message and first response
deliverMessage(sync0, contactId0, sync2, contactId2, 2, "0 to 2");
eventWaiter.await(TIMEOUT, 1);
assertTrue(listener2.requestReceived);
// answer request manually
introductionManager2
.acceptIntroduction(contactId0, listener2.sessionId, time);
// sync second response and ACK and make sure there is no abort
deliverMessage(sync2, contactId2, sync0, contactId0, 2, "2 to 0");
eventWaiter.await(TIMEOUT, 1);
assertTrue(listener0.response2Received);
assertFalse(listener0.aborted);
stopLifecycles();
}
@Test
public void testIntroductionToSameContact() throws Exception {
startLifecycles();
@@ -1169,6 +1217,8 @@ public class IntroductionIntegrationTest extends BriarTestCase {
private volatile boolean requestReceived = false;
private volatile boolean succeeded = false;
private volatile boolean aborted = false;
private volatile boolean answerRequests = true;
private volatile SessionId sessionId;
private final int introducee;
private final boolean accept;
@@ -1194,10 +1244,10 @@ public class IntroductionIntegrationTest extends BriarTestCase {
requestReceived = true;
IntroductionRequest ir = introEvent.getIntroductionRequest();
ContactId contactId = introEvent.getContactId();
SessionId sessionId = ir.getSessionId();
sessionId = ir.getSessionId();
long time = clock.currentTimeMillis();
try {
if (introducee == 1) {
if (introducee == 1 && answerRequests) {
if (accept) {
introductionManager1
.acceptIntroduction(contactId, sessionId,
@@ -1207,7 +1257,7 @@ public class IntroductionIntegrationTest extends BriarTestCase {
.declineIntroduction(contactId, sessionId,
time);
}
} else if (introducee == 2) {
} else if (introducee == 2 && answerRequests) {
if (accept) {
introductionManager2
.acceptIntroduction(contactId, sessionId,

View File

@@ -32,6 +32,7 @@ import org.briarproject.api.sync.GroupId;
import org.briarproject.api.sync.Message;
import org.briarproject.api.sync.MessageId;
import org.briarproject.api.system.Clock;
import org.jetbrains.annotations.Nullable;
import java.io.IOException;
import java.security.GeneralSecurityException;
@@ -243,7 +244,7 @@ class IntroduceeManager {
result) throws DbException, FormatException {
// perform actions based on new local state
performTasks(txn, result.localState);
BdfDictionary followUpAction = performTasks(txn, result.localState);
// save new local state
MessageId storageId =
@@ -269,13 +270,21 @@ class IntroduceeManager {
db.deleteMessage(txn, messageId);
db.deleteMessageMetadata(txn, messageId);
}
// process follow up action at the end if available
if (followUpAction != null) {
IntroduceeEngine engine = new IntroduceeEngine();
processStateUpdate(txn, null,
engine.onLocalAction(result.localState, followUpAction));
}
}
private void performTasks(Transaction txn, BdfDictionary localState)
@Nullable
private BdfDictionary performTasks(Transaction txn, BdfDictionary localState)
throws FormatException, DbException {
if (!localState.containsKey(TASK) || localState.get(TASK) == NULL_VALUE)
return;
return null;
// remember task and remove it from localState
long task = localState.getLong(TASK);
@@ -285,7 +294,7 @@ class IntroduceeManager {
if (localState.getBoolean(EXISTS)) {
// we have this contact already, so do not perform actions
LOG.info("We have this contact already, do not add");
return;
return null;
}
// figure out who takes which role by comparing public keys
@@ -348,10 +357,9 @@ class IntroduceeManager {
BdfDictionary localAction = new BdfDictionary();
localAction.put(TYPE, TYPE_ACK);
// start engine and process its state update
IntroduceeEngine engine = new IntroduceeEngine();
processStateUpdate(txn, null,
engine.onLocalAction(localState, localAction));
// return follow up action to start engine
// and process its state update again
return localAction;
}
// we sent and received an ACK, so activate contact
@@ -394,6 +402,7 @@ class IntroduceeManager {
contactManager.removeContact(txn, contactId);
}
}
return null;
}
private SecretKey deriveSecretKey(byte[] publicKeyBytes,