Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Message validation for the rest of Federation messages #1959

Merged
merged 3 commits into from
Jun 28, 2024
+100 −12
Merged

Message validation for the rest of Federation messages #1959

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e28b324
Added message validation on Federation messages
quadcopterman Jun 27, 2024
6c8076d
Corrected a test
quadcopterman Jun 27, 2024
dff2ff6
Improved the constructor conditions
quadcopterman Jun 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ...ava/com/github/dedis/popstellar/model/network/method/message/data/federation/Challenge.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.github.dedis.popstellar.model.network.method.message.data.federation
import com.github.dedis.popstellar.model.network.method.message.data.Action
import com.github.dedis.popstellar.model.network.method.message.data.Data
import com.github.dedis.popstellar.model.network.method.message.data.Objects
import com.github.dedis.popstellar.utility.MessageValidator
import com.google.gson.annotations.SerializedName
import kotlin.math.max

Expand All @@ -18,6 +19,7 @@ class Challenge
@SerializedName("valid_until") val validUntil: Long

init {
MessageValidator.verify().stringNotEmpty(value, "challenge").validFutureTimes(validUntil)
this.validUntil = max(0L, validUntil)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validation should ideally be before instantiating attributes, since if the input is wrong and it is meant to crash, no need to fill the attribute beforehand.

}

Expand Down
7 changes: 7 additions & 0 deletions .../github/dedis/popstellar/model/network/method/message/data/federation/ChallengeRequest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import com.github.dedis.popstellar.model.Immutable
import com.github.dedis.popstellar.model.network.method.message.data.Action
import com.github.dedis.popstellar.model.network.method.message.data.Data
import com.github.dedis.popstellar.model.network.method.message.data.Objects
import com.github.dedis.popstellar.utility.MessageValidator

/** Data sent to get a challenge */
@Immutable
Expand All @@ -15,6 +16,12 @@ class ChallengeRequest
*/
(val timestamp: Long) : Data {

init {
MessageValidator.verify()
.validPastTimes(timestamp)
.greaterOrEqualThan(timestamp, 0, "timestamp")
}

override val `object`: String
get() = Objects.FEDERATION.`object`

Expand Down
9 changes: 9 additions & 0 deletions .../github/dedis/popstellar/model/network/method/message/data/federation/FederationExpect.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import com.github.dedis.popstellar.model.network.method.message.MessageGeneral
import com.github.dedis.popstellar.model.network.method.message.data.Action
import com.github.dedis.popstellar.model.network.method.message.data.Data
import com.github.dedis.popstellar.model.network.method.message.data.Objects
import com.github.dedis.popstellar.utility.MessageValidator
import com.google.gson.annotations.SerializedName

/** Federation Expect message */
Expand All @@ -23,6 +24,14 @@ class FederationExpect
val challenge: MessageGeneral
) : Data {

init {
MessageValidator.verify()
.isNotEmptyBase64(laoId, "lao_id")
.stringNotEmpty(serverAddress, "server_address")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering if it would be a good idea to create a function in MessageValidator to check for server addresses pattern. It is specified in the protocol :

"server_address": {
            "type": "string",
            "pattern": "^(ws|wss):\/\/.*(:\\d{0,5})?\/.*$",
            "$comment": "public address of the remote organizer server"
        },

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For that point, the pattern might change as (if I remember well) some addresses didn't fit in the pattern. It will be added once the protocol is updated.

.isNotEmptyBase64(publicKey, "public_key")
.validMessage(challenge)
}

override val `object`: String
get() = Objects.FEDERATION.`object`

Expand Down
9 changes: 9 additions & 0 deletions ...om/github/dedis/popstellar/model/network/method/message/data/federation/FederationInit.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import com.github.dedis.popstellar.model.network.method.message.MessageGeneral
import com.github.dedis.popstellar.model.network.method.message.data.Action
import com.github.dedis.popstellar.model.network.method.message.data.Data
import com.github.dedis.popstellar.model.network.method.message.data.Objects
import com.github.dedis.popstellar.utility.MessageValidator
import com.google.gson.annotations.SerializedName

/** Initiates a federation link */
Expand All @@ -23,6 +24,14 @@ class FederationInit
val challenge: MessageGeneral
) : Data {

init {
MessageValidator.verify()
.isNotEmptyBase64(laoId, "lao_id")
.stringNotEmpty(serverAddress, "server_address")
.isNotEmptyBase64(publicKey, "public_key")
.validMessage(challenge)
}

override val `object`: String
get() = Objects.FEDERATION.`object`

Expand Down
16 changes: 16 additions & 0 deletions fe2-android/app/src/main/java/com/github/dedis/popstellar/utility/MessageValidator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,22 @@ object MessageValidator {
return this
}

/**
* Helper method to check that times are about now or in the future. The time values provided
* are assumed to be in Unix epoch time (UTC).
*
* @param times time values to be checked
* @throws IllegalArgumentException if times are in the past
*/
fun validFutureTimes(vararg times: Long): MessageValidatorBuilder {
val currentTime = Instant.now().epochSecond - VALID_FUTURE_DELAY

for (time in times) {
require(time >= currentTime) { "Time cannot be in the past" }
}
return this
}

/**
* Helper method to check that times in a Data are ordered. The time values provided are assumed
* to be in Unix epoch time (UTC).
Expand Down
2 changes: 1 addition & 1 deletion ...ctric/com/github/dedis/popstellar/ui/lao/federation/LinkedOrganizationsScannerJoinTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ class LinkedOrganizationsScannerJoinTest {
private val laoSubject = BehaviorSubject.createDefault(LaoView(LAO))
private val CREATION_TIME = Instant.now().epochSecond
private const val CHALLENGE_VALUE =
"frXgNl-IxJPzsNia07f_3yV0ECYlWOb2RXG_SGvATKcJ7-s0LthmboTrnMqlQS1RnzmV9hW0iumu_5NwAqXwGA"
"ZnJYZ05sLUl4SlB6c05pYTA3Zl8zeVYwRUNZbFdPYjJSWEdfU0d2QVRLY0o3LXMwTHRobWJvVHJuTXFsUVMxUm56bVY5aFcwaXVtdV81TndBcVh3R0E="
private val CHALLENGE = Challenge(CHALLENGE_VALUE, CREATION_TIME)
}
}
10 changes: 10 additions & 0 deletions ...hub/dedis/popstellar/model/network/method/message/data/federation/ChallengeRequestTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,16 @@ class ChallengeRequestTest {
)
}

@Test
fun invalidChallengeRequestTest() {
Assert.assertThrows(IllegalArgumentException::class.java) {
ChallengeRequest(TIMESTAMP + 2000)
}
Assert.assertThrows(IllegalArgumentException::class.java) {
ChallengeRequest(-1)
}
}

companion object {
private val TIMESTAMP = Instant.now().epochSecond
private val CHALLENGE_REQUEST = ChallengeRequest(TIMESTAMP)
Expand Down
17 changes: 13 additions & 4 deletions ...com/github/dedis/popstellar/model/network/method/message/data/federation/ChallengeTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ class ChallengeTest {

@Test
fun challengeLowValidUntilTest() {
Assert.assertEquals(0L, Challenge(CHALLENGE_VALUE, -1).validUntil)
Assert.assertThrows(IllegalArgumentException::class.java) {
Challenge(CHALLENGE_VALUE, -1).validUntil
}
}

@Test
Expand All @@ -40,8 +42,8 @@ class ChallengeTest {
Assert.assertEquals(CHALLENGE.hashCode().toLong(), challenge2.hashCode().toLong())

Assert.assertNotEquals(CHALLENGE, Challenge(CHALLENGE_VALUE, TIMESTAMP + 1))
Assert.assertNotEquals(CHALLENGE, Challenge("aaa", TIMESTAMP))
Assert.assertNotEquals(CHALLENGE, Challenge("bbb", TIMESTAMP - 1))
Assert.assertNotEquals(CHALLENGE, Challenge("YWFh", TIMESTAMP))
Assert.assertNotEquals(CHALLENGE, Challenge("YmJi", TIMESTAMP - 1))
Assert.assertNotEquals(CHALLENGE, null)
}

Expand All @@ -53,8 +55,15 @@ class ChallengeTest {
)
}

@Test
fun testInvalidChallenge() {
Assert.assertThrows(IllegalArgumentException::class.java) {
Challenge("", TIMESTAMP)
}
}

companion object {
private val TIMESTAMP = Instant.now().epochSecond
private val TIMESTAMP = Instant.now().epochSecond + 2000
private const val CHALLENGE_VALUE = "1feb2a2c7c739ea25f2568d056cc82d11be65d361511872cd35e4abd1a20f3d4"
private val CHALLENGE = Challenge(CHALLENGE_VALUE, TIMESTAMP)
}
Expand Down
17 changes: 15 additions & 2 deletions ...hub/dedis/popstellar/model/network/method/message/data/federation/FederationExpectTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ class FederationExpectTest {
Assert.assertEquals(EXPECT, EXPECT)
Assert.assertEquals(EXPECT.hashCode().toLong(), expect2.hashCode().toLong())

val challenge2 = Challenge("aaa", TIMESTAMP)
val challenge2 = Challenge("YmJi", TIMESTAMP)
val mg = MessageGeneral(Base64DataUtils.generateKeyPair(), challenge2, Gson())
Assert.assertNotEquals(EXPECT, FederationExpect(LAO_ID, SERVER_ADDRESS, ORGANIZER.encoded, mg))
Assert.assertNotEquals(EXPECT, FederationExpect("bbb", SERVER_ADDRESS, ORGANIZER.encoded, MG_CHALLENGE))
Assert.assertNotEquals(EXPECT, FederationExpect("YWFh", SERVER_ADDRESS, ORGANIZER.encoded, MG_CHALLENGE))
Assert.assertNotEquals(EXPECT, null)
}

Expand All @@ -64,6 +64,19 @@ class FederationExpectTest {
)
}

@Test
fun invalidExpectTest() {
Assert.assertThrows(IllegalArgumentException::class.java) {
FederationExpect("aaa", SERVER_ADDRESS, ORGANIZER.encoded, MG_CHALLENGE)
}
Assert.assertThrows(IllegalArgumentException::class.java) {
FederationExpect(LAO_ID, "", ORGANIZER.encoded, MG_CHALLENGE)
}
Assert.assertThrows(IllegalArgumentException::class.java) {
FederationExpect(LAO_ID, SERVER_ADDRESS, "aaa", MG_CHALLENGE)
}
}

companion object {
private val ORGANIZER = Base64DataUtils.generatePublicKey()
private val CREATION = Instant.now().epochSecond
Expand Down
17 changes: 15 additions & 2 deletions ...ithub/dedis/popstellar/model/network/method/message/data/federation/FederationInitTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ class FederationInitTest {
Assert.assertEquals(INIT, INIT)
Assert.assertEquals(INIT.hashCode().toLong(), init2.hashCode().toLong())

val challenge2 = Challenge("aaa", TIMESTAMP)
val challenge2 = Challenge("YmJi", TIMESTAMP)
val mg = MessageGeneral(Base64DataUtils.generateKeyPair(), challenge2, Gson())
Assert.assertNotEquals(INIT, FederationInit(LAO_ID, SERVER_ADDRESS, ORGANIZER.encoded, mg))
Assert.assertNotEquals(INIT, FederationInit("bbb", SERVER_ADDRESS, ORGANIZER.encoded, MG_CHALLENGE))
Assert.assertNotEquals(INIT, FederationInit("YWFh", SERVER_ADDRESS, ORGANIZER.encoded, MG_CHALLENGE))
Assert.assertNotEquals(INIT, null)
}

Expand All @@ -64,6 +64,19 @@ class FederationInitTest {
)
}

@Test
fun invalidInitTest() {
Assert.assertThrows(IllegalArgumentException::class.java) {
FederationInit("aaa", SERVER_ADDRESS, ORGANIZER.encoded, MG_CHALLENGE)
}
Assert.assertThrows(IllegalArgumentException::class.java) {
FederationInit(LAO_ID, "", ORGANIZER.encoded, MG_CHALLENGE)
}
Assert.assertThrows(IllegalArgumentException::class.java) {
FederationInit(LAO_ID, SERVER_ADDRESS, "aaa", MG_CHALLENGE)
}
}

companion object {
private val ORGANIZER = Base64DataUtils.generatePublicKey()
private val CREATION = Instant.now().epochSecond
Expand Down
2 changes: 1 addition & 1 deletion .../app/src/test/unit/java/com/github/dedis/popstellar/model/qrcode/FederationDetailsTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class FederationDetailsTest {
private val LAO_ID = Lao.generateLaoId(SENDER, CREATION_TIME, LAO_NAME)
private const val ADDRESS = "localhost:9100"
private const val CHALLENGE_VALUE =
"frXgNl-IxJPzsNia07f_3yV0ECYlWOb2RXG_SGvATKcJ7-s0LthmboTrnMqlQS1RnzmV9hW0iumu_5NwAqXwGA"
"RGlkIHlvdSBrbm93IHRoYXQgdGhpcyBpcyBhbiBhd3NvbWUgY2hhbGxlbmdlIHZhbHVlID8="
private val CHALLENGE = Challenge(CHALLENGE_VALUE, CREATION_TIME)

private val FEDERATION_DETAILS1 = FederationDetails(LAO_ID, ADDRESS, SENDER_KEY.publicKey.encoded)
Expand Down
4 changes: 2 additions & 2 deletions ...est/unit/java/com/github/dedis/popstellar/repository/LinkedOrganizationsRepositoryTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class LinkedOrganizationsRepositoryTest {

@Test
fun repoCallbackTest() {
var c = Challenge("test", 0L)
var c = Challenge("dGVzdA==", TIMESTAMP + 200)
var changed = false
REPO.flush()
REPO.setOnChallengeUpdatedCallback { challenge: Challenge ->
Expand Down Expand Up @@ -175,7 +175,7 @@ class LinkedOrganizationsRepositoryTest {
private val LAO_ID_7 = "aWQ3"
private val TIMESTAMP = Instant.now().epochSecond
private const val SERVER_ADDRESS = "wss://1.1.1.1:9000/client"
private const val CHALLENGE_VALUE = "1feb2a2c7c739ea25f2568d056cc82d11be65d361511872cd35e4abd1a20f3d4"
private const val CHALLENGE_VALUE = "Y2hhbGxlbmdldmFsdWU="
private val TOKENS_ARRAY_1 = arrayOf("dG9rZW4x", "dG9rZW4y")
private val TOKENS_ARRAY_2 = arrayOf("dG9rZW43", "dG9rZW44", "dG9rZW45")
private val CHALLENGE = Challenge(CHALLENGE_VALUE, TIMESTAMP)
Expand Down
Loading