irungentoo / toxcore

The future of online communications.
https://tox.chat/
GNU General Public License v3.0
8.74k stars 1.28k forks source link

Use of nonce #1313

Closed arucard21 closed 7 years ago

arucard21 commented 9 years ago

While looking through the documentation, I noticed that nonce's are used a lot. From what I understand about them, these are intended to make replay attacks impossible. However, it seems like the nonce used in the encrypted parts of the data packet are sent in the same data packet. So while the ciphertext will be changed because of the nonce, the corresponding nonce is provided along with it. This means that you can still replay that data packet, so the nonce doesn't really improve the security. Is this really the case or did I misunderstand something?

irungentoo commented 9 years ago

nonces are just numbers used as a second input to the stream function. As long as the first input (the key) is secret and the second input is always different it will be secure.

arucard21 commented 9 years ago

Yeah, but what I meant was that the nonce doesn't seem to be providing any useful value. From what I can tell, the data packets would be equally as secure if you didn't use the nonce.

codedust commented 9 years ago

@arucard21 @irungentoo The security model of NaCL requires different nonces for different messages. But this is not the primary idea behind them. We need nonces to avoid replay attacks by a MITM. Therefore @arucard21 is right with his first post.

We have to assure the response to any message contains information from the nonce sent with our message (typically this is achieved by using NONCE+1 as the nonce in the response). This effectively prevents a MITM to resend a response recorded in a previous session.

irungentoo commented 9 years ago

There are many ways to avoid replay attacks that do not use nonces.

Sticking a ping number in the encrypted section also prevents MITM.

arucard21 commented 9 years ago

That's true, but only the Ping datapacket has a Ping ID. So the other datapackets are still vulnerable to MitM.

I suppose Sendback Data also prevents MitM, but why would you use a separate (i think custom-made) mechanism for this, when you already have a standardized mechanism for this with the nonces. And responding with NONCE+1 would actually mean you don't need to send a nonce in the response, making the datapackets smaller. Might save some bandwidth too.

irungentoo commented 9 years ago

The reason it's like this is because it made the code much simpler to write which means less potential security issues.

I'll look into shortening these packets next time I decide to improve the DHT. Their format has not changed in a while and yes they could be shorter.

If you can find a place where an actual replay attack can be performed please tell me.

codedust commented 9 years ago

I don't really understand the argument that putting a ping number (which technically is a nonce) into the encrypted section would be easier than using NaCls default location for this purpose. Anyway, making sure that any response is based on the nonce (-> NONCE+n) we originally sent to our communication partner would significantly reduce the risk of replay attacks somewhere in the protocol.

@irungentoo: It would be great to see some documentation of these protocols in the future. This way it's a lot easier to find potential issues with the general design and we could verify that the source code matches the documentation.

irungentoo commented 9 years ago

@codedust The ping number isn't a nonce. The ping number could be the same number more than once. Since the attacker doesn't know the ping number and the chances of it being the same twice are low enough doing a replay attack is pretty much impossible.

arucard21 commented 9 years ago

I think @codedust meant the same thing as what I was trying to say. Namely, that the ping number (as well as the "sendback data") performs the same role as what the nonce is intended for, even if it can not strictly be defined as a nonce. The main difference is that it is used twice, once in the request and once in the response.

While I do agree that simpler code should cause less potential problems, I think that creating your own mechanisms to do so risks far greater problems, especially where security is concerned. It's generally better to use standardized mechanisms from standard libraries (like NaCl). These libraries are used much more widely and will be scrutinized more thoroughly. As a result, they should have much more secure mechanisms (and implementations of those mechanisms) in place than could be developed in Tox itself.

irungentoo commented 9 years ago

I think that creating your own mechanisms to do so risks far greater problems

Ping numbers are a standard way to prevent replay attacks, much more standard than using nonces.

As a result, they should have much more secure mechanisms (and implementations of those mechanisms) in place than could be developed in Tox itself.

Handling nonces in NaCl is done entirely by the user of the library, not by NaCl so this makes no sense at all.

arucard21 commented 9 years ago

I'm not enough of an expert on this matter to say whether ping numbers are also a standard way of preventing replay attacks, but that isn't relevant to what I'm saying. Even if they are a standard method of preventing replay attacks, its actual implementation is entirely custom-built.

While NaCl does seem to leave some of the handling of nonces up to the user of the library, it does require the use of nonces in the crypto_box methods. While it's ultimately up to the user of the library, if you wish to have a secure implementation you should handle nonces as described in the library documentation (in this case the security model documentation that was linked by @codedust ).

Even with well-understood and thoroughly reviewed security mechanisms that are known to be secure by design, there are still many vulnerabilities found because of an incorrect implementation of the mechanism or other limitations in the implementation. This is why you should use security mechanisms that are known to be secure by design and leave as much of their implementation to a specialized library that can be maintained by people more knowledgeable in that field. The parts of the implementation that you do need to implement in your software should conform to the recommendations of that library.

You mentioned that the code would be simpler by using ping numbers instead of nonces for preventing replay attacks. But I think that it actually makes the code more difficult to understand since you now have nonces in your code that have no function beyond satisfying the NaCl requirement of using nonces (a requirement which NaCl has, at least in part, to prevent replay attacks). Additionally, you have ping numbers of which both the software design (in the Tox protocol) and implementation (in toxcore) is specific to Tox, even if it is based on a standard method for preventing replay attacks. Having 2 mechanisms that are intended for the same thing (one of which is broken by design, no less, namely nonces) just seems like it increases the likelihood of security vulnerabilities as well as confuses anyone trying to understand the codebase.

I actually think this goes beyond just preventing replay attacks. It's incredibly difficult to have a secure implementation (which is one of the goals of Tox) and I believe that correct use of well-known security libraries is crucial for achieving this. This would have to be reflected in the design of the Tox protocol as well as its implementation.

irungentoo commented 9 years ago

a requirement which NaCl has, at least in part, to prevent replay attacks

Can you point me to where it says this is the NaCl docs?

codedust commented 9 years ago

NaCl can't prevent replay attacks by itself. It leaves the responsibility of nonce generation to the caller (see http://cr.yp.to/highspeed/coolnacl-20120725.pdf, page 5).

However, I think it's a good idea to use the nonce parameter of NaCl's crypto_box method to store for the nonce we want our communication partner to use for encryption in the response. (Obviously, we have to transfer the nonce we use in the very first request in plaintext.)

arucard21 commented 9 years ago

@codedust that's a good find. It explains things quite well.

I was wondering though, while the ping number does prevent against MitM attacks, it doesn't always prevent replay attacks. With responses, you can't replay an datapacket because the ping ID must match the ping ID that was sent in the request. Presumably, the client won't accept unknown ping ID's, namely ones that have already been matched against a response's ping ID. But with requests, you can replay the request at a later time after the receiver has already sent its response. The receiver should simply accept it as a new request. Or does the receiver also check that ping ID's are always larger than their last ping ID? If so, it would indeed be simpler to only use nonces for this, as suggested in that paper.

If not, the nature of NaCl's security keeps the encrypted part safe and Tox's use of public key as Tox ID means the replayed datapacket will only allow the response to be sent to the original sender of the replayed datapacket. However, the replayability of the datapacket could still have an impact on security. If you consider that the send-nodes (response) datapacket can be up to 3x as large as the get-nodes (request) datapacket, you can use the replayability of the get-nodes request datapacket to execute an asymmetric DoS attack on some DHT nodes. I'm not sure how feasible this is, but it's just an example of what could go wrong.

irungentoo commented 9 years ago

Anyone can send get_node packets though, they wouldn't even have to replay packets to get the same effect.

arucard21 commented 9 years ago

Yeah, but if you send a get_node packet yourself, then the response will also be sent to you. If you replay it, the response will be sent to the original sender of that packet. So if I wanted to execute an asymmetric DoS attack on node X, I'd collect a bunch of requests that were sent by node X (to different nodes) and replay them all at once (and repeat the replay once the response has been sent by the receiving node). This should cause up to 3x as much bandwidth to be generated towards node X as I'm generating with my replayed requests. It can also be executed in a distributed manner, making it easier to attain enough bandwidth for a successful DoS.

irungentoo commented 9 years ago

You could be doing that just by doing a reflection attack, no need for replays.

arucard21 commented 9 years ago

This is indeed very similar to a DRDoS, but instead of spoofing the source IP address, you use the replay vulnerability to reflect the attack to the target address.

But even if a reflection attack is possible, I think a similar attack using replays still needs to be prevented. Otherwise, by that logic people should send their passwords over the internet in plain-text, because someone could use a keylogger on their machine (which is obviously not a good idea).

codedust commented 9 years ago

We should also consider replayed attacks from the future (see https://blog.tox.im/2015/04/01/tox-ftl/). Therefore we should definitely use nonces not once but less than one time. This will hopefully reduce the risk of a successful replay attack before the very first handshake.

arucard21 commented 9 years ago

Hahaha, yeah, that's a definite risk. I love the way the "superfast motor" looks though. Excellent design.

arucard21 commented 9 years ago

It should be possible to prevent replay attacks entirely by sending the nonce as a separate datapacket.This could be done with the following new datapackets, which should be used before the node starts an encrypted interaction with another node:

  1. Send a nonce announcement to another node (unencrypted, but signed). It should contain the node ID, a nonce, an interaction ID and the signature.
  2. Send a nonce acknowledgement (unencrypted, but signed). It should contain the node ID, the interaction ID that's being acknowledged and the signature.

The interaction that follows after this can then be encrypted with the nonce without providing the (full) nonce during the interaction itself. The nonce should be predictably incremented on each use and the recipient must only accept messages encrypted with a nonce value higher than the previously known value. The nonce can be forgotten once the interaction has been completed.

Concurrent interactions can announce different nonces for the same node by using different interaction ID's, but if at all possible each node should maintain a single nonce for each connection to another node. The nonce should remain available for as long as interactions with that node are still ongoing. When all responses to requests to a node have been received, the nonce can be removed for that connection. A new nonce can be announced if more datapackets need to be sent to that node afterwards. If this is possible, the interaction ID is not necessary in the datapacket, since you can simply request the nonce your node uses to connect with the node you wish to interact with (where a new nonce is automatically announced if not available for that node).

Tampering with the signed nonce announcement will be detected when the signature is verified. The signature also ensures that the datapacket was sent by the node it reports to be.

The nonce would be known by each node by simply incrementing it for every message. If this is unreliable (e.g. due to packet loss, packet resending or out-of-order delivery of packets), you could include an increment_count with each datapacket that shows the offset with the original nonce. This should indicate the exact value of the nonce to be used for decryption in a much smaller size. If the size of the datapackets isn't an issue, you can still provide the nonce in the datapacket, as is done now. This might be useful to be able to gradually remove the nonce from other datapacket formats while still giving the added security against replay attacks right away.

The main benefit here is that the request datapackets can no longer be replayed (whereas currently only response datapackets can not be replayed), but a secondary benefit is that you can remove the nonce or replace it with a smaller value in the datapacket.

codedust commented 9 years ago

:+1:

Why not encrypt the nonce announcement and acknowledgement too? Even if it doesn't really add any additional security, it somehow obscures the message a bit. Also, since these messages are sent only once per session, performance shouldn't be an issue here.

arucard21 commented 9 years ago

You can't encrypt the nonce announcement because you don't have anything to encrypt. Everything in it is required to encrypt/decrypt a message, so it needs to be sent in plain-text. It is signed, so it can't be tampered with. So any ciphertext would just be added to the datapacket and wouldn't obscure anything in it.

The nonce acknowledgement can be encrypted, but it doesn't have anything that requires it. The package itself is the acknowledgement, so you only need to have a datapacket type to recognize that it's a nonce acknowledgement, the node ID to verify the signature and of course the signature to ensure the datapacket wasn't tampered with. It could possibly also contain the nonce we sent (or nonce+1), as confirmation that it's the same nonce and to avoid replay attacks.

I think that for the type of additional security you describe, it would be better to provide some kind of channel encryption between the nodes (e.g. TLS/DTLS). This would encrypt all traffic from node to node.

codedust commented 9 years ago

I thought about something like temp_nonce, crypto_box(message, temp_nonce, pk_receiver, sk_sender); where message contains the nonce used for the following session and the interaction ID. This wouldn't really add much overhead (but also not much security). The reason why I still would prefer encryption over signing is that the current implementation doesn't sign anything. I'm not sure if it is even possible to use the encryption keypair (the one tied to the Tox ID) together with NaCl's crypto_sign().

arucard21 commented 9 years ago

If signing isn't possible with the encryption keypair, then I agree that encrypting the announcement like you suggested would be the alternative. Not really for security reasons, just for simplicity. You should still use incrementing nonces and check them against the last known nonce.

In fact, even in the current protocol format, incrementing nonces can be used, they just need to be maintained by the node and checked against the last known nonce. This change would already prevent replay attacks with both request and response packets. The rest is just about optimizing the use of nonces in the datapackets. In which case, the nonce announcement and acknowledgement are not necessary as separate datapackets. The nonce could just be announced with the first encrypted datapacket sent to a node.

The separate nonce datapackets would only become necessary if the nonce is removed from all other datapackets entirely. You could then use these separate nonce datapackets to agree on a nonce with another node. However, if you use the nonce as value for Ping ID in the ping message, then this could be used as nonce announcement and there is no need to create additional datapackets.

codedust commented 9 years ago

You should still use incrementing nonces and check them against the last known nonce.

Definitely. :)

The separate nonce datapackets would only become necessary if the nonce is removed from all other datapackets entirely.

What do you mean by removing the nonces completely from the datapackets?

The nonce could just be announced with the first encrypted datapacket sent to a node.

I think it's a better idea to have distinct nonce announcements. If we use the first datapacket as a nonce announcement, this first datapacked could possibly be replayed. Even if the risk is very low that this datapacked could be used for a successful attack, it's easy to avoid it. I also like the idea of using the ping packets for nonce announcements since they contain exactly the information needed for this purpose.

arucard21 commented 9 years ago

What do you mean by removing the nonces completely from the datapackets?

I mean that you don't need to send the nonce along with the ciphertext in each datapacket as is currently done. You could know the nonce in both nodes and adjust it in the same way to get to the same, new nonce in both nodes. This would likely be error-prone (e.g. after losing datapackets), so it might be best to only send a nonce_offset value or the last few bytes of the nonce as confirmation of the nonce value (as is currently already being done with normal datapackets, see https://github.com/irungentoo/toxcore/blob/master/docs/Tox_middle_level_network_protocol.txt). Either way, the entire nonce does not need to be sent, so it becomes useful to have separate datapackets to send the nonce.

If we use the first datapacket as a nonce announcement, this first datapacked could possibly be replayed

I actually also thought that might be possible for a while, but it's not. Even the first datapacket can not be replayed. What happens is that the nonce that was sent with that first datapacket would be the last one known by the receiving node. If you replay that first datapacket, the nonce would have to be larger than the last one known, so the node should reject that replayed datapacket.

codedust commented 9 years ago

I agree that sending the last few bytes of the nonce (or an offset) definitely is a good idea. It reduces bandwidth usage and also avoids accepting "wrong" nonces by accident (since the message cannot be decrypted without the first part of the nonce).

If you replay that first datapacket, the nonce would have to be larger than the last one known, so the node should reject that replayed datapacket.

This is the case if the node stores all nonces used in the past. If a node doesn't store all used nonces, the first datapacket sent in the second to last session can be replayed. Therefore it might be better/easier to do a nonce announcement than storing a large number of nonces.

Alternatively, it would be possible to store only one nonce and use this nonce in the nonce announcement (the first datapacket) of the next session. However, I wouldn't recommend this because it isn't as fault tolerant as a seperate announcement packet with a fresh nonce.

arucard21 commented 9 years ago

I agree with you on all points, but I think this is just a matter of gradually implementing the changes. First using incrementing nonces and checking them, then creating separate nonce announcement/acknowledgement datapackets, then reducing the size of the nonces sent with each datapacket. This might keep the software more stable than if you make one big change. It should ultimately end up like you suggested though.

codedust commented 9 years ago

Gradually implementing the changes seems to be a good plan. :+1:

GrayHatter commented 8 years ago

anyone have any suggestions on what I should do with this issue other than close a stale?

arucard21 commented 7 years ago

This issue was about improving security through better use of the nonce values. So far, we've come up with a plan on how to do that, so either that will be implemented or it won't. As there's nothing left to discuss or come up with in this issue, I'll close this because it's stale.

iphydf commented 7 years ago

Can you summarise the plan and create a new issue to track its implementation on https://github.com/TokTok/c-toxcore?

On 22 Dec 2016 7:28 am, "arucard21" notifications@github.com wrote:

This issue was about improving security through better use of the nonce values. So far, we've come up with a plan on how to do that, so either that will be implemented or it won't. As there's nothing left to discuss or come up with in this issue, I'll close this because it's stale.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/irungentoo/toxcore/issues/1313#issuecomment-268736630, or mute the thread https://github.com/notifications/unsubscribe-auth/AKJ5gL4SBo6igHQCOsEdQlwMOi6UgUuyks5rKiadgaJpZM4D3MDi .

arucard21 commented 7 years ago

Done, created https://github.com/TokTok/c-toxcore/issues/356