gui: remove OpenSSL PRNG seeding (Windows, Qt only) #17151

pull fanquake wants to merge 1 commits into bitcoin:master from fanquake:windows_qt_openssl_seeding changing 1 files +0 −13
  1. fanquake commented at 5:14 pm on October 15, 2019: member

    This removes the code introduced in #4399 that attempts to add additional entroy to the OpenSSL PRNG using RAND_event(). This is specific to bitcoin-qt running on Windows.

    0RAND_event() collects the entropy from Windows events such as mouse movements and other user interaction.
    1It should be called with the iMsg, wParam and lParam arguments of all messages sent to the window procedure.
    2It will estimate the entropy contained in the event message (if any), and add it to the PRNG.
    3The program can then process the messages as usual.
    

    Besides BIP70, this is the last place we are directly using OpenSSL in the GUI code. All other OpenSSL usage is in random.cpp.

    Note that we are still also still doing other Windows specific gathering using RandAddSeedPerfmon and RAND_screen() on top of the other generation we do.

    Also note that if RAND_event returns 0 here (PRNG has NOT been seeded with enough data), we’re just logging a single message and continuing, which also seems less than ideal.

  2. gui: remove OpenSSL PRNG seeding (Windows, Qt only)
    This removes the code introduced in [#4399](https://github.com/bitcoin/bitcoin/pull/4399)
    that attempts to add additional entroy to the OpenSSL PRNG using Windows messages.
    Note that this is specific to bitcoin-qt running on Windows.
    
    ```
    RAND_event() collects the entropy from Windows events such as mouse movements and other user interaction.
    It should be called with the iMsg, wParam and lParam arguments of all messages sent to the window procedure.
    It will estimate the entropy contained in the event message (if any), and add it to the PRNG.
    The program can then process the messages as usual.
    ```
    
    Besides BIP70, this is the last place we are directly using OpenSSL in the
    GUI code. All other OpenSSL usage is in random.cpp.
    
    Note that we are still also doing Windows specific entropy gathering in multiple
    other places. Such as [RandAddSeedPerfmon](https://github.com/bitcoin/bitcoin/blob/master/src/random.cpp#L268)
    and [RAND_screen()](https://github.com/bitcoin/bitcoin/blob/master/src/random.cpp#L600).
    
    Also note that if RAND_event returns 0 (PRNG has NOT been seeded with enough data), we're
    just logging a message and continuing on, which seems less than ideal.
    cc3b5289ef
  3. fanquake added the label GUI on Oct 15, 2019
  4. fanquake added the label Windows on Oct 15, 2019
  5. fanquake added the label Needs gitian build on Oct 15, 2019
  6. TheBlueMatt commented at 8:37 pm on October 15, 2019: member
    Can we snarf (some portion of) mouse movements via Qt directly (on all platforms)?
  7. sipa commented at 8:50 pm on October 15, 2019: member

    Concept ACK on removing this, especially as it’s Windows only.

    But it would be nice to use UI events in general and feed them into our own RNG (perhaps batches together when there are a bunch of events accumulated).

  8. MarcoFalke commented at 8:54 pm on October 15, 2019: member
    I think removing this (gui-only, windows-only) can be done independently from adding more randomness sources
  9. sipa commented at 9:06 pm on October 15, 2019: member

    I think removing this (gui-only, windows-only) can be done independently from adding more randomness sources

    Agreed.

  10. laanwj commented at 7:32 am on October 16, 2019: member

    ACK cc3b5289ef648dca30469ee4afa615a1ed5b4e04

    Maybe get rid of RAND_screen calls too. They’ve been deprecated a while ago.

    But it would be nice to use UI events in general and feed them into our own RNG (perhaps batches together when there are a bunch of events accumulated).

    I’m not sure it’s worth doing, especially with the GUI and node/wallet split on the horizon. Also conceptually not sure the presence of an UI or not should affect random generation, at least manually: the OS’s cryptographic randomness generation on modern OSes is supposed to take care of this. It has access to much more timings and such than a user-space application ever would.

    (in any case, Marco is correct that this is a separate concern and probably should be discussed somewhere else, or at least isn’t a blocker here)

  11. carnhofdaki commented at 12:21 pm on October 16, 2019: contributor

    the OS’s randomness generation on modern OSes is supposed to take care of this. It has access to much more timings and such than a user-space application ever would.

    Yes. Just for fun, no offence: Reminds me of https://flak.tedunangst.com/post/random-in-the-wild

  12. MarcoFalke commented at 9:22 pm on October 16, 2019: member
    unsigned ACK cc3b5289ef648dca30469ee4afa615a1ed5b4e04
  13. DrahtBot commented at 11:59 pm on October 16, 2019: member

    Gitian builds for commit 4cfb6738e81bf7312f6a17c860a0b61ccdf20642 (master):

    Gitian builds for commit 4093216679663d89e6a491205fd927cf84c9e24d (master and this pull):

  14. DrahtBot removed the label Needs gitian build on Oct 16, 2019
  15. theuni approved
  16. theuni commented at 6:02 pm on October 18, 2019: member

    ACK cc3b5289ef648dca30469ee4afa615a1ed5b4e04.

    Also agree with nuking the RAND_screen() as a follow-up.

  17. fanquake referenced this in commit ddc3cf26ff on Oct 18, 2019
  18. fanquake merged this on Oct 18, 2019
  19. fanquake closed this on Oct 18, 2019

  20. fanquake referenced this in commit e892f9648a on Oct 18, 2019
  21. fanquake deleted the branch on Oct 18, 2019
  22. fanquake referenced this in commit fb2a69697e on Oct 19, 2019
  23. laanwj referenced this in commit 0d6b6b7c65 on Oct 21, 2019
  24. sidhujag referenced this in commit e6a7ea19a7 on Oct 21, 2019
  25. sidhujag referenced this in commit dc57934cb3 on Oct 21, 2019
  26. fanquake referenced this in commit 5cc4f20c70 on Oct 26, 2019
  27. fanquake referenced this in commit 770cd9640e on Oct 28, 2019
  28. sipa referenced this in commit 350200f455 on Oct 28, 2019
  29. sipa referenced this in commit ca8c303c75 on Oct 29, 2019
  30. sipa referenced this in commit e876deebdc on Nov 2, 2019
  31. sipa referenced this in commit 40a7fb4914 on Nov 2, 2019
  32. fanquake referenced this in commit 082fc549ab on Nov 2, 2019
  33. sipa referenced this in commit eabfda8b78 on Nov 6, 2019
  34. sipa referenced this in commit 5eef148178 on Nov 6, 2019
  35. sipa referenced this in commit db176a4a87 on Nov 7, 2019
  36. sipa referenced this in commit 6b0c102209 on Nov 7, 2019
  37. sipa referenced this in commit b51bae1a5a on Nov 12, 2019
  38. Naviabheeman referenced this in commit 928f4ecfdf on Apr 6, 2020
  39. MarkLTZ referenced this in commit 7fdadc73fc on Apr 6, 2020
  40. MarkLTZ referenced this in commit 8436547baa on Apr 6, 2020
  41. Naviabheeman referenced this in commit fe91a6d71d on Apr 7, 2020
  42. HashUnlimited referenced this in commit 6c4be917c9 on Apr 17, 2020
  43. deadalnix referenced this in commit bd779b7d26 on May 20, 2020
  44. deadalnix referenced this in commit 887a72c4c0 on May 20, 2020
  45. Naviabheeman referenced this in commit 6538847bf2 on Jun 5, 2020
  46. ftrader referenced this in commit 5510da5d81 on Aug 17, 2020
  47. silence48 referenced this in commit 16c8e2a4d9 on Nov 14, 2020
  48. silence48 referenced this in commit 14020afe79 on Nov 15, 2020
  49. Fuzzbawls referenced this in commit b437172860 on Mar 30, 2021
  50. Fuzzbawls referenced this in commit 1998737b7e on Mar 31, 2021
  51. Fuzzbawls referenced this in commit 5eed08c62b on Apr 14, 2021
  52. Fuzzbawls referenced this in commit 27cf995d32 on Apr 14, 2021
  53. random-zebra referenced this in commit 93f43f0f81 on Apr 14, 2021
  54. PastaPastaPasta referenced this in commit a9c7974bd9 on Sep 11, 2021
  55. PastaPastaPasta referenced this in commit 214e440e21 on Sep 11, 2021
  56. PastaPastaPasta referenced this in commit e8ded5fcf3 on Sep 12, 2021
  57. PastaPastaPasta referenced this in commit 44ca1d001a on Sep 12, 2021
  58. PastaPastaPasta referenced this in commit 9e763c3acc on Sep 12, 2021
  59. PastaPastaPasta referenced this in commit ed384a43e9 on Sep 14, 2021
  60. PastaPastaPasta referenced this in commit 757b5e7bb3 on Sep 14, 2021
  61. PastaPastaPasta referenced this in commit af8f8b67f6 on Sep 15, 2021
  62. PastaPastaPasta referenced this in commit 391c59af6d on Sep 21, 2021
  63. kittywhiskers referenced this in commit cdf303d395 on Oct 12, 2021
  64. MarcoFalke locked this on Dec 16, 2021

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-10-04 22:12 UTC

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