From d16f92f6442830dd3aa3e8f6b5d00162f322b6b5 Mon Sep 17 00:00:00 2001 From: Parham Saremi Date: Mon, 14 Nov 2022 16:58:04 +0330 Subject: [PATCH 1/2] Network: try-with with specific exceptions Previously, this block had a generic try-with which is dangerous since it can cause unwanted behaviours and cause confusion for future developers. --- NOnion/Exceptions.fs | 3 +++ NOnion/Network/TorCircuit.fs | 3 ++- NOnion/Network/TorGuard.fs | 3 ++- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/NOnion/Exceptions.fs b/NOnion/Exceptions.fs index 4cd21ff8..20c61b25 100644 --- a/NOnion/Exceptions.fs +++ b/NOnion/Exceptions.fs @@ -26,6 +26,9 @@ type CircuitTruncatedException internal (reason: DestroyReason) = type CircuitDestroyedException internal (reason: DestroyReason) = inherit NOnionException(sprintf "Circuit got destroyed, reason %A" reason) +type CircuitDecryptionFailedException internal () = + inherit NOnionException(sprintf "Circuit Decryption Failed") + type TimeoutErrorException internal () = inherit NOnionException("Time limit exceeded for operation") diff --git a/NOnion/Network/TorCircuit.fs b/NOnion/Network/TorCircuit.fs index 57c9f691..77748073 100644 --- a/NOnion/Network/TorCircuit.fs +++ b/NOnion/Network/TorCircuit.fs @@ -336,7 +336,8 @@ and TorCircuit node) | None -> announceDeath() - failwith "Decryption failed!" + + raise <| CircuitDecryptionFailedException() decryptMessage encryptedRelayCell.EncryptedData nodes | _ -> failwith "Unexpected state when receiving relay cell" diff --git a/NOnion/Network/TorGuard.fs b/NOnion/Network/TorGuard.fs index d32e9584..4bdcdb13 100644 --- a/NOnion/Network/TorGuard.fs +++ b/NOnion/Network/TorGuard.fs @@ -332,7 +332,7 @@ type TorGuard private (client: TcpClient, sslStream: SslStream) = try do! circuit.HandleIncomingCell cell with - | ex -> + | :? CircuitDecryptionFailedException as ex -> sprintf "TorGuard: exception when trying to handle incoming cell type=%i, ex=%s" cell.Command @@ -340,6 +340,7 @@ type TorGuard private (client: TcpClient, sslStream: SslStream) = |> TorLogger.Log self.KillChildCircuits() + | ex -> return raise <| FSharpUtil.ReRaise ex | None -> self.KillChildCircuits() failwithf "Unknown circuit, Id = %i" cid From a1534d2b744357fdb0e2d70ac75406c15ee93ee9 Mon Sep 17 00:00:00 2001 From: Parham Saremi Date: Mon, 21 Nov 2022 14:44:13 +0330 Subject: [PATCH 2/2] Network,TorHandshakes: handle handshake fail Previously, TorHandshakes used failwith to raise handshake failed error. Since we need to catch it, as a result of replacing generic try-with which lead to a red CI in the PR, we had to define its special excpetion and catch that: ``` The active test run was aborted. Reason: Test host process crashed : Unhandled exception. System.Exception: Key handshake failed! at NOnion.TorHandshakes.NTorHandshake.NOnion-TorHandshakes-IHandshake-GenerateKdfResult(ICreatedCell serverSideData) in /home/runner/work/NOnion/NOnion/NOnion/TorHandshakes/NTorHandshake.fs:line 112 at .$TorCircuit.handleIncomingCell@348-1.Invoke(Unit unitVar) in /home/runner/work/NOnion/NOnion/NOnion/Network/TorCircuit.fs:line 352 at Microsoft.FSharp.Control.AsyncPrimitives.CallThenInvoke[T,TResult](AsyncActivation`1 ctxt, TResult result1, FSharpFunc`2 part2) in F:\workspace\_work\1\s\src\fsharp\FSharp.Core\async.fs:line 398 at NOnion.Utility.MailboxResultUtil.TryExecuteAsyncAndReplyAsResult@25-3.Invoke(AsyncActivation`1 ctxt) in /home/runner/work/NOnion/NOnion/NOnion/Utility/MailboxUtil.fs:line 25 at NOnion.Utility.MailboxResultUtil.TryExecuteAsyncAndReplyAsResult@24-6.Invoke(AsyncActivation`1 ctxt) in /home/runner/work/NOnion/NOnion/NOnion/Utility/MailboxUtil.fs:line 24 at .$TorCircuit.clo@965-15.Invoke(AsyncActivation`1 ctxt) in /home/runner/work/NOnion/NOnion/NOnion/Network/TorCircuit.fs:line 965 at .$TorCircuit.clo@950-39.Invoke(AsyncActivation`1 ctxt) in /home/runner/work/NOnion/NOnion/NOnion/Network/TorCircuit.fs:line 950 at .$Mailbox.processFirstArrival@303-8.Invoke(AsyncActivation`1 ctxt) in F:\workspace\_work\1\s\src\fsharp\FSharp.Core\mailbox.fs:line 303 at Microsoft.FSharp.Control.Trampoline.Execute(FSharpFunc`2 firstAction) in F:\workspace\_work\1\s\src\fsharp\FSharp.Core\async.fs:line 109 --- End of stack trace from previous location where exception was thrown --- at Microsoft.FSharp.Control.AsyncPrimitives.Start@907-1.Invoke(ExceptionDispatchInfo edi) in F:\workspace\_work\1\s\src\fsharp\FSharp.Core\async.fs:line 907 at Microsoft.FSharp.Control.Trampoline.Execute(FSharpFunc`2 firstAction) in F:\workspace\_work\1\s\src\fsharp\FSharp.Core\async.fs:line 109 at .$Async.-ctor@163-1.Invoke(Object o) in F:\workspace\_work\1\s\src\fsharp\FSharp.Core\async.fs:line 165 at System.Threading.QueueUserWorkItemCallback.Execute() at System.Threading.ThreadPoolWorkQueue.Dispatch() ``` --- NOnion/Exceptions.fs | 3 +++ NOnion/Network/TorGuard.fs | 26 +++++++++++++++++++------- NOnion/TorHandshakes/FastHandshake.fs | 2 +- NOnion/TorHandshakes/NTorHandshake.fs | 2 +- 4 files changed, 24 insertions(+), 9 deletions(-) diff --git a/NOnion/Exceptions.fs b/NOnion/Exceptions.fs index 20c61b25..3a9427bb 100644 --- a/NOnion/Exceptions.fs +++ b/NOnion/Exceptions.fs @@ -29,6 +29,9 @@ type CircuitDestroyedException internal (reason: DestroyReason) = type CircuitDecryptionFailedException internal () = inherit NOnionException(sprintf "Circuit Decryption Failed") +type HandshakeFailedException internal () = + inherit NOnionException(sprintf "Key handshake failed!") + type TimeoutErrorException internal () = inherit NOnionException("Time limit exceeded for operation") diff --git a/NOnion/Network/TorGuard.fs b/NOnion/Network/TorGuard.fs index 4bdcdb13..b5e90d4f 100644 --- a/NOnion/Network/TorGuard.fs +++ b/NOnion/Network/TorGuard.fs @@ -193,6 +193,18 @@ type TorGuard private (client: TcpClient, sslStream: SslStream) = member self.SendAsync (circuidId: uint16) (cellToSend: ICell) = self.Send circuidId cellToSend |> Async.StartAsTask + member private self.HandleIncomingCellException<'T when 'T :> NOnionException> + (cell: ICell) + (ex: 'T) + = + sprintf + "TorGuard: exception when trying to handle incoming cell type=%i, ex=%s" + cell.Command + (ex.ToString()) + |> TorLogger.Log + + self.KillChildCircuits() + member private __.ReceiveInternal() = async { (* @@ -332,14 +344,14 @@ type TorGuard private (client: TcpClient, sslStream: SslStream) = try do! circuit.HandleIncomingCell cell with + | :? HandshakeFailedException as ex -> + self.HandleIncomingCellException + cell + ex | :? CircuitDecryptionFailedException as ex -> - sprintf - "TorGuard: exception when trying to handle incoming cell type=%i, ex=%s" - cell.Command - (ex.ToString()) - |> TorLogger.Log - - self.KillChildCircuits() + self.HandleIncomingCellException + cell + ex | ex -> return raise <| FSharpUtil.ReRaise ex | None -> self.KillChildCircuits() diff --git a/NOnion/TorHandshakes/FastHandshake.fs b/NOnion/TorHandshakes/FastHandshake.fs index bc2fe4ee..173072ee 100644 --- a/NOnion/TorHandshakes/FastHandshake.fs +++ b/NOnion/TorHandshakes/FastHandshake.fs @@ -36,6 +36,6 @@ type FastHandshake = |> Kdf.ComputeLegacyKdf if kdfResult.KeyHandshake <> serverSideData.DerivativeKey then - failwith "Key handshake failed!" + raise <| HandshakeFailedException() else kdfResult diff --git a/NOnion/TorHandshakes/NTorHandshake.fs b/NOnion/TorHandshakes/NTorHandshake.fs index 9f88a43a..0564acb5 100644 --- a/NOnion/TorHandshakes/NTorHandshake.fs +++ b/NOnion/TorHandshakes/NTorHandshake.fs @@ -107,6 +107,6 @@ type NTorHandshake = let auth = calculateHmacSha256 authInput Constants.NTorTMac if auth <> serverSideData.DerivativeKey then - failwith "Key handshake failed!" + raise <| HandshakeFailedException() else Kdf.ComputeRfc5869Kdf secretInput