ID: 599PHID: PHID-TASK-jwbjbkgnxkowp7oe2wvrAuthor: PatrickStatus at Migration Time: resolvedPriority at Migration Time: Normal
Whonix forked bindp#uwt (T561) (bindp.c ).
Compilation:
TODO:
! In T561#11378, @marmarek wrote:bind calls - even those with different address family than AF_INET. In that case, address structure may be different (and also have different size) and strange things may happen. If a particular application use only AF_INET binds, it should be ok. You can check with strace and grep.
/path/to/checksec.sh --file /path/to/libindp.so
RELRO STACK CANARY NX PIE RPATH RUNPATH FILE
Full RELRO No canary found NX enabled PIE enabled No RPATH No RUNPATH libindp.so
This ticket is based on @marmarek ’s feedback in T561#11378.
2017-02-22 08:41:46 UTC
Unfortunately the problem with this code has not been explained clear enough. It says “The only issue I see is it replace address in all bind calls” but in file ‘bindp/bindp.c at master · Kicksecure/bindp · GitHub ’ there is no check for address family in your bind function, instead the check is done in connect function at line 145. Unless the packet which is sent to this connect function is corrupted I don’t see a reason that the condition (rsk_in->sin_family == AF_INET) return a wrong result. A little more explanation about the problem would help solve it better.
2017-02-22 09:07:19 UTC
2017-02-22 10:12:25 UTC
2017-02-22 10:28:54 UTC
2017-02-22 13:29:03 UTC
The purpose of bindp in Whonix is to force applications listening on localhost to rather listen on eth0 so these are reachable from the gateway. Essentially, it lets us use onionshare, ricochet, unMessage and ZeroNet in Whonix 14. More details in T561.
The code where it is being used is this one:https://github.com/Whonix/uwt/blob/master/usr/lib/uwtwrapper#L238-L247
Using it manually without all the setup, this should emulate it:
## requires Debian stretch because ifconfig output changed
BIND_ADDR="$(/sbin/ifconfig "eth0" | grep -oP 'inet \K\S+')"
LD_PRELOAD+=" /path/to//usr/lib/uwt/libindp.so/
export BIND_ADDR
export LD_PRELOAD
run some server application that would listen to 127.0.0.1 as its default)but would now listen on eth0 instead
2017-02-22 14:06:53 UTC
Ok but I still don’t know when exactly the problem occurs. The only place where the protocol family is checked is at line 145. This is all the scenario of the program:
An external program calls bind or connect
If protocol family of the remote address is AF_INET then bind the socket to another address which we already specified.
From the person who reported the issue with this code, all I understand is that for families other than AF_INET still the bind takes place, meaning the condition at line 145 becomes true even while the rsk_in->sin_family is something different than AF_INET. Is this what you say?
2017-02-23 00:28:23 UTC
2017-02-23 01:03:37 UTC
The problem is not in connect function, but in bind. It assume AF_INET family, casting sk pointer to struct sockaddr_in. But if socket family is something different, the structure also may be different (for example struct sockaddr_un for AF_UNIX). In practice, besides misusing this library, the problem only applies to applications listening on both locak (AF_UNIX) and network (AF_INET) sockets. Because you don’t use this library for AF_UNIX-only applications.
Created an issue for that: bind wrapper does not check for address family · Issue #5 · yongboy/bindp · GitHub
2017-03-05 21:26:02 UTC
@marmarek If the problem is only with applications listening on both AF_INET and AF_UNIX, I think the solution should be easy, we can change the bind function in a way that it only changes local address of the sockaddr_storage while the protocol family is AF_INET. For AF_UNIX address family there doesn’t seem to be any need to change the address (the pathname of the socket). By applying these changes will the problem get solved? If yes, I will rewrite the bind function for you to test.
2017-03-05 21:34:14 UTC
@marmarek If the problem is only with applications listening on both AF_INET and AF_UNIX,
Yes, I think this is the only problematic case in practice. In theory
BTW what about children processes? Those also get LD_PRELOAD variable.
I think the solution should be easy, we can change the bind function in a way that it only changes local address of the sockaddr_storage while the protocol family is AF_INET. For AF_UNIX address family there doesn’t seem to be any need to change the address (the pathname of the socket). By applying these changes will the problem get solved? If yes, I will rewrite the bind function for you to test.
Yes, this should work.
2017-03-06 18:16:47 UTC
@marmarek I changed bind function and uploaded the code at: bindp revision - Pastebin.com
You only need to replace this function with the one in the main code. Please test and report the results to me.
2017-03-06 18:48:16 UTC
2017-03-06 18:56:51 UTC
2017-03-06 19:24:37 UTC
2017-03-06 19:38:53 UTC
intent:intent · Kicksecure/bindp@37a6300 · GitHub
Do you know how to fix these compiler warning?
gcc -nostartfiles -fpic -shared bindp.c -o libindp.so -ldl -D_GNU_SOURCE
bindp.c: In function ‘_init’:
bindp.c:83:27: warning: implicit declaration of function ‘inet_addr’ [-Wimplicit-function-declaration]
bind_addr_saddr = inet_addr (bind_addr_env);
^~~~~~~~~
bindp.c: In function ‘bind’:
bindp.c:129:21: warning: implicit declaration of function ‘inet_ntop’ [-Wimplicit-function-declaration]
inet_ntop(AF_INET,&(lsk_in->sin_addr),original_ip,INET_ADDRSTRLEN);
^~~~~~~~~
2017-03-06 19:41:55 UTC
2017-03-06 19:46:44 UTC
! In T599#12599, @s.sh wrote:@Patrick Add
#include <arpa/inet.h>
at the beginning of the file.
Great! That worked. Added:
fix compilation warnings · Kicksecure/bindp@09ea8ad · GitHub
Do you know how to fix this ld warning?
gcc -nostartfiles -fpic -shared bindp.c -o libindp.so -ldl -D_GNU_SOURCE -D_FORTIFY_SOURCE=2 -g -O2 -fPIE -fstack-protector-strong -Wformat -Werror=format-security -fPIE -pie -Wl,-z,relro -Wl,-z,now
/usr/bin/ld: warning: cannot find entry symbol _start; defaulting to 00000000000006e0
2017-03-06 20:03:00 UTC
@Patrick You can compile without -fPIE (I think -fPIC is enough):
gcc -nostartfiles -fpic -shared bindp.c -o libindp.so -ldl -D_GNU_SOURCE -D_FORTIFY_SOURCE=2 -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Wl,-z,relro -Wl,-z,now
But these warnings are not related to my revisions, they probably existed in original code.
2017-03-06 20:27:11 UTC
! In T599#12601, @s.sh wrote:
Yes. That is for sure. These are just part of the original issue description in this ticket at the very top. Was wondering if you’d like to fix those while you are at bindp.
@Patrick You can compile without -fPIE (I think -fPIC is enough):
gcc -nostartfiles -fpic -shared bindp.c -o libindp.so -ldl -D_GNU_SOURCE -D_FORTIFY_SOURCE=2 -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Wl,-z,relro -Wl,-z,now
That indeed removes the ld warning but apparently reduced hardening. We’d like to have all compiler hardening flags enabled for better security. (As long as sensible in this situation.)
gcc -nostartfiles -fpic -shared bindp.c -o libindp.so -ldl -D_GNU_SOURCE -D_FORTIFY_SOURCE=2 -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Wl,-z,relro -Wl,-z,now results in:
RELRO STACK CANARY NX PIE RPATH RUNPATH FILE
Full RELRO Canary found NX enabled DSO No RPATH No RUNPATH libindp.so
PIE, DSO is shown as yellow by checksec.sh. What does DSO mean? Before it was green PIE enabled.
Can we have all hardening with PIE enabled as well as without ld warning?
2017-03-06 20:36:44 UTC
Can we have all hardening with PIE enabled as well as without ld warning?
I don’t think so, but I think PIE is more necessary for regular executables not dynamic libraries, since libraries are relocatable by nature. (Of course PIE adds some more optimization for ASLR mechanism, however)
2017-03-06 20:48:49 UTC
2017-03-07 13:26:20 UTC
@Patrick Add
int main(int argc,char **argv){return 0;}
to the end of bindp.c, now compile with :
gcc -nostartfiles -fpic -shared --entry main bindp.c -o libindp.so -ldl -D_GNU_SOURCE -D_FORTIFY_SOURCE=2 -g -O2 -fPIE -fstack-protector-strong -Wformat -Werror=format-security -fPIE -pie -Wl,-z,relro -Wl,-z,now
The warning should disappear
2017-03-07 15:55:00 UTC
2017-03-07 19:14:27 UTC
@Patrick Nice, now regular testing needs to be done from your side, please keep in mind that connect function has to change as well, but before that I must assure that current bind() works properly and as expected. If everything goes well I will write whole code from scratch for you by modifying init and connect functions.
2017-03-07 21:35:44 UTC
That’s great! Thank you for your help! Just now tested. Unfortunately it does not work.
user@host:~$ BIND_ADDR=10.137.11.80
user@host:~$ LD_PRELOAD=/usr/lib/bindp/libindp.so
user@host:~$ export BIND_ADDR
user@host:~$ export LD_PRELOAD
user@host:~$ /usr/bin/onionshare.anondist-orig a
Segmentation fault
2017-03-07 21:51:24 UTC
no segfault:
gcc -nostartfiles -fpic -shared --entry main bindp.c -o libindp.so -ldl -D_GNU_SOURCE -D_FORTIFY_SOURCE=2 -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Wl,-z,relro -Wl,-z,now
segfault:
gcc -nostartfiles -fpic -shared --entry main bindp.c -o libindp.so -ldl -D_GNU_SOURCE -fPIE -pie -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -fdebug-prefix-map=/home/user/bindp=. -fstack-protector-strong -Wformat -Werror=format-security -Wl,-z,relro -Wl,-z,now
user@host:~/bindp$ LD_PRELOAD=/home/user/bindp/libindp.so onionshare.anondist-orig /home/user/a
Segmentation fault
I think we can safely remove -fPIE? That solves the segfault and still shows all hardening flags.
2017-03-07 21:58:44 UTC
2017-03-07 22:00:34 UTC
It’s functional! Successfully changes the listener IP.
/usr/bin/onionshare.anondist-orig a
Onionshare 0.9.1 | https://onionshare.org/
[-] LIB received AF_INET bind request
[!] AF_INET: Leaving request unchanged
[-] LIB received AF_INET bind request
[!] AF_INET: Leaving request unchanged
Connecting to Tor control port to set up onion service on port 17600.
Staring ephemeral Tor onion service and awaiting publication
Preparing files to share.
[-] LIB received AF_INET bind request
[!] AF_INET: Leaving request unchanged
[-] LIB received AF_INET bind request
[!] AF_INET: Leaving request unchanged
* Running on http://127.0.0.1:17600/ (Press CTRL+C to quit)
Give this URL to the person you're sending the file to:
http://3i4em3y4rkxbntqj.onion/daley-err
Press Ctrl-C to stop server
2017-03-08 06:22:46 UTC
Well the segfault error is strange here, I must run it locally with your setup to check and debug which is not possible for now, but it’s good you made it work finally. Good job Patrick. Still I think PIE related options are not needed for libraries.
2017-03-08 14:54:28 UTC
Okay, great!
Why did you add #ifdef SO_REUSEPORT?
if (l_bind_addr && l_bind_port) {
As a minor point, I think the debug messages are slightly broken. It shows [!] AF_INET: Leaving request unchanged since we are only setting envrionment variable BIND_ADDR and leaving BIND_PORT unset.
2017-03-08 18:27:53 UTC
! In T599#12621, @Patrick wrote:
Why did you add #ifdef SO_REUSEPORT?
It was in the original code; not added by me. SO_REUSEPORT is defined in asm-generic/socket.h but apparently the original author wanted to make sure it is defined.
As a minor point, I think the debug messages are slightly broken. It shows [!] AF_INET: Leaving request unchanged since we are only setting envrionment variable BIND_ADDR and leaving BIND_PORT unset.
Yes I know, I will fix it.
2017-03-08 19:37:16 UTC
2017-03-08 21:48:17 UTC
Added that. And added some intent style changes. Please tell me if my C intent style is actually making things non-standard / worse than before.
bindp/bindp.c at address-family-check · Kicksecure/bindp · GitHub
That version is currently broken. (In comparison git master version is still functional which makes me reasonable sure nothing else in the setup is broken.)
Onionshare 0.9.1 | https://onionshare.org/
[-] LIB received AF_INET bind request
[-] Changing 127.0.0.1 to 10.137.11.80
[-] AF_INET: Leaving port unchanged
[-] LIB received AF_INET bind request
[-] Changing 127.0.0.1 to 10.137.11.80
[-] AF_INET: Leaving port unchanged
[-] LIB received AF_INET bind request
[-] Changing 127.0.0.1 to 10.137.11.80
[-] AF_INET: Leaving port unchanged
[-] LIB received AF_INET bind request
[-] Changing 127.0.0.1 to 10.137.11.80
[-] AF_INET: Leaving port unchanged
Can't connect to Tor control port on port [9151, 9153, 9051]. OnionShare requires Tor Browser to be running in the background to work. If you don't have it you can get it from https://www.torproject.org/.
Might it perhaps bend a few too many connections, i.e does it bend now connections for 127.0.0.1 that it should not change?
2017-03-08 22:05:01 UTC
2017-03-08 22:40:31 UTC
Onionshare 0.9.1 | https://onionshare.org/
[-] LIB received AF_INET bind request
[-] Changing 127.0.0.1 to 10.137.11.80
[-] AF_INET: Leaving port unchanged
[!] connect(): AF_INET connect() call
[-] LIB received AF_INET bind request
[-] Changing 127.0.0.1 to 10.137.11.80
[-] AF_INET: Leaving port unchanged
[!] connect(): AF_INET connect() call
[-] LIB received AF_INET bind request
[-] Changing 127.0.0.1 to 10.137.11.80
[-] AF_INET: Leaving port unchanged
[!] connect(): AF_INET connect() call
[-] LIB received AF_INET bind request
[-] Changing 127.0.0.1 to 10.137.11.80
[-] AF_INET: Leaving port unchanged
Can't connect to Tor control port on port [9151, 9153, 9051]. OnionShare requires Tor Browser to be running in the background to work. If you don't have it you can get it from https://www.torproject.org/.
Could you please base any other changes on top of the current bindp/bindp.c at address-family-check · Kicksecure/bindp · GitHub so it keeps intent and typo fixes?
2017-03-09 15:58:45 UTC
2017-03-09 16:40:08 UTC
Works for me!
/usr/bin/onionshare.anondist-orig a
Onionshare 0.9.1 | https://onionshare.org/
[-] LIB received AF_INET bind request
[-] Changing 127.0.0.1 to 10.137.11.80
[-] AF_INET: Leaving port unchanged
[-] connect(): AF_INET connect() call, binding to local address
[-] LIB received AF_INET bind request
[-] Changing 10.137.11.80 to 10.137.11.80
[-] AF_INET: Leaving port unchanged
Connecting to Tor control port to set up onion service on port 17600.
Staring ephemeral Tor onion service and awaiting publication
Preparing files to share.
[-] LIB received AF_INET bind request
[-] Changing 127.0.0.1 to 10.137.11.80
[-] AF_INET: Leaving port unchanged
[-] connect(): AF_INET connect() call, binding to local address
[-] LIB received AF_INET bind request
[-] Changing 10.137.11.80 to 10.137.11.80
[-] AF_INET: Leaving port unchanged
* Running on http://127.0.0.1:17600/ (Press CTRL+C to quit)
Give this URL to the person you're sending the file to:
[redacted]
sudo netstat -tulpen | grep python
tcp 0 0 10.137.11.80:17600 0.0.0.0:* LISTEN 1000 375953 20647/python3
(The usual operating mode of onionshare [that has nothing to do with bindp] is that it first creates the ephemeral Tor hidden service using Tor control protocol commands add_onion etc. and once is notices that it’s ready starts the webserver on 127.0.0.1. [Which #uwt / #bindp bends to eth0’s IP in this example 10.137.11.80.])
2017-03-09 16:50:24 UTC
2017-03-09 18:19:03 UTC
Functionally speaking, as I tested, it works great in Whonix. Pretty much done.
Do you see any things a malicious application could to gain arbitrary code execution through bindp?
In the final version we need to disable debug mode by default, but that is trivial. (Already tried that and it’s working.)
For now, the only missing thing is a code review for bindp/bindp.c at address-family-check · Kicksecure/bindp · GitHub . Only @marmarek comes to mind who could do that.
Lower priority, but I’d still like to see these changes upstreamed (git pull request to the original author at GitHub - yongboy/bindp: Binding specific IP and Port fro Linux Running Application ). That would give us another code review, prevent us maintaining a fork at Whonix, and we’d benefit from further development/review by others.
2017-03-09 18:35:46 UTC
2017-03-10 11:51:17 UTC
Do you see any things a malicious application could to gain arbitrary code execution through bindp?
Very unlikely. I used standard socket functions. If a malicious program could use bindp to gain code execution it most probably can do the same with regular socket applications too. But I will check it more observantly when I got the time to.
Lower priority, but I’d still like to see these changes upstreamed (git pull request to the original author at GitHub - yongboy/bindp: Binding specific IP and Port fro Linux Running Application ). That would give us another code review, prevent us maintaining a fork at Whonix, and we’d benefit from further development/review by others.
From experience, I can say if the person does not answer within 2 days he won’t answer at all.
2017-03-26 11:53:36 UTC
2017-03-26 11:53:41 UTC
2017-06-16 15:41:23 UTC
2017-06-16 16:15:35 UTC
2017-06-16 17:01:04 UTC
2017-06-16 17:47:10 UTC
2017-06-16 18:16:56 UTC
2017-06-16 18:21:09 UTC
What about…?
I believe the answer to the question that I mentioned in the comment, needs more extensive research, maybe at a later time, with having that fact in mind, I think the code would be prettier if this comment block is removed. Now as you wish.
2017-07-03 11:52:48 UTC
2017-07-03 18:54:58 UTC