A code audit of the bc-shamir and bc-sskr libraries was conducted in Summer 2021 by Radically Open Security (ROS) to ensure the security of the libraries. The complete report is archived here. Our libraries were released as bc-shamir v0.4.0 and bc-sskr v0.3.2 in response to comments from ROS, to resolve various issues, as discussed below.
What follows is Blockchain Commons' commentary on issues reported by the audit. It is a complete listing of all issues in the audit that were marked as a finding — not marked as non-finding or recorded for documentation.
Blockchain Commons is dedicated to open infrastructure, but we cannot audit our own projects. Instead, we work with partners who are putting Blockchain Commons libraries and protocols into production usage so that they can fund security reviews for products that they are using which have not previously been reviewed. These reviews of the bc-sskr and bc-shamir libraries were funded by Bitmark.
Blockchain Commons' review system shows the power of our open libraries: future companies will be able to depend on our Shamir and SSKR libraries thanks to Bitmark’s contribution, and then they’ll be able to make contributions of their own that will be much less than if they’d had to security review an entire suite of libraries themselves.
The core methodology of Shamir's Secret Sharing provides information-theoretic security: it not only cannot be broken, even if an adversary has infinite computing power, but an attacker can gain no information on the secret. Blockchain Commons' implementation of Shamir's Secret Sharing, and thus also its own bc-sskr library, instead provides Computation Security: although vulnerable to the application of computing resources, it cannot be broken in a "reasonable amount" of time.
This results from the inclusion of four bytes of the HMAC-SHA256 digest of the secret inside each share, which are used to detect the submission of incorrect shares for reconstruction.
Hashes are one-way functions: the secret can not be directly reconstructed from the hash. However this creates an oracle: with a brute-force computational attack, an adversary knowing T-1 shares could use the oracle to verify the correct secret by hashing possible secrets, but notably the oracle still has a false-positive rate of 2^-32. As a result, the cost of brute-forcing secrets interpolated with bc-shamir is still considered too high to be feasible and the number of false positives too overwhelming to be of any use, but the exact parameters are dependent on the number of unknown bits in the secret and the number of shares an adversary holds, as discussed in a possible attack, below.
We consider this a definitional issue. It requires us to state that bc-shamir is Computationally Secure, not Information-Theoretic Secure. Since this is an equivalent level of security to many uses of Bitcoin and other cryptocurrencies that will integrate with bc-shamir, we consider it an appropriate and realistic boundary for the security required by a secret-sharding system.
This is the potential attack that is linked to the fact that bc-shamir provides Computational Security. Attackers with one less than the threshold of shares required for reconstruction (T-1) can reduce the possibilities for the last share, and thus determine the secret, through a brute force attack using the four-byte digest as an oracle. However, as noted, the search space is still very large, and we consider the security reasonable for any secret with an entropy of 128 bits or more.
If adversaries were able to gain partial information of the original secret through side channels, or through some flaw in the design, there would be more danger, but this requires portions of the secret to be leaked, and also for the remaining, unknown part of the secret to be sufficiently small that it could be brute-forced in a reasonable amount of time.
We consider this to be a similar level of threat to any brute force attack. The secret could be discovered with the application of sufficient computing power, but it requires the prior revelation of all but a small amount of a secret before it would be reasonable on either current computers or on any non-quantum computers that could be produced in the foreseeable future, even with the reduction in search space resulting from holding almost-enough shares. Because our Shamir libraries were built to protect high-entropy secrets, we find this threat to be minimal except in the case of a catastrophic failure to protect the original secret, outside the bounds of the use of our libraries.
We have included discussions of these issues to be fully transparent on the security level of bc-shamir and bc-sskr. Nonetheless, the report from Radically Open Security lists this as a "low" threat and notes that it is "mainly of theoretical importance". Although they made proposals to restore the information-theoretic properties that originate with Shamir's Secret Sharing, we are instead adopting their prime suggestion, which is to document the design tradeoff.
The decision to incorporate the fragments of the hashed secret was purposefully made to reduce complexity and share size for the sharding, and to allow the detection of malicious shares, and was weighed against the fact that bc-shamir and bc-sskr would be protecting high-entropy secrets such as seeds that would create an unreasonably high cost for a computational attack. This is a decision that is also mirrored in other popular secret-sharing protocols such as SLIP-39, who address this precise issue in their design rationale, noting that when one less than the threshold of shares are held, the entropy is effectively reduced by 32 bits, which remains sufficiently strong for entropy of 128 bits or more.
When our libraries are used with Bitcoin or any other cryptography scheme that depends on computation security, we consider this to be a non-issue. In the case of Bitcoin, any address or public key can serve as an oracle and doesn't even require the attacker to hold shares, just the address!
If a local attacker forces the swap-out of a process running bc-shamir, it can leak sensitive information to disk.
We are currently choosing to document this because we believe that it is a best practice for any software that handles sensitive information in a virtual memory environment. We certainly agree that information could leak in this situation, if local access on a machine were already attained. However, we generally believe that if someone has local access already to a machine, then the secret has a very high likelihood of being compromised, as that is the exact failure condition that we warn about when using Shamir's Secret Sharing: as the secret is being sharded or as it's being reconstructed, it is very vulnerable to local attacks on the machine where that is occurring.
As a result, we consider this hardening suggestion worthy, but not critical. Though we have considered the use of mlock and munlock from the Sodium library as a methodology to keep sensitive information in memory, we are also looking into other solutions such as Bitcoin Core's sophisticated management of pools of locked memory. Any solutions may be limited on some systems and must thus therefore be managed carefully; further, our use of other libraries such as Libwally, which zero the memory of sensitive information, but which do not lock it, make any solutions we could manage on our own at this point limited.
All other issues reported as a finding by Radically Open Security have been resolved by Blockchain Commons and marked as resolved by Radically Open Security, as documented below. Our libraries bc-shamir v0.4.0 and bc-sskr v0.3.2 reflect this level of resolution.
There was insufficient input checking of the length of secrets to be split, which could lead to out-of-bound writes or segmentation faults. Resolved in bc-shamir PR #31 and PR #34.
The use of memset to sanitize sensitive data could have been removed by compiler optimization. It was replaced with memzero in bc-sskr and bc-shamir commits.
Compiler hardening flags such as -fstack-protector-all and D_FORTIFY_SOURCE=2 were missing. They have been added in bc-crypto-base PR #35, bc-shamir PR #36, and bc-sskr PR #10.
Though it did not create any current dangers, a lack of checking for the length parameter in serialize_shard() could have resulted in out-of-bounds writes in future code. This has been resolved in a bc-sskr commit plus bc-sskr PR #11.
An out-of-bounds write was possible in generate_shard() due to insufficient checking of secret length. This was resolved in a bc-sskr commit plus PRs #11 and #12.
The split_secret() function did not sufficiently check threshold and shard_count parameters. This has been fixed as part of bc-shamir PR #37.
A buffer overflow would occur in hazmat_lagrange_basis if n > 24. This was also resolved as part of bc-shamir PR #37.
Similar to the issue with split_secret, there were insufficient checks for threshold and shard_count in recover_secret(). Resolved in bc-shamir PR #38.