Skip to content

Allows the bridge to be in a separate server not just localhost #519

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.

Sign up for GitHub

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

Open
gastonmorixe wants to merge 2 commits into
base: master
Choose a base branch
from
+27 −15
Open

Allows the bridge to be in a separate server not just localhost #519

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b9e11b7
make it local
gastonmorixe Sep 7, 2024
a4735d1
chore: add gitignore rules for local files
gastonmorixe Feb 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
godog.test
debug.test
coverage.html
coverage/
gobinsec-cache*.yml

# Run files
Expand All @@ -34,6 +35,7 @@ vendor-cache
cmd/Desktop-Bridge/deploy
cmd/Import-Export/deploy
proton-bridge
/bridge
cmd/Desktop-Bridge/*.exe
cmd/launcher/*.exe
bin/
Expand All @@ -48,3 +50,13 @@ _doc/
# gRPC auto-generated C++ source files
*.pb.cc
*.pb.h

# Local certificates (never commit)
*.pem
certs/

# Local service script
bridge_service.sh

# System research/audit docs
research/
2 changes: 1 addition & 1 deletion internal/bridge/debug.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func (bridge *Bridge) CheckClientState(ctx context.Context, checkFlags bool, pro
return result, err
}

addr := fmt.Sprintf("127.0.0.1:%v", bridge.GetIMAPPort())
addr := fmt.Sprintf("0.0.0.0:%v", bridge.GetIMAPPort())
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dialing the IMAP client at 0.0.0.0 will generally fail because 0.0.0.0 is not a valid destination address. Use 127.0.0.1/localhost (or the same advertised/connect host used elsewhere, e.g. constants.Host if it remains a connect host) when creating the IMAP client connection for diagnostics.

Suggested change
addr := fmt.Sprintf("0.0.0.0:%v", bridge.GetIMAPPort())
addr := fmt.Sprintf("127.0.0.1:%v", bridge.GetIMAPPort())

Copilot uses AI. Check for mistakes.

for account, mboxMap := range state {
if progressCB != nil {
Expand Down
6 changes: 3 additions & 3 deletions internal/certs/tls.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,13 @@ func NewTLSTemplate() (*x509.Certificate, error) {
Country: []string{"CH"},
Organization: []string{"Proton AG"},
OrganizationalUnit: []string{"Proton Mail"},
CommonName: "127.0.0.1",
CommonName: "0.0.0.0",
},
KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign,
ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth},
BasicConstraintsValid: true,
Comment on lines +52 to 56
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using 0.0.0.0 as the certificate CommonName is problematic because it’s a wildcard bind address rather than a real connection target, and won’t match typical client verification expectations. Prefer generating the cert with SANs for actual connect names (e.g., localhost and 127.0.0.1) and/or the configured advertised host when remote access is enabled.

Copilot uses AI. Check for mistakes.
IsCA: true,
IPAddresses: []net.IP{net.ParseIP("127.0.0.1")},
IPAddresses: []net.IP{net.ParseIP("0.0.0.0")},
NotBefore: time.Now(),
Comment on lines +58 to 59
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
NotAfter: time.Now().Add(20 * 365 * 24 * time.Hour),
}, nil
Expand Down Expand Up @@ -110,7 +110,7 @@ func GetConfig(certPEM, keyPEM []byte) (*tls.Config, error) {
//nolint:gosec // We need to support older TLS versions for AppleMail and Outlook
return &tls.Config{
Certificates: []tls.Certificate{c},
ServerName: "127.0.0.1",
ServerName: "0.0.0.0",
ClientAuth: tls.VerifyClientCertIfGiven,
Comment on lines 111 to 114
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
RootCAs: caCertPool,
ClientCAs: caCertPool,
Expand Down
2 changes: 1 addition & 1 deletion internal/clientconfig/applemail_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@ func TestEscapeXMLString(t *testing.T) {
func _TestInstallCert(t *testing.T) { //nolint:unused
require.NoError(
t,
(&AppleMail{}).Configure(`127.0.0.1`, 1143, 1025, true, false, `user&>>`, `<<abc&&'"def>>`, `user&a`, []byte(`ir8R9vhdNXyB7isWzhyEkQ`)),
(&AppleMail{}).Configure(`0.0.0.0`, 1143, 1025, true, false, `user&>>`, `<<abc&&'"def>>`, `user&a`, []byte(`ir8R9vhdNXyB7isWzhyEkQ`)),
)
Comment on lines +36 to 37
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
}
2 changes: 1 addition & 1 deletion internal/constants/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ const (
KeyChainName = "bridge-v3"

// Host is the hostname of the bridge server.
Host = "127.0.0.1"
Host = "0.0.0.0"
)
Comment on lines 70 to 72
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.

// nolint:goconst
Expand Down
2 changes: 1 addition & 1 deletion internal/focus/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import (
)

const (
Host = "127.0.0.1"
Host = "0.0.0.0"
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
Host = "0.0.0.0"
Host = "127.0.0.1"

Copilot uses AI. Check for mistakes.
serverConfigFileName = "grpcFocusServerConfig.json"
)

Expand Down
2 changes: 1 addition & 1 deletion internal/frontend/bridge-gui/bridge-gui-tester/GRPCServerWorker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ void GRPCServerWorker::run() {
builder.AddListeningPort(QString("unix://%1").arg(fileSocketPath).toStdString(), credentials);
config.fileSocketPath = fileSocketPath;
} else {
builder.AddListeningPort("127.0.0.1:0", credentials, &port);
builder.AddListeningPort("0.0.0.0:0", credentials, &port);
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Binding the tester gRPC server to 0.0.0.0 exposes it on all interfaces, which is risky even for a test helper and can cause port scanning / unintended access on developer machines. Prefer 127.0.0.1 unless there is a strong need for external access, in which case make it explicitly opt-in (config/flag) and document it.

Suggested change
builder.AddListeningPort("0.0.0.0:0", credentials, &port);
builder.AddListeningPort("127.0.0.1:0", credentials, &port);

Copilot uses AI. Check for mistakes.
}

builder.RegisterService(&app().grpc());
Expand Down
2 changes: 1 addition & 1 deletion internal/frontend/bridge-gui/bridge-gui/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ QUrl getApiUrl() {
QUrl url;
// use default url.
url.setScheme("http");
url.setHost("127.0.0.1");
url.setHost("0.0.0.0");
url.setPort(1042);
Comment on lines 157 to 159
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copilot uses AI. Check for mistakes.

// override with what can be found in the prefs.json file.
Expand Down
2 changes: 1 addition & 1 deletion internal/frontend/bridge-gui/bridgepp/bridgepp/FocusGRPC/FocusGRPCClient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ namespace {


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.
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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.

Copilot uses AI. Check for mistakes.


}
Expand Down
4 changes: 2 additions & 2 deletions internal/frontend/bridge-gui/bridgepp/bridgepp/GRPC/GRPCClient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,9 @@ void GRPCClient::connectToServer(QString const &sessionID, QString const &config
grpc::ChannelArguments chanArgs;
if (useFileSocketForGRPC()) {
address = QString("unix://" + config.fileSocketPath);
chanArgs.SetSslTargetNameOverride("127.0.0.1"); // for file socket, we skip name verification to avoid a confusion localhost/127.0.0.1
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);
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
address = QString("0.0.0.0:%1").arg(config.port);
address = QString("127.0.0.1:%1").arg(config.port);

Copilot uses AI. Check for mistakes.
}

SslCredentialsOptions opts;
Expand Down
2 changes: 1 addition & 1 deletion internal/frontend/grpc/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func NewService(
}
} else {
var err error
listener, err = net.Listen("tcp", "127.0.0.1:0") // Port should be provided by the OS.
listener, err = net.Listen("tcp", "0.0.0.0:0") // Port should be provided by the OS.
if err != nil {
logrus.WithError(err).Panic("Could not create gRPC listener")
Comment on lines +144 to 146
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Listening on 0.0.0.0:0 exposes the gRPC server on all interfaces. This is a security/operational change from loopback-only and also doesn't actually enable “separate server” use because the saved config only includes a port (no host) and the GUI client hard-codes the host. Consider keeping loopback default and introducing an explicit configurable listen/bind address plus an advertised/connection address in the config file when remote access is desired.

Copilot uses AI. Check for mistakes.
}
Expand Down
2 changes: 1 addition & 1 deletion utils/port-blocker/port-blocker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func runBlocker(startPort, endPort int) {
}

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))
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
listener, err := net.Listen("tcp", "0.0.0.0:"+strconv.Itoa(port))
listener, err := net.Listen("tcp", "127.0.0.1:"+strconv.Itoa(port))

Copilot uses AI. Check for mistakes.
if err != nil {
fmt.Printf("Port %v is already blocked. Skipping.\n", port)
} else {
Expand Down
2 changes: 1 addition & 1 deletion utils/smtp-send/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
)

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")
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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")

Copilot uses AI. Check for mistakes.
userName = flag.String("user-name", "user", "SMTP user name")
userPassword = flag.String("user-pwd", "password", "SMTP user password")
toAddr = flag.String("toAddr", "", "Address toAddr whom toAddr send the message")
Expand Down
Loading