Make the random generator of _Rsa and RsaPublic configurable.

[roberthdevries]

This was the only class that has hardcoded the random number generator in the implementation.

[dgarske]

Okay to test. Contributor agreement on file.

[dgarske]

@roberthdevries please resolve the merge conflict. Thanks! Awesome work on these cleanups.

[roberthdevries]

Merge conflict is now resolved.

[dgarske]

🐺 Skoll Code Review

Overall recommendation: REQUEST_CHANGES
Findings: 3 total — 3 posted, 1 skipped

Posted findings

• [High] Mutable default argument rng=Random() in _Rsa.__init__ — wolfcrypt/ciphers.py:653
• [Medium] RsaPrivate.__init__ not updated to accept/forward rng parameter — wolfcrypt/ciphers.py:849-851
• [Medium] No tests for the new rng parameter — tests/test_ciphers.py

Skipped findings

• [Medium] No tests for the new rng parameter

Review generated by Skoll via openclaw

[dgarske]

🟠 [High] Mutable default argument rng=Random() in _Rsa.__init__
🚫 BLOCK bug

The default argument rng=Random() is a classic Python mutable-default-argument bug. Random() is evaluated once at class definition time, and the same object is reused for every call that omits rng. This means all RsaPrivate instances (whose __init__ calls _Rsa.__init__(self) without providing rng on line 851) will share the exact same Random object. If one instance is garbage-collected and its Random object cleaned up, every other instance sharing that default will hold a dangling reference. Even without cleanup issues, sharing a single RNG across independent key objects is a correctness and isolation problem. Ironically, the RsaPublic subclass does this correctly with the rng=None / if rng is None: rng = Random() pattern — the base class should use the same approach.

Suggestion:

Suggested change

	def __init__(self, rng=Random()):
	def __init__(self, rng=None):
	self.native_object = _ffi.new("RsaKey *")
	ret = _lib.wc_InitRsaKey(self.native_object, _ffi.NULL)
	if ret < 0: # pragma: no cover
	raise WolfCryptError("Invalid key error (%d)" % ret)
	if rng is None:
	rng = Random()
	self._random = rng

[roberthdevries]

The suggestion is applied (not verbatim), but equivalent.

[dgarske]

🟡 [Medium] RsaPrivate.__init__ not updated to accept/forward rng parameter
💡 SUGGEST api

The PR's stated goal is to make the random generator of _Rsa and RsaPublic configurable. However, RsaPrivate.__init__ (line 849) was not updated to accept an rng parameter, and its call to _Rsa.__init__(self) on line 851 still relies on the default. Since RsaPrivate extends RsaPublic, users would reasonably expect RsaPrivate to also support a configurable RNG. Additionally, RsaPrivate.make_key (line 829) and RsaPublic.from_pem (line 718) also don't forward an rng parameter, leaving the configurability incomplete.

Suggestion:

Suggested change

	idx[0] = 0
	def __init__(self, key=None, hash_type=None, rng=None): # pylint: disable=super-init-not-called
	if rng is None:
	rng = Random()
	_Rsa.__init__(self, rng) # pylint: disable=non-parent-init-called

[roberthdevries]

The rng parameter is also added to the class RsaPrivate and passed to _Rsa.__init__()