Merge pull request #2 from linagora/hashes-rate-limit

feat: hashes rate limit for "lookup" endpoints
linagora · Jan 5, 2024 · c8cf3de · c8cf3de
2 parents 488aeb3 + d192040
commit c8cf3de
Show file tree

Hide file tree

Showing 18 changed files with 1,272 additions and 956 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -21,7 +21,7 @@
     "landing"
   ],
   "scripts": {
-    "build": "npm run softClean && lerna run build --skip-nx-cache",
+    "build": "npm run softClean && lerna run build",
     "clean": "rimraf packages/*/dist packages/*/coverage ./node_modules/.cache/nx packages/*/example/*.js*",
     "doc": "node documentation/swagger.cjs",
     "format:check": "prettier --check .",
@@ -33,7 +33,7 @@
     "lint-fix": "eslint --fix packages/*/src --ext .ts",
     "prepare": "husky install",
     "softClean": "rimraf packages/*/dist",
-    "test": "lerna run test --skip-nx-cache"
+    "test": "lerna run test"
   },
   "devDependencies": {
     "@rollup/plugin-commonjs": "^25.0.7",

diff --git a/packages/federation-server/server.mjs b/packages/federation-server/server.mjs
@@ -15,6 +15,7 @@ const conf = {
   database_name: process.env.DATABASE_NAME,
   database_user: process.env.DATABASE_USER,
   database_password: process.env.DATABASE_PASSWORD,
+  hashes_rate_limit: process.env.HASHES_RATE_LIMIT,
   is_federation_server: true,
   ldap_base: process.env.LDAP_BASE,
   ldap_filter: process.env.LDAP_FILTER,

diff --git a/packages/federation-server/src/config.json b/packages/federation-server/src/config.json
@@ -7,8 +7,10 @@
   "database_host": "./tokens.db",
   "database_name": "",
   "database_password": "",
+  "database_ssl": false,
   "database_user": "",
   "database_vacuum_delay": 3600,
+  "hashes_rate_limit": 100,
   "is_federation_server": true,
   "key_delay": 3600,
   "keys_depth": 5,
@@ -23,6 +25,7 @@
   "matrix_database_host": null,
   "matrix_database_name": null,
   "matrix_database_password": null,
+  "matrix_database_ssl": false,
   "matrix_database_user": null,
   "pepperCron": "0 0 * * *",
   "policies": null,
@@ -40,6 +43,7 @@
   "userdb_host": "./tokens.db",
   "userdb_name": "",
   "userdb_password": "",
+  "userdb_ssl": false,
   "userdb_user": "",
   "template_dir": "./templates",
   "trust_x_forwarded_for": false,

diff --git a/packages/federation-server/src/index.test.ts b/packages/federation-server/src/index.test.ts
@@ -1875,7 +1875,7 @@ describe('Federation server', () => {
           )
         })
 
-        it('should send an error if "addresses" is not a string array', async () => {
+        it('should send an error if "addresses" is not an array', async () => {
           const response = await request(app)
             .post('/_matrix/identity/v2/lookup')
             .set('Accept', 'application/json')
@@ -1901,12 +1901,7 @@ describe('Federation server', () => {
             .set('Accept', 'application/json')
             .set('Authorization', `Bearer ${authToken}`)
             .send({
-              addresses: [
-                '@dwho:example.com',
-                '@rtyler:example.com',
-                2,
-                '@msmith:example.com'
-              ],
+              addresses: ['hash1', 'hash2', 2, 'hash3'],
               algorithm: 'sha256',
               pepper: 'test_pepper'
             })
@@ -1921,6 +1916,27 @@ describe('Federation server', () => {
           )
         })
 
+        it('should send an error if exceeds hashes limit', async () => {
+          const response = await request(app)
+            .post('/_matrix/identity/v2/lookup')
+            .set('Accept', 'application/json')
+            .set('Authorization', `Bearer ${authToken}`)
+            .send({
+              addresses: Array.from({ length: 101 }, (_, i) => `hash${i}`),
+              algorithm: 'sha256',
+              pepper: 'test_pepper'
+            })
+
+          expect(response.statusCode).toEqual(400)
+          expect(JSON.stringify(response.body)).toEqual(
+            JSON.stringify({
+              errcode: 'M_INVALID_PARAM',
+              error:
+                'Error field: Adresses limit of 100 exceeded (property: addresses)'
+            })
+          )
+        })
+
         it('should send an error if getting hashes from db fails', async () => {
           jest
             .spyOn(federationServer.db, 'get')
@@ -1934,7 +1950,7 @@ describe('Federation server', () => {
             .set('Accept', 'application/json')
             .set('Authorization', `Bearer ${authToken}`)
             .send({
-              addresses: ['@dwho:example.com', '@rtyler:example.com'],
+              addresses: ['hash1', 'hash2'],
               algorithm: 'sha256',
               pepper: 'test_pepper'
             })
@@ -1961,7 +1977,7 @@ describe('Federation server', () => {
             .set('Accept', 'application/json')
             .set('Authorization', `Bearer ${authToken}`)
             .send({
-              addresses: ['@dwho:example.com', '@rtyler:example.com'],
+              addresses: ['hash1', 'hash2'],
               algorithm: 'sha256',
               pepper: 'test_pepper'
             })
@@ -1990,7 +2006,7 @@ describe('Federation server', () => {
             .set('Accept', 'application/json')
             .set('Authorization', `Bearer ${authToken}`)
             .send({
-              addresses: ['@dwho:example.com', '@rtyler:example.com'],
+              addresses: ['hash1', 'hash2'],
               algorithm: 'sha256',
               pepper: 'test_pepper'
             })
@@ -2161,6 +2177,48 @@ describe('Federation server', () => {
           )
         })
 
+        it('should send an error if "mappings" values is not an array', async () => {
+          const response = await request(app)
+            .post('/_matrix/identity/v2/lookups')
+            .set('Accept', 'application/json')
+            .set('Authorization', `Bearer ${authToken}`)
+            .send({
+              mappings: { 'test.example.com': 2 },
+              algorithm: 'sha256',
+              pepper: 'test_pepper'
+            })
+
+          expect(response.statusCode).toEqual(400)
+          expect(JSON.stringify(response.body)).toEqual(
+            JSON.stringify({
+              errcode: 'M_INVALID_PARAM',
+              error:
+                'Error field: Mappings object values are not string arrays (property: mappings)'
+            })
+          )
+        })
+
+        it('should send an error if one address is not a string', async () => {
+          const response = await request(app)
+            .post('/_matrix/identity/v2/lookups')
+            .set('Accept', 'application/json')
+            .set('Authorization', `Bearer ${authToken}`)
+            .send({
+              mappings: { 'test.example.com': ['hash1', 'hash2', 2, 'hash3'] },
+              algorithm: 'sha256',
+              pepper: 'test_pepper'
+            })
+
+          expect(response.statusCode).toEqual(400)
+          expect(JSON.stringify(response.body)).toEqual(
+            JSON.stringify({
+              errcode: 'M_INVALID_PARAM',
+              error:
+                'Error field: Mappings object values are not string arrays (property: mappings)'
+            })
+          )
+        })
+
         it('should send an error if getting pepper in keys table fails', async () => {
           jest
             .spyOn(federationServer.db, 'get')

diff --git a/packages/federation-server/src/middlewares/validation.ts b/packages/federation-server/src/middlewares/validation.ts
@@ -1,19 +1,22 @@
-import { body } from 'express-validator'
+import { body, type ValidationChain } from 'express-validator'
 
 export const commonValidators = [
   body('algorithm').exists().isString(),
   body('pepper').exists().isString()
 ]
 
-export const lookupValidator = body('addresses')
-  .exists()
-  .isArray()
-  .custom((value, { req }) => {
-    if ((value as any[]).some((address) => typeof address !== 'string')) {
-      throw new Error('One of the address is not a string')
-    }
-    return true
-  })
+export const lookupValidator = (hashesLimit: number): ValidationChain =>
+  body('addresses')
+    .exists()
+    .isArray()
+    .custom((value, { req }) => {
+      if ((value as any[]).some((address) => typeof address !== 'string')) {
+        throw new Error('One of the address is not a string')
+      } else if ((value as string[]).length > hashesLimit) {
+        throw new Error(`Adresses limit of ${hashesLimit} exceeded`)
+      }
+      return true
+    })
 
 export const lookupsValidator = [
   body('pepper').exists().isString(),
@@ -24,6 +27,17 @@ export const lookupsValidator = [
       if (Object.keys(value).length > 1) {
         throw new Error('Only one server address is allowed')
       }
+      if (
+        Object.keys(value).length === 1 &&
+        !(
+          Array.isArray(Object.values(value)[0]) &&
+          (Object.values(value)[0] as any[]).every(
+            (hash) => typeof hash === 'string'
+          )
+        )
+      ) {
+        throw new Error('Mappings object values are not string arrays')
+      }
       return true
     })
 ]
diff --git a/packages/federation-server/src/routes/routes.ts b/packages/federation-server/src/routes/routes.ts
@@ -139,7 +139,7 @@ export default (
         logger
       ),
       ...commonValidators,
-      lookupValidator,
+      lookupValidator(conf.hashes_rate_limit as number),
       lookup(conf, db),
       errorMiddleware
     )

diff --git a/packages/matrix-identity-server/src/config.json b/packages/matrix-identity-server/src/config.json
@@ -11,6 +11,7 @@
   "database_user": "",
   "database_vacuum_delay": 3600,
   "federation_servers": null,
+  "hashes_rate_limit": 100,
   "is_federation_server": false,
   "key_delay": 3600,
   "keys_depth": 5,

diff --git a/packages/matrix-identity-server/src/db/index.ts b/packages/matrix-identity-server/src/db/index.ts
@@ -41,7 +41,8 @@ type Update = (
 type Get = (
   table: Collections,
   fields: string[],
-  filterFields: Record<string, string | number | Array<string | number>>
+  filterFields: Record<string, string | number | Array<string | number>>,
+  order?: string
 ) => Promise<DbGetResult>
 type GetCount = (
   table: Collections,
@@ -202,9 +203,10 @@ class IdentityServerDb implements IdDbBackend {
   getHigherThan(
     table: Collections,
     fields: string[],
-    filterFields: Record<string, string | number | Array<string | number>>
+    filterFields: Record<string, string | number | Array<string | number>>,
+    order?: string
   ): Promise<DbGetResult> {
-    return this.db.getHigherThan(table, fields, filterFields)
+    return this.db.getHigherThan(table, fields, filterFields, order)
   }
 
   // eslint-disable-next-line @typescript-eslint/explicit-function-return-type, @typescript-eslint/promise-function-async