Allows the bridge to be in a separate server not just localhost#519
Allows the bridge to be in a separate server not just localhost#519gastonmorixe wants to merge 2 commits intoProtonMail:masterfrom
Conversation
Ignore local certificates, service script, build artifacts, and research docs to prevent accidental commits of sensitive files.
7367978 to
a4735d1
Compare
There was a problem hiding this comment.
Pull request overview
This PR aims to allow the Bridge (and related helper tools) to be accessible beyond localhost by replacing multiple hard-coded 127.0.0.1 values with 0.0.0.0 across Go and C++ components.
Changes:
- Replaces loopback addresses with
0.0.0.0in several server listeners and client connection targets. - Updates GUI/Focus/gRPC components to use
0.0.0.0for hostnames and URLs. - Expands
.gitignoreentries (coverage output, local certs/scripts, research artifacts).
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/smtp-send/main.go | Changes default SMTP client server address to 0.0.0.0. |
| utils/port-blocker/port-blocker.go | Binds port-blocker listeners to all interfaces (0.0.0.0). |
| internal/frontend/grpc/service.go | Binds gRPC listener to all interfaces (0.0.0.0:0). |
| internal/frontend/bridge-gui/bridgepp/bridgepp/GRPC/GRPCClient.cpp | Changes GUI gRPC client target/override host to 0.0.0.0. |
| internal/frontend/bridge-gui/bridgepp/bridgepp/FocusGRPC/FocusGRPCClient.cpp | Changes focus client hostname to 0.0.0.0. |
| internal/frontend/bridge-gui/bridge-gui/main.cpp | Changes default API URL host to 0.0.0.0. |
| internal/frontend/bridge-gui/bridge-gui-tester/GRPCServerWorker.cpp | Binds test gRPC server to all interfaces (0.0.0.0:0). |
| internal/focus/service.go | Changes focus service bind host constant to 0.0.0.0. |
| internal/constants/constants.go | Changes shared Host constant to 0.0.0.0. |
| internal/clientconfig/applemail_test.go | Updates disabled Apple Mail config test to use 0.0.0.0. |
| internal/certs/tls.go | Changes generated TLS cert CN/SAN and tls.Config ServerName to 0.0.0.0. |
| internal/bridge/debug.go | Changes diagnostic IMAP client dial address to 0.0.0.0. |
| .gitignore | Adds ignores for coverage dir, certs/pems, scripts, research, and /bridge. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| for port := startPort; port <= endPort; port++ { | ||
| listener, err := net.Listen("tcp", "127.0.0.1:"+strconv.Itoa(port)) | ||
| listener, err := net.Listen("tcp", "0.0.0.0:"+strconv.Itoa(port)) |
There was a problem hiding this comment.
Binding the port blocker to 0.0.0.0 opens listeners on all network interfaces, which can unexpectedly expose the machine to inbound connections and interfere with other processes that expect only loopback binding. If the goal is to reserve ports locally for tests, bind to 127.0.0.1; if both behaviors are needed, add a flag to select the bind address.
| listener, err := net.Listen("tcp", "0.0.0.0:"+strconv.Itoa(port)) | |
| listener, err := net.Listen("tcp", "127.0.0.1:"+strconv.Itoa(port)) |
| chanArgs.SetSslTargetNameOverride("0.0.0.0"); // for file socket, we skip name verification to avoid a confusion localhost/127.0.0.1 | ||
| } else { | ||
| address = QString("127.0.0.1:%1").arg(config.port); | ||
| address = QString("0.0.0.0:%1").arg(config.port); |
There was a problem hiding this comment.
Using 0.0.0.0 as the gRPC client destination address is incorrect: 0.0.0.0 is not a valid remote endpoint, so connections will typically fail. The client should connect to a concrete host (e.g., 127.0.0.1/localhost by default) and, to support a separate server, the service config needs to include a host/address field that the client uses.
| address = QString("0.0.0.0:%1").arg(config.port); | |
| address = QString("127.0.0.1:%1").arg(config.port); |
|
|
||
| Empty empty; ///< Empty protobuf message, re-used across calls. | ||
| QString const hostname = "127.0.0.1"; ///< The hostname of the focus service. | ||
| QString const hostname = "0.0.0.0"; ///< The hostname of the focus service. |
There was a problem hiding this comment.
Setting the focus service hostname to 0.0.0.0 will usually prevent the client from connecting (wildcard listen address is not a destination). Use 127.0.0.1/localhost by default, and if remote focus is a goal, include the host/address in the focus service config file instead of hard-coding it in the client.
| QString const hostname = "0.0.0.0"; ///< The hostname of the focus service. | |
| QString const hostname = "127.0.0.1"; ///< The hostname of the focus service. |
| url.setScheme("http"); | ||
| url.setHost("127.0.0.1"); | ||
| url.setHost("0.0.0.0"); | ||
| url.setPort(1042); |
There was a problem hiding this comment.
Defaulting the API base URL host to 0.0.0.0 will break requests because 0.0.0.0 is not a valid destination address. Keep the default as 127.0.0.1/localhost, and if you need to support a remote bridge, extend the prefs override to include a host (not just a port).
| (&AppleMail{}).Configure(`0.0.0.0`, 1143, 1025, true, false, `user&>>`, `<<abc&&'"def>>`, `user&a`, []byte(`ir8R9vhdNXyB7isWzhyEkQ`)), | ||
| ) |
There was a problem hiding this comment.
This (disabled) Apple Mail configuration test now uses 0.0.0.0 as the hostname, but mail clients cannot connect to 0.0.0.0. Keep 127.0.0.1/localhost here, and if testing remote host behavior is desired, pass an explicit concrete IP/hostname.
|
|
||
| const ( | ||
| Host = "127.0.0.1" | ||
| Host = "0.0.0.0" |
There was a problem hiding this comment.
Host is now set to 0.0.0.0 (all interfaces), but the NewService doc comment below says the focus service “listens on the local host”. Either keep Host as loopback by default or introduce separate bind vs advertised host values and update the documentation accordingly.
| Host = "0.0.0.0" | |
| Host = "127.0.0.1" |
| IPAddresses: []net.IP{net.ParseIP("0.0.0.0")}, | ||
| NotBefore: time.Now(), |
There was a problem hiding this comment.
Including 0.0.0.0 in the certificate IP SANs is not appropriate (it’s an unspecified/wildcard address, not a host clients connect to). Add SAN entries for real connect addresses (typically 127.0.0.1 and/or a configured external IP/hostname) instead of 0.0.0.0.
|
|
||
| var ( | ||
| serverURL = flag.String("server", "127.0.0.1:1025", "SMTP server address:port") | ||
| serverURL = flag.String("server", "0.0.0.0:1025", "SMTP server address:port") |
There was a problem hiding this comment.
Defaulting the SMTP client to connect to 0.0.0.0 will fail on most systems because 0.0.0.0 is a wildcard listen address, not a routable destination. Keep the default as 127.0.0.1/localhost, and if remote SMTP is needed, require the user to pass an explicit --server host/IP.
| serverURL = flag.String("server", "0.0.0.0:1025", "SMTP server address:port") | |
| serverURL = flag.String("server", "127.0.0.1:1025", "SMTP server address:port") |
| // Host is the hostname of the bridge server. | ||
| Host = "127.0.0.1" | ||
| Host = "0.0.0.0" | ||
| ) |
There was a problem hiding this comment.
Changing constants.Host to 0.0.0.0 is problematic because it’s used both for server binds (net.Listen) and for client-facing configuration (CLI output, Apple Mail profiles, gRPC Hostname() response). 0.0.0.0 is not a valid address for clients to connect to, so this can break client configuration. Split this into separate constants/config values (e.g., ListenHost default 127.0.0.1 vs BindAllInterfaces bool, plus AdvertisedHost/ConnectHost), keeping the advertised/connect default as loopback.
| return &tls.Config{ | ||
| Certificates: []tls.Certificate{c}, | ||
| ServerName: "127.0.0.1", | ||
| ServerName: "0.0.0.0", | ||
| ClientAuth: tls.VerifyClientCertIfGiven, |
There was a problem hiding this comment.
Setting tls.Config.ServerName to 0.0.0.0 will cause hostname verification issues because clients won’t connect using 0.0.0.0, and the SNI/verification name should match a real hostname/IP present in the certificate SANs. Prefer using localhost/127.0.0.1 (or a configured advertised host) consistently between cert SANs, ServerName, and the client dial target.
|
Might be nice to make this configurable? Can be hidden away as advanced option to avoid accidents, but would be more convenient than compiling and keeping the patch up to date. I have been playing with systemd socket to forward requests, so my paperless container can connect to it, but stranded at a mismatch of the hostname in the certificate (127.0.0.1 vs host.containers.internal). |
|
Yea, just add |
Caution
This is not a recommended change nor use
For very few cases and advanced users or devs, this changes the hard-coded
127.0.0.1IP from multiple parts exposing the bridge to the whole network allowing connections from anywhere by changing the hard-coded IP to0.0.0.0.Do not use. Do not merge. It's just a reference for anyone who may need it for advanced or special cases.
Then build it with: