svn commit: r359053 - head/sys/kern

[Ian Lepore]

On Wed, 2020-03-18 at 22:36 +0200, Konstantin Belousov wrote:
> On Tue, Mar 17, 2020 at 07:25:01PM -0600, Ian Lepore wrote:
> > On Tue, 2020-03-17 at 17:05 -0700, John Baldwin wrote:
> > > On 3/17/20 3:36 PM, Ian Lepore wrote:
> > > > On Tue, 2020-03-17 at 22:27 +0000, Conrad Meyer wrote:
> > > > > Author: cem
> > > > > Date: Tue Mar 17 22:27:16 2020
> > > > > New Revision: 359053
> > > > > URL: https://svnweb.freebsd.org/changeset/base/359053
> > > > > 
> > > > > Log:
> > > > >   Implement sysctl kern.boot_id
> > > > >   
> > > > >   Boot IDs are random, opaque 128-bit identifiers that
> > > > > distinguish distinct
> > > > >   system boots.  A new ID is generated each time the system
> > > > > boots.  Unlike
> > > > >   kern.boottime, the value is not modified by NTP
> > > > > adjustments.  It remains fixed
> > > > >   until the machine is restarted.
> > > > >   
> > > > >   PR:		244867
> > > > >   Reported by:	Ricardo Fraile <rfraile AT rfraile.eu>
> > > > >   MFC after:	I do not intend to, but feel free
> > > > > 
> > > > > Modified:
> > > > >   head/sys/kern/kern_mib.c
> > > > > 
> > > > > Modified: head/sys/kern/kern_mib.c
> > > > > =============================================================
> > > > > ====
> > > > > =============
> > > > > --- head/sys/kern/kern_mib.c	Tue Mar 17 21:29:03 2020	(
> > > > > r359052)
> > > > > +++ head/sys/kern/kern_mib.c	Tue Mar 17 22:27:16 2020	(
> > > > > r359053)
> > > > > @@ -448,6 +448,32 @@ SYSCTL_PROC(_kern, KERN_HOSTID, hostid,
> > > > >      CTLTYPE_ULONG | CTLFLAG_RW | CTLFLAG_PRISON |
> > > > > CTLFLAG_MPSAFE
> > > > > > CTLFLAG_CAPRD,
> > > > > 
> > > > >      NULL, 0, sysctl_hostid, "LU", "Host ID");
> > > > >  
> > > > > +static struct mtx bootid_lk;
> > > > > +MTX_SYSINIT(bootid_lock, &bootid_lk, "bootid generator
> > > > > lock",
> > > > > MTX_DEF);
> > > > > +
> > > > > +static int
> > > > > +sysctl_bootid(SYSCTL_HANDLER_ARGS)
> > > > > +{
> > > > > +	static uint8_t boot_id[16];
> > > > > +	static bool initialized = false;
> > > > > +
> > > > > +	mtx_lock(&bootid_lk);
> > > > > +	if (!initialized) {
> > > > > +		if (!is_random_seeded()) {
> > > > > +			mtx_unlock(&bootid_lk);
> > > > > +			return (ENXIO);
> > > > > +		}
> > > > > +		arc4random_buf(boot_id, sizeof(boot_id));
> > > > > +		initialized = true;
> > > > > +	}
> > > > > +	mtx_unlock(&bootid_lk);
> > > > > +
> > > > > +	return (SYSCTL_OUT(req, boot_id, sizeof(boot_id)));
> > > > > +}
> > > > > +SYSCTL_PROC(_kern, OID_AUTO, boot_id,
> > > > > +    CTLTYPE_STRUCT | CTLFLAG_RD | CTLFLAG_MPSAFE |
> > > > > CTLFLAG_CAPRD,
> > > > > +    NULL, 0, sysctl_bootid, "", "Random boot ID");
> > > > > +
> > > > >  /*
> > > > >   * The osrelease string is copied from the global (osrelease
> > > > > in
> > > > > vers.c) into
> > > > >   * prison0 by a sysinit and is inherited by child jails if
> > > > > notG
> > > > > changed at jail
> > > > 
> > > > This seems a bit complex.  Why run a sysinit to init a mutex so
> > > > that
> > > > you can safely do a lazy init of boot_id?  Seems like it would
> > > > be
> > > > much
> > > > easier to just use a sysinit at SI_SUB_LAST to init boot_id
> > > > before
> > > > sysctl can reference it.
> > > 
> > > Presumably you may not have enough entropy by SI_SUB_LAST to
> > > generate
> > > it?
> > > 
> > 
> > I thought arc4random in the kernel could provide random numbers
> > immediately (and definitely after jitter in device attachment times
> > at
> > the end of kernel init)?  This doesn't seem like the kind of thing
> > that
> > needs crypto-strength randomness.
> 
> I think that a large simplification can come from the random driver
> initializing the boot_id variable immediately before setting things
> so that is_random_seeded() start returning true.
> 
> But even this might be too complex,  Why not copy the value from the
> boottime on boot, and not ever touch it after.

On some systems (virtually all mips, arm, and some arm64 systems),
there is no RTC and boottime doesn't get set initially until ntpd or
something else in userland runs to set time.

-- Ian