Merge pull request #10 from jambonz/bugfix/short-expires

Bugfix/short expires
2025-12-19 04:27:46 +00:00 · 2023-01-07 12:19:45 -05:00
parent d8d0490388 c7b1044bd4
commit a32d725e66
11 changed files with 2609 additions and 122 deletions
--- a/.github/workflows/docker-publish.yml
+++ b/.github/workflows/docker-publish.yml
@@ -20,7 +20,7 @@ jobs:
    if: github.event_name == 'push'

    steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3

      - name: Build image
        run: docker build . --file Dockerfile --tag $IMAGE_NAME
--- a/.github/workflows/npm-ci.yml
+++ b/.github/workflows/npm-ci.yml
@@ -8,8 +8,8 @@ jobs:
  build:
    runs-on: ubuntu-latest
    steps:
-      - uses: actions/checkout@v2
-      - uses: actions/setup-node@v1
+      - uses: actions/checkout@v3
+      - uses: actions/setup-node@v3
        with:
          node-version: 16.x
      - run: npm ci
--- a/app.js
+++ b/app.js
@@ -8,10 +8,7 @@ assert.ok(process.env.DRACHTIO_HOST, 'missing DRACHTIO_HOST env var');
 assert.ok(process.env.DRACHTIO_PORT, 'missing DRACHTIO_PORT env var');
 assert.ok(process.env.DRACHTIO_SECRET, 'missing DRACHTIO_SECRET env var');

-const opts = Object.assign({
-  timestamp: () => { return `, "time": "${new Date().toISOString()}"`; }
-}, { level: process.env.JAMBONES_LOGLEVEL || 'info' });
-const logger = require('pino')(opts);
+const logger = require('pino')({ level: process.env.JAMBONES_LOGLEVEL || 'info' });
 const Srf = require('drachtio-srf');
 const srf = new Srf();
 const StatsCollector = require('@jambonz/stats-collector');
