freebsd security - Aug 2015 - [PATCH] Please review this rc.d/sshd tiny yet ripe low hanging fruit for me.

it?s no cannon but i did find a foot-aimed .22 in rc.d/ssh i think deserves some
red paint right away. and while in the neighborhood i?d like to share an
improvement i?ve enjoyed for a while. i apologize for the list-bombing, if i may
have a moment of your time:

TLDR:
https://bugs.freebsd.org/bugzilla/attachment.cgi?id=159642&action=diff

Quick Background:
there?s two unrelated issues going on here but i ran across them in close
physical, mental and temporal proximity so let?s just knock them out together
because the solutions are simple, small, and don?t modify today's default
behavior.

My Concerns:
ONE is adding functionality allowing an admin to tweak the key generation sshd
makes upon its first run using variables in rc.conf instead of the current day
requirement of essentially manually generating those keys, hopefully the same
way, putting them hopefully in the right place. (not hard for most of us, i
know.) TWO, then, is adding some sort of red paint to a foot-aimed gun i came
across when considering the variable names in rc.d/sshd and lack of mention in
defaults/rc.conf or man 5 rc.conf.

My Plea:
please put this into head and 10/stable and please 10.2-RC as well. i know it?s
really late for that but seriously if anyone can find anything wrong with this
i?ll send them $10 now just for chiming in or $20 now for improving/fixing it.
hopefully you?ll agree it?s pretty simple and solid as-is and you'll
participate and say so for the warm feeling. :)

Full Story:
i?ve been running the following tweak to my /etc/rc.d/sshd for four years on
dozens of machines and apparently applying it on most installations i do, 10
seconds of my time per, has thus far been less pain than writing this PR and
email, LOL (most times i don?t even copy it from anywhere besides my temporal
lobe, it?s such a tiny mod):

--- /usr/src/etc/rc.d/sshd	2014-11-15 00:04:00.000000000 +0000
+++ /etc/rc.d/sshd	2015-07-14 23:24:11.426005726 +0000
@@ -56,8 +56,9 @@
 	if [ -f "${keyfile}" ] ; then
 		info "$ALG host key exists."
 	else
+		eval keygen_flags=\$sshd_${alg}_flags
 		echo "Generating $ALG host key."
-		/usr/bin/ssh-keygen -q -t $alg -f "$keyfile" -N ""
+		/usr/bin/ssh-keygen -q -t $alg -f "$keyfile" $keygen_flags -N
""
 		/usr/bin/ssh-keygen -l -f "$keyfile.pub"
 	fi
 }

and then that lets me add this to rc.conf.local, for which i hope my purpose is
apparent and goes without saying:

sshd_rsa_flags="-b 4096"
sshd_ecdsa_flags="-b 521?

and thanks to already existing functionality i also use the following in
rc.conf.local in every single installation as well:

sshd_rsa1_enable="NO"
sshd_dsa_enable="NO"

so that?s it! that?s my whole functionality improvement i want included
upstream, LOL. However, it rubs me as not adequate, not ready for general
consumption. There?s something wrong and i couldn?t quite put my finger on it
until lately.

Been doing that patch for years and never thought much of it. Always was aware i
needed to precede sshd?s first-run with these settings and didn?t give it a
second thought after including it in our in-house rollout scripts as well as
many one-offs for family/friends. Then, a tech friend/partner/client picked up
on those lines disabling rsa1 and dsa with a little bit of monkey-see-monkey-do
but never said anything to me about it before applying just those on his 9.3
servers with the stock rc.d/sshd. So i happen to be poking around his hobby rig
the other day and i saw the two *_enable="NO" lines in his
rc.conf.local BUT all five few-month-old keys nonetheless sitting there in his
/etc/sshd/ (!) and, of course, sshd was dutifully responding to hails on the
frequencies he intended to have banned.

So he clearly was tripped up on some surprising disappointment of his
expectation that sshd_dsa_enable=NO might do as named, as i see quickly skimming
the top of rc.d/ssh might lead one to believe, so, i propose changing it to
sshd_dsa_keygen_enable, if that name even helps at all, and, more importantly,
explaining a bit in the defaults list and rc.conf(5) that: to effectively change
any of these [now] ten settings one must then also go delete key files that may
already exist. I don?t even know if i?ve accomplished a sufficient explanation
in defaults/rc.conf and wonder if someone familiar with man pages might improve
upon my terse lines in defaults/rc.conf with detail into rc.conf(5) for us, just
documenting sshd_*_keygen_enable and sshd_*_keygen_flags to make it very clear
they are put into effect at a particular moment in time and to effect a change
one must manually invoke conditions leading again to that particular
circumstance (delete the key so it can regenerate).

