Geoff Thorpe wrote: > There is a patch that illustrates how I've been going about the crypto/bn/ > "audit" that can be browsed/downloaded at; > > http://www.openssl.org/~geoff/bn_debug.diff > > The comment in the bn.h header changes explains what the basic idea is and > of course the macro definitions there define what's going on. However, I > wanted to add a few extra notes in no particular order, in case someone > else wants to take a look. BTW: this is all relative to the head of CVS > (so tonight's snapshot at ftp://ftp.openssl.org/snapshot/ should also be > fine). > > - I've had to turn off the "BN_DEBUG_RAND" stuff in the macros because > this already exposes some non-trivial problems in bn_div.c and (I > think) BN_rshift(). So much of this code is only used in certain > configurations, so use of assembly (or "long long") may not trigger it, > but the "debug-geoff" target certainly does. ;-) > > - I've mostly been using "make test" (and specifically, ./test/bntest) > > - As/when the code can handle BN_DEBUG_RAND turned on, the same "noise" > trick should be used in bn_correct_top() so that there are more code > paths trying to trigger bugs. > > - The macros are all kept together in bn.h, which includes > bn_check_top() which was taken out of bn_lcl.h, but eventually these > should all go back in bn_lcl.h. > > - There are other things to fix/change - including making the existing > macros and functions stop tolerating two representations of zero, this > may highlight more crappy code elsewhere. But one thing at a time ...
We don't care whether they tolerate them, do we? We just care that they don't create them. I understand that making them intolerant is a debugging aid, of course. > - I think I've got most of the function exit paths covered with > bn_check_top() macros, though there may be those I've missed. Might be good to check function entry (at least in debug mode). > - I've not even begun to look at API macros that should have this > checking included, no doubt that could turn up some more bugs too. > > - The bugs that've been found (and fixed) so far are already committed, > but I suspect there's more coming out ... if any of us can bare to dig > into the code, that is. > > - It would be easiest to put this stuff in to CVS, as it will make > collaboration easier and may give some people less reason to ignore it. > Any objections? Go for it. Cheers, Ben. -- http://www.apache-ssl.org/ben.html http://www.thebunker.net/ "There is no limit to what a man can do or how far he can go if he doesn't mind who gets the credit." - Robert Woodruff ______________________________________________________________________ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
