kryptein cryptography package for Node.js

Question

I have written a cryptography NPM package called kruptein. The code is also hosted on GitHub.

Having used multiple "best practices" with regards to key management, algorithm selection, key size, iv size etc. I could use someone with experience in this area to help validate the implementation.

I could use some help determining if I have flaws in any of the following:

1. Key sizes: _matrix(algo) - Are the appropriate key, iv & authentication tag sizes being identified and used correctly based on the algorithm selection? i.e. AES-128-CBC or AES-256-XTS

     _matrix(algo) {
       let obj = {
         at_size: 16,
         iv_size: 16,
         key_size: 32
       };
   
       if (algo.match(/ccm|ocb|gcm/i))
         obj.iv_size = 12;
   
       if (algo.match(/aes/) && algo.match(/ecb/))
         obj.iv_size = 0;
   
       if (algo.match(/aes/) && algo.match(/128/))
         obj.key_size = 16;
   
       if (algo.match(/aes/) && algo.match(/192/))
         obj.key_size = 24;
   
       if (algo.match(/aes/) && algo.match(/xts/))
         obj.key_size = 32;
   
       if (algo.match(/aes/) && algo.match(/xts/) && algo.match(/256/))
         obj.key_size = 64;
   
       return obj;
     }
   }
   

2. Key derivation: _derive_key(secret) - When deriving keyed material from a provided secret are enough iterations used? Is there a problem with the salt generation used for the resulting keyed material? What about the resulting key? For both the IV and key I am concerned about possible weakening of the material when converting to a buffer and slicing to the appropriate sizes.

     _derive_key(secret) {
       let key, hash, salt, result, derived_key;
   
       try {
         hash = this.crypto.createHash(this.hashing);
         hash.update(secret);
         salt = hash.digest();
       } catch(err) {
         throw err;
       }
   
       salt = (Buffer.isBuffer(salt)) ?
         salt.slice(0, 16) : salt.substr(0, 16);
   
       key = this.crypto.pbkdf2Sync(secret, salt, 10000, 64, this.hashing);
   
       return Buffer.from(key.toString(this.encodeas)).slice(0, this.key_size);
     }
   

3. IV generation: _iv(iv_size) - When generating a new IV is there a problem with slicing the results. Most of the node.js crypto API functions use buffers and the sizes must match but I can't help but think it is weakening the IV.

     _iv(iv_size) {
       let iv = this.crypto.randomBytes(iv_size);
   
       return Buffer.from(iv.toString(this.encodeas),
         this.encodeas).slice(0, iv_size);
     }
   

Answer 1

think I should prefix this by saying I'm not a JS or Crypto nerd, but am interested in both! to other people seeing this question, this code/module seems to mostly reimplement the CCM demo at the bottom of the Crypto docs: https://nodejs.org/api/crypto.html#crypto_ccm_mode

anyway on with a few issues I noticed:

1. why isn't _iv just returning crypto.randomBytes(iv_size)? encoding to a string and then getting bytes seems counterproductive at best, it just seems to throw away entropy

2. why use PBKDF2 over just hashing the secret? it's not being saved anywhere, and by using the secret as the salt you've just opened it wide up to rainbow table exploitation again. further, by doing your string encoding things you've just thrown away even more entropy

3. your use of regexes in _matrix looks suspect. why not just match against a few common patterns (e.g. /^\w+-\w+-\w+/) and handle the groups explicitly. allowing modes like ECB at all here looks like a mistake

4. catching exceptions just to immediately rethrow doesn't seem very useful

5. can you think of a better name for the member variable flag? maybe something like is_aead_mode

update:

another big issue just occurred to me; that you're not including any cryptographic details in the structure. this allows the suite used to be upgraded (e.g. NIST update their recommendations) while maintaining backwards compatibility. i.e. you should be saying that you're hashing the secret with SHA512, which algorithm was used for encryption, what you've used as a HMAC (if you've used it, CCM already includes its own MAC so HMAC isn't needed).