Fix: arrive to working keys in case of simultaneous cross connect (#84)
* improve tracing of message exchange run e.g. as ``` nim c -r -d:debug -d:chronicles_enabled=on -d:chronicles_log_level=TRACE -d:chronicles_sinks=textlines[nocolors,stdout] tests/dht/test_providers.nim >err ``` Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com> * add debug on Handshake timeour Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com> * queue messages during handshake and send later If a handshake was already in progress, messages were dropped. Instead of this, it is better to queue these and send as soon as the handshake is finished and thus the encryption key is known. Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com> * rename handshakeInProgress to keyexchangeInProgress Handshake is also a name of a message, which makes previous name less clear. Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com> * keyexchangeInProgress: do not remove on handshake received This is the wrong direction, not needed Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com> * fix cross-connect key exchange Since key exchange can be started both ways simultaneously, and these might not get finalised with UDP transport, we can't be sure what encryption key will be used by the other side: - the one derived in the key-exchange started by us, - the one derived in the key-exchange started by the other node. To alleviate this issue, we store two decryption keys in each session. Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com> --------- Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
This commit is contained in:
Csaba Kiraly
GitHub
parent
66116b9bf6
commit
91b2eaec89
|
|
@ -227,8 +227,8 @@ proc encodeMessagePacket*(rng: var HmacDrbgContext, c: var Codec,
|
|||
|
||||
# message
|
||||
var messageEncrypted: seq[byte]
|
||||
var initiatorKey, recipientKey: AesKey
|
||||
if c.sessions.load(toId, toAddr, recipientKey, initiatorKey):
|
||||
var initiatorKey, recipientKey1, recipientKey2: AesKey
|
||||
if c.sessions.load(toId, toAddr, recipientKey1, recipientKey2, initiatorKey):
|
||||
haskey = true
|
||||
messageEncrypted = encryptGCM(initiatorKey, nonce, message, @iv & header)
|
||||
discovery_session_lru_cache_hits.inc()
|
||||
|
|
@ -423,8 +423,8 @@ proc decodeMessagePacket(c: var Codec, fromAddr: Address, nonce: AESGCMNonce,
|
|||
let srcId = NodeId.fromBytesBE(header.toOpenArray(staticHeaderSize,
|
||||
header.high))
|
||||
|
||||
var initiatorKey, recipientKey: AesKey
|
||||
if not c.sessions.load(srcId, fromAddr, recipientKey, initiatorKey):
|
||||
var initiatorKey, recipientKey1, recipientKey2: AesKey
|
||||
if not c.sessions.load(srcId, fromAddr, recipientKey1, recipientKey2, initiatorKey):
|
||||
# Don't consider this an error, simply haven't done a handshake yet or
|
||||
# the session got removed.
|
||||
trace "Decrypting failed (no keys)"
|
||||
|
|
@ -434,15 +434,24 @@ proc decodeMessagePacket(c: var Codec, fromAddr: Address, nonce: AESGCMNonce,
|
|||
|
||||
discovery_session_lru_cache_hits.inc()
|
||||
|
||||
let pt = decryptGCM(recipientKey, nonce, ct, @iv & @header)
|
||||
var pt = decryptGCM(recipientKey2, nonce, ct, @iv & @header)
|
||||
if pt.isNone():
|
||||
# Don't consider this an error, the session got probably removed at the
|
||||
# peer's side and a random message is send.
|
||||
trace "Decrypting failed (invalid keys)"
|
||||
c.sessions.del(srcId, fromAddr)
|
||||
discovery_session_decrypt_failures.inc()
|
||||
return ok(Packet(flag: Flag.OrdinaryMessage, requestNonce: nonce,
|
||||
srcId: srcId))
|
||||
trace "Decrypting failed, trying other key"
|
||||
pt = decryptGCM(recipientKey1, nonce, ct, @iv & @header)
|
||||
if pt.isNone():
|
||||
# Don't consider this an error, the session got probably removed at the
|
||||
# peer's side and a random message is send.
|
||||
# This might also be a cross-connect. Not deleteing key, as it might be
|
||||
# needed later, depending on message order.
|
||||
trace "Decrypting failed (invalid keys)", address = fromAddr
|
||||
#c.sessions.del(srcId, fromAddr)
|
||||
discovery_session_decrypt_failures.inc()
|
||||
return ok(Packet(flag: Flag.OrdinaryMessage, requestNonce: nonce,
|
||||
srcId: srcId))
|
||||
|
||||
# Most probably the same decryption key will work next time. We should
|
||||
# elevate it's priority.
|
||||
c.sessions.swapr(srcId, fromAddr)
|
||||
|
||||
let message = ? decodeMessage(pt.get())
|
||||
|
||||
|
|
|
|||
|
|
@ -9,6 +9,13 @@
|
|||
## https://github.com/ethereum/devp2p/blob/master/discv5/discv5-theory.md#session-cache
|
||||
##
|
||||
|
||||
## A session stores encryption and decryption keys for P2P encryption.
|
||||
## Since key exchange can be started both ways, and these might not get finalised with
|
||||
## UDP transport, we can't be sure what encryption key will be used by the other side:
|
||||
## - the one derived in the key-exchange started by us,
|
||||
## - the one derived in the key-exchange started by the other node.
|
||||
## To alleviate this issue, we store two decryption keys in each session.
|
||||
|
||||
{.push raises: [Defect].}
|
||||
|
||||
import
|
||||
|
|
@ -27,7 +34,7 @@ const
|
|||
type
|
||||
AesKey* = array[aesKeySize, byte]
|
||||
SessionKey* = array[keySize, byte]
|
||||
SessionValue* = array[sizeof(AesKey) + sizeof(AesKey), byte]
|
||||
SessionValue* = array[3 * sizeof(AesKey), byte]
|
||||
Sessions* = LRUCache[SessionKey, SessionValue]
|
||||
|
||||
func makeKey(id: NodeId, address: Address): SessionKey =
|
||||
|
|
@ -42,18 +49,37 @@ func makeKey(id: NodeId, address: Address): SessionKey =
|
|||
pos.inc(sizeof(address.ip.address_v6))
|
||||
result[pos ..< pos+sizeof(address.port)] = toBytes(address.port.uint16)
|
||||
|
||||
func store*(s: var Sessions, id: NodeId, address: Address, r, w: AesKey) =
|
||||
var value: array[sizeof(r) + sizeof(w), byte]
|
||||
value[0 .. 15] = r
|
||||
value[16 .. ^1] = w
|
||||
s.put(makeKey(id, address), value)
|
||||
func swapr*(s: var Sessions, id: NodeId, address: Address) =
|
||||
var value: array[3 * sizeof(AesKey), byte]
|
||||
let
|
||||
key = makeKey(id, address)
|
||||
entry = s.get(key)
|
||||
if entry.isSome():
|
||||
let val = entry.get()
|
||||
copyMem(addr value[0], unsafeAddr val[16], sizeof(AesKey))
|
||||
copyMem(addr value[16], unsafeAddr val[0], sizeof(AesKey))
|
||||
copyMem(addr value[32], unsafeAddr val[32], sizeof(AesKey))
|
||||
s.put(key, value)
|
||||
|
||||
func load*(s: var Sessions, id: NodeId, address: Address, r, w: var AesKey): bool =
|
||||
func store*(s: var Sessions, id: NodeId, address: Address, r, w: AesKey) =
|
||||
var value: array[3 * sizeof(AesKey), byte]
|
||||
let
|
||||
key = makeKey(id, address)
|
||||
entry = s.get(key)
|
||||
if entry.isSome():
|
||||
let val = entry.get()
|
||||
copyMem(addr value[0], unsafeAddr val[16], sizeof(r))
|
||||
value[16 .. 31] = r
|
||||
value[32 .. ^1] = w
|
||||
s.put(key, value)
|
||||
|
||||
func load*(s: var Sessions, id: NodeId, address: Address, r1, r2, w: var AesKey): bool =
|
||||
let res = s.get(makeKey(id, address))
|
||||
if res.isSome():
|
||||
let val = res.get()
|
||||
copyMem(addr r[0], unsafeAddr val[0], sizeof(r))
|
||||
copyMem(addr w[0], unsafeAddr val[sizeof(r)], sizeof(w))
|
||||
copyMem(addr r1[0], unsafeAddr val[0], sizeof(r1))
|
||||
copyMem(addr r2[0], unsafeAddr val[sizeof(r1)], sizeof(r2))
|
||||
copyMem(addr w[0], unsafeAddr val[sizeof(r1) + sizeof(r2)], sizeof(w))
|
||||
return true
|
||||
else:
|
||||
return false
|
||||
|
|
|
|||
Loading…
Reference in New Issue