-
Notifications
You must be signed in to change notification settings - Fork 200
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RustCrypto misuse resistance recommendation #932
Comments
It would be helpful if you could familiarize yourself with the history of why the APIs are the way are before making such suggestions. Many of your suggestions are more or less "simply delete the potentially misused functionality" without understanding why it exists in the first place, which makes the suggestions somewhat unhelpful. Others are quite vague. The implementations in these crates generally started with misuse resistance in mind, notably we use complete formulas, opt for constant-time per default, disallow representations of invalid points/field elements, and have tried to file away power-user functionality in ways it isn't easily abused. In many cases we struggled for several years to avoid adding hazmat APIs, only to add them after repeated requests and observations of how misuse-resistant APIs weren't actually creating better outcomes in practice.
Variable-time inversions were added to accelerate ECDSA verification. See e.g.: #743 There is a common pattern here that operations on public values can be accelerated by performing them in variable time which is frequently leveraged by many libraries which implement public-key cryptography. We intend to double down on this in the next set of releases, using wNAF form for things like ECDSA verification. We follow a similar pattern in the
The main notable functionality here is signing/verifying prehashed messages. This can lead to a forgery attack if a prehashed message is accepted directly for verification. There is a very long history of trying to avoid exposing such APIs which we eventually gave up on: RustCrypto/traits#1099 Notably there are many constructions which cannot properly be expressed without such an API, such as various bespoke tree hashes. It also greatly assists runtime dynamism, as we just encountered implementing a TLS provider backend: RustCrypto/rustls-rustcrypto#3 (comment)
HKDF is a stateful extract-and-expand construction. The extract step is effectively a constructor for the stateful expander, similar to a XOF. We provide the functionality to initialize HKDF from the ECDH shared secret. Once constructed, the instance works like any other instance of HKDF. Exactly what protocol of expansions they'd like to follow is a caller concern. |
Thank you for the explanation. |
Could you please consider the following interface improvements?
hazmat.rs
with a reasonable warning comment stating the potential harm of non-experts using it. Could you modify this class to private and restrict user access?elliptic-curve/src/ecdh.rs
, which essentially represents a wrapper forHkdf::extract
to obtain key material. A comment stipulates that one should callHkdf::expand
to obtain output key material.Could you please advice why a wrapper for extract exists but not for expand? Furthermore, the differentiations between the two keys is not explained. In light of this, one can advise providing a greater volume of high-level interfaces, as achieved by libsodium13 for instance.
Unfortunately, all design decisions a developer implements whilst using a library may incur associated security flaws; these two factors will increase in tandem.
The text was updated successfully, but these errors were encountered: