Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

listen on 0.0.0.0 #4

Open
dweinstein opened this issue Jun 13, 2016 · 24 comments
Open

listen on 0.0.0.0 #4

dweinstein opened this issue Jun 13, 2016 · 24 comments

Comments

@dweinstein
Copy link

AFAICT it'll only listen on the loopback atm.

@fxfactorial
Copy link
Contributor

Can you give more detail, what would you like? To have the local status server be on 0.0.0.0?

@dweinstein
Copy link
Author

dweinstein commented Jun 13, 2016

I'd like to reach the forwarded port(s) on all listening interfaces rather than having to ssh localhost. this is useful e.g., to run ocaml-usbmux in a docker container.

@dweinstein
Copy link
Author

btw thanks for the quick reply and this is a neat project that I recently started exploring in greater depth so I could have easily overlooked something. AFAICT the forwarded connections are only listening on 127.0.0.1 which means clients can only be on the localhost. I'd like them to listen on 0.0.0.0 (all interfaces) so that I can run the service in a container and expose those ports from the docker container.

@fxfactorial
Copy link
Contributor

fxfactorial commented Jun 13, 2016

I see. Yea, I made it just run on localhost, I can open it up as a command line option. So say command line option that defaults to 127.0.0.1 or 0.0.0.0?

@dweinstein
Copy link
Author

IMO defaulting to 127.0.01 is safer and what people generally would expect. Having it be a command line option would be great so that it can be set at runtime. That will definitely take care of my use case for sure. You might also consider exposing it as an option in your mappings file, but that might just be getting fancy ;-)

@fxfactorial
Copy link
Contributor

fxfactorial commented Jun 14, 2016

Let's start with just opening up on the command line. Do you know any OCaml or have any inclination to learn? Otherwise I can do this quickly for you.

@dweinstein
Copy link
Author

dweinstein commented Jun 14, 2016

I am interested in learning some ocaml, as it looks quite nice but I'd be starting from basically scratch :-)

I couldn't tell if the tunnel host was hidden somewhere in a lower layer module. Mainly I was looking around the code trying to figure out where the host might be set. I can only see mention of the status_server here, but I wasn't sure if that sets the host for the tunnels or just the status server which would be two different things as you pointed out.

I tried switching that line to 0.0.0.0 and then rebuilding inside my docker container and then had trouble building because of missing dependencies. So I gave up that approach and started working on getting a proper environment on my host. FWIW I was trying to use the ocaml/opam:debian docker image to get a dev environment without clobbering up my host. It worked very well for at least installing this tool.

TBH I'd probably learn your modification faster than I'd fumble through it and in the meantime I can work on getting a proper dev environment setup. I wasn't sure if I could get away with doing something equivalent to npm i . to install via opam for dev.

@dweinstein
Copy link
Author

dweinstein commented Jun 14, 2016

The error I was getting in the docker builder was

 ---> Running in 6565825de7bb
ocaml setup.ml -configure
Cannot find file topfind.
Unknown directive `require'.
make: *** [setup.data] Error 2
Makefile:34: recipe for target 'setup.data' failed

and so the googling began :-)

@fxfactorial
Copy link
Contributor

fxfactorial commented Jun 14, 2016

So basically you can follow this commit: 36d9a19 and the line you'd have to change, rather get that updated info from the command line is: https://github.com/onlinemediagroup/ocaml-usbmux/blob/master/src/lib/usbmux.ml#L321 aka inet_addr_loopback to inet_of_string "0.0.0.0", (or something like that)

That docker error suggests that its missing an ~/.ocamlinit, usually added by opam. it just consists of:

let () = 
  try Topdirs.dir_directory (Sys.getenv "OCAML_TOPLEVEL_PATH")
with Not_found -> ()
;;

#use "topfind"

Besides that I haven't used ocaml/opam in docker (non trivially)

For local development, you can do in the root of the project:

opam pin add usbmux . -y

And that ought to install and build all the dependencies. To rebuild after that, do your changes and then opam upgrade usbmux or a plain make in the root of the directory. (If doing a plain make then you'll have a main.native binary (that's gandalf)

@dweinstein
Copy link
Author

I just changed inet_addr_loopback to inet_addr_any in a local copy and did a opam pin add usbmux . -y and that worked great at least for now.

AFAICT for passing the CLI option string I should be able to use inet_addr_of_string

So I started with something like:

diff --git a/src/app/gandalf_args.ml b/src/app/gandalf_args.ml
index 4f2d54f..e49cf23 100644
--- a/src/app/gandalf_args.ml
+++ b/src/app/gandalf_args.ml
@@ -46,6 +46,11 @@ let status_server_port =
   let doc = "Port that $(b,$(tname))'s status server will use" in
   Arg.(value & opt (some int) (Some 5000) & info ["status_port"] ~doc)

+let bind_host =
+  let doc =
+    "Host that tunnels will bind to/listen on" in
+  Arg.(value & opt (some string) None & info ["l"; "bind_host"] ~doc)
+
 let status =
   let doc = "Metadata and pretty print json of \
              currently tunneled devices."
diff --git a/src/app/main.ml b/src/app/main.ml
index d8aaaf0..a058adf 100644
--- a/src/app/main.ml
+++ b/src/app/main.ml
@@ -198,7 +198,8 @@ let entry_point = let open Gandalf_args in
         $ log_plugged_action
         $ log_everything_else
         $ status_server_port
-        $ ignore_all_unix_errors)
+        $ ignore_all_unix_errors
+        $ bind_host)

 let top_level_info =
   let doc = "Control TCP forwarding for iDevices" in
diff --git a/src/lib/usbmux.ml b/src/lib/usbmux.ml
index f972340..9e01874 100644
--- a/src/lib/usbmux.ml
+++ b/src/lib/usbmux.ml
@@ -184,9 +184,10 @@ module Relay = struct
        tunnels_created,
        tunnel_timeouts,
        unix_exn_exit_program,
-       root_gandalf_process_pid) =
+       root_gandalf_process_pid,
+       tunnels_host) =
     Hashtbl.create 24, ref "", ref None, ref 0,
-    ref 0, ref 0, ref false, ref 0
+    ref 0, ref 0, ref false, ref 0, ref false

   let status_server port =
     P.sprintf "http://127.0.0.1:%d" port
@@ -318,7 +319,7 @@ module Relay = struct
     tunnels.forwarding
     |> List.map ~f:(fun {local_port; device_port} ->
         let handler = handle_connection device_id device_port udid local_port in
-        let server_addr = Unix.(ADDR_INET (inet_addr_loopback, local_port)) in
+        let server_addr = Unix.(ADDR_INET (inet_addr_any, local_port)) in
         Lwt_io.establish_server server_addr (fun a ->
             if Unix.getpid () = !root_gandalf_process_pid then
               handler a

What I'd like to do is if the CLI option isn't provided I guess if the option type defaults to the None to have it default to in_addr_loopback and otherwise take the inet_addr_of_string of the CLI-provided string. I'll have to come back to this later.

@fxfactorial
Copy link
Contributor

@dweinstein Awesome work! the ref false actually won't be needed I don't think since the value won't change during execution of the program.

@fxfactorial
Copy link
Contributor

@dweinstein you'll want to update your local repo. I reverted the last two commits on master, it ought to not affect you, just FYI.

@dweinstein
Copy link
Author

# File "src/lib/usbmux.ml", line 485, characters 4-15:
# Error: This expression has type 'a option
#        but an expression was expected of type 'b ref
# Command exited with code 2.

diff --git a/src/app/gandalf_args.ml b/src/app/gandalf_args.ml
index 4f2d54f..e49cf23 100644
--- a/src/app/gandalf_args.ml
+++ b/src/app/gandalf_args.ml
@@ -46,6 +46,11 @@ let status_server_port =
   let doc = "Port that $(b,$(tname))'s status server will use" in
   Arg.(value & opt (some int) (Some 5000) & info ["status_port"] ~doc)

+let bind_host =
+  let doc =
+    "Host that tunnels will bind to/listen on" in
+  Arg.(value & opt (some string) None & info ["l"; "bind_host"] ~doc)
+
 let status =
   let doc = "Metadata and pretty print json of \
              currently tunneled devices."
diff --git a/src/app/main.ml b/src/app/main.ml
index d8aaaf0..3f6b60d 100644
--- a/src/app/main.ml
+++ b/src/app/main.ml
@@ -103,7 +103,8 @@ let begin_program
     log_plugged_inout
     log_everything_else
     stats_server
-    ignore_unix_exn_ =
+    ignore_unix_exn_
+    bind_host =
   let starting_place = Sys.getcwd () in
   if do_daemonize then begin
     (* This order matters, must get this done before anything Lwt
@@ -150,6 +151,7 @@ let begin_program
         Usbmux.Logging.(
           let relay_with =
             R.make_tunnels
+              ?bind_host:bind_host
               ~ignore_unix_exn:ignore_unix_exn_
               ?stats_server
               ?tunnel_timeout
@@ -198,7 +200,8 @@ let entry_point = let open Gandalf_args in
         $ log_plugged_action
         $ log_everything_else
         $ status_server_port
-        $ ignore_all_unix_errors)
+        $ ignore_all_unix_errors
+        $ bind_host)

 let top_level_info =
   let doc = "Control TCP forwarding for iDevices" in
diff --git a/src/lib/usbmux.ml b/src/lib/usbmux.ml
index 26ec014..73fb641 100644
--- a/src/lib/usbmux.ml
+++ b/src/lib/usbmux.ml
@@ -179,6 +179,8 @@ module Relay = struct

   let relay_lock = Lwt_mutex.create ()

+  let tunnel_host = None
+
   let (running_servers,
        mapping_file,
        relay_timeout,
@@ -275,7 +277,9 @@ module Relay = struct
     begin
       tunnels.forwarding |> Lwt_list.map_p (fun {local_port; device_port} ->
           let open Protocol in
-          let server_address = Unix.(ADDR_INET (inet_addr_loopback, local_port)) in
+          let server_address = match tunnel_host with
+            | None -> Unix.(ADDR_INET (inet_addr_loopback, local_port))
+            | Some addr -> Unix.(ADDR_INET (inet_addr_of_string(addr), local_port)) in
           Lwt_io.establish_server server_address begin fun (tcp_ic, tcp_oc) ->
             Lwt_io.with_connection usbmuxd_address begin fun (mux_ic, mux_oc) ->
               let msg = connect_message ~device_id ~device_port in
@@ -462,6 +466,7 @@ module Relay = struct
     |> Lwt.async

   let rec make_tunnels
+      ?(bind_host=None)
       ?(ignore_unix_exn=false)
       ?(log_opts=(!Logging.logging_opts))
       ?stats_server
@@ -477,6 +482,7 @@ module Relay = struct

     (* Should Unix exceptions exit the program? *)
     unix_exn_exit_program := ignore_unix_exn;
+    tunnel_host := bind_host;

     (* Set the logging options *)
     Logging.logging_opts := log_opts;

@fxfactorial
Copy link
Contributor

fxfactorial commented Jun 15, 2016

Right, so tunnel_host is a ref to the option, you need to 'derefence' the pointer in a sense. try: match !tunnel_host with

But the better idiomatic thing to do is to not use the ref at all but to do that match where you did that tunnel_host := bin_host, like let server_addr = match bind_host with None -> "127.0.0.1" | Some a -> a and then thread that server_addr value through the appropriate functions so that it ends up in the place where the addr is actually needed.

EDIT: Oh I didn't notice you did let tunnel_host = None, to make it work you'd need to actually write let tunnel_host = ref None, because OCaml values are immutable by default. (But again recommend doing it without refs all together since this value never changes during the program's life span)

@fxfactorial
Copy link
Contributor

@dweinstein Wanna try again? Or I can incorporate your patch and make it a commit, then fix it up.

@dweinstein
Copy link
Author

@fxfactorial feel free to incorporate whatever is useful from my patch. thank you! sorry that I've dropped the ball on this

@fxfactorial
Copy link
Contributor

@dweinstein No worries at all, just wanted to give you credit where credit is due. Wanna at least do a commit and then I can merge it, clean it up?

@dweinstein
Copy link
Author

@fxfactorial https://github.com/dweinstein/ocaml-usbmux/commits/listen-loopback -- I don't remember the exact state of this, I think I eschewed the configurability and just overrode it manually in this patch (but left some of the plumbing in to have it be controlled). Thanks again!

@mdolinin
Copy link

Hi. Do you plan to merge this feature into main project?

@fxfactorial
Copy link
Contributor

@mdolinin I need to test it, but I assume it would benefit you? I can add it

@mdolinin
Copy link

@fxfactorial Yes it would be great. But may be better be to have it in a branch. And some description how to build and use it locally, so that i can also test it before merging into main release.

@fxfactorial
Copy link
Contributor

sure, I can give that to you in one opam command :)

@mdolinin
Copy link

Cool. Also i have question. Would it be conflicting with other tools like iproxy (https://github.com/TestStudio/usbmuxd/blob/master/tools/iproxy.c) , which also use usbmuxd?

@fxfactorial
Copy link
Contributor

I don't think it should? try and find out

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants