From c7522dae1f7e531b63390721adc6eb9e41f5900b Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 24 Sep 2018 13:16:41 +0100 Subject: [PATCH] Show different error message if QR code is too new. --- .../api/UnsupportedVersionException.java | 9 - .../keyagreement/KeyAgreementConstants.java | 8 +- .../UnsupportedVersionException.java | 20 +++ .../keyagreement/PayloadParserImpl.java | 10 +- .../keyagreement/PayloadParserImplTest.java | 156 ++++++++++++++++++ .../keyagreement/KeyAgreementFragment.java | 12 +- briar-android/src/main/res/values/strings.xml | 3 +- 7 files changed, 201 insertions(+), 17 deletions(-) delete mode 100644 bramble-api/src/main/java/org/briarproject/bramble/api/UnsupportedVersionException.java create mode 100644 bramble-api/src/main/java/org/briarproject/bramble/api/keyagreement/UnsupportedVersionException.java create mode 100644 bramble-core/src/test/java/org/briarproject/bramble/keyagreement/PayloadParserImplTest.java diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/UnsupportedVersionException.java b/bramble-api/src/main/java/org/briarproject/bramble/api/UnsupportedVersionException.java deleted file mode 100644 index f7cf29856..000000000 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/UnsupportedVersionException.java +++ /dev/null @@ -1,9 +0,0 @@ -package org.briarproject.bramble.api; - -import java.io.IOException; - -/** - * An exception that indicates an unrecoverable version mismatch. - */ -public class UnsupportedVersionException extends IOException { -} diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/keyagreement/KeyAgreementConstants.java b/bramble-api/src/main/java/org/briarproject/bramble/api/keyagreement/KeyAgreementConstants.java index 1063b38f3..d329bff3f 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/keyagreement/KeyAgreementConstants.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/keyagreement/KeyAgreementConstants.java @@ -3,7 +3,13 @@ package org.briarproject.bramble.api.keyagreement; public interface KeyAgreementConstants { /** - * The current version of the BQP protocol. Version number 89 is reserved. + * The version of the BQP protocol used in beta releases. This version + * number is reserved. + */ + byte BETA_PROTOCOL_VERSION = 89; + + /** + * The current version of the BQP protocol. */ byte PROTOCOL_VERSION = 4; diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/keyagreement/UnsupportedVersionException.java b/bramble-api/src/main/java/org/briarproject/bramble/api/keyagreement/UnsupportedVersionException.java new file mode 100644 index 000000000..3f861d83b --- /dev/null +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/keyagreement/UnsupportedVersionException.java @@ -0,0 +1,20 @@ +package org.briarproject.bramble.api.keyagreement; + +import java.io.IOException; + +/** + * Thrown when a QR code that has been scanned uses a protocol version that is + * not supported. + */ +public class UnsupportedVersionException extends IOException { + + private final boolean tooOld; + + public UnsupportedVersionException(boolean tooOld) { + this.tooOld = tooOld; + } + + public boolean isTooOld() { + return tooOld; + } +} diff --git a/bramble-core/src/main/java/org/briarproject/bramble/keyagreement/PayloadParserImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/keyagreement/PayloadParserImpl.java index 62dee9e83..2f6096760 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/keyagreement/PayloadParserImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/keyagreement/PayloadParserImpl.java @@ -1,13 +1,13 @@ package org.briarproject.bramble.keyagreement; import org.briarproject.bramble.api.FormatException; -import org.briarproject.bramble.api.UnsupportedVersionException; import org.briarproject.bramble.api.data.BdfList; import org.briarproject.bramble.api.data.BdfReader; import org.briarproject.bramble.api.data.BdfReaderFactory; import org.briarproject.bramble.api.keyagreement.Payload; import org.briarproject.bramble.api.keyagreement.PayloadParser; import org.briarproject.bramble.api.keyagreement.TransportDescriptor; +import org.briarproject.bramble.api.keyagreement.UnsupportedVersionException; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.plugin.BluetoothConstants; import org.briarproject.bramble.api.plugin.LanTcpConstants; @@ -21,6 +21,7 @@ import java.util.List; import javax.annotation.concurrent.Immutable; import javax.inject.Inject; +import static org.briarproject.bramble.api.keyagreement.KeyAgreementConstants.BETA_PROTOCOL_VERSION; import static org.briarproject.bramble.api.keyagreement.KeyAgreementConstants.COMMIT_LENGTH; import static org.briarproject.bramble.api.keyagreement.KeyAgreementConstants.PROTOCOL_VERSION; import static org.briarproject.bramble.api.keyagreement.KeyAgreementConstants.TRANSPORT_ID_BLUETOOTH; @@ -43,8 +44,11 @@ class PayloadParserImpl implements PayloadParser { // First byte: the protocol version int protocolVersion = in.read(); if (protocolVersion == -1) throw new FormatException(); - if (protocolVersion != PROTOCOL_VERSION) - throw new UnsupportedVersionException(); + if (protocolVersion != PROTOCOL_VERSION) { + boolean tooOld = protocolVersion < PROTOCOL_VERSION || + protocolVersion == BETA_PROTOCOL_VERSION; + throw new UnsupportedVersionException(tooOld); + } // The rest of the payload is a BDF list with one or more elements BdfReader r = bdfReaderFactory.createReader(in); BdfList payload = r.readList(); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/keyagreement/PayloadParserImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/keyagreement/PayloadParserImplTest.java new file mode 100644 index 000000000..3b072547f --- /dev/null +++ b/bramble-core/src/test/java/org/briarproject/bramble/keyagreement/PayloadParserImplTest.java @@ -0,0 +1,156 @@ +package org.briarproject.bramble.keyagreement; + +import org.briarproject.bramble.api.Bytes; +import org.briarproject.bramble.api.FormatException; +import org.briarproject.bramble.api.data.BdfList; +import org.briarproject.bramble.api.data.BdfReader; +import org.briarproject.bramble.api.data.BdfReaderFactory; +import org.briarproject.bramble.api.keyagreement.Payload; +import org.briarproject.bramble.api.keyagreement.UnsupportedVersionException; +import org.briarproject.bramble.test.BrambleMockTestCase; +import org.jmock.Expectations; +import org.junit.Test; + +import java.io.ByteArrayInputStream; + +import static org.briarproject.bramble.api.keyagreement.KeyAgreementConstants.BETA_PROTOCOL_VERSION; +import static org.briarproject.bramble.api.keyagreement.KeyAgreementConstants.COMMIT_LENGTH; +import static org.briarproject.bramble.api.keyagreement.KeyAgreementConstants.PROTOCOL_VERSION; +import static org.briarproject.bramble.test.TestUtils.getRandomBytes; +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +public class PayloadParserImplTest extends BrambleMockTestCase { + + private final BdfReaderFactory bdfReaderFactory = + context.mock(BdfReaderFactory.class); + private final BdfReader bdfReader = context.mock(BdfReader.class); + + private final PayloadParserImpl payloadParser = + new PayloadParserImpl(bdfReaderFactory); + + @Test(expected = FormatException.class) + public void testThrowsFormatExceptionIfPayloadIsEmpty() throws Exception { + payloadParser.parse(new byte[0]); + } + + @Test + public void testThrowsUnsupportedVersionExceptionForOldVersion() + throws Exception { + try { + payloadParser.parse(new byte[] {PROTOCOL_VERSION - 1}); + fail(); + } catch (UnsupportedVersionException e) { + assertTrue(e.isTooOld()); + } + } + + @Test + public void testThrowsUnsupportedVersionExceptionForBetaVersion() + throws Exception { + try { + payloadParser.parse(new byte[] {BETA_PROTOCOL_VERSION}); + fail(); + } catch (UnsupportedVersionException e) { + assertTrue(e.isTooOld()); + } + } + + @Test + public void testThrowsUnsupportedVersionExceptionForNewVersion() + throws Exception { + try { + payloadParser.parse(new byte[] {PROTOCOL_VERSION + 1}); + fail(); + } catch (UnsupportedVersionException e) { + assertFalse(e.isTooOld()); + } + } + + @Test(expected = FormatException.class) + public void testThrowsFormatExceptionForEmptyList() throws Exception { + context.checking(new Expectations() {{ + oneOf(bdfReaderFactory).createReader( + with(any(ByteArrayInputStream.class))); + will(returnValue(bdfReader)); + oneOf(bdfReader).readList(); + will(returnValue(new BdfList())); + }}); + + payloadParser.parse(new byte[] {PROTOCOL_VERSION}); + } + + @Test(expected = FormatException.class) + public void testThrowsFormatExceptionForDataAfterList() + throws Exception { + byte[] commitment = getRandomBytes(COMMIT_LENGTH); + + context.checking(new Expectations() {{ + oneOf(bdfReaderFactory).createReader( + with(any(ByteArrayInputStream.class))); + will(returnValue(bdfReader)); + oneOf(bdfReader).readList(); + will(returnValue(BdfList.of(new Bytes(commitment)))); + oneOf(bdfReader).eof(); + will(returnValue(false)); + }}); + + payloadParser.parse(new byte[] {PROTOCOL_VERSION}); + } + + @Test(expected = FormatException.class) + public void testThrowsFormatExceptionForShortCommitment() + throws Exception { + byte[] commitment = getRandomBytes(COMMIT_LENGTH - 1); + + context.checking(new Expectations() {{ + oneOf(bdfReaderFactory).createReader( + with(any(ByteArrayInputStream.class))); + will(returnValue(bdfReader)); + oneOf(bdfReader).readList(); + will(returnValue(BdfList.of(new Bytes(commitment)))); + oneOf(bdfReader).eof(); + will(returnValue(true)); + }}); + + payloadParser.parse(new byte[] {PROTOCOL_VERSION}); + } + + @Test(expected = FormatException.class) + public void testThrowsFormatExceptionForLongCommitment() + throws Exception { + byte[] commitment = getRandomBytes(COMMIT_LENGTH + 1); + + context.checking(new Expectations() {{ + oneOf(bdfReaderFactory).createReader( + with(any(ByteArrayInputStream.class))); + will(returnValue(bdfReader)); + oneOf(bdfReader).readList(); + will(returnValue(BdfList.of(new Bytes(commitment)))); + oneOf(bdfReader).eof(); + will(returnValue(true)); + }}); + + payloadParser.parse(new byte[] {PROTOCOL_VERSION}); + } + + @Test + public void testAcceptsPayloadWithNoDescriptors() throws Exception { + byte[] commitment = getRandomBytes(COMMIT_LENGTH); + context.checking(new Expectations() {{ + oneOf(bdfReaderFactory).createReader( + with(any(ByteArrayInputStream.class))); + will(returnValue(bdfReader)); + oneOf(bdfReader).readList(); + will(returnValue(BdfList.of(new Bytes(commitment)))); + oneOf(bdfReader).eof(); + will(returnValue(true)); + }}); + + Payload p = payloadParser.parse(new byte[] {PROTOCOL_VERSION}); + assertArrayEquals(commitment, p.getCommitment()); + assertTrue(p.getTransportDescriptors().isEmpty()); + } +} diff --git a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/KeyAgreementFragment.java b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/KeyAgreementFragment.java index 86831fdab..19dcd203a 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/KeyAgreementFragment.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/keyagreement/KeyAgreementFragment.java @@ -15,7 +15,6 @@ import android.widget.Toast; import com.google.zxing.Result; -import org.briarproject.bramble.api.UnsupportedVersionException; import org.briarproject.bramble.api.event.Event; import org.briarproject.bramble.api.event.EventBus; import org.briarproject.bramble.api.keyagreement.KeyAgreementResult; @@ -23,6 +22,7 @@ import org.briarproject.bramble.api.keyagreement.KeyAgreementTask; import org.briarproject.bramble.api.keyagreement.Payload; import org.briarproject.bramble.api.keyagreement.PayloadEncoder; import org.briarproject.bramble.api.keyagreement.PayloadParser; +import org.briarproject.bramble.api.keyagreement.UnsupportedVersionException; import org.briarproject.bramble.api.keyagreement.event.KeyAgreementAbortedEvent; import org.briarproject.bramble.api.keyagreement.event.KeyAgreementFailedEvent; import org.briarproject.bramble.api.keyagreement.event.KeyAgreementFinishedEvent; @@ -244,8 +244,14 @@ public class KeyAgreementFragment extends BaseEventFragment task.connectAndRunProtocol(remotePayload); } catch (UnsupportedVersionException e) { reset(); - String msg = getString(R.string.qr_code_unsupported, - getString(R.string.app_name)); + String msg; + if (e.isTooOld()) { + msg = getString(R.string.qr_code_too_old, + getString(R.string.app_name)); + } else { + msg = getString(R.string.qr_code_too_new, + getString(R.string.app_name)); + } showNextFragment(ContactExchangeErrorFragment.newInstance(msg)); } catch (CameraException e) { logCameraExceptionAndFinish(e); diff --git a/briar-android/src/main/res/values/strings.xml b/briar-android/src/main/res/values/strings.xml index cbd7baa60..0bf72a38e 100644 --- a/briar-android/src/main/res/values/strings.xml +++ b/briar-android/src/main/res/values/strings.xml @@ -141,7 +141,8 @@ Contact added: %s Contact %s already exists The QR code is invalid - The QR code you are trying to scan belongs to an old version of %s which is no longer supported.\n\nPlease ensure that both of you are running the latest version and then try again. + The QR code you have scanned comes from an older version of %s.\n\nPlease ask your contact to upgrade to the latest version and then try again. + The QR code you have scanned comes from a newer version of %s.\n\nPlease upgrade to the latest version and then try again. Camera error Connecting to device\u2026 Authenticating with device\u2026