PubNub HIPAA compliant instant messaging encryption vulnerability

pubnub-encryption-vulnerability.md

PubNub is a HIPAA-compliant instant messaging platform that supports message and file encryption using AES-256-CBC.

The provided encryption is flawed, it drops half of the entropy from the encrpytion key, effectively half of the encrpytion key is constant.

The getKey function does an SHA-256 on the provided key string to make it the required 256bit (32byte) length for the AES-256. Then it's hex encoded, doubling the length in bytes then trimmed to 32 bytes. Due to hex encoding and trimming half of the bits in the key are always the same for every encoded message or file.

https://github.com/pubnub/javascript/blob/master/src/crypto/modules/web.js#L70

async getKey(key) {
    const bKey = Buffer.from(key);
    const abHash = await crypto.subtle.digest('SHA-256', bKey.buffer);

    const abKey = Buffer.from(Buffer.from(abHash).toString('hex').slice(0, 32), 'utf8').buffer;

    return crypto.subtle.importKey('raw', abKey, 'AES-CBC', true, ['encrypt', 'decrypt']);
}

Hello!

Thank you for analysis.

... effectively half of the encrpytion key is constant.

Even if it was a whole key, the digest function generates the same output for the key, which the user provides during client instantiation and won't change between function calls.

The most important thing to note from the start is that the server doesn't take part in the encryption / decryption process, and all is done on the client side.

Encryption relies on a key generated by this function and a random initialization vector, which changes for each message.
The decryption process requires a key to be known, so data will be properly decrypted.

In this gist title HIPAA mentioned, is there a specific requirement which potentially has been violated?

Hi!
Thank you for your reply.

Even if it was a whole key, the digest function generates the same output for the key, which the user provides during client instantiation and won't change between function calls.

Half of the encryption key that end up being passed to the AES algorithm are known and it's exactly the same for every single message encrypted by PubNub's client libraries.

The most important thing to note from the start is that the server doesn't take part in the encryption / decryption process, and all is done on the client side.

Yes, it's a problem in the PubNub client side libraries published on github.

Encryption relies on a key generated by this function and a random initialization vector, which changes for each message. The decryption process requires a key to be known, so data will be properly decrypted.

Note that half of the encryption key is known and the random initialization vector is not a secret, it's saved along with the encrypted message.

In this gist title HIPAA mentioned, is there a specific requirement which potentially has been violated?

I'm not a lawyer nor an encryption expert, but there are concerns you should consider in relation to HIPAA.
As far as I understand HIPAA requires at least AES-128. I cannot assess how PubNub's encrypt functionality relates to that, provided that it's technically AES-256, but half of the encryption key is constant.
Please note that PubNub's partners may unable to fix the issue for themselves, as PubNub's BAA may no longer cover HIPAA compliance should they chose to use different encryption.

Half of the encryption key that end up being passed to the AES algorithm are known and it's exactly the same for every single message encrypted by PubNub's client libraries.

digest function generates the same output with each call.

Note that half of the encryption key is known and the random initialization vector is not a secret, it's saved along with the encrypted message.

half of the encryption key is known – who knows about the key, attacker? How would they know the key, by probing payload (with altering content and checking provided output with random IV changed all the time)?
The generated key can be known by someone from outside, if they will inject code into an existing function.

It is pretty unsafe to hardcode secrets / cipher keys into applications for which source code can be checked with a debugger. Most of the users of PubNub has concept of accounts, so those credentials can be sent only after authorization from the user's server and don't stay in the code.

As far as I understand HIPAA requires at least AES-128. I cannot assess how PubNub's encrypt functionality relates to that, provided that it's technically AES-256, but half of the encryption key is constant.

but half of the encryption key is constant – even if we used the whole key generated by digest, it still will be constant.

As for HIPAA compliance it will be better to invite people from PubNub, who worked on HIPAA (if you want) to share more information about compliance.

It is pretty unsafe to hardcode secrets / cipher keys into applications for which source code can be checked with a debugger. Most of the users of PubNub has concept of accounts, so those credentials can be sent only after authorization from the user's server and don't stay in the code.

Yes, users should not hard code the key, then nothing will save them.

digest function generates the same output with each call.

The problem isn't with digest, but the hex encoding and then dropping half of the resulting string. You should use the output of digest directly and please do not hex encode.

After the hex encoding certain bits are fixed no matter what was the input to hex encode. Other bits are more likely to be 0 or 1. Because of this even if the user does a perfect job, gives a proper, securely generated key, stored correctly, PubNub's decode drops half the entropy and now the key is way less secure than it should be.

Please note that PubNub's partners may unable to fix the issue for themselves

SDK users can use their encryption before sending data to publish methods if built-in encryption doesn't work well for them.

We have certain changes on the roadmap, which should let users specify their modules for crypto, network, and JSON parsing – but all this can apply only for new users. Encryption / decryption can't be changed that easy because it will affect existing users, and they will lose access to already generated data stored in history.
New users without apps in production will be able to use new approaches, but old customers will stay on the old one (disabling new approach with configuration flags) because it is impossible to make all app users update altogether, and some old versions won't be able to process received data.

SDK users can use their encryption before sending data to publish methods if built-in encryption doesn't work well for them.

Of course they can! I was referring to HIPAA compliance in that case.
Let's say you remove the encryption from the PubNub API. Can you still advertise PubNub as HIPAA compliant service?
Let's say you keep it like this. PubNub's partner signs a BAA, but then choose not to use the encryption PubNub provides for HIPAA compliance, but do something else. Are they still HIPAA compliant?

Why you should fix this by removing hex encode:

• PubNub hires an encryption expert and 100% sure it's okay this way, wouldn't it be easier if it's just done properly?
• Provided this is okay for HIPAA compliance, PubNub is advertised as using AES-256 on the official website, but the provided encrypt function is less secure than AES-256, even if under the hood it's using AES-256.
• I feel it's embarrassing to have such a mistake in a public repository.

It's a breaking change, and all the other PubNub client libraries should be changed the same way:

async getKey(key) {
    const bKey = Buffer.from(key);
    const abHash = await crypto.subtle.digest('SHA-256', bKey.buffer);

    const abKey = Buffer.from(abHash).buffer;

    return crypto.subtle.importKey('raw', abKey, 'AES-CBC', true, ['encrypt', 'decrypt']);
}

Look, I'm not working for PubNub, nor working for Snyk. I'm not paid to help. I'm just a freelancer, who run into this on a project. I just suggest to have this fixed.

Have a nice day