Generate tokens and ids with cryptoUtils module.

Move object ID, token, and random string generation into their own
module, cryptoUtils.

Remove hat dependency, which was used to generate session and some other
tokens, because it  used non-cryptographic random number generator.
Replace it with the cryptographically secure one. The result has the
same format (32-character hex string, 128 bits of entropy).

Remove randomstring dependency, as we already have this functionality.

Add tests.

Author: dchest

package.json

@@ -16,13 +16,11 @@
     "body-parser": "^1.14.2",
     "deepcopy": "^0.6.1",
     "express": "^4.13.4",
-    "hat": "~0.0.3",
     "mime": "^1.3.4",
     "mongodb": "~2.1.0",
     "multer": "^1.1.0",
     "node-gcm": "^0.14.0",
     "parse": "^1.7.0",
-    "randomstring": "^1.1.3",
     "request": "^2.65.0",
     "winston": "^2.1.1"
   },

spec/cryptoUtils.spec.js

@@ -0,0 +1,83 @@
+var cryptoUtils = require('../src/cryptoUtils');
+
+function givesUniqueResults(fn, iterations) {
+  var results = {};
+  for (var i = 0; i < iterations; i++) {
+    var s = fn();
+    if (results[s]) {
+      return false;
+    }
+    results[s] = true;
+  }
+  return true;
+}
+
+describe('randomString', () => {
+  it('returns a string', () => {
+    expect(typeof cryptoUtils.randomString(10)).toBe('string');
+  });
+
+  it('returns result of the given length', () => {
+    expect(cryptoUtils.randomString(11).length).toBe(11);
+    expect(cryptoUtils.randomString(25).length).toBe(25);
+  });
+
+  it('throws if requested length is zero', () => {
+    expect(() => cryptoUtils.randomString(0)).toThrow();
+  });
+
+  it('returns unique results', () => {
+    expect(givesUniqueResults(() => cryptoUtils.randomString(10), 100)).toBe(true);
+  });
+});
+
+describe('randomHexString', () => {
+  it('returns a string', () => {
+    expect(typeof cryptoUtils.randomHexString(10)).toBe('string');
+  });
+
+  it('returns result of the given length', () => {
+    expect(cryptoUtils.randomHexString(10).length).toBe(10);
+    expect(cryptoUtils.randomHexString(32).length).toBe(32);
+  });
+
+  it('throws if requested length is zero', () => {
+    expect(() => cryptoUtils.randomHexString(0)).toThrow();
+  });
+
+  it('throws if requested length is not even', () => {
+    expect(() => cryptoUtils.randomHexString(11)).toThrow();
+  });
+
+  it('returns unique results', () => {
+    expect(givesUniqueResults(() => cryptoUtils.randomHexString(20), 100)).toBe(true);
+  });
+});
+
+describe('newObjectId', () => {
+  it('returns a string', () => {
+    expect(typeof cryptoUtils.newObjectId()).toBe('string');
+  });
+
+  it('returns result with at least 10 characters', () => {
+    expect(cryptoUtils.newObjectId().length).toBeGreaterThan(9);
+  });
+
+  it('returns unique results', () => {
+    expect(givesUniqueResults(() => cryptoUtils.newObjectId(), 100)).toBe(true);
+  });
+});
+
+describe('newToken', () => {
+  it('returns a string', () => {
+    expect(typeof cryptoUtils.newToken()).toBe('string');
+  });
+
+  it('returns result with at least 32 characters', () => {
+    expect(cryptoUtils.newToken().length).toBeGreaterThan(31);
+  });
+
+  it('returns unique results', () => {
+    expect(givesUniqueResults(() => cryptoUtils.newToken(), 100)).toBe(true);
+  });
+});

src/Controllers/FilesController.js

@@ -4,11 +4,9 @@ import express from 'express';
 import mime from 'mime';
 import { Parse } from 'parse/node';
 import BodyParser from 'body-parser';
-import hat from 'hat';
 import * as Middlewares from '../middlewares';
 import Config from '../Config';
-
-const rack = hat.rack();
+import { randomHexString } from '../cryptoUtils';
 
 export class FilesController {
   constructor(filesAdapter) {
@@ -61,7 +59,7 @@ export class FilesController {
         extension = '.' + mime.extension(contentType);
       }
 
-      let filename = rack() + '_' + req.params.filename + extension;
+      let filename = randomHexString(32) + '_' + req.params.filename + extension;
       this._filesAdapter.createFile(req.config, filename, req.body).then(() => {
         res.status(201);
         var location = this._filesAdapter.getFileLocation(req.config, filename);

src/GCM.js

@@ -2,7 +2,7 @@
 
 const Parse = require('parse/node').Parse;
 const gcm = require('node-gcm');
-const randomstring = require('randomstring');
+const cryptoUtils = require('./cryptoUtils');
 
 const GCMTimeToLiveMax = 4 * 7 * 24 * 60 * 60; // GCM allows a max of 4 weeks
 const GCMRegistrationTokensMax = 1000;
@@ -22,10 +22,7 @@ function GCM(args) {
  * @returns {Object} A promise which is resolved after we get results from gcm
  */
 GCM.prototype.send = function(data, devices) {
-  let pushId = randomstring.generate({
-    length: 10,
-    charset: 'alphanumeric'
-  });
+  let pushId = cryptoUtils.newObjectId();
   let timeStamp = Date.now();
   let expirationTime;
   // We handle the expiration_time convertion in push.js, so expiration_time is a valid date

src/RestWrite.js

@@ -2,13 +2,12 @@
 // that writes to the database.
 // This could be either a "create" or an "update".
 
-var crypto = require('crypto');
 var deepcopy = require('deepcopy');
-var rack = require('hat').rack();
 
 var Auth = require('./Auth');
 var cache = require('./cache');
 var Config = require('./Config');
+var cryptoUtils = require('./cryptoUtils');
 var passwordCrypto = require('./password');
 var facebook = require('./facebook');
 var Parse = require('parse/node');
@@ -56,7 +55,7 @@ function RestWrite(config, auth, className, query, data, originalData) {
     this.data.updatedAt = this.updatedAt;
     if (!this.query) {
       this.data.createdAt = this.updatedAt;
-      this.data.objectId = newStringId(10);
+      this.data.objectId = cryptoUtils.newObjectId();
     }
   }
 }
@@ -252,7 +251,7 @@ RestWrite.prototype.handleFacebookAuthData = function() {
         throw new Parse.Error(Parse.Error.ACCOUNT_ALREADY_LINKED,
                               'this auth is already used');
       } else {
-        this.data.username = rack();
+        this.data.username = cryptoUtils.newToken();
       }
 
       // This FB auth does not already exist, so transform it to a
@@ -273,7 +272,7 @@ RestWrite.prototype.transformUser = function() {
   var promise = Promise.resolve();
 
   if (!this.query) {
-    var token = 'r:' + rack();
+    var token = 'r:' + cryptoUtils.newToken();
     this.storage['token'] = token;
     promise = promise.then(() => {
       var expiresAt = new Date();
@@ -319,7 +318,7 @@ RestWrite.prototype.transformUser = function() {
     // Check for username uniqueness
     if (!this.data.username) {
       if (!this.query) {
-        this.data.username = newStringId(25);
+        this.data.username = cryptoUtils.randomString(25);
       }
       return;
     }
@@ -412,7 +411,7 @@ RestWrite.prototype.handleSession = function() {
   }
 
   if (!this.query && !this.auth.isMaster) {
-    var token = 'r:' + rack();
+    var token = 'r:' + cryptoUtils.newToken();
     var expiresAt = new Date();
     expiresAt.setFullYear(expiresAt.getFullYear() + 1);
     var sessionData = {
@@ -713,20 +712,4 @@ RestWrite.prototype.objectId = function() {
   return this.data.objectId || this.query.objectId;
 };
 
-// Returns a unique string that's usable as an object or other id.
-function newStringId(size) {
-  var chars = ('ABCDEFGHIJKLMNOPQRSTUVWXYZ' +
-               'abcdefghijklmnopqrstuvwxyz' +
-               '0123456789');
-  var objectId = '';
-  var bytes = crypto.randomBytes(size);
-  for (var i = 0; i < bytes.length; ++i) {
-    // Note: there is a slight modulo bias, because chars length
-    // of 62 doesn't divide the number of all bytes (256) evenly.
-    // It is acceptable for our purposes.
-    objectId += chars[bytes.readUInt8(i) % chars.length];
-  }
-  return objectId;
-}
-
 module.exports = RestWrite;

src/Routers/UsersRouter.js

@@ -1,6 +1,5 @@
 // These methods handle the User-related routes.
 
-import hat from 'hat';
 import deepcopy from 'deepcopy';
 
 import ClassesRouter from './ClassesRouter';
@@ -9,8 +8,7 @@ import rest from '../rest';
 import Auth from '../Auth';
 import passwordCrypto from '../password';
 import RestWrite from '../RestWrite';
-
-const rack = hat.rack();
+import { newToken } from '../cryptoUtils';
 
 export class UsersRouter extends ClassesRouter {
   handleFind(req) {
@@ -89,7 +87,7 @@ export class UsersRouter extends ClassesRouter {
           throw new Parse.Error(Parse.Error.OBJECT_NOT_FOUND, 'Invalid username/password.');
         }
 
-        let token = 'r:' + rack();
+        let token = 'r:' + newToken();
         user.sessionToken = token;
         delete user.password;
 

src/cryptoUtils.js

@@ -0,0 +1,44 @@
+import { randomBytes } from 'crypto';
+
+// Returns a new random hex string of the given even size.
+export function randomHexString(size) {
+  if (size === 0) {
+    throw new Error('Zero-length randomHexString is useless.');
+  }
+  if (size % 2 !== 0) {
+    throw new Error('randomHexString size must be divisible by 2.')
+  }
+  return randomBytes(size/2).toString('hex');
+}
+
+// Returns a new random alphanumeric string of the given size.
+//
+// Note: to simplify implementation, the result has slight modulo bias,
+// because chars length of 62 doesn't divide the number of all bytes
+// (256) evenly. Such bias is acceptable for most cases when the output
+// length is long enough and doesn't need to be uniform.
+export function randomString(size) {
+  if (size === 0) {
+    throw new Error('Zero-length randomString is useless.');
+  }
+  var chars = ('ABCDEFGHIJKLMNOPQRSTUVWXYZ' +
+               'abcdefghijklmnopqrstuvwxyz' +
+               '0123456789');
+  var objectId = '';
+  var bytes = randomBytes(size);
+  for (var i = 0; i < bytes.length; ++i) {
+    objectId += chars[bytes.readUInt8(i) % chars.length];
+  }
+  return objectId;
+}
+
+// Returns a new random alphanumeric string suitable for object ID.
+export function newObjectId() {
+  //TODO: increase length to better protect against collisions.
+  return randomString(10);
+}
+
+// Returns a new random hex string suitable for secure tokens.
+export function newToken() {
+  return randomHexString(32);
+}

src/testing-routes.js

@@ -3,13 +3,13 @@
 var express = require('express'),
     cache = require('./cache'),
     middlewares = require('./middlewares'),
-    rack = require('hat').rack();
+    cryptoUtils = require('./cryptoUtils');
 
 var router = express.Router();
 
 // creates a unique app in the cache, with a collection prefix
 function createApp(req, res) {
-  var appId = rack();
+  var appId = cryptoUtils.randomHexString(32);
   cache.apps[appId] = {
     'collectionPrefix': appId + '_',
     'masterKey': 'master'
@@ -70,4 +70,4 @@ router.post('/rest_configure_app',
 
 module.exports = {
   router: router
-};
+};