Skip to content
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

add a no-std, libc-based example #1534

Closed
wants to merge 6 commits into from
Closed

Conversation

japaric
Copy link
Contributor

@japaric japaric commented Oct 13, 2023

This is a simple no-std rustls demo that, in principle, should run on any RTOS or OS that has a C library that's more or less POSIX compliant.
For convenience, this has been built to run on Linux as that can be tested in CI but it should not be too hard to port to other (RT)OS that provide a POSIX compatibility layer.

blockers:

@codecov
Copy link

codecov bot commented Oct 13, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (078f033) 95.71% compared to head (75fb77a) 95.71%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1534   +/-   ##
=======================================
  Coverage   95.71%   95.71%           
=======================================
  Files          78       78           
  Lines       16071    16071           
=======================================
  Hits        15382    15382           
  Misses        689      689           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@japaric
Copy link
Contributor Author

japaric commented Nov 3, 2023

quick update: this is working 🎉 that is, the low-level connection API works in no-std mode and can talk to a local rustls server. I have switched the app to using ring after doing some updates in #1502 and sorting out some no-std support issues in ring which resulted in upstream PRs ( briansmith/ring#1754 and briansmith/ring#1794 ). this is currently using a bunch of git dependencies ( temporary branches ) but those should eventually become crates-io deps as things get merged upstream and released.

@japaric japaric force-pushed the no-std-libc-demo branch 3 times, most recently from 678f98c to 61cd95f Compare November 14, 2023 18:07
@japaric
Copy link
Contributor Author

japaric commented Nov 14, 2023

I've updated this demo to use the PR #1502 as a git dep (that should eventually become a path dep when that PR is merged) and switched almost all other git deps (pemfile, roots) to rustls repos since my PRs there have already been merged (it'd be nice to eventually switch those over to crates.io releases but I don't think we have to block this PR on doing that)

the ring dep is a bit of a problem. the demo needs to enable one of its Cargo features to mark a custom getrandom implementation as being crypto safe (see briansmith/ring#1754 ) and right now that requires [patch.crates-io] because no crates.io release of ring includes that Cargo feature yet. the PR that adds that Cargo feature and another issue that makes ring not failed to link when compiled to no-std targets (a better version of briansmith/ring#1794 ) have not landed upstream so I'd rather not block this PR on waiting until those ring PRs make it into a crates.io release. also, to use [patch.crates-io] we need to put the demo in its own Cargo workspace to not override the ring dep used in the tests of the root workspace.

an alternative to ring is to use the same rust-crypto-based CryptoProvider that provider-example uses. one problem with that is that the rust-crypto do not compile (+) to x86_64-unknown-none because that target has several instructions sets, like SSE, disabled in its codegen (++). as an alternative compilation target we can use aarch64-unknown-none and that one works fine but to build and run the example on a x86_64 machine you would need to use a C cross compiler and QEMU which makes it hard for most devs to run the example locally. in CI, we could simply run cargo check to avoid having to install a C cross compiler and QEMU there so that's less of a problem.

aws-lc-rs is not an option since it has no no_std support.

thoughts on which route to go down? ring or rust-crypto?

(+) they actually produce ICEs in rustc due to LLVM errors
(++) the target is meant to produce code that'll run in kernel space so everything that increases context switching overhead has been turned off in its codegen

@briansmith
Copy link
Contributor

the demo needs to enable one of its Cargo features to mark a custom getrandom implementation as being crypto safe (see briansmith/ring#1754 )

This patch looks good to me and I'll merge it when it passes CI. (I just started the CI run.)

@briansmith
Copy link
Contributor

another issue that makes ring not failed to link when compiled to no-std targets (a better version of briansmith/ring#1794 ) have not landed upstream

I am happy to help you reoslve the issue. I explained some of the details in briansmith/ring#1793.

AFAICT, the -none targets are designed for OS kernels and similar environments where the normal rules of typical userspace don't apply. This is why both ring and rust-crypto (and maybe even the Rust toolchain itself) require work. libcore doesn't provide all the facilities that we need to detect and safely use the CPU features and ABI we need for good crypto implementation (i.e. one that isn't just using the same integer-only fallback logic). Again, I tried to outline what I know about these issues in briansmith/ring#1793.

Also, I am happy to make crates.io releases to support this work, if it would be helpful

@japaric
Copy link
Contributor Author

japaric commented Nov 15, 2023

another option is to use rust-crypto with a custom compilation target (written as a JSON file) that's basically x86_64-unknown-linux-gnu with os = "none", panic = "abort" and minus libstd in its sysroot (see Cargo's -Z build-std). that would require a nightly toolchain to run the demo but that's better than the aarch64 option which requires a cross toolchain and QEMU. I'll give that a try.

@japaric
Copy link
Contributor Author

japaric commented Nov 15, 2023

OK, this is working with a custom x86_64 target and provider-example as the CryptoProvider. the example can be easily run on x86_64 Linux using a nightly toolchain. The remaining tasks are turning the rustls git deps into path deps (which is blocked by #1502 ) and tidying up the git history but otherwise this is ready for review.

@japaric japaric marked this pull request as ready for review November 15, 2023 12:44
@japaric
Copy link
Contributor Author

japaric commented Nov 15, 2023

(+) they actually produce ICEs in rustc due to LLVM errors

for the record, this has been reported to the rust-crypto folks ( RustCrypto/universal-hashes#189 ) and the rust-lang/rust folks ( rust-lang/rust#117938 )

@japaric japaric changed the title [draft] no-std libc demo add a no-std, libc-based example Nov 17, 2023
@japaric
Copy link
Contributor Author

japaric commented Nov 24, 2023

squashed and rebased to deal with changes in provider-example and the pki_types::ServerName change.

one thing I noticed is that hpke-rs does not have no-std support and it's not going to be trivial to add no-std support since it internally uses std::RwLock in hpke-rs-rust-crypto

@japaric
Copy link
Contributor Author

japaric commented Nov 24, 2023

one thing I noticed is that hpke-rs does not have no-std support and it's not going to be trivial to add no-std support since it internally uses std::RwLock in hpke-rs-rust-crypto

upon a closer look it seems possible to remove it. I opened an issue to discuss the possibility ( franziskuskiefer/hpke-rs#52 ) and send several PRs to move the crate towards being no-std compatible

@japaric japaric force-pushed the no-std-libc-demo branch 3 times, most recently from 81a242f to e47845e Compare November 27, 2023 16:21
@briansmith
Copy link
Contributor

with a custom compilation target (written as a JSON file) that's basically x86_64-unknown-linux-gnu with os = "none"

I am all for incremental progress. The main issue I see with your "fake" -none target is that it allows the crypto libraries to depend on features of Linux that aren't guaranteed in real -none targets. There are a surprisingly large number of issues with the real -none targets that don't even have proposals for solving them yet. So I hope we can all work together to get to a workable state.

@japaric
Copy link
Contributor Author

japaric commented Nov 27, 2023

in real -none targets.

these examples were never meant to be run in a bare-metal, no/pre-OS environment (there's a different demo in the works that runs on a Cortex-M microcontroller). they are meant to run on OSes that have a POSIX layer and #![no_std] is used here to achieve some degree of OS portability (the example is not meant to run everywhere without modification). ideally we would have an os = "any" target built into rustc to use here but we don't so we a custom target.

There are a surprisingly large number of issues with the real -none targets that don't even have proposals for solving them yet

I agree but those ought to be addressed in rustc because matching on target_os or the target triple is not going to scale; there's a bunch of OSes that rustc does not know about and people / companies haven't stopped making / rebranding OSes (e.g. Eclipse ThreadX) .

I see that you have jumped into the discussion in rust-lang/rust#117938 ; at least it seems the problem got some renewed attention by the rust devs.

@japaric
Copy link
Contributor Author

japaric commented Nov 28, 2023

some updates here:

  • I've added a server example (turns phase I of no-std-support is enough to get that working)
  • submitted a bunch of PRs adding no-std support to hpke-rs (a few have already been merged)
  • broke out a commit into PR make the provider-example library no-std compatible (almost) #1636
  • reverted to putting no-std-libc-demo into its own workspace. putting it in the root workspace makes cargo any-command --workspace fail because something (+) injects a libstd dependency into ministd, which defines its own #[panic_handler], and that produces a 'duplicate panic_impl lang item' error. given that no-std-libc-demo uses nightly and must be compiled to a custom compilation target there's not much sharing of binary artifacts in the target directory so not having it in the root workspace does not seem like a terrible outcome (plus fuzz is also not in the root workspace so there's precedent for leaving problematic crates out of the root workspace)

(+) test runners are the usual suspects in this case but disabling all of them in Cargo.toml did not fix the issue. I suspect that getrandom or rand_core is injecting the libstd dependency due to Cargo feature union and its use in other parts of the workspace

@briansmith
Copy link
Contributor

they are meant to run on OSes that have a POSIX layer and #![no_std] is used here to achieve some degree of OS portability

OK, I understand better what you're trying to do.

Right now my plan in ring is to use std::arch for feature detection for every operating system except ones for which I have special requirements (Linux and a couple other ones), and then require every operating system to at least stub about libstd, as that's the only portable solution we have today. This way ring will "support" every operating system without the allowlist. I believe the state of libstd today is such that, especially if you have a libc and are POSIX-ish, it is easy to provide a libstd, perhaps one that just has some facilities like fs broken, so this seems sensible. Under this thinking, -none targets are exclusively the domain of targets that don't have typical userspace/kernel separation and/or where we're targeting the OS kernel itself.

Copy link
Member

@cpu cpu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For someone not very familiar with no-std Rust or the embedded space generally this is an interesting branch 😅

Is the ministd glue for the no-std demo something you see other comparable projects maintain in-tree? It's a pretty sizable chunk of code and the parts that are sourced from upstream rust are going to drift over time. However, I also see the value in having a really solid no-std demo to keep the feature working so I don't think it's unreasonable or anything. Just hoping to draw more from your experience here when weighing the tradeoffs.

The OS bindings have been written to run this demo on Linux.
Other OSes have not been tested and may require modification to the bindings in `packages/ministd/src/libc.rs`

As a way to check the FFI bindings, one can run the examples in the `ministd` packages -- there no regular `#[test]`s because the `test` crate is not available in no-std context.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small typo:

Suggested change
As a way to check the FFI bindings, one can run the examples in the `ministd` packages -- there no regular `#[test]`s because the `test` crate is not available in no-std context.
As a way to check the FFI bindings, one can run the examples in the `ministd` packages -- there are no regular `#[test]`s because the `test` crate is not available in no-std context.

@@ -0,0 +1,20 @@
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the uninitiated, how do you arrive at a file like this? The README mentions this is "an OS-agnostic version of x86_64-unknown-linux-gnu", does that mean you source the x86_64-unknown-linux-gnu target configuration from upstream rustc? If so maybe link that in the README. It would also be helpful to know what needed to be customized to achieve agnosticism compared to standard.


entry!(main);

fn main() -> Result<(), Error> {
Copy link
Member

@cpu cpu Dec 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving a note that it would be helpful once #1583 lands to do a consistency pass between this example (and the later server one) vs the ones that landed in 1583. I think there are a few updates from feedback that would apply here as well.

(and I skipped any of the boring feedback about import style on the assumption that stuff can be sorted out later)

@cpu
Copy link
Member

cpu commented May 16, 2024

In the effort of keeping our pull request tracker focused on work in progress I'm going to close this for now. The current consensus on the team is that the ministd glue is too much to maintain in-repo. Were this to be revived I think we would want to see it done in a separate repository.

Thanks all,

@cpu cpu closed this May 16, 2024
@cpu cpu mentioned this pull request May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants