Feat/obscure sensitive bucket credentials (#375)

* obscure sensitive bucket credentials * npm audit fix * fix condition * add test suite encrypt-decrypt.test * revert docker-compose * update pipeline --------- Co-authored-by: mfrindt <m.frindt@cognigy.com>
jambonz · Jan 3, 2025 · 23cd440 · 23cd440
1 parent ce46185
commit 23cd440
Show file tree

Hide file tree

Showing 8 changed files with 315 additions and 35 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -23,5 +23,6 @@ jobs:
           sudo chmod +x /usr/local/bin/docker-compose
           docker-compose --version
       - run: npm test
+      - run: npm run test:encrypt-decrypt
 
 
diff --git a/lib/models/account.js b/lib/models/account.js
@@ -56,9 +56,13 @@ AND effective_end_date IS NULL
 AND pending = 0`;
 
 const extractBucketCredential = (obj) => {
-  const {bucket_credential} = obj;
-  if (bucket_credential) {
-    obj.bucket_credential = JSON.parse(decrypt(bucket_credential));
+  try {
+    const {bucket_credential} = obj;
+    if (bucket_credential) {
+      obj.bucket_credential = JSON.parse(decrypt(bucket_credential));
+    }
+  } catch (error) {
+    console.error('Error while decrypting data', error);
   }
 };
 

diff --git a/lib/routes/api/accounts.js b/lib/routes/api/accounts.js
@@ -19,11 +19,12 @@ const {
   parseCallSid,
   enableSubspace,
   disableSubspace,
-  parseVoipCarrierSid
+  parseVoipCarrierSid,
+  hasValue,
 } = require('./utils');
 const short = require('short-uuid');
 const VoipCarrier = require('../../models/voip-carrier');
-const { encrypt } = require('../../utils/encrypt-decrypt');
+const { encrypt, obscureBucketCredentialsSensitiveData, isObscureKey } = require('../../utils/encrypt-decrypt');
 const { testS3Storage, testGoogleStorage, testAzureStorage } = require('../../utils/storage-utils');
 const translator = short();
 
@@ -555,9 +556,12 @@ router.get('/:sid', async(req, res) => {
     const account_sid = parseAccountSid(req);
     await validateRequest(req, account_sid);
     const service_provider_sid = req.user.hasServiceProviderAuth ? req.user.service_provider_sid : null;
-    const results = await Account.retrieve(account_sid, service_provider_sid);
-    if (results.length === 0) return res.status(404).end();
-    return res.status(200).json(results[0]);
+    const [result] = await Account.retrieve(account_sid, service_provider_sid) || [];
+    if (!result) return res.status(404).end();
+
+    result.bucket_credential = obscureBucketCredentialsSensitiveData(result.bucket_credential);
+
+    return res.status(200).json(result);
   }
   catch (err) {
     sysError(logger, res, err);
@@ -639,26 +643,31 @@ router.delete('/:sid/SubspaceTeleport', async(req, res) => {
   }
 });
 
-function encryptBucketCredential(obj) {
-  if (!obj.bucket_credential) return;
+function encryptBucketCredential(obj, storedCredentials = {}) {
+  if (!hasValue(obj?.bucket_credential)) return;
   const {
     vendor,
     region,
     name,
     access_key_id,
-    secret_access_key,
     tags,
-    service_key,
-    connection_string,
     endpoint
   } = obj.bucket_credential;
+  let {
+    secret_access_key,
+    service_key,
+    connection_string
+  } = obj.bucket_credential;
 
   switch (vendor) {
     case 'aws_s3':
       assert(access_key_id, 'invalid aws S3 bucket credential: access_key_id is required');
       assert(secret_access_key, 'invalid aws S3 bucket credential: secret_access_key is required');
       assert(name, 'invalid aws bucket name: name is required');
       assert(region, 'invalid aws bucket region: region is required');
+      if (isObscureKey(obj.bucket_credential) && hasValue(storedCredentials)) {
+        secret_access_key = storedCredentials.secret_access_key;
+      }
       const awsData = JSON.stringify({vendor, region, name, access_key_id,
         secret_access_key, tags});
       obj.bucket_credential = encrypt(awsData);
@@ -668,18 +677,27 @@ function encryptBucketCredential(obj) {
       assert(secret_access_key, 'invalid aws S3 bucket credential: secret_access_key is required');
       assert(name, 'invalid aws bucket name: name is required');
       assert(endpoint, 'invalid endpoint uri: endpoint is required');
+      if (isObscureKey(obj.bucket_credential) && hasValue(storedCredentials)) {
+        secret_access_key = storedCredentials.secret_access_key;
+      }
       const s3Data = JSON.stringify({vendor, endpoint, name, access_key_id,
         secret_access_key, tags});
       obj.bucket_credential = encrypt(s3Data);
       break;
     case 'google':
       assert(service_key, 'invalid google cloud storage credential: service_key is required');
+      if (isObscureKey(obj.bucket_credential) && hasValue(storedCredentials)) {
+        service_key = storedCredentials.service_key;
+      }
       const googleData = JSON.stringify({vendor, name, service_key, tags});
       obj.bucket_credential = encrypt(googleData);
       break;
     case 'azure':
       assert(name, 'invalid azure container name: name is required');
       assert(connection_string, 'invalid azure cloud storage credential: connection_string is required');
+      if (isObscureKey(obj.bucket_credential) && hasValue(storedCredentials)) {
+        connection_string = storedCredentials.connection_string;
+      }
       const azureData = JSON.stringify({vendor, name, connection_string, tags});
       obj.bucket_credential = encrypt(azureData);
       break;
@@ -737,7 +755,17 @@ router.put('/:sid', async(req, res) => {
       delete obj.registration_hook;
       delete obj.queue_event_hook;
 
-      encryptBucketCredential(obj);
+      let storedBucketCredentials = {};
+      if (isObscureKey(obj?.bucket_credential)) {
+        const [account] = await Account.retrieve(sid) || [];
+        /* to avoid overwriting valid credentials with the obscured secret,
+         * that the frontend might send, we pass the stored account bucket credentials
+         * in the case it is a obscured key, we replace it with the stored one
+         */
+        storedBucketCredentials = account.bucket_credential;
+      }
+
+      encryptBucketCredential(obj, storedBucketCredentials);
 
       const rowsAffected = await Account.update(sid, obj);
       if (rowsAffected === 0) {
@@ -837,6 +865,13 @@ router.post('/:sid/BucketCredentialTest', async(req, res) => {
   try {
     const account_sid = parseAccountSid(req);
     await validateRequest(req, account_sid);
+    /* if the req.body bucket credentials contain an obscured key, replace with stored account.bucket_credential */
+    if (isObscureKey(req.body)) {
+      const service_provider_sid = req.user.hasServiceProviderAuth ? req.user.service_provider_sid : null;
+      const [account] = await Account.retrieve(account_sid, service_provider_sid) || [];
+      if (!account) return res.status(404).end();
+      req.body = account.bucket_credential;
+    }
     const {vendor, name, region, access_key_id, secret_access_key, service_key, connection_string, endpoint} = req.body;
     const ret = {
       status: 'not tested'

diff --git a/lib/routes/api/utils.js b/lib/routes/api/utils.js
@@ -440,6 +440,22 @@ const validatePasswordSettings = async(password) => {
   return;
 };
 
+function hasValue(data) {
+  if (typeof data === 'string') {
+    return data && data.length > 0;
+  } else if (Array.isArray(data)) {
+    return data && data.length > 0;
+  } else if (typeof data === 'object') {
+    return data && Object.keys(data).length > 0;
+  } else if (typeof data === 'number') {
+    return data !== null;
+  } else if (typeof data === 'boolean') {
+    return data !== null;
+  } else {
+    return false;
+  }
+}
+
 module.exports = {
   setupFreeTrial,
   createTestCdrs,
@@ -460,5 +476,6 @@ module.exports = {
   checkLimits,
   enableSubspace,
   disableSubspace,
-  validatePasswordSettings
+  validatePasswordSettings,
+  hasValue,
 };
diff --git a/lib/utils/encrypt-decrypt.js b/lib/utils/encrypt-decrypt.js
@@ -17,10 +17,15 @@ const encrypt = (text) => {
 };
 
 const decrypt = (data) => {
-  const hash = JSON.parse(data);
-  const decipher = crypto.createDecipheriv(algorithm, secretKey, Buffer.from(hash.iv, 'hex'));
-  const decrpyted = Buffer.concat([decipher.update(Buffer.from(hash.content, 'hex')), decipher.final()]);
-  return decrpyted.toString();
+  try {
+    const hash = JSON.parse(data);
+    const decipher = crypto.createDecipheriv(algorithm, secretKey, Buffer.from(hash.iv, 'hex'));
+    const decrpyted = Buffer.concat([decipher.update(Buffer.from(hash.content, 'hex')), decipher.final()]);
+    return decrpyted.toString();
+  } catch (error) {
+    console.error('Error while decrypting data', error);
+    return '{}';
+  }
 };
 
 const obscureKey = (key, key_spoiler_length = 6) => {
@@ -33,8 +38,85 @@ const obscureKey = (key, key_spoiler_length = 6) => {
   return `${key.slice(0, key_spoiler_length)}${key_spoiler_char.repeat(key.length - key_spoiler_length)}`;
 };
 
+function isObscureKey(bucketCredentials) {
+  if (!bucketCredentials) {
+    return false;
+  }
+
+  try {
+    const {
+      vendor,
+      secret_access_key = '',
+      service_key = '',
+      connection_string = ''
+    } = bucketCredentials || {};
+    let pattern;
+    switch (vendor) {
+      case 'aws_s3':
+      case 's3_compatible':
+        pattern = /^([A-Za-z0-9]{4,6}X+$)/;
+        return pattern.test(secret_access_key);
+      case 'azure':
+        pattern = /^https:[A-Za-z0-9\/.:?=&_-]+$/;
+        return pattern.test(connection_string);
+
+      case 'google': {
+        pattern = /^([A-Za-z0-9]{4,6}X+$)/;
+        let {private_key} = JSON.parse(service_key);
+        const key_header = '-----BEGIN PRIVATE KEY-----\n';
+        private_key = private_key.slice(key_header.length, private_key.length);
+        return pattern.test(private_key || '');
+      }
+    }
+    return false;
+  } catch (error) {
+    console.log('Error in isObscureKey', error);
+    return false;
+  }
+}
+
+/**
+ * obscure sensitive data in bucket credentials
+ * an obscured key contains of 6 'spoiled' characters of the key followed by 'X' characters
+ * '123456XXXXXXXXXXXXXXXXXXXXXXXX'
+ * @param {*} obj
+ * @returns
+ */
+function obscureBucketCredentialsSensitiveData(obj) {
+  if (!obj) return obj;
+  const {vendor, service_key, connection_string, secret_access_key} = obj;
+  switch (vendor) {
+    case 'aws_s3':
+    case 's3_compatible':
+      obj.secret_access_key = obscureKey(secret_access_key);
+      break;
+    case 'google':
+      const o = JSON.parse(service_key);
+      let private_key = o.private_key;
+      if (!isObscureKey(obj)) {
+        const key_header = '-----BEGIN PRIVATE KEY-----\n';
+        private_key = o.private_key.slice(key_header.length, o.private_key.length);
+        private_key = `${key_header}${obscureKey(private_key)}`;
+      }
+      const obscured = {
+        ...o,
+        private_key
+      };
+      obj.service_key = JSON.stringify(obscured);
+      break;
+    case 'azure':
+      obj.connection_string = obscureKey(connection_string);
+      break;
+  }
+
+  return obj;
+}
+
+
 module.exports = {
   encrypt,
   decrypt,
-  obscureKey
+  obscureKey,
+  isObscureKey,
+  obscureBucketCredentialsSensitiveData,
 };
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -6,6 +6,7 @@
   "scripts": {
     "start": "node app.js",
     "test": "NODE_ENV=test APPLY_JAMBONZ_DB_LIMITS=1 JWT_SECRET=foobarbazzle JAMBONES_MYSQL_HOST=127.0.0.1 JAMBONES_MYSQL_PORT=3360 JAMBONES_MYSQL_USER=jambones_test JAMBONES_MYSQL_PASSWORD=jambones_test JAMBONES_MYSQL_DATABASE=jambones_test JAMBONES_REDIS_HOST=localhost JAMBONES_REDIS_PORT=16379 JAMBONES_TIME_SERIES_HOST=127.0.0.1 JAMBONES_LOGLEVEL=error JAMBONES_CREATE_CALL_URL=http://localhost/v1/createCall K8S=true K8S_FEATURE_SERVER_SERVICE_NAME=127.0.0.1 K8S_FEATURE_SERVER_SERVICE_PORT=3100 node test/ ",
+    "test:encrypt-decrypt": "ENCRYPTION_SECRET=12345 node --test ./test/encrypt-decrypt.test.js",
     "integration-test": "NODE_ENV=test JAMBONES_AUTH_USE_JWT=1 JAMBONES_TIME_SERIES_HOST=127.0.0.1 AWS_REGION='us-east-1' JAMBONES_CURRENCY=USD JWT_SECRET=foobarbazzle JAMBONES_MYSQL_HOST=127.0.0.1 JAMBONES_MYSQL_PORT=3360 JAMBONES_MYSQL_USER=jambones_test JAMBONES_MYSQL_PASSWORD=jambones_test JAMBONES_MYSQL_DATABASE=jambones_test JAMBONES_REDIS_HOST=localhost JAMBONES_REDIS_PORT=16379 JAMBONES_LOGLEVEL=debug JAMBONES_CREATE_CALL_URL=http://localhost/v1/createCall node test/serve-integration.js",
     "upgrade-db": "node ./db/upgrade-jambonz-db.js",
     "coverage": "./node_modules/.bin/nyc --reporter html --report-dir ./coverage npm run test",
@@ -71,4 +72,4 @@
     "request-promise-native": "^1.0.9",
     "tape": "^5.7.5"
   }
-}
+}