Page MenuHomeFreeBSD

Build libbearssl for loader and sbin/veriexec
ClosedPublic

Authored by sjg on Jul 19 2018, 12:30 AM.

Details

Summary

BearSSL is a tiny library suitable for embedded apps like loader.
It provides all the functionality needed to verify
RSA and ECDSA signatures using X.509 certificate chains.

Initially at least BearSSL needs to be checked out externally to the
FreeBSD tree, with the variable BEARSSL set to point to it.

Makefile.libsa.inc is included by stand/libsa and lists only the
sources needed by the loader.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

sjg created this revision.Jul 19 2018, 12:30 AM
cem added a subscriber: bdrewery.Jul 19 2018, 5:22 PM

I have no objection to BearSSL.

I'd suggest getting someone like @bdrewery to review the Make integration changes.

I like the distinction between the full (lib) version and the libsa minified version, although perhaps it could be done in a single Makefile without the repetition. I don't feel strongly about that.

lib/libbearssl/Makefile.libsa.inc
25 ↗(On Diff #45504)

Do we actually use curve 25519? And if so, for what? NIST has expressed intent to bless it for limited applications (ephemeral key exchange, I think) but has not actually published the document, as far as I can tell:

https://csrc.nist.gov/News/2017/Transition-Plans-for-Key-Establishment-Schemes

So I am surprised at its inclusion in the loader code for veriexec, given your previously expressed hostility to anything not blessed by NIST.

40 ↗(On Diff #45504)

As before, I don't think there is any reason to include SHA1.

sjg added inline comments.Jul 19 2018, 8:22 PM
lib/libbearssl/Makefile.libsa.inc
25 ↗(On Diff #45504)

Damn, I keep forgetting that email cannot be replied to...

Not actually using it no, but
the above list is derrived from running objdump on the
loader to see what actually got used.
Looking at nm ec_c25519_m31.o I don't really see why.
Need to dig further, the only ec_ bits libve pulls in are

br_x509_minimal_set_ecdsa(&mc,
    &br_ec_prime_i31, &br_ecdsa_i31_vrfy_asn1);

We are not using anything but p256 curve right now,
that may need to shift to p384 sooner than I'd like
based on recent guidance from NIST.

40 ↗(On Diff #45504)

It is being used for emulating a TPM's PCR - keeping a running hash of
everything we attempt to verify, so that later boot step can feed it to
TPM if doing measured boot.
This avoids a truck load of stuff in loader to talk directly to TPM.

I guess we could do SHA256 for that.

sjg updated this revision to Diff 47709.Sep 5 2018, 8:26 PM

SHA1 required by OpenPGP for computing key id

sjg added a comment.Sep 5 2018, 10:02 PM

Adding xrefs to related reviews

D16337 for build options etc
D16336 for changes to stand/
D16335 for libve
bdrewery requested changes to this revision.Sep 6 2018, 12:07 AM
bdrewery added inline comments.
lib/libbearssl/Makefile
233–235 ↗(On Diff #47709)

Same comment as D16335, I'm not fond of the pattern here as it's non-standard and unneeded.

This revision now requires changes to proceed.Sep 6 2018, 12:07 AM
cem added inline comments.Sep 6 2018, 7:49 PM
lib/libbearssl/Makefile
211–216 ↗(On Diff #47709)

Seems like we're including a lot more of bearssl than we actually need or use? Given we already ship openssl in base, and that bearssl cannot replace OpenSSL directly, does it make sense to provide an alternative SSL library also in base? I don't think it does, but bringing in the limited pieces needed for loader veriexec is reasonable.

sjg updated this revision to Diff 47782.Sep 6 2018, 11:02 PM

remove check for MK_BEARSSL from Makefile

sjg marked an inline comment as done.Sep 6 2018, 11:03 PM

Hello,

Are there any plans to integrate this patch with tree?

sjg added a comment.Dec 5 2018, 10:18 PM

Hello,
Are there any plans to integrate this patch with tree?

Yes, but need +ve review ...

emaste added a subscriber: emaste.Jan 10 2019, 3:42 PM
emaste added inline comments.
lib/libbearssl/Makefile
48–49 ↗(On Diff #47782)

?

lib/libbearssl/Makefile.libsa.inc
2 ↗(On Diff #47782)

A comment explaining Makefile.libsa.inc is intended to provide?

sjg updated this revision to Diff 52838.Jan 14 2019, 9:55 PM

Update to latest BearSSL

sjg updated this revision to Diff 52895.Jan 16 2019, 5:38 PM

Move some i62 methods to Makefile.libsa.inc

sjg accepted this revision.Feb 26 2019, 7:05 PM

this is committed

Still need to close this review

This revision was not accepted when it landed; it landed in state Needs Review.Mar 23 2019, 6:34 PM
Closed by commit rS344564: Add libbearssl (authored by sjg, committed by ). · Explain Why
This revision was automatically updated to reflect the committed changes.
sjg mentioned this in rS344564: Add libbearssl.