Reduce the use of the general Exception type and improve the exception tarcking in protocol.receive

This commit is contained in:
Zahary Karadjov 2020-02-18 00:47:13 +02:00
parent 79dfe88ec8
commit cca931d0b5
No known key found for this signature in database
GPG Key ID: C8936F8A3073D609
6 changed files with 70 additions and 38 deletions

View File

@ -27,15 +27,24 @@ proc makeKey(id: NodeId, address: Address): array[keySize, byte] =
copyMem(addr result[sizeof(id) + 1], unsafeAddr address.ip.address_v6, sizeof(address.ip.address_v6))
copyMem(addr result[sizeof(id) + 1 + sizeof(address.ip.address_v6)], unsafeAddr address.udpPort, sizeof(address.udpPort))
method storeKeys*(db: DiscoveryDB, id: NodeId, address: Address, r, w: array[16, byte]) =
var value: array[sizeof(r) + sizeof(w), byte]
value[0 .. 15] = r
value[16 .. ^1] = w
db.backend.put(makeKey(id, address), value)
method storeKeys*(db: DiscoveryDB, id: NodeId, address: Address, r, w: array[16, byte]): bool {.raises: [Defect].} =
try:
var value: array[sizeof(r) + sizeof(w), byte]
value[0 .. 15] = r
value[16 .. ^1] = w
db.backend.put(makeKey(id, address), value)
return true
except CatchableError:
return false
method loadKeys*(db: DiscoveryDB, id: NodeId, address: Address, r, w: var array[16, byte]): bool =
let res = db.backend.get(makeKey(id, address))
if res.len == sizeof(r) + sizeof(w):
method loadKeys*(db: DiscoveryDB, id: NodeId, address: Address, r, w: var array[16, byte]): bool {.raises: [Defect].} =
try:
let res = db.backend.get(makeKey(id, address))
if res.len != sizeof(r) + sizeof(w):
return false
copyMem(addr r[0], unsafeAddr res[0], sizeof(r))
copyMem(addr w[0], unsafeAddr res[sizeof(r)], sizeof(w))
result = true
return true
except CatchableError:
return false

View File

