[kewde]

Hi,

If you want users to take your comments seriously then you should be a bit more specific about what's scare. CBC and ECDH are the basis of most modern end-to-end encrypted messaging schemes.

I agree with the key exchange, it doesn't have a proper ratchet, it's on our to-do list.

[tptacek]

I'm really, really comfortable with who does and doesn't take my comments on this particular subject seriously. If you rephrase your comment in the form of a question, I'll respond.

[kewde]

Hi,

I didn't mean to come off as rude, it's a bit late and I'm quick to the trigger sometimes.

What are your concerns with our scheme? Do you have any suggestions on improving it?

We really need to implement a key ratchet, that's a sure thing.

[tptacek]

It's 2002-grade crypto primitives, hand-rolled, written in C++, apparently on top of OpenSSL's primitives.

Nobody is going to be able to sanity check this code "in real time" in response to message board comments. But I skimmed it, and I see concerns right away. For instance, it looks like the CBC IV isn't included in the MAC calculation.

In terms of primitive selections:

* CBC+HMAC is fine if you're very careful. I'd certainly do that before I tried to hand-code a real AEAD. But then, see above! A modern protocol would use either a hermetically sealed AEAD construction like GCM, or, even easier, use ChaPoly, as provided in TweetNacl.

* I can't tell what curve this is based on, but I'm guessing because this is all bitcoin related it's secp256k1. New protocols shouldn't use secp256k1, if only because it's not misuse-resistant. I'm assuming this project inherits its secp256k1 code from wherever bitcoin gets it, but anyone else who tried to interoperate with ShadowChat has to carefully avoid invalid curve attacks as well.

* ECDH is fine. Whatever. The problem is secp256k1, not ECDH.

* But ECDSA is not fine. It relies on a random nonce and explodes like a pipe bomb if so much as a single bit of the nonce is biased, which is surprisingly hard to get right. It's also difficult to make ECDSA constant time with arbitrary curves.

That's just the really basic stuff, though. Secure messaging systems are surprisingly difficult to get right. You need to understand the underlying crypto primitives very well, but then also a whole mess of almost- domain-specific stuff about messaging protocols with particular threat models.

This is not a good thing to DIY.

Later

And, I mean, I have structural concerns with this C++, too. For instance: there's a place that passes an integer computation to std::vector's resize, then memcpy's into it on the assumption that the computation didn't shrink the vector. Is this a real problem? Fuck if I know. You know what it costs per day to really audit C++ code like this?

Later Later

Sorry, I forgot: compressing plaintext creates traffic-analytic side channels. Back in 2002, people thought it was a good idea to compress plaintext, because it destroyed structure and made some attacks harder. But then CRIME was published and now we all worry about compression --- also, it turns out, the code for the exploits compression was supposed to stop gets trickier, but not impossible: see "Dancing On The Lip Of A Volcano" for more.

Also: how much of this code is your project's, and how much is inherited?

[kewde]

Hi tptacek,

We know that thorough review does not happen instantly. I want to let you know that we have bounties and funds available for doing code review. https://shadowproject.io/en/bounties

* That's indeed true, we will be including the IV in the HMAC. Will be patched in our next version. Thank you!

* ChaPoly by TweetNaCl - dearly noted. The thing is, libs need compatibility with our curve. TweetNaCl only supports Ed25519 :/

* secp256k1 is correct.

* Do you have any references to the "exploding like a pipe bomb" problem? We're using the ECDSA from the bitcoins secp256k1 library - which is constant time and should be secure enough. Function: RecoverCompact()

We initially paid for a cryptographic review but the guy, whom we know by real name, disappeared from the planet. We're scouting for cryptographers willing to do code reviews, for a payment ofcourse.

If you're capable and interested then you should reach out to us on GitHub or Slack!

We do appreciate you doing this in your free time, so we're sending Bitcoin tips to whoever makes good suggestions. Please post your Bitcoin address if you'd like to receive some.

---

Most code is inherited from Bitcoin, custom code with cryptography: smessage.cpp, ringsig.cpp and stealth.cpp

[tptacek]

People. Come on.

It is OK that you don't know the best curve to use, or that an IV needs to be included in your MAC.

What is not OK is shipping privacy code to users without understanding this stuff. You don't get to learn this stuff on the job. Sorry. I know it's not fair. But to do otherwise would be even less fair to your users.

First be sure of your crypto. Then set up the bug bounty.

You can donate mine, for the MAC bug (which is severe), to Partners In Health.