Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 20 Jun 2018 08:43:29 -0700
From:      Conrad Meyer <cem@freebsd.org>
To:        "Simon J. Gerraty" <sjg@juniper.net>
Cc:        src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org,  svn-src-head@freebsd.org
Subject:   Re: svn commit: r335402 - head/sbin/veriexecctl
Message-ID:  <CAG6CVpXtK1uRow3=R=n6i82bhHKBB_3qGvCB0SxctsMLb=RDjQ@mail.gmail.com>
In-Reply-To: <96021.1529475664@kaos.jnpr.net>
References:  <201806200108.w5K18sIR050132@repo.freebsd.org> <CAG6CVpV124ze%2BY6xX2ZFqbM%2B3hJNEJWR2qpnChpey=PmiW6qXg@mail.gmail.com> <96021.1529475664@kaos.jnpr.net>

next in thread | previous in thread | raw e-mail | index | archive | help
I want to preface this by saying: this discussion should be happening
in either arch@ or phabricator, after the patch series was temporarily
reverted pending necessary improvements.  I have asked for the series
to be reverted and am still waiting on that.

I am happy to promise to respond promptly to updates on this
particular series so you are not waiting for two years again.

I think it's quite close to something reasonably general, but as-is,
it is worse than useless =E2=80=94 it promises a security feature but _does
not deliver it_, which is the "emperor's new clothes" of security.

On Tue, Jun 19, 2018 at 11:21 PM, Simon J. Gerraty <sjg@juniper.net> wrote:
> Conrad Meyer <cem@freebsd.org> wrote:
>> First and foremost: nothing is actually signed, anywhere.  The
>
> The signing of manifests is external.  The veriexecctl tool is I assume
> a straight copy of what's in NetBSD (I've not looked at it in at least a
> decade).

The signing of manifests does not exist in the patch series committed.

(If NetBSD does something broken, that is not an excuse to copy it.)

> A veriexec loader that leverages signed manifests requires some signing
> infra.  That's a big topic all by itself.

It *may* require that.  However, even without that, admins could
reasonably manage their own PKI in some fashion, independent of
FreeBSD's infra.  But it requires the support code to verify
signatures, as in the "verify" part of veriexec, which is wholly
absent.

> As I mentioned in my talk at BSDCan,

(FWIW, I was not at your talk, and it is not a justification for bad
design or implementation anyway.)

> the signing server we use is open
> source and handles pretty much anything OpenSSL can, as well as OpenPGP
> (and others).

It doesn't really matter if there's a signing server, because nothing
in the patch series *verifies* signatures.

> Tweaking the veriexec loader to only process manifests after
> verification is not hard

Then I even more do not understand why it was not done prior to commit.

> - one of the first things I did when pulling
> veriexec into Junos almost 15 years ago.
>
>> As a corollary to the above, the name "signature file" is used
>> repeatedly in the code, which is misleading.  The file contains hashes
>> (digests), not signatures (MACs).  The file itself is unsigned.
>> Nothing about this has signatures.
>
> NetBSD refers to the hashes as fingerprints - AFAIK that terminology is
> retained.

Fingerprints is fine, "signatures" is not.  "Signatures file" is used
to refer to the manifest file, which only contains fingerprints, in
multiple locations in the code.

> If the term signature is used to refer to anything other than the signed
> manifests that should be fixed.

Should have been fixed prior to commit, yes.

>> There's absolutely no reason to use sha1 or ripemd in new designs.
>> These should be removed.
>
> Sorry I disagree - not with ripem (we never supported that or any of the
> non-NIST approved hashes), but sha1 is still approved by NIST for
> firmware integrity checks - which is what this is, and sha1 is cheaper
> than sha256.

Again =E2=80=94 this is a discussion for arch or phabricator, with the seri=
es
reverted first.

I reckon you're wrong.  If you're unwilling to trust me, I believe you
should get and accept input from a 3rd party vaguely familiar with
cryptography (maybe cperciva@ or gordon@ or delphij@) before
introducing SHA1 or Ripe-MD in a novel integrity-protection design.

(Some modern Intel and AMD CPUs have intrinsics support for SHA-2, and
on those machines SHA-2 256 is about the same performance as SHA-1.)

Performance is absolutely not a reason to use a known weak hash
algorithm in 2018, especially when the feature as-committed has so
many other glaring performance problems.  If you care about MAC
performance in a secure algorithm in 2018, perhaps look at any of
these great options:

* SHA-3 (Keccak)
* Blake2-b
* Poly1305-{AES,Salsa,ChaCha}

All have highly efficient software implementations that smoke SHA-2
and don't have SHA-1's known weakness.  Blake2 is even in-tree
already.

> As I mentioned in my talk we've included support for sha256 for 10+
> years,

FreeBSD has had this code for 0 years.  It's a novel feature here.
There is no reason to introduce SHA-1 in novel security features in
2018.

> but do not plan to drop sha1 until NIST deprecate it for that
> purpose since boot time is a very sensitive subject for us.

See above.  There are lots of secure hashes faster than SHA-1 without
its known weaknesses.

>> The patchset is littered with style issues.  One fairly obvious issue
>> is mixed indentation styles =E2=80=94 some files vary between space and =
tab
>> indentation from line to line.
>
> You can probably blame me for some of that.
> I only recently found a style9.el that does a half decent job of
> formatting per style(9).

To commit to the tree, you are supposed to be able to get it right, or
at least close to right.

>> Please revert this patchset.  It's not ready.

>> - Maybe use HMACs instead of raw hashes
>
> Why?

Again, this design discussion is for arch or phab.

HMAC have a number of advantages over raw SHA-1/-2:

* Key verification
* No length extension attacks

Neither of these may apply here IFF the manifest is signed and
trusted, and there aren't other implementation flaws.  But
as-committed, there is no signed manifest.  A MAC implementation would
enable just storing these values as extended attributes on-disk
instead of a separate file that gets loaded.  Another design question
for arch.

>> - Maybe sign the source-of-trust file
>
> We do.  As noted above, we cannot upstream that until FreeBSD has
> suitable signing infra.

I disagree.  What was committed absolutely does not sign nor verify
the source-of-trust file.

And no, upstreaming the signature verification code is completely
orthogonal to implementing signing infrastructure.

The "veriexec" patchset in FreeBSD today:

1. Does not verify anything in a cryptographically sound manner
2. Slows down all concurrent access to associated filesystems

Why is this either necessary or helpful to be in the FreeBSD tree
as-is?  I don't think it is, and you should revert it.  Please.  I
don't know if there's a maintainer timeout on this kind of thing, but,
you are forewarned.

Thank you,
Conrad



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAG6CVpXtK1uRow3=R=n6i82bhHKBB_3qGvCB0SxctsMLb=RDjQ>