feat(p2p): add AutoTLS support for secure WebSocket connections #5187
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Co-authored-by: Janoš Guljaš <janos@resenje.org>
gacevicljubisa
changed the title
pkg/p2p/libp2p/libp2p.go
Outdated
| func parseAddress(addr string) (*parsedAddress, error) { | ||
| host, port, err := net.SplitHostPort(addr) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("address: %w", err) |
There was a problem hiding this comment.
The error message "address: ..." is generic. Consider adding the actual address to the error for debugging.
There was a problem hiding this comment.
Good point. I have updated the error.
| for _, underlay := range multiUnderlay { | ||
| // ping each underlay address, pick first available | ||
| start = time.Now() | ||
| if _, err := s.streamer.Ping(ctx, underlay); err != nil { |
There was a problem hiding this comment.
Is this intentional to remove ping check before adding the peer ?
There was a problem hiding this comment.
Yes, the ping in hive is not needed as the peer may not be available right away, but most importantly the removal of this Ping also cause much more stable topology when running integration tests on testnet clusters.
pkg/hive/hive.go
Outdated
| if err := s.addressBook.Put(bzzAddress.Overlay, bzzAddress); err != nil { | ||
| s.metrics.StorePeerErr.Inc() | ||
| s.logger.Warning("skipping peer in response", "peer_address", p.String(), "error", err) | ||
| return |
There was a problem hiding this comment.
Why the log is mentionning warning / skip and there is a return ?
There was a problem hiding this comment.
Good catch, I have change return to continue to try other addresses and I've updated the log message to be more clear.
| Limit: libp2prate.Limit{RPS: 10.0, Burst: 40}, | ||
| }, | ||
| }, | ||
| GracePeriod: 10 * time.Second, |
There was a problem hiding this comment.
Maybe extracting these numbers into separate variables, with a small comment explaining the rationale behind them ?
There was a problem hiding this comment.
I have added some comments. If you still think that extracting variables instead having here literals, I will make them, but it appeared to me that such indirection made this part of the code less readable, in this particular case.
pkg/p2p/libp2p/libp2p.go
Outdated
| } | ||
|
|
||
| // If we skipped all addresses due to self-connection, return an error | ||
| if skippedSelf && info != nil && info.ID == s.host.ID() { |
There was a problem hiding this comment.
If skippedSelf is true, info.ID == s.host.ID() will be true also, I was wondering whether both checks are needed here?
There was a problem hiding this comment.
Correct, I have remove the unnecessary checks.
Checklist
Description
This pull request introduces an AutoTLS implementation for the Bee node. The primary motivation for this feature is to support the In-Browser project, which requires secure communication channels for clients.
Modern web browsers strictly enforce secure contexts, meaning they can only connect to endpoints using secure protocols like HTTPS or Secure WebSockets (wss://). AutoTLS automates the process of obtaining, managing, and renewing valid TLS certificates from a Certificate Authority (like Let's Encrypt).
By implementing AutoTLS, Bee nodes can now accept wss:// connections directly, enabling browsers to interact with the Bee network securely and seamlessly.
Implementation Details
The implementation is based on the approach outlined in the official libp2p blog post on AutoTLS and follows the patterns from the go-libp2p AutoTLS example.
Testing & Validation
The functionality was manually tested by running a Bee node (built with the flag above) and attempting to connect to it using wscat over a secure websocket.
Test Command:
wscat --no-check -c wss://<node-public-ip>.<peer-id>.libp2p.direct:5500Result:
A successful connection was established, and the node responded with the multistream header, confirming the wss listener is active and secured with a valid TLS certificate:
Related Issue (Optional)
#5171