Add ret_size checking in server keystore

[padelsbach]

Fixes finding 125

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #302

Scan targets checked: wolfhsm-core-bugs, wolfhsm-src

No new issues found in the changed files. ✅

[bigbrett]

we want to maintain the following pattern (for ease of inspection, making critical sections as flat and easy to see as possible):

ret = WH_SERVER_NVM_LOCK(server);
if (ret == WH_ERROR_OK) {
    /* do stuff */

    (void)WH_SERVER_NVM_UNLOCK(server);
} /* WH_SERVER_NVM_LOCK() */

Could you ensure this additional error checking still abides by this pattern?

[bigbrett]

this should be macro protected, vs defining a dummy whTest_KeystoreReqSize().

Also pls remove bug# callout in source code.

[bigbrett]

I'm out to lunch as to whether we want to add a unit test for every single bug as this can end up polluting and slowing a test suite, especially given that this one sets up its own harness just to test a few API edge cases.

For example, should every single API test statically allocate a 1MB flash buffer???

I'd like to see us unify the test framework around a few ready-to-use harnesses that unit tests run on top of, depending on what they do and what they need. For example, this function could be refactored to take an initialized server context as a pointer argument, just like the client tests take an initialized client context pointer and then run the tests on top of that initialized context.

That way we could have a whTest_ServerCfg() similar to whTest_ClientCfg() and one of the many server-only tests to be run as part of that would be this one. Or perhaps further subcategorization where this is a "server only API test" where we can put all the various API arg parsing functions. Not sure if you have already done something like that in #313 - havent looked yet.

I think this is fine for now but would like to see this refactored later - pull the server config out into a whTest_ServerCfg() function that does the setup and then runs this test on it.