Merge pull request #26 from linagora/fix-code-scanning-errors

Fix code scanning errors

DOCKER-VARIABLES.md

@@ -34,3 +34,5 @@
  * `OIDC_ISSUER`: Lemon URL. Example: `https://auth.company.com`
  * `SERVER_NAME`: Matrix "server name" _(ie domain)_. Example: `company.com`
  * `TEMPLATE_DIR`: Local path to templates dir (mail template).
+ * `RATE_LIMITING_WINDOW`: How long to remember requests for, in milliseconds.
+ * `RATE_LIMITING_NB_REQUESTS`: How many requests to allow.

Dockerfile

@@ -36,7 +36,9 @@ env BASE_URL= \
     UPDATE_USERS_CRON="*/15 * * * *" \
     SMS_API_LOGIN= \
     SMS_API_URL= \
-    SMS_API_KEY=
+    SMS_API_KEY= \
+    RATE_LIMITING_WINDOW= \
+    RATE_LIMITING_NB_REQUESTS=
 
 RUN apt update && apt -y dist-upgrade
 

docker.md

@@ -33,4 +33,6 @@ $ docker run -d -p 3000:3000 \
     -e MATRIX_DATABASE_SSL=true \
     -e OIDC_ISSUER=https://auth.example.com/ \
     -e SERVER_NAME=example.com \
+    -e RATE_LIMITING_WINDOW=10
+    -e RATE_LIMITING_NB_REQUESTS=100
     linagora/tom-server

packages/federation-server/src/__testData__/config.json

@@ -17,6 +17,7 @@
   "matrix_database_host": "./src/__testData__/matrix.db",
   "pepperCron": "*/15 * * * * *",
   "policies": null,
+  "rate_limiting_window": 10000,
   "redis_uri": "",
   "server_name": "example.com",
   "smtp_password": "",

packages/federation-server/src/config.json

@@ -29,6 +29,8 @@
   "matrix_database_user": null,
   "pepperCron": "0 0 * * *",
   "policies": null,
+  "rate_limiting_window": 600000,
+  "rate_limiting_nb_requests": 100,
   "redis_uri": "",
   "server_name": "localhost",
   "smtp_password": "",

packages/federation-server/src/index.test.ts

@@ -8,7 +8,7 @@ import { execFileSync } from 'node:child_process'
 import { createServer } from 'node:https'
 import os from 'os'
 import path from 'path'
-import request from 'supertest'
+import request, { type Response } from 'supertest'
 import {
   DockerComposeEnvironment,
   Wait,
@@ -1711,6 +1711,26 @@ describe('Federation server', () => {
           )
         })
 
