lobby server<->client protocol encryption - Page 2

lobby server<->client protocol encryption

For the discussion of infrastructure improvements and changes.

Moderator: Moderators

Kloot
Spring Developer
Posts: 1867
Joined: 08 Oct 2006, 16:58

Re: lobby server<->client protocol encryption

Post by Kloot »

This is now fully implemented and will go live with the next server update.

For the time being secure sessions are optional (the server can configure whether use of encryption is enforced or left to clients), but the switch may be flipped to "mandatory" at any point. Hopefully sooner, probably later.

Early adopters will want to review the new crypto-related protocol commands here (search for "Crypto:").
User avatar
PicassoCT
Journeywar Developer & Mapper
Posts: 10450
Joined: 24 Jan 2006, 21:12

Re: lobby server<->client protocol encryption

Post by PicassoCT »

Suck up to this clan [NSA]! Cheating just got a lot harder.

But yeah, cool to have it. Also i vote for keeping this optional.
Some players in some country would have to jump through a lot of slow down hoops to play spring, if encryption was enforced.
User avatar
Peet
Malcontent
Posts: 4383
Joined: 27 Feb 2006, 22:04

Re: lobby server<->client protocol encryption

Post by Peet »

Kloot wrote: the switch may be flipped to "mandatory" at any point. Hopefully sooner, probably later.
how will this work with the irc bridge?
Kloot
Spring Developer
Posts: 1867
Joined: 08 Oct 2006, 16:58

Re: lobby server<->client protocol encryption

Post by Kloot »

It won't.

The bridge server would have to be modified (by someone other than me) to make it transparently handle the crypto protocol for proxied lobby connections. Then IRC users could host their own local bridges (added benefit: one less service running on springrts.com) and point their clients to those to get secure sessions. I don't see that happening though.
abma
Spring Developer
Posts: 3798
Joined: 01 Jun 2009, 00:08

Re: lobby server<->client protocol encryption

Post by abma »

i've restarted the lobby server and it contains now the patches which allows encrypted connections.
dansan
Server Owner & Developer
Posts: 1203
Joined: 29 May 2010, 23:40

Re: lobby server<->client protocol encryption

Post by dansan »

I just was a bit sad, that old passwords are stored unencrypted in the database and poked a bit at the source. Then I realized, that a migration is already in place for new-clients(TM) - nice!

But why not encrypt the passwords of old client as well? IMO passwords should never be saved unhashed, because a successful attacker would get hold of all passwords at once. It doesn't matter, that the passwords were previously transmitted unencrypted. An eavesdropper would only capture those currently used passwords and not all.

If I comment out the checks for legacy connections, it seamlessly converts the passwords of all clients.
Attachments
quickndirty_encrypt_old_clients_pw.patch.txt
(1.65 KiB) Downloaded 15 times
User avatar
Silentwings
Posts: 3720
Joined: 25 Oct 2008, 00:23

Re: lobby server<->client protocol encryption

Post by Silentwings »

+1 to that
Kloot
Spring Developer
Posts: 1867
Joined: 08 Oct 2006, 16:58

Re: lobby server<->client protocol encryption

Post by Kloot »

So you want to convert the server's old MD5(s) hashes to SHA(MD5(s) + SALT)'s?

That could be done but it complicates matters a little bit if a client later started using secure sessions: either the client would then have to send ENCRYPT(MD5(s)) to be able to login, or the server would have to perform MD5(DECRYPT(s)) for that client. More importantly though this would promote continued reliance on MD5, which IMO is not a good idea.
dansan
Server Owner & Developer
Posts: 1203
Joined: 29 May 2010, 23:40

Re: lobby server<->client protocol encryption

Post by dansan »

Uh oh - I didn't pay attention. I saw only the b64(s) - no hashing at all, but md5 is done... please ignore my post! :oops:
User avatar
aegis
Posts: 2456
Joined: 11 Jul 2007, 17:47

Re: lobby server<->client protocol encryption

Post by aegis »

TL;DR: Great intentions, but use TLS and bcrypt/scrypt instead.
Context: my day job is software security.

Password hashing
If you have a bunch of spare bitcoin miners to throw at the DB, yes.
This is an easy mistake to make. bcrypt and scrypt are tunable algorithms. The needs of a password hash are different from hashes those in Litecoin/Altcoin mining, so you tune the algorithm differently. scrypt-based cryptocurrencies are intentionally very difficult to mine. You should use bcrypt or scrypt for password hashing, and set the work factor for your use case. The work factor also doesn't need to be updated over time as computers are faster, as it's more like a deadline than a set number of iterations.

Symmetric crypto
DES and RC4 are the broken algorithms. 3DES isn't as broken but it's gross so you shouldn't use it anyway. AES is probably what you want, but *don't use the primitives yourself*.

Transport
This appears to be a crypto transport layer from scratch. Do not do this. You will screw it up. Use TLS. TLS is battle hardened. It's fine to not use the CA infrastructure, just make sure you have a sane way of distributing the server's public key (I kinda agree with the idea of throwing it on a server under https://, just remember phpbb is running on springrts.com and phpbb is historically one of the most compromised pieces of software on the internet). Also, as this is home-rolled, *every single client* implementing it will need to do so without any mistakes. With TLS, you have far fewer variables (yet it's still possible to do it wrong).

Actual problems I observed
CryptoHandler.py:17 This looks gross. Can these imports fail? What happens when you don't set padding/signature schemes?
Client.py:188 This seems vulnerable to a byte-trickling denial of service. msg_length_limit is not checked until after decryption, so an attacker can trickle garbage bytes in under the rate limit without sending a newline to cause a potentially infinite amount of memory allocation. This works with or without crypto, it's just something I noticed.
It looks like the AES helper is vulnerable to CBC bit flipping, which allows you to blindly modify messages (use GCM instead of CBC).
I also wouldn't be surprised if this is vulnerable to a padding oracle (which in this case would allow someone sniffing traffic to convince the server to decrypt traffic by brute-forcing valid padding).

Because some basic errors were made here, I'm going to go ahead and say both the implementer and code reviewers are not sufficient domain experts to be shipping home-grown crypto solutions. I only gave this a cursory review and fixing these problems should *not* qualify as completely securing the protocol and implementation. This implementation should be removed and replaced a TLS wrapped connection.

In case you don't believe/trust me, google "roll my own crypto" or read here:
https://security.stackexchange.com/ques ... ll-our-own
Bruce Scheier wrote:Anyone, from the most clueless amateur to the best cryptographer, can create an algorithm that he himself can't break. It's not even hard. What is hard is creating an algorithm that no one else can break, even after years of analysis. And the only way to prove that is to subject the algorithm to years of analysis by the best cryptographers around.
*slowly slides back into the shadows*
Kloot
Spring Developer
Posts: 1867
Joined: 08 Oct 2006, 16:58

Re: lobby server<->client protocol encryption

Post by Kloot »

Thanks for your feedback, but you may have jumped the gun a little bit.
aegis wrote:This is an easy mistake to make. bcrypt and scrypt are tunable algorithms. The needs of a password hash are different from hashes those in Litecoin/Altcoin mining, so you tune the algorithm differently.
The comment was in reference to an attacker's ability to crack (a DB of) non-iterated SHA hashes, not *crypt.

Nevertheless, switching SQLUsers to bcrypt is still one of my todo's.
aegis wrote: This appears to be a crypto transport layer from scratch. Do not do this. You will screw it up. Use TLS. TLS is battle hardened.
After BEAST, POODLE, CRIME, etc, that might be a premature statement.

Your favorite N-letter agency probably has a few more undisclosed attacks in its catalogue, because TLS is a complex beast.
aegis wrote:CryptoHandler.py:17 What happens when you don't set padding/signature schemes?
Read http://cseweb.ucsd.edu/~mihir/papers/oae.pdf (TLDR: plain RSA is malleable without it).
aegis wrote:Client.py:188 This seems vulnerable to a byte-trickling denial of service. msg_length_limit is not checked until after decryption, so an attacker can trickle garbage bytes in under the rate limit without sending a newline to cause a potentially infinite amount of memory allocation. This works with or without crypto, it's just something I noticed.
Well, that piece of logic is yours: https://github.com/lunixbochs/uberserve ... nt.py#L108

I'm not assigning blame, just pointing out how being removed from your own code for a while causes you to spot such things more easily.
aegis wrote:It looks like the AES helper is vulnerable to CBC bit flipping, which allows you to blindly modify messages (use GCM instead of CBC).

I also wouldn't be surprised if this is vulnerable to a padding oracle (which in this case would allow someone sniffing traffic to convince the server to decrypt traffic by brute-forcing valid padding).
This came up before.

Padding oracles require a server to leak data about the decryption state, which uberserver never does. (You also need to be able to spam it without having your connection closed on you.)

The server also supports HMAC's, so if you mess with a single byte your crafted message will fail to authenticate before being decrypted and goes straight to /dev/null.
aegis wrote:
In case you don't believe/trust me, google "roll my own crypto" or read here:
https://security.stackexchange.com/ques ... ll-our-own
Not that I question the truth of this (being intimately familiar with it) but the "don't roll your own" mantra typically pertains to ciphers, though protocols seem to be just as hard.
User avatar
aegis
Posts: 2456
Joined: 11 Jul 2007, 17:47

Re: lobby server<->client protocol encryption

Post by aegis »

If I'm coming off as harsh/aggressive, it's nothing to do with you/Spring. I'm trying to strike a nerve because this is crypto. You did quite a few things right, but screwing up crypto code is one of the easiest things for programmers of any skill level to do.
Kloot wrote:The comment was in reference to an attacker's ability to crack (a DB of) non-iterated SHA hashes, not *crypt.
Oops, was reading the thread backwards and mixed cause/effect there :)
Kloot wrote:I'm not assigning blame, just pointing out how being removed from your own code for a while causes you to spot such things more easily.
Yeah, that line is probably at least 6 years old. My comment on it was tangental to the protocol, hence the "applies with or without crypto". I honestly think uberserver's a bit of a mess and wouldn't mind chucking it and rewriting with a better event framework or in something like Go ;). I think we should keep this conversation about the crypto though, not about us.
Kloot wrote:After BEAST, POODLE, CRIME, etc, that might be a premature statement.
BEAST was "a long-known cipher block chaining (CBC) vulnerability in TLS 1.0" before it was exploited.
POODLE is a padding oracle on SSL v3.0

TLS 1.1 was defined in 2006, so these are more an artifact with legacy systems requiring TLS 1.0 and SSLv3 to still be supported than an actual problem with the current protocol.

CRIME/BREACH are chosen-plaintext attacks against compression, and not TLS-specific. Uberserver would be just as vulnerable as HTTP if it compressed before encrypting (would need a chosen-plaintext attack, but there are definitely messages you can send to the server that end up on other clients).

Code: Select all

        if (our_mac == msg_mac):
            return (self.decode_decrypt_bytes(enc_msg, null_decode))
Non-constant-time comparison is remotely exploitable. I recognize a disconnect or rekey could make a replay not applicable, but there's no point in leaving things like this open when the fix is so easy.

The HMAC key is also the same for both client and server here, so you can replay messages sent from either side to the other side. This also makes commands like FORGEMSG scary, because they give you valid ciphertext you can send the other direction.

I missed the per-message HMAC in my original skim over the source, so that's my bad. Either way, sheesh, the minimum message size is 110 bytes? Even if I just send a PING\n? Does the HMAC here prevent replay attacks? You're probably better off with AES-GCM (which addresses both of these) than AES-CBC+HMAC, especially considering #122.

What about forward secrecy?

Anyway, I think I used too many words to say "How is a 1200 line patch implementing its own crypto protocol not immediately scary?"
MetalSucker
Posts: 98
Joined: 22 Sep 2014, 20:29

Re: lobby server<->client protocol encryption

Post by MetalSucker »

Your favorite N-letter agency probably has a few more undisclosed attacks in its catalogue, because TLS is a complex beast.
You mean S.H.I.E.L.D., right?

This is a case of https://en.wikipedia.org/wiki/Not_invented_here

Whatever aegis said ++
Kloot
Spring Developer
Posts: 1867
Joined: 08 Oct 2006, 16:58

Re: lobby server<->client protocol encryption

Post by Kloot »

aegis wrote:Non-constant-time comparison is remotely exploitable. I recognize a disconnect or rekey could make a replay not applicable, but there's no point in leaving things like this open when the fix is so easy.
Done (replays were already impossible however).
aegis wrote:The HMAC key is also the same for both client and server here, so you can replay messages sent from either side to the other side. ... Does the HMAC here prevent replay attacks?
Nope, but the mandatory timestamps do.
aegis wrote:Either way, sheesh, the minimum message size is 110 bytes? Even if I just send a PING\n? ... You're probably better off with AES-GCM (which addresses both of these) than AES-CBC+HMAC, especially considering #122.
Unfortunately GCM isn't supported by PyCrypto.

The best option for clients to kill the overhead would be to encrypt a buffer of commands, which the protocol is designed for.
aegis wrote:What about forward secrecy?
All session keys are cryptographically random, not derived from another, and can be renegotiated. (Obviously clients other than TestClient will have to be trusted to get this right.)

aegis wrote: If I'm coming off as harsh/aggressive, it's nothing to do with you/Spring. I'm trying to strike a nerve because this is crypto. You did quite a few things right, but screwing up crypto code is one of the easiest things for programmers of any skill level to do.
I know and I don't take any offensive because this is clearly coming from someone knowledgable, but would you mind at least reading the code (closer to 600 lines excluding whitespace ;)) comprehensively before striking more?
aegis wrote: Anyway, I think I used too many words to say "How is a 1200 line patch implementing its own crypto protocol not immediately scary?"
Serious question: considering Heartbleed, do you still trust the entire TLS implementation in openssl, which (despite having absolute experts work on it) by all accounts is a really scary giant ball of spaghetti-C?

MetalSucker wrote:Whatever aegis said ++
Contribute meaningfully or not at all, mkay?
MetalSucker
Posts: 98
Joined: 22 Sep 2014, 20:29

Re: lobby server<->client protocol encryption

Post by MetalSucker »

It is highly meaningful. Don't roll your own. Ever.
User avatar
aegis
Posts: 2456
Joined: 11 Jul 2007, 17:47

Re: lobby server<->client protocol encryption

Post by aegis »

Aw, the PyCrypto AES-GCM stuff is just in the alpha 2.7 release (2.7a1).
Kloot wrote:Serious question: considering Heartbleed, do you still trust the entire TLS implementation in openssl, which (despite having absolute experts work on it) by all accounts is a really scary giant ball of spaghetti-C?
Heartbleed was pretty much inexcusable, which is why LibreSSL exists. There's also a Frama-C proven version of PolarSSL, and Go's TLS stack is mostly reasonable.

The core TLS 1.2 implementation isn't what I'm worried about there so much as all the extensions and protocols slapped on top. Cryptographers have been banging away on all the TLS stacks for a long time and have mostly been finding bugs in the outdated protocols. Heartbleed was a weird exception, and iirc was mostly due to a badly managed feature push/release.
Kloot wrote:All session keys are cryptographically random, not derived from another, and can be renegotiated.
The problem is in the session key exchange. If you exchange the temp session key using a permanent asymmetric private key, recovery of the private key allows someone who sniffed the key exchange to decrypt the whole session.

I think Perfect Forward Secrecy is definitely worth a look here: https://en.wikipedia.org/wiki/Forward_secrecy

Basically, session keys are encrypted by an ephemeral keypair and the primary keypair is just used to authenticate. This is important if you were following the Lavabit case, where they subpoena'd the private key.

I did read through all the code, I didn't completely grasp the control flow the first time. I was reading the patch from top to bottom and looking directly at CryptoHandler.
abma
Spring Developer
Posts: 3798
Joined: 01 Jun 2009, 00:08

Re: lobby server<->client protocol encryption

Post by abma »

@kloot, aegis:

as you both seems to have experience with encryption: can you suggest a libary which can be used to implement a client which supports the encryption?

current whishlist is: http://springrts.com/mantis/view.php?id=4647#c13922
- AES, RSA and HMAC
- c or c++ api
- license is gpl2+ / allows static linking
- should be in debian/ubuntu repositories at least
- should be a lib in mxe: http://mxe.cc/
- no / few external dependencies
Kloot
Spring Developer
Posts: 1867
Joined: 08 Oct 2006, 16:58

Re: lobby server<->client protocol encryption

Post by Kloot »

I hate to recommend it, but that wishlist probably only leaves openssl (gnutls is LGPL, libressl is still BSD-only).

On the topic of PFS which I missed earlier: this could be as trivial as periodically restarting the server with fresh keys (or regenerating them on the fly and pushing NEWPUBLICKEY messages), with the period length depending on your definition of ephemeral. The only minor practicality is that the period would need to have some conservative lower bound to avoid draining the server's entropy pool.
abma
Spring Developer
Posts: 3798
Joined: 01 Jun 2009, 00:08

Re: lobby server<->client protocol encryption

Post by abma »

Kloot wrote:I hate to recommend it, but that wishlist probably only leaves openssl (gnutls is LGPL, libressl is still BSD-only).
what about libgcrypt?

- license is gpl2+ / allows static linking
i meant either the license is gpl2+ or allows static linking to gpl2 software. if i understood lgpl2 correctly, then it allows static linkage to gpl application (a user must have the opportunity that he can "relink" the application)

so a bsd-licensed library should be allowed, too.
Post Reply

Return to “Infrastructure Development”