| Summary: | chroot and shell check ambiguity | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Portable OpenSSH | Reporter: | Alex Beregszaszi <alex> | ||||||||||
| Component: | sshd | Assignee: | Assigned to nobody <unassigned-bugs> | ||||||||||
| Status: | CLOSED FIXED | ||||||||||||
| Severity: | normal | CC: | djm | ||||||||||
| Priority: | P2 | ||||||||||||
| Version: | 5.3p1 | ||||||||||||
| Hardware: | Other | ||||||||||||
| OS: | All | ||||||||||||
| Bug Depends on: | |||||||||||||
| Bug Blocks: | 1626 | ||||||||||||
| Attachments: |
|
||||||||||||
Created attachment 1738 [details]
Initial patch
Attached a wrong patch first.
Created attachment 1777 [details]
/home/djm/auth-check-chroot-shell.diff
tweaked patch
Created attachment 1778 [details]
/home/djm/auth-check-chroot-shell.diff
oops, patch without tabs vs. space issues
Comment on attachment 1778 [details] /home/djm/auth-check-chroot-shell.diff >+ strcasecmp(options.chroot_directory, "none") != 0) { Do we do case-insensitive matches on "none"? (In reply to comment #4) > (From update of attachment 1778 [details]) > >+ strcasecmp(options.chroot_directory, "none") != 0) { > > Do we do case-insensitive matches on "none"? We are inconsistent already, I'll send a diff to use case-insensitive comparisons everywhere. (In reply to comment #4) > (From update of attachment 1778 [details]) > >+ strcasecmp(options.chroot_directory, "none") != 0) { > > Do we do case-insensitive matches on "none"? This is consistent with the other match against "none" for chrootdirectory, but in general we are not consistent in matching :( Anyway, patch applied - this will be in OpenSSH 5.4 - thanks for the report and patch! With the release of 5.4p1, this bug is now considered closed. |
Created attachment 1737 [details] Initial patch OpenSSH will fail in a scenario where the server is configured with chroot and a shell used by a user is not available outside, just inside the chroot. The reason behind this is that ssh checks whether the given shell is a file and is executable, but this check doesn't takes the chroot path into account. I also attach an initial patch diffed against CVS (checked out few minutes ago), which I am not happy with but it can stand here as a record. I copied the chroot part from session.c