Closed cfcs closed 4 years ago
in general: why not. in more detail:
LD_PRELOAD
stuff in here -- yes, we talked about this, but there are other (maybe valid?) uses of LD_PRELOAD
that aren't torsocks.so, are there any torsocks + jackline users? unclear whether we'll find them in this discussion. what about: if torsocks is running and used for jackline (can we find this out? maybe looking into the process table?), bail out and warn the user about it!?
udns
not released: Sure, can keep this here until it is released.
The reason this PR tries to fall back to Lwt_unix.getaddrinfo
when it detects LD_PRELOAD
is that I thought it would be reasonable to give torify
/libtorsocks
users a chance to adjust before leaking their .onion
domains over regular DNS. I don't know if we have any, but the README mentions that this torify works
, so I assumed it came up in discussion at least once: https://github.com/hannesm/jackline/blame/master/README.md#L31-L33
cli_state
is changed because
1) I wanted to report the warning message without erroring the thread, wasn't obvious there was another method to achieve this by.
2) selflog
only allowed reporting messages with the Info
, and I think this should be a Warning
(I guess mostly relevant for coloring).
Not sure what you mean re: logging threading, the Lwt.async
stuff? I'm not too fond of that either, but I tried to mimick how the existing code seemed to deal with logging.
LD_PRELOAD
: I guess, and a more reliably way to detect if this path should be chosen would be to consult dlsym
and dlerror
with RTLD_NEXT
(these symbols are already imported from libc by the OCaml runtime) to determine if there is more than one symbol available for getaddrinfo
. That seems like a lot more code to me though. In any case - LD_PRELOAD
or not - the lookups will still work with this PR, and the goal would be to display this error message to people so they have a chance to update their configuration before we switch over to udns
-only.
Re: worth it replacing getaddrinfo
with udns
: That's more of a political discussion, but my arguments FOR are:
1) If we eventually want a MirageOS port of jackline (I do), we will need an alternative to Lwt_unix.getaddrinfo
2) Practically all C implementations of getaddrinfo
(including this very OCaml FFI stub) have a history of security vulnerabilities, I'm sad we expose our users to this risk by feeding untrusted network data into this function.
3) I believe we should eat our own dog food.
superseeded by #205 thanks!
ping @hannesm