diff --git a/briar-headless/README.md b/briar-headless/README.md index b608e5436..3b37bd571 100644 --- a/briar-headless/README.md +++ b/briar-headless/README.md @@ -105,7 +105,7 @@ The link and the alias should be posted as a JSON object: } ``` -This starts the process of adding the contact. +Adding a pending contact starts the process of adding the contact. Until it is completed, a pending contact is returned as JSON: ```json @@ -116,6 +116,71 @@ Until it is completed, a pending contact is returned as JSON: } ``` +Possible errors when adding a pending contact are: + +#### 400: Pending contact's link is invalid + +```json +{ + "error": "INVALID_LINK" +} +``` + +#### 400: Pending contact's handshake public key is invalid + +```json +{ + "error": "INVALID_PUBLIC_KEY" +} +``` + +#### 403: A contact with the same handshake public key already exists + +This error may be caused by someone attacking the user with the goal +of discovering the contacts of the user. + +In the Android client, upon encountering this issue a message dialog +is shown that asks whether the contact and the just added pending contact +are the same person. If that's the case, a message is shown that the +contact already exists and the pending contact isn't added. +If that's not the case and they are two different persons, the Android +client +[shows the following message](https://code.briarproject.org/briar/briar/-/blob/beta-1.2.14/briar-android/src/main/res/values/strings.xml#L271) +when this happens: +> [Alice] and [Bob] sent you the same link. +> +> One of them may be trying to discover who your contacts are. +> +> Don't tell them you received the same link from someone else. + +```json +{ + "error": "CONTACT_EXISTS", + "remoteAuthorName": "Bob" +} +``` + +#### 403: A pending contact with the same handshake public key already exists + +This error, too, may be caused by someone attacking the user with the goal +of discovering the contacts of the user. + +Just like above, upon encountering this issue a message dialog is shown in +the Android client that asks whether the contact and the just added pending +contact are the same person. If that's the case, the pending contact gets +updated. If that's not the case and they are two different persons, the +Android client shows the same message as above, warning the user about the +possible attack. + +```json +{ + "error": "PENDING_EXISTS", + "pendingContactId": "jsTgWcsEQ2g9rnomeK1g/hmO8M1Ix6ZIGWAjgBtlS9U=", + "pendingContactAlias": "Alice" +} +``` +----------- + Before users can send messages to contacts, they become pending contacts. In this state Briar still needs to do some work in the background (e.g. spinning up a dedicated hidden service and letting the contact connect to it). diff --git a/briar-headless/src/main/java/org/briarproject/briar/headless/contact/ContactControllerImpl.kt b/briar-headless/src/main/java/org/briarproject/briar/headless/contact/ContactControllerImpl.kt index 6815f2c52..168520e56 100644 --- a/briar-headless/src/main/java/org/briarproject/briar/headless/contact/ContactControllerImpl.kt +++ b/briar-headless/src/main/java/org/briarproject/briar/headless/contact/ContactControllerImpl.kt @@ -3,6 +3,8 @@ package org.briarproject.briar.headless.contact import com.fasterxml.jackson.databind.ObjectMapper import io.javalin.http.BadRequestResponse import io.javalin.http.Context +import io.javalin.http.ForbiddenResponse +import io.javalin.http.HttpResponseException import io.javalin.http.NotFoundResponse import org.briarproject.bramble.api.connection.ConnectionRegistry import org.briarproject.bramble.api.contact.ContactManager @@ -12,8 +14,10 @@ import org.briarproject.bramble.api.contact.event.ContactAddedEvent import org.briarproject.bramble.api.contact.event.PendingContactAddedEvent import org.briarproject.bramble.api.contact.event.PendingContactRemovedEvent import org.briarproject.bramble.api.contact.event.PendingContactStateChangedEvent +import org.briarproject.bramble.api.db.ContactExistsException import org.briarproject.bramble.api.db.NoSuchContactException import org.briarproject.bramble.api.db.NoSuchPendingContactException +import org.briarproject.bramble.api.db.PendingContactExistsException import org.briarproject.bramble.api.event.Event import org.briarproject.bramble.api.event.EventListener import org.briarproject.bramble.api.identity.AuthorConstants.MAX_AUTHOR_NAME_LENGTH @@ -25,8 +29,11 @@ import org.briarproject.briar.headless.event.WebSocketController import org.briarproject.briar.headless.getContactIdFromPathParam import org.briarproject.briar.headless.getFromJson import org.briarproject.briar.headless.json.JsonDict +import org.eclipse.jetty.http.HttpStatus.BAD_REQUEST_400 +import org.eclipse.jetty.http.HttpStatus.FORBIDDEN_403 import org.spongycastle.util.encoders.Base64 import org.spongycastle.util.encoders.DecoderException +import java.security.GeneralSecurityException import javax.annotation.concurrent.Immutable import javax.inject.Inject import javax.inject.Singleton @@ -91,9 +98,32 @@ constructor( override fun addPendingContact(ctx: Context): Context { val link = ctx.getFromJson(objectMapper, "link") val alias = ctx.getFromJson(objectMapper, "alias") - if (!LINK_REGEX.matcher(link).find()) throw BadRequestResponse("Invalid Link") + if (!LINK_REGEX.matcher(link).find()) { + ctx.status(BAD_REQUEST_400) + val details = mapOf("error" to "INVALID_LINK") + return ctx.json(details) + } checkAliasLength(alias) - val pendingContact = contactManager.addPendingContact(link, alias) + val pendingContact = try { + contactManager.addPendingContact(link, alias) + } catch (e: GeneralSecurityException) { + ctx.status(BAD_REQUEST_400) + val details = mapOf("error" to "INVALID_PUBLIC_KEY") + return ctx.json(details) + } catch (e: ContactExistsException) { + ctx.status(FORBIDDEN_403) + val details = + mapOf("error" to "CONTACT_EXISTS", "remoteAuthorName" to e.remoteAuthor.name) + return ctx.json(details) + } catch (e: PendingContactExistsException) { + ctx.status(FORBIDDEN_403) + val details = mapOf( + "error" to "PENDING_EXISTS", + "pendingContactId" to e.pendingContact.id.bytes, + "pendingContactAlias" to e.pendingContact.alias + ) + return ctx.json(details) + } return ctx.json(pendingContact.output()) } diff --git a/briar-headless/src/test/java/org/briarproject/briar/headless/contact/ContactControllerIntegrationTest.kt b/briar-headless/src/test/java/org/briarproject/briar/headless/contact/ContactControllerIntegrationTest.kt index 2515eba10..ee88dfce1 100644 --- a/briar-headless/src/test/java/org/briarproject/briar/headless/contact/ContactControllerIntegrationTest.kt +++ b/briar-headless/src/test/java/org/briarproject/briar/headless/contact/ContactControllerIntegrationTest.kt @@ -98,6 +98,49 @@ class ContactControllerIntegrationTest: IntegrationTest() { assertEquals(401, response.statusCode) } + @Test + fun `adding a pending contact with invalid link`() { + val alias = "AliasFoo" + val json = """{ + "link": "briar://invalid", + "alias": "$alias" + }""" + val response = post("$url/contacts/add/pending", json) + assertEquals(400, response.statusCode) + assertEquals("INVALID_LINK", response.jsonObject.getString("error")) + } + + @Test + fun `adding a pending contact with invalid public key`() { + val alias = "AliasFoo" + val json = """{ + "link": "briar://aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "alias": "$alias" + }""" + val response = post("$url/contacts/add/pending", json) + assertEquals(400, response.statusCode) + assertEquals("INVALID_PUBLIC_KEY", response.jsonObject.getString("error")) + } + + @Test + fun `adding a pending contact that already exists as pending contact`() { + val alias = "AliasFoo" + val json = """{ + "link": "${getRealHandshakeLink(crypto)}", + "alias": "$alias" + }""" + var response = post("$url/contacts/add/pending", json) + assertEquals(200, response.statusCode) + + val pendingContactId = response.jsonObject.getString("pendingContactId") + + response = post("$url/contacts/add/pending", json) + assertEquals(403, response.statusCode) + assertEquals("PENDING_EXISTS", response.jsonObject.getString("error")) + assertEquals(pendingContactId, response.jsonObject.getString("pendingContactId")) + assertEquals(alias, response.jsonObject.getString("pendingContactAlias")) + } + @Test fun `removing a pending contact needs authentication token`() { val response = deleteWithWrongToken("$url/contacts/add/pending") diff --git a/briar-headless/src/test/java/org/briarproject/briar/headless/contact/ContactControllerTest.kt b/briar-headless/src/test/java/org/briarproject/briar/headless/contact/ContactControllerTest.kt index 7a8476b1e..1f4c75cca 100644 --- a/briar-headless/src/test/java/org/briarproject/briar/headless/contact/ContactControllerTest.kt +++ b/briar-headless/src/test/java/org/briarproject/briar/headless/contact/ContactControllerTest.kt @@ -1,6 +1,8 @@ package org.briarproject.briar.headless.contact import io.javalin.http.BadRequestResponse +import io.javalin.http.ForbiddenResponse +import io.javalin.http.HttpResponseException import io.javalin.http.NotFoundResponse import io.javalin.plugin.json.JavalinJson.toJson import io.mockk.Runs @@ -8,6 +10,7 @@ import io.mockk.every import io.mockk.just import io.mockk.mockkStatic import io.mockk.runs +import io.mockk.verify import org.briarproject.bramble.api.Pair import org.briarproject.bramble.api.contact.Contact import org.briarproject.bramble.api.contact.ContactId @@ -18,8 +21,10 @@ import org.briarproject.bramble.api.contact.event.ContactAddedEvent import org.briarproject.bramble.api.contact.event.PendingContactAddedEvent import org.briarproject.bramble.api.contact.event.PendingContactRemovedEvent import org.briarproject.bramble.api.contact.event.PendingContactStateChangedEvent +import org.briarproject.bramble.api.db.ContactExistsException import org.briarproject.bramble.api.db.NoSuchContactException import org.briarproject.bramble.api.db.NoSuchPendingContactException +import org.briarproject.bramble.api.db.PendingContactExistsException import org.briarproject.bramble.api.identity.AuthorConstants.MAX_AUTHOR_NAME_LENGTH import org.briarproject.bramble.api.plugin.event.ContactConnectedEvent import org.briarproject.bramble.api.plugin.event.ContactDisconnectedEvent @@ -30,9 +35,11 @@ import org.briarproject.bramble.util.StringUtils.getRandomString import org.briarproject.briar.headless.ControllerTest import org.briarproject.briar.headless.getFromJson import org.briarproject.briar.headless.json.JsonDict +import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertNotNull import org.junit.jupiter.api.Assertions.assertThrows import org.junit.jupiter.api.Test +import java.security.GeneralSecurityException import kotlin.random.Random internal class ContactControllerTest : ControllerTest() { @@ -96,9 +103,10 @@ internal class ContactControllerTest : ControllerTest() { "alias": "$alias" }""" every { ctx.body() } returns body - assertThrows(BadRequestResponse::class.java) { - controller.addPendingContact(ctx) - } + every { ctx.status(400) } returns ctx + every { ctx.json(mapOf("error" to "INVALID_LINK")) } returns ctx + controller.addPendingContact(ctx) + verify { ctx.status(400) } } @Test @@ -139,6 +147,84 @@ internal class ContactControllerTest : ControllerTest() { } } + @Test + fun testAddPendingContactPublicKeyInvalid() { + val link = "briar://adnsyffpsenoc3yzlhr24aegfq2pwan7kkselocill2choov6sbhs" + val alias = "Alias123" + val body = """{ + "link": "$link", + "alias": "$alias" + }""" + every { ctx.body() } returns body + every { ctx.status(400) } returns ctx + every { + contactManager.addPendingContact( + link, + alias + ) + } throws GeneralSecurityException() + every { ctx.json(mapOf("error" to "INVALID_PUBLIC_KEY")) } returns ctx + controller.addPendingContact(ctx) + verify { ctx.status(400) } + } + + @Test + fun testAddPendingContactSameContactKey() { + val link = "briar://adnsyffpsenoc3yzlhr24aegfq2pwan7kkselocill2choov6sbhs" + val alias = "Alias123" + val body = """{ + "link": "$link", + "alias": "$alias" + }""" + every { ctx.body() } returns body + every { ctx.status(403) } returns ctx + every { + contactManager.addPendingContact( + link, + alias + ) + } throws ContactExistsException(null, author) + every { + ctx.json( + mapOf( + "error" to "CONTACT_EXISTS", + "remoteAuthorName" to author.name + ) + ) + } returns ctx + controller.addPendingContact(ctx) + verify { ctx.status(403) } + } + + @Test + fun testAddPendingContactSamePendingContactKey() { + val link = "briar://adnsyffpsenoc3yzlhr24aegfq2pwan7kkselocill2choov6sbhs" + val alias = "Alias123" + val body = """{ + "link": "$link", + "alias": "$alias" + }""" + every { ctx.body() } returns body + every { ctx.status(403) } returns ctx + every { + contactManager.addPendingContact( + link, + alias + ) + } throws PendingContactExistsException(pendingContact) + every { + ctx.json( + mapOf( + "error" to "PENDING_EXISTS", + "pendingContactId" to pendingContact.id.bytes, + "pendingContactAlias" to pendingContact.alias + ) + ) + } returns ctx + controller.addPendingContact(ctx) + verify { ctx.status(403) } + } + @Test fun testListPendingContacts() { every { contactManager.pendingContacts } returns listOf(