Address last review comments

This commit is contained in:
Torsten Grote
2018-04-27 11:04:08 -03:00
parent 80a9689316
commit 44f5a9db1e
8 changed files with 79 additions and 8 deletions

View File

@@ -391,7 +391,6 @@ class IntroduceeProtocolEngine
mac = crypto.authMac(ourMacKey, s, localAuthor.getId());
signature = crypto.sign(ourMacKey, localAuthor.getPrivateKey());
} catch (GeneralSecurityException e) {
// TODO
if (LOG.isLoggable(WARNING))
LOG.log(WARNING, e.toString(), e);
return abort(txn, s);

View File

@@ -343,6 +343,12 @@ class IntroducerProtocolEngine
// The dependency, if any, must be the last remote message
if (isInvalidDependency(s, m.getGroupId(), m.getPreviousMessageId()))
return abort(txn, s);
// The message must be expected in the current state
boolean senderIsAlice = senderIsAlice(s, m);
if (senderIsAlice && s.getState() != B_DECLINED)
return abort(txn, s);
else if (!senderIsAlice && s.getState() != A_DECLINED)
return abort(txn, s);
// Mark the response visible in the UI
markMessageVisibleInUi(txn, m.getMessageId());
@@ -350,7 +356,6 @@ class IntroducerProtocolEngine
messageTracker
.trackMessage(txn, m.getGroupId(), m.getTimestamp(), false);
boolean senderIsAlice = senderIsAlice(s, m);
Introducee introduceeA, introduceeB;
if (senderIsAlice) {
introduceeA = new Introducee(s.getIntroduceeA(), m.getMessageId());

View File

@@ -19,6 +19,9 @@ interface IntroductionCrypto {
/**
* Returns true if the local author is alice
*
* Alice is the Author whose unique ID has the lower ID,
* comparing the IDs as byte strings.
*/
boolean isAlice(AuthorId local, AuthorId remote);

View File

@@ -1,6 +1,5 @@
package org.briarproject.briar.introduction;
import org.briarproject.bramble.api.Bytes;
import org.briarproject.bramble.api.FormatException;
import org.briarproject.bramble.api.client.ClientHelper;
import org.briarproject.bramble.api.crypto.CryptoComponent;
@@ -68,9 +67,7 @@ class IntroductionCryptoImpl implements IntroductionCrypto {
@Override
public boolean isAlice(AuthorId local, AuthorId remote) {
byte[] a = local.getBytes();
byte[] b = remote.getBytes();
return Bytes.COMPARATOR.compare(new Bytes(a), new Bytes(b)) < 0;
return local.compareTo(remote) < 0;
}
@Override

View File

@@ -353,7 +353,12 @@ class IntroductionManagerImpl extends ConversationClientImpl
try {
// Look up the session
StoredSession ss = getSession(txn, sessionId);
if (ss == null) throw new IllegalArgumentException();
if (ss == null) {
// Actions from the UI may be based on stale information.
// The contact might just have been deleted, for example.
// Throwing a DbException here aborts gracefully.
throw new DbException();
}
// Parse the session
Contact contact = db.getContact(txn, contactId);
GroupId contactGroupId = getContactGroup(contact).getId();

View File

@@ -155,7 +155,7 @@ class IntroductionValidator extends BdfMessageValidator {
byte[] sessionIdBytes = body.getRaw(1);
checkLength(sessionIdBytes, UniqueId.LENGTH);
byte[] previousMessageId = body.getOptionalRaw(2);
byte[] previousMessageId = body.getRaw(2);
checkLength(previousMessageId, UniqueId.LENGTH);
byte[] mac = body.getOptionalRaw(3);

View File

@@ -63,6 +63,7 @@ import static org.briarproject.briar.introduction.IntroductionConstants.SESSION_
import static org.briarproject.briar.introduction.IntroductionConstants.SESSION_KEY_SESSION_ID;
import static org.briarproject.briar.introduction.MessageType.ACCEPT;
import static org.briarproject.briar.introduction.MessageType.AUTH;
import static org.briarproject.briar.introduction.MessageType.DECLINE;
import static org.briarproject.briar.test.BriarTestUtils.assertGroupCount;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
@@ -681,6 +682,41 @@ public class IntroductionIntegrationTest
assertTrue(listener0.aborted);
}
/**
* One introducee sends an DECLINE and then another DECLINE message.
* The introducer should notice this and ABORT the session.
*/
@Test
public void testDoubleDecline() throws Exception {
addListeners(false, true);
// make the introduction
long time = clock.currentTimeMillis();
introductionManager0
.makeIntroduction(contact1From0, contact2From0, null, time);
// sync REQUEST to introducee1
sync0To1(1, true);
// save DECLINE from introducee1
DeclineMessage m = (DeclineMessage) getMessageFor(c1.getClientHelper(),
contact0From1, DECLINE);
// sync DECLINE back to introducer
sync1To0(1, true);
// fake a second DECLINE message also from introducee1
Message msg = c1.getMessageEncoder()
.encodeDeclineMessage(m.getGroupId(), m.getTimestamp() + 1,
m.getMessageId(), m.getSessionId());
c1.getClientHelper().addLocalMessage(msg, new BdfDictionary(), true);
// sync fake DECLINE back to introducer
sync1To0(1, true);
assertTrue(listener0.aborted);
}
/**
* One introducee sends two AUTH messages.
* The introducer should notice this and ABORT the session.
@@ -1093,6 +1129,9 @@ public class IntroductionIntegrationTest
if (type == ACCEPT) {
//noinspection ConstantConditions
return c0.getMessageParser().parseAcceptMessage(m, body);
} else if (type == DECLINE) {
//noinspection ConstantConditions
return c0.getMessageParser().parseDeclineMessage(m, body);
} else if (type == AUTH) {
//noinspection ConstantConditions
return c0.getMessageParser().parseAuthMessage(m, body);

View File

@@ -268,6 +268,21 @@ public class IntroductionValidatorTest extends ValidatorTestCase {
validator.validateMessage(message, group, body);
}
@Test(expected = FormatException.class)
public void testRejectsInvalidPreviousMsgIdForAuth() throws Exception {
BdfList body = BdfList.of(AUTH.getValue(), sessionId.getBytes(),
1, getRandomBytes(MAC_BYTES),
signature);
validator.validateMessage(message, group, body);
}
@Test(expected = FormatException.class)
public void testRejectsPreviousMsgIdNullForAuth() throws Exception {
BdfList body = BdfList.of(AUTH.getValue(), sessionId.getBytes(), null,
getRandomBytes(MAC_BYTES), signature);
validator.validateMessage(message, group, body);
}
@Test(expected = FormatException.class)
public void testRejectsTooShortMacForAuth() throws Exception {
BdfList body = BdfList.of(AUTH.getValue(), sessionId.getBytes(),
@@ -358,6 +373,14 @@ public class IntroductionValidatorTest extends ValidatorTestCase {
validator.validateMessage(message, group, body);
}
@Test(expected = FormatException.class)
public void testRejectsPreviousMsgIdNullForActivate() throws Exception {
BdfList body =
BdfList.of(ACTIVATE.getValue(), sessionId.getBytes(), null,
mac);
validator.validateMessage(message, group, body);
}
@Test(expected = FormatException.class)
public void testRejectsInvalidMacForActivate() throws Exception {
BdfList body = BdfList.of(ACTIVATE.getValue(), sessionId.getBytes(),