Re: malloc-related SIGSEGV when cross-building i386 on NetBSD-10.0/amd64

[Taylor R Campbell]

> Date: Tue, 16 Apr 2024 00:59:31 +
> From: Emmanuel Dreyfus 
> 
> Program terminated with signal SIGSEGV, Segmentation fault.
> #0  0xf4bde597 in tsd_fetch_impl (minimal=false, init=true)
> at 
> /usr/src/external/bsd/jemalloc/lib/../include/jemalloc/internal/tsd.h:270
> 270 return tsd;
> 
> (gdb) bt
> #0  0xf4bde597 in tsd_fetch_impl (minimal=false, init=true)
> at 
> /usr/src/external/bsd/jemalloc/lib/../include/jemalloc/internal/tsd.h:270
> [...]
> #14 tsd_fetch ()
> at 
> /usr/src/external/bsd/jemalloc/lib/../include/jemalloc/internal/tsd.h:291
> [...]
> (gdb) disas 0xf4bde597
> [...]
>0xf4bde591 <+37>:   mov-0x2d0(%ebx),%ecx
> => 0xf4bde597 <+43>:   mov%gs:0x0,%esi
>0xf4bde59e <+50>:   add%ecx,%esi
> 
> (gdb) info reg
> [...]
> gs 0x0 0

Great, thanks!  Can you file a PR with this information so we can use
it to track fixes and pullups?

It would also be great if you could catch a stack trace in a crash
that isn't in a signal handler itself, just to keep the diagnosis
simpler.

It looks like somehow the thread's tls pointer (%gs) is null, but my
guess is that this happens only sometimes, e.g. during involuntary
kernel context switches that involve saving and restoring a trap
frame, which is why it appears to be happening randomly and not
immediately when any process tries to use malloc.

Re: malloc-related SIGSEGV when cross-building i386 on NetBSD-10.0/amd64

[Taylor R Campbell]

> Date: Mon, 15 Apr 2024 00:21:32 +
> From: Emmanuel Dreyfus 
> 
> I cross-build i386 packages on an amd64 machine, using pkg_comp with
> CFLAHS-m32 in /etc/mk.conf.
> 
> That worked well on NetBSD-9 but after upgradint to NetBSD-10.0, I get
> random SIGSEGV. Here are a few backtrace examples:
> 
> In sed:
> #0  0xf0d63c00 in je_malloc_tsd_boot0 () from /lib/libc.so.12
> #1  0xf0dc17ea in malloc_init_hard () from /lib/libc.so.12
> #2  0xfb8d6633 in _rtld_call_init_function () from /usr/libexec/ld.elf_so
> #3  0xfb8d68f0 in _rtld_call_init_functions () from /usr/libexec/ld.elf_so
> #4  0xfb8d7297 in _rtld () from /usr/libexec/ld.elf_so
> #5  0xfb8d051a in rtld_start () from /usr/libexec/ld.elf_so

Can you get a stack trace with debug.tgz installed so we have source
location for all of this?

Can you share `disas 0xf0d63c00' output?

> In perl:
> #0  0xf47fd5d9 in calloc () from /lib/libc.so.12

same with 0xf47fd5d9

> In rm:
> #0  0xf7bec157 in free () from /lib/libc.so.12

same here

> In sh:
> #)  0xefe46ae7 in _malloc_postfork_child () from /lib/libc.so.12

same here

Re: Thread-local storage issues arose again? Firefox frequently crashes on 10.0 aarch64

[Taylor R Campbell]

> Date: Sun, 14 Apr 2024 14:07:26 +0900
> From: PHO 
> 
> As I mentioned in 
> http://mail-index.netbsd.org/netbsd-users/2024/04/12/msg030915.html, 
> Firefox tab processes crash very frequently on NetBSD/aarch64 10.0. 
> Building it with PKG_OPTIONS.firefox+=debug-info revealed that when it 
> crashes it segfaults at one of these two places non-deterministically:

Thanks for tracking this down -- can you file a PR with this
information, plus `readelf -d libxul.so' output, so we have a place to
track possible fixes and pullups?

Can you also track down exactly what the definitions of `thread_local'
and `THREAD_LOCAL' are in this code (maybe use cc -E to get the
preprocessor output), just so we don't have to guess?

Can you also grant me access to your binary package set, so I can
examine the binaries and libraries in it?

Can you also find out what firefox is dlopening, if anything?

Can you also try building an ld.elf_so with -DDEBUG -DRTLD_DEBUG, and
run firefox with LD_DEBUG=1 in the environment using this ld.elf_so,
and skim through the output or share it with me?

Re: Thread-local storage issues arose again? Firefox frequently crashes on 10.0 aarch64

[Taylor R Campbell]

> Date: Sun, 14 Apr 2024 05:15:31 +
> From: Emmanuel Dreyfus 
> 
> On Sun, Apr 14, 2024 at 02:07:26PM +0900, PHO wrote:
> > As I mentioned in
> > http://mail-index.netbsd.org/netbsd-users/2024/04/12/msg030915.html, Firefox
> > tab processes crash very frequently on NetBSD/aarch64 10.0. Building it with
> > PKG_OPTIONS.firefox+=debug-info revealed that when it crashes it segfaults
> > at one of these two places non-deterministically:
> 
> Not sure it is related, but it could be: I get random SISEGV when 
> bulk-building NetBSD-10.0/i386 packages on NetBSD-10.0/amd64 XEN3_DOMU.

This is almost certainly unrelated to the issue on firefox/aarch64
which pho@ narrowed down to a thread-local storage matter; can you
start a separate thread or PR for it, with diagnostic information?

Re: Using mmap(2) in sort(1) instead of temp files

[Taylor R Campbell]

> Date: Fri, 5 Apr 2024 07:36:42 -0400 (EDT)
> From: Mouse 
> 
> (4) Are there still incoherencies between mmap and read/write access?
> At one time there were, and I never got a good handle on what needed to
> be done to avoid them.

This bug was fixed nearly a quarter century ago, in November 2000,
with the merge of the unified buffer cache.  You can read about it
here:

http://www.usenix.org/event/usenix2000/freenix/full_papers/silvers/silvers.pdf

I think using any version of NetBSD released in this millennium should
be good to avoid the bug.

OpenSSL SHA-2 symbol mess

[Taylor R Campbell]

12/224 and SHA-512/256 buffer overruns).  We might
want to run a bulk build to see how it pans out.


Thoughts?
>From 7c2a554b476a0b17ff015646bdaaecb761acc097 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell 
Date: Fri, 15 Mar 2024 22:23:27 +
Subject: [PATCH] openssl: Just rename the sha2 symbols.

Ditch all our other local changes related to them.

These symbols end up as private symbols in libcrypto, and our
libcrypto has never exported the sha2 symbols anyway so that can't
break existing applications.  So this might even be safe to pull up
to branches.

This changes some libcrypto symbols listed in crypto.map -- but those
symbols weren't defined anyway!  And ld apparently doesn't care if
they're not defined.

PR bin/51333
PR lib/58039
---
 .../bsd/openssl/dist/crypto/evp/legacy_sha.c  |  4 +-
 .../bsd/openssl/dist/include/openssl/sha.h| 28 +++---
 .../implementations/digests/sha2_prov.c   |  6 +-
 .../bsd/openssl/lib/libcrypto/Makefile|  1 +
 .../bsd/openssl/lib/libcrypto/crypto.map  | 28 +++---
 .../bsd/openssl/lib/libcrypto/libc-sha1.c | 45 --
 .../bsd/openssl/lib/libcrypto/libc-sha256.c   | 49 --
 .../bsd/openssl/lib/libcrypto/libc-sha2xx.c   | 90 ---
 .../bsd/openssl/lib/libcrypto/libc-sha512.c   | 49 --
 .../bsd/openssl/lib/libcrypto/sha.inc | 18 ++--
 10 files changed, 39 insertions(+), 279 deletions(-)
 delete mode 100644 crypto/external/bsd/openssl/lib/libcrypto/libc-sha1.c
 delete mode 100644 crypto/external/bsd/openssl/lib/libcrypto/libc-sha256.c
 delete mode 100644 crypto/external/bsd/openssl/lib/libcrypto/libc-sha2xx.c
 delete mode 100644 crypto/external/bsd/openssl/lib/libcrypto/libc-sha512.c

diff --git a/crypto/external/bsd/openssl/dist/crypto/evp/legacy_sha.c 
b/crypto/external/bsd/openssl/dist/crypto/evp/legacy_sha.c
index 1649601cf92b..ca9a3264978a 100644
--- a/crypto/external/bsd/openssl/dist/crypto/evp/legacy_sha.c
+++ b/crypto/external/bsd/openssl/dist/crypto/evp/legacy_sha.c
@@ -49,9 +49,9 @@ static int nm##_init(EVP_MD_CTX *ctx) 
 \
 #define sha512_256_Initsha512_256_init
 
 #define sha512_224_Update  SHA512_Update
-#define sha512_224_Final   sha512_224_final /* XXX NetBSD libc sha2 */
+#define sha512_224_Final   SHA512_Final
 #define sha512_256_Update  SHA512_Update
-#define sha512_256_Final   sha512_256_final /* XXX NetBSD libc sha2 */
+#define sha512_256_Final   SHA512_Final
 
 IMPLEMENT_LEGACY_EVP_MD_METH(sha1, SHA1)
 IMPLEMENT_LEGACY_EVP_MD_METH(sha224, SHA224)
diff --git a/crypto/external/bsd/openssl/dist/include/openssl/sha.h 
b/crypto/external/bsd/openssl/dist/include/openssl/sha.h
index c7084bf9889e..bb620faf91d9 100644
--- a/crypto/external/bsd/openssl/dist/include/openssl/sha.h
+++ b/crypto/external/bsd/openssl/dist/include/openssl/sha.h
@@ -70,16 +70,16 @@ typedef struct SHA256state_st {
 unsigned int num, md_len;
 } SHA256_CTX;
 
-OSSL_DEPRECATEDIN_3_0 int SHA224_Init(SHA256_CTX *c);
+OSSL_DEPRECATEDIN_3_0 int SHA224_Init(SHA256_CTX *c) 
__RENAME(_OpenSSL_SHA224_Init);
 OSSL_DEPRECATEDIN_3_0 int SHA224_Update(SHA256_CTX *c,
-const void *data, size_t len);
-OSSL_DEPRECATEDIN_3_0 int SHA224_Final(unsigned char *md, SHA256_CTX *c);
-OSSL_DEPRECATEDIN_3_0 int SHA256_Init(SHA256_CTX *c);
+const void *data, size_t len) 
__RENAME(_OpenSSL_SHA224_Update);
+OSSL_DEPRECATEDIN_3_0 int SHA224_Final(unsigned char *md, SHA256_CTX *c) 
__RENAME(_OpenSSL_SHA224_Final);
+OSSL_DEPRECATEDIN_3_0 int SHA256_Init(SHA256_CTX *c) 
__RENAME(_OpenSSL_SHA256_Init);
 OSSL_DEPRECATEDIN_3_0 int SHA256_Update(SHA256_CTX *c,
-const void *data, size_t len);
-OSSL_DEPRECATEDIN_3_0 int SHA256_Final(unsigned char *md, SHA256_CTX *c);
+const void *data, size_t len) 
__RENAME(_OpenSSL_SHA256_Update);
+OSSL_DEPRECATEDIN_3_0 int SHA256_Final(unsigned char *md, SHA256_CTX *c) 
__RENAME(_OpenSSL_SHA256_Final);
 OSSL_DEPRECATEDIN_3_0 void SHA256_Transform(SHA256_CTX *c,
-const unsigned char *data);
+const unsigned char *data) 
__RENAME(_OpenSSL_SHA256_Transform);
 # endif
 
 unsigned char *SHA224(const unsigned char *d, size_t n, unsigned char *md);
@@ -120,16 +120,16 @@ typedef struct SHA512state_st {
 unsigned int num, md_len;
 } SHA512_CTX;
 
-OSSL_DEPRECATEDIN_3_0 int SHA384_Init(SHA512_CTX *c);
+OSSL_DEPRECATEDIN_3_0 int SHA384_Init(SHA512_CTX *c) 
__RENAME(_OpenSSL_SHA384_Init);
 OSSL_DEPRECATEDIN_3_0 int SHA384_Update(SHA512_CTX *c,
-const void *data, size_t len);
-OSSL_DEPRECATEDIN_3_0 int SHA384_Final(unsigned char *md, SHA512_CTX *c);
-OSSL_DEPRECATEDIN_3_0 int SHA512_Init(SHA512_CTX *c);
+const void *data, size_t len) 
__RENAME(_OpenSSL_SHA384_Upd

Checking library symbols

[Taylor R Campbell]

[bcc tech-toolchain, followups to tech-userlevel to stay in one place]

Every now and then we have some embarrassing bug with accidental
removal or exposure of symbols in a shared library.  This has been a
constant problem with libm (e.g., https://gnats.NetBSD.org/57960,
https://gnats.NetBSD.org/57881, https://gnats.NetBSD.org/56577) but it
can also affect other libraries too.

The attached patch creates a build-time mechanism to audit libraries
for unintentional symbol changes with bsd.lib.mk:

- To opt in, you write down a sorted list of all the symbols (and, if
  applicable, version suffixes) expected to be defined by a library
  LIB=foo in foo.expsym.

- When the library is built, bsd.lib.mk will use nm(1) to generate a
  sorted list of symbols actually defined in libfoo.so(.M.N), and diff
  it against foo.expsym.

If there are differences, the build will fail and the differences will
be printed.  Only symbol names and versions are checked, not addresses
or types.

This check is orthogonal to writing a version map: it doesn't require
changing the library itself to opt into symbol versions
(https://wiki.NetBSD.org/symbol_versions), and it checks for missing
symbols too which ld doesn't check for even if they're written in the
version map.

So far this is experimental -- I only tried it with openssl libcrypto
to verify another experiment with renaming sha2 symbols.  It's limited
to shared libraries, not static libraries, for now.  In practice we
might require more mechanism to handle MD variation in the set of
defined symbols (especially with libm), and we might encounter other
problems I haven't foreseen if we try to apply this to more libraries,
but I thought I'd share a first draft.

Thoughts?
>From aa30c139d6874d9f7733484474e804622f0b0dce Mon Sep 17 00:00:00 2001
From: Taylor R Campbell 
Date: Sat, 16 Mar 2024 21:53:41 +
Subject: [PATCH] bsd.lib.mk: Check expected vs actual symbols at build-time.

If, for LIB=foo, you create a file foo.expsym, bsd.lib.mk will list
the dynamic symbols and their versions with

nm --dynamic --extern-only --defined-only --with-symbol-versions

and compare the names (not addresses or types) to foo.expsym.  If
there are any differences, they will be printed and the build will
fail.

foo.expsym should be sorted with `LANG=C sort -u'.

This way, you can verify changes don't inadvertently add or remove
symbols.  If you do want to add (or, if you're bumping the major,
remove) symbols, you can verify the changes and edit the foo.expsym
file accordingly.  This will also help to enforce rules about symbol
changes on pullups in release branches.

Note that using a version map (-Wl,--version-script=...) doesn't
catch symbol removal -- ld quietly ignores symbols in the version map
that aren't actually defined by any object in the library.  So this
supplements the version map.
---
 share/mk/bsd.lib.mk | 20 
 1 file changed, 20 insertions(+)

diff --git a/share/mk/bsd.lib.mk b/share/mk/bsd.lib.mk
index 4db7809dda40..9df4bcc834f3 100644
--- a/share/mk/bsd.lib.mk
+++ b/share/mk/bsd.lib.mk
@@ -648,6 +648,26 @@ ${_LIB.so.full}: ${_MAINLIBDEPS}
${HOST_LN} -sf ${_LIB.so.full} ${_LIB.so}.tmp
${MV} ${_LIB.so}.tmp ${_LIB.so}
 
+# If there's a file listing expected symbols, fail if the diff from it
+# to the actual symbols is nonempty, and show the diff in that case.
+.if exists(${.CURDIR}/${LIB}.expsym)
+realall: ${_LIB.so.full}.diffsym
+${_LIB.so.full}.diffsym: ${LIB}.expsym ${_LIB.so.full}.actsym
+   ${_MKTARGET_CREATE}
+   if diff -u ${.ALLSRC} >${.TARGET}.tmp; then \
+   ${MV} ${.TARGET}.tmp ${.TARGET}; \
+   else \
+   ret=$$?; \
+   cat ${.TARGET}.tmp; \
+   exit $$ret; \
+   fi
+${_LIB.so.full}.actsym: ${_LIB.so.full}
+   ${NM} --dynamic --extern-only --defined-only --with-symbol-versions \
+   ${_LIB.so.full} \
+   | cut -d' ' -f3 | sort -u >${.TARGET}.tmp
+   ${MV} ${.TARGET}.tmp ${.TARGET}
+.endif
+
 .if !empty(LOBJS)  # {
 LLIBS?=-lc
 ${_LIB.ln}: ${LOBJS}

Re: Issues with lseek(2) on a block device

[Taylor R Campbell]

> Date: Sat, 24 Feb 2024 16:21:42 -0500
> From: Thor Lancelot Simon 
> 
> On Wed, Feb 21, 2024 at 09:20:55PM +0000, Taylor R Campbell wrote:
> > I think this is a bug, and it would be great if stat(2) just returned
> > the physical medium's size in st_size -- currently doing this reliably
> > takes at least three different ioctls to handle all storage media, if
> > I counted correctly in sbin/fsck/partutil.c's getdiskinfo.
> 
> I am not sure this can be done for all block devices.  Tapes have block
> devices, and open-reel tape drives do not really know the length of the
> loaded media, while on any other tape drive supporting compression, there
> may really be no such size.

Sure, it's fine if it doesn't give an answer (or, returns st_size=0)
for devices where there's no reasonable answer.

But for a medium which does have a definite size that is known up
front, it should just be returned by stat(2) in st_size instead of
requiring a dance of multiple different NetBSD-specific ioctls to
guess at which one will work.

Re: Issues with lseek(2) on a block device

[Taylor R Campbell]

> Date: Wed, 21 Feb 2024 10:52:55 +
> From: Sad Clouds 
> 
> Hello, for most operating systems determining the size of a block
> device can be done with:
> 
> lseek(fd, 0, SEEK_END);
> 
> However, on NetBSD this does not seem to work.

Internally, this is happens for more or less the same reason that
stat(2) doesn't return the size of a block device in st_size.

Each file system on which device nodes can reside implements its own
VOP_GETATTR (used by both lseek(2) and stat(2)), and most of them use
the _inode_ size (more or less) rather than querying the block device
that the device node represents for its physical size.

I think this is a bug, and it would be great if stat(2) just returned
the physical medium's size in st_size -- currently doing this reliably
takes at least three different ioctls to handle all storage media, if
I counted correctly in sbin/fsck/partutil.c's getdiskinfo.

But it's a little annoying to make that happen because it requires
going through all the file systems that have device nodes and
convincing their VOP_GETATTR implementations to do something different
for block devices (and ideally also character devices, if they
represent fixed-size media too).

strtoi(3) ERANGE vs ENOTSUP

[Taylor R Campbell]

PR lib/57828 (https://gnats.NetBSD.org/57828) proposes changing an
edge case of the strtoi(3) function so that if ERANGE and ENOTSUP are
both applicable then it should fail with ERANGE rather than ENOTSUP.

This is the case when the number is out of range, and there's trailing
garbage -- e.g., s="42z", min=3, max=7.


The rationale is that the caller can trivially detect the ENOTSUP
condition (test end[0] != '\0'), but not the ERANGE condition (short
of parsing the number again, which would defeat the purpose).  And
applications may find the out-of-range case more significant than the
trailing garbage case.

For example, libutil's pidfile_read(3) tries to parse a pid on a
single line, so any file whose content matches /[0-9]+\n/ with the
resulting number in the interval [1, INT_MAX] is accepted, and one
might hope that anything else should be rejected:

https://nxr.netbsd.org/xref/src/lib/libutil/pidfile.c?r=1.16#132

But if the pidfile contains, e.g., `999\n',
pidfile_read(3) as implemented will accept this -- and interpret it as
pid 2147483647.  If it's `-999\n',
pidfile_read(3) will accept it as pid 1.


Counterarguments:
- We've had this semantics since NetBSD 7, so changing it is risky --
  we'd have to audit all the existing code that uses it.
- There are copies of the code in various places and they might
  diverge.
- I searched github for /[^a-zA-Z0-9_]strtoi[^a-zA-Z0-9_]/ and came up
  with almost 16k results, so auditing that myself isn't going to
  happen unless we can narrow it down a lot.
- If you need to act on the trailing stuff, maybe it's better to just
  use a real lexer instead of relying on edge cases of error cases.
- If you rely on this change, your code won't work in any of the
  deployed implementations of strtoi anyway.


Thoughts?

The attached patch implements the proposed change in libc and the
automatic tests.
>From a64349a5ac209d9967ba3c5d6a62aa243f44f603 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell 
Date: Sat, 20 Jan 2024 18:17:27 +
Subject: [PATCH 1/2] strtoi(3): Test for ERANGE before ENOTSUP.

Expect failure because that's not what we implemented.

PR lib/57828
---
 tests/lib/libc/stdlib/t_strtoi.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/tests/lib/libc/stdlib/t_strtoi.c b/tests/lib/libc/stdlib/t_strtoi.c
index 3301058a5be1..093b71abe04a 100644
--- a/tests/lib/libc/stdlib/t_strtoi.c
+++ b/tests/lib/libc/stdlib/t_strtoi.c
@@ -313,14 +313,9 @@ ATF_TC_BODY(strtoi_erroredges, tc)
{ "+5z", 5, 0, "z", 3, 7, ENOTSUP },
{ "+6z", 6, 0, "z", 3, 7, ENOTSUP },
{ "+7z", 7, 0, "z", 3, 7, ENOTSUP },
-   /*
-* PR lib/57828 suggests these should yield ERANGE, not
-* ENOTSUP, because ENOTSUP can be discovered
-* separately by the caller anyway.
-*/
-   { "-42z",3, 0, "z", 3, 7, ENOTSUP }, /* XXX ERANGE */
-   { "2z",  3, 0, "z", 3, 7, ENOTSUP }, /* XXX ERANGE */
-   { "42z", 7, 0, "z", 3, 7, ENOTSUP }, /* XXX ERANGE */
+   { "-42z",3, 0, "z", 3, 7, ERANGE },
+   { "2z",  3, 0, "z", 3, 7, ERANGE },
+   { "42z", 7, 0, "z", 3, 7, ERANGE },
};
 
intmax_t rv;
@@ -330,6 +325,9 @@ ATF_TC_BODY(strtoi_erroredges, tc)
 
for (i = 0; i < __arraycount(t); i++) {
 
+   if (t[i].rstatus == ERANGE)
+   atf_tc_expect_fail("PR lib/57828");
+
errno = 0;
rv = strtoi(t[i].str, , t[i].base, t[i].lo, t[i].hi, );
 

>From ee1b6083ea09f569e95ae5137b5fe870beb9d017 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell 
Date: Sat, 20 Jan 2024 18:19:34 +
Subject: [PATCH 2/2] strtoi(3): Return ERANGE before ENOTSUP if both are
 applicable.

Caller of strtoi can trivially detect the ENOTSUP case themselves by
checking whether end[0] == '\0', but they can't trivially detect the
ERANGE case (short of doing their own parsing which would defeat the
purpose of using strtoi(3)).

PR lib/57828
---
 common/lib/libc/stdlib/_strtoi.h | 9 ++---
 tests/lib/libc/stdlib/t_strtoi.c | 3 ---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/common/lib/libc/stdlib/_strtoi.h b/common/lib/libc/stdlib/_strtoi.h
index e6b0ce778b60..d48d58350050 100644
--- a/common/lib/libc/stdlib/_strtoi.h
+++ b/common/lib/libc/stdlib/_strtoi.h
@@ -104,9 +104,6 @@ INT_FUNCNAME(_int_, _FUNCNAME, _l)(const char * __restrict 
nptr,
/* No digits were found */
if (nptr == *endptr)
*rstatus = ECANCELED;
-   /* There are further c

Re: resource leaks in sethostent.c

[Taylor R Campbell]

> Date: Thu, 7 Dec 2023 13:00:40 -0800
> From: enh 
> 
> the malloc one was reported to Android (as
> https://android-review.googlesource.com/c/platform/bionic/+/2856549)
> by wuhaitao3 ; Chris Ferris
>  then spotted the goto nospc one.
> 
> the other function in the file looks fine.

Thanks, I filed PR lib/57758 (https://gnats.NetBSD.org/57758) to track
this fix and pullups into release branches.

Re: child of a multithreaded process looping in malloc()

[Taylor R Campbell]

> Date: Thu, 2 Nov 2023 20:05:28 +0100
> From: Edgar Fuß 
> 
> Oh well.
> 
> > It effectively does -- the calls to _malloc_prefork/postfork(_child)
> > are baked into fork in NetBSD's libc:
> It does in -current, but the problem shows up in -8.
> I'll try to back-port the changes.
> 
> Am I right that I'll need the changes of malloc.c 1.60, jemalloc.c 1.53 
> (and 1.48), extern.h 1.26 and pthread_atfork.c 1.15? Did I miss something?

Looks like this:

https://mail-index.netbsd.org/source-changes/2020/05/15/msg117364.html

Note that because it added symbols, it triggered a libc minor bump:

https://mail-index.netbsd.org/source-changes/2020/05/15/msg117366.html

So that might be why it didn't get pulled up.  (Although it's really
kind of a libc-private symbol, so maybe that doesn't matter much; on
the other hand, it does impose the requirement that you also override
the new symbols if you want to override malloc and not have any
jemalloc code around, e.g. with libbsdmalloc.)

Re: child of a multithreaded process looping in malloc()

[Taylor R Campbell]

> Date: Thu, 2 Nov 2023 18:26:43 +0100
> From: Edgar Fuß 
> 
> I've a long-standing problem of a process eating cpu time without doing 
> anything useful, which, most probably, goes like this:
> 
> Of a multi-threaded process,
> one thread is in malloc() while another thread fork()s.
> (So the child is born with the malloc lock held.)
> The child process (becoming single-threaded by the fork) calls malloc().
> The child loops forever because there's no other thread to release the lock.
> 
> Strictly speaking, malloc(3) is not declared to be async-signal-safe, so a 
> threaded program shouldn't call malloc() in a child before fork()ing.
> In my case, the code doing the fork/malloc is the Perl interpreter embedded 
> into collectd, so there's little I can do about it.
> 
> Couldn't malloc simply install a pthread_atfork() handler that releases 
> the lock in the child?

It effectively does -- the calls to _malloc_prefork/postfork(_child)
are baked into fork in NetBSD's libc:

https://nxr.netbsd.org/xref/src/lib/libc/gen/pthread_atfork.c?r=1.17#174
https://nxr.netbsd.org/xref/src/lib/libc/gen/pthread_atfork.c?r=1.17#183
https://nxr.netbsd.org/xref/src/lib/libc/gen/pthread_atfork.c?r=1.17#189

So you'll need to gather some more details about what's going wrong
here.

Perhaps this is a bug in the built-in _malloc_postfork, although that
seems unlikely.

Perhaps you've substituted your own private malloc, in which case in
order to make this work you'll have to either put your own locking
around your calls to it and fork to serialize them, or implement your
own _malloc_prefork/postfork(_child) instead.

Re: tar vs device special files

[Taylor R Campbell]

> Date: Sat, 28 Oct 2023 21:29:47 -0400 (EDT)
> From: Mouse 
> 
> So there _is_ a POSIX spec for tarchives?  Is the spec available, or is
> this yet another pay-to-play "standard"?  I've gone looking for specs
> for tar before, but each time I have, I've been unable to find anything
> that isn't behind a paywall of one sort or another (and thus a total
> nonstarter for me).
> 
> Admittedly, I haven't looked recently.

https://pubs.opengroup.org/onlinepubs/9699919799/utilities/pax.html#tag_20_92_13_06

Re: Hard link creation witout write access

[Taylor R Campbell]

[trimming tech-userlevel and tech-kern from cc list to avoid
cross-posting the entire thread]

> Date: Thu, 7 Sep 2023 11:53:56 + (UTC)
> From: RVP 
> 
> On Thu, 7 Sep 2023, Taylor R Campbell wrote:
> 
> > I think we should have these knobs on by default, but of course in
> > principle that might break existing configurations.  So maybe we could
> > put it in the default /etc/sysctl.conf -- that way you only get it on
> > upgrade if you merge updates to /etc.
> 
> I played with this after christos@ added the knobs last year[1], and then
> sort(1) broke badly. See PR 56775. Expect further squalls if this is turned
> on by default.

That suggests the semantics we've implemented for the sysctl knobs is
not quite right:

if (hardlink_check_uid && kauth_cred_geteuid(cred) != va.va_uid)
goto checkroot;

if (hardlink_check_gid && kauth_cred_groupmember(cred, va.va_gid) != 0)
goto checkroot;

It seems to me the rule should be:

1. If you own the file you can make hard links.
2. If you are in the file's group and the file is group-writable you
   can make hard links.
3. Maybe if the file is other-writable you can make hard links.

The problem with sort in /tmp is that hardlink_check_gid requires you
to be in the file's group _even if you own the file_, which is also a
bonkers restriction.

Hard link creation witout write access

[Taylor R Campbell]

Today I learned that you can create hard links to a file you don't own
and can't write to or even read from:

$ su -l root -c 'touch /tmp/foo && chmod 600 /tmp/foo'
$ ln /tmp/foo /tmp/bar

This strikes me as bonkers and a likely source of security issues.

POSIX says:

> The implementation may require that the calling process has
> permission to access the existing file.
>
> https://pubs.opengroup.org/onlinepubs/9699919799/functions/link.html

So this behaviour is allowed by POSIX but it would also be allowed to
make this fail with EACCES.  Unclear whether POSIX means ownership,
group membership, write access, or read access, but unless a POSIX
language lawyer can cite chapter & verse for the specific definition
of `has permission to access', I think this means the implementation
is allowed to apply any of those access rules?

Apparently we have sysctl knobs

security.models.extensions.hardlink_check_uid
security.models.extensions.hardlink_check_gid

to prohibit this bonkers linking, by prohibiting anyone but the owner
(hardlink_check_uid) or members of the group (hardlink_check_gid) from
creating hard links.  But the knobs are off by default.

Linux has a knob fs.protected_hardlinks which, if set, requires the
user to own or have write access to the file.

I think we should have these knobs on by default, but of course in
principle that might break existing configurations.  So maybe we could
put it in the default /etc/sysctl.conf -- that way you only get it on
upgrade if you merge updates to /etc.

Thoughts?

Re: new certificate stuff

[Taylor R Campbell]

> Date: Sun, 3 Sep 2023 12:21:23 -0700 (PDT)
> From: Paul Goyette 
> 
> If I migrate to this new world order (ie, I delete existing package
> and clean out the /etc/openssl/certs directory), what happens to any
> packages that currently depend on mozilla-rootcerts?  Will they
> somehow magically not need to install the mozilla-rootcerts package?

Packages that merely depend on mozilla-rootcerts are unaffected.  You
don't need to delete mozilla-rootcerts.  It can remain installed.  The
same applies to ca-certificates.  After you delete /etc/openssl/certs
and run postinstall:

- Packages that look to /etc/openssl/certs for trust anchors will
  begin to see the base ones, not the mozilla-rootcerts package ones.

- Packages that look to $LOCALBASE/share/mozilla-rootcerts for trust
  anchors will continue to see the mozilla-rootcerts package ones.

We'll eventually fix most or all of the packages in the second group
to stop looking to $LOCALBASE/share/mozilla-rootcerts and to instead
look to /etc/openssl/certs on NetBSD, /etc/ssl/certs on FreeBSD,
/etc/pki/whatever on Fedora,   But that won't happen immediately --
not pkgsrc-2023Q3.

(mozilla-rootcerts-openssl is a different story.  Nothing should
depend on this.  I just fixed the one case in pkgsrc of such an
incorrect dependency.  The new postinstall item for certctl(8) should
gracefully handle an existing mozilla-rootcerts-openssl install,
though.  If you want to let certctl(8) take over, you will have to
deinstall mozilla-rootcerts-openssl.)

Re: new certificate stuff

[Taylor R Campbell]

> Date: Mon, 28 Aug 2023 10:41:32 +0200
> From: Manuel Bouyer 
> 
> Maybe postinstall should check the /etc/openssl/certs.conf existance,
> and fail the 'fix opensslcerts' asking for it to be manually created;
> as we do for e.g. uid/gid if some are missing ?

I split it into two postinstall items:

- opensslcertsconf: handles missing /etc/openssl/certs.conf, in case
  you neglect to apply etcupdate or equivalent to bring in new config
  files.

  If you appear to be managing /etc/openssl/certs manually already,
  this sets `manual' in certs.conf; otherwise it copies the default
  one from /usr/share/examples/certctl/certs.conf.

- opensslcertsrehash: handles regenerating the /etc/openssl/certs
  cache from config.

  I also added a check operation so that this complains if and only if
  `certctl rehash' would create something different from what is
  currently in /etc/openssl/certs (or if it doesn't seem to be managed
  by certctl(8), but /etc/openssl/certs.conf doesn't set `manual').

Please let me know if you have any trouble with upgrades!

I'm trying to make sure this will provide a seamless fresh install and
upgrade path so that if you were already managing /etc/openssl/certs,
it stays that way, but if you weren't, certctl(8) takes over and makes
the Mozilla trust anchors available.  And I'd like to get this into 10
ASAP.

Make mtree(8) fail with missing/extra files?

[Taylor R Campbell]

Can mtree(8) be made to fail, i.e., return a nonzero exit status
(preferably 2), if there are any missing or extra files in the
directory tree relative to the specification?

If not, I think we should provide a way to do this.  What should the
option be called?  We may be running low on letters with mtree(8).

Alternative is to parse the output, I guess, which seems suboptimal.

Re: new certificate stuff

[Taylor R Campbell]

> Date: Mon, 28 Aug 2023 08:42:58 -0400
> From: Greg Troxel 
> 
> Taylor R Campbell  writes:
> 
> > How is using /etc/openssl/certs/.certctl as the token different from
> > using /etc/openssl/certs.conf as the token?
> 
> Because normal updates merge etc in various ways, and if certs.conf
> comes along with that (because it is in etc.tgz) then that is automatic
> and not an admin action.
> 
> > How does this satisfy the two criteria I set down in the previous
> > message?  Repeating here:
> >
> > (a) new installations get /etc/openssl/certs populated out of the box,
> 
> that dir is empty and certctl can write to it and set the sentinel.
> There is no problem populating an empty dir because that's not removing
> admin config.

OK, thanks, that's a good idea and I implemented it.  (I also fixed an
embarrassing mistake where I had added certs.conf to base rather than
to etc!)

The critical part I had missed is that certctl can claim _either_ a
directory it has already claimed, _or_ an empty directory, so it works
for new installations and to update pristine but old installations.

The way it works in HEAD is now:

1. New installations get /etc/openssl/certs populated out of the box.

2. On updates to existing installations that have _empty_
   /etc/openssl/certs, postinstall will take charge of it and populate
   it like a new installation.

3. On updates to existing installations (whether from 9.x to 10.0, or
   from 10.0 to 10.1) that have _populated_ /etc/openssl/certs,
   postinstall will fail and ask you to either set `manual' in
   /etc/openssl/certs.conf or move /etc/openssl/certs out of the way.

   This applies to anyone who is currently using mozilla-rootcerts,
   mozilla-rootcerts-openssl, or ca-certificates from pkgsrc, or
   anyone who has, like bouyer@, added any custom entries by hand.

   This will also apply to anyone who had updated to current and run
   postinstall in the time between 2023-08-26 and now, even if they
   hadn't otherwise populated /etc/openssl/certs, so I put a note in
   UPDATING about it.

   (Exception: If for some reason you had created the file
   /etc/openssl/certs/.certctl already, postinstall will quietly blow
   away your /etc/openssl/certs directory, of course.)

4. If you set `manual' in /etc/openssl/certs.conf, postinstall may
   print a message but will not touch /etc/openssl/certs and will not
   fail.

Let me know if any of this seems wrong, or if the implementation seems
to behave differently from what I described.  I added automatic tests
for the various cases of certctl, and I manually tested postinstall
with:

(a) nonexistent /etc/openssl/certs (create and populate)
(b) empty /etc/openssl/certs (populate)
(c) nonempty /etc/openssl/certs (fail)
(c) nonempty /etc/openssl/certs + manual (leave alone and pass)
(d) /etc/openssl/certs with .certctl (delete and recreate)


Regarding etcupdate: I agree that interactive prompting is bad for
deployment.  I don't actually use etcupdate myself, partly because it
is too interactive but mainly because it can't do three-way merges --
instead, I use a bespoke script called etcmerge that does three-way
merges using the old and new etc.tgz.

https://mumble.net/cgit/campbell/etcmerge

(Maybe some day I will propose to import this into base, but it needs
a lot more polish and testing first, and some tweaks to the usage
model which currently has too many things going on at once.)

But while I agree with your criticism of etcupdate, it's what we have
in base and what we recommend in the guide.  So that's what we have to
work with as a baseline to gauge the impact of changes like this on
update procedures; it's hard to meaningfully gauge if I have to guess
everything that anyone might try to do.

That said, you're right that it's better not to create things that
rely on the interactive prompt.

Re: new certificate stuff

[Taylor R Campbell]

> Date: Mon, 28 Aug 2023 06:30:05 -0400
> From: Greg Troxel 
> 
> Maybe this is too much, but perhaps certctl should look for a .certctl
> in /etc/openssl/certs and only if present rm/replace.  Or really only
> limit the rm; adding to an empty dir is fine.   Basically a token that
> says the dir is under the control of certctl.  -f can override the
> check and write the token.

How is using /etc/openssl/certs/.certctl as the token different from
using /etc/openssl/certs.conf as the token?

How does this satisfy the two criteria I set down in the previous
message?  Repeating here:

(a) new installations get /etc/openssl/certs populated out of the box,

and

(b) on _future_ updates (like 10.0 to 10.1, where both releases have
certctl(8) and a default certs.conf), /etc/openssl/certs gets
updated too (unless you set `manual' in /etc/openssl/certs.conf).


Please be specific either about what mechanism you mean and how it
meets these criteria, or about scenarios that you think the current
mechanism will inadequately address.


The status quo in HEAD (if pulled up to netbsd-10) will be:

1. New installations get /etc/openssl/certs populated out of the box.

2. Updates to existing installations that follow the long-standing
   procedure of

   - update kernel
   - unpack non-etc sets
   - run etcupdate
   - run postinstall

   will interactively prompt during etcupdate to install the new
   /etc/openssl/certs.conf.

   The file has comments explaining what it does and how to override
   it -- here's the current content:

  netbsd-certctl 20230816

  #   $NetBSD: certs.conf,v 1.1 2023/08/26 05:27:15 riastradh Exp $
  #
  # Configuration file for certctl(8) to manage HTTPS root CA
  # certificates in /etc/openssl/certs.
  #

  path/usr/share/certs/mozilla/server

  # For manual control over /etc/openssl/certs, e.g. if you want to
  # install a separate CA bundle from pkgsrc, uncomment the next line:
  #manual

   If the user declines to install /etc/openssl/certs.conf in
   etcupdate, then postinstall will print a message about failing to
   open it.  (postinstall should also fail with nonzero exit code in
   this case -- bug to be fixed.)

3. If:
   - you have /etc/openssl/certs.conf from 10.0,
   - you haven't set it to manual,
   - there are changes to mozilla-certdata from 10.0 to 10.1, and
   - you update to 10.1,
   then those changes will be reflected in /etc/openssl/certs on
   postinstall (but etcupdate won't prompt anything about certs.conf
   -- unless something has also changed in that, of course).

4. If you _don't_ have /etc/openssl/certs.conf from 10.0 (you deleted
   it or you never let etcupdate create it), and you update to 10.1,
   then it's the same deal as (2).

If you follow a different procedure for update, like naively unpacking
etc sets or running a bespoke etcupdate replacement, well, you should
expect to have to deal with the fallout yourself.

Re: new certificate stuff

[Taylor R Campbell]

> Date: Sat, 26 Aug 2023 19:15:01 +0200
> From: Manuel Bouyer 
> 
> On Sat, Aug 26, 2023 at 04:48:59PM +0000, Taylor R Campbell wrote:
> > [...]
> > If you currently use security/mozilla-rootcerts or
> > security/ca-certificates (or security/mozilla-rootcerts-openssl) to
> > populate /etc/openssl/certs, and you want to continue to use it, you
> > will have to put the line `manual' in /etc/openssl/certs.conf before
> > you next run postinstall(8).
> 
> Will postinstall remove any certificate in /etc/openssl/certs/
> if there is no certs.conf ? I have server certificates here, in addition
> to some local (private) CA roots.

Currently, if /etc/openssl/certs.conf doesn't exist, `certctl rehash'
(the crux of `postinstall fix opensslcerts') will print an error
message and then exit with status 0.  This combination is a bug --
need to think a bit about it, but probably better to exit nonzero than
to suppress the error message.

So if you unpack new _non-etc_ sets, `postinstall fix' won't
clobber your /etc/openssl/certs directory.

The etc.tgz set, however, will have /etc/openssl/certs.conf.  So if
you naively unpack etc.tgz, `postinstall fix' will clobber your
/etc/openssl/certs directory.

That said, I think if you use etcupdate(8), it will interactively
prompt you before creating the new /etc/openssl/certs.conf.  (Have
made a note to add this in my etcmerge(8) tool to do a three-way merge
for updating (x)etc sets too.)

I'm open to other suggestions about how to handle the transition from
manually maintained /etc/openssl/certs on (say) 9.x with no certs.conf
or certctl(8) to 10.0 with new default certs.conf and certctl(8),
provided that

(a) new installations get /etc/openssl/certs populated out of the box,

and

(b) on _future_ updates (like 10.0 to 10.1, where both releases have
certctl(8) and a default certs.conf), /etc/openssl/certs gets
updated too (unless you set `manual' in /etc/openssl/certs.conf).

Re: new certificate stuff

[Taylor R Campbell]

> Date: Sat, 26 Aug 2023 08:20:50 -0700 (PDT)
> From: Paul Goyette 
> 
> OK, I tried to read and understand the thread, but not really sure I
> succeeded with the understanding part.  (In fact, i'm pretty sure I
> failed that part, miserably.)

This is about enabling TLS clients -- like ftp(1), pkg_add(1),  --
connecting to a server to verify that the server owns the name you
used to connect to it, according to a directory of certification
authorities (CAs) curated and shipped by Mozilla.

This is specifically about managing /etc/openssl/certs, the place
where applications using OpenSSL will look by default for trusted CA
certificates (or `trust anchors').

> I've got a simple set-up here, running postfix and pine for Email, and
> of course f-fox for browsing.  I've never done anything (at least, not
> deliberately) with certificates;  reading and writing Email just works,
> as does most browsing.
> 
> Will I need to do anything new (or differently) as a result of these
> recent changes?

Probably not.

- If pine is just reading a local mbox or maildir, or talking to an
  imap server at localhost, it won't be affected.

- I don't think Postfix will do any TLS validation unless you ask it
  to explicitly with smtp_tls_* or smtpd_tls_* options or similar,
  which you presumably haven't done.

- Firefox uses its own internal trust anchors and is not affected by
  /etc/openssl/certs.

If you currently use security/mozilla-rootcerts or
security/ca-certificates (or security/mozilla-rootcerts-openssl) to
populate /etc/openssl/certs, and you want to continue to use it, you
will have to put the line `manual' in /etc/openssl/certs.conf before
you next run postinstall(8).

Re: postinstall(8): Add opensslcerts item to regen /etc/openssl/certs.

[Taylor R Campbell]

> Date: Sat, 26 Aug 2023 06:50:22 -0400
> From: Jason Thorpe 
> 
> > On Aug 26, 2023, at 1:59 AM, Taylor R Campbell  wrote:
> > 
> > postinstall(8): Add opensslcerts item to regen /etc/openssl/certs.
> > 
> > Works only with destdir /, since it relies on running openssl(1),
> > which is not available as a tool or required in the cross-build
> > environment.
> 
> Maybe there should be a boot-time check in an rc script for an
> out-of-date trust cache?

That would be reasonable, but I didn't want to create a new reason
requiring /etc to be writable during normal boot.

Right now, to keep it simple and reliable, certctl(8) works by
deleting /etc/openssl/certs and recreating it; there's no mechanism to
update /etc/openssl/certs incrementally or check whether it is out of
date.  So at the moment, `certctl rehash' always requires /etc to be
writable.

We could create a mechanism to check whether it is out of date (both
to check for missing symlinks and to check for extraneous symlinks and
to check for mismatched symlinks), and define a new command to invoke
it, and add new tests for it, and use that in an /etc/rc.d script.

It wouldn't hurt to have all that, but it's a bunch of extra work.
And the normal install (and upgrade) procedure always goes through
postinstall(8) anyway.  So that's where I started.

Re: [PATCH] HTTPS/TLS CA certificates in base

[Taylor R Campbell]

> Date: Wed, 23 Aug 2023 16:29:21 -0400
> From: Thor Lancelot Simon 
> 
> I would like to be sure we will avoid any use of public CA's certificates
> to establish trust for upgrades of NetBSD itself, or of packages.  Otherwise,
> we will find ourselves in a situation where we can never recover if a CA
> goes rogue.

Well, right now, there's _nothing_ used to automatically verify binary
upgrades or packages, so it's already worse than the problem you're
alluding to.  (The only authenticated end-to-end path is source-only.)

With the change, the public CA certificates would be available to
validate TLS/HTTPS connections used to download sets and packages in
transit, at least (cdn-to-end, that is -- still not end-to-end).

But these will not be used to verify signatures on binary upgrades or
packages at rest (end-to-end, i.e., builder-to-end), if that's what
you're asking.

The public CA certificates may still be used _on top_, of course, by
doing downloads through HTTPS, but not for verifying signatures on the
binary sets/packages (or manifests of them) from the origin.  Separate
plans for that, more to come later.

Re: [PATCH] HTTPS/TLS CA certificates in base

[Taylor R Campbell]

> Date: Sun, 20 Aug 2023 22:32:38 +0100
> From: David Brownlee 
> 
> There was a previous thread that mooted the idea of using the project
> built mozilla-rootcerts packages (which are just tarfiles) as the
> source for some mechanism to populate on-system certificates, such as
> your proposed certctl. (mozilla-rootcerts is the base package which
> just populates into PREFIX, not mozilla-rootcerts-openssl which put
> data in /etc)
> 
> https://mail-index.netbsd.org/tech-userlevel/2023/08/04/msg014092.html

I considered that but I think this path requires too much of a Rube
Goldberg machine of moving parts to get working and maintain.  The
patch I posted is ready, today; took just two days of local work in my
spare time to draft and test; and puts no additional burden on releng
or package infrastructure.

> It would probably involve:
> - Ensuring that each quarterly package release put the latest
> mozilla-rootcerts in a Well Defined Location

Requires creating a new notion of architecture-independent packages
and a new well-defined location to put them and processes to build and
maintain them long-term.

certctl(8) already has an obvious place (base.tgz), which already has
established long-term processes to build and maintain.  It functions
like nsswitch as a point of system integration that can use built-in
parts (/usr/share/certs/mozilla) or be reconfigured with other
certificate sources you have the option of installing elsewhere.

> Which would give:
> - Always getting the latest certificates on install, whether
> installing 10.0 the moment its released, or in three years time

I expect in three years time there won't be much reason to install
10.0; instead it'll be 10.3 or 10.4.  And by that time (or well before
it), I'm hoping to have a cheaper idea of patch releases that can be
done on a weekly timescale and updated automatically so the 10.3
installer can promptly update to 10.3.17 with certificate updates --
and other security fixes.

> - The same location to pick up updated certificates for a previously
> installed system

True.  But I'm not sure it matters that much.  Anyone who really needs
the stability of an older release, like sborrill, can presumably just
build their own images with the package pre-included -- and is
probably already doing that.

> There is still the bootstrapping issue, which could be managed by any of:
> - Including just enough NetBSD certificates in base to make the initial 
> download
> - Signed packages
> - Ignoring the issue and just installing over https without validation
> 
> The mechanism for getting the mozilla-rootcerts package data onto the
> system could be:
> 1) certctl Just Downloads And Extracts The Package Tarfile
> 2) Having the default sysinst flow install pkgin and mozilla-rootcerts
> (with opt-out), which also provides a ready mechanism to keep the data
> updated (pkgin upgrade)

My priority #1 here is to make netbsd-10 do TLS validation out of the
box with a reasonable, if not perfect, update story.  I don't want
that to be held up on any other major moving parts like signed
packages, pkgin, and base updates.

That's not to say these other moving parts aren't important to get
done, but I sifted through several avenues for getting them _all_ done
(some of which are sitting in my drafts as we speak), and decided that
this certctl(8) proposal has the best cost/reward ratio at the same
time as having the least risk for netbsd-10 as well as a clear path to
integration with future base updates and replacement by packages.

(With apologies for stepping on your toes about this --
architecture-neutral packaging may be a good idea but it's just not
going to be ready in a week and netbsd-10 is already extremely late.)

Re: [PATCH] HTTPS/TLS CA certificates in base

[Taylor R Campbell]

> Date: Sun, 20 Aug 2023 10:38:01 -0400
> From: Greg Troxel 
> 
> I'd like to see three things handled (which might be already):
> 
> 1)
> 
>   a way for a user to install a CA cert (as a trust anchor -- I think it
>   would be good for docs to use pkix terminology) that is not part of
>   the mozilla root set, such that as updates/etc. happen, the trust
>   anchor set remains
> union of
>   user-configured
>   mozilla set, if opted into, minus those excluded
> 
> This is just code and I don't care if the user has to put the cert in
> /etc/openssl/manual-trust-anchors or someplace like that to be available
> for unioning, which would make certctl be able to just write /certs.
> I am guessing your patch already handles this somehow, but I wanted to
> comment quickly before reading this since I have ranted so much.

This is exactly what you get if you populate a directory
/usr/local/mycerts with the .pem certificates you want and then add
the line

path /usr/local/mycerts

to /etc/openssl/certs.conf, alongside the line

path /usr/share/certs/mozilla/server

which is already in it.

The only restriction is that the base names must be distinct across
all the paths specified.

> 2) On installation, either
> 
>   a yes/no question "do you want to configure the Mozilla root
>   certificate set as trusted"?

I understand the sentiment, but our installer is already a nightmare
of impenetrable questions, trapping you into making obscure technical
decisions on the spot whose consequences are not obvious and it's
unclear whether you can change your mind later or how.

We need to reduce these questions in sysinst, not add more questions
especially when they have obviously sensible default answers which
most users don't even know to contemplate changing.  (`What the heck
is a BIOS disk geometry?')

The rare users who know they want custom trust anchors will know to
look for how to change them, or build custom images with custom
certs.conf.  We don't need to impose a prompt on everyone just to
satisfy those users.

> or
> 
>   a "trust mozilla root certificate set" as a yes/no option in
>   configuration where you turn on sshd/ntpd, and I don't really care how
>   it defaults.

An unobtrusive optional item in the sysinst utility and/or config menu
would be fine by me, along with a note in afterboot(8).  (I feel like
the config menu ought to have a reference to afterboot(8) so you know
how to go back and reconsider the config later.)  But I don't think
the initial commit needs to block on this.

> 3) If a user wants to configure (as an example)
>The US DOD root (a "real CA" not in the mozilla set)
>a personal CA that they created
>Let's Encrypt, and 8 other specific CAs from mozilla
>
> then that should be supported somehow in the certctl config file.  I
> suspect you've enabled that, and it's just omitting the "use the mozilla
> set" and adding lines for "use this" and "use that".

Create a directory of .pem files or symlinks to them and put it in a
`path' line instead of `path /usr/share/certs/mozilla/server' in
certs.conf.

Or, create three directories and add three `path' lines.

> A) I think it's fine to only have "postinstall fix" run certctl in the
> DESTDIR=/ case and not at all urgent to address.   This is sort of like
> how we do'n build locate/man db etc. in images but only on real systems.
> 
> (Alternatively we could run certctl rehash at boot if certctl=YES,
> default YES and skip it from postinstall.  I have not thought through
> pros and cons.)

I considered that, but postinstall(8) is better because it doesn't add
more reasons that /etc must be writable at boot-time.

That said, we can change the mechanism afterward if we decide there's
a better way.

> B) There is talk of enforcing validation, but we already enforce
> validation with an empty list.  I think the only change is installing a
> bunch of CAs as trust anchors (in a manageable way), and there is no
> change to validation.  If I'm confused on this point please tell me.

The patch I attached only adds the certificates, certctl(8), and
automatic tests.  It does not change whether any applications use
/etc/openssl/certs or require validation.

Currently, as of February, ftp(1) has validation enabled by default,
with a default-off option `sslnoverify' to disable validation.  That
means it just doesn't work out of the box right now on real-world web
sites.

I don't know offhand whether pkg_add(1) has validation enabled or
disabled by default, but we should enable it after this goes in.
(That's aside from signing packages, which we should also do, but my
plan for that is not ready yet.)

> C) The openssl nomenclature /etc/openssl/certs is confusing because it
> blurs what is a certificate and what certificates are trust anchors; it
> really (I think) means the latters.  I realize that few understand this
> naming distinction, but a CA cert is just a cert whose private key can
> sign other certs, and you may or may not 

Re: [PATCH] HTTPS/TLS CA certificates in base

[Taylor R Campbell]

> Date: Sun, 20 Aug 2023 05:43:23 +0200
> From: Jan Danielsson 
> 
> My objection in the past has been along the line of:  If an 
> organization is not willing to keep a CA bundle up-to-date for a user, 
> then it should not dump a CA bundle that may grow stale onto their 
> system either.  But that's more of a "pick a well-trusted CA bundle, and 
> provide a mirror of it that people can synchronize from -- and keep it 
> up-to-date." argument, rather than a "don't do it" argument.

Here's what I wrote in the original message:

> > Some objections in the past included:
> > [...]
> > - We can't do this until we have a way to ship automatic updates
> >   frequently enough to remove compromised CAs' certificates.
> >   => It is true we need a better automatic update story, especially
> >  for security patches, but that's not specific to CAs.
> >   => Removal mostly happens because of expiry or disuse, not for audit
> >  failure or compromise.  Although it does happen, removal for
> >  audit failure is rare, and removal for compromise is extremely
> >  rare.
> >   => Even before we implement automatic updates, under this proposal
> >  NetBSD security advisories can provide quick and easy steps for
> >  remediation to exclude individual CA certificates.

More details below.

> Will the in-tree bundle be updated regularly?  I could probably live 
> with "Keep your NetBSD base system updated to keep your CA bundle 
> updated", but if I would rebuild my systems from the latest sources and 
> not get the latest bundle I'd probably find it to be a little annoying.

Yes, the idea is to keep it updated regularly.

The last upstream update was on 2023-07-19.  The last one before that
was 2022-11-29.  This is not high frequency, so it shouldn't be much
of a maintenance burden; we have other upstream software that updates
with higher frequency.  Here's the commit log:

https://hg.mozilla.org/projects/nss/log/tip/lib/ckfw/builtins/certdata.txt

The most recent security-relevant incident was on 2022-11-30 when
TrustCor flunked public scrutiny.  (But there was no evidence of
mis-issued certificates.)

> Rhetorical Devil's advocate question:  What's the potential blast 
> radius for the worst case scenario where a CA's private key is 
> compromised before its certificate expires and a bunch of NetBSD users 
> don't update their bundle for two years?

This is no different from any other security issue on a machine on the
internet.

Once we have an automatic update system, upstream updates to
certdata.txt can be carried through it.

Until then, we can continue doing what we've been doing for security
issues: publish a security advisory.

The security advisory can include the following one-liner step for
remediation:

# certctl untrust 
/usr/share/mozilla/certs/server/Acme_Certs_and_Roadrunner_Traps.pem

Alternatively, you can keep doing what you were doing with (say)
mozilla-rootcerts or ca-certificates from pkgsrc, by overwriting
/etc/openssl/certs.conf as follows to ensure that, when you update
NetBSD, certctl(8) won't clobber the /etc/openssl/certs directory
populated by the package:

# cat 

Re: system(3) semantics when SIGCHLD is SIG_IGN'd

[Taylor R Campbell]

> Date: Sun, 13 Aug 2023 13:03:09 +0200
> From: Rhialto 
> 
> On Sat 12 Aug 2023 at 11:58:36 +0000, Taylor R Campbell wrote:
> > Cons:
> > - POSIX doesn't ask system(3) to work when SIGCHLD is set to SIG_IGN
> >   or when it has SA_NOCLDWAIT set, so this code is nonportable anyway;
> 
> I read through
> https://pubs.opengroup.org/onlinepubs/007908799/xsh/system.html and it
> seems entirely silent on the matter. How does this mean that system(3)
> is allowed to fail in this situation? Since the state is not mentioned
> as a special case, I expect that it is supposed to work.

Newer versions of POSIX give a sample implementation of system(3)
which fails the same way under these circumstances:

https://pubs.opengroup.org/onlinepubs/9699919799/functions/system.html

The only thing it does with SIGCHLD is block it temporarily.

Re: epoll exposure

[Taylor R Campbell]

> Date: Sat, 12 Aug 2023 19:21:06 -0400
> From: Christos Zoulas 
> 
> 2. Nobody has given an example of an application that breaks, or answered
> the question if we understand how the Illumos feature is breaking things,
> or even if the Illumos implementation is similar to ours. Theodore 
> mentioned
> that aside from the fork issue, we should be fully compatible. Do we know
> of any applications that open an epoll file descriptor and then fork? Was
> that the reason the the Illumos implementation is failing?

I'm not sure and I'd be curious to hear more.  The illumos man page
details some semantic differences: .
But I think the pkgsrc experience should definitely give us pause and
we should try to coordinate it, e.g. by doing bulk builds and testing
applications that we determine use epoll, rather than barge ahead and
assume pkgsrc developers will find and fix the fallout.

Backing out the change later, if we might decide to do that, is very
painful, and it's why we're still stuck with the awful getrandom(2)
API that I made the mistake of adopting back in 2020.  (Although if we
have to rebuild all netbsd-10 packages anyway for openssl3, maybe
now's our chance to ditch it...)

> 3. We discussed adding epoll as a native syscall in tech-kern and there were
> no objections.

It was proposed on 2023-06-21 at
.

The first reply within hours on 2023-06-21 at

asked why it is a good API to adopt, and whether for testing the
compat syscall we could add an emul_syscall so it doesn't get exposed
to applications.

The second reply within a week on 2023-06-25 at

asked that it be done in a way that can be used by ATF but will not be
detected by configure scripts.

I read both of these as objections.  No reply answering either of
these, and the commit went in in a way that is exposed to configure
scripts and not limited to the ATF tests.

I think it is fair for nia to find this frustrating: mixed feedback on
adopting the syscall, two objections with specific requests for how to
start, and it goes in in exactly the way that the objections objected
to.

>It is common courtesy to discuss reversions before taking
> action, specially when things do not appear to be broken. It would have
> taken a minute or so for Nia to write an email and one or two more days
> with epoll(2) exposed would not have harmed anyone. I was under the
> impression that the conversation was still going on.

The commit went in on Friday, 2023-07-28, and on Monday, 2023-07-31,
nia re-raised the same objection as before, with more detail:
.

nia suggested a concrete alternative to enable testing (installing it
at sys/epoll_compat.h) on 2023-08-01:

and there was silence in the thread for another ten days, suggesting
the discussion was over and nia's objections were being ignored.

It is true that our rule is for reverts to go through discussion and
then core@ first, which this could be seen to violate.  But:

(a) nia did try to go through discussion by raising objections twice
over the course of a month, with no response;
(b) this wasn't entirely a revert -- all the code is still there, just
not exposed from libc; and
(c) I think a lot of people have gotten the impression that core@ is
anemic and inattentive, and it's on us to dispel that impression
by acting responsively and responsibly in the community, not just
by asserting authority by insisting on the text of rules.

So, moving forward:

How about we prioritize implementing an emul_syscall or something so
we can automatically test compat_linux?

This will be a much bigger win, I think, than adopting epoll(2), and
if epoll(2) is really worth it (and I'm not sure it is; I'd like to
see a more compelling positive argument for adopting it) we can
separately coordinate that with pkgsrc resources.

Re: system(3) semantics when SIGCHLD is SIG_IGN'd

[Taylor R Campbell]

> Date: Sat, 12 Aug 2023 22:48:05 -0400 (EDT)
> From: Mouse 
> 
> > If the calling process has SA_NOCLDWAIT set or has SIGCHLD set to
> > SIG_IGN, [...]
> 
> Check my understanding: this applies to wait(2), but not alternatives
> like waitpid(2) or wait4(2), right?

If you read all the way to the end of the sentence I quoted, you'll
find your answer:

[...] and the process has no unwaited for children that were
transformed into zombie processes, the calling thread will block
until all of the children of the process containing the calling
thread terminate, and wait() and waitpid() will fail and set errno
to [ECHILD].

https://pubs.opengroup.org/onlinepubs/7908799/xsh/wait.html

Newer POSIX extends this to waitid(2).  wait3, wait4, and wait6 are
technically non-POSIX extensions and so could, in priciple, be used to
avoid this if there were a mechanism.

But there's no mechanism in the kernel for recording the information
when SIGCHLD is set to SIG_IGN because the information is destroyed
when the process exits and there's nowhere to store it -- that's what
a zombie process is for, and setting SIGCHLD to SIG_IGN/SA_NOCLDWAIT
asks the kernel not to leave zombie processes.  The kernel would need
to know that the parent wants a zombie for the the child spawned by
system(3) but not any other children.

> > So, should we do anything about this in system(3)?
> 
> > Cons:
> > [...]
> > - Changing signal actions has the side effect of clearing the signal
> >   queue, and I don't see a way around that.
> 
> But is changing the signal action the only way to make system(3)
> "work"?  I'm not convinced it is.

Open to other ideas!  But I'm leaning toward thinking we should just
leave it as is.  It looks like glibc doesn't do anything about this
either, for instance.

system(3) semantics when SIGCHLD is SIG_IGN'd

[Taylor R Campbell]

What should system(3) do when the signal action for SIGCHLD is
SIG_IGN, or has SA_NOCLDWAIT set?


Setting SIGCHLD to SIG_IGN has the effect of reaping zombie children
automatically, so that calling wait(2) is unnecessary to reap them --
and, further, doesn't return _at all_ until the last child has exited.

This semantics -- same as setting SA_NOCLDWAIT -- is enshrined in
POSIX:

If the calling process has SA_NOCLDWAIT set or has SIGCHLD set to
SIG_IGN, and the process has no unwaited for children that were
transformed into zombie processes, the calling thread will block
until all of the children of the process containing the calling
thread terminate, and wait() and waitpid() will fail and set errno
to [ECHILD].

https://pubs.opengroup.org/onlinepubs/7908799/xsh/wait.html

So if a process already has a child, and calls system(3) as it is
currently implemented in libc in ~all versions of NetBSD, system(3)
will hang indefinitely until the existing child exits.

This manifests in newer versions of ctwm which set SIGCHLD to SIG_IGN
if you have a .xsession file that does something like:

xterm &
xclock &
exec ctwm

This causes ctwm to start with two children already, which in turn
causes system(3) to hang when you try to start an application from the
ctwm menu.

The ctwm hang led to PR kern/57527 (https://gnats.netbsd.org/57527,
`kern' because at first it looked like a missing wakeup in the kernel
before we realized this is exactly how POSIX expects SIG_IGN and
SA_NOCLDWAIT to behave), which has some litmus tests for the semantics
and draft code to mitigate the situation in system(3).

So, should we do anything about this in system(3)?

Pro: Makes existing code code like ctwm work.

Cons:
- POSIX doesn't ask system(3) to work when SIGCHLD is set to SIG_IGN
  or when it has SA_NOCLDWAIT set, so this code is nonportable anyway;
  might break on other systems too, so breakage on NetBSD leading to
  an upstream bug report is helpful.
- Changing signal actions has the side effect of clearing the signal
  queue, and I don't see a way around that.

Alternative would be to say: don't do that; fix the buggy code that
calls system(3) with SIGCHLD ignored, either by having it set a signal
handler that calls waitpid(-1, NULL, WNOHANG) until success, or by
having it use something other than system(3).

Re: /etc/services losses

[Taylor R Campbell]

> Date: Thu, 3 Aug 2023 12:41:53 +0200
> From: Hauke Fath 
> 
> IANA appears to have settled on submissions for port 465 (which, 
> coincidentally, was assigned to 'urd' in the netbsd-5 version, and the 
> NetBSD addition then declared the smtps alias). A web search for 
> 'smtps' confirms widespread use, so this should be maintained as an 
> alias for 'submissions' IMO.

`smtp(s)' and `submission(s)' are subtly different protocols and
should not be aliases:

- smtp(s) is for MTA<->MTA exchange of fully formed internet mail
  messages with complete headers.

- submission(s) is for an MUA to submit new messages, which may not
  have complete headers or fully qualified addresses or otherwise be
  fully formed, via a mail submission agent into the internet mail
  system.

I'm not sure if there is any port number that can rightly be called
`smtps' today.  I'm also not sure it matters if a TLS session is
preceded by the ten bytes `STARTTLS\r\n' on the wire or not.

Re: sed(1) and LC_CTYPE

[Taylor R Campbell]

> Date: Wed, 26 Jul 2023 17:32:03 +0200
> From: tlaro...@polynum.com
> 
> If setting LC_CTYPE to this:
> 
> $ export LC_CTYPE=fr_FR.ISO8859-15
> 
> and then:
> 
> $ echo "éé" | sed 's/é/\/g'
> sed: 1: "s/é/\/g": RE error: trailing backslash (\)
> 
> Where does the program manage to find a backslash i.e. 0134? While
> 'é' is 0351.

Exactly what bytes are passed as an argument to sed?  Can you write a
program that will hexdump argv[1] and pass the same argument to it?

Next step, if that reveals the expected 0xe9, is to find exactly what
string is passed to regcomp inside sed.

Re: DRM/KMS: countdown started

[Taylor R Campbell]

> Date: Thu, 13 Jul 2023 11:35:50 +0200
> From: tlaro...@polynum.com
> 
> Severing will be the first thing done. Whether I will be able, in this
> first stage, to allow, from this newly created branch, to still
> optionally use the current implementation of DRM/KMS is an open
> question, and this goal is beyond the objectives of the first stage.

You know you can just omit the drm drivers from the kernel, right?
For example, on amd64:

no i915drmkms* at pci?
no radeon* at pci?
no nouveau* at pci?
#no amdgpu* at pci?   # (not even enabled by default on amd64)

You can also disable them at boot-time with userconf, and you'll get a
dumb pci framebuffer with genfb instead:

   > boot -c
   userconf> disable i915drmkms
   userconf> disable radeon
   userconf> disable nouveau
   userconf> quit

Same for the Arm boards that use the drm modesetting APIs, like
tilcdc(4) and tifb(4).  But you'd have to rewrite all those drivers --
and test them on a menagerie of Arm boards where the drm stuff has
already been tested and works and has very small maintenance burden at
this point.

I think you'll find you've misplaced where the painful parts of all
this are, but go ahead...

Re: Trivial program size inflation

[Taylor R Campbell]

> Date: Tue, 4 Jul 2023 18:30:58 +
> From: Taylor R Campbell 
> 
> Other low-hanging fruit I see would be to split getenv out of the
> other environment variable operations, to get rid of locking and
> rbtree stuff.  But it's diminishing returns at this point -- not clear
> the attached patchset/diff is worth committing.  Gets me down to this:
> 
>textdata bss dec hex filename
>   3074432803376   374009218 main
> 
> After that, maybe we could find a substitute for the syslog_ss stuff
> in stack-protector -- but that's probably not worth the trouble, for
> what is maybe another few kilobytes or so.

...actually attached
>From 14201a18296b67e2e254566ce0c5f3106c0b3e67 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell 
Date: Tue, 4 Jul 2023 18:16:08 +
Subject: [PATCH 1/3] rbtree(3): New RB_TREE_INITIALIZER macro.

Allows static initialization of an rbtree.

XXX pullup-10
---
 sys/sys/rbtree.h | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/sys/sys/rbtree.h b/sys/sys/rbtree.h
index b4a8ba3f7fc1..686cf2ed299a 100644
--- a/sys/sys/rbtree.h
+++ b/sys/sys/rbtree.h
@@ -125,12 +125,17 @@ TAILQ_HEAD(rb_node_qh, rb_node);
 #defineRB_TAILQ_INSERT_HEAD(a, b, c)   TAILQ_INSERT_HEAD(a, b, 
c)
 #defineRB_TAILQ_INSERT_BEFORE(a, b, c) TAILQ_INSERT_BEFORE(a, 
b, c)
 #defineRB_TAILQ_INSERT_AFTER(a, b, c, d)   TAILQ_INSERT_AFTER(a, 
b, c, d)
+
+#defineRBDEBUG_TREE_INITIALIZER(t) 
  \
+   .rbt_nodes = TAILQ_INITIALIZER((t).rbt_nodes),
 #else
 #defineRB_TAILQ_REMOVE(a, b, c)do { } while 
(/*CONSTCOND*/0)
 #defineRB_TAILQ_INIT(a)do { } while 
(/*CONSTCOND*/0)
 #defineRB_TAILQ_INSERT_HEAD(a, b, c)   do { } while 
(/*CONSTCOND*/0)
 #defineRB_TAILQ_INSERT_BEFORE(a, b, c) do { } while 
(/*CONSTCOND*/0)
 #defineRB_TAILQ_INSERT_AFTER(a, b, c, d)   do { } while 
(/*CONSTCOND*/0)
+
+#defineRBDEBUG_TREE_INITIALIZER(t) /* nothing */
 #endif /* RBDEBUG */
 
 /*
@@ -181,6 +186,12 @@ typedef struct rb_tree {
 #defineRBSTAT_DEC(v)   do { } while (/*CONSTCOND*/0)
 #endif
 
+#defineRB_TREE_INITIALIZER(t, ops) (rb_tree_t) 
  \
+{\
+   .rbt_ops = (ops), \
+   RBDEBUG_TREE_INITIALIZER(t)   \
+}
+
 void   rb_tree_init(rb_tree_t *, const rb_tree_ops_t *);
 void * rb_tree_insert_node(rb_tree_t *, void *);
 void * rb_tree_find_node(rb_tree_t *, const void *);

>From ef18c568cb559da7e82e7951050cf7cd54b0a350 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell 
Date: Tue, 4 Jul 2023 18:17:23 +
Subject: [PATCH 2/3] libc: Use RB_TREE_INITIALIZER to nix initfini.c/_env.c
 coupling.

XXX pullup-10
---
 lib/libc/misc/initfini.c |  3 ---
 lib/libc/stdlib/_env.c   | 10 ++
 2 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/lib/libc/misc/initfini.c b/lib/libc/misc/initfini.c
index 9f5cec1d2cce..6d17c167c71e 100644
--- a/lib/libc/misc/initfini.c
+++ b/lib/libc/misc/initfini.c
@@ -132,7 +132,4 @@ _libc_init(void)
 
/* Initialize the atexit mutexes */
__libc_atexit_init();
-
-   /* Initialize environment memory RB tree. */
-   __libc_env_init();
 }
diff --git a/lib/libc/stdlib/_env.c b/lib/libc/stdlib/_env.c
index 6608522790f8..316459356bf7 100644
--- a/lib/libc/stdlib/_env.c
+++ b/lib/libc/stdlib/_env.c
@@ -73,7 +73,8 @@ static const rb_tree_ops_t env_tree_ops = {
 };
 
 /* The single instance of above tree. */
-static rb_tree_t   env_tree;
+static rb_tree_t   env_tree =
+RB_TREE_INITIALIZER(env_tree, _tree_ops);
 
 /* The allocated environment. */
 static char**allocated_environ;
@@ -401,10 +402,3 @@ __unlockenv(void)
 }
 
 #endif
-
-/* Initialize environment memory RB tree. */
-void __section(".text.startup")
-__libc_env_init(void)
-{
-   rb_tree_init(_tree, _tree_ops);
-}

>From 1d295afec1bd9645c672d8702ce6ee7ad5c32304 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell 
Date: Tue, 4 Jul 2023 17:26:18 +
Subject: [PATCH 3/3] WIP: libc: Split read-only environment logic into a
 separate unit.

The idea is to prune all the locking, rb, allocation, , from
statically linked programs that just call getenv.

Still WIP because I can't figure out how to make statically linked
read-only access elide the locking.

XXX pullup-10 (if we work out the issues)
---
 lib/libc/stdlib/Makefile.inc |   2 +-
 lib/libc/stdlib/_env.c   | 107 ++
 lib/libc/stdlib/_envlock.c   |  90 +
 lib/libc/stdlib/_findenv.c   | 108 +++
 lib/libc/stdlib/local.h  |   2 +
 5 files changed, 207

Re: Trivial program size inflation

[Taylor R Campbell]

> Date: Fri, 30 Jun 2023 13:37:10 -0400 (EDT)
> From: Mouse 
> 
> sparc, my mutant 1.4T:
> 
> textdatabss dec hex filename
> 12616   124 288 13028   32e4main
> 
> amd64, my mutant 5.2:
> 
>text  data bss dec hex filename
>  152613  4416   16792  173821   2a6fd main
> 
> amd64, 9.0_STABLE (ftp.n.o):
> 
>textdata bss dec hex filename
>  562318   29064 2176416 2767798  2a3bb6 main
> 
> 12K to do nothing is bad enough (I'm going to be looking at why it's
> that big).  149K is even more disturbing (I'll be looking at that too).
> But over half a meg of text and two megs of BSS?  To do nothing?
> Surely something is wrong somewhere.

I just touched up libbsdmalloc in HEAD so that you can use it as a
replacement for jemalloc in libc by statically linking with
-lbsdmalloc:

$ cat main.c
int main(void) { return 0; }
$ cc -o main main.c -static -Wl,--whole-archive -lbsdmalloc 
-Wl,--no-whole-archive -Wl,-Map=main.map
$ size main
   textdata bss dec hex filename
  3568033123472   42464a5e0 main

The --whole-archive dance is unnecessary if your program calls malloc,
calloc, realloc, aligned_alloc, posix_memalign, or free.  If the size
still turns up unreasonably large, you can examine main.map to see
what's pulling in what.

Other low-hanging fruit I see would be to split getenv out of the
other environment variable operations, to get rid of locking and
rbtree stuff.  But it's diminishing returns at this point -- not clear
the attached patchset/diff is worth committing.  Gets me down to this:

   textdata bss dec hex filename
  3074432803376   374009218 main

After that, maybe we could find a substitute for the syslog_ss stuff
in stack-protector -- but that's probably not worth the trouble, for
what is maybe another few kilobytes or so.

Re: Trivial program size inflation

[Taylor R Campbell]

> Date: Mon, 3 Jul 2023 07:45:27 + (UTC)
> From: RVP 
> 
> On Mon, 3 Jul 2023, RVP wrote:
> 
> > Somebody should maybe add calloc() to bsdmalloc.
> 
> And posix_memalign() (any others?) too, else you end up with 2
> different arenas and free()/realloc() operating on the wrong one.

The attached patch adds calloc, posix_memalign, and C11 aligned_alloc
to libbsdmalloc.

However, if I try to link something that calls malloc or realloc with
-static -lbsdmalloc, ld objects:

$ cat null.c
#include 
int main(void) { return malloc(1) != NULL; }

$ make null LDLIBS=-static\ -lbsdmalloc\ -Wl,-Map=null.map
cc -O2-o null null.c -static -lbsdmalloc -Wl,-Map=null.map
ld: /usr/lib/libc.a(jemalloc.o): in function `malloc':
jemalloc.c:(.text+0x30a6): multiple definition of `malloc'; 
/usr/lib/libbsdmalloc.a(malloc.o):malloc.c:(.text+0x0): first defined here
ld: /usr/lib/libc.a(jemalloc.o): in function `realloc':
jemalloc.c:(.text+0x49c6): multiple definition of `realloc'; 
/usr/lib/libbsdmalloc.a(malloc.o):malloc.c:(.text+0x282): first defined here
ld: /usr/lib/libc.a(jemalloc.o): in function `free':
jemalloc.c:(.text+0x4ca0): multiple definition of `free'; 
/usr/lib/libbsdmalloc.a(malloc.o):malloc.c:(.text+0x22b): first defined here

calloc, free, posix_memalign, and aligned_alloc all work fine here.
Not sure why calling malloc or realloc still causes jemalloc.o to be
pulled in, in the presence of -lbsdmalloc.

Not investigating further; if someone wants to investigate and make
sure this actually works, feel free to commit this patch.
>From 4c1ca44f21e0d12ad2eac22dc8c3acbf56cfe23b Mon Sep 17 00:00:00 2001
From: Taylor R Campbell 
Date: Mon, 3 Jul 2023 12:58:34 +
Subject: [PATCH] libbsdmalloc: Provide more allocator frontends.

- aligned_alloc
- calloc
- posix_memalign

Otherwise these will pull in the jemalloc definitions from libc,
which defeats the purpose.

Note: libbsdmalloc doesn't set errno=ENOMEM on malloc failure, but
these front ends do (even aligned_alloc, which is from C11, which
doesn't define ENOMEM at all, but this pacifies our aligned_alloc
tests in t_posix_memalign.c).  Might want to fix that.
---
 lib/libbsdmalloc/Makefile | 12 ++-
 lib/libbsdmalloc/aligned_alloc.c  | 58 +++
 lib/libbsdmalloc/calloc.c | 52 +++
 lib/libbsdmalloc/posix_memalign.c | 56 +
 4 files changed, 177 insertions(+), 1 deletion(-)
 create mode 100644 lib/libbsdmalloc/aligned_alloc.c
 create mode 100644 lib/libbsdmalloc/calloc.c
 create mode 100644 lib/libbsdmalloc/posix_memalign.c

diff --git a/lib/libbsdmalloc/Makefile b/lib/libbsdmalloc/Makefile
index 03384670b2b9..b3e5d40fd12a 100644
--- a/lib/libbsdmalloc/Makefile
+++ b/lib/libbsdmalloc/Makefile
@@ -7,7 +7,17 @@ WARNS= 2
 .include 
 
 LIB=   bsdmalloc
-SRCS=  malloc.c
+SRCS+= aligned_alloc.c
+SRCS+= calloc.c
+SRCS+= malloc.c
+SRCS+= posix_memalign.c
+
+CFLAGS+=   -fno-builtin-aligned_alloc
+CFLAGS+=   -fno-builtin-calloc
+CFLAGS+=   -fno-builtin-free
+CFLAGS+=   -fno-builtin-malloc
+CFLAGS+=   -fno-builtin-posix_memalign
+CFLAGS+=   -fno-builtin-realloc
 
 CPPFLAGS+= -D_REENT -D_REENTRANT -I${.CURDIR}/../libc/include/
 
diff --git a/lib/libbsdmalloc/aligned_alloc.c b/lib/libbsdmalloc/aligned_alloc.c
new file mode 100644
index ..76c427557075
--- /dev/null
+++ b/lib/libbsdmalloc/aligned_alloc.c
@@ -0,0 +1,58 @@
+/* $NetBSD$*/
+
+/*-
+ * Copyright (c) 2023 The NetBSD Foundation, Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE NETBSD FOUNDATION, INC. AND CONTRIBUTORS
+ * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE FOUNDATION OR CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include 
+__RCSID("$NetBSD$");
+
+#include 
+#includ

Re: Trivial program size inflation

[Taylor R Campbell]

> Date: Sat, 1 Jul 2023 15:11:56 - (UTC)
> From: mlel...@serpens.de (Michael van Elst)
> 
> crt0 pulls in
> - atexit
> - environment
> - static TLS
> - stack guard
> 
> which all more or less pull in jemalloc, stdio and string functions.
> 
> You need to replace these with dummies (compile with -fno-common)
> and of course, your program must not make use of the functionality...

A quicker way to address most of it is to just define your own malloc:

$ cat null.o
#include 
void *malloc(size_t n) { return NULL; }
void *realloc(void *p, size_t n) { return NULL; }
void *calloc(size_t n, size_t sz) { return NULL; }
void free(void *p) {}
int main(void) { return 0; }
$ cc -g -O2 -static -o null null.c
$ size null
   textdata bss dec hex filename
  2672432083184   33116815c null

This still has printf, rbtree, string, atomic, , but not jemalloc,
giving a ~20x size reduction from half a megabyte to 25 KB or so.

If someone really wants to do the work to reduce the overhead without
providing an alternative malloc, or reduce more than you get with an
alternative malloc, here are some avenues that might be worth pursuing
without incurring too much overhead:

> int atexit(void) { return 0; };

The runtime startup logic, csu, relies on atexit.  But perhaps csu
could use an internal __atexit that reserves 4 or 5 static slots, and
the libc atexit uses the last one to call handlers in slots that are
dynamically allocated by malloc.

As long as your program doesn't call atexit, this only uses a fixed
amount of space from csu and won't bring in malloc.

> char *__allocenvvar() { return 0; };
> bool __canoverwriteenvvar() { return true; };
> size_t __envvarnamelen() { return 0; };
> void *__findenv() { return 0; };
> void *__findenvvar() { return 0; };
> void __freeenvvar() { };
> ssize_t __getenvslot() { return 0; };
> void __libc_env_init() { };
> bool __readlockenv() { return true; };
> bool __unlockenv() { return true; };
> bool __writelockenv() { return false; };

Programs that use only getenv don't need any of the machinery to
allocate environment slots.  The logic that getenv uses could be
isolated to its own .c file with no allocation.

This more or less requires splitting up __getenvslot into two separate
functions, one for the allocate=true case and the other for the
allocate=false case, with a .h file to mediate the global state
between the two .c files.

__libc_env_init (which is what pulls all this in even if you don't use
getenv, setenv, ) could perhaps be a weak symbol with a strong
alias in the .c file that does allocation and modification.

> void __libc_rtld_tls_allocate() { };
> void __libc_rtld_tls_free() { };
> void __libc_static_tls_setup() { };
> void __libc_tls_get_addr() { };

I'm stumped about this one.  In principle the linker has enough
information to decide whether __libc_static_tls_setup is needed (which
is what, in _libc_init, pulls all this in), but in practice I don't
know of any path that would let us conditionalize its use on whether
the object has any static TLS relocations.  Maybe rtld could be
responsible for mmapping the initial thread's static TLS space so libc
is not but I'm not sure if that will work without a lot of effort.

> void __chk_fail() { };
> void __guard_setup() { };
> void __stack_chk_fail() { };
> void __stack_chk_fail_local() { };
> int __stack_chk_guard;

This calls syslog_ss, which brings in xsyslog.c.  Not sure if that
brings in malloc or anything exciting beyond vsnprintf_ss (which
itself shouldn't malloc or be exciting, since it has to be
async-signal-safe).

But if it does, maybe the call in stack_protector.c __fail to
syslog_ss could be defined in terms of some weak symbol
__stack_chk_log which is defined by xsyslog.c using syslog machinery,
with a fallback to write to STDERR_FILENO; that way it only even tries
to use syslog if anything else in the program already uses syslog.


(But I'm not going to do this work, and I'm not sure if there's going
to be a good way to kick malloc out of the static TLS business without
toolchain and/or rtld support.)

Re: Trivial program size inflation

[Taylor R Campbell]

> Date: Sat, 1 Jul 2023 06:57:40 -0700
> From: Jason Thorpe 
> 
> Oh look, the entirety of jemalloc seems to be included in the
> binary.  WTF knows why that's happening, but apparently it is, and
> jemalloc pulls in a ton of additional stuff.

Comes in from at least two different things:

1. jemalloc.c defines a constructor which brings in most of jemalloc.

2. csu/common/crt0-common.c calls atexit, which brings in
   libc/stdlib/atexit.c, which calls malloc.

For a statically linked program that doesn't use malloc, these aren't
necessary: we don't need the jemalloc constructor, and the crt exit
handlers calls could be served by a static-only __atexit that atexit
also uses to wire up to dynamic exit handlers.

But someone would have to do the work to make this happen.

Not 100% sure this covers everything -- easy to test kicking out
atexit.c by defining __cxa_finalize, __cxa_atexit, __libc_atexit_init,
and atexit in your .c file; not sure offhand how to kick out the
jemalloc constructor without building a new libc.

Re: How to profile NSS code?

[Taylor R Campbell]

> Date: Mon, 5 Jun 2023 15:01:44 +
> From: Emmanuel Dreyfus 
> 
> I would like to understand why nss_ldap is slow. I rebuilt the module
> with -pg and made a simple program that calls getpwnam(), also built
> with -pg

Is it slow because it's eating CPU time (as shown by top(1)), or is it
slow because it's waiting for network I/O?  My guess is the latter.
Have you tried ktracing the program that calls getpwnam to see what
network entities it talks to, and how long it waits for them (with
kdump -T/-R to show timestamps)?

Re: style, sysexits(3), and man RETURN VALUES for sys programs

[Taylor R Campbell]

> Date: Sat, 3 Jun 2023 14:12:21 +0200
> From: tlaro...@polynum.com
> 
> Le Sat, Jun 03, 2023 at 12:02:20PM +0000, Taylor R Campbell a écrit :
> > > Date: Sat, 3 Jun 2023 13:45:44 +0200
> > > From: tlaro...@polynum.com
> > > 
> > > So I suggest to add a mention of sysexits(7) to style.
> > 
> > I don't think sysexits(7) is consistently used enough, or really
> > useful enough, to warrant being a part of the style guide.  Very few
> > programs, even those in src, use it, and I don't think anything
> > _relies_ on it for semantics in calling programs.
> 
> But I think it is a loss of information to put everything in
> EXIT_FAILURE. All in all, the majority of scripts will simply test
> against 0, so being more fine grained (there are only 15 exit values at
> the moment) doesn't cause problems and, IMHO, adds some value that can
> be useful.

It's not really a loss of information: usually the error message
printed to stderr is much more informative.

The question is whether it's useful for composition, so that calling
programs can make meaningful decisions to take useful action on the
basis of the called program's exit code -- like the convention of zero
for success, nonzero for failure, which is absolutely useful for
composition.

Unless you're devising a scheme to do that with sysexits(3), and
implementing it systematically so that other programs derive some
benefit from it, spending time to make inetd(8) scrupulously adhere to
the sysexits(3) ontology of failure modes is likely a distraction from
your main goals.

Re: style, sysexits(3), and man RETURN VALUES for sys programs

[Taylor R Campbell]

> Date: Sat, 3 Jun 2023 13:45:44 +0200
> From: tlaro...@polynum.com
> 
> So I suggest to add a mention of sysexits(7) to style.

I don't think sysexits(7) is consistently used enough, or really
useful enough, to warrant being a part of the style guide.  Very few
programs, even those in src, use it, and I don't think anything
_relies_ on it for semantics in calling programs.

> But I'd like also to request some additions to sysexits(3):
> [...]

Sounds like overthinking this, unless you see specific semantic value
for composing programs that goes beyond the standard convention of 0
for success and nonzero for failure.

There are extremely rare cases of making finer distinctions than that.
For example, cmp(1) returns 0 for identical, 1 for difference, >1 for
error.

> Furthermore, I'm adding a RETURN VALUES section to inetd.8 and I think
> it should be standard practice for sys programs.

Normally this would go under EXIT STATUS, not RETURN VALUES.

> BTW, and still concerning style, is there a defined way of generating a
> MAN page needing to edit some part of the manual (ex.: usage) depending
> on some macros defined or not (in the case of inetd.8---even if this is
> not an actual problem because LIBWRAP is always defined---the [-l] flag
> depends on the macro; but it is always present in the usage).

I'd just document it unconditionally, and if it's really important,
mention in the text that it depends on a compile-time option.

Re: inetd(8): continue or exit on error?

[Taylor R Campbell]

> Date: Mon, 29 May 2023 10:11:09 +0200
> From: tlaro...@polynum.com
> 
> The question is what to do in case of a config file not found (this is
> the initial problem: the realpath() return status is not tested and a
> structure is inconditionnally added to a linked list with an unreachable
> config file).

I'm not sure inetd(8) has any business calling realpath in the first
place.  Can we just remove it and let it continue to use paths exactly
as written in the config file?

Re: inetd(8): continue or exit on error?

[Taylor R Campbell]

> Date: Mon, 29 May 2023 13:13:33 +0200
> From: tlaro...@polynum.com
> 
> I'm for: exit on any error. If we provide a way to check, that's the
> responsability of the administrator to check his config before trying to
> run the thing.

Yes please.  Should provide an option to check a configuration file
without starting inetd(8), make `service inetd check' do this, and
make `service inetd reload' (maybe also `service inetd restart') do
check first.  This should be standard practice for all daemons.

Re: pulling in changes to nsswitch from FreeBSD?

[Taylor R Campbell]

> Date: Thu, 18 May 2023 07:53:55 +0930
> From: Brett Lymn 
> 
> For "reasons" I have been looking to build nss-pam-ldapd and found that
> there are there are some changes that have been made by FreeBSD to nss
> that make it easier to port linux based applications that use nsswitch.
> 
> The first is fairly simple, import nss.h which provides a compat layer.
> It is a small and simple header so it shouldn't be contraversial.
> 
> The more complicated (slightly, I think) would be to import the updated
> nsswitch.conf which had some renamed #defines and a couple of extra
> #defines.  Of course that would have a flow on effect to the underlying
> nss code.

Can you be more specific about the changes needed?

Do they have any compatibility implications?

Re: style(5) proposal: forbid extern in .c

[Taylor R Campbell]

> Date: Thu, 16 Mar 2023 01:58:48 +0900
> From: Izumi Tsutsui 
> 
> > Proposal: Forbid extern declarations in .c files.
>  :
> > Pretty simple.  Any objections?
> 
> No objection, but I wonder how we can resolve this case:
>  
> https://nxr.netbsd.org/xref/src/sys/arch/hp300/stand/common/if_le.c?r=1.14#101
> 
> ---
> extern struct netif_stats le_stats[];
> 
> static struct netif_dif le_ifs[] = {
> /*dif_unitdif_nseldif_stats   dif_private */
> { 0,  NLE0CONF,   _stats[0],   le0conf,},
> };
> #define NLE_IFS (sizeof(le_ifs) / sizeof(le_ifs[0]))
> 
> struct netif_stats le_stats[NLE_IFS];

Two options:

1. If le_stats is used outside this file, move the extern declaration
   to a common .h file used by if_le.c and all users of le_stats.

2. If le_stats is used only inside this file, replace `extern' by
   `static' and add `static' to the definition.  No need for extern
   here.  The forward declaration inside the .c file would still be
   allowed under this change.

style(5) proposal: forbid extern in .c

[Taylor R Campbell]

Proposal: Forbid extern declarations in .c files.

extern declarations in .c files invite easily avoided bugs where the
definition and use have mismatched types, because the compiler doesn't
have an opportunity to check them.  Fix: Always put the extern
declaration in a .h file shared by the .c file defining it and the .c
files using it.

Pretty simple.  Any objections?
>From f3e8932d02d1c80582de05cc0869aa3693ec339c Mon Sep 17 00:00:00 2001
From: Taylor R Campbell 
Date: Wed, 15 Mar 2023 10:47:47 +
Subject: [PATCH] style(5): Forbid extern in .c files.

---
 share/misc/style | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/share/misc/style b/share/misc/style
index 060f01f9eaf7..765d1eced109 100644
--- a/share/misc/style
+++ b/share/misc/style
@@ -345,9 +345,11 @@ function(int a1, int a2, float fl, int a4)
 * declarations next to their first use, and initialize
 * opportunistically. This avoids over-initialization and
 * accidental bugs caused by declaration reordering.
+*
+* Never declare extern variables in .c files.  Declare them in the
+* appropriate .h file shared by the .c file where they are defined
+* and the .c file where they are used.
 */
-   extern u_char one;
-   extern char two;
struct foo three, *four;
double five;
int *six, seven;

Re: SHA3 implementation problem

[Taylor R Campbell]

tl;dr: I think we're in the clear with no further effort in src.


> Date: Tue, 7 Mar 2023 09:55:33 +0100
> From: Thomas Klausner 
> 
> "This paper describes a vulnerability in several implementations of
> the Secure Hash Algorithm 3 (SHA-3) that have been released by its
> designers. [...]"

https://github.com/XKCP/XKCP/issues/105
https://github.com/XKCP/XKCP/commit/fdc6fef075f4e81d6b1bc38364248975e08e340a

The flaw arises from a 32-bit integer overflow in a bounds test:

 /* normal lane: using the message queue */
 partialBlock = (unsigned int)(dataByteLen - i);
 if (partialBlock+instance->byteIOIndex > rateInBytes)
 partialBlock = rateInBytes-instance->byteIOIndex;
 i += partialBlock;

(Certainly it's not surprising that it's the variable-input-processing
part that was exploitable; that's the most annoying part to get
right.)

> I looked for SHA 3 and keccak and found at least the following hits in
> our tree:
> 
> common/lib/libc/hash/sha3/sha3.c

I wrote this independently.  The variable-input-processing logic I
used is different and unlikely to be affected by this mistake.
Verified that it prints the correct SHA3-224 hash for 4294967296
zeroes split into a one-byte chunk and a 4294967295-byte chunk:

c5bcc3bc73b5ef45e91d2d7c70b64f196fac08eee4e4acf6e6571ebe

I confirmed this with a version of Python released since they switched
from KCP to tiny_sha3, using the fragment in the paper to reproduce a
crash for older vulnerable Python versions:

   >>> import hashlib
   >>> h = hashlib.sha3_224()
   >>> h.update(b"\x00" * 1)
   >>> h.update(b"\x00" * 4294967295)
   >>> print(h.hexdigest())
   c5bcc3bc73b5ef45e91d2d7c70b64f196fac08eee4e4acf6e6571ebe

> crypto/external/bsd/openssl/dist/crypto/evp/m_sha3.c
> crypto/external/bsd/openssl/dist/crypto/sha/asm/
> crypto/external/bsd/openssl/dist/crypto/sha/keccak1600.c
> crypto/external/bsd/openssl/dist/crypto/evp/m_sha3.c
> crypto/external/bsd/openssl.old/...

These were also written independently.  The paper notes that the
OpenSSL implementation is not vulnerable to this attack.  I did not
independently verify that it produces the correct hash for an input
split that way, mainly because reminding myself how to use the EVP API
is a pain in the arse.