@ -32,13 +32,14 @@ type
ephemeralKey*: array[64, byte]
response*: seq[byte]
RandomSourceDepleted* = object of CatchableError
const
gcmTagSize = 16
proc randomBytes(v: var openarray[byte]) =
if nimcrypto.randomBytes(v) != v.len:
raise newException(Exception, "Could not randomize bytes") # TODO:
raise newException(RandomSourceDepleted, "Could not randomize bytes")
proc idNonceHash(nonce, ephkey: openarray[byte]): array[32, byte] =
var ctx: sha256
@ -50,12 +51,12 @@ proc idNonceHash(nonce, ephkey: openarray[byte]): array[32, byte] =
proc signIDNonce(c: Codec, idNonce, ephKey: openarray[byte]): SignatureNR =
if signRawMessage(idNonceHash(idNonce, ephKey), c.privKey, result) != EthKeysStatus.Success:
raise newException(Exception, "Could not sign idNonce")
raise newException(EthKeysException, "Could not sign idNonce")
proc deriveKeys(n1, n2: NodeID, priv: PrivateKey, pub: PublicKey, challenge: Whoareyou, result: var HandshakeSecrets) =
var eph: SharedSecretFull
if ecdhAgree(priv, pub, eph) != EthKeysStatus.Success:
raise newException(Exception, "ecdhAgree failed")
raise newException(EthKeysException, "ecdhAgree failed")
# TODO: Unneeded allocation here
var info = newSeqOfCap[byte](idNoncePrefix.len + 32 * 2)
@ -85,16 +86,12 @@ proc makeAuthHeader(c: Codec, toNode: Node, nonce: array[gcmNonceSize, byte],
if challenge.recordSeq < ln.record.seqNum:
resp.record = ln.record
var remotePubkey: PublicKey
if not toNode.record.get(remotePubkey):
raise newException(Exception, "Could not get public key from remote ENR") # Should not happen!
let ephKey = newPrivateKey()
let ephPubkey = ephKey.getPublicKey().getRaw
resp.signature = c.signIDNonce(challenge.idNonce, ephPubkey).getRaw
deriveKeys(ln.id, toNode.id, ephKey, remotePubkey, challenge, handhsakeSecrets)
deriveKeys(ln.id, toNode.id, ephKey, toNode.node.pubKey, challenge, handhsakeSecrets)
let respRlp = rlp.encode(resp)
@ -134,8 +131,8 @@ proc encodeEncrypted*(c: Codec, toNode: Node, packetData: seq[byte], challenge:
headEnc = c.makeAuthHeader(toNode, nonce, sec, challenge)
writeKey = sec.writeKey
c.db.storeKeys(toNode.id, toNode.address, sec.readKey, sec.writeKey)
# TODO: is it safe to ignore the error here?
discard c.db.storeKeys(toNode.id, toNode.address, sec.readKey, sec.writeKey)
var body = packetData
let tag = packetTag(toNode.id, c.localNode.id)
@ -228,7 +225,8 @@ proc decodeEncrypted*(c: var Codec, fromId: NodeID, fromAddr: Address, input: se
# Swap keys to match remote
swap(sec.readKey, sec.writeKey)
c.db.storeKeys(fromId, fromAddr, sec.readKey, sec.writeKey)
# TODO: is it safe to ignore the error here?
discard c.db.storeKeys(fromId, fromAddr, sec.readKey, sec.writeKey)
readKey = sec.readKey
else:
@ -249,7 +247,7 @@ proc decodeEncrypted*(c: var Codec, fromId: NodeID, fromAddr: Address, input: se
proc newRequestId*(): RequestId =
if randomBytes(addr result, sizeof(result)) != sizeof(result):
raise newException(Exception, "Could not randomize bytes") # TODO:
raise newException(RandomSourceDepleted, "Could not randomize bytes") # TODO:
proc numFields(T: typedesc): int =
for k, v in fieldPairs(default(T)): inc result

View File

@ -84,7 +84,7 @@ proc makeEnrAux(seqNum: uint64, pk: PrivateKey, pairs: openarray[(string, Field)
var sig: SignatureNR
if signRawMessage(keccak256.digest(toSign).data, pk, sig) != EthKeysStatus.Success:
raise newException(Exception, "Could not sign ENR (internal error)")
raise newException(EthKeysException, "Could not sign ENR (internal error)")
result.raw = block:
var w = initRlpList(result.pairs.len * 2 + 2)
@ -136,6 +136,10 @@ proc get*(r: Record, key: string, T: type): T =
elif T is string:
requireKind(f, kString)
return f.str
elif T is PublicKey:
requireKind(f, kBytes)
if recoverPublicKey(f.bytes, result) != EthKeysStatus.Success:
raise newException(ValueError, "Invalid public key")
elif T is array:
when type(result[0]) is byte:
requireKind(f, kBytes)
@ -156,11 +160,11 @@ proc get*(r: Record, pubKey: var PublicKey): bool =
proc tryGet*(r: Record, key: string, T: type): Option[T] =
try:
return some r.get(key, T)
return some get(r, key, T)
except CatchableError:
discard
func toTypedRecord*(r: Record): Option[TypedRecord] =
proc toTypedRecord*(r: Record): Option[TypedRecord] =
let id = r.tryGet("id", string)
if id.isSome:
var tr: TypedRecord

View File

@ -63,7 +63,7 @@ proc send(d: Protocol, n: Node, data: seq[byte]) =
proc randomBytes(v: var openarray[byte]) =
if nimcrypto.randomBytes(v) != v.len:
raise newException(Exception, "Could not randomize bytes") # TODO:
raise newException(RandomSourceDepleted, "Could not randomize bytes") # TODO:
proc `xor`[N: static[int], T](a, b: array[N, T]): array[N, T] =
for i in 0 .. a.high:
@ -123,9 +123,18 @@ proc handleFindNode(d: Protocol, fromNode: Node, fn: FindNodePacket, reqId: Requ
let distance = min(fn.distance, 256)
d.sendNodes(fromNode, reqId, d.routingTable.neighboursAtDistance(distance))
proc receive(d: Protocol, a: Address, msg: Bytes)
{.gcsafe, raises:[RlpError, Exception].} =
# TODO: figure out where the general exception comes from and clean it up
proc receive(d: Protocol, a: Address, msg: Bytes) {.gcsafe,
raises: [
Defect,
# TODO This is now coming from Chronos's callSoon
Exception,
# TODO All of these should probably be handled here
RlpError,
IOError,
TransportAddressError,
EthKeysException,
Secp256k1Exception,
].} =
if msg.len < 32:
return # Invalid msg
@ -136,9 +145,12 @@ proc receive(d: Protocol, a: Address, msg: Bytes)
var pr: PendingRequest
if d.pendingRequests.take(whoareyou.authTag, pr):
let toNode = pr.node
let (data, _) = d.codec.encodeEncrypted(toNode, pr.packet, challenge = whoareyou)
d.send(toNode, data)
try:
let (data, _) = d.codec.encodeEncrypted(toNode, pr.packet, challenge = whoareyou)
d.send(toNode, data)
except RandomSourceDepleted as err:
debug "Failed to respond to a who-you-are msg " &
"due to randomness source depletion."
else:
var tag: array[32, byte]
@ -169,7 +181,7 @@ proc receive(d: Protocol, a: Address, msg: Bytes)
if d.awaitedPackets.take((node, packet.reqId), waiter):
waiter.complete(packet.some)
else:
debug "TODO: handle packet: ", packet = packet.kind, origin = node
debug "TODO: handle packet: ", packet = packet.kind, origin = $node
else:
debug "Could not decode, respond with whoareyou"

View File

@ -69,8 +69,9 @@ template packetKind*(T: typedesc[SomePacket]): PacketKind =
elif T is FindNodePacket: findNode
elif T is NodesPacket: nodes
method storeKeys*(db: Database, id: NodeId, address: Address, r, w: array[16, byte]) {.base.} = discard
method loadKeys*(db: Database, id: NodeId, address: Address, r, w: var array[16, byte]): bool {.base.} = discard
method storeKeys*(db: Database, id: NodeId, address: Address, r, w: array[16, byte]): bool {.base, raises: [Defect].} = discard
method loadKeys*(db: Database, id: NodeId, address: Address, r, w: var array[16, byte]): bool {.base, raises: [Defect].} = discard
proc toBytes*(id: NodeId): array[32, byte] {.inline.} =
id.toByteArrayBE()

View File

@ -21,10 +21,18 @@ type
contains(DB, KeccakHash) is bool
# XXX: poor's man vtref types
PutProc = proc (db: RootRef, key, val: openarray[byte]) {.gcsafe.}
GetProc = proc (db: RootRef, key: openarray[byte]): Bytes {.gcsafe.} # Must return empty seq if not found
DelProc = proc (db: RootRef, key: openarray[byte]) {.gcsafe.}
ContainsProc = proc (db: RootRef, key: openarray[byte]): bool {.gcsafe.}
PutProc = proc (db: RootRef, key, val: openarray[byte]) {.
gcsafe, raises: [Defect, CatchableError] .}
GetProc = proc (db: RootRef, key: openarray[byte]): Bytes {.
gcsafe, raises: [Defect, CatchableError] .}
## The result will be empty seq if not found
DelProc = proc (db: RootRef, key: openarray[byte]) {.
gcsafe, raises: [Defect, CatchableError] .}
ContainsProc = proc (db: RootRef, key: openarray[byte]): bool {.
gcsafe, raises: [Defect, CatchableError] .}
TrieDatabaseRef* = ref object
obj: RootRef