Re: [PATCH v2 0/7] crypto: fuzz algorithms against their generic implementation

[Eric Biggers <ebiggers@kernel.org>]

On Sat, Apr 27, 2019 at 03:24:38PM +0000, Horia Geanta wrote:
> On 4/26/2019 7:54 PM, Eric Biggers wrote:
> > Hi Horia,
> > 
> > On Fri, Apr 26, 2019 at 04:35:05PM +0000, Horia Geanta wrote:
> >> On 4/12/2019 8:00 AM, Eric Biggers wrote:
> >>> So far I've tested all generic, x86, arm, and arm64 algorithms, plus
> >>> some PowerPC algorithms.  I have not tested hardware drivers.  I
> >>> encourage people to run the tests on drivers and other architectures, as
> >>> they will find more bugs.
> >>>
> >> I am seeing some errors in caam hardware driver.
> >> They are due to error code mismatch b/w generic algorithm implementation and
> >> what caam driver returns.
> >>
> >> Random skcipher tests for block cipher algorithms are expected to fail when
> >> input size is not a multiple of algorithm block size.
> >> Generic implementation returns -EINVAL.
> >> caam driver returns the status received from HW.
> >>
> >> This probably has to be fixed in caam driver, but I wonder if there's an
> >> agreement on what error code should be returned in every single case (since I'll
> >> have to do a N:M mapping b/w errors returned by HW and errors expected by crypto
> >> API).
> >> Should I take the generic S/W implementation as reference?
> >>
> >> Thanks,
> >> Horia
> > 
> > Yes, use the generic driver as a reference.  I don't understand why you're
> > saying there are so many cases to handle, though.  The only error cases I'd
> > expect to actually be encountered during the tests are invalid input lengths and
> > invalid key lengths, where you should return -EINVAL.  There may be other errors
> There's at least one more in testmgr: -EBADMSG.

AEADs must return -EBADMSG when the authentication tag is wrong, but you said it
was an skcipher algorithm.  Which algorithm are you talking about, exactly?

> 
> > your driver could theoretically produce, but I wouldn't expect them to be
> > encountered during the tests unless there are testmgr, driver, or hardware bugs.
> > 
> Is the error code matching a crypto API requirement or a testmgr requirement?
> 
> I think testmgr doesn't cover all possible failures. Thus if somebody wants to
> make sure the implementation is _fully_ compliant (and not only passing testmgr
> tests), a lot of effort will be required.
> 

Everything testmgr tests for is an "API requirement".  There are also API
requirements that testmgr doen't yet test for, e.g. for skciphers that the
source data is not modified unless it coincides with the destination data.

However with regards to failures, as I see it the only failures which *must* be
handled consistently are those that apply to every implementation.  I think this
only includes cases where the input is bad, e.g. invalid key or message length,
or authentication tag mismatch.  If you also have implementation specific
failures, e.g. your hardware randomly stopped working or something, then for
them you may choose appropriate error codes from errno.h.

> > But remember you must always return a -errno code, not a driver-specific code.
> > 
> Correct, I'll fix this.
> 
> Thanks,
> Horia