> external/public-domain/sqlite/dist/shell.c

This appears to have been written independently, and kept considerably
simpler, likely at some cost in performance.  It is also impossible
for this particular bug to happen because the SHA3Update function is
limited to sizes representable in unsigned int.

Ordering of sh(1) pathname expansion

[Taylor R Campbell]

In sh(1) pathname expansion, the pattern doesn't constrain ordering,
only inclusion criteria.  It appears that NetBSD's sh(1) always sorts
lexicographically with strcmp(3), however.

1. Does POSIX sh guarantee strcmp(3) lexicographic ordering?

2. Does NetBSD sh(1) guarantee strcmp(3) lexicographic ordering?

3. Should the sh(1) man page be amended to specify the order?

Re: strftime(3) oddities with %s, %z

[Taylor R Campbell]

> Date: Sun, 06 Nov 2022 17:22:22 +0700
> From: Robert Elz 
> 
> One thing to beware of though, is that there is not enough info in a
> struct tm to determine which timezone created it.   It simply cannot
> (in general) be done.   Changing struct tm (however appealing that
> might be - and believe me, I would really like to) is not practical.
> For a specialised purpose, a new struct, containing a struct tm, and
> also whatever extra data is required, could be designed & used of course.

I don't think that's needed for what uwe@ and dholland@ were looking
for, which -- if I understand correctly -- is that:

struct tm gtm, ltm;
char gbuf[128], lbuf[128];

gmtime_r(, );
localtime_r(, );
strftime_XXX(gbuf, sizeof(gbuf), "... %s ...", );
strftime_XXX(lbuf, sizeof(lbuf), "... %s ...", );

should format the same number at %s in the output (i.e., the value of
t), but otherwise format the broken-down time (%Y/m/d/H/M/S/...)
according to UTC for gtm and the local time for ltm, i.e. just format
the .tm_* members -- and %z should format .tm_gmtoff, which is +
for gtm but the local offset for ltm.

After all, these struct tm objects are supposed to represent the same
moment in time (t), just phrased in different civil time zones.  So %s
should be the same for both, and %z should identify the UTC offset
that enables all the other broken-down time components to be
interpreted correctly to give the same moment in time.

I believe there is already enough information stored in struct tm to
do this for everything except perhaps %Z (but %Z doesn't seem to be
very well-specified anyway).  It's not clear to me if you can get this
semantics out of strftime_z but I suspect not.

Re: strftime(3) oddities with %s, %z

[Taylor R Campbell]

> Date: Sat, 05 Nov 2022 20:49:57 +0700
> From: Robert Elz 
> 
> That's a variation of strftime() where you can tell it which time zone
> you want to use, instead of local time, for the conversions, so if you
> were to do
> 
>   z = tzalloc("UTC");
>   t = gmtime(_time_t_variable);
>   strftime_z(z, buf, sizeof buf, "whatever .. including %s %z and %Z", t);
>   
> then you'd get %s/%z/%Z values as specified by the UTC timezone, instead
> of current local time.   (Don't forget to tzfree(z) somewhere - or just 
> exit()).
> 
> That is, we don't need a new API, we already have the new API, just none
> of the participants here seem to have bothered to notice it.

I'm not sure this is adequately specified either.  The man page just
says:

 The strftime_z() function is similar to strftime(), but it also takes a
 const timezone_t tz argument.

This could mean several things:

1. strftime_z interprets the struct tm relative to tz, and formats it
   in tz.

2. strftime_z interprets the struct tm in its internal time zone
   recorded by gmtime/localtime, converts it to tz, and formats it in
   tz.

3. strftime_z interprets the struct tm in $TZ, converts it to tz, and
   formats it in tz.

4. strftime_z interprets the struct tm in tz, converts it to $TZ, and
   formats it in $TZ.

I expect it's one of these (probably (1)) but it's not obvious which
one and it's also not obvious that the one it implements is most
sensible or meets the expectations of uwe@ and dholland@ that spawned
this thread (probably (2)).

And, there's another option which I think is what uwe@ and dholland@
wanted for strftime which is inconvenient to get with any of these
options for strftime_z:

5. strftime_XXX interprets the struct tm in its internal time zone
   recorded by gmtime/localtime, converts it to $TZ, and formats it in
   $TZ.

Re: [PATCH] regex.c: fix assertion

[Taylor R Campbell]

> Date: Fri, 4 Nov 2022 14:53:42 -0700
> From: enh 
> 
> errbuf is only required to be non-NULL if errbuf_size != 0.

Thanks, applied!

Re: strftime(3) oddities with %s, %z

[Taylor R Campbell]

> Date: Wed, 2 Nov 2022 15:59:00 +0300
> From: Valery Ushakov 
> 
> In other words, class tm doesn't have a public constructor that
> provides a way to specify TZ info.  There are other factory methods
> that allow one to obtain an instance of tm that has the TZ info (in
> its private parts). ...

Suppose you create a struct tm _without_ gmtime(3) or localtime(3),
using designated initializers or memset for zero-initialization, with
only what is included in POSIX:

struct tm tm = {
.tm_sec = 56,
.tm_min = 34,
.tm_hour = 12,
.tm_mday = 1,
.tm_mon = 12 - 1,   /* December */
.tm_year = 2021 - 1900,
.tm_wday = 3,   /* Wednesday */
.tm_yday = 334, /* zero-based day of year (%j - 1) */
.tm_isdst = 0,
};

Nothing I've found in POSIX suggests you can't construct a struct tm
like this and use it with mktime, and the EXAMPLES section of

certainly suggests you can -- indeed, tm_wday and tm_yday could even
be omitted.  (If you think otherwise: Why do you think you can't
construct a struct tm like this?)

This struct tm doesn't specify a time zone in which to interpret the
calendar date.  So what time_t do you get out of mktime(), or what
number is represented by the string you get out of strftime(..., "%s",
)?

First, in any particular context, I hope these should be the same!
(If you think otherwise: Why should they be different?)

If TZ=UTC, I think we can all agree that the answer should be
1638362096.  (If you think otherwise: What should the answer be?)

Now what if TZ is not UTC, say TZ=Europe/Berlin?  It obviously depends
on whether mktime and strftime examine TZ or tm_gmtoff.  Here are some
possible rules to answer this:

1. mktime and strftime ignore tm_gmtoff and respect TZ, as if
   tm_gmtoff did not exist.

   In that case, we should get 1638358496, which is 1638362096 - 3600
   because Europe/Berlin is +0100 at that calendar date, 1h ahead of
   UTC.

   This is the semantics that portable applications currently rely on,
   so whatever the rule is had better agree with this!

2. mktime and strftime respect tm_gmtoff and ignore TZ.

   In that case, we should get 1638362096 because tm_gmtoff=0 in this
   code.  But suddenly this is different from what portable
   applications can rely on, so this can't be the right rule.

3. mktime and strftime respect tm_gmtoff if it is nonzero, meaning it
   has been initialized by something not currently portable in POSIX,
   and use TZ if tm_gmtoff=0.

   In that case, we should get 1638358496, because tm_gmtoff=0 in this
   code, so it interprets the struct tm in TZ=Europe/Berlin.

   However, this has a funny side effect.  Suppose we get struct tm
   values from the following pseudocode:

TZ=Atlantic/Reykjavik localtime_r(1638362096, _is);
TZ=Europe/Rome localtime_r(1638362096, _it);
TZ=Israel localtime_r(1638362096, _il);
TZ=Asia/Baghdad localtime_r(1638362096, _iq);

   Since these have filled in the time zones, it shouldn't matter what
   TZ is set to when we feed tm_is/it/il/iq into mktime or
   strftime("%s"), right?

   Unfortunately, it _does_ matter.  If we have, say, TZ=Europe/Rome,
   then under this rule we would get:

tm_is: 1638358496
tm_it: 1638362096
tm_il: 1638362096
tm_iq: 1638362096

   That's because with TZ=Atlantic/Reykjavik, tm_gmtoff=0.  (Same with
   some others like TZ=Europe/London, at least during the winter.)

   So although this rule preserves the semantics of portably
   constructed struct tm, it has wacky semantics for struct tm
   constructed with a tm_gmtoff-aware localtime(3) -- I think we can
   all agree this is obviously wrong.

4. mktime and strftime respect tm_gmtoff if tm_zone is nonnull, and
   use TZ if tm_zone is null.

   First, I'm not sure if tm_zone is always initialized to something
   nonnull by localtime and gmtime -- it's unclear to me what naming
   standard it follows, and I wouldn't be surprised if that included
   sometimes leaving it as null.  But let's suppose it is always
   initialized to nonnull.

   In that case, we should get 1638358496, because tm_gmtoff=0 in this
   code, so it interprets the struct tm in TZ=Europe/Berlin.

   But this avoids conflating zero-initialized tm_gmtoff with a
   baked-in time zone of UTC, so with the various localtime calls in
   case (3) we would always get 1638362096 out of mktime.

   I think this might be closer to what uwe@ and dholland@ want: if
   you didn't specify a time zone, then mktime uses TZ, but you can
   specify a time zone or let localtime record what TZ was and it will
   be passed on to mktime no matter what TZ is later.

   However, this still changes semantics that portable applications
   can currently rely on _even if they don't construct their own
   struct tm objects_.

   For example, POSIX currently guarantees that the 

Re: unhide reallocarray

[Taylor R Campbell]

> Date: Wed, 31 Aug 2022 17:36:56 +0700
> From: Robert Elz 
> 
> This might also be an appropriate time to fix the man page for
> reallocarr(3) so it actually says something more than giving one
> (mediocre) example.

Better now?

Re: Permissions of the root dot files

[Taylor R Campbell]

> Date: Tue, 30 Aug 2022 09:18:47 -0400 (EDT)
> From: Mouse 
> 
> >> Is there any particular reason why /root/.profile and /root/.cshrc
> >> (that have hard links in / too, for the single user mode i guess)
> >> are not writable?
> > Aside from applications like vi rm mv etc (probably more) which
> > require a slight bit more effort if the file has no write permission,
> > what difference does the user 'w' (or 'r' ... 'x' does matter)
> > permission bit really make on a root owned file?
> 
> Absent some reason to _not_ turn on their 0200 bits, I would say that
> "slight bit more effort" is reason enough.  I've been annoyed often
> enough by those slight bits more effort myself.

This appears to have been an oversight in writing the Makefiles that
create the files (and, for /.cshrc and /.profile, create the links).
I've fixed it for these and several other cases, so you can edit them
without editor complaints.  mtree will also no longer object to the
modes out of the box.

Re: Permissions of the root dot files

[Taylor R Campbell]

> Date: Tue, 30 Aug 2022 08:38:02 +0700
> From: Robert Elz 
> 
> Date:Tue, 30 Aug 2022 02:27:33 +0300
> From:Valery Ushakov 
> Message-ID:  
> 
>   | Is there any particular reason why /root/.profile and /root/.cshrc
>   | (that have hard links in / too, for the single user mode i guess) are
>   | not writable?
> 
> Aside from applications like vi rm mv etc (probably more) which require
> a slight bit more effort if the file has no write permission, what
> difference does the user 'w' (or 'r' ... 'x' does matter) permission
> bit really make on a root owned file?
> 
> I assume you aren't talking about group/other 'w' permission,
> which might be fine for you to grant on your own system, but
> certainly not to ship with NetBSD.

The default umask is 022.  The files are meant to be sourced, not
executed, so it makes sense to clear the x bits.  Other than that,
clearing any other bits needs justification.

These files are (privileged) user-editable configuration files from
the etc set which is always presumed to need local changes merged on
update.  In contrast, say, /bin/sh is from base and potentially
subject to automatic update without the presumption of need for
merging local changes.

cgdconfig(8) support for sharing a main key between disks

[Taylor R Campbell]

[bcc tech-crypto@ tech-security@, followups to tech-userlevel@]

The attached patch series implements support for sharing a single main
key between multiple disks.

This way, for instance, you can enter a password once, and the system
will compute Argon2id or talk to an interactive hardware token only
once to derive a main key (expensive), and then use HKDF-HMAC-SHA256
to derive subkeys for each of the disks configured to use the shared
key (cheap).

The disks won't be encrypted with the _same_ key -- their keys are
just derived by HKDF-HMAC-SHA256 from a main key shared between them.
To anyone who doesn't know the main key (and can't break SHA-256), the
disk keys appear to be independent uniform random keys.

Example setup:

cgdconfig -g -S -o /etc/cgd/wd0 -V gpt adiantum
cgdconfig -g -S -P /etc/cgd/wd0 -o /etc/cgd/ld1 \
-V disklabel aes-cbc 256

Then, with wd0 and ld1 listed in /etc/cgd/cgd.conf, `cgdconfig -C'
will prompt for only one password, but will configure both disks.

The generated wd0 and ld1 parameters files might look like this:

# wd0
algorithm adiantum;
iv-method encblkno1;
keylength 256;
verify_method gpt;
keygen pkcs5_pbkdf2/sha1 {
iterations 463571;
salt gBnGwXjRa38XSv+PN/G0Urs=;
shared "bdec4067-a77d-4fd6-9f1b-71b8b82a8f8e" \
algorithm hkdf-hmac-sha256 \
subkey gG9MMEA7Wkl2E/vKtmPXshE=;
};

# ld1
algorithm aes-cbc;
iv-method encblkno1;
keylength 256;
verify_method disklabel;
keygen pkcs5_pbkdf2/sha1 {
iterations 463571;
salt gBnGwXjRa38XSv+PN/G0Urs=;
shared "bdec4067-a77d-4fd6-9f1b-71b8b82a8f8e" \
algorithm hkdf-hmac-sha256 \
subkey gJFTFfBFBcEuf/jYGPF3s+k=;
};

Note that the keygen method, number of iterations, salt, and shared
name and algorithm are all the same -- inside the keygen block, only
the subkey identifier is different.

The shared key name, here bdec4067-a77d-4fd6-9f1b-71b8b82a8f8e, is
used by `cgdconfig -C' as a key for a cache of key generations to
reuse between parameters files; it has no other significance.  Each
parameters file can also be used on its own without the others.
Nothing is stored persistently, and there are no background daemons
like ssh-agent: once `cgdconfig -C' exits the cache is discarded.

Thoughts?  Review and testing welcome!


P.S.  Currently this doesn't provide a way to convert an existing
parameters file to be shareable, but in principle `cgdconfig -G' could
be taught to do that by generating a shared clause in the existing
keygen block and then adding an appropriate `keygen storedkey' block
as it already does to make it work -- TBD.
>From e21179d60d2e884fc9b691bfa4aa13ac30696d75 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell 
Date: Sun, 26 Jun 2022 15:32:15 +
Subject: [PATCH 1/5] cgdconfig(8): New -t operation just prints the derived
 key in base64.

For testing purposes.
---
 distrib/sets/lists/tests/mi  |  1 +
 sbin/cgdconfig/cgdconfig.8   |  5 ++
 sbin/cgdconfig/cgdconfig.c   | 57 --
 tests/dev/cgd/Makefile   |  3 +-
 tests/dev/cgd/t_cgdconfig.sh | 94 
 5 files changed, 156 insertions(+), 4 deletions(-)
 create mode 100644 tests/dev/cgd/t_cgdconfig.sh

diff --git a/distrib/sets/lists/tests/mi b/distrib/sets/lists/tests/mi
index a8f2705bbea7..6dc6afb2d236 100644
--- a/distrib/sets/lists/tests/mi
+++ b/distrib/sets/lists/tests/mi
@@ -1426,6 +1426,7 @@
 ./usr/tests/dev/cgd/t_cgd_adiantum tests-fs-tests  
atf,compattestfile,rump
 ./usr/tests/dev/cgd/t_cgd_aes  tests-fs-tests  
atf,compattestfile,rump
 ./usr/tests/dev/cgd/t_cgd_blowfish tests-fs-tests  
atf,compattestfile,rump
+./usr/tests/dev/cgd/t_cgdconfigtests-fs-tests  
compattestfile,atf
 ./usr/tests/dev/clock_subr tests-fs-tests  
compattestfile,atf
 ./usr/tests/dev/clock_subr/Atffile tests-fs-tests  
compattestfile,atf
 ./usr/tests/dev/clock_subr/Kyuafiletests-fs-tests  
compattestfile,atf,kyua
diff --git a/sbin/cgdconfig/cgdconfig.8 b/sbin/cgdconfig/cgdconfig.8
index 8e6652349e94..7cd662bb5ce6 100644
--- a/sbin/cgdconfig/cgdconfig.8
+++ b/sbin/cgdconfig/cgdconfig.8
@@ -60,6 +60,9 @@
 .Ar alg
 .Op Ar keylen
 .Nm
+.Fl t
+.Ar paramsfile
+.Nm
 .Fl l
 .Op Fl v Ns Op Cm v
 .Op Ar cgd
@@ -143,6 +146,8 @@ in question to be unconfigured rather than prompting for 
the passphrase
 again.
 .It Fl s
 Read the key (nb: not the passphrase) from stdin.
+.It Fl t
+Generate the key and print it to standard output encoded in base64.
 .It Fl U
 Unconfigure all the de

Re: fidocrypt(1): `storing' cgd keys on U2F/FIDO keys

[Taylor R Campbell]

> Date: Sat, 6 Aug 2022 18:47:47 -0400
> From: Gabriel Rosenkoetter 
> 
> I mostly use macOS at home and Windows at work in that "desktop" context 
> these days, so I threw up my hands a few years ago and wrote my own 
> credential manager whose datastore is a USB mass storage device attached 
> to my keychain, and is largely a UI wrapper around a PGP-encrypted file 
> in a specific format. (I prefer USB devices that either have hardware 
> encryption or are formatted with an encrypted file system, but whatever.)

To be clear, fidocrypt(1) is not a general-purpose credential manager.
A fidocrypt file on disk stores a _single_ secret, which can be opened
by any one of the U2F/FIDO devices registered with the file.

That secret might be used to unlock a general-purpose credential
manager, or a cgd(4) volume (e.g., your USB mass storage device), so
that when they're locked you need the U2F/FIDO device (or some other
backup of the secret) to unlock them.

> But that's a bunch of home-rolled garbage, whereas I think what you're 
> proposing to add to base is an interface to a standards-compliant (and 
> somewhat-open) device specification. Right?

The _file format_ used by fidocrypt(1) is bespoke, based on sqlite3.

The _protocol_ fidocrypt(1) uses to talk to U2F/FIDO devices is
standard, and is the same as you can use with web browsers in webauthn
for securing things like Twitter and Gmail accounts against phishing.
The same U2F/FIDO device can be safely reused for both purposes.

> I guess my follow-up Devil's advocacy question would be: why does this 
> need to be in base, rather than provided via ports?

cgdconfig runs early at boot before most file systems are mounted --
often before the file systems on which any packages are installed are
mounted.  The cgdroot ramdisk, for instance, has a very minimal NetBSD
userland in a ramdisk just to configure cgd(4) volumes before mounting
the `real' root from them and chrooting into it.  fidocrypt could be
baked into this ramdisk.

fidocrypt(1): `storing' cgd keys on U2F/FIDO keys

[Taylor R Campbell]

[bcc tech-crypto@ tech-security@, followups to tech-userlevel@]

I would like to import the fidocrypt(1) utility into base:

https://github.com/riastradh/fidocrypt/

fidocrypt(1) is a small program that lets you `store' a secret on
U2F/FIDO keys, with a little state on disk that enables you to
register or unregister keys without changing the secret, so that any
one of the registered keys can be used to open the secret.  Example:

$ export FIDOCRYPT_RPID=fidocrypt.example.com
$ fidocrypt enroll -N Falken -u falken -n yubi5nano example.crypt
tap key to enroll; waiting...
tap key again to verify; waiting...
$ fidocrypt enroll -N Falken -u falken -n redsolokey example.crypt
tap a key that's already enrolled; waiting...
tap key to enroll; waiting...
tap key again to verify; waiting...
$ fidocrypt list example.crypt
2 redsolokey
1 yubi5nano
$ fidocrypt get -F base64 example.crypt
tap key; waiting...
yTpyXp1Hk3F48Wx3Mp7B2gNOChPyPW0VOH3C7l5AM9A=

The secret might be, for instance, a cgd(4) disk encryption key --
fidocrypt(1) can be used in a shell_cmd keygen block of a cgdconfig(8)
parameters file.  For this to work, fidocrypt(1) has to be available
early at boot time before any file systems on cgd(4) volumes have been
mounted -- this is why I suggest putting it in base and not just, say,
in pkgsrc.

The fidocrypt(1) program and associated libfidocrypt.so library are
reasonably small, a couple hundred kilobytes on amd64, although having
/bin/fidocrypt would require us to move libfido2.so and libsqlite3.so
into /lib, adding a megabyte or two in total to / (not counting
/rescue where it might also be worthwhile to add fidocrypt):

   textdata bss dec hex filename
  5216020421864   56066db02 fidocrypt
  693321544   0   70876   114dc libfidocrypt.so
 1444802048  32  146560   23c80 libfido2.so
1136213   225041368 1160085  11b395 libsqlite3.so

(Of course, sqlite3 in /lib might be useful for other purposes too!)

Thoughts?


(The `-N Falken -u falken' names, and the FIDOCRYPT_RPID relying party
id, are required by the FIDO protocol, but have no significance to
fidocrypt(1) itself -- other than that the relying party id can't be
changed without creating a new fidocrypt file to encapsulate the same
secret.  The `-n yubi5nano' just gives a nickname to each registered
key in case you want to revoke it later.)

Re: /rescue/tar needing liblzma.so.2

[Taylor R Campbell]

Seems to me if /rescue/tar is going to run gzip as a subprocess, it
should have the full path /rescue/gzip (or gzcat or whatever) baked
in so its behaviour doesn't depend on PATH.

Re: sysupgrade in src?

[Taylor R Campbell]

> Date: Fri, 15 Apr 2022 15:29:49 +
> From: nia 
> 
> -a etcupdate can automatically update files which have not
>been modified locally.  The -a flag instructs etcupdate to
>store MD5 checksums in /var/etcupdate and use these
>checksums to determine if there have been any local
>modifications.
> 
> -l Automatically skip files with unchanged RCS IDs.  This has
>the effect of leaving alone files that have been altered
>locally but which have not been changed in the reference
>files.  Since this works using RCS IDs, files without RCS
>IDs will not be skipped even if only modified locally.
>This flag may be used together with the -a flag described
>above.
> 
> Downside: this doesn't work for modified files without RCS IDs, which
> of course means that etcupdate prompts about /etc/passwd (etc.)
> every single time.

I have never found these options to be useful after struggling to get
value out of etcupdate.  The basic problem is that it's not doing a
three-way merge, and it should.  I drafted a utility to do this last
year with a couple different usage models for experimentation:

https://www.netbsd.org/~riastradh/tmp/20210108/etcmerge.1.html

It needs to be integrated into a system upgrade process that
automatically stores away the current etc.tgz so it can do the
three-way merge later.  I started working on that last year, so it
could be integrated into how releng builds are done to make manifests
for upgrade channels and incremental update tar sets for patch
releases, but got sidetracked with other things like drm.

Re: ZFS - mounting filesystems (Was CVS commit: src/etc)

[Taylor R Campbell]

> Date: Thu, 17 Mar 2022 08:32:40 -0400
> From: Greg Troxel 
> 
> Simon Burge  writes:
> 
> >  5. Move all local mounts to /etc/rc.d/mountcritlocal (ala
> > FreeBSD) and possibly rename this to /etc/rc.d/mountlocal .
> 
> I think the only thing we lose with this is the ability to mount local
> filesystems on top of mount points that are in remote filesystems,
> without playing the noauto/rc.local game.  I am ok with this personally,
> but it feels hard to be sure it won't cause trouble, and I do expect
> some trouble.

Does anyone actually do this -- have local mounts on top of remote
mounts?

I keep hearing about the theoretical possibility of /usr on nfs and
/usr/src or /usr/local on local ffs.  But you'd really have to go out
of your way to set that up -- certainly sysinst won't do that for you.
Not sure it's too much to ask that if you do set something up like
that you use noauto/rc.local or a local rc.d script to manage it.

Is this obscure use case actually worth worrying about enough to add
new knobs, invent new NetBSD-specific zfs properties, and/or keep a
confusing series of mount stages in rc.d?

I think it would be better to just nix the `critical' distinction:
mount root, then mount all local, then start networking and mount all
remote.  Keep it simple unless there's a strong reason not to.

