3
\$\begingroup\$

I'm not an expert in authentication, so I want to make sure I have done everything right.

I am using crypto.createCipheriv and crypto.createDecipheriv for authentication.

I will store the key in the database, and the initialization vector, IV, and encrypted data in cookies. When requested the server will try to decrypt encrypted data from cookies using the IV from the cookies and the key from the database. If the decryption is successful the request can proceed.

If the database is compromised, the hacker will not be able to use the database keys, because they will need the code to generate the IV and the data cookies.

The database key is the same for multiple sessions, only the IV and data are different.

Initial login conformation will be handled using external service, so I can focus on interacting with the cookies.

Are there problems with my solution?

import crypto from 'crypto'

function encrypt(dataToEncrypt /*random data*/) {
    let dbKey = crypto.randomBytes(32) //db
    let userKey = crypto.randomBytes(16) //cookie

    let cipher = crypto.createCipheriv('aes-256-cbc', Buffer.from(dbKey), userKey)
    let encryptedData = cipher.update(JSON.stringify(dataToEncrypt))
    encryptedData = Buffer.concat([encryptedData, cipher.final()])

    return { userKey: userKey.toString('hex'), dbKey: dbKey.toString('hex'), encryptedData: encryptedData.toString('hex') }
}

function decrypt(userProvidedKey, dbKey, dataToDecrypt) {
    let userKeyBuffer = Buffer.from(userProvidedKey, 'hex')
    let dbKeyBuffer = Buffer.from(dbKey, 'hex')

    let decryptedData = Buffer.from(dataToDecrypt, 'hex')
    let decipher = crypto.createDecipheriv('aes-256-cbc', dbKeyBuffer, userKeyBuffer)
    decryptedData = decipher.update(decryptedData)
    decryptedData = Buffer.concat([decryptedData, decipher.final()])

    return decryptedData.toString()
}
\$\endgroup\$
8
  • \$\begingroup\$ "Is my method correct?" Did you test it? \$\endgroup\$
    – Mast
    Jan 21 '20 at 13:30
  • \$\begingroup\$ @Mast yes it's working and seems fine. It's more theory/implementation question. \$\endgroup\$
    – ZiiMakc
    Jan 21 '20 at 13:31
  • 1
    \$\begingroup\$ Good answer. If you'd wanted us to focus on the theory you'd be in the wrong place, so to avoid disappointment I'd to make sure. \$\endgroup\$
    – Mast
    Jan 21 '20 at 13:36
  • 1
    \$\begingroup\$ Hey! I've edited the wording of your post. I found it challenging to read and understand. I have attempted to make it easier for everyone else to also understand your question. Please could you read over my changes to ensure I've not messed up any of your intended meaning. Thank you. \$\endgroup\$
    – Peilonrayz
    Jan 21 '20 at 14:44
  • 1
    \$\begingroup\$ @RTW No problem, best of luck! :) \$\endgroup\$
    – Peilonrayz
    Jan 21 '20 at 15:04
2
\$\begingroup\$

I am using crypto.createCipheriv and crypto.createDecipheriv for authentication.

Encryption is mainly used for providing confidentiality. You'd use a MAC (message authentication code) or authenticated encryption if you want authentication.

Note that CTR decryption always succeeds, even if it may result in incorrect ciphertext. CBC decryption succeeds 1 out of 256 times if PKCS#7 compatible padding is used (and it is) if random ciphertext is fed to it. Besides that, CBC is vulnerable to padding oracle attacks. Unauthenticated ciphertext is always vulnerable to plaintext oracle attacks.

Basically, you need a MAC (such as a HMAC over the IV and the ciphertext) or authenticated encryption.

GCM mode is provided by CryptoJS, if I'm not mistaken. GCM already authenticates the IV - but make sure it does authenticate the tag!

I will store the key in the database, and the initialization vector, IV, and encrypted data in cookies.

I presume this is a textual error, as the initialization vector is the IV.

If the database is compromised, the hacker will not be able to use the database keys, because they will need the code to generate the IV and the data cookies.

The IV should never be considered a secret. The data may of course be considered secret, but the algorithm to generate the data should not act as a key either (see Kerckhoff's principle).

The database key is the same for multiple sessions, only the IV and data are different.

That's right, as long as you don't create millions of "messages" (in this case cookies). If you overextend the key you may want to create a different key (for instance using key derivation). Probably this is not a problem for you though.


Let's move to the code (which we now know is not secure).

function encrypt(dataToEncrypt /*random data*/) {

Random has a very specific meaning in cryptography, so I'd try and avoid that adjective, and use "any" instead. I'd simply document your functions, and not put inline comments in the code.

let dbKey = crypto.randomBytes(32) //db

Fine, although I would use 256 / 8, preferably with a constant KEY_SIZE = 256 for the key size.

Don't use end-of-line comments either, as refactoring will make them disappear behind the right margin or trigger a nasty text wrap in editors.

Where the key is stored is not within this method, so the comment should not be present in the final code.

 let userKey = crypto.randomBytes(16) //cookie

No! An IV is not a key, and - even if it was - only the first block of ciphertext cannot be decrypted without the IV in CBC mode. Again, at least use a constant like BLOCK_SIZE_BYTES = 16 or IV_SIZE_BYTES = 16.

let cipher = crypto.createCipheriv('aes-256-cbc', Buffer.from(dbKey), userKey)

Wrong mode, but yeah.

let encryptedData = cipher.update(JSON.stringify(dataToEncrypt))

JSON-ify should be performed on a separate line. Now a significant operation is hidden inside another statement.

return { userKey: userKey.toString('hex'), dbKey: dbKey.toString('hex'), encryptedData: encryptedData.toString('hex') }

Ciphertext and keys are binary. I'd not stringify them unless strictly necessary. The IV and ciphertext may be concatenated and then converted to (URL-safe) base 64 to be more efficient than hex.


Skipping some lines to the final part of the decryption:

return decryptedData.toString()

That's no good, first you JSON-ify the input, and then you don't do the reverse during decryption. These methods should be symmetric. Either you take it out of encryption or you add it to decryption (if that's possible).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.