radmin - infinite loop
This is with freeradius 2.1.10 The server is running as root (yes I know, but I can't change that easily right now). radmin works fine with: listen { type = control socket = ${run_dir}/${name}.sock uid = root mode = rw } Now, in order to step towards interfacing to a web app, I changed this to uid = www-data Of course, I can't yet connect from www-data because the server is running as root, and the socket is owned by 0/0 mode 550. But the strange thing is, if I run radmin as root, it no longer works; it doesn't even display the startup banner. strace shows that it's going into an infinite loop: ... set_tid_address(0x7f2fd45419d0) = 7688 set_robust_list(0x7f2fd45419e0, 0x18) = 0 futex(0x7fff3b496cdc, FUTEX_WAKE_PRIVATE, 1) = 0 futex(0x7fff3b496cdc, FUTEX_WAIT_BITSET_PRIVATE|FUTEX_CLOCK_REALTIME, 1, NULL, 7f2fd4541700) = -1 EAGAIN (Resource temporarily unavailable) rt_sigaction(SIGRTMIN, {0x7f2fd3abd870, [], SA_RESTORER|SA_SIGINFO, 0x7f2fd3ac78f0}, NULL, 8) = 0 rt_sigaction(SIGRT_1, {0x7f2fd3abd900, [], SA_RESTORER|SA_RESTART|SA_SIGINFO, 0x7f2fd3ac78f0}, NULL, 8) = 0 rt_sigprocmask(SIG_UNBLOCK, [RTMIN RT_1], NULL, 8) = 0 getrlimit(RLIMIT_STACK, {rlim_cur=8192*1024, rlim_max=RLIM_INFINITY}) = 0 socket(PF_FILE, SOCK_STREAM, 0) = 3 connect(3, {sa_family=AF_FILE, path="/var/run/freeradius/freeradius.sock"}, 37) = 0 read(3, "", 8) = 0 read(3, "", 8) = 0 read(3, "", 8) = 0 read(3, "", 8) = 0 read(3, "", 8) = 0 read(3, "", 8) = 0 read(3, "", 8) = 0 read(3, "", 8) = 0 read(3, "", 8) = 0 ... etc forever I presume it's stuck here: /* * Read initial magic && version information. */ for (size = 0; size < 8; size += len) { len = read(sockfd, buffer + size, 8 - size); if (len < 0) { fprintf(stderr, "%s: Error reading initial data from socket: %s\n", progname, strerror(errno)); exit(1); } } And this I imagine is because of the uid test failing in the server, and the connection being dropped. Normally read() returning 0 would indicate an end-of-file condition, but for some reason that I don't understand, the socket is being set into non-blocking mode, so it's not as simple as that. However, I thought that if no data was available you'd get -1 and EAGAIN or EWOULDBLOCK? But as the code is currently written, it aborts on any condition where len < 0. I'm no expert in this area, but I suggest something like this (untested): for (size = 0; size < 8; ) { len = read(sockfd, buffer + size, 8 - size); if (len < 0) { if (errno == EAGAIN || errno == EWOULDBLOCK) continue; fprintf(stderr, "%s: Error reading initial data from socket: %s\n", progname, strerror(errno)); exit(1); } if (len == 0) { fprintf(stderr, "%s: Connection dropped by server\n", progname); exit(1); } size += len; } A couple of other suggestions. (1) When checking the uid of the socket peer, if you find it's 0 (root) you may as well let them in anyway. (2) When starting the control server, if the 'uid' or 'gid' values are set, it could try chown'ing or chgrp'ing the socket to that value. Thanks, Brian.
Also, I think I found another problem: in command.c, I believe that if (getpeereid(listener->fd, &uid, &gid) < 0) { should be if (getpeereid(newfd, &uid, &gid) < 0) { Because right now, it reports that connections are always from uid 0, even if the caller is definitely a different uid. Client window: $ sudo su - www-data $ id uid=33(www-data) gid=33(www-data) groups=33(www-data) $ ruby -e 'puts Process.euid, Process.uid' 33 33 $ strace /usr/sbin/radmin (same infinite loop as before) Server output shows the following: Ready to process requests. ... new connection request on command socket. Unauthorized connection to /var/run/freeradius/freeradius.sock from gid 0 Ready to process requests. (But it's not gid 0, it's gid 33!) strace on the freeradius -X: write(1, "Ready to process requests.\n", 27Ready to process requests. ) = 27 select(211, [203 205 206 207 208 209 210], NULL, NULL, NULL) = 1 (in [207]) write(1, " ... new connection request on c"..., 47 ... new connection request on command socket. ) = 47 accept(207, {sa_family=AF_FILE, NULL}, [2]) = 211 getsockopt(207, SOL_SOCKET, SO_PEERCRED, "\231%\0\0\0\0\0\0\0\0\0\0", [12]) = 0 write(1, "Unauthorized connection to /var/"..., 74Unauthorized connection to /var/run/freeradius/freeradius.sock from uid 0 ) = 74 close(211) = 0 write(1, "Ready to process requests.\n", 27Ready to process requests. ) = 27 select(211, [203 205 206 207 208 209 210], NULL, NULL, NULL So getsockopt returns 0, which is success. I just think it's asking about the wrong socket. Regards, Brian.
Brian Candler wrote:
Also, I think I found another problem: in command.c, I believe that ... Because right now, it reports that connections are always from uid 0, even if the caller is definitely a different uid.
Oops. See github. I've pushed a bunch of patches to fix all of this. Alan DeKok.
See github. I've pushed a bunch of patches to fix all of this
Thanks Alan. A couple of minor suggestions: (1) If sock->uid_name or sock->gid_name are not provided, I can't see where sock->uid or sock->gid get a value - so I'm guessing they get zero. However, if they were initialised to -1 instead, then you could use fchown() to set just the uid or the gid (if only one were set in the config file). AFAICS, you have to provide both now to get the fchown() to run. (2) If both 'uid' and 'gid' are set, might it not be better to allow the user in if they match either? At the moment they have to match both. Regards, Brian.
Brian Candler wrote:
See github. I've pushed a bunch of patches to fix all of this
Thanks Alan. A couple of minor suggestions:
(1) If sock->uid_name or sock->gid_name are not provided, I can't see where sock->uid or sock->gid get a value - so I'm guessing they get zero.
Yes.
However, if they were initialised to -1 instead, then you could use fchown() to set just the uid or the gid (if only one were set in the config file). AFAICS, you have to provide both now to get the fchown() to run.
Good point. I'll take a look at that.
(2) If both 'uid' and 'gid' are set, might it not be better to allow the user in if they match either? At the moment they have to match both.
Maybe. That's a change in behavior, albeit minor. Alan DeKok.
On Tue, Jul 05, 2011 at 12:58:00PM +0200, Alan DeKok wrote:
(2) If both 'uid' and 'gid' are set, might it not be better to allow the user in if they match either? At the moment they have to match both.
Maybe. That's a change in behavior, albeit minor.
I thought about that, but then since it never worked until now, it's probably reasonable to define the desired behaviour :-) Cheers, Brian.
From latest commit:
* Don't chown it from (possibly) non-root to root. * Do chown it from (possibly) root to non-root. */ - if ((sock->uid != 0) && (sock->gid != 0)) { + if ((sock->uid != -1) || (sock->gid != -1)) { This is more or less what I was thinking of, but I notice that the comment no longer agrees with the behaviour.
Brian Candler wrote:
From latest commit:
* Don't chown it from (possibly) non-root to root. * Do chown it from (possibly) root to non-root. */ - if ((sock->uid != 0) && (sock->gid != 0)) { + if ((sock->uid != -1) || (sock->gid != -1)) {
This is more or less what I was thinking of, but I notice that the comment no longer agrees with the behaviour.
Yup. But it works. Alan DeKok.
Brian Candler wrote:
The server is running as root (yes I know, but I can't change that easily right now). radmin works fine with: ... I presume it's stuck here:
Yes. I've pushed a fix to github.
A couple of other suggestions.
(1) When checking the uid of the socket peer, if you find it's 0 (root) you may as well let them in anyway.
Good idea. Done.
(2) When starting the control server, if the 'uid' or 'gid' values are set, it could try chown'ing or chgrp'ing the socket to that value.
That, too. Alan DeKok.
participants (2)
-
Alan DeKok -
Brian Candler