gsadmin/spinifex
1
0
Fork 0
mirror of https://github.com/mulgadc/spinifex.git synced 2026-10-15 09:01:31 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: move client address handling to ingress

Browse Source
This commit is contained in:
joshsiv-mulga
2026-10-02 13:47:45 +10:00
parent c229a11714
commit 9f83f899a4
9 changed files with 111 additions and 91 deletions
Download Patch File Download Diff File Expand all files Collapse all files
spinifex/gateway/admin.go
+2 -2
View File
@@ -14,7 +14,7 @@ import (
"github.com/go-chi/chi/v5"
"github.com/mulgadc/spinifex/spinifex/admin"
"github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
"github.com/mulgadc/spinifex/spinifex/utils"
ingresshttp "github.com/mulgadc/spinifex/spinifex/ingress/http"
)
// adminMethods lists the callable /admin/<Method> names. A method absent here
@@ -168,7 +168,7 @@ func (gw *GatewayConfig) authorizeAdmin(r *http.Request, method string) error {
deny := func(reason string) error {
slog.Warn("Admin: access denied", "method", method, "reason", reason,
"service", service, "accountID", accountID, "identity", identity,
"principalType", principalType, "sourceIP", utils.ClientIP(r.RemoteAddr))
"principalType", principalType, "sourceIP", ingresshttp.ClientIP(r.RemoteAddr))
return errors.New(awserrors.ErrorAccessDenied)
}
spinifex/gateway/auth.go
+2 -2
View File
@@ -18,7 +18,7 @@ import (
gateway_eks "github.com/mulgadc/spinifex/spinifex/gateway/eks"
handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam"
handlers_sts "github.com/mulgadc/spinifex/spinifex/handlers/sts"
"github.com/mulgadc/spinifex/spinifex/utils"
ingresshttp "github.com/mulgadc/spinifex/spinifex/ingress/http"
)
// AKID prefixes. Prefix-first dispatch prevents a misfiled record from being
@@ -38,7 +38,7 @@ func (gw *GatewayConfig) SigV4AuthMiddleware() func(http.Handler) http.Handler {
}
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
clientIP := utils.ClientIP(r.RemoteAddr)
clientIP := ingresshttp.ClientIP(r.RemoteAddr)
if errCode := gw.RateLimiter.CheckIP(clientIP); errCode != "" {
gw.writeSigV4Error(w, r, errCode, "")
return
spinifex/gateway/gateway.go
+2 -1
View File
@@ -41,6 +41,7 @@ import (
handlers_ochrevector "github.com/mulgadc/spinifex/spinifex/handlers/ochrevector"
handlers_quota "github.com/mulgadc/spinifex/spinifex/handlers/quota"
handlers_sts "github.com/mulgadc/spinifex/spinifex/handlers/sts"
ingresshttp "github.com/mulgadc/spinifex/spinifex/ingress/http"
"github.com/mulgadc/spinifex/spinifex/utils"
"github.com/nats-io/nats.go"
"github.com/nats-io/nats.go/jetstream"
@@ -686,7 +687,7 @@ func requestConditionKeys(r *http.Request, principal principalContext) iampolicy
// Derived from the request rather than read from the context: the OCI
// registry chain never runs SigV4AuthMiddleware, so a context-carried
// address would be absent there and every aws:SourceIp condition inert.
if ip := utils.ClientIP(r.RemoteAddr); ip != "" {
if ip := ingresshttp.ClientIP(r.RemoteAddr); ip != "" {
keys[iampolicy.KeySourceIP] = ip
}
// Resolved from the credential record, never from anything the caller
spinifex/gateway/requestaudit.go
+2 -2
View File
@@ -12,7 +12,7 @@ import (
"go.opentelemetry.io/otel/trace"
"github.com/mulgadc/spinifex/spinifex/foundation/telemetry"
"github.com/mulgadc/spinifex/spinifex/utils"
ingresshttp "github.com/mulgadc/spinifex/spinifex/ingress/http"
)
// ctxAudit carries the per-request audit record. It is a pointer, unlike every
@@ -43,7 +43,7 @@ type requestAudit struct {
// logging happens to be enabled.
func requestAuditMiddleware(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
audit := &requestAudit{clientIP: utils.RequestClientIP(r), requestID: requestIDFrom(r)}
audit := &requestAudit{clientIP: ingresshttp.RequestClientIP(r), requestID: requestIDFrom(r)}
ctx := context.WithValue(r.Context(), ctxAudit, audit)
next.ServeHTTP(w, r.WithContext(ctx))
audit.annotate(ctx)
spinifex/ingress/http/client_ip.go
+35
View File
@@ -0,0 +1,35 @@
// Package http provides generic HTTP ingress helpers.
package http
import (
"net"
"net/http"
"net/netip"
"strings"
)
// RequestClientIP is the caller's address: X-Real-IP when the connection is
// from loopback, where only a local proxy (the nginx edge or console) can have
// set it, and RemoteAddr otherwise, so a direct client cannot choose what is
// logged.
func RequestClientIP(r *http.Request) string {
ip := ClientIP(r.RemoteAddr)
if addr, err := netip.ParseAddr(ip); err != nil || !addr.IsLoopback() {
return ip
}
realIP, err := netip.ParseAddr(strings.TrimSpace(r.Header.Get("X-Real-IP")))
if err != nil {
return ip
}
return realIP.String()
}
// ClientIP returns the IP from a RemoteAddr, stripping the port. Handles both
// IPv4 and IPv6, and tolerates a RemoteAddr that is already a bare IP (no port).
func ClientIP(remoteAddr string) string {
host, _, err := net.SplitHostPort(remoteAddr)
if err != nil {
return remoteAddr
}
return host
}
spinifex/ingress/http/client_ip_test.go
+66
View File
@@ -0,0 +1,66 @@
package http
import (
"net/http"
"net/http/httptest"
"testing"
)
func TestClientIP(t *testing.T) {
tests := []struct {
name string
remoteAddr string
expected string
}{
{"IPv4 with port", "192.168.1.1:12345", "192.168.1.1"},
{"IPv6 with port", "[::1]:12345", "::1"},
{"IPv4 bare", "192.168.1.1", "192.168.1.1"},
{"IPv6 full with port", "[2001:db8::1]:443", "2001:db8::1"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := ClientIP(tt.remoteAddr); got != tt.expected {
t.Errorf("ClientIP(%q) = %q, want %q", tt.remoteAddr, got, tt.expected)
}
})
}
}
// X-Real-IP is honoured only on a loopback connection, where the local edge set
// it. From anywhere else it, and X-Forwarded-For, are the client's own claim.
func TestRequestClientIP(t *testing.T) {
tests := []struct {
name string
remoteAddr string
realIP string
xff string
expected string
}{
{"loopback trusts X-Real-IP", "127.0.0.1:40000", "198.51.100.7", "", "198.51.100.7"},
{"IPv6 loopback trusts X-Real-IP", "[::1]:40000", "2001:db8::7", "", "2001:db8::7"},
{"4in6 loopback trusts X-Real-IP", "[::ffff:127.0.0.1]:40000", "198.51.100.7", "", "198.51.100.7"},
{"loopback without header", "127.0.0.1:40000", "", "", "127.0.0.1"},
{"loopback with garbage header", "127.0.0.1:40000", "not-an-ip", "", "127.0.0.1"},
{"loopback ignores X-Forwarded-For", "127.0.0.1:40000", "", "203.0.113.1", "127.0.0.1"},
{"spoofed X-Real-IP from public client", "203.0.113.9:5555", "10.0.0.1", "", "203.0.113.9"},
{"spoofed X-Forwarded-For from public client", "203.0.113.9:5555", "", "127.0.0.1", "203.0.113.9"},
{"spoofed both from private client", "10.2.0.3:5555", "127.0.0.1", "127.0.0.1", "10.2.0.3"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
r := httptest.NewRequest(http.MethodGet, "/", nil)
r.RemoteAddr = tt.remoteAddr
if tt.realIP != "" {
r.Header.Set("X-Real-IP", tt.realIP)
}
if tt.xff != "" {
r.Header.Set("X-Forwarded-For", tt.xff)
}
if got := RequestClientIP(r); got != tt.expected {
t.Errorf("RequestClientIP() = %q, want %q", got, tt.expected)
}
})
}
}
spinifex/runtime/roles/spinifexui/proxy.go
+2 -2
View File
@@ -14,7 +14,7 @@ import (
"time"
"github.com/mulgadc/bluebottle/pkg/tlsconfig"
"github.com/mulgadc/spinifex/spinifex/utils"
ingresshttp "github.com/mulgadc/spinifex/spinifex/ingress/http"
"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
)
@@ -63,7 +63,7 @@ func newReverseProxy(backendHost, pathPrefix string, transport http.RoundTripper
// The backend trusts X-Real-IP from loopback, which this hop always is.
// The UI also listens publicly, so re-derive it rather than forward a
// client's own claim. X-Request-ID passes through unchanged.
pr.Out.Header.Set("X-Real-IP", utils.RequestClientIP(pr.In))
pr.Out.Header.Set("X-Real-IP", ingresshttp.RequestClientIP(pr.In))
// Strip the prefix from both path forms. RawPath must survive: a
// client that percent-encodes a segment (a ':' in a bedrock
spinifex/utils/utils.go
-27
View File
@@ -14,12 +14,10 @@ import (
"log/slog"
"net"
"net/http"
"net/netip"
"os"
"os/signal"
"reflect"
"slices"
"strings"
"time"
"github.com/aws/aws-sdk-go/aws"
@@ -415,31 +413,6 @@ func HashMAC(id string) string {
return net.HardwareAddr(b).String()
}
// RequestClientIP is the caller's address: X-Real-IP when the connection is from
// loopback, where only a local proxy (the nginx edge, the console) can have set
// it, and RemoteAddr otherwise, so a direct client cannot choose what is logged.
func RequestClientIP(r *http.Request) string {
ip := ClientIP(r.RemoteAddr)
if addr, err := netip.ParseAddr(ip); err != nil || !addr.IsLoopback() {
return ip
}
realIP, err := netip.ParseAddr(strings.TrimSpace(r.Header.Get("X-Real-IP")))
if err != nil {
return ip
}
return realIP.String()
}
// ClientIP returns the IP from a RemoteAddr, stripping the port. Handles both
// IPv4 and IPv6, and tolerates a RemoteAddr that is already a bare IP (no port).
func ClientIP(remoteAddr string) string {
host, _, err := net.SplitHostPort(remoteAddr)
if err != nil {
return remoteAddr
}
return host
}
func dirExists(path string) bool {
info, err := os.Stat(path)
if os.IsNotExist(err) {
spinifex/utils/utils_test.go
-55
View File
@@ -1775,25 +1775,6 @@ func TestDistroFamily(t *testing.T) {
}
}
func TestClientIP(t *testing.T) {
tests := []struct {
name string
remoteAddr string
expected string
}{
{"IPv4 with port", "192.168.1.1:12345", "192.168.1.1"},
{"IPv6 with port", "[::1]:12345", "::1"},
{"IPv4 bare", "192.168.1.1", "192.168.1.1"},
{"IPv6 full with port", "[2001:db8::1]:443", "2001:db8::1"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.expected, ClientIP(tt.remoteAddr))
})
}
}
func TestAvailableImages_ECSNodeEntry(t *testing.T) {
img, ok := AvailableImages["spinifex-ecs-node"]
require.True(t, ok, "spinifex-ecs-node must be in the system image catalog")
@@ -1899,39 +1880,3 @@ func TestWithRequestID_PointerToStruct(t *testing.T) {
require.Equal(t, "req-456", v.FieldByName("RequestId").String())
require.Equal(t, []string{"b"}, v.FieldByName("Names").Interface())
}
// X-Real-IP is honoured only on a loopback connection, where the local edge set
// it. From anywhere else it, and X-Forwarded-For, are the client's own claim.
func TestRequestClientIP(t *testing.T) {
tests := []struct {
name string
remoteAddr string
realIP string
xff string
expected string
}{
{"loopback trusts X-Real-IP", "127.0.0.1:40000", "198.51.100.7", "", "198.51.100.7"},
{"IPv6 loopback trusts X-Real-IP", "[::1]:40000", "2001:db8::7", "", "2001:db8::7"},
{"4in6 loopback trusts X-Real-IP", "[::ffff:127.0.0.1]:40000", "198.51.100.7", "", "198.51.100.7"},
{"loopback without header", "127.0.0.1:40000", "", "", "127.0.0.1"},
{"loopback with garbage header", "127.0.0.1:40000", "not-an-ip", "", "127.0.0.1"},
{"loopback ignores X-Forwarded-For", "127.0.0.1:40000", "", "203.0.113.1", "127.0.0.1"},
{"spoofed X-Real-IP from public client", "203.0.113.9:5555", "10.0.0.1", "", "203.0.113.9"},
{"spoofed X-Forwarded-For from public client", "203.0.113.9:5555", "", "127.0.0.1", "203.0.113.9"},
{"spoofed both from private client", "10.2.0.3:5555", "127.0.0.1", "127.0.0.1", "10.2.0.3"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
r := httptest.NewRequest(http.MethodGet, "/", nil)
r.RemoteAddr = tt.remoteAddr
if tt.realIP != "" {
r.Header.Set("X-Real-IP", tt.realIP)
}
if tt.xff != "" {
r.Header.Set("X-Forwarded-For", tt.xff)
}
assert.Equal(t, tt.expected, RequestClientIP(r))
})
}
}