Skip to content

Generate tokens with CSPRNG #372

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 12, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
83 changes: 83 additions & 0 deletions spec/cryptoUtils.spec.js
Original file line number Diff line number Diff line change
@@ -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);
});
});
6 changes: 2 additions & 4 deletions src/Controllers/FilesController.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down
7 changes: 2 additions & 5 deletions src/GCM.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down
29 changes: 6 additions & 23 deletions src/RestWrite.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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();
}
}
}
Expand Down Expand Up @@ -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
Expand All @@ -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();
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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;
6 changes: 2 additions & 4 deletions src/Routers/UsersRouter.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// These methods handle the User-related routes.

import hat from 'hat';
import deepcopy from 'deepcopy';

import ClassesRouter from './ClassesRouter';
Expand All @@ -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) {
Expand Down Expand Up @@ -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;

Expand Down
44 changes: 44 additions & 0 deletions src/cryptoUtils.js
Original file line number Diff line number Diff line change
@@ -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);
}
6 changes: 3 additions & 3 deletions src/testing-routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -70,4 +70,4 @@ router.post('/rest_configure_app',

module.exports = {
router: router
};
};