+        it('should reject if more than 100 requests are done in less than 10 seconds', async () => {
+          let response
+          let token
+          // eslint-disable-next-line @typescript-eslint/no-for-in-array, @typescript-eslint/no-unused-vars
+          for (const i in [...Array(101).keys()]) {
+            token = Number(i) % 2 === 0 ? `Bearer ${authToken}` : 'falsy_token'
+            response = await request(app)
+              .post('/_matrix/identity/v2/lookup')
+              .set('Accept', 'application/json')
+              .set('Authorization', token)
+              .send({
+                addresse: [],
+                algorithm: 'sha256',
+                pepper: 'test_pepper'
+              })
+          }
+          expect((response as Response).statusCode).toEqual(429)
+          await new Promise((resolve) => setTimeout(resolve, 11000))
+        })
+
         it('should send an error if auth token is invalid', async () => {
           const response = await request(app)
             .post('/_matrix/identity/v2/lookup')
@@ -2038,6 +2058,26 @@ describe('Federation server', () => {
           )
         })
 
+        it('should reject if more than 100 requests are done in less than 10 seconds', async () => {
+          let response
+          let ipAddress
+          // eslint-disable-next-line @typescript-eslint/no-for-in-array, @typescript-eslint/no-unused-vars
+          for (const i in [...Array(101).keys()]) {
+            ipAddress = Number(i) % 2 === 0 ? trustedIpAddress : '192.168.1.25'
+            response = await request(app)
+              .post('/_matrix/identity/v2/lookups')
+              .set('Accept', 'application/json')
+              .set('X-forwarded-for', ipAddress)
+              .send({
+                mappings: {},
+                algorithm: 'sha256',
+                pepper: 'test_pepper'
+              })
+          }
+          expect((response as Response).statusCode).toEqual(429)
+          await new Promise((resolve) => setTimeout(resolve, 11000))
+        })
+
         it('should send an error if requester ip does not belong to trusted ip addresses', async () => {
           const response = await request(app)
             .post('/_matrix/identity/v2/lookups')
@@ -2314,6 +2354,21 @@ describe('Federation server', () => {
           )
         })
 
+        it('should reject if more than 100 requests are done in less than 10 seconds', async () => {
+          let response
+          let token
+          // eslint-disable-next-line @typescript-eslint/no-for-in-array, @typescript-eslint/no-unused-vars
+          for (const i in [...Array(101).keys()]) {
+            token = Number(i) % 2 === 0 ? `Bearer ${authToken}` : 'falsy_token'
+            response = await request(app)
+              .get('/_matrix/identity/v2/hash_details')
+              .set('Accept', 'application/json')
+              .set('Authorization', token)
+          }
+          expect((response as Response).statusCode).toEqual(429)
+          await new Promise((resolve) => setTimeout(resolve, 11000))
+        })
+
         it('should send an error if deleting hashes in hashbyserver table fails', async () => {
           jest
             .spyOn(federationServer.db, 'get')

packages/federation-server/src/index.ts

@@ -2,6 +2,7 @@ import configParser, { type ConfigDescription } from '@twake/config-parser'
 import { type TwakeLogger } from '@twake/logger'
 import MatrixIdentityServer from '@twake/matrix-identity-server'
 import { Router } from 'express'
+import rateLimit from 'express-rate-limit'
 import fs from 'fs'
 import defaultConfig from './config.json'
 import initializeDb from './db'
@@ -48,7 +49,19 @@ export default class FederationServer extends MatrixIdentityServer {
     this.logger.debug(
       `Trusted servers: ${this.conf.trusted_servers_addresses.join(', ')}`
     )
-    this.authenticate = Authenticate(this.db)
+    this.rateLimiter = rateLimit({
+      windowMs: this.conf.rate_limiting_window,
+      limit: this.conf.rate_limiting_nb_requests,
+      validate: {
+        trustProxy: this.conf.trust_x_forwarded_for
+      }
+    })
+    this.authenticate = Authenticate(
+      this.db,
+      this.conf.trusted_servers_addresses,
+      this.conf.trust_x_forwarded_for,
+      this.logger
+    )
     const superReady = this.ready
     this.ready = new Promise((resolve, reject) => {
       superReady

packages/federation-server/src/middlewares/auth.ts

@@ -4,93 +4,73 @@ import {
   Utils,
   type tokenContent
 } from '@twake/matrix-identity-server'
-import { type NextFunction, type RequestHandler, type Response } from 'express'
+import { type NextFunction, type Response } from 'express'
 import { type AuthRequest, type IdentityServerDb } from '../types'
 import { convertToIPv6 } from '../utils/ip-address'
-import { FederationServerError } from './errors'
 
-export const Authenticate = (
-  db: IdentityServerDb
-): Utils.AuthenticationFunction => {
-  const tokenRe = /^Bearer (\S+)$/
-  return (req, res, callback) => {
-    let token: string | null = null
-    if (req.headers.authorization != null) {
-      const re = req.headers.authorization.match(tokenRe)
-      if (re != null) {
-        token = re[1]
-      }
-      // @ts-expect-error req.query exists
-    } else if (req.query != null) {
-      // @ts-expect-error req.query.access_token may be null
-      token = req.query.access_token
-    }
-    if (token != null) {
-      db.get('accessTokens', ['data'], { id: token })
-        .then((rows) => {
-          callback(JSON.parse(rows[0].data as string), token)
-        })
-        .catch((e) => {
-          Utils.send(res, 401, MatrixErrors.errMsg('unAuthorized'))
-        })
-    } else {
-      throw new FederationServerError({
-        status: 401,
-        code: MatrixErrors.errCodes.unAuthorized
-      })
-    }
-  }
-}
+const tokenTrustedServer = 'TOKEN_TRUSTED_SERVER'
 
-export const auth = (
-  authenticator: Utils.AuthenticationFunction,
+export const Authenticate = (
+  db: IdentityServerDb,
   trustedServersList: string[],
   trustXForwardedForHeader: boolean,
   logger: TwakeLogger
-): RequestHandler => {
+): Utils.AuthenticationFunction => {
   const trustedServersListAsIPv6 = trustedServersList.map((ip) =>
     convertToIPv6(ip)
   )
-  return (req: AuthRequest, res: Response, next: NextFunction): void => {
+  const tokenRe = /^Bearer (\S+)$/
+  return (req, res, callbackMethod) => {
+    const request = req as AuthRequest
     const originalRequesterIPAddress = trustXForwardedForHeader
       ? // eslint-disable-next-line @typescript-eslint/strict-boolean-expressions
-        (req.headers['x-forwarded-for'] as string) ||
-        (req.socket.remoteAddress as string)
-      : (req.socket.remoteAddress as string)
+        (request.headers['x-forwarded-for'] as string) ||
+        (request.socket.remoteAddress as string)
+      : (request.socket.remoteAddress as string)
     logger.info('', {
       ip: originalRequesterIPAddress,
-      httpMethod: req.method,
-      endpointPath: req.originalUrl
+      httpMethod: request.method,
+      endpointPath: request.originalUrl
     })
     try {
       const requesterIPAddress = convertToIPv6(originalRequesterIPAddress)
       logger.debug(`Authenticated request from ${originalRequesterIPAddress}`)
       // istanbul ignore if
       if (trustedServersListAsIPv6.includes(requesterIPAddress)) {
         logger.debug('IP is in list')
-        next()
+        callbackMethod({ sub: '', epoch: 0 }, tokenTrustedServer)
       } else if (
         trustedServersListAsIPv6.some((ip) => {
           const res = requesterIPAddress.isInSubnet(ip)
           if (res) logger.debug(`IP is in ${ip.address}`)
           return res
         })
       ) {
-        next()
+        callbackMethod({ sub: '', epoch: 0 }, tokenTrustedServer)
       } else {
         logger.debug(`${originalRequesterIPAddress} isn't in white list`)
-        authenticator(req, res, (data: tokenContent, token: string | null) => {
-          /* istanbul ignore if */
-          if (data.sub === undefined) {
-            throw new Error('Invalid data')
-          }
-
-          req.userId = data.sub
-          if (token != null) {
-            req.accessToken = token
+        let token: string | null = null
+        if (req.headers.authorization != null) {
+          const re = req.headers.authorization.match(tokenRe)
+          if (re != null) {
+            token = re[1]
           }
-          next()
-        })
+          // @ts-expect-error req.query exists
+        } else if (req.query != null) {
+          // @ts-expect-error req.query.access_token may be null
+          token = req.query.access_token
+        }
+        if (token != null) {
+          db.get('accessTokens', ['data'], { id: token })
+            .then((rows) => {
+              callbackMethod(JSON.parse(rows[0].data as string), token)
+            })
+            .catch((e) => {
+              Utils.send(res, 401, MatrixErrors.errMsg('unAuthorized'))
+            })
+        } else {
+          Utils.send(res, 401, MatrixErrors.errMsg('unAuthorized'))
+        }
       }
     } catch (error) {
       logger.debug(
@@ -99,13 +79,30 @@ export const auth = (
       )
       logger.error(`Unauthorized`, {
         ip: originalRequesterIPAddress,
-        httpMethod: req.method,
-        endpointPath: req.originalUrl
-      })
-      throw new FederationServerError({
-        status: 401,
-        code: MatrixErrors.errCodes.unAuthorized
+        httpMethod: request.method,
+        endpointPath: request.originalUrl
       })
+      Utils.send(res, 401, MatrixErrors.errMsg('unAuthorized'))
     }
   }
 }
+
+export const auth = (authenticator: Utils.AuthenticationFunction) => {
+  return (req: AuthRequest, res: Response, next: NextFunction): void => {
+    authenticator(req, res, (data: tokenContent, token: string | null) => {
+      if (token === tokenTrustedServer) {
+        next()
+        return
+      }
+      /* istanbul ignore if */
+      if (data.sub === undefined) {
+        throw new Error('Invalid data')
+      }
+      req.userId = data.sub
+      if (token != null) {
+        req.accessToken = token
+      }
+      next()
+    })
+  }
+}

packages/federation-server/src/routes/routes.ts

@@ -2,7 +2,6 @@ import { type TwakeLogger } from '@twake/logger'
 import { type IdServerAPI, type Utils } from '@twake/matrix-identity-server'
 import { Router, json, urlencoded } from 'express'
 import { hashDetails, lookup, lookups } from '../controllers/controllers'
-import { auth } from '../middlewares/auth'
 import { errorMiddleware } from '../middlewares/errors'
 import {
   allowCors,
@@ -20,6 +19,7 @@ import {
   type expressAppHandler,
   type middlewaresList
 } from '../types'
+import { auth } from '../middlewares/auth'
 
 const errorMiddlewares = (middleware: expressAppHandler): middlewaresList => [
   allowCors,
@@ -132,12 +132,7 @@ export default (
       allowCors,
       json(),
       urlencoded({ extended: false }),
-      auth(
-        authenticate,
-        conf.trusted_servers_addresses,
-        conf.trust_x_forwarded_for,
-        logger
-      ),
+      auth(authenticate),
       ...commonValidators,
       lookupValidator(conf.hashes_rate_limit as number),
       lookup(conf, db),
@@ -159,12 +154,7 @@ export default (
       allowCors,
       json(),
       urlencoded({ extended: false }),
-      auth(
-        authenticate,
-        conf.trusted_servers_addresses,
-        conf.trust_x_forwarded_for,
-        logger
-      ),
+      auth(authenticate),
       hashDetails(db, logger),
       errorMiddleware
     )
@@ -251,12 +241,7 @@ export default (
       allowCors,
       json(),
       urlencoded({ extended: false }),
-      auth(
-        authenticate,
-        conf.trusted_servers_addresses,
-        conf.trust_x_forwarded_for,
-        logger
-      ),
+      auth(authenticate),
       ...commonValidators,
       lookupsValidator,
       lookups(db),