I know that might seem obvious to most here but it?s not currently intuitive to
those learning. This guy?s a bright protege and i had to feel bad after i
thwacked him in the head with my bo-staff for having his RSA1 and DSA zippers
open because the more i thought about it the more i realized that as it sits
it?s sort of counter-intuitive.

Conclusion:
so i?ve improved that years-old tiny patch with slightly better variable names
for the existing vars, plus my own added vars renamed to follow form, as well as
added hopefully meaningful if terse description to defaults/rc.conf while
mapping the old existing names in rc.d/sshd to the better names along with
issuing a warning about them if they?re set. any of those decisions/details are
optional, i just want the original functionality of my original tiny patch
(aforementioned, inlined) in head at least, if not my full patch (link, top of
message) into 10.2-RC asap.

Thanks everyone so much for your efforts on FreeBSD and hopefully I haven?t
taken up too much of your time pontificating on the color of the little shed for
my poodle?s mini-tricycle and we can quickly agree this is a good morsel of low
hanging fruit to improve the security, consistency and usability of FreeBSD at
the same time and fast-track it into 10.2, if i?m not being too arrogant and
bold in proposing so.

-Chad J. Milios

On Aug 7, 2015, at 1:46 PM, Chad J. Milios <milios at ccsys.com>
wrote:
> ...i apologize for the list-bombing, if i may have a moment of your time:
> TLDR:
> https://bugs.freebsd.org/bugzilla/attachment.cgi?id=159642&action=diff
> ?..
> My Concerns:
> ONE is adding functionality allowing an admin to tweak the key generation
sshd makes upon its first run using variables in rc.conf instead of the current
day requirement of essentially manually generating those keys, hopefully the
same way, putting them hopefully in the right place. (not hard for most of us, i
know.) TWO, then, is adding some sort of red paint to a foot-aimed gun i came
across when considering the variable names in rc.d/sshd and lack of mention in
defaults/rc.conf or man 5 rc.conf.
> ?..
FYI, I have ported the identical functionality now to the
security/openssl-portable and security/openssl-portable-devel ports so no one
has to miss out. Please would you try one out and now configure your (-b)etter
keys in a consistent way in new deployments from now on or upgrade yours if you
are using defaults and delete existing /etc/ssh/ssh_host_foo_key* files manually
if you intend to update them.

Knocking out little fixes like this will keep making things like sysrc more
useful and mergemaster even more worthless, bless its tired heart. Help assure
this works as intended in many cases with as many ssh options as possible.
THANKS

PATCHES: either...

base system:
https://bugs.freebsd.org/bugzilla/attachment.cgi?id=159642&action=diff
<https://bugs.freebsd.org/bugzilla/attachment.cgi?id=159642&action=diff>

ports/security/openssl-portable
https://bz-attachments.freebsd.org/attachment.cgi?id=159654
<https://bz-attachments.freebsd.org/attachment.cgi?id=159654>

ports/security/openssl-portable-devel
https://bugs.freebsd.org/bugzilla/attachment.cgi?id=159655&action=diff
<https://bugs.freebsd.org/bugzilla/attachment.cgi?id=159655&action=diff>

Thank you all. PS here are a couple configs I?d like to hear everyones thoughts
on. Let?s mix up the monoculture more:

openssh_rsa1_keygen_enable="NO"
openssh_dsa_keygen_enable="NO"
openssh_rsa_keygen_flags="-b 4096"
openssh_ecdsa_keygen_flags="-b 521"
openssh_ed25519_keygen_enable="YES" #default

sshd_rsa1_keygen_enable="NO"
sshd_dsa_keygen_enable="NO"
sshd_rsa_keygen_flags="-b 16384"
sshd_ecdsa_keygen_enable="NO"
sshd_ed25519_keygen_enable="NO"

openssh_rsa1_keygen_enable="NO"
openssh_dsa_keygen_enable="NO"
openssh_rsa_keygen_enable="NO"
openssh_ecdsa_keygen_enable="NO"
openssh_ed25519_keygen_enable="YES" #default

Can we have a conversation about how best to configure things to require
&& (and) keys instead of || (or) keys for certain/all users? Using
sshd_config and/or PAM?

openssh_rsa1_keygen_flags="-b 16384?
openssh_dsa_keygen_enable="YES" #default
openssh_rsa_keygen_flags="-b 16384"
openssh_ecdsa_keygen_flags="-b 521"
openssh_ed25519_keygen_enable="YES" #default