Re-implement the SOCKS impl. without @dinosaure's patches

dinosaure · Feb 7, 2024 · 440fb1d · 440fb1d
1 parent 3c22a3e
commit 440fb1d
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 27 deletions.
diff --git a/bob.opam b/bob.opam
@@ -42,5 +42,5 @@ depends: [
 ]
 
 pin-depends: [
-  [ "socks.dev" "git+https://github.com/dinosaure/ocaml-socks.git#31399dab6812cb3fbf28b9eea5c5e4a3939abe4b" ]
+  [ "socks.dev" "git+https://github.com/cfcs/ocaml-socks.git#c46ed399df18307e5240457b9d5d4ff8a6739f06" ]
 ]
diff --git a/com.opam b/com.opam
@@ -62,7 +62,7 @@ pin-depends: [
   [ "mirage-crypto-rng.dev" "git+https://github.com/dinosaure/mirage-crypto.git#83b160823cc601ffb19f8be2e32d643ffcbea60d" ]
   [ "mtime.dev" "git+https://github.com/dinosaure/mtime.git#d5d70f38c40da90e3e173eb60346df55b64a4a0a" ]
   [ "gmp.dev" "git+https://github.com/mirage/ocaml-gmp.git#90c7941b3b2649c19f2596538417e5ce1df61b9d" ]
-  [ "socks.dev" "git+https://github.com/dinosaure/ocaml-socks.git#31399dab6812cb3fbf28b9eea5c5e4a3939abe4b" ]
+  [ "socks.dev" "git+https://github.com/cfcs/ocaml-socks.git#c46ed399df18307e5240457b9d5d4ff8a6739f06" ]
 ]
 
 x-mirage-opam-lock-location: "com.opam.locked"

diff --git a/lib/bob_socks.ml b/lib/bob_socks.ml
@@ -185,6 +185,15 @@ let finally_connect_socks5 socket payload dst =
   in
   go payload
 
+let parse_socks5_response payload =
+  match (payload.[0], String.length payload >= 2) with
+  | '\x05', true ->
+      let leftover = String.sub payload 2 (String.length payload - 2) in
+      Ok (Socks.socks5_authentication_method_of_char payload.[1], leftover)
+  | '\x05', false -> Error `Incomplete_response
+  | _ -> Error `Invalid_response
+  | exception _ -> Error `Incomplete_response
+
 let connect_socks5 ~happy_eyeballs ~credential ~addr dst =
   let open Fiber in
   Bob_happy_eyeballs.connect happy_eyeballs addr >>? fun (_sockaddr, socket) ->
@@ -199,16 +208,10 @@ let connect_socks5 ~happy_eyeballs ~credential ~addr dst =
   >>| open_write_error
   >>? fun () ->
   let rec go payload =
-    match Socks.parse_socks5_username_password_response payload with
-    | Ok (No_acceptable_methods, _) ->
-        Fiber.return
-          (Error
-             (`Msg
-               "No acceptable authentication methods for the SOCKSv5 server"))
-    | Ok (No_authentication_required, leftover) when credential = None ->
+    match parse_socks5_response payload with
+    | Ok (No_authentication_required, leftover) ->
         Fiber.return (Ok (`No_authentication_required leftover))
-    | Ok (Username_password (_username, _password), leftover)
-      when credential <> None ->
+    | Ok (Username_password _, leftover) when credential <> None ->
         let username_password_req =
           match credential with
           | None | Username _ -> assert false
@@ -217,12 +220,13 @@ let connect_socks5 ~happy_eyeballs ~credential ~addr dst =
               |> Result.get_ok
         in
         Fiber.return (Ok (`Authentication (username_password_req, leftover)))
-    | Ok (_, _) ->
+    | Ok _ ->
         Fiber.return
-          (Error (`Msg "Unexpected response from the SOCKSv5 server."))
-    | Error `Invalid_request ->
+          (error_msgf
+             "No acceptable authentication methods for the SOCKSv5 server")
+    | Error `Invalid_response ->
         Fiber.return (Error (`Msg "Invalid response from the SOCKSv5 server"))
-    | Error `Incomplete_request -> (
+    | Error `Incomplete_response -> (
         Fiber.read socket >>| reword_error (fun err -> `Unix err) >>? function
         | `End ->
             Fiber.return
@@ -237,18 +241,20 @@ let connect_socks5 ~happy_eyeballs ~credential ~addr dst =
       >>| open_write_error
       >>? fun () ->
       let rec go payload =
-        if String.length payload < 2 then
-          Fiber.read socket >>| reword_error (fun err -> `Unix err) >>? function
-          | `End ->
-              Fiber.return
-                (Error (`Msg "Incomplete response from SOCKSv5 server"))
-          | `Data bs -> go (payload ^ bigstring_to_string bs)
-        else
-          match String.sub payload 0 2 with
-          | "\x01\x00" ->
-              Fiber.return
-                (Ok (String.sub payload 2 (String.length payload - 2)))
-          | _ -> Fiber.return (Error (`Msg "Invalid username/password"))
+        match Socks.parse_socks5_username_password_response payload with
+        | Ok (true, leftover) -> Fiber.return (Ok leftover)
+        | Ok (false, _leftover) ->
+            Fiber.return (Error (`Msg "Invalid username/password"))
+        | Error `Invalid_request ->
+            Fiber.return
+              (Error (`Msg "Invalid response from the SOCKSv5 server"))
+        | Error `Incomplete_request -> (
+            Fiber.read socket >>| reword_error (fun err -> `Unix err)
+            >>? function
+            | `End ->
+                Fiber.return
+                  (Error (`Msg "Incomplete response from SOCKSv5 server"))
+            | `Data bs -> go (payload ^ bigstring_to_string bs))
       in
       go payload >>? fun payload -> finally_connect_socks5 socket payload dst
   | `No_authentication_required payload ->