Merge pull request #1959 from dedis/work-fe2-johan-message-validation

Message validation for the rest of Federation messages
dedis · Jun 28, 2024 · 69b0e0a · 69b0e0a
2 parents 444a91c + dff2ff6
commit 69b0e0a
Show file tree

Hide file tree

Showing 12 changed files with 100 additions and 12 deletions.
diff --git a/...ava/com/github/dedis/popstellar/model/network/method/message/data/federation/Challenge.kt b/...ava/com/github/dedis/popstellar/model/network/method/message/data/federation/Challenge.kt
@@ -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
 
@@ -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)
   }
 

diff --git a/.../github/dedis/popstellar/model/network/method/message/data/federation/ChallengeRequest.kt b/.../github/dedis/popstellar/model/network/method/message/data/federation/ChallengeRequest.kt
@@ -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
@@ -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`
 

diff --git a/.../github/dedis/popstellar/model/network/method/message/data/federation/FederationExpect.kt b/.../github/dedis/popstellar/model/network/method/message/data/federation/FederationExpect.kt
@@ -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 */
@@ -23,6 +24,14 @@ class FederationExpect
     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`
 

diff --git a/...om/github/dedis/popstellar/model/network/method/message/data/federation/FederationInit.kt b/...om/github/dedis/popstellar/model/network/method/message/data/federation/FederationInit.kt
@@ -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 */
@@ -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`
 

diff --git a/fe2-android/app/src/main/java/com/github/dedis/popstellar/utility/MessageValidator.kt b/fe2-android/app/src/main/java/com/github/dedis/popstellar/utility/MessageValidator.kt
@@ -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).

diff --git a/...ctric/com/github/dedis/popstellar/ui/lao/federation/LinkedOrganizationsScannerJoinTest.kt b/...ctric/com/github/dedis/popstellar/ui/lao/federation/LinkedOrganizationsScannerJoinTest.kt
@@ -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)
     }
 }
diff --git a/...hub/dedis/popstellar/model/network/method/message/data/federation/ChallengeRequestTest.kt b/...hub/dedis/popstellar/model/network/method/message/data/federation/ChallengeRequestTest.kt
@@ -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)

diff --git a/...com/github/dedis/popstellar/model/network/method/message/data/federation/ChallengeTest.kt b/...com/github/dedis/popstellar/model/network/method/message/data/federation/ChallengeTest.kt
@@ -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
@@ -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)
     }
 
@@ -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)
     }

diff --git a/...hub/dedis/popstellar/model/network/method/message/data/federation/FederationExpectTest.kt b/...hub/dedis/popstellar/model/network/method/message/data/federation/FederationExpectTest.kt
@@ -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)
     }
 
@@ -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

diff --git a/...ithub/dedis/popstellar/model/network/method/message/data/federation/FederationInitTest.kt b/...ithub/dedis/popstellar/model/network/method/message/data/federation/FederationInitTest.kt
@@ -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)
     }
 
@@ -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

diff --git a/.../app/src/test/unit/java/com/github/dedis/popstellar/model/qrcode/FederationDetailsTest.kt b/.../app/src/test/unit/java/com/github/dedis/popstellar/model/qrcode/FederationDetailsTest.kt
@@ -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)

diff --git a/...est/unit/java/com/github/dedis/popstellar/repository/LinkedOrganizationsRepositoryTest.kt b/...est/unit/java/com/github/dedis/popstellar/repository/LinkedOrganizationsRepositoryTest.kt
@@ -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 ->
@@ -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)