Repository: hyperledger/anoncreds-clsignatures-rs (language: rust)

Issue:
Inconsistency of OpenSSL secure heap usage for BN
https://github.com/hyperledger/anoncreds-clsignatures-rs/pull/25#issuecomment-1785873783 by @andrewwhitehead says:

> This [`BigNumContext::new_secure()`] defines the openSSL context used in all the BigNum operations, and using new_secure means that it is zeroed out when freed. If there's a need to be compatible with older versions then that could be added as a feature flag, but I can't say that I see the benefit.

but the private keys are stored in regular BNs, not in a context. They may be in a context for some period but would be overwritten almost immediately by other operation artifacts.
Based on that, it seems more important to me to use BN_secure_new calls.
Here is an example:
private key https://github.com/hyperledger/anoncreds-clsignatures-rs/blob/main/src/types.rs#L348
creation of the key https://github.com/hyperledger/anoncreds-clsignatures-rs/blob/main/src/issuer.rs#L869
creation of the p https://github.com/hyperledger/anoncreds-clsignatures-rs/blob/main/src/issuer.rs#L843
allocation of the memory https://github.com/hyperledger/anoncreds-clsignatures-rs/blob/main/src/bn/openssl.rs#L485

Should we update `BN::new()` usage to `BN::new_secure()` for private keys?


Discussion / hints:
Yes, it does appear that we should be using `new_secure` for creating BigNumbers. I had thought that each was allocated within a provided context but that doesn't seem to be the case.

Fix patch:
diff --git a/src/bn/openssl.rs b/src/bn/openssl.rs
index 12c43ee..52aab2d 100644
--- a/src/bn/openssl.rs
+++ b/src/bn/openssl.rs
@@ -34,7 +34,7 @@ impl BigNumber {
     }
 
     pub fn new() -> ClResult<BigNumber> {
-        let bn = BigNum::new()?;
+        let bn = BigNum::new_secure()?;
         Ok(BigNumber { openssl_bn: bn })
     }
 


Test patch: