test: BIP324: add check for detection of missing garbage terminator #28634

1. 
   theStack commented at 8:04 PM on October 10, 2023: contributor

   This PR adds test coverage for the "missing garbage terminator" detection on incoming v2 transport (BIP324) connections: https://github.com/bitcoin/bitcoin/blob/04265ba9378efbd4c35b33390b1e5cf246d420a9/src/net.cpp#L1205-L1209

   Note that this always happens at the same exact amount of bytes sent in (after 64 + 4095 + 16 = 4175 bytes), if at no point, the last 16 bytes of potential authentication data match the garbage, i.e. all the previous bytes after the ellswift pubkey. To keep it simple, we just send in zero-value bytes here and verify that the detection hits exactly after the last bytes is sent.

   AFAICT, with this PR all the v2 transport errors that can be triggered in this simple way of "just open a socket and send in a fixed byte-string" are covered. For more advanced test, we need BIP324 cryptography in the test framework in order to perform a v2 handshake etc. (PRs #28374, #24748).

2. 
   DrahtBot commented at 8:04 PM on October 10, 2023: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

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

   <!--006a51241073e994b41acfe9ec718e94-->

   Code Coverage

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

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	laanwj, sipa

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

3. DrahtBot added the label Tests on Oct 10, 2023
4. theStack force-pushed on Oct 10, 2023
5. test: BIP324: add check for missing garbage terminator detection 3bb51c29df
6. in test/functional/p2p_v2_transport.py:162 in 68c51db3d1 outdated

    157 | +            s.sendall(b'\x00' * (MAX_KEY_GARB_AND_GARBTERM_LEN - 1))
    158 | +            self.wait_until(lambda: self.nodes[0].getpeerinfo()[-1]["bytesrecv"] == MAX_KEY_GARB_AND_GARBTERM_LEN - 1)
    159 | +            with self.nodes[0].assert_debug_log("V2 transport error: missing garbage terminator"):
    160 | +                s.sendall(b'\x00')  # send out last byte
    161 | +            # should disconnect immediately
    162 | +            self.wait_until(lambda: len(self.nodes[0].getpeerinfo()) == num_peers)
   

   ---

   ---

   

   maflcko commented at 7:43 AM on October 11, 2023:

                   self.wait_until(lambda: len(self.nodes[0].getpeerinfo()) == num_peers)
   

   Would be good to use this sync helper to sync, not assert_debug_log.

   ---

   

   theStack commented at 9:58 AM on October 11, 2023:

   Thanks, done.

7. theStack force-pushed on Oct 11, 2023
8. fanquake requested review from sipa on Oct 11, 2023
9. fanquake requested review from mzumsande on Oct 11, 2023
10. laanwj approved
11. 
    laanwj commented at 12:47 PM on October 11, 2023: member

    ACK 3bb51c29df596aab2c1fde184667cee435597715 Test code looks correct, and similar to other test cases in this unit

12. 
    sipa commented at 12:59 PM on October 11, 2023: member

    utACK 3bb51c29df596aab2c1fde184667cee435597715

13. DrahtBot removed review request from sipa on Oct 11, 2023
14. fanquake merged this on Oct 12, 2023
15. fanquake closed this on Oct 12, 2023

    ---

16. theStack deleted the branch on Oct 12, 2023
17. theStack referenced this in commit 616c090e21 on Oct 12, 2023
18. theStack referenced this in commit ac4caf3366 on Oct 13, 2023
19. achow101 referenced this in commit 78b7e95518 on Oct 13, 2023
20. Frank-GER referenced this in commit 0a38a7b9bc on Oct 13, 2023
21. Frank-GER referenced this in commit 5629efd2d6 on Oct 21, 2023
22. kwvg referenced this in commit cd23d93a95 on Sep 17, 2024
23. kwvg referenced this in commit 3ca00c6d20 on Sep 17, 2024
24. kwvg referenced this in commit b4208544e7 on Sep 17, 2024
25. kwvg referenced this in commit 78d76b72c6 on Oct 2, 2024
26. kwvg referenced this in commit cdc6515798 on Oct 9, 2024
27. bitcoin locked this on Oct 11, 2024

Contributors

theStackDrahtBotmaflckolaanwjsipa

Review Requested
mzumsande