Bug 23944
| Summary: | Patch for ftpd to add a cd after the chroot. | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | fschapachnik <fschapachnik> | ||||||||
| Component: | bin | Assignee: | Yar Tikhiy <yar> | ||||||||
| Status: | Closed FIXED | ||||||||||
| Severity: | Affects Only Me | ||||||||||
| Priority: | Normal | ||||||||||
| Version: | 4.2-RELEASE | ||||||||||
| Hardware: | Any | ||||||||||
| OS: | Any | ||||||||||
| Attachments: |
|
||||||||||
|
Description
fschapachnik
2000-12-29 16:20:01 UTC
I've rewritten some of your patch to not use dangling pointers. I also added the cd_dir functionality to the ftp guest account, not because I think it's a great idea, but because it will be consistent behavior. (If someone had specified /info/ftp/./pub as HOME for ftp, your patch would have chroot'd ftp into /info/ftp/pub, instead of /info/ftp. And this 'bug' would only appear for the ftp account. ;-)) In the event that this mail doesn't get munged the way I'd like, please respond to dhagan@colltech.com Thanks, Daniel Index: ftpcmd.y =================================================================== RCS file: /raid/ncvs/src/libexec/ftpd/ftpcmd.y,v retrieving revision 1.19 diff -u -r1.19 ftpcmd.y --- ftpcmd.y 2000/12/16 19:19:19 1.19 +++ ftpcmd.y 2001/01/04 15:55:42 @@ -92,6 +92,8 @@ extern char tmpline[]; extern int readonly; extern int noepsv; +extern int dochroot; +extern char *cd_dir, *chroot_dir; off_t restart_point; @@ -505,8 +507,11 @@ | CWD check_login CRLF { if ($2) { - if (guest) - cwd("/"); + if (guest || dochroot) + if (cd_dir != NULL) + cwd(cd_dir); + else + cwd("/"); else cwd(pw->pw_dir); } Index: ftpd.8 =================================================================== RCS file: /raid/ncvs/src/libexec/ftpd/ftpd.8,v retrieving revision 1.36 diff -u -r1.36 ftpd.8 --- ftpd.8 2000/12/18 08:33:25 1.36 +++ ftpd.8 2001/01/04 15:46:24 @@ -311,13 +311,14 @@ or the user is a member of a group with a group entry in this file, i.e. one prefixed with .Ql \&@ , -the session's root will be changed to the user's login directory by +the session's root will be changed to the user's login directory (up to the first /./) by .Xr chroot 2 as for an .Dq anonymous or .Dq ftp account (see next item). +The user is placed into the directory that remainds after stripping the former from the user's login directory. This facility may also be triggered by enabling the boolean "ftp-chroot" capability in .Xr login.conf 5 . Index: ftpd.c =================================================================== RCS file: /raid/ncvs/src/libexec/ftpd/ftpd.c,v retrieving revision 1.72 diff -u -r1.72 ftpd.c --- ftpd.c 2000/12/20 03:34:54 1.72 +++ ftpd.c 2001/01/04 15:56:59 @@ -140,6 +140,7 @@ int anon_only = 0; /* Only anonymous ftp allowed */ int guest; int dochroot; +char *cd_dir = NULL, *chroot_dir = NULL; int stats; int statfd = -1; int type; @@ -188,6 +189,9 @@ char *pid_file = NULL; +/* WARNING: FTP_CHROOT_SEPARATOR *MUST* end in / */ +#define FTP_CHROOT_SEPARATOR "/./" + /* * Timeout intervals for retrying connections * to hosts that don't accept PORT cmds. This @@ -251,6 +255,7 @@ static char *sgetsave __P((char *)); static void reapchild __P((int)); static void logxfer __P((char *, long, long)); +static void get_chroot_and_cd_dirs __P((char *, char **, char **)); static char * curdir() @@ -1038,6 +1043,8 @@ logged_in = 0; guest = 0; dochroot = 0; + free(chroot_dir); + free(cd_dir); } #if !defined(NOPAM) @@ -1291,19 +1298,20 @@ login_getcapbool(lc, "ftp-chroot", 0) || #endif checkuser(_PATH_FTPCHROOT, pw->pw_name, 1); - if (guest) { + if (guest || dochroot) { /* * We MUST do a chdir() after the chroot. Otherwise * the old current directory will be accessible as "." * outside the new root! */ - if (chroot(pw->pw_dir) < 0 || chdir("/") < 0) { - reply(550, "Can't set guest privileges."); - goto bad; - } - } else if (dochroot) { - if (chroot(pw->pw_dir) < 0 || chdir("/") < 0) { - reply(550, "Can't change root."); + get_chroot_and_cd_dirs(pw->pw_dir, &chroot_dir, &cd_dir); + /* + * Do not free chroot_dir & cd_dir b/c they are used in + * processing CWD commands from client. They should be + * free'd during a user logout. + */ + if (chroot(chroot_dir) < 0 || chdir(cd_dir) < 0) { + reply(550, guest ? "Can't set guest privileges." : "Can't change root."); goto bad; } } else if (chdir(pw->pw_dir) < 0) { @@ -2802,5 +2810,50 @@ ctime(&now)+4, ident, remotehost, path, name, size, now - start + (now == start)); write(statfd, buf, strlen(buf)); + } +} + +/* + * Make a pointer to the chroot dir and another to the cd dir. + * The first is all the path up to the first FTP_CHROOT_SEPARATOR. + * The later is the remaining chars, not including the FTP_CHROOT_SEPARATOR, + * but prepending a '/', if FTP_CHROOT_SEPARATOR is found. + * Otherwise, return user_home_dir as chroot_dir and "/" as cd_dir. + */ +static void +get_chroot_and_cd_dirs(user_home_dir, chroot_dir, cd_dir) + char *user_home_dir; + char **chroot_dir; + char **cd_dir; +{ + char *p; + + /* Make a pointer to first character of string FTP_CHROOT_SEPARATOR + inside user_home_dir. */ + p = (char *) strstr(user_home_dir, FTP_CHROOT_SEPARATOR); + if (p == NULL) { + /* + * There is not FTP_CHROOT_SEPARATOR string inside + * user_home_dir. Return user_home_dir as chroot_dir, + * and "/" as cd_dir. + */ + if ((*chroot_dir = (char *) strdup(user_home_dir)) == NULL) + fatal("Ran out of memory."); + if ((*cd_dir = (char *) strdup("/")) == NULL) + fatal("Ran out of memory."); + } else { + /* + * Use strlen(user_home_dir) as maximun length for + * both cd_dir and chroot_dir, as both are substrings of + * user_home_dir. + */ + if ((*chroot_dir = malloc(strlen(user_home_dir))) == NULL) + fatal("Ran out of memory."); + if ((*cd_dir = malloc(strlen(user_home_dir))) == NULL) + fatal("Ran out of memory."); + (void) strncpy(*chroot_dir, user_home_dir, p-user_home_dir); + /* Skip FTP_CHROOT_SEPARATOR (except the last /). */ + p += strlen(FTP_CHROOT_SEPARATOR)-1; + (void) strncpy(*cd_dir, p, strlen(p)); } } Oh, forgot to mention -- this is against -CURRENT not -STABLE.
Daniel
Daniel Hagan wrote:
>
> I've rewritten some of your patch to not use dangling pointers. I also
Here's a patch that addresses several different PRs and discussions recently concerning ftpd. Available at http://vtopus.cs.vt.edu/~dhagan/freebsd/ftpd.patch Daniel Index: ftpcmd.y =================================================================== RCS file: /raid/ncvs/src/libexec/ftpd/ftpcmd.y,v retrieving revision 1.19 diff -u -r1.19 ftpcmd.y --- ftpcmd.y 2000/12/16 19:19:19 1.19 +++ ftpcmd.y 2001/01/04 20:16:35 @@ -91,7 +91,10 @@ extern int transflag; extern char tmpline[]; extern int readonly; +extern int readonly_user; extern int noepsv; +extern int dochroot; +extern char *cd_dir, *chroot_dir; off_t restart_point; @@ -505,8 +508,11 @@ | CWD check_login CRLF { if ($2) { - if (guest) - cwd("/"); + if (guest || dochroot) + if (cd_dir != NULL) + cwd(cd_dir); + else + cwd("/"); else cwd(pw->pw_dir); } @@ -979,6 +985,10 @@ { if (readonly) { reply(202, "Command ignored. Server is in readonly mode."); + $$ = 0; + } + else if (readonly_user) { + reply(202, "Command ignored. User is in readonly session."); $$ = 0; } else Index: ftpd.8 =================================================================== RCS file: /raid/ncvs/src/libexec/ftpd/ftpd.8,v retrieving revision 1.36 diff -u -r1.36 ftpd.8 --- ftpd.8 2000/12/18 08:33:25 1.36 +++ ftpd.8 2001/01/04 20:23:27 @@ -158,6 +158,10 @@ .It Fl r Put server in read-only mode. All commands which may modify the local filesystem are disabled. +Read-only mode may be set on a per account basis in +.Xr login.conf 5 +with the boolean capability +.Dq ftp-readonly . .It Fl E Disable the EPSV command. This is useful for servers behind older firewalls. @@ -258,8 +262,12 @@ .Pp The ftp server will abort an active file transfer only when the ABOR -command is preceded by a Telnet "Interrupt Process" (IP) -signal and a Telnet "Synch" signal in the command Telnet stream, +command is preceded by a Telnet +.Dq "Interrupt Process" +.Pq IP +signal and a Telnet +.Dq Synch +signal in the command Telnet stream, as described in Internet RFC 959. If a STAT @@ -299,7 +307,8 @@ .It The login name must not be a member of a group specified in the file .Pa /etc/ftpusers . -Entries in this file interpreted as group names are prefixed by an "at" +Entries in this file interpreted as group names are prefixed by an +.Dq at .Ql \&@ sign. .It @@ -311,14 +320,19 @@ or the user is a member of a group with a group entry in this file, i.e. one prefixed with .Ql \&@ , -the session's root will be changed to the user's login directory by +the session's root will be changed to the user's login directory +.Pq "up to the first /./" +by .Xr chroot 2 as for an .Dq anonymous or .Dq ftp account (see next item). -This facility may also be triggered by enabling the boolean "ftp-chroot" +The user is placed into the directory that remains after stripping the +former from the user's login directory. +This facility may also be triggered by enabling the boolean +.Dq ftp-chroot capability in .Xr login.conf 5 . However, the user must still supply a password. Index: ftpd.c =================================================================== RCS file: /raid/ncvs/src/libexec/ftpd/ftpd.c,v retrieving revision 1.72 diff -u -r1.72 ftpd.c --- ftpd.c 2000/12/20 03:34:54 1.72 +++ ftpd.c 2001/01/04 20:11:48 @@ -140,6 +140,7 @@ int anon_only = 0; /* Only anonymous ftp allowed */ int guest; int dochroot; +char *cd_dir = NULL, *chroot_dir = NULL; int stats; int statfd = -1; int type; @@ -149,6 +150,7 @@ int usedefault = 1; /* for data transfers */ int pdata = -1; /* for passive mode */ int readonly=0; /* Server is in readonly mode. */ +int readonly_user = 0; /* User's session is readonly. */ int noepsv=0; /* EPSV command is disabled. */ sig_atomic_t transflag; off_t file_size; @@ -188,6 +190,9 @@ char *pid_file = NULL; +/* WARNING: FTP_CHROOT_SEPARATOR *MUST* end in / */ +#define FTP_CHROOT_SEPARATOR "/./" + /* * Timeout intervals for retrying connections * to hosts that don't accept PORT cmds. This @@ -251,6 +256,7 @@ static char *sgetsave __P((char *)); static void reapchild __P((int)); static void logxfer __P((char *, long, long)); +static void get_chroot_and_cd_dirs __P((char *, char **, char **)); static char * curdir() @@ -1038,6 +1044,10 @@ logged_in = 0; guest = 0; dochroot = 0; + readonly_user = 0; + free(chroot_dir); + free(cd_dir); + chroot_dir = cd_dir = NULL; } #if !defined(NOPAM) @@ -1291,19 +1301,23 @@ login_getcapbool(lc, "ftp-chroot", 0) || #endif checkuser(_PATH_FTPCHROOT, pw->pw_name, 1); - if (guest) { +#ifdef LOGIN_CAP /* Check for ftp-readonly */ + readonly_user = login_getcapbool(lc, "ftp-readonly", 0); +#endif + if (guest || dochroot) { /* * We MUST do a chdir() after the chroot. Otherwise * the old current directory will be accessible as "." * outside the new root! */ - if (chroot(pw->pw_dir) < 0 || chdir("/") < 0) { - reply(550, "Can't set guest privileges."); - goto bad; - } - } else if (dochroot) { - if (chroot(pw->pw_dir) < 0 || chdir("/") < 0) { - reply(550, "Can't change root."); + get_chroot_and_cd_dirs(pw->pw_dir, &chroot_dir, &cd_dir); + /* + * Do not free chroot_dir & cd_dir b/c they are used in + * processing CWD commands from client. They should be + * free'd during a user logout. + */ + if (chroot(chroot_dir) < 0 || chdir(cd_dir) < 0) { + reply(550, guest ? "Can't set guest privileges." : "Can't change root."); goto bad; } } else if (chdir(pw->pw_dir) < 0) { @@ -2802,5 +2816,50 @@ ctime(&now)+4, ident, remotehost, path, name, size, now - start + (now == start)); write(statfd, buf, strlen(buf)); + } +} + +/* + * Make a pointer to the chroot dir and another to the cd dir. + * The first is all the path up to the first FTP_CHROOT_SEPARATOR. + * The later is the remaining chars, not including the FTP_CHROOT_SEPARATOR, + * but prepending a '/', if FTP_CHROOT_SEPARATOR is found. + * Otherwise, return user_home_dir as chroot_dir and "/" as cd_dir. + */ +static void +get_chroot_and_cd_dirs(user_home_dir, chroot_dir, cd_dir) + char *user_home_dir; + char **chroot_dir; + char **cd_dir; +{ + char *p; + + /* Make a pointer to first character of string FTP_CHROOT_SEPARATOR + inside user_home_dir. */ + p = (char *) strstr(user_home_dir, FTP_CHROOT_SEPARATOR); + if (p == NULL) { + /* + * There is not FTP_CHROOT_SEPARATOR string inside + * user_home_dir. Return user_home_dir as chroot_dir, + * and "/" as cd_dir. + */ + if ((*chroot_dir = (char *) strdup(user_home_dir)) == NULL) + fatal("Ran out of memory."); + if ((*cd_dir = (char *) strdup("/")) == NULL) + fatal("Ran out of memory."); + } else { + /* + * Use strlen(user_home_dir) as maximun length for + * both cd_dir and chroot_dir, as both are substrings of + * user_home_dir. + */ + if ((*chroot_dir = malloc(strlen(user_home_dir))) == NULL) + fatal("Ran out of memory."); + if ((*cd_dir = malloc(strlen(user_home_dir))) == NULL) + fatal("Ran out of memory."); + (void) strncpy(*chroot_dir, user_home_dir, p-user_home_dir); + /* Skip FTP_CHROOT_SEPARATOR (except the last /). */ + p += strlen(FTP_CHROOT_SEPARATOR)-1; + (void) strncpy(*cd_dir, p, strlen(p)); } } Hi, Here it is an updated version of the patch against 4.5-RELEASE. I'd like to see it come into the main tree. Regards. Fernando P. Schapachnik Gerente de tecnología de red y sistemas de información VIA NET.WORKS ARGENTINA S.A. fschapachnik@vianetworks.com.ar Tel.: (54-11) 4323-3381 Responsible Changed From-To: freebsd-bugs->yar Another ftpd PR with alot of patches. Hi Fernando, I'm installing a patch to FreeBSD-current that should add the feature you proposed 2 years ago to ftpd(8) (it's a shame it took us that long to deal with your proposal.) Could you test my solution in -current, or shall I provide a patch against 4.7-stable for your convenience? -- Yar State Changed From-To: open->patched Implemented in -current. Hi Yar, I'm relocated now. Unfortunatedly all I have are RELENG_4_7 machines. I'd gladly test it. I can also provide my current patches, in case you need them. Kind regards! Lic. Fernando Schapachnik Proyecto de Informática Ministerio de Economía ----- Forwarded message from Hernan Nunez <hnunez@vianetworks.com.ar> ----- X-Sieve: cmu-sieve 1.3 From: Hernan Nunez <hnunez@vianetworks.com.ar> Reply-To: <hnunez@vianetworks.com.ar> To: "Yar Tikhiy" <yar@FreeBSD.org> Cc: "Fernando Schapachnik" <fernando@mecon.gov.ar> Subject: Re: bin/23944: Patch for ftpd to add a cd after the chroot. Date: Wed, 29 Jan 2003 10:20:35 -0300 X-Priority: 3 X-MSMail-Priority: Normal X-Mailer: Microsoft Outlook Express 6.00.2800.1106 X-MimeOLE: Produced By Microsoft MimeOLE V6.00.2800.1106 Yar, You could contact Fernando at fernando@mecon.gov.ar. ----- Original Message ----- From: "Yar Tikhiy" <yar@FreeBSD.org> To: <freebsd-gnats-submit@FreeBSD.org>; <fpscha@ns1.via-net-works.net.ar> Sent: Wednesday, January 29, 2003 6:02 AM Subject: Re: bin/23944: Patch for ftpd to add a cd after the chroot. > Hi Fernando, > > I'm installing a patch to FreeBSD-current that should add the feature > you proposed 2 years ago to ftpd(8) (it's a shame it took us that > long to deal with your proposal.) Could you test my solution in > -current, or shall I provide a patch against 4.7-stable for your > convenience? > > -- > Yar > ----- End forwarded message ----- Fernando,
On Wed, Jan 29, 2003 at 10:28:01AM -0300, Fernando Schapachnik wrote:
>
> I'm relocated now. Unfortunatedly all I have are RELENG_4_7 machines.
>
> I'd gladly test it. I can also provide my current patches, in case you
> need them.
Thank you. I'll make a patch against RELENG_4_7 and send it to you ASAP.
As for your current patches, may I have a look at them?
--
Yar
En un mensaje anterior, Yar Tikhiy escribió:
> Thank you. I'll make a patch against RELENG_4_7 and send it to you ASAP.
> As for your current patches, may I have a look at them?
Here you have it.
They are against 4.6.1, but I think there aplied cleanly against 4.7.
Kind regards.
Fernando.
State Changed From-To: patched->closed The proposed feature has been merged to STABLE as well. Thanks! |
