NetBSD Problem Report #56835

From www@netbsd.org  Sat May 14 18:23:06 2022
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id DB6A71A921F
	for <gnats-bugs@gnats.NetBSD.org>; Sat, 14 May 2022 18:23:05 +0000 (UTC)
Message-Id: <20220514182304.3E3691A923C@mollari.NetBSD.org>
Date: Sat, 14 May 2022 18:23:04 +0000 (UTC)
From: tgl@sss.pgh.pa.us
Reply-To: tgl@sss.pgh.pa.us
To: gnats-bugs@NetBSD.org
Subject: sshd startup script produces very misleading "UNSAFE KEYS" warnings
X-Send-Pr-Version: www-1.0

>Number:         56835
>Category:       bin
>Synopsis:       sshd startup script produces very misleading "UNSAFE KEYS" warnings
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    bin-bug-people
>State:          closed
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Sat May 14 18:25:01 +0000 2022
>Closed-Date:    Sun May 15 11:48:00 +0000 2022
>Last-Modified:  Sun May 15 11:50:01 +0000 2022
>Originator:     Tom Lane
>Release:        HEAD/202205021430Z (problem may be old, though)
>Organization:
PostgreSQL Global Development Group
>Environment:
NetBSD sss2.sss.pgh.pa.us 9.99.96 NetBSD 9.99.96 (GENERIC) #2: Sat May 14 11:02:34 EDT 2022  tgl@nuc1.sss.pgh.pa.us:/home/tgl/netbsd-H-202205021430Z/obj.hppa/sys/arch/hppa/compile/GENERIC hppa
>Description:
/etc/rc.d/sshd modifies /etc/motd to warn you if your sshd keys have been generated with low entropy.  That's great, or would be if the warning weren't so misleading.  What it actually does is to update /etc/motd unconditionally based on the state of "kern.entropy.needed" when you last started sshd, regardless of the age of your keys.  This has at least two bad failure modes:

* If, for some reason, you boot without adequate entropy, it will start warning you, even if your key files predate the reboot and are perfectly secure.  (I ran into this case when the entropy file disappeared due to a kernel panic.)

* After you reboot with adequate entropy, it will stop warning you, even if your key files were made with inadequate entropy.  IMO this is bad enough to be on the edge of being a security bug; anyone who takes the warning at face value will be misled.
>How-To-Repeat:
* Install new system on machine with no hardware entropy source.  Enable sshd.

* Log in, note presence of warning (good).

* Provide entropy, eg cat whatever >/dev/urandom.

* Once kern.entropy.needed is down to zero, reboot, *without* doing the recommended key regen.

* Log in, note absence of warning (bad).
>Fix:
I think the script should only change motd when it actually made new key files.  Here's one attempt:
-----

Index: sshd
===================================================================
RCS file: /cvsroot/src/etc/rc.d/sshd,v
retrieving revision 1.31
diff -u -r1.31 sshd
--- sshd        26 Sep 2021 10:53:20 -0000      1.31
+++ sshd        14 May 2022 17:54:16 -0000
@@ -45,6 +45,7 @@
 (
        keygen="/usr/bin/ssh-keygen"
        umask 022
+       madesomething=no
        while read type bits filename;  do
                f="/etc/ssh/$filename"
                if [ "$1" != "force" ] && [ -f "$f" ]; then
@@ -58,14 +59,17 @@
                esac
                "${keygen}" -t "${type}" ${bitarg} -f "${f}" -N '' -q && \
                    printf "ssh-keygen: " && "${keygen}" -f "${f}" -l
+               madesomething=yes
        done << _EOF
 dsa    1024    ssh_host_dsa_key
 ecdsa  521     ssh_host_ecdsa_key
 ed25519        -1      ssh_host_ed25519_key
 rsa    0       ssh_host_rsa_key
 _EOF
+       if [ "$madesomething" = yes ]; then
+           sshd_motd_unsafe_keys_warning
+       fi
 )
-       sshd_motd_unsafe_keys_warning
 }

 sshd_precmd()

-----
This has still got some failure modes, notably if we updated only some of the key files for some reason.  That seems like a pretty edgy edge case though, and I'm not sure there's a good way to deal with it.  In any case, this seems a lot better than what's there now.

>Release-Note:

>Audit-Trail:
From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: bin/56835: sshd startup script produces very misleading "UNSAFE KEYS" warnings
Date: Sun, 15 May 2022 07:48:55 +0700

     Date:        Sat, 14 May 2022 18:25:01 +0000 (UTC)
     From:        tgl@sss.pgh.pa.us
     Message-ID:  <20220514182501.49BA21A923E@mollari.NetBSD.org>

 No coment here on the need for some change here, but if
 something like this patch was to be applied:

   |
   | Index: sshd
   | ===================================================================
   | RCS file: /cvsroot/src/etc/rc.d/sshd,v
   | retrieving revision 1.31
   | diff -u -r1.31 sshd
   | --- sshd        26 Sep 2021 10:53:20 -0000      1.31
   | +++ sshd        14 May 2022 17:54:16 -0000
   | @@ -45,6 +45,7 @@
   |  (
   |         keygen="/usr/bin/ssh-keygen"
   |         umask 022
   | +       madesomething=no
 +       madesomething=false
   |         while read type bits filename;  do
   |                 f="/etc/ssh/$filename"
   |                 if [ "$1" != "force" ] && [ -f "$f" ]; then
   | @@ -58,14 +59,17 @@
   |                 esac
   |                 "${keygen}" -t "${type}" ${bitarg} -f "${f}" -N '' -q && \
   |                     printf "ssh-keygen: " && "${keygen}" -f "${f}" -l
   | +               madesomething=yes
 +               madesomething=true
   |         done << _EOF
   |  dsa    1024    ssh_host_dsa_key
   |  ecdsa  521     ssh_host_ecdsa_key
   |  ed25519        -1      ssh_host_ed25519_key
   |  rsa    0       ssh_host_rsa_key
   |  _EOF
   | +       if [ "$madesomething" = yes ]; then
 +       if "$madesomething"; then
   | +           sshd_motd_unsafe_keys_warning
   | +       fi
   |  )
   | -       sshd_motd_unsafe_keys_warning
   |  }
   |  
   |  sshd_precmd()
   |

 is a better way to code it.

State-Changed-From-To: open->closed
State-Changed-By: martin@NetBSD.org
State-Changed-When: Sun, 15 May 2022 11:48:00 +0000
State-Changed-Why:
Patch applied (with minor changes). Thanks!


From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/56835 CVS commit: src/etc/rc.d
Date: Sun, 15 May 2022 11:47:42 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Sun May 15 11:47:42 UTC 2022

 Modified Files:
 	src/etc/rc.d: sshd

 Log Message:
 PR 56835: fix sshd startup script to only whine about bogus keys it
 created if it actualy did create keys (one should thing that a
 function called sshd_keygen() only is called to create keys, but
 the "precmd" magic makes it run every time sshd is started or stopped).

 Patch from Tom Lane, with modifications suggested by kre and a minor
 additional cosemtic change.


 To generate a diff of this commit:
 cvs rdiff -u -r1.31 -r1.32 src/etc/rc.d/sshd

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

>Unformatted:

Home	PR Database Search