Merge pull request #3 from overleaf/spd-security-and-deprecation

Replace deprecated createCipher with createCipheriv for new tokens

libraries/access-token-encryptor/lib/coffee/AccessTokenEncryptor.coffee

@@ -5,6 +5,9 @@ ALGORITHM = 'aes-256-ctr'
 keyFn = (password, salt, callback)->
 	return crypto.pbkdf2(password, salt, 10000, 64, 'sha1', callback)
 
+keyFn32 = (password, salt, keyLength, callback)->
+	return crypto.pbkdf2(password, salt, 10000, 32, 'sha1', callback)
+
 class AccessTokenEncryptor
 
 	constructor: (settings) ->
@@ -19,25 +22,52 @@ class AccessTokenEncryptor
 
 	encryptJson: (json, callback) ->
 		string = JSON.stringify(json)
-		salt = crypto.randomBytes(16)
-		keyFn @cipherPassword, salt, (err, key) =>
-			if err?
-				logger.err err:err, "error getting Fn key"
-				return callback(err)
-			cipher = crypto.createCipher(ALGORITHM,  key)
-			crypted  = cipher.update(string, 'utf8', 'base64') + cipher.final('base64')
-			callback(null, @cipherLabel + ":" + salt.toString('hex') + ":" + crypted)
+		crypto.randomBytes 32, (err, bytes) =>
+			return callback(err) if err
+			salt = bytes.slice(0, 16)
+			iv = bytes.slice(16, 32)
+
+			keyFn32 @cipherPassword, salt, 32, (err, key) =>
+				if err?
+					logger.err err:err, "error getting Fn key"
+					return callback(err)
+
+				cipher = crypto.createCipheriv(ALGORITHM, key, iv)
+				crypted = cipher.update(string, 'utf8', 'base64') + cipher.final('base64')
+
+				callback(null, "#{@cipherLabel}:#{salt.toString('hex')}:#{crypted}:#{iv.toString('hex')}")
 
 	decryptToJson: (encryptedJson, callback) ->
-		[label, salt, cipherText] = encryptedJson.split(':', 3)
+		[label, salt, cipherText, iv] = encryptedJson.split(':', 4)
 		password = @settings.cipherPasswords[label]
 		return callback(new Error("invalid password")) if not password? or password.length < 16
-		keyFn password, new Buffer(salt, 'hex'), (err, key) =>
+
+		if iv
+			@decryptToJsonV2(password, salt, cipherText, iv, callback)
+		else
+			@decryptToJsonV1(password, salt, cipherText, callback)
+
+	decryptToJsonV1: (password, salt, cipherText, callback) ->
+		keyFn password, Buffer.from(salt, 'hex'), (err, key) =>
 			if err?
 				logger.err err:err, "error getting Fn key"
 				return callback(err)
 			decipher = crypto.createDecipher(ALGORITHM, key)
 			dec = decipher.update(cipherText, 'base64', 'utf8') + decipher.final('utf8')
+			try
+				json = JSON.parse(dec)
+			catch e
+				return callback(new Error("error decrypting token"))
+			callback(null, json, true)
+
+	decryptToJsonV2: (password, salt, cipherText, iv, callback) ->
+		keyFn32 password, Buffer.from(salt, 'hex'), 32, (err, key) =>
+			if err?
+				logger.err err:err, "error getting Fn key"
+				return callback(err)
+
+			decipher = crypto.createDecipheriv(ALGORITHM, key, Buffer.from(iv, 'hex'))
+			dec = decipher.update(cipherText, 'base64', 'utf8') + decipher.final('utf8')
 			try
 				json = JSON.parse(dec)
 			catch e

libraries/access-token-encryptor/package.json

@@ -5,7 +5,7 @@
   "private": true,
   "main": "index.js",
   "scripts": {
-    "test": "echo \"Error: no test specified\" && exit 1"
+    "test": "grunt coffee && grunt mochaTest"
   },
   "author": "",
   "license": "Proprietary",
