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

Encrypted/maced Unix Domain Socket #555

Closed
maxtaco opened this issue Jul 23, 2015 · 24 comments
Closed

Encrypted/maced Unix Domain Socket #555

maxtaco opened this issue Jul 23, 2015 · 24 comments

Comments

@maxtaco
Copy link
Contributor

maxtaco commented Jul 23, 2015

Instead of maxtaco/go-framed-msgpack-rpc#6, we should make a generic wrapper around a Unix Domain socket, that when seeded with a shared key, will present an encrypted/mac'ed stream to the OS, that the application can read/write as if it were plaintext. We'll need AEAD (not AE) to prevent replay attacks.

@akalin-keybase
Copy link
Contributor

I wonder if we can adapt the code in https://www.tarsnap.com/spiped.html to do the crypto for us.

@maxtaco
Copy link
Contributor Author

maxtaco commented Jul 24, 2015

I figured we'd have to do it in Go? I think we're talking about a ~20 line wrapper around Go's AES-GCM implementation.

@akalin-keybase
Copy link
Contributor

Ah, ok, that's easy enough.

@maxtaco
Copy link
Contributor Author

maxtaco commented Jul 24, 2015

(I could be way wrong though!)

@maxtaco
Copy link
Contributor Author

maxtaco commented Jul 24, 2015

We'd want to wrap the net.Conn class. A thought was that the wire protocol would look like:

┌───────────────┬─────────────────────────────────┐
│Frame (4-byte) │  Encrypted data (variable len)  │
└───────────────┴─────────────────────────────────┘

The encrypted data is of the form:

┌──────────────────┬──────────────────────────────┐
│ Nonce (12-byte)  │          Ciphertext          │
└──────────────────┴──────────────────────────────┘

And the plaintext is of the form:

┌───────────────┬─────────────────────────────────┐
│Seqno (4-byte) │     payload (variable len)      │
└───────────────┴─────────────────────────────────┘

The receiver will reject messages that fail to decrypt or are out-of-order.

@maxtaco
Copy link
Contributor Author

maxtaco commented Jul 24, 2015

BTW, we'll have to do buffering on either end, so it might not be ~20 lines. Maybe ~100 lines.

@patrickxb
Copy link
Contributor

bufio.NewReadWriter is only one line :)

libkb/stream.go uses bufio for buffering.

@maxtaco
Copy link
Contributor Author

maxtaco commented Jul 24, 2015

Should have remembered that, whoops

@gabriel
Copy link
Contributor

gabriel commented Jul 24, 2015

You can assume you'll have a shared secret key (32 bytes?). I have that part working in a branch, available from the GlobalContext (somewhere) right after app start.

@maxtaco
Copy link
Contributor Author

maxtaco commented Jul 24, 2015

Correct.

@gabriel
Copy link
Contributor

gabriel commented Jul 24, 2015

We could use NaCl (libsodium's) choice for AEAD? https://download.libsodium.org/doc/secret-key_cryptography/aead.html

I have a libsodium Objective-C wrapper that can do this: https://github.com/gabriel/NAChloride/blob/master/README.md#aead

@maxtaco
Copy link
Contributor Author

maxtaco commented Jul 24, 2015

It's another option, but will be slower than AES-GCM since we'll almost certainly have hardware acceleration.

@gabriel
Copy link
Contributor

gabriel commented Jul 24, 2015

is there a good C library for aes gcm? openssl (eww)?

@akalin-keybase
Copy link
Contributor

nacl does aes gcm, I believe.

@akalin-keybase
Copy link
Contributor

Hunh, maybe not. (Looking at the web page, I guess it's been TODO for a while.)

@maxtaco
Copy link
Contributor Author

maxtaco commented Jul 24, 2015

Oh, you mean on the Objective C side. Shoot, forgot to consider this, good point. openssl definitely implements it, and there are some snippets online.

@gabriel
Copy link
Contributor

gabriel commented Jul 24, 2015

the thought of using openssl or snippets online makes me nervous... If there is a widely used clean/portable implementation with minimal dependencies in a github for aes128gcm them I'm all for it.

can we do chacha/poly first and then switch if we see perf issues? I don't feel strong about this though. It's just that nacl/libsodium is such a nice library :)

@maxtaco
Copy link
Contributor Author

maxtaco commented Jul 24, 2015

Sure, let's wait until libnacl or libsodium implements it. (Or use SecretBox in the mean time, sounds good with me)

@maxtaco maxtaco added this to the Milestone 3 milestone Jul 24, 2015
@maxtaco
Copy link
Contributor Author

maxtaco commented Jul 24, 2015

(Btw, I'm assigning this stuff to M3. I think we can launch the private beta without it)

@gabriel
Copy link
Contributor

gabriel commented Jul 30, 2015

Seems like we are framing data again.

Why don't we remove the framing option from the rpc and do it at a higher level and simplify everything since we don't need "streaming".

Send:

  • Encode msgpack rpc message to bytes
  • Encrypt bytes
  • Send header: prefix 0xE255 (2 bytes) + numbytes (4 bytes) + bytes

Receive:

  • Receive header (6 bytes): Check prefix. Get numbytes.
  • Read numbytes
  • Decrypt bytes
  • Decode bytes to msgpack rpc message

@maxtaco
Copy link
Contributor Author

maxtaco commented Jul 30, 2015

I like the layering of data and framing at each layer, I think there's lots of good precedent for that. E.g., https is HTTP over TLS over TCP over IP. Each layer has its own framing. I think it's nice to use OSI-style layering here because we want much the code to be the same on platforms that don't use the encryption/MAC'ing. So for linux, the Msgpack layer is the same as on OSX, but on OSX, we have the NaCl encapsulation.

BTW, your proposal leaves out sequence numbers, but we'll definitely need sequence numbers or else are susceptible to replay and reordering attacks.

@gabriel
Copy link
Contributor

gabriel commented Jul 30, 2015

ok sure.. it always bugged me a little bit that we use a msgpack encoded
number instead of a 4 byte int, we still want to keep that? especially

yeah assume when i said encrypt/decrypt steps it was AEAD and the AD is
seqno?

On Wed, Jul 29, 2015 at 5:25 PM, Maxwell Krohn [email protected]
wrote:

I like the layering of data and framing at each layer, I think there's
lots of good precedent for that. E.g., https is HTTP over TLS over TCP over
IP. Each layer has its own framing. I think it's nice to use OSI-style
layering here because we want much the code to be the same on platforms
that don't use the encryption/MAC'ing. So for linux, the Msgpack layer is
the same as on OSX, but on OSX, we have the NaCl encapsulation.

BTW, your proposal leaves out sequence numbers, but we'll definitely need
sequence numbers or else are susceptible to replay and reordering attacks.


Reply to this email directly or view it on GitHub
#555 (comment).

@gabriel
Copy link
Contributor

gabriel commented Jul 30, 2015

oops premature hit send...

msgpack encoded number for the frame header instead of a fixed byte...

also is it possible to open a stream in the middle (corrupted somehow,
buffer left over or something) and then read a random number and then be
stuck with a large frame size just reading random data? (this would be an
argument for a prefix in the header, magic number or something)

On Thu, Jul 30, 2015 at 9:02 AM, Gabriel Handford [email protected]
wrote:

ok sure.. it always bugged me a little bit that we use a msgpack encoded
number instead of a 4 byte int, we still want to keep that? especially

yeah assume when i said encrypt/decrypt steps it was AEAD and the AD is
seqno?

On Wed, Jul 29, 2015 at 5:25 PM, Maxwell Krohn [email protected]
wrote:

I like the layering of data and framing at each layer, I think there's
lots of good precedent for that. E.g., https is HTTP over TLS over TCP over
IP. Each layer has its own framing. I think it's nice to use OSI-style
layering here because we want much the code to be the same on platforms
that don't use the encryption/MAC'ing. So for linux, the Msgpack layer is
the same as on OSX, but on OSX, we have the NaCl encapsulation.

BTW, your proposal leaves out sequence numbers, but we'll definitely need
sequence numbers or else are susceptible to replay and reordering attacks.


Reply to this email directly or view it on GitHub
#555 (comment).

@maxtaco
Copy link
Contributor Author

maxtaco commented Jul 31, 2015

We can change the framing on msgpack-rpc if you feel strongly. It would be a bit of a hairy rollout, but we can do it.

As for getting off sync in the stream, probably we should have a maximum packet size, and if that's exceeded, than kill and drop the connection. And similarly, kill and drop the connection if there's a MAC error. The chances of getting off sync and reading a packet that MAC correctly are 0. So I don't think we'd need to use a magic header.

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

No branches or pull requests

4 participants