Re: ZFS - mounting filesystems (Was CVS commit: src/etc)

[Taylor R Campbell]

> Date: Mon, 14 Mar 2022 08:01:53 -0700 (PDT)
> From: Paul Goyette 
> 
> On Tue, 15 Mar 2022, Simon Burge wrote:
> 
> > Do we have any valid need to have non-critical local filesystems?
> 
> Well, I have a dedicated filesystem for builds, separate from my
> OS.  The /build happens to be my nvme SSD.
> 
> Building (or being able to build) is not critical to having the
> machine running (and receiving mail).
> 
> So, yeah, I think non-critical local filesystems are meaningful.

But critical vs non-critical is just a question of ordering at boot.
Do you have rc.d scripts that have to run some time between
mountcritremote and mountall, for which it matters to defer mounting
some file systems until mountall?  If yes, why?

(Separately, there's `noauto' in fstab if you don't want the file
system to be mounted automatically at boot.)

Re: crypt_r()?

[Taylor R Campbell]

Discussion about a reasonable API is ongoing, but I'll give some
review comments anyway -- including some of my thoughts on why the
crypt_r API is not ideal -- in case they're helpful.


   --- include/unistd.h 15 Oct 2021 22:32:28 -  1.162
   +++ include/unistd.h 12 Feb 2022 12:47:04 -
   @@ -325,8 +325,15 @@
 * Implementation-defined extensions
 */
#if defined(_NETBSD_SOURCE)
   +struct crypt_data {
   +int initialized; /* glibc compat */
   +char __buf[512]; /* buffer returned by crypt_r */
   +};

I'd avoid putting this in unistd.h -- maybe a public  header
file instead if it's to match the API documented in
.

Using _NETBSD_SOURCE _or_ _GNU_SOURCE, for APIs compatible with glibc,
has precedent -- e.g., for feenableexcept/fedisableexcept/fegetexcept.

Where does 512 come from?  This should be documented in a comment.

If the result is to be statically allocated -- and I'm not opining on
that, just taking it as a premise -- it needs to be large enough for
all of the password hashes in tree.  512 bytes is almost certainly
enough, but this magic number should be documented somewhere, and
verified with __CTASSERT for all of the password hashes involved.

Quick back-of-the-envelope estimate: with a 32-byte salt and a 32-byte
hash (which is the largest that there is any cryptographic motivation
for, even in the face of quantum computers), base64-encoded into up to
43 bytes each, that leaves 426 bytes for parameters and other overhead
like the `$argon2' tag, which seems like a lot more than the caller
has to reserve stack space for.

So maybe, say, 188 bytes instead of 512 bytes would be enough (still
leaves over 100 bytes for overhead); then the whole struct fits in 192
bytes or three cache lines on many CPUs.  I'm not saying 188 is the
right choice, just giving an example with reasoning -- maybe 512 is
better but it needs a rationale.  Obviously we want to overestimate
this to future-proof the ABI, but this also shouldn't be unreasonably
burdensome on the caller's stack space which is often much more
limited than heap space.

And, of course, this is a reason to prefer heap allocation, even if
that means slightly more API complexity (caller must free the result):
we just don't need to think about burden on the caller's stack usage.

On the other hand, there is some value to just being able to drop this
API in and have existing code work.  Certainly the glibc approach of
putting >128 KB in struct crypt_data is, uhhh, not great!  No modern
justification for doing that -- either it's too little for modern
password hashes, or it's a waste of stack space because they'll
allocate storage separately on the heap anyway.

   --- lib/libcrypt/bcrypt.c   16 Oct 2021 10:53:33 -  1.22
   +++ lib/libcrypt/bcrypt.c   12 Feb 2022 12:47:04 -
   @@ -74,9 +74,9 @@
static void encode_base64(u_int8_t *, u_int8_t *, u_int16_t);
static void decode_base64(u_int8_t *, u_int16_t, const u_int8_t *);

   -crypt_private char *__bcrypt(const char *, const char *);  /* XXX */
   +/* crypt_private char *__bcrypt(const char *, const char *);*/ /* XXX */

   -static charencrypted[_PASSWORD_LEN];
   +/* static charencrypted[_PASSWORD_LEN]; */
 
Just delete unused things like this, instead of commenting them out,
and __CTASSERT sizeof((struct crypt_data *)0) >= _PASSWORD_LEN.

(or maybe struct crypt_data::__buf should just be _PASSWORD_LEN bytes
long)
 
   --- lib/libcrypt/crypt-argon2.c 22 Nov 2021 14:30:24 -  1.15
   +++ lib/libcrypt/crypt-argon2.c 12 Feb 2022 12:47:04 -
   @@ -379,10 +379,10 @@
   /* argon2 pwd buffer */
   char pwdbuf[128];
   /* returned static buffer */
   -   static char rbuf[512];
   +   /* static char rbuf[512]; */
 
delete, __CTASSERT

This should also be a __CTASSERT to confirm that the size in
crypt_data is enough -- with a name or reference for where 512 came
from, like the Argon2id paper or API documentation:

__CTASSERT(sizeof(data->__buf) >= ARGON2_MAX_HASH);

   --- lib/libcrypt/crypt-sha1.c   29 Oct 2021 13:22:08 -  1.10
   +++ lib/libcrypt/crypt-sha1.c   12 Feb 2022 12:47:04 -
   @@ -107,12 +107,12 @@
 * hmac key.
 */
crypt_private char *
   -__crypt_sha1 (const char *pw, const char *salt)
   +__crypt_sha1 (const char *pw, const char *salt, struct crypt_data *data)
{
static const char *magic = SHA1_MAGIC;
static unsigned char hmac_buf[SHA1_SIZE];
   -static char passwd[(2 * sizeof(SHA1_MAGIC)) +
   -  CRYPT_SHA1_SALT_LENGTH + SHA1_SIZE];
   +/* static char passwd[(2 * sizeof(SHA1_MAGIC)) +
   +  CRYPT_SHA1_SALT_LENGTH + SHA1_SIZE]; */

Same deal with __CTASSERT here.

   --- lib/libcrypt/crypt.c22 Feb 2020 10:29:17 -  1.38
   +++ lib/libcrypt/crypt.c12 Feb 2022 12:47:04 -
   @@ -466,7 +466,8 @@


static C_block 

Re: crypt_r()?

[Taylor R Campbell]

> Date: Wed, 16 Feb 2022 10:27:08 -0500 (EST)
> From: Mouse 
> 
> > Thi is an essential hardening step against FPGA/custom ASIC
> > implementations.
> 
> I can't help feeling that there should be better ways to do that.  I
> like the idea of resistance to such things, but, for at least my
> purposes, the ability to check passwords without major resource
> consumption is a routine desire; resistance against an attacker willing
> to invest in custom hardware is not.

Then for your purposes, you can set default parameters in
/etc/passwd.conf that are bounded according to the resources of the
least capable machine in your environment.

But a _program_ that is supposed to work with any /etc/master.passwd
has to be able to handle the parameters set there, so it's not
sensible to ask the caller to preallocate enough storage for any
password hashing computation since there is, a priori, no static upper
bound on how much storage that might be (not to mention it might also
need to spawn threads for parallelism).

Re: [PATCH] argon2 key generation method for cgdconfig(8)

[Taylor R Campbell]

> Date: Mon, 8 Nov 2021 13:33:27 +
> From: nia 
> 
> On Sat, Nov 06, 2021 at 09:42:04AM +0000, Taylor R Campbell wrote:
> > That said, since we already argon2 logic as part of libcrypt, does it
> > make sense to have another copy in cgdconfig?
> > 
> > I guess the main issue is with pthreads.  Maybe we can find a way
> > around this with non-threaded weak aliases in libargon2 (maybe
> > argon2_thread_create/join), so applications can override them with
> > strong symbols that use pthreads but out of the box libcrypt.so
> > doesn't require libpthread?
> 
> I decided I don't want to add a new library dependency to libcrypt
> because external software will be linking against it and it's
> surprising for those use cases.
> 
> Do we want to use libcrypt here, though? It would add extra
> string processing and it also stores hashes secrets in a static
> variable, which may be a problem for cgd because we need the hash
> to be secret.

What I had in mind was linking against a common libargon2 in /lib.
But maybe the engineering cost isn't worth however many hundred
kilobytes or so the extra copy of libargon2 incurs.


The PBKDF2 calibration code does a second run to verify the timing,
and starts over if it isn't reproducible.  Maybe argon2id_calibrate
should too?  (Not a blocker.)


Have you tested a release build, and maybe running through sysinst?
LGTM by code inspection other than some minor nits.


> + if (sysctl(mib, __arraycount(mib),
> + , _len, NULL, 0) < 0) {

sysctl(...) == -1, not sysctl(...) < 0

> + if (getrlimit(RLIMIT_AS, ) < 0)
> + return usermem64;

same

> + const uint64_t usermem = get_usermem();

This is in units of 2^10 bytes too, right?  Comment, here and on
definition of get_usermem?

> + if (clock_gettime(CLOCK_MONOTONIC, ) == -1)
> + goto error;
> [...]
> +error:
> + errx(EXIT_FAILURE, "failed to calculate hash parameters");

Would be nice to show the errno message, for the branches where errno
is set.

> +error_a2:
> + errx(EXIT_FAILURE,
> + "failed to calculate Argon2 hash, error code %d\n", err);

No \n in err message.

> + argon2id_calibrate(BITS2BYTES(keylen), DEFAULT_SALTLEN,
> + >kg_iterations, >kg_memory, >kg_parallelism);

Might be nice to have some feedback to the console that cgdconfig(8)
is calibrating, maybe if `-v' is passed.  (Same with the PBKDF2
calibration!)

Re: [PATCH] argon2 key generation method for cgdconfig(8)

[Taylor R Campbell]

> Date: Sat, 6 Nov 2021 00:20:58 +
> From: nia 
> 
> > Why dup what you just created?  Why not kg->kg_key = bits_new(raw,
> > keylen)?  This looks like a (minor) memory leak.
> 
> This is what the existing code for pkcs5_pbkdf2/sha1 does. I believe
> that the returned pointer (raw) is immediately freed after being
> compared by the consumer.

Oh, I see -- it saves the copy in kg->kg_key, and returns ret to the
caller; missed the latter part.

> --- sbin/cgdconfig/Makefile   1 Jul 2016 22:50:09 -   1.15
> +++ sbin/cgdconfig/Makefile   6 Nov 2021 00:17:47 -
> [...]
> +LDADD+=  -Wl,-Bstatic
> +PROGDPLIBS+= argon2 ${ARGON2DIR}/lib/libargon2
> +LDADD+=  -Wl,-Bdynamic

Err...  I don't think this will do what you want it to do.  The effect
will presumably be to add something like

-Wl,-Bstatic -Wl,-Bdynamic ... -largon2

to the linker command line eventually, because PROGDPLIBS gets
processed much later on.

That said, since we already argon2 logic as part of libcrypt, does it
make sense to have another copy in cgdconfig?

I guess the main issue is with pthreads.  Maybe we can find a way
around this with non-threaded weak aliases in libargon2 (maybe
argon2_thread_create/join), so applications can override them with
strong symbols that use pthreads but out of the box libcrypt.so
doesn't require libpthread?

> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ sbin/cgdconfig/argon2_utils.c 6 Nov 2021 00:17:48 -
> [...]
> + mem = usermem / 10;

What units are these in?  Maybe add a comment explaining so the number
10 is a little more obvious?

> + /* Increase 'time' until we reach a second */
> +
> + delta.tv_sec = 0;
> + delta.tv_usec = 0;
> +
> + if (getrusage(RUSAGE_SELF, ) == -1)
> + goto error;
> +
> + for (; delta.tv_sec < 1 && time < ARGON2_MAX_TIME; ++time) {
> + if ((err = argon2_hash(time, mem, ncpus,
> + tmp_pwd, sizeof(tmp_pwd),
> + salt, saltlen,
> + key, keylen,
> + NULL, 0,
> + Argon2_id, ARGON2_VERSION_NUMBER)) != ARGON2_OK) {
> + goto error_a2;
> + }
> + if (getrusage(RUSAGE_SELF, ) == -1)
> + goto error;
> + timersub(_utime, _utime, );
> + }

Slightly confused by this logic.  It looks like we increase time
linearly, and then stop when the _entire sequence of trials_ has
exceeded 1sec.  Shouldn't we increase time (say) exponentially, and
stop when _one trial_ has exceeded one second?

> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ sbin/cgdconfig/argon2_utils.h 6 Nov 2021 00:17:48 -
> [...]
> +void argon2id_calibrate(size_t, size_t, size_t *, size_t *, size_t *);

Need #include  for size_t.

Re: [PATCH] argon2 key generation method for cgdconfig(8)

[Taylor R Campbell]

> Date: Fri, 5 Nov 2021 16:25:05 +
> From: nia 
> 
> This patch adds an "argon2id" password-based key generation method
> to cgdconfig(8).

Cool, thanks for working on this!

(For future patches: please use the `-p' option with cvs diff!)

> +++ lib/Makefile  5 Nov 2021 15:41:41 -
> @@ -54,6 +54,10 @@
>  SUBDIR+= libnvmm
>  .endif
>  
> +.if (${MKARGON2} != "no")
> +SUBDIR+= ../external/apache2/argon2/lib/libargon2
> +.endif
> [...]
> --- external/apache2/Makefile 12 Oct 2021 17:24:36 -  1.4
> +++ external/apache2/Makefile 5 Nov 2021 15:41:41 -
> @@ -2,6 +2,10 @@
>  
>  .include 
>  
> +.if (${MKARGON2} != "no")
> +SUBDIR+= argon2
> +.endif
> +

It looks like we'll descend into external/apache2/argon2 twice this
way.  Am I mistaken?  Is this intentional?

> --- sbin/cgdconfig/Makefile   1 Jul 2016 22:50:09 -   1.15
> +++ sbin/cgdconfig/Makefile   5 Nov 2021 15:41:43 -
> [...]
> +ARGON2DIR=   ${NETBSDSRCDIR}/external/apache2/argon2
> +ARGON2OBJDIR!=   cd ${ARGON2DIR}/lib/libargon2 && ${PRINTOBJDIR}
> +CPPFLAGS+=   
> -I${NETBSDSRCDIR}/external/apache2/argon2/dist/phc-winner-argon2/include
> +CPPFLAGS+=   -DHAVE_ARGON2
> +LDADD+=  -Wl,-Bstatic
> +LDADD+=  -L${ARGON2OBJDIR} -largon2
> +LDADD+=  -Wl,-Bdynamic
> +LDADD+=  -pthread
> +DPADD+=  ${ARGON2OBJDIR}/libargon2.a ${LIBPTHREAD}

Can this be made to use LIBDPLIBS, or are there too many moving parts
here?

> RCS file: sbin/cgdconfig/argon2_utils.c
> diff -N sbin/cgdconfig/argon2_utils.c
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ sbin/cgdconfig/argon2_utils.c 5 Nov 2021 15:41:43 -
> @@ -0,0 +1,165 @@
> +/*   $NetBSD$ */

Missing __RCSID in body of .c file?

> + uint8_t tmp_pwd[17];
> + char tmp_encoded[512];

Where do these magic constants come from?  Can they be named or made
more obvious?

> + char encoded[512];

Is it necessary to pass this in?  We're not using it; can we pass in
null instead so argon2 doesn't bother to encode it?

> RCS file: sbin/cgdconfig/argon2_utils.h
> diff -N sbin/cgdconfig/argon2_utils.h
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ sbin/cgdconfig/argon2_utils.h 5 Nov 2021 15:41:43 -

Missing include guard.

> --- sbin/cgdconfig/cgdconfig.830 Apr 2021 21:07:34 -  1.50
> +++ sbin/cgdconfig/cgdconfig.85 Nov 2021 15:41:43 -
> [...]
> +.It memory Ar integer
> +Memory consumption in kilobytes.

I wonder whether this can nicely be made a little more obvious, like

memory 123M;

or something.

I wonder whether cgdconfig -g/-G could have an option to specify the
percentage of RAM.

> --- sbin/cgdconfig/cgdconfig.c16 Jun 2021 23:22:08 -  1.52
> +++ sbin/cgdconfig/cgdconfig.c5 Nov 2021 15:41:43 -
> [...]
> + ret = bits_new(raw, keylen);
> + kg->kg_key = bits_dup(ret);

Why dup what you just created?  Why not kg->kg_key = bits_new(raw,
keylen)?  This looks like a (minor) memory leak.

Would be nice if we had automatic tests for cgdconfig -g/-G.


P.S.  Would also be nice if cgdconfig(8) had a way to enter a single
password from which multiple keys are derived, so you can do a single
password entry (and, perhaps, a shell command for a U2F/FIDO button
press interaction like fidocrypt) and use the result for multiple
disks encrypted with separate keys.  Not part of argon2 support, of
course, just musing...

Re: Changing the default localcipher in passwd.conf to argon2id

[Taylor R Campbell]

> Date: Wed, 20 Oct 2021 14:37:41 -0700
> From: Alistair Crooks 
> 
> I think it's a good idea, BUT I'd be a lot happier if the argon2 support
> was in a regular release (I know it's just the default cipher going
> forward, but I suspect some people have got into the nasty habit of cloning
> some of /etc from git or hg - maybe even cvs? :) - repos in some places,
> and onto various vintages of hosts)

Maybe passwd(1) should have a destdir option so if you want to update
~/config/etc/master.passwd in git/hg/cvs then you can do

passwd -D ~/config/etc agc

or something and it will use ~/config/etc/passwd.conf instead of
/etc/passwd.conf to determine localcipher?

I don't think it is generally reasonable to require all /etc defaults
to work out of the box in all supported past releases -- better to
find ways to improve configuration management than to put such a
restriction on base.

Re: long double losing mantissa bits

[Taylor R Campbell]

> Date: Tue, 19 Jan 2021 22:16:32 +
> From: Taylor R Campbell 
> 
>input: 72057594037927937 -> scanf: 72057594037927937.00
>frac: 0.50 0x8.08p-4 sexp: 57
>uexp: b9
>ufrac: 0080 
>56   : 00ff 
>Unexpected result: 5c80   
>  expected   : 5c80   0001
>056200: sign:  0 uexp:  b9 ufrac: 00   
> 
> Here frac = 0x8.08p-4 = (1 + 2^56)/2^57, so
> frac*72057594037927936 = frac * 2^56 = 0x8.08p52 =
> 0x80.8p52.
^^^
Excuse me -- the exponent at the very end was a typo.  It should read:

> Here frac = 0x8.08p-4 = (1 + 2^56)/2^57, so
> frac*72057594037927936 = frac * 2^56 = 0x8.08p52 =
> 0x80.8.

Or, frac * 2^56 = (1 + 2^56)/2 = 1/2 + 2^55.

Re: long double losing mantissa bits

[Taylor R Campbell]

> Date: Tue, 19 Jan 2021 22:16:32 +
> From: Taylor R Campbell 
> 
> This is not an integer, so conversion to uint64_t rounds it to
> nearest, with ties to even, so you get ufrac = 0x80 =
> 36028797018963968 as shown in the result.

Oops -- one more correction: conversion to uint64_t rounds toward
zero, not to nearest.  The result happens to coincide in this case.

> It looks like you have an off-by-one error in your exponent handling.
> If you want to scale the fractional part into an integer, you need to
> multiply by 2^57, not by 2^56.

Another possibility is that you expected round-to-odd or something
instead of toward-zero or to-nearest -- or perhaps the code works as
intended but the test cases are buggy.

Re: long double losing mantissa bits

[Taylor R Campbell]

> Date: Tue, 19 Jan 2021 22:10:56 +0100
> From: Rhialto 
> 
> The loss of precision probably occurs at the line
> 
> // The following big literal is 2 to the 56th power:
> ufrac = (uint64_t) (frac * 72057594037927936.0);
> 
> where frac is in the range [ 0.5 , 1.0 >, so I would expect that
> multiplying with 2**56 is perfectly feasable (just changes the
> exponent). Debugging output shows that differences in the lsbits that
> were detectable before (when printed with %La), were no longer after.

In the following test case:

   input: 72057594037927937 -> scanf: 72057594037927937.00
   frac: 0.50 0x8.08p-4 sexp: 57
   uexp: b9
   ufrac: 0080 
   56   : 00ff 
   Unexpected result: 5c80   
 expected   : 5c80   0001
   056200: sign:  0 uexp:  b9 ufrac: 00   

Here frac = 0x8.08p-4 = (1 + 2^56)/2^57, so
frac*72057594037927936 = frac * 2^56 = 0x8.08p52 =
0x80.8p52.

This is not an integer, so conversion to uint64_t rounds it to
nearest, with ties to even, so you get ufrac = 0x80 =
36028797018963968 as shown in the result.

It looks like you have an off-by-one error in your exponent handling.
If you want to scale the fractional part into an integer, you need to
multiply by 2^57, not by 2^56.

Re: Waiting for Randot

[Taylor R Campbell]

> Date: Sun, 17 Jan 2021 10:43:41 +0200
> From: Andreas Gustafsson 
> 
> For the Nth time: if the problem is solved at the system integration
> level, there will be no blocking and therefore no need for your
> proposed change.

But conversely, if the problem is solved at the system integration
level, there will be no need for blocking and therefore no harm from
the proposed change.

So what I would like to hear is how causing applications to silently
hang indefinitely helps to solve the problem.

The experience I've seen over a year where the unblocking criterion
is, for the first time, actually meaningful -- and not just `eh, a few
keystrokes or network packets oughta be ``random enough'', right?' --
is that applications silently hanging indefinitely confuses people,
leads them to think NetBSD is broken, and/or gets them frustrated at
the stupid paternalistic entropy system.  (`Why don't you just put
``dd if=/dev/urandom of=/dev/random bs=32 count=1'' in the man page,
or do it in rc to fix the issue automatically?')

> I for one did feel trapped by this, but it's easily fixed.  I also
> think the UI is too complicated and intimidating in general and
> needs to be simplified.  But I still think sysinst is the best place
> to do this, and certainly more user friendly than being dropped into
> single user mode on boot.

You and I may be perfectly happy with understanding and addressing the
technical details at installation time, but I'm not willing to impose
the same burden on everyone around me.

There's a tension between several things here:

1. Minimizing burden on users -- which means avoiding asking deeply
   technical questions they may not be competent to answer like `what
   is a string you just picked uniformly at random from 2^256
   possibilities?', especially when captive while running sysinst
   where there's little opportunity to explore and read man pages at
   leisure.

2. Working on a large variety of hardware -- which includes hardware
   that does not have a HWRNG.

3. Guaranteeing security or failing closed -- which means suddenly
   ceasing to work for deeply technical reasons.

We can't satisfy all three at the same time.  You would like to do (2)
and (3); someone whose machines all have HWRNGs might only care for
(1) and (3); others who are just trying to get things done on private
networks will pick (1) and (2).

So I'm trying to simultaneously do a few things to improve the
situation for everyone:

- improve silent out-of-the-box entropy on as many systems as possible
  (more HWRNG drivers; hardclock samples as makeshift ring oscillator;
  also considering spinlock contention as another one)

- give users unobtrusive feedback about their own security (concise
  warnings on console, daily security report, maybe motd) so they are
  alerted to the problem with a reference to further reading about how
  to address it

- provide the option for security-critical systems to fail closed at
  boot with a clear indication of why (entropy=check/wait in rc.conf)

- potentially log the keys that may need to be regenerated to assist
  the operator in fixing the problem in case of, e.g., installation by
  writing a live image only accessible via ssh

and

- keep the people who aren't security nerds happy enough that they
  aren't tempted to go around creating workarounds that make things
  worse just to get done what they were trying to do (like suppress
  _all_ feedback with the dd hack)

But it's becoming less and less clear to me what value there is to
making applications hang indefinitely in this system -- not just as an
isolated matter of a single component failing closed, but how it helps
the whole system.

Re: Waiting for Randot (or: nia and maya were right and I was wrong)

[Taylor R Campbell]

> Date: Sat, 16 Jan 2021 14:34:47 +0200
> From: Andreas Gustafsson 
> 
> Even if the unblocking criteria of Linux and FreeBSD are questionable,
> they still provide more security than your proposal which amounts to
> having extremely strict criteria but then completely ignoring.

This is not accurate.  The strict criterion is still available _for
system integration_, which is where the problem needs to be solved
anyway, just like other security measures like securelevel and
fetch_pkg_vulnerabilities.

The effect of making _applications_ just hang like this is that it
confuses everyone involved and breaks builds, because in practical
terms nothing is going to unblock them until the operator plugs in a
USB HWRNG or equivalent and it is not helpful to ask every application
to include logic that meaningfully prompts the user to do so.

> >Such an application, like a Python program in the middle of just doing
> >`import multiprocessing', is not in a position to remedy the situation
> >or even usefully alert an operator to the problem.
> 
> Which is why we should deal with the issue by creating an entropy seed
> at the time of installation or first boot.

We already do this, and it incorporates any entropy obtained during
the installation process.

This will include keystroke timings during installation, and hundreds
of samples of a makeshift ring oscillator on each boot, if the
hardclock timer interrupt and CPU cycle counter or system timecounter
are driven by independent clocks.  Of course, it is hard to tell for
sure whether this is the case from MI software -- so it remains best
effort, without a confidently positive assessment.

> To start with, we should re-enable the code Martin already added to
> sysinst to prompt the user for missing entropy at install time, and we
> should continue to work on making it easier to use.

It's not just a prompt -- it will make users feel _trapped_ and hate
the whole thing, in an installer that already has too many mandatory
incomprehensible questions.  As a matter of user experience, a
mandatory question like this that gets in the way of doing anything
else is a dead end.

Security features that paternalistically prevent people from getting
things done will drive them to workarounds that defeat the security.

> We should also provide more entropy sources with conservative but
> nonzero estimates to make those prompts increasingly rare.

I would be happy to consider literature with conservative assessments
of these entropy sources if you have any references!

But a year after drafting the changes, nobody has provided such
references, and many people have run into practical problems that the
system in place is not conducive to fixing.

That's why, based on the experience, I'm trying a different approach:
provide clear hints and nudges where there may be a problem, without
making anyone feel trapped and resentful toward the issue, and without
causing apparently unrelated things to break on principle.

Re: Waiting for Randot (or: nia and maya were right and I was wrong)

[Taylor R Campbell]

> Date: Sat, 16 Jan 2021 13:21:21 +0100
> From: Kamil Rytarowski 
> 
> On 11.01.2021 02:25, Taylor R Campbell wrote:
> > Many of you have no doubt noticed that a lot more things hang waiting
> > for entropy than used to on machines without hardware random number
> > generators (even as we've added a bunch of new drivers for HWRNGs) --
> > e.g., python, firefox.
> 
> Can we overload the ENOSYS return value and return it for CPUs without
> hardware assisted random number generator? This way we certainly catch
> real bugs in software that do not handle ENOSYS anyway.

How does that detect real bugs?  How does it improve anything?

Re: Waiting for Randot (or: nia and maya were right and I was wrong)

[Taylor R Campbell]

> Date: Fri, 15 Jan 2021 15:35:26 -0500 (EST)
> From: Mouse 
> 
> >Such an application, like a Python program in the middle of just
> >doing `import multiprocessing', [...]
> 
> Seems to me the elephant in the room here is: why would Python "just
> doing `import multiprocessing'" need cryptographic-strength randomness?
> What am I missing that makes this reasonable?

I already addressed this in a previous thread that I cited at the
start of this one, and I'm not interested in getting into a discussion
about how we should patch Python to work differently on NetBSD from
every other platform.  Here's the background:

https://mail-index.netbsd.org/current-users/2020/12/05/msg040019.html

Re: Waiting for Randot (or: nia and maya were right and I was wrong)

[Taylor R Campbell]

> Date: Fri, 15 Jan 2021 20:54:21 +0200
> From: Andreas Gustafsson 
> 
>  Your proposal would mean that
> such an application would generate predictable keys on NetBSD when no
> entropy is available, even though it will not on Linux.  To me, that's
> completely unacceptable.

This is not accurate, because the criteria for unblocking are
qualitatively different.

Linux will unblock when the `entropy estimator' -- which is designed
without knowledge of any of the physical details of the entropy
sources, and roughly counts whether successive differences of
successive differences of 32-bit samples are distinct -- has been
fooled into counting enough bits.

FreeBSD will unblock when a certain fixed number of bytes of samples
have been entered, no matter what sources they came from.  (I forget
what the number is offhand but I seem to recall it's around 64 bytes.)

More details and references from the prior discussion:
https://mail-index.netbsd.org/tech-userlevel/2020/05/09/msg012390.html
https://mail-index.netbsd.org/tech-userlevel/2020/05/10/msg012406.html

In answer to a question you had which seems to have gone unanswered at
the time:

> Are these the same criteria as those used for unblocking /dev/random?

Yes.

And NetBSD, as you know, has a much more stringent criterion for
unblocking anything.  Over the past ~year of experience, we've seen
the blocking behaviour of getrandom(p,n,0) lead to practical problems
that are confusing and leave applications in a state that isn't
conducive to remedying the problem -- as I said in the initial message
(https://mail-index.netbsd.org/tech-userlevel/2021/01/11/msg012807.html):

   It's certainly a problem when keys are generated with too little
   entropy -- e.g., https://factorable.net -- but it's increasingly clear
   that _the middle of an application trying to get something else done_
   is not a good place for hanging until someone plugs in a USB HWRNG.

   Such an application, like a Python program in the middle of just doing
   `import multiprocessing', is not in a position to remedy the situation
   or even usefully alert an operator to the problem.

I'm open to improvements to the system integration.  For example,
aside from the security report and entropy=check/wait in rc.conf and a
potential one-line notice in the motd, I'm also considering how to
unobtrusively record a log of long-term keys that may have been
generated without enough entropy (e.g., ssh-keygen in /etc/rc.d/sshd),
so that an operator can conveniently review the log and regenerate
them once entropy is available.

But if we're to keep the blocking behaviour at the point where any
application is generating a key, then I'd like to have a stronger
justification for how making every other Python program just silently
hang forever on some machines helps further any broader goal.

Re: Waiting for Randot (or: nia and maya were right and I was wrong)

[Taylor R Campbell]

> Date: Fri, 15 Jan 2021 20:41:25 +0100
> From: Reinoud Zandijk 
> 
> Well no, on install, open a tcp connection to a TNF hosted server...

We've discussed this before.  Before continuing on the topic, please
review the prior discussion like I just asked:

https://mail-index.netbsd.org/tech-crypto/2020/05/13/msg000779.html

And please consider starting a separate thread if you have anything
new to add about it.  I started this thread to discuss a specific
proposal about the getrandom and getentropy API, not for discursive
commentary on every imaginable physical system we might set up and
observe for entropy:

https://mail-index.netbsd.org/tech-userlevel/2021/01/11/msg012807.html

Re: Waiting for Randot (or: nia and maya were right and I was wrong)

[Taylor R Campbell]

Folks, this thread was to discuss a specific proposal about the
getrandom and getentropy C API:

   With these in mind, I propose that we change getrandom(p,n,0) so that
   it does not block -- under the premise that dealing with low entropy
   is a system integration problem, not a problem that it is helpful to
   ask an application to resolve in the heat of the sampling moment.

   Programs can still poll /dev/random (or getrandom(p,n,GRND_RANDOM)),
   if testing for entropy is actually their goal, but the default
   recommended choice for all applications to generate keys, which is
   getrandom(p,n,0), will not.

   I also propose we introduce never-blocking getentropy like nia@
   briefly did last year, as an alias for getrandom(p,n,0) soon to be in
   POSIX (https://www.austingroupbugs.net/view.php?id=1134), under the
   premise that the never-block semantics (from the original in OpenBSD)
   is justified again by treating low entropy as a system integration
   problem.

While I appreciate ideas about how to improve further on the system
integration out of the box, it would also help if you could review the
entropy(7) man page before making suggestions that we already do:

https://man.NetBSD.org/entropy.7

Here's the previous thread where introducing a random.netbsd.org
server came up:

https://mail-index.netbsd.org/tech-crypto/2020/05/13/msg000779.html

I encourage you to read the analysis there before discussing it
further.  That said, I'd also like to focus on the API question in
this thread.

Re: Waiting for Randot (or: nia and maya were right and I was wrong)

[Taylor R Campbell]

> Date: Thu, 14 Jan 2021 13:21:58 +0100
> From: Manuel Bouyer 
> 
> On Thu, Jan 14, 2021 at 10:15:41AM +, nia wrote:
> > I still think my idea to record a second of noise from /dev/audio on
> > machines that totally lack other strong sources is a good one. We did
> > already put together the code and test it on a range of hardware and
> > VMs.
> 
> And what about systems that don't have a /dev/audio (or system that have
> play-only /dev/audio) ?

What about them?  Systems without usable microphone noise are no worse
off than they would have been without nia's suggestion.

If a machine doesn't have any unpredictable inputs, well, there's no
magic we can do -- you can copy a seed over from another machine (on a
private network where you're confident there's no eavesdropper), and
there's always the last resort of flipping a coin 256 times and doing
`echo tththtttht... >/dev/urandom' (or rolling a six-sided die 100
times, ).

Re: Waiting for Randot (or: nia and maya were right and I was wrong)

[Taylor R Campbell]

> Date: Thu, 14 Jan 2021 10:15:41 +
> From: nia 
> 
> I still think my idea to record a second of noise from /dev/audio on
> machines that totally lack other strong sources is a good one. We did
> already put together the code and test it on a range of hardware and
> VMs.

I agree -- I think sysinst should take advantage of that if it can be
done unobtrusively, even better if it can be done reliably without
saying anything to the user.

> Overall though I'm reasonably happy with this compromise, although
> it would still make me sleep safer at night if we very conservatively
> added a bit from environmental sensors ever so often - perhaps based
> on a advance measurements from a range of hardware rather than
> runtime calculations.

We do incorporate the data; we just don't count it.  So the only
effect that this change would have is to unblock things _earlier_ than
they would otherwise unblock -- i.e., this change could only make
things `less safe'.  That said, if you have some reasonable analysis
for particular devices, I would be happy to consider it!

> The man page is very clearly written, aside from the parts that
> recommend tossing coins.

Thanks!  I know tossing coins sounds silly.  But as a fallback if you
have no other options, it really is 100% guaranteed to work, it's easy
for anyone to confidently understand, and as a bonus it avoids any
concerns with supply chain attacks on HWRNGs,   So that's why I
mention it -- buried near the bottom, as a last resort.

Re: Waiting for Randot (or: nia and maya were right and I was wrong)

[Taylor R Campbell]

> Date: Mon, 11 Jan 2021 12:23:46 +0100
> From: Martin Husemann 
> 
> On Mon, Jan 11, 2021 at 01:25:36AM +0000, Taylor R Campbell wrote:
> > We might also do something similar with the motd -- add a single line,
> > citing entropy(7) for more details, if there's not enough entropy.
> 
> Please don't - that is one of the least usefull places to put such a
> note.

Can you expand on why?

It's a common place these days for, e.g., `software updates have been
installed requiring a reboot' (Ubuntu); it can be a single line (we
used to have paragraphs of text!); it would give a reference to the
user-oriented documentation to read for more information.

> I still think that this should be dealt with (once and for all) at
> installation time (as we did for a short period, for some machines and
> install methods) - but apparently it is impossible to reach consensus
> on the wording and supported methods, so I won't touch it.

It's fine to put _optional_ functionality into sysinst, perhaps in the
utility menu or in the post-installation config menu alongside setting
the timezone and enabling ssh   What's not fine is making the user
feel trapped until they take some remedial action about entropy,
before they can proceed to anything else in the installation.

The installation process is already too cumbersome and full of
incomprehensible jargon with mandatory questions about things like
BIOS disk geometry that even I (despite being a member of the core
team) have no idea how to answer correctly.

I'm not saying this to blame you for complexity -- I appreciate the
work you've put into improving sysinst.  I just expect that the impact
of thrusting mandatory questions full of jargon in a user's face is to
make them hate the questions and do whatever minimal activity they can
to get through without really improving security.  Cf. certificate
warnings, which just induce mindless clickthrough.

Waiting for Randot (or: nia and maya were right and I was wrong)

[Taylor R Campbell]

[bcc tech-kern, tech-security, tech-crypto; followups to
tech-userlevel to keep discussion in one place]

Many of you have no doubt noticed that a lot more things hang waiting
for entropy than used to on machines without hardware random number
generators (even as we've added a bunch of new drivers for HWRNGs) --
e.g., python, firefox.

This is related to the adoption of the getrandom system call from
Linux, which we adopted with the semantics that getrandom(p,n,0) will
block until the kernel is certain there is enough entropy for
security.

In retrospect, based on experience with the change, such as the
following threads and bugs (as well as many private discussions on
IRC), I think adopting getrandom with this semantics was a mistake:

https://gnats.NetBSD.org/55641
https://gnats.netbsd.org/55847
https://mail-index.NetBSD.org/current-users/2020/09/02/msg039470.html
https://mail-index.NetBSD.org/current-users/2020/11/21/msg039931.html
https://mail-index.netbsd.org/current-users/2020/12/05/msg040019.html

It's certainly a problem when keys are generated with too little
entropy -- e.g., https://factorable.net -- but it's increasingly clear
that _the middle of an application trying to get something else done_
is not a good place for hanging until someone plugs in a USB HWRNG.

Such an application, like a Python program in the middle of just doing
`import multiprocessing', is not in a position to remedy the situation
or even usefully alert an operator to the problem.  To better address
the system integration, I added hooks in /etc/rc and /etc/security for
alerting the operator to the problem with entropy:

- setting `entropy=check' in /etc/rc.conf will abort multiuser boot
  and enter single-user mode if there's not enough entropy before
  starting any network services (or setting `entropy=wait' will make
  multiuser boot hang -- caveat: possibly indefinitely)

- the daily /etc/security script will check for entropy and send an
  alert citing the new entropy(7) man page in the security report

We might also do something similar with the motd -- add a single line,
citing entropy(7) for more details, if there's not enough entropy.

With these in mind, I propose that we change getrandom(p,n,0) so that
it does not block -- under the premise that dealing with low entropy
is a system integration problem, not a problem that it is helpful to
ask an application to resolve in the heat of the sampling moment.

Programs can still poll /dev/random (or getrandom(p,n,GRND_RANDOM)),
if testing for entropy is actually their goal, but the default
recommended choice for all applications to generate keys, which is
getrandom(p,n,0), will not.

I also propose we introduce never-blocking getentropy like nia@
briefly did last year, as an alias for getrandom(p,n,0) soon to be in
POSIX (https://www.austingroupbugs.net/view.php?id=1134), under the
premise that the never-block semantics (from the original in OpenBSD)
is justified again by treating low entropy as a system integration
problem.

Thoughts?


P.S.  Previous discussions about getrandom, getentropy, blocking, and
changes to the kernel entropy subsystem for NetBSD 10:

https://gnats.NetBSD.org/55659
https://mail-index.NetBSD.org/tech-userlevel/2020/05/02/msg012333.html
https://mail-index.NetBSD.org/current-users/2020/05/01/msg038495.html
https://mail-index.NetBSD.org/tech-kern/2019/12/21/msg025876.html

Re: [PATCH] Make more of stdlib.h visible if _NETBSD_SOURCE isn't defined

[Taylor R Campbell]

> Date: Mon, 31 Aug 2020 00:25:52 +
> From: m...@netbsd.org
> 
> The following script fails to compile, it shouldn't.
> 
> #!/bin/sh
> 
> cat << EOF > test.c
> #include 
> #include 
>  
> void f1(void)
> {
> puts("pushed first");
> fflush(stdout);
> }
>  
> void f2(void)
> {
> puts("pushed second");
> }
>  
> int main(void)
> {
> at_quick_exit(f1);
> at_quick_exit(f2);
> quick_exit(0);
> }
> EOF
> cc test.c -std=c11 -Werror

Works for me?

% cat << EOF > test.c
...
EOF
% cc test.c -std=c11 -Werror
% ./a.out
pushed second
pushed first
% 

Re: [chris...@netbsd.org: CVS import: src/external/mit/libuv/dist]

[Taylor R Campbell]

The reason we imported unbound/nsd in the first place was so that we
wouldn't be stuck with tracking bind.  Why don't we just remove it
like we planned to do years ago?

The only reason we imported nsd, too, was to mitigate possible fallout
from losing any authoritative nameserver in base when we removed bind.
Really I don't see a good reason why we need to have an authoritative
nameserver in base at all -- it's not crucial for system integration
or for getting onto the internet the way a recursive resolver is.

This is likely to cause trouble with libuv from pkgsrc, which in
recent experience has had fast-moving API changes.

Re: Initial entropy with no HWRNG

[Taylor R Campbell]

[trimming cc list to tech-crypto]

> Date: Tue, 12 May 2020 11:45:58 -0400
> From: Thor Lancelot Simon 
> 
> 1) It's hard to understand how many bits of entropy to assign to a
>sample from one of these sources.  [...]
> 
>The delta estimator _was_ good for these things, particularly for
>things like fans or thermistors (where the macroscopic,
>non-random physical processes _are_ expected to have continuous
>behavior), because it could tell you when to very conservatively
>add 1 bit.

What is the model you're using to justify this claim that actually
bears some connection to the physical devices involved?

Without a physically justifiable model -- one that generally works on
_all_ hardware of any type that a driver supports -- or a claim from a
vendor about what's going on in the device, that's not something we
should be fabricating from whole cloth and foisting on users.

> B) One thing we *could* do to help out such systems would be to actually run
>a service to bootstrap them with entropy ourselves, from the installer,
>across the network.  Should a user trust such a service?  I will argue
>"yes".  Why?
> 
> B1) Because they already got the binaries or the sources from us; we could
> simply tamper those to do the wrong thing instead.

Tampering is loud, but eavesdropping is quiet.  There is no way to do
this that is resistant to eavesdropping without a secret on the client
side.

(This would also make TNF's infrastructure a much juicier target,
because it would grant access to the keys on anything running a new
NetBSD installation without requiring tampering.)

Re: getrandom and getentropy

[Taylor R Campbell]

> Date: Mon, 11 May 2020 12:42:13 -0700 (PDT)
> From: Paul Goyette 
> 
> Why can't we allow the user to configure/enable estimation on a
> per-source basis?  The default can certainly be "disabled", but
> why not override?  Just like any other super-user thing, there's
> no reason not to enable shoot-my-random-foot mode.

You can shoot yourself in the foot with wild abandon by doing

dd if=/dev/urandom of=/dev/random

The `estimation' -- as in, feeding samples through an extremely dumb
statistical model that bears no relation whatsoever to the physical
process that produced them, and deciding based on that how much
entropy must be in that physical process -- is no longer in the kernel
at all, because it is not merely a foot-gun; it is total nonsense.
Having it around at all is -- and always has been -- essentially
dishonest.

When each driver feeds a sample in, it specifies a number which
represents a reasonable lower bound on the number of bits of entropy
in the process that generated the sample; usually this number is
either zero, or determined by reading the manufacturer's data sheet
and/or studying the physical device that the driver handles.

There's no way for the operator to override the number that is passed
alongside every sample at run-time because that would add substantial
complexity to an already complex subsystem -- you would, effectively,
be changing the driver logic at run-time -- for essentially zero
benefit.

It would be comparable to inventing a general mechanism by which to
make every DELAY issued by a driver configurable at run-time, even
though the delays are documented in the data sheet.  (If appropriate
for a particular driver, that driver could have a sysctl knob to
adjust it, of course.)

Re: getrandom and getentropy

[Taylor R Campbell]

> Date: Mon, 11 May 2020 16:16:12 - (UTC)
> From: mlel...@serpens.de (Michael van Elst)
> 
> Previously we could trust in random processes, whether the entropy
> estimation was scientific or not. We could also chose what source
> to trust.

Still can.  NetBSD just doesn't do bogus pseudoscientific
prestidigitation any more.

> Now we put all trust in loading a constant file.

This is still false, just like it was the last time you made this
claim.

> >This hardware can reasonably block forever on first boot, due to
> >the large number of sources of entropy that are no longer measured.
> 
> Not "can". It does, definitely, always.
> 
> And it never blocks on second boot.

This is false.

Please do your homework first, and then take this to a thread where it
is on topic, not the thread about a choice of C API to adopt.

For example, here's a thread where you had months of opportunity to
raise your concerns, and where your misapprehensions could have been
addressed without the sarcastic commentary:

https://mail-index.NetBSD.org/tech-kern/2019/12/21/msg025876.html

You're also welcome to ask me privately if you're unclear on anything
specific about it, or you think there's a specific mistake (but please
be specific, rather than just raising general FUD).

Re: getrandom and getentropy

[Taylor R Campbell]

> Date: Sun, 10 May 2020 22:10:55 +
> From: m...@netbsd.org
> 
> I still don't find the getrandom man page you provided to be good, it
> talks about "/dev/random behaviour" which is something you've changed to
> not have this behaviour.

Can you be specific?

The phrase `/dev/random behaviour' does not appear in the man page I
wrote, but the behaviour of getrandom(..., GRND_RANDOM) and the
behaviour of reads from /dev/random are exactly the same in Linux
since getrandom was introduced, and in the patch I proposed, so I'm
not sure what the issue is.

I tried to emphasize that GRND_RANDOM is provided only for source
compatibility with Linux and should generally be avoided:

  `This is provided mainly for source compatibility with Linux; there
   is essentially no reason to ever use it.'
  `(No examples of GRND_RANDOM because it is not useful.)'
  `GRND_RANDOM is a little silly.'

But perhaps I left something in that is still confusing and/or gives
the impression that GRND_RANDOM _should_ be used for some
applications; if so, please let me know where?

It's also entirely possible that I'm so steeped in this that what I
wrote is unclear for anyone who is not so steeped in it.  Concrete
suggestions or feedback about places where the prose I wrote remains
confusing are welcome.

Re: getrandom and getentropy

[Taylor R Campbell]

> Date: Sun, 10 May 2020 14:24:00 +0300
> From: Andreas Gustafsson 
> 
> The getentropy() man pages on OpenBSD, FreeBSD, and Linux all say it
> returns "high-quality" entropy, and do not caution against using it
> for security critical purposes such as key generation, so presumably
> applications do in fact use if for such purposes.  Given that,
> implementing it as getrandom(..., GRND_INSECURE) seems like a bad
> idea.

The term `high-quality' is, as they say, bearing a lot of load there.
My message was meant to compare exactly what it means across several
systems that have adopted the name `getentropy'.  It turns out that:

- On FreeBSD, it means 64 bytes of data (not data from a source with
  512 bits of entropy total -- just 64 bytes of data) have been fed
  into the pool.

- On Linux, it means that maybe some interrupts have come in at
  irregular times within a single boot -- whatever is enough to fool
  the `delta estimator', even if the times are exactly the same from
  boot to boot and there is no physical justification for believing
  them to be unpredictable.

In other words, it doesn't mean very much!

On NetBSD-current we do not count either of these metrics.  Instead,
what NetBSD-current does is:

1. Makes best effort to get as many samples as it can into the entropy
   pool early on -- like OpenBSD, which introduced the getentropy API
   in the first place under that premise.

2. Counts bits of entropy that have some justification:
   . HWRNG drivers can make claims to the kernel based on the
 documentation of how the physical device works or advertisements
 made by the manufacturer (which the operator can disable with
 rndctl -E if they know better than the driver author).
   . The operator can make claims to the kernel by writing data to
 /dev/random explicitly, so, e.g., you can flip a coin 256 times
 and and write the outcomes to /dev/random.
   . The kernel will believe one seed file entropy estimate per boot,
 subject to various criteria (read/write medium, ) so it can
 keep up the 256 coin flips from boot to boot.

The standard advice for many years has been that applications should
read key material from /dev/urandom, and systems should be engineered
to make sure that it's ready by the time applications run.

The semantics of getrandom(..., GRND_INSECURE) is the same as the
semantics of reading from /dev/urandom on Linux and in the patch I
proposed, and I think that semantics is more or less what developers
generally expect with the name getentropy.

On the other hand, (2) is a more stringent criterion than either
FreeBSD or Linux applies -- for example, where Linux might believe
there is some entropy in the timing of `keyboard' input provided by
anita, NetBSD-current does not.

> Also, two of the man pages explicitly mention blocking, so any
> portable software using getentropy() should already deal with it.
> So why not do the safe thing and implement it as getrandom(..., 0)
> == read from /dev/random?

That issue is why I'm not really happy about the getentropy API: it
was originally defined to never block, and some systems have made it
block for reasons that don't really mean very much.

This is an argument for providing just getrandom -- the API contract
is is clearer and doesn't require long detailed messages summarizing
research into exactly what the blocking criteria are and what the
practical consequences for them are.

Re: getrandom and getentropy

[Taylor R Campbell]

> Date: Sun, 10 May 2020 07:44:36 +0200
> From: Michael van Elst 
> 
> On Sun, May 10, 2020 at 05:23:37AM +0000, Taylor R Campbell wrote:
> > But how is this question relevant to the discussion at hand of whether
> > to adopt a de facto standard C API or which?
> 
> It isn't. 

Then please take it to another thread.

Re: getrandom and getentropy

[Taylor R Campbell]

> Date: Sun, 10 May 2020 04:58:23 - (UTC)
> From: mlel...@serpens.de (Michael van Elst)
> 
> riastr...@netbsd.org (Taylor R Campbell) writes:
> 
> >(Obviously, it is possible to run a kernel with an /etc/rc script that
> >skips loading the seed from disk altogether; I'm not considering weird
> >customized installations like that.  System engineers who futz with
> >this are responsible for getting the details right.)
> 
> Like installing a system from read-only media or a system that crashed
> and starts again with the same seed.

This is why:

- rndctl -L ignores entropy estimates in seeds on read-only media

- rndctl -L fails noisily if anything goes wrong with updating the
  seed file -- which entails writing it and fsyncing it before
  committing it with rename.

Can this go wrong?  Sure, and we can discuss ways to make it more
reliable if you like.  But my point here was not to get into all the
details of how the seed saving and restoring process works.

My point was to compare the different processes and blocking criteria
across different operating systems -- to show side by side any
meaningful differences in what you're guaranteed by the time NetBSD
sysctl kern.arandom returns data vs the time OpenBSD getentropy
returns data vs the time Linux getrandom(..., 0) returns data, , in
order to assess how best to provide consistent semantics, if we can.

> I'm wondering, how you can trust a god-sent file from persistent storage
> but not an unspecified random process?

I don't know what you mean by this.  NetBSD-current counts the entropy
specified by HWRNG drivers when they do rnd_add_data(rs, buf, len,
entropybits), and counts data you write to /dev/random as if it came
from a process with full entropy; it also records the count in the
seed file saved with rndctl -S so you can load it again with rndctl -L
if possible.

But how is this question relevant to the discussion at hand of whether
to adopt a de facto standard C API or which?

Re: getrandom and getentropy

[Taylor R Campbell]

> Date: Sun, 10 May 2020 00:10:49 +
> From: m...@netbsd.org
> 
> On Sat, May 09, 2020 at 10:56:51PM +0000, Taylor R Campbell wrote:
> > Given that, I think it is reasonable to implement getentropy(...)  as
> > an alias for getrandom(..., GRND_INSECURE) == read from /dev/urandom
> > == sysctl kern.arandom (as nia@ just committed the other day), which
> > is consistent with the somewhat nuanced interpretation of the
> > semantics above, and to provide getrandom(...,0) as I originally
> > suggested alongside it.
> 
> Given that getentropy as KERN_ARND is good enough for everyone, why not
> stick #define getrandom(a,b,c) getentropy(a,b) In some header?

That does not correctly implement the semantics, even aside from
blocking or pool state or any of the interesting stuff -- getentropy
is limited to 256 bytes and returns 0 or -1; getrandom is not so
limited and returns the number of bytes generated or -1.

Re: getrandom and getentropy

[Taylor R Campbell]

> Date: Sun, 3 May 2020 21:43:15 +
> From: nia 
> 
> On Sun, May 03, 2020 at 09:28:37PM +0000, Taylor R Campbell wrote:
> > But on closer inspection, it's not clear there's a consensus on what
> > the semantics is supposed to be -- apparently _what_ everyone seems to
> > implement is slightly different:
> > 
> > - OpenBSD originally defined getentropy to be block-never, as in
> >   getrandom(GRND_INSECURE)
> > 
> > - glibc, FreeBSD, and illumos implement block-at-boot, as in
> >   getrandom(0)
> 
> The thing is, I'm not convinced there's a meaningful difference between
> the first two. As you said, /systems/ should be engineered to block
> until enough entropy is available. block-on-boot beaviour shouldn't be
> hit because the system should wait until entropy is acquired before any
> application software with serious entropy needs gets initialized.

OK, I did a little more research into exactly what goes on behind the
scenes in a few different systems.  Here are four different models:

- NetBSD-current
  . RESEED SCHEDULE:
o reseed when 256 bits of entropy have been counted
o reseed on rndctl -L (done early in userland, /etc/rc.d/random_seed)
o reseed on write to /dev/random or sysctl -w kern.entropy.consolidate=1
  . ENTROPY COUNTING: driver estimate for HWRNG samples only
  . BLOCKING CRITERION: when 256 bits of entropy have been counted
  . WHAT'S READY IN USERLAND: all samples gathered before userland
(including HWRNG if available), and seed on disk, after
/etc/rc.d/random_seed has run (very early on in rc, just after the
local file systems needed to find the seed are mounted)
  . SOURCE REFERENCE:
https://nxr.NetBSD.org/xref/src/sys/kern/kern_entropy.c
https://nxr.NetBSD.org/xref/src/sys/kern/subr_cprng.c
https://nxr.NetBSD.org/xref/src/sys/dev/random.c

- OpenBSD
  . RESEED SCHEDULE:
o reseed with whatever we have before reaching userland
o reseed after every 10min
o reseed after every 16 bytes of output
o reseed after write to /dev/random
  . ENTROPY COUNTING: none
  . BLOCKING CRITERION: none
o Relies on gathering full entropy immediately, under the premise
  that either there's a HWRNG or there's enough entropy in timing
   measurements early at boot.
  . WHAT'S READY IN USERLAND: all samples gathered before userland
(including HWRNG if available), and seed on disk, after an early
point in rc
  . SOURCE REFERENCE:

https://cvsweb.openbsd.org/src/sys/dev/rnd.c?rev=1.204=text/x-cvsweb-markup

https://cvsweb.openbsd.org/src/etc/rc?rev=1.543=text/x-cvsweb-markup

- FreeBSD (assuming default configuration with Fortuna)
  . RESEED SCHEDULE:
o according to Fortuna model in Ferguson/Schneier/Kohno's book
o initial seeding depends on having enough physical bytes of data
  from any source (no entropy estimate involved)
  . ENTROPY COUNTING: none
  . BLOCKING CRITERION: block until initially seeded
o Relies on gathering full entropy within the first 64 bytes of
  data gathered (or kern.random.fortuna.minpoolsize).
o May not have been initially seeded by the time userland starts.
  . WHAT'S READY IN USERLAND: not necessarily anything, as far as I can tell
  . SOURCE REFERENCE:

https://svnweb.freebsd.org/base/head/sys/dev/random/randomdev.c?revision=356096=markup

https://svnweb.freebsd.org/base/head/sys/dev/random/fortuna.c?revision=358379=markup

- Linux >=5.6 (more or less the NetBSD<=9.0 model too)
  . RESEED SCHEDULE:
o reseed when 128 bits of entropy have been counted
o reseed every 5min after initially seeded
o reseed on ioctl(RNDRESEEDCRNG)
  . ENTROPY COUNTING:
o based on value of samples with `delta' estimator for timing samples
o driver estimate for HWRNG samples
  . BLOCKING CRITERION: block until first reseed
o Relies on accurate `delta' estimator or HWRNG.
  . WHAT'S READY IN USERLAND: not necessarily anything, as far as I can tell
  . SOURCE REFERENCE;

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/char/random.c?h=v5.6

In all of these models, userland getentropy(...) and getrandom(..., 0)
(where they are implemented) will generally not return anything until
it has been initialized:

- In OpenBSD and NetBSD, this is because they incorporate all of the
  samples at boot and a seed from disk early on.

- In FreeBSD and Linux, this is because they block until first seeded,
  although the blocking criterion is rather different in FreeBSD and
  Linux (Fortuna's data- and driver-independent schedule, vs Linux's
  delta estimator).

So, although OpenBSD's getentropy never blocks while FreeBSD's and
Linux's may block, I think it is possible to interpret the semantics
reasonably consistently across OpenBSD, FreeBSD, and Linux as follows
(and I think this is what joerg was getting at somewhat obliquely
earlier):

- On FreeBSD, getentro

Re: getrandom and getentropy

[Taylor R Campbell]

> Date: Sun, 3 May 2020 19:13:23 +
> From: nia 
> 
> Since most of the objections so far have been aimed at the design
> and implementation of getrandom, does anyone have anything bad to say
> about getentropy?

That's what I had in mind at the start of the thread after verifying
_that_ most other systems implement something called getentropy, under
the assumption that they would all agree on the semantics.

But on closer inspection, it's not clear there's a consensus on what
the semantics is supposed to be -- apparently _what_ everyone seems to
implement is slightly different:

- OpenBSD originally defined getentropy to be block-never, as in
  getrandom(GRND_INSECURE)

- glibc, FreeBSD, and illumos implement block-at-boot, as in
  getrandom(0)

- Oracle Solaris maybe implements block-often, as in traditional
  /dev/random, but it's not clear; it might be block-at-boot (I don't
  have source code to reference, but there is some weak evidence that
  Oracle's getentropy is or was painfully slow, which might arise from
  blocking repeatedly, whereas getrandom worked fine:
  )

So I'm not longer confident that the name is meaningfully reliable as
an API to adopt, and if anything OpenBSD is in the minority now in
treating it as our sysctl kern.arandom semantics -- in spite of the
fact that OpenBSD introduced the name in the first place.

In contrast, the semantics of getrandom is clear once it is written
down more clearly than the Linux man page, even if the GRND_RANDOM
part of it is silly.  Specifically, it seems to be the case that:

- The following idiom will reliably either generate a 32-byte key or
  block until the system thinks it is seeded (or abort if anything is
  horribly awry), everywhere it is defined:

if (getrandom(buf, 32, 0) == -1)
abort();

- The following idiom (the non-blocking version of the above) will
  reliably either generate a 32-byte key or abort immediately if the
  system is _not_ seeded (or if anything is horribly awry), everywhere
  it is defined:

if (getrandom(buf, 32, GRND_NONBLOCK) == -1)
abort();

- The following idiom will reliably either generate 32 bytes of data
  derived from whatever is in the pool (or abort immediately if
  anything is horribly awry), without blocking, irrespective of
  whether the system is seeded, everywhere it is defined:

if (getrandom(buf, 32, GRND_INSECURE) == -1)
abort();

(This applies up to 256 bytes, actually; for longer requests,
behaviour seems to vary and you may need to check the return code for
truncation.  But 32 is more than you ever need for cryptography.)

These are all more or less reasonable things to want to express in an
application or library but none of these statements remains generally
true if I replace getrandom(buf,32,...) by getentropy(buf,32).

Re: getrandom and getentropy

[Taylor R Campbell]

> Date: Sun, 3 May 2020 10:28:08 +0200
> From: Kurt Roeckx 
> 
> [OpenBSD] seem to use RDRAND when it's available in the bootloader, or
> something else when it's not. It's still my understanding that
> the bootloader is responisble for providing the entropy. You can
> argue that it might not contain as much entropy as you would like
> in all cases.
> [...]
> Various of their drivers have support for RNGs that are available
> on hardware, which seems to include: amdpm, glxsb, pchb, hifn,
> safe, ccp, tpm, amlrng, bcmirng, bcmrng, mvrng, octrng, omrng,
> rkrng, urng, uonerng. It's unclear to me if any of them are used
> in the bootloader.

NetBSD has drivers for various hardware RNGs too which will generally
gather entropy before userland starts (usually as soon as they are
discovered during bus enumeration at boot), and on x86 NetBSD will
gather entropy from RDRAND/RDSEED very early on in the kernel boot:

https://nxr.netbsd.org/xref/src/sys/arch/amd64/amd64/machdep.c#1680

The NetBSD bootloader doesn't do anything with RDRAND/RDSEED, although
I'm not sure it makes much of a difference to do it in the bootloader
vs doing it this early on in the kernel proper.  That's why I say that
if what OpenBSD does satisfies you, what NetBSD does should probably
satisfy you too.

In any case, not every machine _has_ a hardware RNG, and not every
machine is necessarily seeded, which is why NetBSD still adopts a
blocking model available through /dev/random and perhaps soon through
getrandom.

> Date: Sun, 3 May 2020 10:48:41 +0200
> From: Kurt Roeckx 
> 
> On Fri, May 01, 2020 at 07:19:09PM +, Taylor R Campbell wrote:
> > +Despite the name, this is secure as long as you only do it
> > +.Em after
> > +at least one successful call without
> > +.Dv GRND_INSECURE ,
> > +such as
> > +.Li "getrandom(..., 0)"
> 
> At which point calling with GRND_INSECURE is the same as calling
> with 0 ...

Generally yes, although the sysctl knob kern.entropy.depletion=1 may
cause it to block again, so that you can easily test the impact of
blocking on any application before you deploy it into the field where
conditions may be different as a kind of fault injection, whereas
getrandom(...,GRND_INSECURE) is guaranteed never to block as a
reliable part of the API contract everywhere.  Conceivably if a
process in a VM were migrated from one host to another,
getrandom(...,0) might also block twice in the same process.

Point is: getrandom(...,0) has blocking as part of the API contract in
edge cases, and getrandom(...,GRND_INSECURE) does not.

But in normal operation without kern.entropy.depletion=1, yes, you are
right.

(If you want to discuss whether the Linux API should have
GRND_INSECURE at all, that's more of a discussion for the LKML.  The
path has existed in most OSes for a couple decades, anyway -- whether
via /dev/urandom, or via getentropy, or via kern.arandom.)

> > +or
> > +.Li "getrandom(..., GRND_RANDOM)" ,
> > +or after reading at least one byte from
> > +.Pa /dev/random .
> 
> Note that this is not the cases anymore on Linux. After reading 1
> byte from /dev/random, /dev/urandom can still be unintialized, and
> so GRND_INSECURE is still insecure on Linux.

In Linux 5.6, both getrandom(GRND_RANDOM) and /dev/random call
wait_for_random_bytes(), which waits until crng_ready() is true,
before returning a single byte:

/dev/random: 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/char/random.c?h=v5.6#n1836
getrandom(GRND_RANDOM): 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/char/random.c?h=v5.6#n2000
wait_for_random_bytes: 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/char/random.c?h=v5.6#n1611

crng_ready() returns true only if crng_init > 1, which in turn is set
only if (a) the CPU provided data via RDRAND/RDSEED or equivalent and
all the PRNG state has been initialized, or (b) enough entropy has
been gathered that that the system (via credit_entropy_bits) or the
operator (via ioctl(RNDRESEEDCRNG)) decided to reseed:

crng_ready(): 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/char/random.c?h=v5.6#n464
crng_init = 2, option (a): 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/char/random.c?h=v5.6#n804
crng_init = 2, option (b): 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/char/random.c?h=v5.6#n949

The bytes returned from /dev/random and getrandom(GRND_RANDOM) are
then returned from the same source as /dev/urandom:

getrandom(GRND_RANDOM): 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/char/random.c?h=v5.6#n2004
/dev/random: 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/char/random.c?h=v5.6#n1839
/dev/urandom: 
htt

Re: getrandom and getentropy

[Taylor R Campbell]

> Date: Sat, 2 May 2020 18:07:54 +0200
> From: Kurt Roeckx 
> 
> On Sat, May 02, 2020 at 03:38:43PM +0000, Taylor R Campbell wrote:
> > > Date: Sat, 2 May 2020 11:10:44 +0200
> > > From: Kurt Roeckx 
> > > 
> > > I hink we've previously talked about it, and you said the OpenBSD
> > > manpage doesn't mention anything related to it. But it's implied
> > > behaviour for OpenBSD, they never had an interface where you can
> > > get random numbers before it's properly seeded.
> > 
> > I reviewed the OpenBSD implementation at
> > 
> > https://cvsweb.openbsd.org/src/sys/dev/rnd.c?rev=1.204=text/x-cvsweb-markup
> > 
> > and I see no evidence of blocking.  Where does it block?
> 
> It's my understanding that it never blocks because the bootloader
> provides entropy. Be time time the first user can call genentropy,
> it has already been seeded.

On NetBSD we try to do that where possible too, but in the real world
it can't be 100% guaranteed to work on NetBSD or on OpenBSD -- for
example, if you copy the same fresh OS image onto multiple machines
(every machine might generate the same keys), then it won't work, or
if your / is mounted on a read-only medium, then it won't work (boot
again and you might get the same keys).

If you're satisfied with what OpenBSD does here, then I think you
should generally be satisfied with what NetBSD does too.

Re: getrandom and getentropy

[Taylor R Campbell]

> Date: Sat, 2 May 2020 12:22:20 +
> From: m...@netbsd.org
> 
> The getrandom interface intentionally traps people to make questionable
> design choices.

I agree that the getrandom interface is not perfect, and I found the
Linux documentation rather confusing, but after having reimplemented
the semantics I realized the confusion is mostly a matter of
documentation, which is why I addressed this in my original message:

   ### Why were you initially reluctant to adopt them and what changed?

   - I found the Linux documentation for getrandom difficult to follow,
 and I suspect most other people do too.  It is hard to see what
 configuration of flags give you the semantics you want, and with
 eight different choices for the three separate flags it seemed like
 a good way to have lots of people shoot themselves in the foot.

 However, after implementing the semantics and distilling it, I
 realized that it cleanly breaks down into only three usage models
 (with an nonblocking option -- meaning when it would block, it
 returns EAGAIN/EWOULDBLOCK instead), two of which are reasonable
 (flags=0 and flags=GRND_INSECURE), and one of which we didn't
 already have a pathway for.

 So the cost of adopting a silly operation (flags=GRND_RANDOM)
 strikes me as quite small in exchange for the benefit of source
 compatibility, and I tried to address the confusing API by writing
 short usage guidelines with clear examples in the man page
  for
 what the flags argument can be.  If in doubt, use flags=0.

In the end, getrandom(...,0) is the _only_ API to reliably block once
after boot and then return arbirarily much data across many operating
systems; neither /dev/random nor /dev/urandom nor getentropy nor
sysctl kern.arandom nor anything else does that reliably on many
operating systes.

So I think it is worthwhile to adopt that API, and if we have that it
takes very little to get source compatibility with the other getrandom
options -- getrandom(...,GRND_INSECURE) is just another name for what
we already have as sysctl(kern.arandom,...), and getrandom(...,
GRND_RANDOM) is just another name for reading from /dev/random.

> We might immediately take https://www.2uo.de/myths-about-urandom/
> 
> And re-write bits about /dev/random as being about getrandom(...,GRND_RANDOM).

This would be a reasonable article to write, not because NetBSD might
adopt getrandom(...,GRND_RANDOM) as a silly quirk, but because lots of
other systems support _and advertise_ getrandom(...,GRND_RANDOM), like

and .

> Calling it "/dev/random" behaviour is ambiguous. Didn't you fix
> /dev/random to not have this limitation, and be more like
> getrandom(...,0)?

getrandom(...,GRND_RANDOM) has a small limit on the data it will
return, as in Linux.  The same has been true of /dev/random for a long
time -- ask to read 1000 bytes, and you might get 16.  /dev/random has
never been appropriate for large reads; GRND_RANDOM is the same.

Generally GRND_RANDOM is silly, and I don't think it's worth spending
time on it, except for the sake of source compatibility with Linux and
everyone else, and to point out in the man page that it's a bad idea
and to decline to give examples of using it.

> Having compat shims in libc is as good as having compat shims in syscall
> because it isn't possible to share raw syscall code between NetBSD and
> other operating systems -- our calling convention is different.

I agree -- any sort of compatibility between the underlying syscalls
is _not_ the point of adding a new getrandom syscall.  The point, from
my original message, is that we simply don't have a path in the kernel
that blocks the way getrandom is expected to, without access to the
/dev/random file, so we need to add _some_ underlying syscall or other
interface to the kernel:

   ### Why a new getrandom system call?  Why not a userland libc wrapper?

   While getentropy(p,n) is essentially the same as
   sysctl(kern.arandom,p,n), the semantics of getrandom(p,n,0) (and the
   slightly silly semantics of getrandom(p,n,GRND_RANDOM)) requires a
   potentially blocking code path that is currently available to userland
   only via /dev/random.

   In other words, we have no path accessible from userland to
   simultaneously to satisfy desiderata (2) and (4).  So if we want to
   satisfy them, we need a new path into the kernel.

Re: getrandom and getentropy

[Taylor R Campbell]

> Date: Sat, 2 May 2020 11:10:44 +0200
> From: Kurt Roeckx 
> 
> On Fri, May 01, 2020 at 07:19:09PM +0000, Taylor R Campbell wrote:
> > 
> > The alias getentropy(p,n) := getrandom(p,n,GRND_INSECURE)
> 
> At several places in your document you imply this. But
> getentropy(p,n) is more like getrandom(p,n,0). That is, it also
> waits until it's seeded, it only blocks a single time.
> 
> I hink we've previously talked about it, and you said the OpenBSD
> manpage doesn't mention anything related to it. But it's implied
> behaviour for OpenBSD, they never had an interface where you can
> get random numbers before it's properly seeded.

I reviewed the OpenBSD implementation at

https://cvsweb.openbsd.org/src/sys/dev/rnd.c?rev=1.204=text/x-cvsweb-markup

and I see no evidence of blocking.  Where does it block?

On OpenBSD, /dev/random and /dev/urandom are the same -- they both
never block, according to <https://man.openbsd.org/urandom>.

I reviewed the OpenBSD commit logs, and I don't see any evidence of
any blocking paths since 2010 when /dev/srandom was removed:

https://cvsweb.openbsd.org/src/sys/dev/rnd.c?rev=1.103=text/x-cvsweb-markup

Generally whether something can block or not is an important part of
the API contract, and it seems to me that from the beginning when
OpenBSD introduced getentropy it has never blocked.