From owner-freebsd-current@FreeBSD.ORG Wed Jan 8 22:37:04 2014 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id A5ABA42E; Wed, 8 Jan 2014 22:37:04 +0000 (UTC) Received: from mx1.stack.nl (relay04.stack.nl [IPv6:2001:610:1108:5010::107]) (using TLSv1 with cipher ADH-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 520F618C1; Wed, 8 Jan 2014 22:37:04 +0000 (UTC) Received: from snail.stack.nl (snail.stack.nl [IPv6:2001:610:1108:5010::131]) by mx1.stack.nl (Postfix) with ESMTP id 7E828B8065; Wed, 8 Jan 2014 23:37:01 +0100 (CET) Received: by snail.stack.nl (Postfix, from userid 1677) id 6A98528497; Wed, 8 Jan 2014 23:37:01 +0100 (CET) Date: Wed, 8 Jan 2014 23:37:01 +0100 From: Jilles Tjoelker To: Nathan Whitehorn Subject: Re: [CFT] bsdinstall and zfsboot enhancements Message-ID: <20140108223701.GA87058@stack.nl> References: <52769CFE.5080707@freebsd.org> <5281340E.8080009@callfortesting.org> <52813E53.20403@freebsd.org> <5281441E.7060806@freebsd.org> <529A6862.7060308@freebsd.org> <20131201123442.GA6818@stack.nl> <52C9C8C3.7050108@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <52C9C8C3.7050108@freebsd.org> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: "Teske, Devin" , Current Current , "freebsd-arch@freebsd.org" , Devin Teske , Peter Grehan , Michael Dexter X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Jan 2014 22:37:04 -0000 On Sun, Jan 05, 2014 at 04:04:03PM -0500, Nathan Whitehorn wrote: > On 12/01/13 07:34, Jilles Tjoelker wrote: > > On Sat, Nov 30, 2013 at 04:36:18PM -0600, Nathan Whitehorn wrote: > >> This took much longer than I'd anticipated, but the patch to init is > >> attached. I chose not to make the changes to init rather than > >> getttyent() and friends in libc, which I am open to revisiting. > > lib/libpam/modules/pam_securetty/pam_securetty.c calls getttynam(3) and > > will not allow root login on a "fake" TTY that getttynam() does not > > know. This module is enabled by default for the "login" service. > > So it is probably better to patch libc rather than init. > OK, here's a revised patch. This one is shorter and works by introducing > an "auto" flag (ideas for names appreciated) that means "on" if the line > is an active console and "off" otherwise. Note that the behavior is now: > - ttys marked "off" stay off > - ttys marked "on" stay on > - ttys marked "auto" are enabled iff they are console devices > - ttys not present in /etc/ttys stay off > This behavior change is much easier to implement when doing it in libc > for various structural reasons and allows the terminal type, etc. to be > specified in the usual way. Instead of "auto" you could use "ifconsole". This looks sensible to me. > > As a preparatory patch, you could remove se_index and session_index from > > init. They are only used to warn about a changed slot number in utmp(5) > > which is irrelevant with utmpx. This noise warning would also appear > > in most cases when changing from a "fake" console entry to a real line > > in /etc/ttys. Also, if you do decide to fake ttys entries in init rather > > than libc, the patch to init will be simpler. > With the new patch, this is indeed the case: no changes to init are > necessary at all. This does not change any behavior unless explicitly > requested in /etc/ttys, so unless there are any objections in the next > couple days, I will commit it. I have some small remarks inline below. I would like to remove se_index and session_index from init soon if it does not conflict with other work. > Index: include/ttyent.h > =================================================================== > --- include/ttyent.h (revision 260331) > +++ include/ttyent.h (working copy) > @@ -37,6 +37,7 @@ > > #define _TTYS_OFF "off" > #define _TTYS_ON "on" > +#define _TTYS_AUTO "auto" > #define _TTYS_SECURE "secure" > #define _TTYS_INSECURE "insecure" > #define _TTYS_WINDOW "window" > Index: lib/libc/gen/getttyent.c > =================================================================== > --- lib/libc/gen/getttyent.c (revision 260331) > +++ lib/libc/gen/getttyent.c (working copy) > @@ -39,6 +39,9 @@ > #include > #include > > +#include > +#include > + > static char zapchar; > static FILE *tf; > static size_t lbsize; > @@ -64,6 +67,32 @@ > return (t); > } > > +static int > +auto_tty_status(const char *ty_name) > +{ > + size_t len; > + char *buf, *cons, *nextcons; > + > + /* Check if this is an enabled kernel console line */ > + buf = NULL; > + if (sysctlbyname("kern.console", NULL, &len, NULL, 0) == -1) > + return (0); /* Errors mean don't enable */ > + buf = malloc(len); > + if (sysctlbyname("kern.console", buf, &len, NULL, 0) == -1) > + return (0); conscontrol also does this, but is it possible that the length increases between the checks? > + > + if ((cons = strchr(buf, '/')) == NULL) > + return (0); > + *cons = '\0'; > + nextcons = buf; > + while ((cons = strsep(&nextcons, ",")) != NULL && strlen(cons) != 0) { > + if (strcmp(cons, ty_name) == 0) > + return (TTY_ON); > + } > + > + return (0); > +} > + > struct ttyent * > getttyent(void) > { > @@ -126,6 +155,8 @@ > tty.ty_status &= ~TTY_ON; > else if (scmp(_TTYS_ON)) > tty.ty_status |= TTY_ON; > + else if (scmp(_TTYS_AUTO)) > + tty.ty_status |= auto_tty_status(tty.ty_name); > else if (scmp(_TTYS_SECURE)) > tty.ty_status |= TTY_SECURE; > else if (scmp(_TTYS_INSECURE)) > Index: libexec/getty/ttys.5 > =================================================================== > --- libexec/getty/ttys.5 (revision 260331) > +++ libexec/getty/ttys.5 (working copy) > @@ -102,8 +102,11 @@ > .Pp > As flag values, the strings ``on'' and ``off'' specify that > .Xr init 8 > -should (should not) execute the command given in the second field, > -while ``secure'' (if ``on'' is also specified) allows users with a > +should (should not) execute the command given in the second field. > +``auto'' will cause this line to be enabled if and only if it is > +an active kernel console device (it is equivalent to ``on'' in this > +case). > +The flag ``secure'' (if ``on'' is also specified) allows users with a Please change "if ``on'' is also specified" to something like "if the line is enabled". A line may also be enabled if ``auto'' is specified. > uid of 0 to login on > this line. > The flag ``dialin'' indicates that a tty entry describes a dialin This (mostly obsolete) flag seems to have no effect now, because almost 12 years ago the condition for the DIALUP log message was accidentally changed to require a remote hostname (-h) as well as a dialup tty (it was used to require a dialup tty and no remote hostname). -- Jilles Tjoelker