Skip to content

Adapt to chacha20 #1642

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ A [separate changelog is kept for rand_core](rand_core/CHANGELOG.md).
You may also find the [Upgrade Guide](https://rust-random.github.io/book/update.html) useful.

## [Unreleased]
### Changed
- The dependency on `rand_chacha` has been replaced with a dependency on `chacha20`. This changes the implementation behind `StdRng`, but the output remains the same. There may be some API breakage when using the ChaCha-types directly as these are now the ones in `chacha20` instead of `rand_chacha`.

### Deprecated
- Deprecate `rand::rngs::mock` module and `StepRng` generator (#1634)

Expand Down
11 changes: 8 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ serde = ["dep:serde", "rand_core/serde"]

# Option (enabled by default): without "std" rand uses libcore; this option
# enables functionality expected to be available on a standard platform.
std = ["rand_core/std", "rand_chacha?/std", "alloc"]
std = ["rand_core/std", "alloc"]

# Option: "alloc" enables support for Vec and Box when not using "std"
alloc = []
Expand All @@ -46,7 +46,7 @@ os_rng = ["rand_core/os_rng"]
simd_support = []

# Option (enabled by default): enable StdRng
std_rng = ["dep:rand_chacha"]
std_rng = ["dep:chacha20"]

# Option: enable SmallRng
small_rng = []
Expand All @@ -70,11 +70,16 @@ members = [
]
exclude = ["benches", "distr_test"]

# Force chacha20 to use the local rand_code to avoid compiling two different "rand_core"s.
# This is necessary even if the specified versions are the same.
[patch.crates-io]
rand_core = { path = "rand_core" }
Comment on lines +73 to +76
Copy link
Member

Choose a reason for hiding this comment

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

This is going to make it impossible to make releases.

Ugh. I think there are only three options here:

  1. Do this, but remove the patch when publishing. Undesirable because then the source does not correspond to the published versions.
  2. Move chacha20 source into this repo (likely undesirable; CC @tarcieri)
  3. Remove rand_core from the workspace. This will require using a patch similar to this for development whenever rand_core has breaking changes, but at least allows clean compiling otherwise.

This is why rand_chacha is co-located in this repo with external dependencies.

Choose a reason for hiding this comment

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

I think crate releases ignore these directives and try to build directly against crates.io?

Copy link
Author

Choose a reason for hiding this comment

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

There could be a 4th option: Depend on chacha20 but without the "rng" feature, then implement the required traits for chacha inside the rand crate instead.

Copy link
Member

Choose a reason for hiding this comment

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

Does it? I wish their documentation would clarify.

cargo publish --dry-run looks okay, so this might work.

Copy link
Member

Choose a reason for hiding this comment

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

But even assuming this does work, how are we supposed to handle breaking changes to rand_core in the future? If we ever make any changes incompatible with chacha20 our local builds will simply not work until upstream chacha20 gets fixed.

I don't think that's viable. (And the only reason it's not definitely unviable is that I don't expect much in the way of breaking changes to rand_core in the future.)

Copy link
Member

Choose a reason for hiding this comment

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

@hpenne I think the patch.crates-io approach should be fine. We use it frequently in @RustCrypto repos where we have external interdependencies

@tarcieri @newpavlov do you have a policy on breaking changes to affected crates (e.g. digest, signature)? Do you just simultaneously commit a fix to other affected repos and pin the version? Or do you never really get breaking changes which affect the build in practice?

Choose a reason for hiding this comment

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

Yes, often we'll have a PR to the other repo ready to go.

That said we'll hopefully be wrapping up breaking changes soon and be stable for awhile.

Copy link
Author

Choose a reason for hiding this comment

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

But even assuming this does work, how are we supposed to handle breaking changes to rand_core in the future? If we ever make any changes incompatible with chacha20 our local builds will simply not work until upstream chacha20 gets fixed.

@dhardy Isn't that a deadlock situation? With the "patch"-solution, if you make changes to rand_core so that local builds fail (due to chacha being forced to use the newer version that it is not compatible with), then you cannot publish a new version of rand_core, which means that chacha20 cannot be updated to support the new rand_core version either, because there is no such version published. Can that deadlock be broken in any acceptable way? If I implement the newtype/adapter workaround above, then we can use the patch solution 99% of the time to ensure that rand_core is only built once, but if you get into this potential deadlock situation then you can remove the patch temporarily to resolve it (everything will work, but you get rand_core built twice) and put it back again when both rand_core and chacha20 has been updated. You will get to have your cake and eat it too.

Copy link
Member

Choose a reason for hiding this comment

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

@hpenne publishing a new version of rand_core only requires that the rand_core build succeeds, not the whole workspace, so breaking changes don't cause a deadlock, no.

I guess the patch solution is tolerable. If you do try implementing the newtype/adapter workaround, please push as a new PR.

Copy link
Author

Choose a reason for hiding this comment

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

@hpenne publishing a new version of rand_core only requires that the rand_core build succeeds, not the whole workspace, so breaking changes don't cause a deadlock, no.

Thanks for clearing that up for me. I suppose that the problem of rand itself potentially not building in that situation due to breaking changes in rand_core can be resolved by modifying rand to fetch rand_core from crates.io for a while (using the previous release of rand_core), until chacha20 is updated to use the new rand_core and everything is in sync again so that it all builds with the local current rand_core from the rand git repo. Or perhaps there are even better ways, these kinds of sort-of-circular dependencies are a little new to me.

I guess the patch solution is tolerable. If you do try implementing the newtype/adapter workaround, please push as a new PR.

Noted. It probably won't be needed as long as the situation can be resolved by publishing any new breaking version rand_core separately.


[dependencies]
rand_core = { path = "rand_core", version = "0.9.0", default-features = false }
log = { version = "0.4.4", optional = true }
serde = { version = "1.0.103", features = ["derive"], optional = true }
rand_chacha = { path = "rand_chacha", version = "0.9.0", default-features = false, optional = true }
chacha20 = { version = "=0.10.0-rc.0", default-features = false, features = ["rng"], optional = true }

[dev-dependencies]
rand_pcg = { path = "rand_pcg", version = "0.9.0" }
Expand Down
5 changes: 3 additions & 2 deletions examples/rayon-monte-carlo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,9 @@
//! over BATCH_SIZE trials. Manually batching also turns out to be faster
//! for the nondeterministic version of this program as well.

use chacha20::ChaCha8Rng;
use rand::distr::{Distribution, Uniform};
use rand_chacha::{rand_core::SeedableRng, ChaCha8Rng};
use rand_core::SeedableRng;
use rayon::prelude::*;

static SEED: u64 = 0;
Expand All @@ -56,7 +57,7 @@ fn main() {
// We chose ChaCha because it's fast, has suitable statistical properties for simulation,
// and because it supports this set_stream() api, which lets us choose a different stream
// per work item. ChaCha supports 2^64 independent streams.
rng.set_stream(i);
rng.set_stream(u128::from(i));
let mut count = 0;
for _ in 0..BATCH_SIZE {
let a = range.sample(&mut rng);
Expand Down
2 changes: 1 addition & 1 deletion rand_core/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,7 @@ mod test {
}
rng.next_u32();

let result = rng.next_u64();
let _ = rng.next_u64();
assert_eq!(rng.index(), 1);
}
}
4 changes: 2 additions & 2 deletions src/rngs/reseeding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ use rand_core::{CryptoRng, RngCore, SeedableRng, TryCryptoRng, TryRngCore};
/// # Example
///
/// ```
/// use chacha20::ChaCha20Core; // Internal part of ChaChaRng that
/// // implements BlockRngCore
/// use rand::prelude::*;
/// use rand_chacha::ChaCha20Core; // Internal part of ChaChaRng that
/// // implements BlockRngCore
/// use rand::rngs::OsRng;
/// use rand::rngs::ReseedingRng;
///
Expand Down
81 changes: 79 additions & 2 deletions src/rngs/std.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
use rand_core::{CryptoRng, RngCore, SeedableRng};

#[cfg(any(test, feature = "os_rng"))]
pub(crate) use rand_chacha::ChaCha12Core as Core;
pub(crate) use chacha20::ChaCha12Core as Core;

use rand_chacha::ChaCha12Rng as Rng;
use chacha20::ChaCha12Rng as Rng;

/// A strong, fast (amortized), non-portable RNG
///
Expand Down Expand Up @@ -121,4 +121,81 @@ mod test {

assert_eq!([x0, x1], target);
}

#[test]
fn test_chacha12_tv1_tv2() {
// Test vectors 1 and 2 from
// https://tools.ietf.org/html/draft-nir-cfrg-chacha20-poly1305-04
// but the expected values are replaced with the ones produced
// by Chacha12 (as generated by rand_chacha).
let seed = [0u8; 32];
let mut rng = StdRng::from_seed(seed);

let results: [u32; 16] = core::array::from_fn(|_| rng.next_u32());
let expected = [
1788540059, 1408849159, 315498369, 3582142047, 3150129412, 343203913, 2777219198,
1595256366, 2046321669, 3236133586, 875096108, 2039282348, 748642874, 426368672,
803736417, 3190166337,
];
assert_eq!(results, expected);

let results = core::array::from_fn(|_| rng.next_u32());
let expected = [
1099486475, 4269030944, 863108230, 1024974988, 3085641926, 3435904281, 562369813,
3028926540, 3448579394, 301026465, 2545418950, 1137216539, 3393593065, 3226554768,
2963686439, 378791447,
];
assert_eq!(results, expected);
}

#[test]
fn test_chacha12_tv3() {
// Test vector 3 from
// https://tools.ietf.org/html/draft-nir-cfrg-chacha20-poly1305-04
// but the expected values are replaced with the ones produced
// by Chacha12 (as generated by rand_chacha).
let seed = [
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 1,
];
let mut rng = StdRng::from_seed(seed);

// Skip block 0
for _ in 0..16 {
rng.next_u32();
}

let results: [u32; 16] = core::array::from_fn(|_| rng.next_u32());
let expected = [
1149489484, 1453872210, 205606722, 2561053166, 4209612569, 2282920975, 2059979989,
3051314295, 4135011526, 3904759261, 3697107527, 2431012387, 1801979597, 1269452364,
3197257745, 2154707421,
];
assert_eq!(results, expected);
}

#[test]
fn test_chacha12_tv4() {
// Test vector 4 from
// https://tools.ietf.org/html/draft-nir-cfrg-chacha20-poly1305-04
// but the expected values are replaced with the ones produced
// by Chacha12 (as generated by rand_chacha).
let seed = [
0, 0xff, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0,
];

// Test block 2 by skipping block 0 and 1
let mut rng = StdRng::from_seed(seed);
for _ in 0..32 {
rng.next_u32();
}
let results: [u32; 16] = core::array::from_fn(|_| rng.next_u32());
let expected = [
1614024310, 613097064, 193455541, 494774344, 2671734178, 1658534307, 1449802595,
1853279994, 129091709, 2135285029, 2884964524, 3629948889, 2335941772, 2471549797,
887756884, 3388159322,
];
assert_eq!(results, expected);
}
}
Loading