Nocrypto.Rng.S.gen_bits throws exceptions / always returns 0 when [n] is large

[cfcs]

FWIW, may want to throw an exception instead of returning 0

utop # Nocrypto.Rng.Int32.gen_bits max_int;;
- : int32 = 0l
utop # Nocrypto.Rng.Int.gen_bits 0x3fff_ffff_ffff_ffff;;
- : int = 0

Also this exception is not documented

utop # Nocrypto.Rng.Int.gen_bits 0x2fff_ffff_ffff_ffff;;
Stack overflow during evaluation (looping recursion?)
─( 10:55:46 )─< command 135 >────────────────────────────────────{ counter: 0 }─
utop # try Nocrypto.Rng.Int32.gen_bits 0x3fff_ffff_ff with Stack_overflow -> 44203_l;;
- : int32 = 44203l

[pqwy]

• 0 happens because bytes max_int = cdiv max_int 8 overflows, giving a negative size to fortuna, which in turn returns an empty string. It it didn't, you just asked it to generate around 512PB of material. What is the behavior you expect when invoking Rng.generate max_int? What do you expect to happen when you attempt to generate max_int bits of a 32-bit quantity?

• If you don't overflow, fortuna smashes the stack. As of https://github.com/mirleft/ocaml-nocrypto/commit/885e7c1de64feb92a7b1b2fe0a9c68a6f714671f it will proceed to generate past the current, accidental, several-GB limit. Can you please test it?

More broadly, you keep on poking random functions with absurd inputs. Network-facing stuff usually has various sanity-checks. Stuff you call inside does not. What is the point here? To be well-behaved when the user asks for half an exabyte of random material?

[pqwy]

Then again... why not.

The expected outcome of Rng.generate max_int should be an OOM kill.

Here's how you can help: massacre all of the functions exported in the mli that take int as an argument with obnoxious inputs and report breakage here. If this is going to be fixed, I want to do it systematically.

[cfcs]

Sorry, I was not very clear on my reason for opening this post. I'll try to elaborate a bit on my thoughts.

Generally about exceptions: I personally dislike exceptions in OCaml because it's not easy to see what conditions may lead to an exception being thrown, or which parts of the code can raise which exceptions. I think this is one of the things Java did right (making the exceptions part of the function signature / making it very clear). Some failure modes are so general and exceptional (OOM and stack overflows) that they can happen in most functions, and independent of the input. Those are pretty much the only exceptions I believe are justified in OCaml as it is now. Until a system is in place to allow programmers to reason about these failures I think using either ('ok, 'error) result or 'ok option types for return values is preferable.

Specific case: This function takes an integer as input. Depending on how people use it they might end up passing the result of an arithmetic operation as the bits parameter. Here you can either hold their hands (return an error when the parameter is unreasonable) or decide that it is ridiculous to pass "give me half an exabyte of data" and not do any validation of the parameter. If you opt for the latter (completely reasonable in this case IMHO) I would suggest adding a comment in the mli docstring, both to make people aware that it is a potential source of errors, and to enable them to track down the source of the exceptions they might run into during unit testing (this is especially relevant when doing automated testing with crowbar or qcheck). You have such a comment in, for example, Nocrypto.Dh.gen_group (@raise Invalid_argument if [bits] is ridiculously small.). I think it would make sense to be consistent in this regard.

This function returns Stack_overflow when the stack is overflown - completely reasonable. The 0 being returned here, on the other hand, is dangerous because it's a silent failure that is easy to miss (if you are doing arithmetic to (incorrectly) calculate the number of bits to generate). As a user I would expect it to either return an error, or simply crash with an exception. I also think that would be vastly preferable to silently/accidentally nulling the request for random bits.

[cfcs]

• BTW: The reason I was playing around with this function is that Nocrypto.Rng.Int32.gen_r does not let me get a an integer in the range [min_int;max_int], so if I want to generate a uniformly random number with 2 ** 32 bits I need to use gen_bits Nocrypto.Numeric.Int32.(bit_bound zero) (with gen_r I will have a bias because it will never be max_int). I wanted to generate uniformly random UDP port numbers (2**16), so I tried to define a Uint16 : Nocrypto.Numeric.S and ran into this quirk. I can imagine that this interface would intuitively also be useful for picking random elements of custom types (card decks or whatever). This is of course possible with the gen_bits function, but you have to jump through some hoops (read: think) to get there.

[cfcs]

• Re: Systematically going through the functions: I am slowly working my way through them! :-)

[pqwy]

Did a pass over the API; the behavior should be closer to what the user specified.

Please do report other oddities of this sort, like unexpected exceptions, overflows, or outputs obviously discontinuous in input.

Thanks for reporting!

N.B.

You can also fix the bias and recover the full interval by:

• sampling from Rng.Int32.gen_r 0 0x80000000, then flipping a coin to add add 0x80000000 (or _ lor 0x80000000) to that, which works because the full interval is 2^k and splits evenly; or
• since you really want uniform bit coverage, and not just any old interval, you can use Rng.Int32.gen_bits b for any value with the last b bits set uniformly at random.

Something I notably did not document in the slightest is that bit_bound is totally internal and used to start the splitting operation that computes bits on unbounded numbers. On others, you should just replace it with the appropriate constant.

[cfcs]

Thank you, this is super nice!