@@ -13,16 +13,18 @@
     "coffee-script": "^1.10.0"
   },
   "devDependencies": {
-    "nock": "0.15.2",
-    "sandboxed-module": "",
-    "sinon": "",
-    "chai": "",
-    "grunt-contrib-clean": "~0.5.0",
-    "grunt-mocha-test": "~0.10.2",
-    "grunt-contrib-coffee": "~0.10.1",
-    "grunt": "~0.4.5",
-    "grunt-execute": "~0.2.1",
     "bunyan": "~0.22.3",
-    "grunt-bunyan": "~0.5.0"
+    "chai": "",
+    "grunt": "~0.4.5",
+    "grunt-bunyan": "~0.5.0",
+    "grunt-cli": "^1.3.2",
+    "grunt-contrib-clean": "~0.5.0",
+    "grunt-contrib-coffee": "~0.10.1",
+    "grunt-execute": "~0.2.1",
+    "grunt-mocha-test": "^0.13.3",
+    "mocha": "^6.2.2",
+    "nock": "0.15.2",
+    "sandboxed-module": "^2.0.3",
+    "sinon": "^7.5.0"
   }
 }

libraries/access-token-encryptor/test/unit/coffee/AccessTokenEncryptorTests.coffee

@@ -10,24 +10,26 @@ describe 'AccessTokenEncryptor', ->
 
 	beforeEach ->
 		@testObject = {"hello":"world"}
-		@Encrypted = "2016.1:6e7ac79ab13a18b5749eace965ec7962:sAAYt1yQZqpvOnu6l8iUD/Y="
-		@oldEncrypted = "2015.1:473a66fb5d816bc716f278ab819d88a5:+mTg7O9sgUND8pNQFG6h2GE="
+		@encrypted2015 = "2015.1:473a66fb5d816bc716f278ab819d88a5:+mTg7O9sgUND8pNQFG6h2GE="
+		@encrypted2016 = "2016.1:76a7d64a444ccee1a515b49c44844a69:m5YSkexUsLjcF4gLncm72+k="
+		@encrypted2019 = "2019.1:627143b2ab185a020c8720253a4c984e:7gnY6Ez3/Y3UWgLHLfBtJsE=:bf75cecb6aeea55b3c060e1122d2a82d"
 		@badLabel = "xxxxxx:c7a39310056b694c:jQf+Uh5Den3JREtvc82GW5Q="
 		@badKey = "2015.1:d7a39310056b694c:jQf+Uh5Den3JREtvc82GW5Q="
 		@badCipherText = "2015.1:c7a39310056b694c:xQf+Uh5Den3JREtvc82GW5Q="
 		@settings =
-			cipherLabel: "2016.1"
+			cipherLabel: "2019.1"
 			cipherPasswords:
 				"2016.1": "11111111111111111111111111111111111111"
 				"2015.1": "22222222222222222222222222222222222222"
-		AccessTokenEncryptor = SandboxedModule.require modulePath, @requires
-		@encryptor = new AccessTokenEncryptor(@settings)
+				"2019.1": "33333333333333333333333333333333333333"
+		@AccessTokenEncryptor = SandboxedModule.require modulePath
+		@encryptor = new @AccessTokenEncryptor(@settings)
 
 	describe "encrypt", ->
 		it 'should encrypt the object', (done)->
 			@encryptor.encryptJson @testObject, (err, encrypted)->
 				expect(err).to.be.null
-				encrypted.should.match(/^2016.1:[0-9a-f]+:[a-zA-Z0-9=+\/]+$/)
+				encrypted.should.match(/^2019.1:[0-9a-f]{32}:[a-zA-Z0-9=+\/]+:[0-9a-f]{32}$/)
 				done()
 
 		it 'should encrypt the object differently the next time', (done)->
@@ -45,8 +47,20 @@ describe 'AccessTokenEncryptor', ->
 					expect(decrypted).to.deep.equal @testObject
 					done()
 
-		it 'should decrypt an old string to get the same object', (done)->
-			@encryptor.decryptToJson @oldEncrypted, (err, decrypted)=>
+		it 'should decrypt an 2015 string to get the same object', (done)->
+			@encryptor.decryptToJson @encrypted2015, (err, decrypted)=>
+				expect(err).to.be.null
+				expect(decrypted).to.deep.equal @testObject
+				done()
+
+		it 'should decrypt an 2016 string to get the same object', (done)->
+			@encryptor.decryptToJson @encrypted2016, (err, decrypted)=>
+				expect(err).to.be.null
+				expect(decrypted).to.deep.equal @testObject
+				done()
+
+		it 'should decrypt an 2019 string to get the same object', (done)->
+			@encryptor.decryptToJson @encrypted2019, (err, decrypted)=>
 				expect(err).to.be.null
 				expect(decrypted).to.deep.equal @testObject
 				done()