Secret key generation and cleaning #749

issue real-or-random openend this issue on May 1, 2020
  1. real-or-random commented at 11:42 am on May 1, 2020: contributor

    My feeling is this entire issue of secret key handling deserves a broader discussion.

    We don’t have a key generation function and Core won’t need one. If we’re now targeting different users more, maybe it’s time to have one. But it’s hard, see all the discussion points above. And it really depends on the user’s platform. So actually I’d be happy with good examples and some references to correct methods to get randomness on different OSes.

    On the other hand, nothing should stop us from exporting a cleaning function in #636, even if we don’t have one for key gen.

    What are other libraries doing for key generation?

    Originally posted by @real-or-random in #748 (comment)

  2. real-or-random commented at 11:47 am on May 1, 2020: contributor

    See libsodium for example.

    https://github.com/jedisct1/libsodium/blob/2f915846ff41191c1a17357f0efaae9d500e9858/src/libsodium/randombytes/internal/randombytes_internal_random.c

    This is heavy stuff. If we really want a function, we could simply steal this or another one. No need to reinvent the wheel.

    I don’t like to have that complexity in our code but the question is whether we prefer to have it in our code or let our users write correct system PRG calls…

  3. elichai commented at 2:37 pm on May 1, 2020: contributor

    There’s also what bitcoin does https://github.com/bitcoin/bitcoin/blob/master/src/random.cpp

    But I’d assume this is the main reason why there’s no key generation function in this library, because all of the needed complexity.

    (I also think that in the examples regardless of if we add a key generation function we can still provide something simple that’ll work on common modern systems)

  4. real-or-random commented at 2:44 pm on May 1, 2020: contributor
    Well, the thing that people would usually do is to move that code to the library and just call it. But for SHA256 we’re working on doing it the other way around actually. I’ve never quite heard and/or understood the full reasoning behind this. @sipa Can you elaborate a little bit?
  5. jonasnick commented at 7:56 am on May 4, 2020: contributor
    I’m fine with keeping key generation out of scope. But adding a good reference to the usage section would be helpful. @real-or-random do you have a particular reference in mind?
  6. sipa commented at 8:15 am on May 4, 2020: contributor

    @real-or-random My thinking is that libsecp256k1 should be focussing on doing secp256k1 EC crypto, and not become a general purpose crypto library.

    Part of this is scope creep… there is a lot that can be done around efficient hash functions for various architectures, random number generators for various platforms… and I think it would take us too far. If we want to do all those things near-perfectly it will mean duplicating a lot of work that’s done elsewhere, plus maintenance to keep up with new developments in hardware and operating systems. If we do less than a near optimal job, callers who care will still be forced to reimplement or use other libraries.

    Of course, this is partially in conflict with the goal of exposing a safe high-level API. We’ve already been forced to include a (naive) SHA256 implementation, but even there I feel like it would be a waste of our efforts to try to extend it to e.g. SSE4/AVX2/SHA-NI support. Permitting the caller to plug in their own version (at runtime or compile time) feels like a better option to getting better performance out of that. Most callers won’t care about the mediocre performance of our SHA256, and those that do probably already have access to better versions.

    I don’t think it’s that different for random generation. I wouldn’t be opposed to adding a simple minimal built-in one, but I fear that any decent one is already going to be a pain in terms of build system integration etc alone.

  7. real-or-random commented at 10:00 am on May 5, 2020: contributor

    That seems wise.

    What does that mean for us? I think then giving a bit of useful advice in the examples and/or the usage section is what we should do right now. This helps the user but we don’t promise to maintain any code in our library. We should cover at least (modern versions) Linux, MacOS, and Windows. I can try to find some references.

    I’m against adding a minimal key gen function then, because then the user may just use that one without looking at it. If we offer nothing, the user may indeed read our docs.

    edit: I think we can safely export a cleaning function though. Ours is pretty portable and depends much less on the environment than randomness generation. It still depends on the compiler, but if our cleaning function is not effective, then it anyway won’t help that the user uses a different working one just for his code.

    Sorry, didn’t want to close here actually

  8. real-or-random closed this on May 5, 2020

  9. real-or-random reopened this on May 5, 2020

  10. real-or-random cross-referenced this on May 6, 2020 from issue Add usage examples by elichai

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-10-30 01:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me