hematite_server icon indicating copy to clipboard operation
hematite_server copied to clipboard

Impl symmetric AES encryption on top of TcpStream

Open s1gtrap opened this issue 9 years ago • 7 comments

  • The server will now generate a 1024-bit certificate and corresponding private key at startup which is used during the encryption handshake (the certificates public key is sent to the client after which the private key is used to decrypt the shared secret).
  • Upon success the TcpStream is wrapped in the SymmStream type, after which all subsequent reads and writes are passed through two separate 128-bit AES ciphers (decrypter and encrypter respectively.)

The clients response is not verified atm, since the login::clientbound::Disconnect packet isn't implemented (guessing it won't be long, since @toqueteos has made efforts towards encoding ChatJson?)

The nonce isn't randomly generated either, which I suppose isn't strictly necessary, but would be nice to have. It's also how cuberite does it (kind of.)

I'm also not happy with overruling the compiler in terms of Send and Sync safety (see src/vanilla/server.rs), but I'm under the impression that the openssl crate is as thread safe as it can get, and that marking PKeys as such is a non-issue (I could totally be wrong, I'm not particularly well versed in Rust.)

Still to do:

  • [ ] Verify verify_token
  • [ ] Generate verify_token as opposed to hardcoding it
  • [x] Resolve thread safety issues (if any?)

s1gtrap avatar Jan 06 '16 03:01 s1gtrap

I'll get on this as soon as I can. read_exact can easily be replaced with read_exactly and clone_from_slices implementation is also fairly straight forward.

s1gtrap avatar Jan 06 '16 05:01 s1gtrap

Welcom to hematite_server @bheart and thanks for sending in this PR!

You are right, login::clientbound::Disconnect is not implemented on master. It is implemented on ~~#97~~ #106 which hasn't been merged yet :sob:

Apparently io::copy can be used with slices too. Would that help?

We probably should use take instead of read_exactly this is something I always forget to open an issue for...

toqueteos avatar Jan 06 '16 11:01 toqueteos

Oh, I forgot! It'll would be a good thing to enable encryption iff online-mode is true (#44).

toqueteos avatar Jan 06 '16 11:01 toqueteos

Wasn't aware of io::copy! Thanks! The thing about read_exact/read_exactly is that they both error if they couldn't read the specified amount (right?), whereas an io::Take (when read_to_end) will happily return less. ~~Not sure if that's desirable tbh.~~

s1gtrap avatar Jan 06 '16 14:01 s1gtrap

Just to be sure, when you say read_exactly are you referencing the one that's implemented on hematite_server/util?

toqueteos avatar Jan 06 '16 17:01 toqueteos

Yea, don't read_exact and read_exactly work in pretty much the same way?

Regardless, using an io::Take does make more sense, all things considered. There are valid reasons for why the bytes read can be less than the output buffer's size after all.

s1gtrap avatar Jan 06 '16 18:01 s1gtrap

@bheart Yeah, read_exactly is inspired/copied so we can use it on stable. io::Take makes sense but maybe for some specific cases we'll need the exact version.

toqueteos avatar Jan 06 '16 23:01 toqueteos