log: Mitigate disk filling attacks by globally rate limiting LogPrintf(…) #21706

pull dergoegge wants to merge 2 commits into bitcoin:master from dergoegge:g_log_ratelimiting changing 5 files +258 −15
  1. dergoegge commented at 4:29 PM on April 16, 2021: member

    This is an alternative to #21603 in an attempt to solve #21559.

    Example for the approach in this PR: Location A logs excessively and logging gets suppressed for up to one hour. All logs from any other location will also be dropped during the suppression period. After ~one hour logging restarts and a message with a report on which locations have been suppressed is printed to the log.

    The key difference to #21603 is that logging is suppressed globally instead of "per source location" when at least one source location is logging excessively.

    Approach review is probably the best next step to determine which of the two PRs to move forward with.

  2. dergoegge cross-referenced this on Apr 16, 2021 from issue log: Mitigate disk filling attacks by rate limiting LogPrintf by dergoegge
  3. DrahtBot added the label Validation on Apr 16, 2021
  4. DrahtBot commented at 11:07 PM on April 16, 2021: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #21603 (log: Mitigate disk filling attacks by rate limiting LogPrintf by dergoegge)
    • #21526 (validation: UpdateTip/CheckBlockIndex assumeutxo support by jamesob)
    • #16673 (Relog configuration args on debug.log rotation by LarryRuane)
    • #15719 (Wallet passive startup by ryanofsky)
    • #15606 ([experimental] UTXO snapshots by jamesob)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  5. DrahtBot cross-referenced this on Apr 17, 2021 from issue validation: UpdateTip/CheckBlockIndex assumeutxo support by jamesob
  6. DrahtBot cross-referenced this on Apr 17, 2021 from issue Relog configuration args on debug.log rotation by LarryRuane
  7. DrahtBot cross-referenced this on Apr 20, 2021 from issue MOVEONLY: Move common init code to init/common by ryanofsky
  8. DrahtBot added the label Needs rebase on Apr 23, 2021
  9. DrahtBot commented at 9:32 AM on May 3, 2021: contributor

    <!--4a62be1de6b64f3ed646cdc7932c8cf5-->

    🕵️ @sipa has been requested to review this pull request as specified in the REVIEWERS file.

  10. dergoegge force-pushed on May 13, 2021
  11. DrahtBot removed the label Needs rebase on May 13, 2021
  12. DrahtBot cross-referenced this on May 28, 2021 from issue assumeutxo by jamesob
  13. DrahtBot cross-referenced this on Jun 3, 2021 from issue [Bundle 7/7] validation: Farewell, global Chainstate! by dongcarl
  14. MarcoFalke commented at 7:13 AM on June 3, 2021: member

    From ci: test/logging_tests.cpp(78): error: in "logging_tests/rate_limiting": check curr_log_file_size - prev_log_file_size == 1024 * 1024 has failed

  15. dergoegge force-pushed on Jun 4, 2021
  16. DrahtBot added the label Needs rebase on Jun 12, 2021
  17. practicalswift commented at 8:05 AM on June 12, 2021: contributor

    @dergoegge Thanks for working on this. Would you mind rebasing? I would like to review the updated version :)

  18. practicalswift commented at 8:55 AM on June 12, 2021: contributor

    Concept ACK

    While I have a slight preference for the approach taken in #21603 (rate-limiting per log location), but the approach in this PR (rate-limiting globally) works too. Concept ACK on both: the important thing is that we kill this bug class :)

  19. log: Mitigate disk filling attacks by temporarily and globally rate limiting LogPrintf(…) d05d55f2d4
  20. test: Add logging test for rate limiting aa6a139824
  21. dergoegge force-pushed on Jun 12, 2021
  22. DrahtBot removed the label Needs rebase on Jun 12, 2021
  23. DrahtBot cross-referenced this on Jun 15, 2021 from issue Wallet passive startup by ryanofsky
  24. dergoegge commented at 7:11 PM on June 17, 2021: member

    We will move forward with the other approach in #21603. See the PR description there for the reasoning.

  25. dergoegge closed this on Jun 17, 2021
  26. bitcoin locked this on Aug 18, 2022
Contributors
dergoeggeDrahtBotMarcoFalkepracticalswift
Labels
Validation
Linked (view graph)
#21559 Add mitigation for disk fill via logging attacks#21603 log: Mitigate disk filling attacks by rate limiting LogPrintf
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: 2026-05-20 06:54 UTC