Bug 3349 - Test sshd in chroot fails when syscall close_range is undefined and openssh is built with glibc 2.34
Summary: Test sshd in chroot fails when syscall close_range is undefined and openssh i...
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: sshd (show other bugs)
Version: 8.7p1
Hardware: All Linux
: P5 normal
Assignee: Assigned to nobody
URL:
Keywords:
Depends on:
Blocks: V_8_9
  Show dependency treegraph
 
Reported: 2021-09-23 08:36 AEST by william.wilson
Modified: 2022-07-31 04:39 AEST (History)
3 users (show)

See Also:


Attachments
failed ssh and sshd logs (72.60 KB, text/x-log)
2021-09-23 08:36 AEST, william.wilson
no flags Details
shim closefrom and check kernel version (1.59 KB, patch)
2021-10-08 18:17 AEDT, Darren Tucker
no flags Details | Diff
use close_range with fallback (2.02 KB, patch)
2021-10-08 20:44 AEDT, Darren Tucker
no flags Details | Diff
remove unneeded includes (1.42 KB, patch)
2021-10-08 22:10 AEDT, Darren Tucker
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description william.wilson 2021-09-23 08:36:04 AEST
Created attachment 3548 [details]
failed ssh and sshd logs

This was discovered due to a failing test: regress/sftp-chroot. The host kernel is 5.4, and a container is using glibc 2.34 to build openssh. When attempting to start sshd in the chroot, the libc fallback_closefrom function attempts to read /proc/self/fd, which is not present in the chroot. This glibc implementation of closefrom proceeds to fail silently, causing the sftp-chroot test to fail.

The test works on kernels with close_range defined, because the fallback is never reached. I have so far recreated the failure on amd64, arm64, and armhf. The attached logs are from an amd64 failure.

Running ./configure with ac_cv_func_closefrom=no resolves this failure. We will be doing this in Ubuntu for the time being, but if there is a better solution for handling this glibc implementation of closefrom we will implement that instead.
Comment 1 william.wilson 2021-09-23 08:37:35 AEST
I have also created https://sourceware.org/bugzilla/show_bug.cgi?id=28377 to track this issue with glibc.
Comment 2 Darren Tucker 2021-09-23 13:41:13 AEST
I'm not sure what OpenSSH could do about this other than entirely ignoring closefrom on Linux.  closefrom returns void so we are not aware of it having failed at runtime.  Detecting this at build time would require root permissions, and since the behaviour varies with running kernel version it probably wouldn't help much anyway.
Comment 3 Florian Weimer 2021-09-23 21:26:36 AEST
Does closefrom actually return in your test, or does it crash the process?

It is impossible to implement closefrom emulation on Linux without access to /proc: the descriptor range is not constrained by rlimit and can theoretically extend up to INT_MAX. If you want something that does not terminate the process, you need to call close_range and perform your own emulation instead.
Comment 4 Darren Tucker 2021-10-08 13:26:57 AEDT
(In reply to Florian Weimer from comment #3)
> Does closefrom actually return in your test, or does it crash the
> process?

Based on the glibc sources it looks like it throws an error and terminates due to FORTIFY_SOURCE:

void
__closefrom (int lowfd)
{
  int maxfd = __getdtablesize ();
  if (maxfd == -1)
    __fortify_fail ("closefrom failed to get the file descriptor table size");
Comment 5 Darren Tucker 2021-10-08 18:17:34 AEDT
Created attachment 3550 [details]
shim closefrom and check kernel version

The best we could come up with to still be able to use the native closefrom was to shim it, check the kernel version at run time and call the existing fallback function if the kernel was older than the first version that had close_range.  Please try this patch (I don't have anything with glibc 2.34 to test on).

I'm not sure if this is better or worse than disabling native closefrom() entirely on Linux.

We also discussed moving the last closefrom for this case to immediately before the chroot but we were less confident in that.
Comment 6 Florian Weimer 2021-10-08 20:01:51 AEDT
(In reply to Darren Tucker from comment #5)
> Created attachment 3550 [details]
> shim closefrom and check kernel version

This patch is not the right way to do this. You should call close_range (either the glibc wrapper or the system call via the generic syscall function) and perform emulation as a fallback. Failure with close_range (including lack of kernel support) does not terminate the process.
Comment 7 Darren Tucker 2021-10-08 20:44:19 AEDT
Created attachment 3551 [details]
use close_range with fallback

(In reply to Florian Weimer from comment #6)
> This patch is not the right way to do this. You should call
> close_range (either the glibc wrapper or the system call via the
> generic syscall function) and perform emulation as a fallback.
> Failure with close_range (including lack of kernel support) does not
> terminate the process.

oh, I didn't realise close_range was exposed by glibc.
Comment 8 Florian Weimer 2021-10-08 21:40:36 AEDT
(In reply to Darren Tucker from comment #7)
> Created attachment 3551 [details]
> use close_range with fallback
> 
> (In reply to Florian Weimer from comment #6)
> > This patch is not the right way to do this. You should call
> > close_range (either the glibc wrapper or the system call via the
> > generic syscall function) and perform emulation as a fallback.
> > Failure with close_range (including lack of kernel support) does not
> > terminate the process.
> 
> oh, I didn't realise close_range was exposed by glibc.

Thanks, this patch looks much better to me. However, including <linux/close_range.h> does not add value here. The prototype is declared in <unistd.h> (if at all), and you don't need any constants from the UAPI header.
Comment 9 Darren Tucker 2021-10-08 22:10:05 AEDT
Created attachment 3552 [details]
remove unneeded includes

thanks for the feedback.  I'll put this on the list for the next release.  I'd appreciate someone with a glibc 2.34 system could test that it actually solves the problem.
Comment 10 william.wilson 2021-11-09 02:41:36 AEDT
I have tested the patch in attachment 3552 [details] and can confirm it is working.
Comment 11 Darren Tucker 2021-11-10 12:41:45 AEDT
Thanks.  Patch applied and will be in the next major release.
Comment 12 Damien Miller 2022-02-25 13:57:17 AEDT
closing bugs resolved before openssh-8.9