Date: Wed, 8 Jan 2014 23:37:01 +0100 From: Jilles Tjoelker <jilles@stack.nl> To: Nathan Whitehorn <nwhitehorn@freebsd.org> Cc: "Teske, Devin" <Devin.Teske@fisglobal.com>, Current Current <freebsd-current@freebsd.org>, "freebsd-arch@freebsd.org" <freebsd-arch@freebsd.org>, Devin Teske <dteske@freebsd.org>, Peter Grehan <grehan@freebsd.org>, Michael Dexter <editor@callfortesting.org> Subject: Re: [CFT] bsdinstall and zfsboot enhancements Message-ID: <20140108223701.GA87058@stack.nl> In-Reply-To: <52C9C8C3.7050108@freebsd.org> References: <ABD90FE2-1540-410A-959E-D91D0BE811E3@freebsd.org> <52769CFE.5080707@freebsd.org> <5281340E.8080009@callfortesting.org> <F3512B82-7B2E-40D9-A513-C4C2430F9255@fisglobal.com> <52813E53.20403@freebsd.org> <5281441E.7060806@freebsd.org> <D81082F2-8273-449F-A2EB-DAA12779CAE7@fisglobal.com> <529A6862.7060308@freebsd.org> <20131201123442.GA6818@stack.nl> <52C9C8C3.7050108@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <ctype.h>
> #include <string.h>
>
> +#include <sys/types.h>
> +#include <sys/sysctl.h>
> +
> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140108223701.GA87058>