Add veriexec to loader

[sjg]

Summary

This feature depends on an external library: BearSSL
There are two new libraries libbearssl and libve
both of which can be consumed into libsa for the loader
or built as normal libraries.

I needed to add -O1 to stand/defs.mk else the loader was too big
to boot.

lib/libve/local.trust.mk should be considered an example.

[cem]

lib/libve/h/verify.h

27–28

Both of these concepts seem pretty dubious.

The loader and kernel should not be guessing signing policy — period.

Files either have a signature or do not. If they do, they must be verified. If they don't, they cannot be verified. Not sure what try or want has to do with that.

lib/libve/veopen.c

118

This is not going to be pretty with any significant number of verified files.

137–138

This is a lot of stack — is this file userspace-only?

191

style: missing space between comma and following parenthesis.

255

style: the semi-colon goes on its own line

303

Why are we comparing a printed hash at all?

(Also, strcmp is not constant time.)

392

style: return (foo);

405–413

None of the previous portion of this comment adds anything of value for the reader beyond the information already present in the function name and parameter types and names below.

lib/libve/verify.c

71–72

This mixed return of values and negative errors isn't especially BSD-like. Linux does a lot of it, but not FreeBSD.

Additionally, it seems like these could be ordinary errnos instead of new, made-up names and values.

82

style: this indent looks wrong

108–110

Ouch.

123

Usually in BSD we would use the queue.h macros here.

129–130

Usually global variables are declared at the top of files.

271

What does "try" mean in this context?

381

kludge?

lib/libve/vets.c

126

this indentation appears to be inconsistent

209

Just remove it. There's no reason to be using SHA1 in novel cryptographic designs in 2018. If you use it in JunOS, keep the SHA1 stuff in JunOS, but I'd suggest moving away from SHA1 there, too.

223

magic numbers

295

Ew. Is this loader-only code?

382

should this be timingsafe_memcmp?

439

then don't commit it

484

large stack buffer

498–501

This is pretty magical

stand/common/module.c

155

style: return (foo)

[sjg]

stand/common/bootstrap.h

328

This bit won't be committed.
Was just allowing testing on a Junos vm.

[sjg]

Address style9 issues

[sjg]

Address style9 issues

[sjg]

lib/libve/h/verify.h

27–28

These defines address how to handle the case that no matching hash is found for a file.
With the exception of the module loading code (load_elf.c) which always passes VE_MUST, the loader generally has no clue what it just opened.

A file which does not have a hash cannot be verified - correct, but should it be accepted?
For some files the answer is yes - eg, we explicitly want to allow for an unverified loader.conf file

lib/libve/vectx.c

125

We are using BearSSL because it provides all the functionality required and is tiny.
This whole implementation adds about 100K to loader (which stops working around 500K - BTX aborts), the necessary code from OpenSSL is > 3M

lib/libve/veopen.c

118

The loader does not deal with significant numbers of files.
Nor does it look at them more than once.

137–138

It is a lot, and I re-worked it at one point to use malloc, but IIRC the heap handling in libsa is very crude, the stack has not (yet) been an issue and makes the code much simpler.

303

Because the manifest records the printed hash.
This makes them simple to generate and simple for admin to read and double check things for themselves.

Nothing here is subject to timing attacks - there's no "authentication" going on.

405–413

Minimal doxygen markup to allow generating api docs.

lib/libve/verify.c

71–72

What errno's do you think would make sense?

271

It means verify if we can, but we really do not expect a hash to be provided.

381

Taking advantage of the fact that the pathname is as much verified as its content.
This allows us to safely communicate tuning info to loader.
Eg. to enable strict enforcement (for FIPS/CC compliance) - panic if self tests failed,
or even turn it all off for data center folk who don't care.

lib/libve/vets.c

223

Yes, I added the comment to indicate that this conversion is from BearSSL.
This btw is only needed for loader which has no real concept of time.

295

No.

382

I don't see that anything would be gained.
There's no secret info to be guessed at.

498–501

