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

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. DrahtBot added the label Validation on Apr 16, 2021
3. 
   DrahtBot commented at 11:07 pm on April 16, 2021: member

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

   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.

4. DrahtBot added the label Needs rebase on Apr 23, 2021
5. 
   DrahtBot commented at 9:32 am on May 3, 2021: member

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

6. dergoegge force-pushed on May 13, 2021
7. DrahtBot removed the label Needs rebase on May 13, 2021
8. 
   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
9. dergoegge force-pushed on Jun 4, 2021
10. DrahtBot added the label Needs rebase on Jun 12, 2021
11. 
    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 :)
12. 
    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 :)

13. log: Mitigate disk filling attacks by temporarily and globally rate limiting LogPrintf(…) d05d55f2d4
14. test: Add logging test for rate limiting aa6a139824
15. dergoegge force-pushed on Jun 12, 2021
16. DrahtBot removed the label Needs rebase on Jun 12, 2021
17. 
    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.
18. dergoegge closed this on Jun 17, 2021

    ---

19. DrahtBot locked this on Aug 18, 2022

Contributors
dergoegge DrahtBot MarcoFalke practicalswift

Labels
Validation