mcginty / snow

A Rust implementation of the Noise Protocol Framework
Apache License 2.0
884 stars 120 forks source link

no_std #62

Open aep opened 5 years ago

aep commented 5 years ago

we're migrating away from rust, but one of the things that's holding me back is snow, since it's fairly high quality and noise-c is .. not.

how do you feel about supporting no_std? That is, i'll do the majority of the work of course, but you'd have to maintain and defend it later :D

mcginty commented 5 years ago

I am very in favor of supporting no_std, targeting the 0.7 release. I started researching a bit removing our extensive use of Box. So far it's been promising, and would love feedback and collaboration on the architecture for this change.

aep commented 5 years ago

nice! intermediately, you can just use the alloc crate, which offers Box.

but reducing heap in general is very welcome

aep commented 4 years ago

this is more difficult than expected because cargo will pull in rand with dependencies anyway because its specified somewhere down the default resolver dependencies

carving out the default implementation into a separate crate would solve this. would you be ok with it?

tarcieri commented 4 years ago

@aep are you running into one of these issues, perhaps?

https://github.com/rust-lang/cargo/issues/4664 https://github.com/rust-lang/cargo/issues/4866

aep commented 4 years ago

yes! its criterion. that's actually easier to fix then.

with this diff it works https://github.com/aep/snow/commit/78426ea780231b36b133f767514ba36c5b55630f

all of these are upstreamable, except removing criterion, but this isn't even strictly necessary. would be nice to have CI complain when someone breaks no-std tho

tarcieri commented 4 years ago

@aep aah yep, I've run into that a lot with criterion specifically.

One option is to make a benches (or thereabouts) crate and make it part of the same workspace. This should (hopefully) work around the spurious feature activation, and cargo bench --all will still work

mcginty commented 4 years ago

That change (making separate benches crate in the same workspace) sounds fine to me if it allows us to be no_std compatible :).

nickray commented 4 years ago

@aep are you still planning to do any work on this? I am interested in Snow for a variant of PIV I'm implementing on Cortex-M33 microcontrollers, where I'd like to use Noise to establish Secure Messaging. Ideally, I would like to avoid alloc, so I'm wondering whether it makes more sense to modify Snow to be alloc-free, or rather to reimplement a subset in a separate library.

@mcginty how's your research going? One way I'd hope that Box<dyn Hash> and friends could be replaced is with generics, but it's a bit hard to get rustc to work this way. Specifically, I tried having multiple backends in a microcontroller Ed25519 library I'm working on, but ultimately gave up and used feature flags, meaning algorithm choices need to be made at compile time. Another approach would be doing what enum_dispatch does.

tarcieri commented 4 years ago

@nickray I'm interested in doing something quite similar! (not PIV or that class of token, but trying to encrypt a channel to a USB device)

One way I'd hope that Box and friends could be replaced is with generics

I'd suggest using traits from the RustCrypto project:

They're object safe and widely supported. There's an open PR for ring-digest, and I'd like to make a similar wrapper for using ring with the Aead trait. It'd also let you use e.g. the chacha20poly1305 and aes-gcm crates, which are pure Rust and should be both small and embedded-friendly.

I see how the current type erasure-based API is convenient. I'm curious if it might make sense to split the API between a monomorphizing version with generic parameters and one that uses Box/type erasure.

Perhaps, with default generic parameters, it'd be possible to make the Box-based API a default generic type? e.g.

struct Encryptor<C: Cipher = Box<dyn Cipher>> { ... }

I've never tried that before but it seems like it could work?

Otherwise it seems like you could split the generic vs type erasing structs into two different ones, and have one wrap the other.

aep commented 4 years ago

@nickray unfortunately no. after 5 years of trying to do rust on embedded we have decided that it's not the right tool. Reimplementing noise in zz (https://github.com/aep/zz) was a more effective path than maintaining the hacks necessary to get the last remaining rust code (this crate) working.

tarcieri commented 4 years ago

Note: I made a ring-aead crate for using the aead::Aead trait with ring's cipher implementations:

https://github.com/RustCrypto/AEADs/tree/master/ring

benma commented 4 years ago

unfortunately no. after 5 years of trying to do rust on embedded we have decided that it's not the right tool

@nickray could you please elaborate a bit on why it wasn't the right tool for you?

nickray commented 4 years ago

That's @aep :)

aep commented 4 years ago

@benma http://aep.github.io/from-rust-to-zz/ :)

benma commented 4 years ago

Thanks. For what it's worth, async/.await seems to work nicely now on no_std / embedded, after https://github.com/rust-lang/rust/pull/69033 was merged. Then I guess it depends on what kind of system you are targeting, some might be better supported than others. With FFI, one can still reuse all the SDK code, no need to rewrite everything in Rust.

aep commented 4 years ago

Yes that's ferrous, not the rust devs, trying to make rust work on embedded. They're doing a great job.

i don't think that's the right place to discuss this. everyone in the github issue gets notifications. happy to continue on twitter, reddit, or in a new issue in the zz repo.

benma commented 4 years ago

How is progress going on this issue? Is 0.7.0 still the target for no_std support (but with alloc)?

aep commented 4 years ago

@benma btw if I get your use case right this might relevant: snow used to support hacl-star which is formally verified high quality crypto by Microsoft research and works fine on embedded.

benma commented 4 years ago

Thanks @aep . With custom patches? I am using a no_std environment, which does not seem to be supported yet.

aep commented 4 years ago

no, i never got rust really stable.

but since it works for you, you might want to use the snow hacl-star resolver which itself is F* not rust, so works fine on embedded. was removed from snow some while ago, probably in an effort to use all rust dependencies

mcginty commented 4 years ago

HACL* support was removed because of a correctness issue in the snow's wrapper for it, but it could be easily added back in. I removed it in the mean time because I didn't want to keep code in snow that I wasn't confident in.

aep commented 4 years ago

Could you tell more about the correctness issue?