refactor(recovery): Use base32 for recovery code generation (#372), r=@vbudhram

config/config.js

@@ -170,12 +170,6 @@ module.exports = function (fs, path, url, convict) {
       env: 'SENTRY_DSN'
     },
     recoveryCodes: {
-      keyspace: {
-        doc: 'Characters allowed in a recovery code',
-        default: 'abcdefghijklmnopqrstuvwxyz0123456789',
-        format: 'String',
-        env: 'RECOVERY_CODE_KEYSPACE'
-      },
       length: {
         doc: 'The length of a recovery code',
         default: 10,

lib/db/mem.js

@@ -51,7 +51,6 @@ const SESSION_DEVICE_FIELDS = [
   'lastAccessTime'
 ]
 
-const RECOVERY_CODE_KEYSPACE = config.recoveryCodes.keyspace
 const RECOVERY_CODE_LENGTH = config.recoveryCodes.length
 
 module.exports = function (log, error) {
@@ -1313,7 +1312,7 @@ module.exports = function (log, error) {
     let codes = []
     return getAccountByUid(uid)
       .then(() => {
-        return dbUtil.generateRecoveryCodes(count, RECOVERY_CODE_KEYSPACE, RECOVERY_CODE_LENGTH)
+        return dbUtil.generateRecoveryCodes(count, RECOVERY_CODE_LENGTH)
       })
       .then((result) => {
         codes = result

lib/db/mysql.js

@@ -34,7 +34,6 @@ const ER_DELETE_PRIMARY_EMAIL = 2100
 const ER_EXPIRED_TOKEN_VERIFICATION_CODE = 2101
 const ER_SIGNAL_NOT_FOUND = 1643
 
-const RECOVERY_CODE_KEYSPACE = config.recoveryCodes.keyspace
 const RECOVERY_CODE_LENGTH = config.recoveryCodes.length
 
 module.exports = function (log, error) {
@@ -1460,7 +1459,7 @@ module.exports = function (log, error) {
     // recovery codes is done is two separate procedures. First one
     // deletes all current codes and the second one inserts the
     // hashed randomly generated codes.
-    return dbUtil.generateRecoveryCodes(count, RECOVERY_CODE_KEYSPACE, RECOVERY_CODE_LENGTH)
+    return dbUtil.generateRecoveryCodes(count, RECOVERY_CODE_LENGTH)
       .then((codes) => {
         return this.read(DELETE_RECOVERY_CODES, [uid])
           .then(() => codes.map((code) => dbUtil.createHashScrypt(code)))

lib/db/random.js

@@ -0,0 +1,25 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+'use strict'
+const randomBytes = require('../promise').promisify(require('crypto').randomBytes)
+
+// The alphabet is lowercase for backwards compatibility with the previous keyspace
+// and the content-server also normalizes the codes to lowercase on submit
+const ALPHABET = '0123456789abcdefghjkmnpqrstvwxyz'
+
+function base32(len) {
+  return randomBytes(len)
+    .then(bytes => {
+      const out = []
+
+      for (let i = 0; i < len; i++) {
+        out.push(ALPHABET[bytes[i] % 32])
+      }
+
+      return out.join('')
+    })
+}
+
+module.exports = base32

lib/db/util.js

@@ -8,6 +8,7 @@ const crypto = require('crypto')
 const P = require('../promise')
 const randomBytes = P.promisify(require('crypto').randomBytes)
 const scryptHash = P.promisify(require('scrypt-hash'))
+const base32 = require('./random')
 
 const BOUNCE_TYPES = new Map([
   ['__fxa__unmapped', 0], // a bounce type we don't yet recognize
@@ -41,6 +42,7 @@ const VERIFICATION_METHODS = new Map([
   ['recovery-code', 3]   // Recovery code
 ])
 
+
 module.exports = {
 
   mapEmailBounceType(val) {
@@ -97,25 +99,15 @@ module.exports = {
       .then((hash) => crypto.timingSafeEqual(hash, verifyHash))
   },
 
-  generateRecoveryCodes(count, keyspace, length) {
-    const randomByteCodes = []
+
+  generateRecoveryCodes(count, length) {
+    const randomCodes = []
     for (let i = 0; i < count; i++) {
-      randomByteCodes.push(randomBytes(length))
+      randomCodes.push(base32(length))
     }
 
-    return P.all(randomByteCodes)
-      .then((result) => {
-        return result.map((randomCode) => {
-          const charsLength = keyspace.length
-          const result = []
-          let currentIndex = 0
-          for (let i = 0; i < 10; i++) {
-            currentIndex += randomCode[i]
-            result[i] = keyspace[currentIndex % charsLength]
-          }
-          return result.join('')
-        })
-      })
+    return P.all(randomCodes)
+
   },
 
   // A helper function for aggregating name:value pairs into a JSON object.

test/lib/utils.js

@@ -49,10 +49,10 @@ describe('utils', () => {
   })
 
   describe('generateRecoveryCodes', () => {
-    const codeLength = 10, codeCount = 5, keyspace = 'abcdefghijklmnopqrstuvwxyz0123456789'
+    const codeLength = 10, codeCount = 5
     let codes
     before(() => {
-      return dbUtils.generateRecoveryCodes(codeCount, keyspace, codeLength)
+      return dbUtils.generateRecoveryCodes(codeCount, codeLength)
         .then((result) => codes = result)
     })
 

test/local/random.js

@@ -0,0 +1,21 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+'use strict'
+
+const assert = require('insist')
+
+const base32 = require('../../lib/db/random')
+
+describe('random', () => {
+  it('should generate random code', () => {
+    return base32(10)
+      .then(code => {
+        assert.equal(code.length, 10)
+        assert.equal(code.indexOf('i'), -1, 'should not contain i')
+        assert.equal(code.indexOf('l'), -1, 'should not contain l')
+        assert.equal(code.indexOf('o'), -1, 'should not contain o')
+        assert.equal(code.indexOf('u'), -1, 'should not contain u')
+      })
+  })
+})