Closed HeroicKatora closed 5 years ago
Thanks. Definitely we need to improve the documentation.
The bigger learnability problem with the AEAD interface, IMO, is actually that the API is hard to use. Thus, I've avoided documenting the current API because I want to replace the API with one that's easier to use and then document that new API.
I agree that documentation should be a focussed effort after a clear interface has been established.
This would also prevent outdated or unclear examples, which might end up giving a false sense of security. For example, it was also noted that some examples in other libraries are using unauthenticated crypto or defaulting to PKCS padding. Those weaker primitives are then carried over into actual code by inexperienced developers or because their downsides are not clearly marked for the sake of keeping the examples simple. The last point is, imho, a smell that the api could be better.
Without getting side tracked, such example documentation could also be a kind of Litmus test on the usability of the interface.
There are other interfaces that could use polish. Many come from personal experience in different contexts:
digest::Digest
only exposes as_ref
as an interface to comparing hashes. Comparison to other Digest
values should be offered. In addition, the documentation for as_ref
should refer to constant_time
for the cases in which direct usage is not feasible. Not needing the security of constant time for the rather minimal performance improvements should be a conscious choice.
The previous point holds true especially for hmac
which has a very similar interface. The verify
method makes sure that in simple cases the comparison is safe. But it takes a &[u8]
, subverting the type level assurance of the other interfaces. As a proposal, I think an associated method on Signature
with that job might offer the best protection against mistakes.
It is also weird that SigningContext
exists but VerificationContext
does not. In combination with the above, this could make it likely that a comparison of Digest
values is necessary for a multi step signature.
The pbkdf
interface does surprisingly not offer the high-level digest abstraction as well. On the bright side, it nudges users into using verify
by default. It could be nice to link to the methods as they appear on the bottom of the page.
The example in pbkdf
fills more than a page and only very few methods actually come from its interface. While the complete example is nice because you can run it standalone, a shorter version would offer better integration into other projects directly from the example code.
Documentation on pbkdf::{digest,verify}
could be more explicit on parameters. Especially mentioning a mildly sane default for iterations
and the reasoning behind is important from a security perspective. Side note, the last parameter of verify
is called previously_derived
but the documentation still refers to it as out
in the Panic section.
Edit: this grew larger than initially expected. Maybe we should split this and track it separately.
Thanks. I agree with the idea of #671 and I hope to get to that soon.
I'm closing this issue. Rather than have a tracking issue tracking the need for better documentation, let's just have people write specific PRs that improve the APIs and documentation one at at time.
See https://arxiv.org/abs/1806.04929 , investigating cryptographical implementations in Rust.
It concludes, very briefly, that users of ring produce less unsafe cryptography due to its API but rust-crypto users are more efficient due to its extensive examples.
Also, some interfaces do not comprehensively document all of their parameters. As an example, the paper notes that the
nonce
andad
parameters provide no explanation thus relying fully on external gathering information.Since this library seems to have an edge on the technical side, I expect that we can improve the documentation to at least the level provided by other libraries.