Skip to content

Commit c86cb8a

Browse files
authored
fix(auth): bind OAuth callback listener to loopback only (#6238)
The interactive login callback server bound ":port" (all interfaces) while advertising a localhost redirect URL. For the lifetime of a login, any host on the LAN could probe the listener and abort the flow by hitting /callback?error=... (no state needed on the error path). Login-code theft is not possible: the 128-bit state is validated before token exchange, so the impact is login DoS only. Bind 127.0.0.1 explicitly, matching the redirect URL host and the loopback address the port-availability probe already uses. Added TestListenAddrLoopback asserting the listener address is loopback. Signed-off-by: Sasha Mitchell <sash.t.mitchell@gmail.com>
1 parent 6f04df4 commit c86cb8a

2 files changed

Lines changed: 30 additions & 1 deletion

File tree

pkg/auth/oauth/flow.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,14 @@ func (f *Flow) generateState() error {
187187
return nil
188188
}
189189

190+
// listenAddr returns the address the callback server binds to. The redirect
191+
// URL points at localhost, so the listener is bound to the IPv4 loopback
192+
// instead of all interfaces: a wildcard bind would expose the callback
193+
// endpoint to every host on the LAN for the lifetime of the login.
194+
func (f *Flow) listenAddr() string {
195+
return fmt.Sprintf("127.0.0.1:%d", f.port)
196+
}
197+
190198
// Start starts the OAuth authentication flow
191199
func (f *Flow) Start(ctx context.Context, skipBrowser bool) (*TokenResult, error) {
192200
// Create channels for communication
@@ -199,7 +207,7 @@ func (f *Flow) Start(ctx context.Context, skipBrowser bool) (*TokenResult, error
199207
mux.HandleFunc("/", f.handleRoot())
200208

201209
f.server = &http.Server{
202-
Addr: fmt.Sprintf(":%d", f.port),
210+
Addr: f.listenAddr(),
203211
Handler: mux,
204212
ReadHeaderTimeout: 10 * time.Second,
205213
}

pkg/auth/oauth/flow_test.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"encoding/base64"
1010
"encoding/json"
1111
"fmt"
12+
"net"
1213
"net/http"
1314
"net/http/httptest"
1415
"net/url"
@@ -739,6 +740,26 @@ func TestStart(t *testing.T) {
739740
}
740741
}
741742

743+
func TestListenAddrLoopback(t *testing.T) {
744+
t.Parallel()
745+
config := &Config{
746+
ClientID: "test-client",
747+
AuthURL: "https://example.com/auth",
748+
TokenURL: "https://example.com/token",
749+
}
750+
751+
flow, err := NewFlow(config)
752+
require.NoError(t, err)
753+
754+
host, port, err := net.SplitHostPort(flow.listenAddr())
755+
require.NoError(t, err)
756+
assert.Equal(t, fmt.Sprintf("%d", flow.port), port, "listen address should use the flow's callback port")
757+
758+
ip := net.ParseIP(host)
759+
require.NotNil(t, ip, "listen host should be an IP address, got %q", host)
760+
assert.True(t, ip.IsLoopback(), "callback listener must bind to a loopback address, got %q", host)
761+
}
762+
742763
func TestWriteSuccessPage(t *testing.T) {
743764
t.Parallel()
744765
flow := &Flow{}

0 commit comments

Comments
 (0)