Yes, as the comment says this is the DER encoded OID for commonName field
which we want so we can report who signed the file.
BearSSL is a very small library

[cem]

@sjg wrote (email):

In D16155#342583, @cem wrote:

• Why are we using PGP in any new cryptosystem?

Because lots of people in the real world want to be able to use it?

I suspect most people don't really care for PGP specifically. They want the end result — cryptographically verified filesystem manifests.

PGP in particular has a poor security track record.

Like everything else, it is optional.

Poor-quality "optional" modes do not benefit, and sometimes harm, crypto systems (e.g., POODLE, FREAK, Logjam). Why include it when we've already got a better mode?

• Why are we using ASN.1 / x509 — a notoriously difficult scheme to parse correctly — in the kernel or loader?

Because it provides a high level of flexibility.

Flexibility and good crypto system design do not go well together. What you want is the most limited solution that will meet your needs. I'm have not been convinced that a CA system with intermediary authority makes any sense for manifest verification. What's wrong with a single public key?

Ie. I can load a brand new version of Junos on a 10+ year old version,
and it will be able to verify all the signatures.
Unlike some other designs that would force me to install intermediate
versions in sequence.

Is this really something you need to do with any frequency? Note that you'll have similar problems with, say, upgrading from 10 year old web browsers.

• Why the plethora of duplicated functionality (e.g., above)? (Also, the five different SHA hashes. Pick one that's good enough

For the above requirement I need to ensure that the s/w I shipped 10+
years ago, knows how to handle hashes that were not required then.

Again, if you want to keep the options in JunOS, you are welcome to. But for FreeBSD there is no reason to have more than one hash and one signature method.

and just use it everywhere.  SHA512 provides no meaningful benefit
over SHA256, but it also isn't clear that a basic hash is the
right construct anyway.  There is no reason to use SHA384 at all.)

Yes, there is - if/when SHA256 is deprecated.

I don't think this is responsive to my comment. SHA512 uses a more or less identical construction to SHA256. I suspect the entire SHA-2 family would be deprecated at the same time.

Hopefully SHA512/256 will be approved before then.

Approved? What is there to approve? It's just a truncated SHA512 with different initial constants — and still a member of the SHA-2 family.

• What alternatives to x509 or PGP were considered and dismissed? Please make it clear that you've done your research and examined other options.

I gave a talk about this at BSDCan in June.

Feel free to clip a specific excerpt you think is relevant, or please write a summary.

At the end of the day, you can propose and implement an alternate design
and see if anyone wants it.

That's not how review works. It's up to the submitter to meet the bar.

• Why RSA or ECDSA (both have easy ways to foot-shoot) in new cryptosystems, vs something like Ed25519?

Because as a vendor who sells to US Govt I'm limited to algorithms
approved for that purpose.

Maybe a great reason for JunOS, but not really relevant for FreeBSD. The best argument I can come up with for RSA or ECDSA veriexec in FreeBSD is that BearSSL does not support any others.

Some people, especially those in non-US countries, suspect that the NIST ECDSA curves are backdoored by the NSA. Additionally, although this probably does not matter very much, ECDSA signatures are slower to verify than RSA certificates of similar strength. ECDSA is more complicated to implement.

Given that, I'm not sure there's any reason to have ECDSA at all.

• SHA1 should not be used at all.

It is optional for that reason and not enabled by default.
However we are using it and will continue to until NIST ban it.

Again — you can keep it in JunOS. There is absolutely no reason to put it in FreeBSD.

Meta issues:

- Use of strcmp() for signature comparison.  You want timingsafe_memcmp().

In the loader? It is a single threaded app.

Sorry, I did not realize what files correspond to what contexts. It isn't super clear. If signatures are public then leaking them is not a concern.

• Have you thought about wiping key memory before releasing it? I don't see any invocations of explicit_bzero() (or bzero/memset, for that matter).

There are no private keys involved here at all.
Public keys do not need any such treatment - they are not sensitive
data.

Right.

+#define VE_GUESS -1 /* let verify_file work it out */
+#define VE_TRY 0 /* we don't mind if unverified */
+#define VE_WANT 1 /* we want this verified */

Both of these concepts seem pretty dubious.

The loader and kernel should not be guessing signing policy — period.

If you can propose a means whereby every bit of lua and .4th can
communicate to the loader the verification requirements...

Policy should be dictated by the administrator via manifest. No?

Files either have a signature or do not. If they do, they must be
verified. If they don't, they cannot be verified. Not sure what try
or want has to do with that.

Please go watch my talk from BSDCan.
This is all covered.

Again, please point me at a specific clip or excerpt.

Yes, anything which has a hash must verify correctly.

The above all only apply when no hash is found.
Whether that is acceptible depends on the caller - never acceptible for
modules, but may be ok for loader.conf and other files which may need to
be mutable.

I see. The problem with the manifests is that they do not distinguish between "this file must not exist" and "this file may exist, but we don't care about the signature." I don't like this approach, but I don't have a good counter-proposal either; I'll think about it.

lib/libve/h/verify.h

27–28

I'm still not clear on what GUESS or TRY are for. Can you elaborate further?

I think it would be good to add a comment here with the exact semantics differences between the options.

lib/libve/vectx.c

125

I think the answer to my question is — this code is loader only, and therefore cannot use OCF. Is that correct?

I'm definitely not asking to import OpenSSL into loader. :-)

There is a very similar instance of invoking BearSSL directly somewhere else in this diff — I'll try to find it and comment there to clarify context.

lib/libve/veopen.c

118

If it does not need them more than once, why even keep them on a list?

137–138

I'm ok with this if this code is not used in the kernel.

303

It's fine (well, bloats space, which might impact embedded) to a have a text manifest. But maybe we should store the binary version of the manifest contents after parsing?

Something that checks if a hash matches a known value and returns OK or WRONG looks like authentication — can you elaborate on why it isn't? Is it intended that the manifest hashes are public information?

321–325

Is this file also in loader context?

405–413

And? It's still spam.

If doxygen can't tell you what function types and parameters are without the comment, maybe it's not a helpful tool. Other options that don't go stale with source code changes are ctags, cscope, or OpenGrok.

lib/libve/verify.c

71–72

0, ENOENT, ENXIO

271

How is that different from VE_WANT?

It seems weird to me to put this kind of policy in the code instead of, say, the admin-provided manifest.

345

VE_STATUS_VALID?

381

I meant, why are we hardcoding the name (prefix?) "loader.ve." here?

lib/libve/vets.c

223

86400 is 60*60*24 and defining it as such would make the derivation clear. Or #define SECONDS_PER_DAY 86400. Either way.

719528 looks to about 1970 years, so br_x509_minimal_context starts at year zero, I guess. Again, #define X509_DAYS_TO_UTC0 719528 would make that clear.

295

Then please move the storage to callers instead of returning a reference to function-local static storage.

382

Ok

498–501

Surely these could be named constants?

[cem]

Ah, and I should add: NaCl is permissively licensed, supports Ed25519 and SHA-2 hashes, and is similarly suitable for embedding in loader (TweetNaCl without any LTO is about 7.5kB of code and 2kB of data).

[sjg]

Address feedback

[sjg]

lib/libve/h/verify.h

27–28

TRY just explicitly indicate that caller does no mind if file has no hash.
Nothing in the loader actually passes that, everything but load_elf.c passes GUESS which gets turned into TRY or WANT by a call to severity_guess()
WANT means much the same as try, but depending on how strict the loader has told to be, lack of a hash may or may not be accepted.

lib/libve/vectx.c

125

As the title of the review would suggest, this is all loader only.

lib/libve/veopen.c

118

Perhaps a pointer to my slides would help.Adding verification to FreeBSD loader slides

137–138

As I mentioned somewhere, by using an md_image for initial rootfs and having the loader verify that, you can avoid the need for any of this in the kernel

303

We are talking about the loader.
Everything it does is tossed after a few seconds.
Optimal storage is not nearly as important as simplicity.

Everything in the manifest is public information.

321–325

As the title implies this is all loader

lib/libve/verify.c

271

The difference is in value of the tunable accept_no_fp

345

actually something like VE_NOT_CHECKING would be more accurate.
if ve_trust_init returns 0 we cannot verify anything

381

a define could be used - but's only needed in one place

lib/libve/vets.c

223

Sure.

[sjg]

I spent an hour or so this morning responding to cem's comments above, only to find that email responses bounce.
I'm not going to type it all in again, some of your comments are hard to respond to without coming off as antagonistic...

OpenPGP might suck - I don't much care - it is widely deployed and understood, and provides a handy get out of jail card for GPLv3 - which is a big deal as soon as you start signing software.

What you want is the most limited solution that will meet your
needs.  I'm have not been convinced that a CA system with
intermediary authority makes any sense for manifest verification.
What's wrong with a single public key?

Sorry, but a "single public key" might be fine for a personal use box, but does not come close to cutting it for a commercial product that needs to accommodate evolving requirements over an extended lifetime. What happens when you need to revoke that key?

We use X.509 for precisely that reason, and have field tested this on 1000's of devices for nearly 15 years.
And yes it is very common for core routers (that carry the Internet) to run very old versions of Junos, new hardware is the main driver for software updates in core networks.
At the edge of the network, the opposite holds.

Regardless; the design here allows you to just use your single PGP key if you want.
But don't force that design choice on everyone else.
Same holds for choice of hashes - you can choose one - and for the loader that's quite practical.
But others might want to choose differently.

> Hopefully SHA512/256 will be approved before then.

Approved?  What is there to approve?  It's just a truncated SHA512
with different initial constants \u2014 and still a member of the SHA-2
family.

I've mentiontion before that I'm only interested in ciphers, hashes
and such that I can use in systems that need to pass evaluations by
numerous governments, eg FIPS 140-2 and various Common Criteria
protection profiles (which for the crypto usually have FIPS 140 as a
basis even if they change the names).

So for me and and anyone in the same boat, that means NIST (and NSA)
need to "approve".

> Yes, anything which has a hash must verify correctly.
>
> The above all only apply when no hash is found.
>  Whether that is acceptible depends on the caller - never acceptible for
>  modules, but may be ok for loader.conf and other files which may need to
>  be mutable.

I see.  The problem with the manifests is that they do not
distinguish between "this file must not exist" and "this file may
exist, but we don't care about the signature."  I don't like this
approach, but I don't have a good counter-proposal either; I'll
think about it.

Actually you can express both of those.
The "this file must not exist" can be handled by a manifest entry with
a hash that will never match - all 0's for example.
The other is trivial and done extensively in Junos - a file entry with
no corresponding hash.

This manifest design has been field tested for almost 15 years.
We've not found a reason to change it.

[imp]

lib/libve/h/libve.h

30

We spell this _STANDALONE these days.

38

This is kinda bogus. Either just do #else or not. There's nothing else that's going to map to this, so STAND_H won't ever be defined.

[sjg]

Address imp feedback

[sjg]

lib/libve/h/libve.h

30

Sure

[sjg]

Minimize BearSSL srcs compiled into libsa

[sjg]

Simplify config of manifest and signature search

[sjg]

Allow no manifests if testing

[imp]

stand/defs.mk

155

Not sure that this is the right place...

[sjg]

After discussion with imp, breaking this up to a set of smaller reviews:

https://reviews.freebsd.org/D16337 for build options etc
https://reviews.freebsd.org/D16336 for changes to stand/
https://reviews.freebsd.org/D16335 for libve
https://reviews.freebsd.org/D16334 for libbearssl

[emaste]

Little tip, if you use just the short D##### tag Phabricator will automatically add links both ways:

• D16337 for build options etc
• D16336 for changes to stand/
• D16335 for libve
• D16334 for libbearssl

[emaste]

I needed to add -O1 to stand/defs.mk else the loader was too big
to boot.

This is the i386 CSM loader I assume? How about the UEFI loader? Do you have before/after file sizes handy?