Raise specialized WolfCryptApiError for failed API calls.

[roberthdevries]

This exception takes the return value and converts that to a human-readable string (if error string support is compiled in.)

The specialized exception is derived from WolfCryptError meaning that existing users will not notice the difference except for the nicer description.

[JeremiahM37]

Skoll Code Review

Scan type: review
Overall recommendation: COMMENT
Findings: 6 total — 3 posted, 3 skipped
2 finding(s) posted as inline comments (see file-level comments below)

Posted findings

• [Medium] Inconsistent conversion in ECC decode_key: only one of two adjacent size checks was migrated — wolfcrypt/ciphers.py:1267-1272
• [Medium] WolfCryptApiError fed non-error-code values (sizes, lengths) — produces misleading messages — wolfcrypt/ciphers.py:741,904,1269; wolfcrypt/asn.py:56-62
• [Medium] Bug exposure: RsaPrivate.make_key passes wrong variable as err_code — wolfcrypt/ciphers.py:874

Skipped findings

• [Medium] No tests for the new WolfCryptApiError class
• [Low] Behavioral change in _Hmac._init: error condition narrowed from != 0 to < 0
• [Low] Inconsistent conversion in asn.py: make_signature and hash_oid_from_class left as WolfCryptError

Review generated by Skoll

[JeremiahM37]

🟠 [Medium] Inconsistent conversion in ECC decode_key: only one of two adjacent size checks was migrated

In EccPrivate.decode_key the diff converts the self.size <= 0 check to WolfCryptApiError, but the immediately adjacent self.max_signature_size <= 0 check was left as WolfCryptError("Key decode error (%d)" % self.max_signature_size). The two checks are semantically the same kind of guard (both check that a derived size attribute is positive), so leaving one in the old form and not the other is a bug-magnet: future readers will think one of them is wrong. The same pattern (one size check converted, one left behind) was missed in EccPublic.decode_key (lines ~1090-1095), the various Ed25519/Ed448 decode_key paths, etc. — self.size was never edited there but WolfCryptError checks remain. Pick one and be consistent (see related finding about whether these should use WolfCryptApiError at all).

if ret < 0:
                 raise WolfCryptApiError("Key decode error", ret)
             if self.size <= 0:  # pragma: no cover
                 raise WolfCryptApiError("Key decode error", self.size)
             if self.max_signature_size <= 0:  # pragma: no cover
                 raise WolfCryptError(
                     "Key decode error (%d)" % self.max_signature_size)

Recommendation: Either revert self.size <= 0 back to WolfCryptError to match the sibling check (preferred — see API-misuse finding), or convert all the max_signature_size <= 0 checks throughout ciphers.py to WolfCryptApiError. Don't ship a half-converted pair of adjacent guards.

[JeremiahM37]

🟠 [Medium] Bug exposure: RsaPrivate.make_key passes wrong variable as err_code

In RsaPrivate.make_key, after a successful wc_MakeRsaKey, the code re-uses ret (now 0 from the success) as the error code passed to WolfCryptApiError:

ret = _lib.wc_MakeRsaKey(...)
if ret < 0:
    raise WolfCryptApiError("Key generation error", ret)

rsa.output_size = _lib.wc_RsaEncryptSize(rsa.native_object)
rsa.size = size
if rsa.output_size <= 0:  # pragma: no cover
    raise WolfCryptApiError("Invalid key size error", ret)   # <-- ret is stale

The pre-existing code had the same % ret bug, but this PR makes the consequence worse: previously the user saw "Invalid key size error (0)", now they will see "Invalid key size error (0): no error" (or whatever wc_GetErrorString(0) yields), which actively misleads debugging — the message claims success.

rsa.output_size = _lib.wc_RsaEncryptSize(rsa.native_object)
                 rsa.size = size
                 if rsa.output_size <= 0:  # pragma: no cover
-                    raise WolfCryptError("Invalid key size error (%d)" % ret)
+                    raise WolfCryptApiError("Invalid key size error", ret)

Recommendation: Fix the variable: pass rsa.output_size (or use WolfCryptError since the value is a size, not a wc_* return). This is technically a pre-existing bug in the line, but the PR intensifies its user-visible damage by pumping a stale ret=0 into wc_GetErrorString.