Subject: Re: DIAGASSERT in cryptographic functions.
To: None <tech-userlevel@netbsd.org>
From: Luke Mewburn <lukem@cs.rmit.edu.au>
List: tech-userlevel
Date: 09/17/1999 18:17:00
Frank van der Linden writes:
> On Fri, Sep 17, 1999 at 12:02:11AM +0200, Christoph Badura wrote:
> > ISTR Luke saying something about not wanting to have critical programs
> > dump core on him.  One possible solution is to fork() and let the child
> > dump core.
> 
> Well, I hope Luke will speak up about this himself, but... If you have
> a critical program, and you link it with the DIAG library, that would
> imply to me that you suspect something is wrong. So it's better to
> have it bail out at that point, leaving evidence, then having it
> continue in an unknown state, which is certainly not what you want
> from a "critical program".
> 
> In any case, continuing where it isn't expected, leading to undefined
> behaviour, is never a good thing.


The original reason for not dumping core during my implementation
phase was because if I was too aggressive in adding _DIAGASSERT()s
and my shell starts dumping core, it makes things a bit hard to debug.

The current implementation of _DIAGASSERT just does
	fprintf(stderr,    "assertion ... failed");
	syslog(user.debug, "assertion ... failed");

I plan to make it user configurable (with $DIAGASSERT ?) so that the
user can specify one or more of the following actions to occur, in
the order given in the environment var:
	fprintf(stderr,    "assertion ... failed");
	syslog(user.debug, "assertion ... failed");
	abort()
	nothing

This is on my list of things `to do'.


Back to the issue of the hash/md routines. Unfortunately, these
provide no way of feeding back to the caller that the arguments are
invalid. There's a few other routines in the libraries with similar
attributes.

I've gone through the library looking for funtions which just `return;' 
if the diagnostic checks fail, and added `XXXLUKEM' to them. A diff is
available at:
	ftp://ftp.netbsd.org/pub/NetBSD/misc/lukem/diagassert.part2.diffs


Depending on the routine, on of the following should be done:

	* replace the return with abort().
	  The hash/md functions should probably have this done.

	* remove the /* XXXLUKEM */ comment; it's ok to return in this
	  case.  I effectively did this for functions which do stuff
	  like free a list; it really doesn't matter if the top-level
	  pointer is checked against NULL because the invoker doesn't
	  care anyway.

Opinions/thoughts?