nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
108.08k stars 29.83k forks source link

openssl_cipher_iv_length #22304

Closed lagden closed 4 years ago

lagden commented 6 years ago

I see some issues about invalid IV length or Invalid key length. In PHP, there is a helper method (openssl_cipher_iv_length) that return the correct length for IV.

Node could have a static method for this, something like:

Simple example:

const ivLen = crypto.cipherivLength('des-ede3-ofb') // => 8
const keyLen = crypto.cipherkeyLength('des-ede3-ofb') // => 24

Complex example:

'use strict'

const assert = require('assert').strict
const crypto = require('crypto')

const SALT = 'foobar'

function genKey(alg) {
    const keyLen = 24 // => crypto.cipherkeyLength(alg)
    const hash = crypto.createHash('sha256').update(SALT).digest()
    return hash.slice(0, keyLen)
}

function encrypt(value, alg = 'des-ede3-ofb') {
    const ivLen = 8 // => crypto.cipherivLength(alg)
    const iv = crypto.randomBytes(ivLen)
    const key = genKey(alg)
    const cipher = crypto.createCipheriv(alg, key, iv, {authTagLength: ivLen})
    const encryptedUpdate = cipher.update(value)
    const encryptedFinal = cipher.final()
    const encrypted = Buffer.concat([encryptedUpdate, encryptedFinal], encryptedUpdate.byteLength + encryptedFinal.byteLength)
    return [encrypted, iv]
}

function decrypt([encrypted, iv], alg = 'des-ede3-ofb') {
    const ivLen = iv.byteLength
    const key = genKey(alg)
    const cipher = crypto.createDecipheriv(alg, key, iv, {authTagLength: ivLen})
    const decryptedUpdate = cipher.update(encrypted)
    const decryptedFinal = cipher.final()
    return Buffer.concat([decryptedUpdate, decryptedFinal], decryptedUpdate.byteLength + decryptedFinal.byteLength)
}

// Testing
const input = 'test'
const output = decrypt(encrypt(input)).toString('utf8')
assert.strictEqual(input, output) // => OK
Trott commented 6 years ago

@nodejs/security-wg (proposed new API, I don't think there's any security implications but being cautious)

tniessen commented 6 years ago

What about ciphers such as CCM or OCB that don't have a single fixed IV length?

Trott commented 6 years ago

@nodejs/crypto Is this something we'd consider adding? If likely not, should this be closed?

tniessen commented 6 years ago

@Trott I can see that this might be useful. However, there is no single correct IV length for some ciphers. I believe OpenSSL simply returns the shortest permitted length.

mcollina commented 6 years ago

I think this could be a nice addition. Is this easy information to get from openssl?

sam-github commented 6 years ago

Even a minimum would be useful. It doesn't seem reasonable that an API can return "length invalid", and yet somehow not know what lengths are valid. I'll take a look at this when I have some time.

bnoordhuis commented 6 years ago

Is this easy information to get from openssl?

EVP_CIPHER_key_length() and EVP_CIPHER_iv_length(). The latter returns 0 for ciphers that don't take an IV (ECB mode ciphers, RC4, maybe more.)

Even a minimum would be useful.

I don't know. What business do you have using a cipher if you don't know its key size or IV size? Adding functions to help automate key/IV creation seems like setting up people to shoot themselves in the foot.

(Same argument goes for the block size, that currently isn't exposed either. Anyone want to argue that it should be?)

Take the OP's example code. If it derived the key/IV size automatically and you tricked it into picking RC4-40, you can guess the key in at most 2^40 tries. Foot, meet bullet.

tniessen commented 6 years ago

I don't know. What business do you have using a cipher if you don't know its key size or IV size? Adding functions to help automate key/IV creation seems like setting up people to shoot themselves in the foot.

I share this concern.

sam-github commented 6 years ago

What business do you have using a cipher if you don't know its key size or IV size?

Example, a file encryption tool where the user specifies the file, algorithm, and a pass phrase, and the utility runs the PBKDF, generates the key, creates a random sufficiently large IV, and writes CMS encrypted data to disk.

The tool could have a data structure of acceptable algorithms, and with each alg keep its key size, its IV size, etc. But if OpenSSL already includes this, why duplicate this into user-land?

lagden commented 6 years ago

In Go lang we have a helper: https://golang.org/pkg/crypto/aes/ (BlockSize) In PHP we have a helper: http://php.net/manual/en/function.openssl-cipher-iv-length.php

...and why not in Node.js?

I don't know. What business do you have using a cipher if you don't know its key size or IV size? Adding functions to help automate key/IV creation seems like setting up people to shoot themselves in the foot.

I have one In my app, the user can choose which cipher he want use to chat with another person So, I had to make a small map (key/IV) for that, but I'd like to make like in Golang: iv := ciphertext[:aes.BlockSize]