Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Insecure Cryptography #1

Closed
paragonie-scott opened this issue May 23, 2017 · 9 comments
Closed

Insecure Cryptography #1

paragonie-scott opened this issue May 23, 2017 · 9 comments

Comments

@paragonie-scott
Copy link

paragonie-scott commented May 23, 2017

hardbin/js/hardbin.js

Lines 5 to 11 in c77c2d7

function encrypt(data, key) {
return CryptoJS.AES.encrypt(data, key).toString();
}
function decrypt(data, key) {
return CryptoJS.AES.decrypt(data, key).toString(CryptoJS.enc.Utf8);
}
-> http://cryptojs.altervista.org/secretkey/doc/doc_aes_cryptojs-v3.html

You're using unauthenticated AES-CBC. https://paragonie.com/blog/2015/05/using-encryption-and-authentication-correctly

Your About section says:

Since nobody can modify the code, and nobody can view the key unless you show it to them, nobody without the key can either read the plaintext or ship a malicious viewer which would exfiltrate the plaintext (or key).

...but this is strictly not true with unauthenticated encryption. In fact, you can recover the plaintext with a padding oracle attack.

Also, your RNG is biased.

hardbin/js/hardbin.js

Lines 13 to 25 in c77c2d7

function generate_key() {
var alphabet = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ';
var randombytes = new Uint8Array(32);
crypto.getRandomValues(randombytes);
var k = '';
for (var i = 0; i < 32; i++) {
var idx = Math.floor(randombytes[i] * alphabet.length / 256);
k += alphabet.charAt(idx);
}
return k;
}

@Tangent128
Copy link

A padding oracle, as I understand, is an attacker-accessible service that is in possession of the decryption key; an attacker can request the oracle attempt to decrypt an arbitrary message, at which point flaws in the oracle leak some information about the reason decryption failed.

Can you describe where this oracle exists in hardbin?

@Tangent128
Copy link

Clarifying what I mean with an example:

A link to a hardbin paste looks like http://localhost:8080/ipfs/QmQ1cv4uYUSZt8UTh6DC6EqAZPgxY6ZrgnQuAgzxmbgS2h/#74pTFAWgkPw6Xe6EiFHXXuy8karuW6xB

The presumption is that the link is already conveyed over a secure channel, as otherwise an attacker can MITM and read or substitute the link as they please.

After that point: QmQ1cv4uYUSZt8UTh6DC6EqAZPgxY6ZrgnQuAgzxmbgS2h is a Merkle hash covering the ciphertext, as well as the code which will be used to decrypt it. 74pTFAWgkPw6Xe6EiFHXXuy8karuW6xB is the (as you noted, weaker than it should be) key to decrypt the ciphertext; this is passed in the URL fragment, so the gateway and IPFS network as a whole never see it.

The only entities with the key are the sender and recipient, and said key is associated with an immutable ciphertext.

paragonie-scott added a commit to paragonie-scott/hardbin that referenced this issue May 23, 2017
@paragonie-scott
Copy link
Author

I cannot describe a whole-systems attack, but merely was pointing out protocol deficits that should be fixed.

@joey-hypr
Copy link

In the current implementation of hardbin there may be no measurable danger using unauthenticated encryption, however, someone using this code or forking the project for educational or slightly different implementation purposes may end up creating an insecure variation based on this code.

The bigger problem here seems to be the misleading language used to describe the guarantees provided by the security algorithms actually implemented.

@Tangent128
Copy link

OK, I misunderstood the concern. Fair point.

@kpcyrd
Copy link

kpcyrd commented May 23, 2017

There's a different library for the same problem (pastes in ipfs) built by a friend and me. It's based on nacl and shouldn't suffer from those problems: enpaco/spec

Since hackernews is reviewing crypto today, please feel free to give this a look as well. It's currently using plain nacl boxes but we intend to switch to nacl-stream-js since everything else is written to support large binary files as well.

@jes
Copy link
Owner

jes commented May 23, 2017

@paragonie-scott thanks for reporting. Fixed the key generation bug. Also upgraded keyspace to 256 bits as some people were concerned by that.

I'm going to close this issue now as authenticated encryption doesn't make any difference.

If you can get #2 to work I'll happily merge it.

Cheers.

@jes jes closed this as completed May 23, 2017
@jes
Copy link
Owner

jes commented May 23, 2017

Oh, and one more thing.

misleading language used to describe the guarantees provided by the security algorithms actually implemented.

While the "world's most secure" part was clearly tongue-in-cheek:

Every other encrypted pastebin in the world, that I am aware of, is very insecure in the face of a malicious (e.g. compromised) server. It can ship malicious code that will exfiltrate the plaintext or key at decryption time. IPFS solves that.

@paragonie-scott
Copy link
Author

I will totally grant that IPFS offers a better security proposition than "blindly trust the server".

A note about key sizes: If I can fix whatever bugs are causing #2 to break (likely something that is causing CryptoJS to load the wrong things; I may need to build a custom roll-up?), it is worth considering adopting a 512-bit key. (The first 256 bits would be for AES, the latter 256 for HMAC.)

This isn't strictly necessary; I'm not aware of any cross-protocol attacks possible for using the same key for AES-256-CBC and HMAC-SHA256. I'm satisfied if the answer to this is, "It has been considered, and the answer is 'No'."

In defuse/php-encryption, we use salted HKDF-HMAC-SHA256 to split the given key into two keys: one for AES, and one for HMAC. The key users store is only 256 bits. If you're more comfortable with the HKDF approach, I'll see what I can do to make that happen as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants