Add support for Redox #901

AdminXVII · 2019-09-29T15:27:55Z

Add redox to the urandom for src/rand.rs. Use the rand: URI on Redox rather than /dev/urandom.

jackpot51

This looks good to me!

jackpot51 · 2020-05-21T19:00:27Z

@briansmith Any news on this?

briansmith · 2020-05-21T21:22:47Z

@jackpot51 Does redox provide an API that's more like getrandom() in Linux? PRNG APIs that require file handles are really dangerous and a last resort option. It seems like you are a leader of the Redox project; if so, I would be happy to discuss this with you on a call or via email.

jackpot51 · 2020-05-21T23:25:31Z

At the moment, no. Why are file handles considered dangerous?

briansmith · 2020-05-21T23:39:06Z

At the moment, no. Why are file handles considered dangerous?

POSIX requires something along the lines of "The file descriptor returned by a successful call [to open] will be the lowest-numbered file descriptor not currently open for the process." I'm guessing Redox has a similar constraint.

The main problem I'm aware of is when somebody accidentally closes the file descriptor that the crypto library is assuming to be valid for rand:, and then opens a different file with that file descriptor. Then the crypto library will read from that other file instead of rand:. For example, let's say the crypto library opens rand: and gets file descriptor 3. Then somebody loops through all open file descriptors and closes them (I have seen this happen in real programs), and then opens three files. Now ring will still read from file descriptor 3 but that will be the wrong file.

This issue comes up in almost every crypto library. See https://bugs.python.org/issue21207 for example. Thus I recommend every operating system provide a syscall like getrandom() instead of relying on a file interface.

jackpot51 · 2020-05-21T23:44:34Z

Redox is a microkernel. By design most things go through file handles.

briansmith · 2020-05-21T23:56:37Z

Redox is a microkernel. By design most things go through file handles.

I understand the appeal of the design that's been chosen so far. However, I do want to emphasize this is a significant downside. Zircon is a microkernel-based OS too, and provides a getrandom()-like API called cprng_draw.

Going back to ring, I am hoping to soon deprecate and/or remove the fallback to the file-based approach on other operating systems because I see this as too much of a hazard based on the real-world failures I have seen. Obviously, programs/libraries who close the file descriptors they shouldn't close are severely broken. However, it's really unfortunate when a library like ring, that is otherwise able to protect itself very well, has no defense against this. So I hope a new syscall interface for Redox is considered.

That said, I'm not going to use this PR as a means of forcing the issue. When I loop back around to adding support for new targets I will merge this as-is is there is no better alternatives.

briansmith · 2020-05-21T23:56:59Z

A bigger issue that is likely to slow this down: I don't want to merge support new targets for which we do not have CI/CD coverage. So first I will need to learn how to set up a CI/CD environment that runs on a Linux host. IDK when I will get around to that. It would be very nice if somebody could add Redox to cargo cross or otherwise help with the CI/CD.

briansmith · 2020-05-22T00:19:41Z

On Twitter, @dcarosone suggested that some operating systems might provide a "do not close" fctl(). I do think it might make sense to be ask open() to reject all attempts to close or reuse the file descriptor, similarly to how we can request the file descriptor is closed on exec(). Even better, a way to reject all operations on the file descriptor other than read().

jackpot51 · 2020-05-22T01:48:34Z

I'll think about kernel randomness, and a syscall for getrandom. There are other good uses for kernel randomness, and there is probably a reasonable way to have a kernel random source get fed entropy from userspace drivers.

As for CI, it is pretty simple with the redoxer tool. The readme describes its use. To test this PR, for example, use the following:

git clone https://github.com/AdminXVII/ring -b fix-redox ring_redoxer
cd ring_redoxer/
cargo install redoxer
redoxer toolchain
redoxer test

Redoxer currently uses a nightly rustc that may not compile ring with warnings forbidden. This is due to use of inline assembly in Redox projects for hardware access, such as drivers and the kernel. Hopefully this is fixed soon with the new asm! macro.

jackpot51 · 2020-05-22T01:50:23Z

After allowing warnings in build.rs and src/lib.rs, I was able to run the tests with Redoxer successfully

briansmith · 2020-05-22T18:20:01Z

I'll think about kernel randomness, and a syscall for getrandom. There are other good uses for kernel randomness, and there is probably a reasonable way to have a kernel random source get fed entropy from userspace drivers.

Great!

As for CI, it is pretty simple with the redoxer tool. The readme describes its use. To test this PR, for example, use the following:
git clone https://github.com/AdminXVII/ring -b fix-redox ring_redoxer
cd ring_redoxer/
cargo install redoxer
redoxer toolchain
redoxer test

Excellent!

briansmith · 2020-11-24T17:58:04Z

This should go in mk/install-build-tools.sh when the target is redox:

git clone https://github.com/AdminXVII/ring -b fix-redox ring_redoxer
cd ring_redoxer/
[check out a specific revision here]
cargo install redoxer
redoxer toolchain
redoxer test

Change mk/cargo.sh to do something like this for redox:

   export CC_x86_64_unknown_linux_musl=clang-10
   export AR_x86_64_unknown_linux_musl=llvm-ar-10
   export CARGO_TARGET_X86_64_UNKNOWN_LINUX_MUSL_LINKER=clang-10

Update .github/workflows/ci.yml to add the redox target. aarch64-linux-musl is probably a good example to follow.

This PR is old and uses lazy_static! so the code changes will also need to be rebased.

briansmith

I've unfortunately kind of given up on the idea of having CI/CD for every supported target. Similarly, I've also relaxed my stance on "/dev/urandom" and similar.

Consequently, if this gets rebased on top of main and the minor issues I commented on are fixed, I'll merge it.

I'll file an issue against Redox suggesting Redox adopt an API more like Fuchsia's.

briansmith · 2020-11-30T19:58:17Z

src/rand.rs

@@ -183,7 +183,8 @@ use self::sysrand_or_urandom::fill as fill_impl;
    target_os = "freebsd",
    target_os = "netbsd",
    target_os = "openbsd",
-    target_os = "solaris"
+    target_os = "solaris",
+    target_os = "redox"


Please keep the list alphabetized.

briansmith · 2020-11-30T19:58:24Z

src/rand.rs

@@ -357,7 +358,8 @@ mod sysrand_or_urandom {
    target_os = "freebsd",
    target_os = "netbsd",
    target_os = "openbsd",
-    target_os = "solaris"
+    target_os = "solaris",
+    target_os = "redox"


briansmith · 2020-11-30T20:05:42Z

I'll file an issue against Redox suggesting Redox adopt an API more like Fuchsia's.

An issue of that sort was already filed in the issue tracker. I added a comment to it: https://gitlab.redox-os.org/redox-os/redox/-/issues/816#note_22234

briansmith · 2022-10-25T19:43:38Z

Thanks for the PR. I'm planning to switch the implementation to use the getrandom crate as part of the goal of encapsulating/minimizing operating-system-specific code in ring; see draft PR #1531. I'm closing this in favor of that approach.

jackpot51 · 2022-10-26T03:53:48Z

#1341 fixed this already. Good to hear you will be using getrandom.

Add support for Redox

584ef24

jackpot51 approved these changes Sep 29, 2019

View reviewed changes

HacKanCuBa mentioned this pull request May 22, 2020

fix cases where insecure randomness could be used coreutils/coreutils#32

Closed

briansmith requested changes Nov 30, 2020

View reviewed changes

jackpot51 mentioned this pull request May 22, 2021

Add Redox support #1300

Closed

briansmith closed this Oct 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Redox #901

Add support for Redox #901

AdminXVII commented Sep 29, 2019

jackpot51 left a comment

jackpot51 commented May 21, 2020

briansmith commented May 21, 2020

jackpot51 commented May 21, 2020

briansmith commented May 21, 2020 •

edited

Loading

jackpot51 commented May 21, 2020

briansmith commented May 21, 2020

briansmith commented May 21, 2020

briansmith commented May 22, 2020

jackpot51 commented May 22, 2020

jackpot51 commented May 22, 2020

briansmith commented May 22, 2020

briansmith commented Nov 24, 2020

briansmith left a comment

briansmith Nov 30, 2020

briansmith Nov 30, 2020

briansmith commented Nov 30, 2020

briansmith commented Oct 25, 2022

jackpot51 commented Oct 26, 2022

Add support for Redox #901

Add support for Redox #901

Conversation

AdminXVII commented Sep 29, 2019

jackpot51 left a comment

Choose a reason for hiding this comment

jackpot51 commented May 21, 2020

briansmith commented May 21, 2020

jackpot51 commented May 21, 2020

briansmith commented May 21, 2020 • edited Loading

jackpot51 commented May 21, 2020

briansmith commented May 21, 2020

briansmith commented May 21, 2020

briansmith commented May 22, 2020

jackpot51 commented May 22, 2020

jackpot51 commented May 22, 2020

briansmith commented May 22, 2020

briansmith commented Nov 24, 2020

briansmith left a comment

Choose a reason for hiding this comment

briansmith Nov 30, 2020

Choose a reason for hiding this comment

briansmith Nov 30, 2020

Choose a reason for hiding this comment

briansmith commented Nov 30, 2020

briansmith commented Oct 25, 2022

jackpot51 commented Oct 26, 2022

briansmith commented May 21, 2020 •

edited

Loading