Asserts considered harmful (or GMP spills its sensitive information)

[Jeffrey Walton]

Thanks Marco. Comments inline.

On Thu, Jan 3, 2019 at 2:55 PM Marco Bodrato <bodrato at mail.dm.unipi.it> wrote:
>
> Il Lun, 31 Dicembre 2018 7:03 pm, Jeffrey Walton ha scritto:
> [...skipping opinions...]
>
> > Here's a small example of triggering an assert using the Nettle
> > library.
>
> This absolutely is NOT a "small example", it requires to build two entire
> libraries!

Well, if you can let us know how to reduce it further then we would be
delighted to hear it.

I thought it did a good job because it did not muck with your system,
and it used independent data provided by someone familiar with the
library. That is, I did not craft a sneaky test case to make a point.
And a 'rm -rf /tmp/gmp-test' is all that's needed to remove it.

> > ARM A-32 does not work at the moment due to GMP build errors.
>
> Can we suggest you to read the GMP manual on how to build the library?
> GMP works fine on many ARM configurations we test and there are lots of
> projects out there (eg. many GNU/Linux distributions) that builds GMP for
> different ARM processors.

Here's what I witness on a BananaPi and a couple of other boards. Can
you provide info on the ARM boards you are using? I have about 8 of
them for testing, and I may be able to duplicate your [successful]
result.

bananapi:~$ ./test-gmp.sh
--2019-01-03 15:07:11--  https://ftp.gnu.org/gnu/gmp/gmp-6.1.2.tar.bz2
...

gcc -std=gnu99 -c -DHAVE_CONFIG_H -I. -I.. -D__GMP_WITHIN_GMP -I..
-DOPERATION_lshift -I/tmp/gmp-test/include -DNDEBUG -g2 -O2
-march=native -fPIC -Wa,--noexecstack tmp-lshift.s -fPIC -DPIC -o
.libs/lshift.o
tmp-lshift.s: Assembler messages:
tmp-lshift.s:106: Error: selected processor does not support ARM mode
`vdup.32 d6,r3'
tmp-lshift.s:108: Error: selected processor does not support ARM mode
`vdup.32 d7,r3'
tmp-lshift.s:114: Error: selected processor does not support ARM mode
`vshl.u64 d18,d19,d7'
tmp-lshift.s:120: Error: selected processor does not support ARM mode
`vshl.u64 d4,d19,d6'
...

> > In the case below Nettle is using benign data and not maliciously
> > crafted data.
>
> I'm sorry, but your analysis was incorrect.
>
> I agree, Nettle is not using "maliciously crafted data", but I do not
> agree when you say that it "is using benign data".
>
> With your build options, Nettle calls the GMP function mpn_sec_powm with
> an invalid parameter: ebn = 0.
>
> Because of an error in the Nettle library you built, GMP receives "non
> benign data". To avoid further memory corruptions, GMP aborts.
>
> Thanks to this behaviour of GMP, you was able to catch the incorrect built
> of the library using it. ;-)
>
> Using mpn_sec_powm with an exponent of zero bits is obviously a nonsense,
> and in general the documentation of GMP clearly says that arguments of
> size zero are not supported.
>
> On GMP side, we can only specify even more explicitly in the documentation
> of that function the need for non-zero sized arguments.

Returning a failure from mpn_sec_powm would be a most welcomed
improvement. It would be a welcomed improvement if GMP does it in
other places, too. Crashing is least welcomed behavior for many uses
cases, including those where availability and confidentiality is a
concern.

Gracefully handling failure serves several purposes. First, returning
failure is what developers expect to happen. If a program uses a
function incorrectly then it is expected to fail. Developers are
usually good about checking return values at call sites.

Second, when GMP crashes it is setting a policy for the application.
This is ass-backwards - the application sets its own policies, not
libraries. Only the application knows the requirements to  dictate
runtime behaviors.

Related, even GMP calling exit(-1) rather than abort() is GMP setting
policies. GMP does not know what the policies and requirements are, so
it is not in a position to dictate behavior.

Third, it improves Availability in CIA. A crashed service or app does
not service requests, so there is no availability.

Fourth, it ensures Confidentiality in CIA. A core dump with sensitive
information leaks information and violates security policies. Crashing
results in sensitive information leave's the app's security boundary,
is written to the file system and is sent to platform provider .

Finally, both returning a failure, exiting, and crashing preserves
Integrity in CIA. However, there are too many tangential problems with
exiting or crashing.

Earlier I said, "A core dump with sensitive information leaks
information and violates security policies". I've worked in US DoD, US
Financial and US gov on security architecture teams. I've read a lot
of security policies and helped write a few. I can unequivocally say
no organization would allow sensitive information to leave the
security boundary without proper controls, including crash dumps
(crash dumps are just another egress point or dataflow). That
application or library would be rejected and sent into risk
acceptance.

And for completeness, I personally adore asserts. I use them liberally
in my programs because they create self debugging programs. I spend
next to no time under a debugger because asserts are such a
tattletale. But come production/release time, the time for debugging
is over. Asserts are not present in production code because I don't
depend on crashes to help keep my safe and program secure.

Jeff