-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
xrdp fails without IPv6 #714
Comments
I appriciate your detailed explanation and spending your time to solve the issue. I'll read it later. Either if this is to satisfaction or not, can you try to send your patch as a pull request? We review patches on github. If you patch submitted as a pull request, we can comment on each line of diffs. These documents will help you. |
Didn't we already fix this issue months ago?
|
Thanks for links to information, I need to read! However I'm confused about git. Things can be done in so many ways. Is it "master" or "devel" branch I should begin with? Can I do like this: |
Please use devel branch for your Pull Requests.
בתאריך יום ד׳, 29 במרץ 2017 ב-12:40 מאת MichaelSweden <
[email protected]>:
Thanks for links to information, I need to read!
However I'm confused about git. Things can be done in so many ways. Is it
"master" or "devel" branch I should begin with?
Can I do like this:
git checkout devel
git fetch https://github.com/neutrinolabs/xrdp
git pull --rebase --autostash
git checkout -b support-disabled-ipv6
git add -u
git commit -m"Fix to handle OS disabled IPv6, issue #714.
- Changes made only in the os_calls.c file.
- Exported functions changed: g_tcp_bind g_tcp_bind_address g_tcp_connect
- Support three network configurations:
1) Normal network, with IPv6
2) Partly disabled IPv6 via sysctl.conf
3) Total disabled IPv6 via grub"
git push origin support-disabled-ipv6:support-disabled-ipv6
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#714 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADTH1MOx7cjMoiEkHdQYXYPUmwMFf16Oks5rqicdgaJpZM4MrnTQ>
.
--
Idan Freiberg
PGP FP: 8108 7EC9 806E 4980 75F2 72B3 8AD3 2D04 337B 1F18
|
The "If IPv6 not supported, fall back to IPv4" Fixes #432: This only changed the socket call. The socket can fallback and open an IPv4 socket. Later on (doing connect) asking getaddrinfo for an IPv6 address, we probably will get an IPv6 address. That will fail. Maybe some OS version have given an IPv4 address back (in some configurations) and it could succeed. For bind we asked for AF_UNSPEC, so it would work for IPv4 address. However depending on how IPv6 is disabled (and other cituations in the system) the socket can succeed to open as IPv6, but only IPv4 is available. In that case we have an IPv6 socket, but get an IPv4 address. This will fail. This is how it is today with support for IPv6: I think I don't have access to push, PS. Here is a patch file with the extra debug text I had during part of the verification: |
@MichaelSweden thanks for the detailed explanation and research. |
- Changes made only in the os_calls.c file. - Exported functions changed: g_tcp_bind g_tcp_bind_address g_tcp_connect - Support three network configurations: 1) Normal network, with IPv6 2) Partly disabled IPv6 via sysctl.conf 3) Total disabled IPv6 via grub
Okay, now it is visible above that I managed to create a pull request. My first! I would like to point out that my proposal keeps the libcommon interface intact. Another solution would be to change the interface and use two functions like: g_tcp_socket_bind (if argument address is null, bind to any) and g_tcp_socket_connect (if argument address is null, bind to loopback). However that will be a bigger change. |
- Changes made only in the os_calls.c file. - Exported functions changed: g_tcp_bind g_tcp_bind_address g_tcp_connect - Support three network configurations: 1) Normal network, with IPv6 2) Partly disabled IPv6 via sysctl.conf 3) Total disabled IPv6 via grub
Solved by #714. |
No, solved by #717. |
In what build this problem was fixed? I have 0.9.1-7build1 and the problem still here. |
I have 0.9.1-7build1 and the problem still here. How I can have ipv4 and ipv6 ports listened simultaneously? |
This issue is not fixed in debian 9.1 (stretch) using the repo apt-get package for xrdp. I disabled ipv6 via the grub command line method as noted above, and xrdp will never accept connection. I enable ipv6 again, and xrdp accepts connection. Basically what I encountered, now, is pretty much identical to the debian bug reported in the past... Guys, please fix this, this is a very visible and frustrating issue to keep bouncing around. |
I would like to share some information. Sorry for this very late information. I found this out a couple of month ago. I thought my original problem was two, xrdp and maybe some generic network problem in OS. After release of Ubuntu 17.04 I took a closer look again. One problem was that the two machines I run on before behaved different. The one with more problem, the HP, is in use and cannot be used for more testing. However I found out a way to reproduce the problem. This I think is independed of hardware. I always can reproduce it by doing this: So doing this will result in broken xrdp (will not be able to start listen on socket). However I notice ssh server is working, so no major fault. The fault seems to be in xrdp only. So now I removed the normal xrdp (via apt) and instead took the source code, build and install (this include the fix I have done, i.e. this issue). Now it works! I didn't think about, nor test, this while I did this fix. So we got lucky, this was also fixed - great! PS. Still Ubuntu 17.04 using old version of xrdp, without this fix. |
I always can
reproduce it by doing this:
Ubuntu 17.04 + static IPv4 address + no network cable attached during
boot
Could this be due to DAD failing and no LL address being set on the interface?
I saw that issue in a completely different context.
|
Attempting to bind to an IPv4 address in xrdp.ini results in no ports being bound at all and xrdp failing to start on Debian 9.4. IPv6 does work on this machine and is configured with SLAAC. |
Raise your own issue if your issue is in a different context. |
This is the same issue. Attempting to set any listen address other than |
|
History
The xrdp service don't start on one of my machines, if nic uses static ip address. I have seen the fault on another machine (different hardware) also, however it depends on how Ubuntu Server 16.10 is installed. I skip all details but I can say that I found out that IPv6 isn't always available during start of services at boot. This let me find out that xrdp can only work if an IPv6 network (IPv6 address on the nic) is available. For me right now I have solved by start the xrdp a second time a few seconds later. However, if a user want to run without IPv6 it is problem.
Platform
Ubuntu Server 16.10 amd64 (also Xubuntu 16.10 and Kubuntu 16.10)
Version
xrdp 0.9.0~20160601+git703fedd-3, included in Ubuntu Server 16.10
(the proposal is done for the latest github code)
Fault
The error messages at boot of system:
xrdp-sesman [ERROR] bind error on port '3350': 22 (Invalid argument)
xrdp [ERROR] xrdp_listen_main_loop: listen error, possible port already in use
How to reproduce
Boot the machine and check that xrdp-sesman and xrdp services is runnig (sudo lsof -i). Also look in the syslog.
sudo systemctl stop xrdp
Disable IPv6 on the machine.
sudo systemctl start xrdp-sesman
-> xrdp service will not start.
Details
In function g_tcp_bind_flags, getaddrinfo() will find two addresses. An IPv4 and an IPv6. The IPv4 will failed to bind and the IPv6 will be ok to bind. However if the IPv6 isn't available only IPv4 will be found and it will fail to bind.
Source code file: common/os_calls.c
Disable IPv6
It is possible to disable IPv6 in several ways. Here are the two ways I have used:
d6A) /etc/sysctl.conf:
net.ipv6.conf.all.disable_ipv6 = 1
net.ipv6.conf.default.disable_ipv6 = 1
net.ipv6.conf.lo.disable_ipv6 = 1
sudo sysctl -p
d6B) grub:
Ubuntu: /etc/default/grub: GRUB_CMDLINE_LINUX_DEFAULT "ipv6.disable=1" ; sudo update-grub
CentOS: /etc/default/grub: GRUB_CMDLINE_LINUX "ipv6.disable=1" ; sudo grub2-mkconfig -o /boot/grub2/grub.cfg
Progress
I believe both IPv4 and IPv6 should be supported, It is an automatic mapping available. The socket should be AF_INET6 and use flag AI_V4MAPPED. Need to remove flag AI_ADDRCONFIG. This work, however if the IPv6 loopback interface are disabled, it fails on the loopback port. I needed to make a special handling of this. By making a bind to "::FFFF:127.0.0.1". I had a proposal, but... Now I discover something in the latest source code (github); Fixes #432, commit 849a807 "If IPv6 not supported, fall back to IPv4". This doesn't solve the issue I have seen. After reading more, I found that this was for CentOS. I install a virtual machine and tested. Yes - difference. However this was because IPv6 was disabled in difference ways. If I disable IPv6 in the same way (via grub) in Ubuntu, I got the same behaviour. I have done a lot of testing with getaddrinfo and bind. This with small test applications. I'm not satisfied with how getaddrinfo works. For example using AI_V4MAPPED | AI_ADDRCONFIG | AI_PASSIVE and AF_UNSPEC, will return IPv4 before IPv6 net. I realise that it seems to be best to have special handling of loopback and any. This without getaddrinfo. Check the string and if loopback, try bind in6addr_loopback (AF_INET6) and if fail bind to INADDR_LOOPBACK (AF_INET). When the same for any. Check the string and if any, try bind in6addr_any (AF_INET6) and if fail bind to INADDR_ANY (AF_INET). Now we need to take care of other addresses. I believe the purpose of this is to be able to let xrdp only listen on a specific interface/network (nic). Today this is handle by inet_pton (AF_INET) and inet_pton (AF_INET6). However it cannot handle IPv6 addresses correctly. Nowadays the scope id needs to be specified in an IPv6 address, inet_pton cannot handle it (so bind will fail). Here I see little strange in the code, getaddrinfo is used to find networks and when the address is created in different ways. I realize that this functionality is what getaddrinfo is aimed to do. It can also handle scope id (interface name), added to the end of an IPv6 address with a percent sign. It is actually possible to use getaddrinfo for both IPv4 and IPv6 address strings. It works as supposed to, at least the combinations I haved tested. However I haven't found a nice way to implement it, because it is a problem with the today design. The call to socket is done in g_tcp_socket, which is called before call to g_tcp_bind and g_tcp_bind_address (which uses getaddrinfo). However getaddrinfo needs to be called first and when socket, since it's only after getaddrinfo we know for which family the socket should be created. I see no other nice option than to force a change outside of this source file. Now I realize these functions are used from many places. Okay, so here I start to think more about if it would be possible to solve without making changes outside the os_calls.c file. I come up with a proposal! So here I present a proposal that keep the interface to the os_calls intact. During first verification I found out that g_tcp_connect also needed to be modified. First I did a small change, if failed a call to getaddrinfo("::FFFF:127.0.0.1"). This was ok for d6A, however not for d6B. Here it goes strange, well I was confused and tested a lot. Let short it to my end result. We need be able to do 3 different connect calls. The connect is done non-blocking, so connect can return EINPROGRESS. It isn't specified how this function shall do, however at error it is called again and again. So it probably shouldn't wait. After an in-progress the next call to connect will be ok, this no matter of arguments to the connect call. Now I also guess that the reason for the EISCONN thing in the code, is for this second call (on some OS). At final verification it shows that Xubuntu 16.04 behave differently, so I needed to change the connect call and have all 3 alternatives in the new function.
Proposal
When bind to loopback or any, try IPv6, IPv4 and IPv4 mapped (this also apply to the connect call).
For other addresses, use getaddrinfo to create the socket address and bind to it.
API functions changed: g_tcp_bind g_tcp_bind_address g_tcp_connect
Local functions removed: address_match g_tcp_bind_flags
Local functions added: bind_loopback getaddrinfo_bind connect_loopback
Extra comments
a) The list from getaddrinfo wasn't freed.
b) In function g_tcp_socket I think it is some strange code. The IPV6_V6ONLY option is read out and only if it isn't zero, it is written. So for example if define XRDP_ENABLE_IPV6ONLY the option is only written to 1 if it is already 1. I think this isn't the wanted behavior.
c) It was a bug in g_tcp_connect that could result in infinite loop, the row "rp = h" within the for loop.
d) Please remove trailing white space in source code files.
e) g_tcp_connect is only fixed for the loopback address.
f) While do IPv6 connection from rdesktop, the local interface (nic name) needs to be given in the address, i.e.
x::x:x:x:x%<interface>
.My build-A
$ sudo apt install devscripts dpkg-dev
$ sudo apt build-dep xrdp
$ apt source xrdp
$ cd
Edit os_calls.c (merged my new stuff and fix432 into the old file)
$ debuild -us -uc -b
My build-B
$ sudo apt install gcc make autoconf automake libtool pkgconf nasm git
$ sudo apt install xserver-xorg-dev libssl-dev libpam0g-dev libxfixes-dev libxrandr-dev
$ git clone --recursive https://github.com/neutrinolabs/xrdp
$ cd xrdp
$ ./bootstrap
$ ./configure --enable-ipv6
$ patch -p1 -i MyPatch.patch
$ make
$ sudo make install
$ cd ..
$ git clone --recursive https://github.com/neutrinolabs/xorgxrdp
$ cd xorgxrdp
$ ./bootstrap
$ ./configure
$ make
$ sudo make install
$ sudo ln -s /usr/local/sbin/xrdp /usr/sbin
$ sudo ln -s /usr/local/sbin/xrdp-sesman /usr/sbin
$ sudo systemctl enable xrdp
$ reboot
Test plan
Check points for each configuration:
Configurations to test:
%<interface>
).Verification
Disclaimer
I haven't tested other build configurations or operating systems.
Patch file
xrdp20170327proposal.patch.txt
Comments are welcome!
PS. If this is to satisfaction, I can push to git. However I'm new here on github, so please bare with me if I ask details on how I should do.
Best regards
Michael
The text was updated successfully, but these errors were encountered: