-
Notifications
You must be signed in to change notification settings - Fork 182
Fix/885 update default bind address #892
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Fix/885 update default bind address #892
Conversation
…ead of wildcard (0.0.0.0) across multiple examples and utilities.
…sure secure binding addresses (127.0.0.1) are used instead of wildcard (0.0.0.0)
@yashksaini-coder : Great contribution, Yash. Appreciate your efforts. This PR is coming along nicely. Looking forward to detailed review by @acul71 on this PR. |
(venv) luca@r17:~/PNL_Launchpad_Curriculum/Libp2p/py-libp2p$ identify-demo
First host listening (using raw protobuf format). Run this from another console:
identify-demo --raw-format -d /ip4/127.0.0.1/tcp/37511/p2p/QmVUKiaLFMt9AVALFTPCRycCM2FArzugTLScVKaeeoCyxc or like that (venv) luca@r17:~/PNL_Launchpad_Curriculum/Libp2p/py-libp2p$ ping-demo
Run this from the same folder in another console:
ping-demo -d /ip4/0.0.0.0/tcp/42547/p2p/QmRShbR5CqVSkF4zWKCUhx7BwU9MonXV5LNxFDT1CborEk
Waiting for incoming connection... it should be instead similar to this: (venv) luca@r17:~/PNL_Launchpad_Curriculum/Libp2p/py-libp2p$ echo-demo
Listener ready, listening on:
/ip4/192.168.1.17/tcp/43857/p2p/16Uiu2HAmJcvmEow2iaFDEexsN2HMqzX8DcwxhDct87YYnoQNhv7m
/ip4/10.156.233.51/tcp/43857/p2p/16Uiu2HAmJcvmEow2iaFDEexsN2HMqzX8DcwxhDct87YYnoQNhv7m
/ip4/127.0.0.1/tcp/43857/p2p/16Uiu2HAmJcvmEow2iaFDEexsN2HMqzX8DcwxhDct87YYnoQNhv7m
Run this from the same folder in another console:
echo-demo -d /ip4/192.168.1.17/tcp/43857/p2p/16Uiu2HAmJcvmEow2iaFDEexsN2HMqzX8DcwxhDct87YYnoQNhv7m
Waiting for incoming connections... So that it shows all available interfaces and a default command for the client (
|
@seetadev @sumanjeet0012 Hi, I've worked and improved this PR with updated function I need assistance, since the checks are going failed. Seems that the py-cid repo is not able to clone Please take a look, I've run all checks locally, pre-commit, make pr, make fix |
@yashksaini-coder The GitHub account of Luca is currently suspended, which is causing this issue. |
yes I got the message yesterday, I have got the extended review and follow up steps on this PR, I will update you later. currently occupied on job work. Thanks |
…rd support - Introduced `get_wildcard_address` function for explicit wildcard binding. - Updated examples to use `get_available_interfaces` and `get_optimal_binding_address` for address selection. - Ensured consistent usage of the new address paradigm across all example files. - Added tests to verify the implementation of the new address paradigm and wildcard feature.
@yashksaini-coder @seetadev |
@yashksaini-coder Does it need to be reviewed ? |
@sumanjeet0012 , @acul71 : Lets do a detailed review on it. |
yes |
Code Review: Address Paradigm Implementation (PR #885)The implementation successfully introduces the new address paradigm with ✅ Good Changes
❌ Critical IssuesIssue 1: Mass Conversion of 0.0.0.0 to 127.0.0.1 (Commit 8755011)Problem: Changed
Impact: All examples became localhost-only, breaking network accessibility Issue 2: Incomplete Fix in Latest CommitProblem: Only partially implemented the new paradigm, leaving many files still broken
🔧 Recommended FixesOption 1: Use New Paradigm (Recommended)# Replace hardcoded addresses with:
listen_addrs = get_available_interfaces(port)
# Then use: async with host.run(listen_addrs=listen_addrs) Option 2: Keep Wildcard for Demos# If examples need to be accessible from other machines:
listen_addr = multiaddr.Multiaddr(f"/ip4/0.0.0.0/udp/{port}/quic-v1") ConclusionThe implementation has two major problems:
The entire examples directory needs to be systematically updated to use the new address paradigm functions instead of hardcoded Status: ❌ Major fixes required before merge |
@acul71 glad to see you are back, I'm working on the interops testing of python and js, so the branch didn't update there was a third commit that handle update to all the files, I will commit after checking and updating branch, thanks for detailed analysis. |
I can see that in identify-demo (venv) luca@r17:~/PNL_Launchpad_Curriculum/Libp2p/py-libp2p$ identify-demo
2025-09-17 19:56:15,126 [DEBUG] [async_service.Manager] <Manager[Swarm] flags=SRcfe>: _handle_cancelled waiting for cancellation
First host listening (using raw protobuf format).
Listener ready, listening on:
/ip4/192.168.1.232/tcp/46067/p2p/QmVorJGQfjEA3bJh6k4EaiQd2wPWYM82BT5ttqFg2YvTAB
/ip4/10.156.233.51/tcp/46067/p2p/QmVorJGQfjEA3bJh6k4EaiQd2wPWYM82BT5ttqFg2YvTAB
/ip4/127.0.0.1/tcp/46067/p2p/QmVorJGQfjEA3bJh6k4EaiQd2wPWYM82BT5ttqFg2YvTAB
Run this from the same folder in another console:
identify-demo --raw-format -d /ip4/192.168.1.232/tcp/46067/p2p/QmVorJGQfjEA3bJh6k4EaiQd2wPWYM82BT5ttqFg2YvTAB
|
Code Review: Address Paradigm Implementation (PR #885) - Follow-up✅ MAJOR IMPROVEMENTS - Issues Successfully AddressedThe developer yashksaini-coder has successfully addressed ALL critical issues identified in the previous review. The implementation now properly uses the new address paradigm throughout the codebase. ✅ Fixed IssuesIssue 1: QUIC Examples Fixed ✅Previously broken files now properly implemented:
Issue 2: All Encryption Examples Fixed ✅All encryption examples now use the new paradigm:
Issue 3: All Other Examples Fixed ✅All remaining examples properly updated:
✅ Implementation QualityConsistent Pattern UsageAll examples now follow the recommended pattern: # Get available interfaces
listen_addrs = get_available_interfaces(port)
# Get optimal address for client commands
optimal_addr = get_optimal_binding_address(port)
# Use in host.run()
async with host.run(listen_addrs=listen_addrs): QUIC-Specific HandlingQUIC examples properly handle protocol conversion: # Convert TCP addresses to QUIC addresses
quic_addrs = []
for addr in listen_addrs:
addr_str = str(addr).replace("/tcp/", "/udp/") + "/quic-v1"
quic_addrs.append(Multiaddr(addr_str)) Address Validation ModuleThe
✅ Network Accessibility Restored
✅ Code Quality Improvements
❌ Critical Bug Found in identify-demoIssue: Inverted Format LogicProblem: The Current behavior:
Root cause: Line 289 in # WRONG: This inverts the logic
use_varint_format = args.raw_format Should be: # CORRECT: --raw-format means NOT using varint format
use_varint_format = not args.raw_format Evidence from testing:
🔧 Required FixFix the format logic in
|
@acul71 okh changed the default param during ![]() |
What was wrong?
My previous PR #811 lacked the code changes on, multiple
examples
andcore
modules that were using wildcard addresses (0.0.0.0
) for binding, which could expose services on all network interfaces and create security vulnerabilities.How was it fixed?
I updated all the modules files and refactored to replace all wildcard bind addresses (
0.0.0.0
) with secure loopback addresses (127.0.0.1
) across the entire codebase:Examples Directory (17 files updated)
ping.py
,chat.py
,bootstrap.py
,mDNS.py
,pubsub.py
,random_walk.py
,identify.py
,identify_push_listener_dialer.py
examples/doc-examples/
directorynetwork_discover.py
(updated fallback functions)Core Library Updates
libp2p/utils/address_validation.py
: Updated fallback addresses from0.0.0.0
to127.0.0.1
Documentation Updates (5 files)
.rst
files indocs/examples.*.rst
to reflect new secure addressesTesting & Validation
tests/utils/test_default_bind_address.py
: Comprehensive tests for secure address selectiontests/examples/test_examples_bind_address.py
: Validation that all examples use secure addressesRelease Notes
newsfragments/885.feature.rst
: Security enhancement notificationTo-Do
cc: @acul71 @seetadev @pacrob