@@ -60,6 +57,7 @@ const {

 srf.locals = {
  ...srf.locals,
+  logger,
  stats,
  addToSet, removeFromSet, isMemberOfSet, retrieveSet,
  registrar: new Registrar(logger, {
@@ -163,10 +161,10 @@ const authenticator = require('@jambonz/http-authenticator')(lookupAuthHook, log
 srf.use('register', [
  initLocals,
  responseTime(rttMetric),
-  rejectIpv4(logger),
+  rejectIpv4,
  regParser,
-  checkCache(logger),
-  checkAccountLimits(logger),
+  checkCache,
+  checkAccountLimits,
  authenticator]);

 srf.use('options', [
--- a/lib/middleware.js
+++ b/lib/middleware.js
@@ -4,36 +4,41 @@ const {NAT_EXPIRES} = require('./utils');

 const initLocals = (req, res, next) => {
  req.locals = req.locals || {};
+  req.locals.logger = req.srf.locals.logger;
  next();
 };

-const rejectIpv4 = (logger) => {
-  return (req, res, next) => {
-    const uri = parseUri(req.uri);
-    if (!uri?.host || /^(?:[0-9]{1,3}\.){3}[0-9]{1,3}$/.test(uri.host)) {
-      debug(`rejecting REGISTER from ${req.uri} as it has an ipv4 address and sip realm is required`);
-      res.send(403);
-      return req.srf.endSession(req);
-    }
-    next();
-  };
+const rejectIpv4 = (req, res, next) => {
+  const {logger} = req.locals;
+  const uri = parseUri(req.uri);
+  if (!uri?.host || /^(?:[0-9]{1,3}\.){3}[0-9]{1,3}$/.test(uri.host)) {
+    logger.info(`rejecting REGISTER from ${req.uri} as it has an ipv4 address and sip realm is required`);
+    res.send(403);
+    return req.srf.endSession(req);
+  }
+  next();
 };

-const checkCache = (logger) => {
-  return async(req, res, next) => {
-    const registration = req.registration;
-    const uri = parseUri(registration.aor);
-    const aor = `${uri.user}@${uri.host}`;
-    req.locals.realm = uri.host;
+const checkCache = async(req, res, next) => {
+  const {logger} = req.locals;
+  const registration = req.registration;
+  const uri = parseUri(registration.aor);
+  const aor = `${uri.user}@${uri.host}`;
+  req.locals.realm = uri.host;

-    if (registration.type === 'unregister') return next();
+  if (registration.type === 'unregister') return next();

-    const registrar = req.srf.locals.registrar;
-    const result = await registrar.query(aor);
-    if (result) {
-      // if known valid registration coming from same address, no need to hit the reg callback hook
-      if (result.proxy === `sip:${req.source_address}:${req.source_port}`) {
-        debug(`responding to cached register for ${aor}`);
+  const registrar = req.srf.locals.registrar;
+  const result = await registrar.query(aor);
+  if (result) {
+    // if known valid registration coming from same address, no need to hit the reg callback hook
+    if (result.proxy === `sip:${req.source_address}:${req.source_port}`) {
+
+      // though...check if the expiry is closer than NAT_EXPIRES, if so we do need to auth
+      if (Date.now() + (NAT_EXPIRES * 1000) < result.expiryTime) {
+        const ex = new Date(result.expiryTime).toISOString();
+        const check = new Date(Date.now() + (NAT_EXPIRES * 1000)).toISOString();
+        logger.debug({ex, check}, `responding to cached register for ${aor}`);
        res.cached = true;
        res.send(200, {
          headers: {
@@ -43,75 +48,73 @@ const checkCache = (logger) => {
        });
        return req.srf.endSession(req);
      }
+      else {
+        logger.debug(`cached registration for ${aor} is about to expire, need to re-authenticate`);
+      }
    }
-    next();
-  };
+  }
+  next();
 };

-const checkAccountLimits = (logger) => {
-  return async(req, res, next) => {
-
-    const {lookupAccountBySipRealm, lookupAccountCapacitiesBySid} = req.srf.locals.dbHelpers;
-    const {realm} = req.locals;
-    const {registrar, writeAlerts, AlertType} = req.srf.locals;
-    try {
-      const account = await lookupAccountBySipRealm(realm);
-      if (account) {
-        req.locals = {
-          ...req.locals,
-          account_sid: account.account_sid,
-          webhook_secret: account.webhook_secret
-        };
-        debug(account, `checkAccountLimits: retrieved account for realm: ${realm}`);
-      }
-      else if (process.env.JAMBONES_HOSTING) {
-        debug(`checkAccountLimits: unknown sip realm ${realm}`);
-        logger.info(`checkAccountLimits: rejecting register for unknown sip realm: ${realm}`);
-        return res.send(403);
-      }
-
-      if ('unregister' === req.registration.type || !process.env.JAMBONES_HOSTING) return next();
-
-      /* only check limits on the jambonz hosted platform */
-      const {account_sid} = account;
-      const capacities = await lookupAccountCapacitiesBySid(account_sid);
-      debug(JSON.stringify(capacities));
-      const limit_calls = capacities.find((c) => c.category == 'voice_call_session');
-      let limit_registrations = limit_calls.quantity * account.device_to_call_ratio;
-      const extra = capacities.find((c) => c.category == 'device');
-      if (extra && extra.quantity) limit_registrations += extra.quantity;
-      debug(`call capacity: ${limit_calls.quantity}, device capacity: ${limit_registrations}`);
-
-      if (0 === limit_registrations) {
-        debug('checkAccountLimits: device calling not allowed for this account');
-        logger.info({account_sid}, 'checkAccountLimits: device calling not allowed for this account');
-        writeAlerts({
-          alert_type: AlertType.ACCOUNT_DEVICE_LIMIT,
-          account_sid,
-          count: 0
-        }).catch((err) => logger.info({err}, 'checkAccountLimits: error writing alert'));
-
-        return res.send(503, 'Max Devices Registered');
-      }
-
-      const deviceCount = await registrar.getCountOfUsers(realm);
-      if (deviceCount >= limit_registrations) {
-        debug(account_sid, `checkAccountLimits: limit ${limit_registrations} count ${deviceCount}`);
-        logger.info({account_sid}, 'checkAccountLimits: registration rejected due to limits');
-        writeAlerts({
-          alert_type: AlertType.ACCOUNT_DEVICE_LIMIT,
-          account_sid,
-          count: limit_registrations
-        }).catch((err) => logger.info({err}, 'checkAccountLimits: error writing alert'));
-        return res.send(503, 'Max Devices Registered');
-      }
-      debug(`checkAccountLimits - passed: devices registered ${deviceCount}, limit is ${limit_registrations}`);
-      next();
-    } catch (err) {
-      logger.error({err, realm}, 'checkAccountLimits: error checking account limits');
-      res.send(500);
+const checkAccountLimits = async(req, res, next) => {
+  const {logger} = req.locals;
+  const {lookupAccountBySipRealm, lookupAccountCapacitiesBySid} = req.srf.locals.dbHelpers;
+  const {realm} = req.locals;
+  const {registrar, writeAlerts, AlertType} = req.srf.locals;
+  try {
+    const account = await lookupAccountBySipRealm(realm);
+    if (account) {
+      req.locals = {
+        ...req.locals,
+        account_sid: account.account_sid,
+        webhook_secret: account.webhook_secret
+      };
+      debug(account, `checkAccountLimits: retrieved account for realm: ${realm}`);
    }
-  };
+    else if (process.env.JAMBONES_HOSTING) {
+      logger.debug(`checkAccountLimits: unknown sip realm ${realm}`);
+      logger.info(`checkAccountLimits: rejecting register for unknown sip realm: ${realm}`);
+      return res.send(403);
+    }
+
+    if ('unregister' === req.registration.type || !process.env.JAMBONES_HOSTING) return next();
+
+    /* only check limits on the jambonz hosted platform */
+    const {account_sid} = account;
+    const capacities = await lookupAccountCapacitiesBySid(account_sid);
+    const limit_calls = capacities.find((c) => c.category == 'voice_call_session');
+    let limit_registrations = limit_calls.quantity * account.device_to_call_ratio;
+    const extra = capacities.find((c) => c.category == 'device');
+    if (extra && extra.quantity) limit_registrations += extra.quantity;
+    debug(`call capacity: ${limit_calls.quantity}, device capacity: ${limit_registrations}`);
+
+    if (0 === limit_registrations) {
+      logger.info({account_sid}, 'checkAccountLimits: device calling not allowed for this account');
+      writeAlerts({
+        alert_type: AlertType.ACCOUNT_DEVICE_LIMIT,
+        account_sid,
+        count: 0
+      }).catch((err) => logger.info({err}, 'checkAccountLimits: error writing alert'));
+
+      return res.send(503, 'Max Devices Registered');
+    }
+
+    const deviceCount = await registrar.getCountOfUsers(realm);
+    if (deviceCount > limit_registrations + 1) {
+      logger.info({account_sid}, 'checkAccountLimits: registration rejected due to limits');
+      writeAlerts({
+        alert_type: AlertType.ACCOUNT_DEVICE_LIMIT,
+        account_sid,
+        count: limit_registrations
+      }).catch((err) => logger.info({err}, 'checkAccountLimits: error writing alert'));
+      return res.send(503, 'Max Devices Registered');
+    }
+    logger.debug(`checkAccountLimits - passed: devices registered ${deviceCount}, limit is ${limit_registrations}`);
+    next();
+  } catch (err) {
+    logger.error({err, realm}, 'checkAccountLimits: error checking account limits');
+    res.send(500);
+  }
 };

 module.exports = {
--- a/lib/register.js
+++ b/lib/register.js
@@ -1,6 +1,5 @@
 const {isUacBehindNat, getSipProtocol, NAT_EXPIRES} = require('./utils');
 const parseUri = require('drachtio-srf').parseUri;
-const debug = require('debug')('jambonz:sbc-registrar');

 module.exports = handler;

@@ -26,10 +25,10 @@ async function register(logger, req, res) {
    let contactHdr = req.get('Contact');

    // reduce the registration interval if the device is behind a nat
-    if (isUacBehindNat(req)) {
+    if (isUacBehindNat(req) && expires > NAT_EXPIRES) {
      expires = NAT_EXPIRES;
-      contactHdr = contactHdr.replace(/expires=\d+/, `expires=${expires}`);
    }
+    contactHdr = contactHdr.replace(/expires=\d+/, `expires=${expires}`);
    const opts = {
      contact: req.getParsedHeader('Contact')[0].uri,
      sbcAddress: req.server.hostport,
@@ -38,10 +37,8 @@ async function register(logger, req, res) {
      callHook: req.authorization.grant.call_hook,
      callStatusHook: req.authorization.grant.call_status_hook
    };
-    logger.debug(`adding aor to redis ${aor}`);
-    const result = await registrar.add(aor, opts, grantedExpires);
-    debug(`result ${result} from adding ${JSON.stringify(opts)}`);
-    logger.debug(`successfully added ${aor} to redis, sending 200 OK`);
+    logger.debug(`adding aor to redis ${aor} with expires ${grantedExpires}`);
+    await registrar.add(aor, opts, grantedExpires);

    res.send(200, {
      headers: {
--- a/package-lock.json
+++ b/package-lock.json
--- a/package.json
+++ b/package.json
@@ -29,7 +29,7 @@
  "dependencies": {
    "@jambonz/db-helpers": "^0.7.3",
    "@jambonz/http-authenticator": "^0.2.2",
-    "@jambonz/mw-registrar": "^0.2.2",
+    "@jambonz/mw-registrar": "^0.2.3",
    "@jambonz/realtimedb-helpers": "^0.4.34",
    "@jambonz/stats-collector": "^0.1.6",
    "@jambonz/time-series": "^0.2.5",
@@ -44,6 +44,7 @@
    "clear-module": "^4.1.2",
    "eslint": "^7.32.0",
    "eslint-plugin-promise": "^4.3.1",
+    "nyc": "^15.1.0",
    "tape": "^4.15.1"
  }
 }
--- a/test/db/populate-test-data.sql
+++ b/test/db/populate-test-data.sql
@@ -15,7 +15,7 @@ values ('ed649e33-e771-403a-8c99-1780eabbc803', '3f35518f-5a0d-4c2e-90a5-2407bb3
 insert into account_subscriptions(account_subscription_sid, account_sid, pending)
 values ('f4e1848d-3ff8-40eb-b9c1-30e1ef053f94','ed649e33-e771-403a-8c99-1780eabbc803',0);
 insert into account_products(account_product_sid, account_subscription_sid, product_sid,quantity)
-values ('f23ff996-6534-4aba-8666-4b347391eca2', 'f4e1848d-3ff8-40eb-b9c1-30e1ef053f94', '2c815913-5c26-4004-b748-183b459329df', 1);
+values ('f23ff996-6534-4aba-8666-4b347391eca2', 'f4e1848d-3ff8-40eb-b9c1-30e1ef053f94', '2c815913-5c26-4004-b748-183b459329df', 2);
 insert into account_products(account_product_sid, account_subscription_sid, product_sid,quantity)
 values ('f23ff997-6534-4aba-8666-4b347391eca2', 'f4e1848d-3ff8-40eb-b9c1-30e1ef053f94', 'c4403cdb-8e75-4b27-9726-7d8315e3216d', 20);

--- a/test/scenarios/uac-re-register-auth-success.xml
+++ b/test/scenarios/uac-re-register-auth-success.xml
@@ -0,0 +1,30 @@
+<?xml version="1.0" encoding="ISO-8859-1" ?>
+<!DOCTYPE scenario SYSTEM "sipp.dtd">
+
+<scenario name="Basic Sipstone UAC">
+  <send retrans="500">
+  <![CDATA[
+REGISTER sip:[field1] SIP/2.0
+Via: SIP/2.0/[transport] [local_ip]:[local_port];branch=[branch]
+Max-Forwards: 70
+From: "sipp" <sip:[field0]@[field1]>;tag=[call_number]
+To: "sipp" <sip:[field0]@[field1]>
+Call-ID: reg///[call_id]
+Subject: uac-register-auth-success
+CSeq: 8 REGISTER
+Contact: <sip:sipp@[local_ip]:[local_port]>
+Expires: 3600
+Content-Length: 0
+User-Agent: SIPp
+  ]]>
+  </send>
+
+  <recv response="100" optional="true">
+  </recv>
+
+  <recv response="200">
+  </recv>
+
+  <ResponseTimeRepartition value="10, 20"/>
+  <CallLengthRepartition value="10"/>
+</scenario>
--- a/test/scenarios/uac-register-auth-success-jane.xml
+++ b/test/scenarios/uac-register-auth-success-jane.xml
@@ -0,0 +1,54 @@
+<?xml version="1.0" encoding="ISO-8859-1" ?>
+<!DOCTYPE scenario SYSTEM "sipp.dtd">
+
+<scenario name="Basic Sipstone UAC">
+  <send retrans="500">
+  <![CDATA[
+REGISTER sip:[field1] SIP/2.0
+Via: SIP/2.0/[transport] [local_ip]:[local_port];branch=[branch]
+Max-Forwards: 70
+From: "sipp" <sip:[field0]@[field1]>;tag=[call_number]
+To: "sipp" <sip:[field0]@[field1]>
+Call-ID: reg///[call_id]
+Subject: uac-register-auth-success
+CSeq: 7 REGISTER
+Contact: <sip:sipp@[local_ip]:[local_port]>
+Expires: 20
+Content-Length: 0
+User-Agent: SIPp
+  ]]>
+  </send>
+
+  <recv response="100" optional="true">
+  </recv>
+
+  <recv response="401" auth="true" rtd="true">
+  </recv>
+
+  <send retrans="500">
+  <![CDATA[
+REGISTER sip:[field1] SIP/2.0
+Via: SIP/2.0/[transport] [local_ip]:[local_port];branch=[branch]
+Max-Forwards: 70
+From: "sipp" <sip:[field0]@[field1]>;tag=[call_number]
+To: "sipp" <sip:[field0]@[field1]>
+Call-ID: reg///[call_id]
+CSeq: 8 REGISTER
+Subject: uac-register-auth-success
+Contact: <sip:sipp@[local_ip]:[local_port]>
+Expires: 20
+Content-Length: 0
+User-Agent: SIPp
+[field2]
+  ]]>
+</send>
+
+  <recv response="100" optional="true">
+  </recv>
+
+  <recv response="200">
+  </recv>
+
+  <ResponseTimeRepartition value="10, 20"/>
+  <CallLengthRepartition value="10"/>
+</scenario>
--- a/test/sip-register-tests.js
+++ b/test/sip-register-tests.js
@@ -40,6 +40,19 @@ test('register tests', (t) => {
    })
    .then(() => {
      t.pass('successfully registered when using valid credentials (service provider level auth hook)');
+      return sippUac('uac-re-register-auth-success.xml', sippRegObj);
+    })
+    .then(() => {
+      t.pass('successfully re-registered against cached registration');
+      sippRegObj.data_file = 'good_user2.csv';
+      return sippUac('uac-register-auth-success-jane.xml', sippRegObj);
+    })
+    .then(() => {
+      t.pass('successfully registered against short expiry');
+      return sippUac('uac-register-auth-success-jane.xml', sippRegObj);
+    })
+    .then(() => {
+      t.pass('successfully re-registered against short registration with re-auth');
      if (srf.locals.lb) srf.locals.lb.disconnect();
      srf.disconnect();
      t.end();