crypto: chacha20: always use our fallback timingsafe_bcmp rather than libc’s #29815

pull theuni wants to merge 1 commits into bitcoin:master from theuni:no-libc-bcmp changing 2 files +2 −13
  1. theuni commented at 4:06 pm on April 5, 2024: member

    Looking at libc sources, apple and openbsd implementations match our naive fallback. Only FreeBSD (and only x86_64) seems to implement an optimized version.

    It’s not worth the hassle of using a platform-specific function for such little gain.

    Additionally, as mentioned below, this is the only case outside of sha2 that requires an autoconf check, and I have upcoming PRs to remove the sha2 ones.

    Apple’s impl is unoptimized.

    As-is OpenBSD’s impl.

    Relevant IRC conversation with sipa:

    <cfields> sipa: chacha20poly1305.cpp uses libc’s timingsafe_bcmp when possible. But looking around at apple/freebsd/openbsd, I don’t see any impl that doesn’t use the naive implementation that matches our fallback… <cfields> is there any reason to belive there’s an optimized impl somewhere that we’re actually hitting? <cfields> asking because after cleaning up sha2, timingsafe_bcmp is the last autoconf check that remains in all of crypto. It’d make life easy if we could just always use our internal one. <cfields> *all of crypto/ <sipa> cfields: let’s get rid of the dependency then <sipa> it’s a trivial function <sipa> and if we need it for some platforms, no real reason not to use it on all

    After the above discusstion, I did end up finding the x86_64-optimized FreeBSD impl, but I don’t think that’s all that significant.

  2. crypto: chacha20: always use our fallback timingsafe_bcmp rather than libc's
    Looking at apple/freebsd/openbsd sources, their implementations match our naive
    fallback. It's not worth the hassle of using a platform-specific function for
    no gain.
    2d1819455c
  3. DrahtBot commented at 4:06 pm on April 5, 2024: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Code Coverage

    For detailed information about the code coverage, see the test coverage report.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK sipa, fanquake, TheCharlatan, theStack
    Concept ACK hebasto

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

  4. DrahtBot added the label Utils/log/libs on Apr 5, 2024
  5. theuni commented at 4:07 pm on April 5, 2024: member
    Ping @sipa for a quick concept ACK.
  6. sipa commented at 4:25 pm on April 5, 2024: member
    utACK 2d1819455cb4c516f6cdf81c11e869a23dee3e6b
  7. fanquake commented at 4:39 pm on April 5, 2024: member
    Concept ACK
  8. hebasto commented at 5:45 pm on April 5, 2024: member
    Concept ACK.
  9. theStack commented at 0:35 am on April 6, 2024: contributor
    Concept ACK
  10. fanquake approved
  11. fanquake commented at 6:24 pm on April 6, 2024: member
    ACK 2d1819455cb4c516f6cdf81c11e869a23dee3e6b
  12. DrahtBot requested review from hebasto on Apr 6, 2024
  13. DrahtBot requested review from theStack on Apr 6, 2024
  14. TheCharlatan approved
  15. TheCharlatan commented at 7:14 pm on April 6, 2024: contributor
    ACK 2d1819455cb4c516f6cdf81c11e869a23dee3e6b
  16. theStack approved
  17. theStack commented at 7:31 pm on April 6, 2024: contributor

    ACK 2d1819455cb4c516f6cdf81c11e869a23dee3e6b

    As a historical side-note, it seems like this function was first introduced for OpenSSH (managed in the OpenBSD tree) in 2010, with the name timing_safe_cmp back then: https://github.com/openbsd/src/commit/8488487f0974f365bb51defabda91a3e5dcdfaa6#diff-1baa12ad01bad68b45e89594ef3309ad070f9848f764f976e08c435ade846ae5R833

  18. fanquake merged this on Apr 6, 2024
  19. fanquake closed this on Apr 6, 2024


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-11-21 09:12 UTC

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