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

Should crypto RNGs implement serialization? #31

Closed
dhardy opened this issue Oct 31, 2017 · 5 comments
Closed

Should crypto RNGs implement serialization? #31

dhardy opened this issue Oct 31, 2017 · 5 comments

Comments

@dhardy
Copy link
Owner

dhardy commented Oct 31, 2017

In #13 (see here) @pitdicker expresses his reservations about CSPRNGs revealing their internal state easily via serialization. I point out that APIs can't provide memory protection anyway.

Anyone else have an opinion?

My thoughts:

  1. It's not that important for CSPRNGs to implement serialization, but there has been interest in at least simpler PRNGs having serialization, and uniformity is generally better
  2. The suggested alternative is not so easily or generally workable
  3. Presumably anyone who is able to serialize a CSPRNG could also get its state some other way (such as transmutation / reinterpret_cast), so lack of serialization isn't much in the way of security
@dhardy dhardy added this to the rand-core RFC milestone Oct 31, 2017
@UserAB1236872
Copy link

UserAB1236872 commented Oct 31, 2017

A few thoughts:

  1. I don't think many people are using ChaCha for anything that needs to be serialized, but I think there's a small issue that Isaac underlies StdRng which isn't "advertised" on the docs page as being a CSPRNG, rather just that it's "efficient on the current platform".

    Making Isaac serializable is needed to make StdRng serializable. This can be worked around with something like:

    #[derive(Serialize,Deserialize)]
    struct Isaac64Internal { ... }
    
    pub struct Isaac64 { rng: Isaac64Internal }
    

    That is, hiding the serializable implementation behind a struct that just doesn't "re-export" the serialization functionality, but it seems a little needless.

    Personally, I'd like StdRng at least to be (de)serializable. It's an attractive alternative to XorShift, because while fast, XOR Shift-based RNGs aren't known for the highest quality randomness (irrespective of their cryptographic security), which may be important in some contexts.

  2. It's feature gated, and off by default, so users need to opt-in to the behavior.

  3. If need be, we could split the features to serde-1 (XorShift and other non-CS PRNGs) and csprng-serde-1 with appropriate disclaimers about how you could undermine the security by using it.

  4. The only channel I see this being abused if someone can't snipe memory (and the user isn't sending it on the wire for some reason) is if they're constantly serializing and deserializing the CSPRNG on the backend, and even then both operations should run in constant time so they'd at most probably be able to tell which CSPRNG you're using and not much else.

@dhardy
Copy link
Owner Author

dhardy commented Nov 1, 2017

I don't think StdRng should be serializable — in particular, we wish to be able to replace the implementation in the future without breaking code. But that doesn't mean IssacRng can't be serializable.

XorShift is not very good and may well be removed. Hopefully we'll add some alternatives. ISAAC has two significant drawbacks: (1) large memory footprint and (2) while there appear to be no known attacks, it's not exactly a "proven" CSPRNG.

Personally I'm in favour of just implementing for all PRNGs (including CSPRNGs).

@clamydo
Copy link

clamydo commented Jan 22, 2018

I don't buy the security argument neither. A user of a library has access the memory anyway if I'm informed correctly. What would the attack vector here? The internal state should be shielded against external processes, I agree. But I don't see any connection to implementing Serde::Serialize to that.

Can somebody explain to me please, how implementing that trait would affect the security at all? Seriously interested. Maybe I'm missing something.

@dhardy
Copy link
Owner Author

dhardy commented Jan 22, 2018

As far as I know the only possibility is that the host process serializes state which happens to get stored somewhere insecure. The thing is if serialisation is required, a key has to be stored somewhere anyway — unless fresh random numbers are sufficient, which is why it's been suggested that ReseedingRng implement "deserialisation" by reseeding from the OS. I guess the danger is that a process may serialise its entire state without the programmer realising that secret keys are included, but I wouldn't expect security-related programs to be serialising state anyway.

@dhardy
Copy link
Owner Author

dhardy commented Mar 11, 2018

Well, we now have serialisation implemented for non-crypto RNGs. I guess that will do for now; if not someone should open an issue in the main rand repo.

@dhardy dhardy closed this as completed Mar 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants