From nobody@FreeBSD.org Mon Apr 24 06:30:54 2006 Return-Path: Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 2322416A402 for ; Mon, 24 Apr 2006 06:30:54 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (www.freebsd.org [216.136.204.117]) by mx1.FreeBSD.org (Postfix) with ESMTP id C290743D4C for ; Mon, 24 Apr 2006 06:30:53 +0000 (GMT) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (localhost [127.0.0.1]) by www.freebsd.org (8.13.1/8.13.1) with ESMTP id k3O6Urj3040437 for ; Mon, 24 Apr 2006 06:30:53 GMT (envelope-from nobody@www.freebsd.org) Received: (from nobody@localhost) by www.freebsd.org (8.13.1/8.13.1/Submit) id k3O6Ur01040436; Mon, 24 Apr 2006 06:30:53 GMT (envelope-from nobody) Message-Id: <200604240630.k3O6Ur01040436@www.freebsd.org> Date: Mon, 24 Apr 2006 06:30:53 GMT From: Alex Kozlov To: freebsd-gnats-submit@FreeBSD.org Subject: vipw fail on RO /etc X-Send-Pr-Version: www-2.3 >Number: 96248 >Category: bin >Synopsis: vipw fail on RO /etc >Confidential: no >Severity: non-critical >Priority: low >Responsible: freebsd-bugs >State: closed >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Mon Apr 24 06:40:10 GMT 2006 >Closed-Date: Mon Apr 30 20:30:25 GMT 2007 >Last-Modified: Mon Apr 30 20:30:25 GMT 2007 >Originator: Alex Kozlov >Release: FreeBSD 6.1-RC >Organization: private >Environment: FreeBSD localhost 6.1-RC FreeBSD i386 >Description: if rootfs mount as read-only, vipw fall to execute witch vipw: pw_tmp(): Read-only file system error. >How-To-Repeat: #mount |grep -w / /dev/da0s1 on / (ufs, local, read-only) #vipw vipw: pw_tmp(): Read-only file system >Fix: Change temporary file patch in pw_tmp() from if (snprintf(tempname, sizeof(tempname), "%.*spw.XXXXXX", (int)(p - masterpasswd), masterpasswd) >= (int)sizeof(tempname)) { to more appropriate? >Release-Note: >Audit-Trail: From: Maxim Konovalov To: Alex Kozlov Cc: bug-followup@freebsd.org Subject: Re: bin/96248: vipw fail on RO /etc Date: Mon, 24 Apr 2006 11:17:08 +0400 (MSD) [...] > if rootfs mount as read-only, vipw fall to execute witch vipw: > pw_tmp(): Read-only file system error. > >How-To-Repeat: > #mount |grep -w / > /dev/da0s1 on / (ufs, local, read-only) > > #vipw > vipw: pw_tmp(): Read-only file system > >Fix: > Change temporary file patch in pw_tmp() from > > if (snprintf(tempname, sizeof(tempname), "%.*spw.XXXXXX", > (int)(p - masterpasswd), masterpasswd) >= (int)sizeof(tempname)) { > > to more appropriate? And what is more appropriate? -- Maxim Konovalov From: Alex Kozlov To: Maxim Konovalov Cc: bug-followup@freebsd.org Subject: Re: bin/96248: vipw fail on RO /etc Date: Mon, 24 Apr 2006 11:07:54 +0300 On Mon, Apr 24, 2006 at 11:17:08AM +0400, Maxim Konovalov wrote: > [...] > > if rootfs mount as read-only, vipw fall to execute witch vipw: > > pw_tmp(): Read-only file system error. > > >How-To-Repeat: > > #mount |grep -w / > > /dev/da0s1 on / (ufs, local, read-only) > > > > #vipw > > vipw: pw_tmp(): Read-only file system > > >Fix: > > Change temporary file patch in pw_tmp() from > > > > if (snprintf(tempname, sizeof(tempname), "%.*spw.XXXXXX", > > (int)(p - masterpasswd), masterpasswd) >= (int)sizeof(tempname)) { > > > > to more appropriate? > > And what is more appropriate? Quite good solution may be to add fallback mechanism in case if masterpasswd directory not writable. There are any (security?) reasons, which to prevent the storing of pw_tmp file in /tmp ? -- Adios From: Maxim Konovalov To: Alex Kozlov Cc: bug-followup@freebsd.org Subject: Re: bin/96248: vipw fail on RO /etc Date: Mon, 24 Apr 2006 12:24:29 +0400 (MSD) On Mon, 24 Apr 2006, 11:07+0300, Alex Kozlov wrote: > On Mon, Apr 24, 2006 at 11:17:08AM +0400, Maxim Konovalov wrote: > > [...] > > > if rootfs mount as read-only, vipw fall to execute witch vipw: > > > pw_tmp(): Read-only file system error. > > > >How-To-Repeat: > > > #mount |grep -w / > > > /dev/da0s1 on / (ufs, local, read-only) > > > > > > #vipw > > > vipw: pw_tmp(): Read-only file system > > > >Fix: > > > Change temporary file patch in pw_tmp() from > > > > > > if (snprintf(tempname, sizeof(tempname), "%.*spw.XXXXXX", > > > (int)(p - masterpasswd), masterpasswd) >= (int)sizeof(tempname)) { > > > > > > to more appropriate? > > > > And what is more appropriate? > Quite good solution may be to add fallback mechanism in case if masterpasswd > directory not writable. > > There are any (security?) reasons, which to prevent the storing of > pw_tmp file in /tmp ? Perhaps they are, I don't know. I don't think changing passwd temp files location is a good idea. What is the problem you are trying to solve? -- Maxim Konovalov From: Alex Kozlov To: Maxim Konovalov Cc: bug-followup@freebsd.org Subject: Re: bin/96248: vipw fail on RO /etc Date: Mon, 24 Apr 2006 14:39:53 +0300 On Mon, Apr 24, 2006 at 12:24:29PM +0400, Maxim Konovalov wrote: > On Mon, 24 Apr 2006, 11:07+0300, Alex Kozlov wrote: > > > On Mon, Apr 24, 2006 at 11:17:08AM +0400, Maxim Konovalov wrote: > > > [...] > > > > if rootfs mount as read-only, vipw fall to execute witch vipw: > > > > pw_tmp(): Read-only file system error. > > > > >How-To-Repeat: > > > > #mount |grep -w / > > > > /dev/da0s1 on / (ufs, local, read-only) > > > > > > > > #vipw > > > > vipw: pw_tmp(): Read-only file system > > > > >Fix: > > > > Change temporary file patch in pw_tmp() from > > > > > > > > if (snprintf(tempname, sizeof(tempname), "%.*spw.XXXXXX", > > > > (int)(p - masterpasswd), masterpasswd) >= (int)sizeof(tempname)) { > > > > > > > > to more appropriate? > > > > > > And what is more appropriate? > > Quite good solution may be to add fallback mechanism in case if masterpasswd > > directory not writable. > > > > There are any (security?) reasons, which to prevent the storing of > > pw_tmp file in /tmp ? > >Perhaps they are, I don't know. I don't think changing passwd temp >files location is a good idea. In case of /tmp? Perhaps. Just to be on safe side, choose directory writable only for root. Say, /var/run. Sudo already use /var/run/sudo. Any security advantages /etc in comparison with /var/run ? Both have equal permissions. If crash happens, /var/run/pw.XXXXXX will be cleaned on next startup, /etc/pw.XXXXX and especially /path/to/unknown/pw.XXXXXX - never. Rice working on /var/run but not on /etc ? Hmm. >What is the problem you are trying to solve? You probably suggest do something like: sudo less /etc/master.passwd sudo mount -uw / sudo vipw ? One more line for sudoers. One more time type password. Perhaps. -- Adios From: Alex Kozlov To: Maxim Konovalov Cc: bug-followup@freebsd.org Subject: Re: bin/96248: vipw fail on RO /etc Date: Mon, 24 Apr 2006 21:46:18 +0300 Hi, Maxim No need to touch pw_util. Have pw_tmp in one certain place will be nice, but different fs problem... Hmm. Much easier to teach vipw call PAGER if pw_edit fall on RO fs. This patch can be accepted? If no, you may close PR. -- Adios From: Maxim Konovalov To: Alex Kozlov Cc: bug-followup@freebsd.org Subject: Re: bin/96248: vipw fail on RO /etc Date: Mon, 24 Apr 2006 23:16:01 +0400 (MSD) On Mon, 24 Apr 2006, 21:46+0300, Alex Kozlov wrote: > Hi, Maxim > > No need to touch pw_util. > > Have pw_tmp in one certain place will be nice, but different fs problem... > Hmm. > > Much easier to teach vipw call PAGER if pw_edit fall on RO fs. > > This patch can be accepted? If no, you may close PR. I haven't seen any patches yet but personally don't see much sense in such subtle mechanism. I believe a simple script could solve the problem in your environment. -- Maxim Konovalov From: Alex Kozlov To: Maxim Konovalov Cc: bug-followup@freebsd.org Subject: Re: bin/96248: vipw fail on RO /etc Date: Tue, 25 Apr 2006 01:44:02 +0300 On Mon, Apr 24, 2006 at 11:16:01PM +0400, Maxim Konovalov wrote: > On Mon, 24 Apr 2006, 21:46+0300, Alex Kozlov wrote: > > > Hi, Maxim > > > > No need to touch pw_util. > > > > Have pw_tmp in one certain place will be nice, but different fs problem... > > Hmm. > > > > Much easier to teach vipw call PAGER if pw_edit fall on RO fs. > > > > This patch can be accepted? If no, you may close PR. > > I haven't seen any patches yet but personally don't see much sense in > such subtle mechanism. It trivial. Something like this: --- vipw.c Tue Apr 19 19:18:07 2006 +++ vipw.c.new Tue Apr 25 00:16:16 2006 @@ -50,9 +50,13 @@ #include #include +#include +#include #include +#include #include +#include #include #include #include @@ -66,9 +70,14 @@ main(int argc, char *argv[]) { const char *passwd_dir = NULL; + const char *pager; int ch, pfd, tfd; + int pstat; char *line; size_t len; + struct sigaction sa, sa_int, sa_quit; + sigset_t oldsigset, sigset; + static pid_t pagerpid = -1; while ((ch = getopt(argc, argv, "d:")) != -1) switch (ch) { @@ -93,8 +102,67 @@ err(1, "pw_lock()"); } if ((tfd = pw_tmp(pfd)) == -1) { - pw_fini(); - err(1, "pw_tmp()"); + if (errno == EROFS) { + /* pw_view */ + + (void)close(tfd); + + if ((pager = getenv("PAGER")) == NULL) + //pager = _PATH_MORE + pager = "/usr/bin/more"; + + sa.sa_handler = SIG_IGN; + sigemptyset(&sa.sa_mask); + sa.sa_flags = 0; + sigaction(SIGINT, &sa, &sa_int); + sigaction(SIGQUIT, &sa, &sa_quit); + sigemptyset(&sigset); + sigaddset(&sigset, SIGCHLD); + sigprocmask(SIG_BLOCK, &sigset, &oldsigset); + switch ((pagerpid = fork())) { + case -1: + err(1, "pw_view()"); + case 0: + sigaction(SIGINT, &sa_int, NULL); + sigaction(SIGQUIT, &sa_quit, NULL); + sigprocmask(SIG_SETMASK, &oldsigset, NULL); + + errno = 0; + dup2(pfd, STDIN_FILENO); + execlp(pager, basename(pager), NULL, (char *)NULL); + _exit(errno); + default: + (void)close(pfd); + pw_fini(); + /* parent */ + break; + } + + for (;;) { + if (waitpid(pagerpid, &pstat, WUNTRACED) == -1) { + if (errno == EINTR) + continue; + pagerpid = -1; + break; + } else if (WIFSTOPPED(pstat)) { + raise(WSTOPSIG(pstat)); + } else if (WIFEXITED(pstat) && WEXITSTATUS(pstat) == 0) { + pagerpid = -1; + break; + } else { + pagerpid = -1; + break; + } + } + sigaction(SIGINT, &sa_int, NULL); + sigaction(SIGQUIT, &sa_quit, NULL); + sigprocmask(SIG_SETMASK, &oldsigset, NULL); + + exit(0); + } else { + pw_fini(); + err(1, "pw_tmp()"); + } } (void)close(tfd); /* Force umask for partial writes made in the edit phase */ > I believe a simple script could solve the problem in your environment. Almost any missing features can be done with wrapper script. But programs continue to improve. Also I don't want script in sudo case. P.S. Btw, lib/libutil don't respect NO_INET6 knob. Fix is simple. Shall I fill another PR? -- Adios From: Maxim Konovalov To: Alex Kozlov Cc: bug-followup@freebsd.org Subject: Re: bin/96248: vipw fail on RO /etc Date: Wed, 26 Apr 2006 00:39:38 +0400 (MSD) [...] > > I believe a simple script could solve the problem in your environment. > Almost any missing features can be done with wrapper script. > But programs continue to improve. Yes, in a way they don't break POLA. I'd be really surprised if eventually vipw(8) run 'more' instead of 'vi' :-) > Also I don't want script in sudo case. I know nothing about your environment but if you trust users to run vipw(8) from which they can easily change root password, create a priv. account, jump to root shell, why don't trust them to run a script? > P.S. Btw, lib/libutil don't respect NO_INET6 knob. Fix is simple. > Shall I fill another PR? Yes, sure. -- Maxim Konovalov From: Alex Kozlov To: Maxim Konovalov Cc: bug-followup@freebsd.org Subject: Re: bin/96248: vipw fail on RO /etc Date: Wed, 26 Apr 2006 00:57:56 +0300 On Tue, Apr 25, 2006 at 08:40:14PM +0000, Maxim Konovalov wrote: > > > > I believe a simple script could solve the problem in your environment. > > Almost any missing features can be done with wrapper script. > > But programs continue to improve. > > Yes, in a way they don't break POLA. I'd be really surprised if > eventually vipw(8) run 'more' instead of 'vi' :-) > > > Also I don't want script in sudo case. > > I know nothing about your environment but if you trust users to run > vipw(8) from which they can easily change root password, create a > priv. account, jump to root shell, why don't trust them to run a > script? Ok. I take your point. > > P.S. Btw, lib/libutil don't respect NO_INET6 knob. Fix is simple. > > Shall I fill another PR? > Yes, sure. Done. -- Adios State-Changed-From-To: open->closed State-Changed-By: maxim State-Changed-When: Mon Apr 30 20:29:02 UTC 2007 State-Changed-Why: It seems we found a consensus this not a bug but rather a feature that doesn't need a fix. http://www.freebsd.org/cgi/query-pr.cgi?pr=96248 >Unformatted: