authorgravatar for 124872+jedisct1@users.noreply.github.comFrank Denis <124872+jedisct1@users.noreply.github.com> 2020-10-09 14:33:16+02:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2020-10-15 20:57:16-04:00
log51a3d0603c116d99c0a93dd451a69c79dd0cbca2
treefde3b301f1e668a35ebf62fd5fc809b41a4a9790
parentf701459f04e24a46faf3edc5121f81fbf132f436

std.rand: set DefaultCsprng to Gimli, and require a larger seed

`DefaultCsprng` is documented as a cryptographically secure RNG. While `ISAAC` is a CSPRNG, the variant we have, `ISAAC64` is not. A 64 bit seed is a bit small to satisfy that claim. We also saw it being used with the current date as a seed, that also defeats the point of a CSPRNG. Set `DefaultCsprng` to `Gimli` instead of `ISAAC64`, rename the parameter from `init_s` to `secret_seed` + add a comment to clarify what kind of seed is expected here. Instead of directly touching the internals of the Gimli implementation (which can change/be architecture-specific), add an `init()` function to the state. Our Gimli-based CSPRNG was also not backtracking resistant. Gimli is a permutation; it can be reverted. So, if the state was ever leaked, future secrets, but also all the previously generated ones could be recovered. Clear the rate after a squeeze in order to prevent this. Finally, a dumb test was added just to exercise `DefaultCsprng` since we don't use it anywhere.

2 files changed, 35 insertions(+), 10 deletions(-)

lib/std/crypto/gimli.zig+9
...@@ -28,6 +28,15 @@ pub const State = struct {...@@ -28,6 +28,15 @@ pub const State = struct {
2828
29 const Self = @This();29 const Self = @This();
3030
31 pub fn init(initial_state: [State.BLOCKBYTES]u8) Self {
32 var data: [BLOCKBYTES / 4]u32 = undefined;
33 var i: usize = 0;
34 while (i < State.BLOCKBYTES) : (i += 4) {
35 data[i / 4] = mem.readIntLittle(u32, initial_state[i..][0..4]);
36 }
37 return Self{ .data = data };
38 }
39
31 /// TODO follow the span() convention instead of having this and `toSliceConst`40 /// TODO follow the span() convention instead of having this and `toSliceConst`
32 pub fn toSlice(self: *Self) []u8 {41 pub fn toSlice(self: *Self) []u8 {
33 return mem.sliceAsBytes(self.data[0..]);42 return mem.sliceAsBytes(self.data[0..]);
lib/std/rand.zig+26-10
...@@ -34,7 +34,7 @@ const maxInt = std.math.maxInt;...@@ -34,7 +34,7 @@ const maxInt = std.math.maxInt;
34pub const DefaultPrng = Xoroshiro128;34pub const DefaultPrng = Xoroshiro128;
3535
36/// Cryptographically secure random numbers.36/// Cryptographically secure random numbers.
37pub const DefaultCsprng = Isaac64;37pub const DefaultCsprng = Gimli;
3838
39pub const Random = struct {39pub const Random = struct {
40 fillFn: fn (r: *Random, buf: []u8) void,40 fillFn: fn (r: *Random, buf: []u8) void,
...@@ -749,29 +749,35 @@ pub const Gimli = struct {...@@ -749,29 +749,35 @@ pub const Gimli = struct {
749 random: Random,749 random: Random,
750 state: std.crypto.core.Gimli,750 state: std.crypto.core.Gimli,
751751
752 pub fn init(init_s: u64) Gimli {752 pub const secret_seed_length = 32;
753
754 /// The seed must be uniform, secret and `secret_seed_length` bytes long.
755 /// It can be generated using `std.crypto.randomBytes()`.
756 pub fn init(secret_seed: [secret_seed_length]u8) Gimli {
757 var initial_state: [std.crypto.core.Gimli.BLOCKBYTES]u8 = undefined;
758 mem.copy(u8, initial_state[0..secret_seed_length], &secret_seed);
759 mem.set(u8, initial_state[secret_seed_length..], 0);
753 var self = Gimli{760 var self = Gimli{
754 .random = Random{ .fillFn = fill },761 .random = Random{ .fillFn = fill },
755 .state = std.crypto.core.Gimli{762 .state = std.crypto.core.Gimli.init(initial_state),
756 .data = [_]u32{0} ** (std.crypto.gimli.State.BLOCKBYTES / 4),
757 },
758 };763 };
759 self.state.data[0] = @truncate(u32, init_s >> 32);
760 self.state.data[1] = @truncate(u32, init_s);
761 return self;764 return self;
762 }765 }
763766
764 fn fill(r: *Random, buf: []u8) void {767 fn fill(r: *Random, buf: []u8) void {
765 const self = @fieldParentPtr(Gimli, "random", r);768 const self = @fieldParentPtr(Gimli, "random", r);
766769
767 self.state.squeeze(buf);770 if (buf.len != 0) {
771 self.state.squeeze(buf);
772 } else {
773 self.state.permute();
774 }
775 mem.set(u8, self.state.toSlice()[0..std.crypto.core.Gimli.RATE], 0);
768 }776 }
769};777};
770778
771// ISAAC64 - http://www.burtleburtle.net/bob/rand/isaacafa.html779// ISAAC64 - http://www.burtleburtle.net/bob/rand/isaacafa.html
772//780//
773// CSPRNG
774//
775// Follows the general idea of the implementation from here with a few shortcuts.781// Follows the general idea of the implementation from here with a few shortcuts.
776// https://doc.rust-lang.org/rand/src/rand/prng/isaac64.rs.html782// https://doc.rust-lang.org/rand/src/rand/prng/isaac64.rs.html
777pub const Isaac64 = struct {783pub const Isaac64 = struct {
...@@ -1139,6 +1145,16 @@ fn testRangeBias(r: *Random, start: i8, end: i8, biased: bool) void {...@@ -1139,6 +1145,16 @@ fn testRangeBias(r: *Random, start: i8, end: i8, biased: bool) void {
1139 }1145 }
1140}1146}
11411147
1148test "CSPRNG" {
1149 var secret_seed: [DefaultCsprng.secret_seed_length]u8 = undefined;
1150 try std.crypto.randomBytes(&secret_seed);
1151 var csprng = DefaultCsprng.init(secret_seed);
1152 const a = csprng.random.int(u64);
1153 const b = csprng.random.int(u64);
1154 const c = csprng.random.int(u64);
1155 assert(a ^ b ^ c != 0);
1156}
1157
1142test "" {1158test "" {
1143 std.testing.refAllDecls(@This());1159 std.testing.refAllDecls(@This());
1144}1160}