SNOW-0000: Move rust to random-access and general polish#8
SNOW-0000: Move rust to random-access and general polish#8sfc-gh-grubin wants to merge 6 commits intomainfrom
Conversation
rust/floe/src/floe_result.rs
Outdated
There was a problem hiding this comment.
Errors cannot have Box in them when this is no-std. I will look for a way around this during this pass
There was a problem hiding this comment.
I just stripped them out and accepted that we won't be able to track errors.
| } | ||
|
|
||
| impl FloeCryptor for FloeEncryptor { | ||
| impl FloeSequentialCryptor for FloeSequentialEncryptor { |
There was a problem hiding this comment.
These names are really close to each other. Could you make them separate? If not, is it necessary that there be a trait at all?
There was a problem hiding this comment.
I find it makes it easier to build higher level constructions which use FLOE as an underlying transformation. For example, I want to create a transforming iterator at some point in the future, and almost all of the logic can be made generic over this common interface.
| * | ||
| * // Normally you'd save your key in some safe location. But for the demo we'll generate a random one | ||
| * // GCM256_IV256_1M is a generally useful chunk size | ||
| * let sample_key = FloeKey::new_random(GCM256_IV256_1M); |
There was a problem hiding this comment.
Note for future: the norm now is that you put these functions behind a default getrandom feature. The fallback is a function that just takes an RNG
| * ``` | ||
| */ | ||
| pub trait FloeCryptor { | ||
| pub trait FloeSequentialCryptor { |
There was a problem hiding this comment.
Why would you want a trait that covers both encryption and decryption? Why a trait at all, and not just a separate API for the encryptors and decryptors?
There was a problem hiding this comment.
See prior note. I find it makes higher constructions simpler. I'm willing to accept that this is a bad idea and remove it though.
rust/Cargo.toml
Outdated
| path="floe/src/lib.rs" | ||
|
|
||
| [dependencies] | ||
| aead = { version = "0.5.2", features = ["std"] } |
…ust (#9) * refactor: Use the chain_update method when deriving a key Since we have a lot of update calls in a sequence it is nicer to use the chain_update method which was created just for such scenarios. * chore: Rename raw to key_array inside the FloeKey::derive_key method Raw is a weak keyword[1] in used for the raw borrow operators[2], so let's avoid that name. [1]: https://doc.rust-lang.org/reference/keywords.html#weak-keywords [2]: https://doc.rust-lang.org/reference/expressions/operator-expr.html#raw-borrow-operators * refactor: Copy secret key material bytes directly instead of using to_owned It is an open secret that Rust will compile a move of a value into a memory copy[1]. While using to_owned on a byte slice isn't a move, copying the bytes directly makes it more obvious that no moving will happen under the hood. [1]: https://benma.github.io/2020/10/16/rust-zeroize-move.html * fix: Zeroize the intermediate secret key bytes in derive_key * docs: Add some documentation for the derive_key method and FloePurpose enum * refactor: Avoid allocations in the FloePurpose::as_bytes method
sfc-gh-grubin
force-pushed
the
03-18-snow-0000_initial_move_to_ra_core_for_rust
branch
from
6153278 to
5a751f6
Compare
3 participants
This PR is the first of several to prepare the rust reference code for more general use.
cargo(required duplicating KATs)Further work includes: