Skip FuzzRSASignPSS if fuzz key size too small

[dagood]

I happened to notice this error in an example fuzz test run. Easily reproducible locally, unlike the issues with memory we've been running into. It's just an issue with the inputs, not the library's behavior.

https://dev.azure.com/dnceng/internal/_build/results?buildId=2002188&view=logs&j=55c8d132-ed74-525b-1d69-265188317290&t=b5ce8968-48a5-5335-4111-4f857844d28d&l=3380

--- FAIL: FuzzRSASignPSS (2083.31s)
    --- FAIL: FuzzRSASignPSS (0.00s)
        rsa_test.go:126: crypto/rsa: invalid key size
    
    Failing input written to testdata\fuzz\FuzzRSASignPSS\f8e9f0db75c54772972fb2f46f6871a0c2311fc17ad6bcc68cb84c789bdd9e2c
    To re-run:
    go test -run=FuzzRSASignPSS/f8e9f0db75c54772972fb2f46f6871a0c2311fc17ad6bcc68cb84c789bdd9e2c

Also repros without cngcrypto locally:

--- FAIL: FuzzRSASignPSS (0.00s)
    --- FAIL: FuzzRSASignPSS/f8e9f0db75c54772972fb2f46f6871a0c2311fc17ad6bcc68cb84c789bdd9e2c (0.00s)
        rsa_test.go:134: crypto/rsa: key size too small for PSS signature

I dug around for the logic to test for this condition: the check seems to be fairly deep in the crypto methods in a way that I can't see how to reuse here, so I copied out the relevant lines.

I'm guessing we shouldn't check in f8e9f0db75c54772972fb2f46f6871a0c2311fc17ad6bcc68cb84c789bdd9e2c because this isn't really an interesting result, it's just an input validity check we didn't have yet. Or should we check it in anyway so fuzz testing can see an example of this return branch being hit?

[qmuntal]

I would check the error message instead of trying to replicate the length check logic, i.e., if the Sign fails with crypto/rsa: invalid key size or crypto/rsa: key size too small for PSS signature, return and keep going.

This is a common practice in Go tests in cases where the error can't be inferred by its type but just by its message. If at some point the error message change, the test case will have to be adapted, but it is preferable than crypto logic inside tests.

[dagood]

This is a common practice in Go tests in cases where the error can't be inferred by its type but just by its message. If at some point the error message change, the test case will have to be adapted, but it is preferable than crypto logic inside tests.

Thanks, yeah, I wondered about this. It seemed like the precondition logic made more sense because the errors aren't exported and we have two messages to deal with, but that's not too bad if it's expected to require some maintenance when the crypto implementation details change.