From a06603858ebfd5c88be429b04bbc149653452615 Mon Sep 17 00:00:00 2001 From: Suryansh Prajapati <58465650+drk1rd@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:07:16 +0530 Subject: [PATCH 01/18] awsapi: authenticate before buffering request bodies; require host and x-amz-target to be signed The AWS handler read up to 100 MB of body before looking at the signature, so anyone could make the server buffer that much per connection. It now checks the signature's freshness and access key first, verifies the signature before reading the body when the client declared the payload hash, and caps unsigned (public operation) bodies at 1 MB. SigV4 requests that do not sign host, or x-amz-target when present, are rejected as AWS does. --- cli/internal/awsapi/awsapi.go | 62 +++++++++++++++------- cli/internal/awsapi/preauth_test.go | 79 +++++++++++++++++++++++++++++ cli/internal/awsapi/sigv4.go | 9 ++++ 3 files changed, 133 insertions(+), 17 deletions(-) create mode 100644 cli/internal/awsapi/preauth_test.go diff --git a/cli/internal/awsapi/awsapi.go b/cli/internal/awsapi/awsapi.go index 500ee3ca..2bfaeaea 100644 --- a/cli/internal/awsapi/awsapi.go +++ b/cli/internal/awsapi/awsapi.go @@ -311,7 +311,11 @@ func Match(r *http.Request) bool { return lookupUnsigned(r) != nil } -const maxBody = 100 << 20 +const ( + maxBody = 100 << 20 + // maxPublicBody bounds what an unsigned (public operation) request may send. + maxPublicBody = 1 << 20 +) func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { start := time.Now() @@ -388,19 +392,49 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { q.Protocol = Query } - // Read the body (unless the service streams it) and check the signature. + // Before buffering a body, make sure the caller is worth it: the signature + // must be fresh and name a live access key, and when the client declared the + // payload hash the signature is verified before the body is read. Otherwise + // anyone could make the server buffer maxBody bytes per connection. payloadHash := "" + var secret string + var principal *httpx.Principal + verified := false + now := time.Now() + if h.Now != nil { + now = h.Now() + } if sig != nil { payloadHash = sig.PayloadHash + if err := sig.checkTime(now); err != nil { + q.fail(err) + return + } + var err error + if secret, principal, err = h.Creds.SigningSecret(sig.AccessKeyID, sig.SessionToken); err != nil { + q.fail(err) + return + } + if payloadHash != "" && !strings.HasPrefix(payloadHash, "STREAMING-") || sig.Presigned { + if err := sig.Verify(r, secret, payloadHash, now); err != nil { + q.fail(err) + return + } + verified = true + } } if !(q.Svc.StreamBody && q.Protocol == REST) { - b, err := io.ReadAll(io.LimitReader(r.Body, maxBody+1)) + limit := int64(maxBody) + if sig == nil { + limit = maxPublicBody // only public operations are reachable unsigned + } + b, err := io.ReadAll(io.LimitReader(r.Body, limit+1)) if err != nil { q.fail(Errorf(http.StatusBadRequest, "IncompleteBody", "read body: %v", err)) return } - if len(b) > maxBody { - q.fail(Errorf(http.StatusRequestEntityTooLarge, "RequestEntityTooLarge", "request body exceeds %d MB", maxBody>>20)) + if int64(len(b)) > limit { + q.fail(Errorf(http.StatusRequestEntityTooLarge, "RequestEntityTooLarge", "request body exceeds %d MB", limit>>20)) return } q.Body = b @@ -431,18 +465,12 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { public := (q.Svc.PublicOps[q.Op] && q.Protocol != REST) || (q.Protocol == REST && q.Svc.Unsigned != nil) if sig != nil { - now := time.Now() - if h.Now != nil { - now = h.Now() - } - secret, p, err := h.Creds.SigningSecret(sig.AccessKeyID, sig.SessionToken) - if err != nil { - q.fail(err) - return - } - if err := sig.Verify(r, secret, payloadHash, now); err != nil { - q.fail(err) - return + p := principal + if !verified { + if err := sig.Verify(r, secret, payloadHash, now); err != nil { + q.fail(err) + return + } } q.Sig, q.Secret, q.P = sig, secret, p p.AddRequestContext(r) diff --git a/cli/internal/awsapi/preauth_test.go b/cli/internal/awsapi/preauth_test.go new file mode 100644 index 00000000..48e79acf --- /dev/null +++ b/cli/internal/awsapi/preauth_test.go @@ -0,0 +1,79 @@ +package awsapi + +import ( + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/homecloudhq/homecloud/cli/internal/httpx" +) + +type noCreds struct{} + +func (noCreds) SigningSecret(string, string) (string, *httpx.Principal, error) { + return "", nil, Errorf(http.StatusForbidden, "InvalidClientTokenId", "The security token included in the request is invalid.") +} + +// tripBody fails the test when the server reads it. +type tripBody struct{ t *testing.T } + +func (b tripBody) Read([]byte) (int, error) { + b.t.Error("the body of an unauthenticated request was read") + return 0, errors.New("read") +} +func (tripBody) Close() error { return nil } + +const testSvc = "regsvc" + +func init() { + Register(&Service{Name: testSvc, JSONPrefix: "RegSvc", Ops: map[string]Op{"Ping": func(*Req) (any, error) { return nil, nil }}, + PublicOps: map[string]bool{"Open": true}}) +} + +func signedReq(t *testing.T, signed string, body *strings.Reader) *http.Request { + now := time.Now().UTC().Format(amzDateFormat) + r := httptest.NewRequest(http.MethodPost, "/", body) + r.Header.Set("X-Amz-Date", now) + r.Header.Set("X-Amz-Target", "RegSvc.Ping") + r.Header.Set("Authorization", "AWS4-HMAC-SHA256 Credential=AKIAUNKNOWN/"+now[:8]+"/us-east-1/"+testSvc+"/aws4_request, SignedHeaders="+signed+", Signature=00") + return r +} + +// A request signed with an unknown access key must be refused before its body +// is buffered: otherwise anyone can make the server allocate up to maxBody bytes +// per connection. +func TestBodyNotReadBeforeAuthentication(t *testing.T) { + h := &Handler{Creds: noCreds{}} + r := signedReq(t, "host;x-amz-date;x-amz-target", strings.NewReader("")) + r.Body = tripBody{t} + w := httptest.NewRecorder() + h.ServeHTTP(w, r) + if w.Code != http.StatusForbidden { + t.Fatalf("status %d: %s", w.Code, w.Body) + } +} + +func TestUnsignedBodyIsCapped(t *testing.T) { + h := &Handler{Creds: noCreds{}} + r := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(strings.Repeat("a", maxPublicBody+10))) + r.Header.Set("X-Amz-Target", "RegSvc.Open") + w := httptest.NewRecorder() + h.ServeHTTP(w, r) + if w.Code != http.StatusRequestEntityTooLarge { + t.Fatalf("status %d: %s", w.Code, w.Body) + } +} + +func TestHostAndTargetMustBeSigned(t *testing.T) { + for _, signed := range []string{"x-amz-date", "host;x-amz-date"} { + if _, err := ParseSignature(signedReq(t, signed, strings.NewReader(""))); err == nil { + t.Errorf("SignedHeaders=%s accepted", signed) + } + } + if _, err := ParseSignature(signedReq(t, "host;x-amz-date;x-amz-target", strings.NewReader(""))); err != nil { + t.Error(err) + } +} diff --git a/cli/internal/awsapi/sigv4.go b/cli/internal/awsapi/sigv4.go index 280fb723..9e302382 100644 --- a/cli/internal/awsapi/sigv4.go +++ b/cli/internal/awsapi/sigv4.go @@ -91,6 +91,15 @@ func ParseSignature(r *http.Request) (*Signature, error) { if len(parts) != 5 || parts[4] != "aws4_request" || s.Signature == "" || len(s.SignedHeaders) == 0 { return nil, Errorf(http.StatusBadRequest, "IncompleteSignature", "the request signature is malformed") } + // As AWS does, insist that the host (and the operation header) are signed, so a + // captured signature can't be replayed against another host or operation. + signed := map[string]bool{} + for _, h := range s.SignedHeaders { + signed[h] = true + } + if !signed["host"] || (r.Header.Get("X-Amz-Target") != "" && !signed["x-amz-target"]) { + return nil, Errorf(http.StatusForbidden, "SignatureDoesNotMatch", "the host and x-amz-target headers must be signed") + } s.AccessKeyID, s.Date, s.Region, s.Service = parts[0], parts[1], parts[2], parts[3] t, err := time.Parse(amzDateFormat, dateStr) if err != nil { From 196abfcd466b2feced5e652287beebfcffc334fc Mon Sep 17 00:00:00 2001 From: Suryansh Prajapati <58465650+drk1rd@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:08:15 +0530 Subject: [PATCH 02/18] Throttle sign-in before the password check, hide user existence, cap public request bodies The console sign-in counted a failure only after verifying the password, so a burst of parallel guesses all passed the throttle; it now reserves a slot first and gives it back on success. Unknown users pay the same bcrypt cost as known ones. Sign-in and Cognito public routes cap their JSON body at 64 KB instead of 64 MB, and servers get an idle timeout and a header size limit. --- cli/internal/httpx/httpx.go | 12 ++++++++ cli/internal/server/server.go | 4 +-- cli/internal/server/workloads.go | 2 +- cli/internal/svc/cognito/cognito.go | 10 +++--- cli/internal/svc/iam/iam.go | 27 +++++++++++++++++ cli/internal/svc/iam/login_test.go | 47 +++++++++++++++++++++++++++++ cli/internal/svc/iam/routes.go | 16 +++++++--- 7 files changed, 106 insertions(+), 12 deletions(-) create mode 100644 cli/internal/svc/iam/login_test.go diff --git a/cli/internal/httpx/httpx.go b/cli/internal/httpx/httpx.go index 179b8bc4..7ecae506 100644 --- a/cli/internal/httpx/httpx.go +++ b/cli/internal/httpx/httpx.go @@ -91,8 +91,17 @@ type routeOpts struct { resource string public bool deferred bool + maxBody int64 } +// PublicBodyLimit is the request body cap of routes marked SmallBody: sign-in +// and similar calls whose bodies are a few hundred bytes. +const PublicBodyLimit = 64 << 10 + +// SmallBody caps the request body at PublicBodyLimit; use it on public routes, +// which are reachable without credentials. +func SmallBody() Opt { return func(o *routeOpts) { o.maxBody = PublicBodyLimit } } + type Opt func(*routeOpts) // Res sets the resource ARN template checked against the route's action; @@ -124,6 +133,9 @@ func (rt *Router) Handle(pattern, action string, h Handler, opts ...Opt) { c := &Ctx{W: w, R: r, Account: rt.Account} sw := &statusWriter{ResponseWriter: w, status: 200} c.W = sw + if o.maxBody > 0 { + r.Body = http.MaxBytesReader(sw, r.Body, o.maxBody) + } resource := strings.ReplaceAll(strings.ReplaceAll(o.resource, "{account}", rt.Account), "{region}", core.Region) resource = placeholder.ReplaceAllStringFunc(resource, func(m string) string { return r.PathValue(m[1 : len(m)-1]) diff --git a/cli/internal/server/server.go b/cli/internal/server/server.go index 4f272534..ca226905 100644 --- a/cli/internal/server/server.go +++ b/cli/internal/server/server.go @@ -400,7 +400,7 @@ func Run(ctx context.Context, cfg core.Config, opts Options) error { return err } var handler http.Handler = trust.Wrap(root) - srv := &http.Server{Addr: cfg.APIAddr, Handler: handler, ReadHeaderTimeout: 10 * time.Second} + srv := &http.Server{Addr: cfg.APIAddr, Handler: handler, ReadHeaderTimeout: 10 * time.Second, IdleTimeout: 2 * time.Minute, MaxHeaderBytes: 256 << 10} if workloadLn != nil { // Workloads are never proxies: their listeners ignore forwarding headers. defer serveWorkloads(workloadLn, root, logf)() @@ -419,7 +419,7 @@ func Run(ctx context.Context, cfg core.Config, opts Options) error { if goruntime.GOOS == "linux" { if host, _, _ := net.SplitHostPort(cfg.APIAddr); host == "127.0.0.1" || host == "localhost" { if gw := dk.BridgeGateway(); gw != "" { - extra := &http.Server{Addr: net.JoinHostPort(gw, apiPort), Handler: root, ReadHeaderTimeout: 10 * time.Second} + extra := &http.Server{Addr: net.JoinHostPort(gw, apiPort), Handler: root, ReadHeaderTimeout: 10 * time.Second, IdleTimeout: 2 * time.Minute, MaxHeaderBytes: 256 << 10} go func() { var err error if cfg.TLSCert != "" { diff --git a/cli/internal/server/workloads.go b/cli/internal/server/workloads.go index c4bf7f77..e2fb8b74 100644 --- a/cli/internal/server/workloads.go +++ b/cli/internal/server/workloads.go @@ -32,7 +32,7 @@ func workloadListenAddr(goos, bridgeGateway string) (addr string, ok bool) { // serveWorkloads serves h on l until the server is closed; the returned // function stops it. func serveWorkloads(l net.Listener, h http.Handler, logf func(string, ...any)) (stop func()) { - srv := &http.Server{Handler: h, ReadHeaderTimeout: 10 * time.Second} + srv := &http.Server{Handler: h, ReadHeaderTimeout: 10 * time.Second, IdleTimeout: 2 * time.Minute, MaxHeaderBytes: 256 << 10} go func() { if err := srv.Serve(l); err != nil && !errors.Is(err, http.ErrServerClosed) { logf("workload endpoint %s: %v", l.Addr(), err) diff --git a/cli/internal/svc/cognito/cognito.go b/cli/internal/svc/cognito/cognito.go index 33aacec6..ee51f7e4 100644 --- a/cli/internal/svc/cognito/cognito.go +++ b/cli/internal/svc/cognito/cognito.go @@ -952,12 +952,12 @@ func (s *Service) removeGroup(pool, g string) (Pool, error) { // ---- public (application-facing) routes ---- func (s *Service) publicRoutes(r *httpx.Router) { - r.Handle("POST /cognito/{pool}/sign-up", "", s.signUp, httpx.Public()) - r.Handle("POST /cognito/{pool}/auth", "", s.auth, httpx.Public()) - r.Handle("POST /cognito/{pool}/respond", "", s.respond, httpx.Public()) - r.Handle("POST /cognito/{pool}/change-password", "", s.changePassword, httpx.Public()) + r.Handle("POST /cognito/{pool}/sign-up", "", s.signUp, httpx.Public(), httpx.SmallBody()) + r.Handle("POST /cognito/{pool}/auth", "", s.auth, httpx.Public(), httpx.SmallBody()) + r.Handle("POST /cognito/{pool}/respond", "", s.respond, httpx.Public(), httpx.SmallBody()) + r.Handle("POST /cognito/{pool}/change-password", "", s.changePassword, httpx.Public(), httpx.SmallBody()) r.Handle("GET /cognito/{pool}/userinfo", "", s.userinfo, httpx.Public()) - r.Handle("POST /cognito/{pool}/sign-out", "", s.signOut, httpx.Public()) + r.Handle("POST /cognito/{pool}/sign-out", "", s.signOut, httpx.Public(), httpx.SmallBody()) r.Handle("GET /cognito/{pool}/.well-known/jwks.json", "", s.jwks, httpx.Public()) r.Handle("GET /cognito/{pool}/.well-known/openid-configuration", "", s.discovery, httpx.Public()) } diff --git a/cli/internal/svc/iam/iam.go b/cli/internal/svc/iam/iam.go index f97db255..58cda2fe 100644 --- a/cli/internal/svc/iam/iam.go +++ b/cli/internal/svc/iam/iam.go @@ -189,6 +189,33 @@ func (s *Service) throttled(ip string, fail bool) bool { return len(recent) >= maxFailures } +// releaseAttempt gives back the throttle slot of a sign-in that succeeded. +func (s *Service) releaseAttempt(ip string) { + s.failMu.Lock() + defer s.failMu.Unlock() + if l := s.failures[ip]; len(l) > 1 { + s.failures[ip] = l[:len(l)-1] + } else { + delete(s.failures, ip) + } +} + +var ( + dummyOnce sync.Once + dummyBcr string +) + +// dummyHash is a valid bcrypt hash nobody knows the password of, compared +// against when a sign-in names an unknown user so the response time doesn't +// reveal which user names exist. +func dummyHash() string { + dummyOnce.Do(func() { + h, _ := bcrypt.GenerateFromPassword([]byte(core.NewSecret(24)), bcrypt.DefaultCost) + dummyBcr = string(h) + }) + return dummyBcr +} + // BootstrapResult carries credentials that exist only at first start. type BootstrapResult struct { AccountID string diff --git a/cli/internal/svc/iam/login_test.go b/cli/internal/svc/iam/login_test.go new file mode 100644 index 00000000..4dc03fb8 --- /dev/null +++ b/cli/internal/svc/iam/login_test.go @@ -0,0 +1,47 @@ +package iam_test + +import ( + "bytes" + "net/http" + "strings" + "sync" + "testing" + + "github.com/homecloudhq/homecloud/cli/internal/awsapi/awstest" +) + +// A burst of parallel guesses must not get past the throttle just because none +// of them had failed yet when the others were checked. +func TestLoginThrottleHoldsUnderParallelBurst(t *testing.T) { + h := awstest.New(t) + var mu sync.Mutex + codes := map[int]int{} + var wg sync.WaitGroup + for i := 0; i < 60; i++ { + wg.Add(1) + go func() { + defer wg.Done() + st := loginStatus(t, h, "root", "wrong-password") + mu.Lock() + codes[st]++ + mu.Unlock() + }() + } + wg.Wait() + if codes[401] > 10 || codes[429] < 50 { + t.Fatalf("status counts %v: more than 10 guesses were evaluated", codes) + } +} + +func TestLoginRejectsHugeBodies(t *testing.T) { + h := awstest.New(t) + body := `{"username":"root","password":"` + strings.Repeat("a", 200<<10) + `"}` + resp, err := http.Post(h.URL+"/api/v1/auth/login", "application/json", bytes.NewReader([]byte(body))) + if err != nil { + t.Fatal(err) + } + resp.Body.Close() + if resp.StatusCode != http.StatusBadRequest { + t.Fatalf("status %d", resp.StatusCode) + } +} diff --git a/cli/internal/svc/iam/routes.go b/cli/internal/svc/iam/routes.go index 8ddb2029..84993158 100644 --- a/cli/internal/svc/iam/routes.go +++ b/cli/internal/svc/iam/routes.go @@ -16,7 +16,7 @@ import ( func (s *Service) Routes(r *httpx.Router) { iamRes := func(kind string) httpx.Opt { return httpx.Res("arn:aws:iam::{account}:" + kind) } - r.Handle("POST /api/v1/auth/login", "", s.login, httpx.Public()) + r.Handle("POST /api/v1/auth/login", "", s.login, httpx.Public(), httpx.SmallBody()) r.Handle("POST /api/v1/auth/logout", "sts:Logout", s.logout) r.Handle("GET /api/v1/auth/whoami", "sts:GetCallerIdentity", s.whoami) @@ -70,15 +70,23 @@ func (s *Service) login(c *httpx.Ctx) (any, error) { return nil, err } ip := httpx.ClientIP(c.R) - if s.throttled(ip, false) { + // Count the attempt before checking the password, so a burst of parallel + // requests cannot all pass the throttle before the first failure is recorded; + // a successful sign-in gives its attempt back. + if s.throttled(ip, true) { + s.releaseAttempt(ip) return nil, core.Errf(http.StatusTooManyRequests, "TooManyRequests", "too many failed sign-in attempts; try again in a few minutes") } u, err := store.Get[User](s.env.Store, cUsers, in.Username) - if err != nil || u.PasswordHash == "" || bcrypt.CompareHashAndPassword([]byte(u.PasswordHash), []byte(in.Password)) != nil { - s.throttled(ip, true) + hash := u.PasswordHash + if err != nil || hash == "" { + hash = dummyHash() // spend the same time whether or not the user exists + } + if bcrypt.CompareHashAndPassword([]byte(hash), []byte(in.Password)) != nil || err != nil || u.PasswordHash == "" { time.Sleep(300 * time.Millisecond) return nil, core.Errf(http.StatusUnauthorized, "AuthFailure", "incorrect user name or password") } + s.releaseAttempt(ip) token := "hcs_" + core.NewSecret(40) exp := time.Now().Add(sessionTTL) if err := store.Put(s.env.Store, cSessions, hashSecret(token), session{Token: "", UserName: u.Name, UserID: u.ID, Expires: exp}); err != nil { From 60001980498f413d7292abb688494de1727d1fcc Mon Sep 17 00:00:00 2001 From: Suryansh Prajapati <58465650+drk1rd@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:10:36 +0530 Subject: [PATCH 03/18] Guard API Gateway HTTP_PROXY integrations and widen the outbound target blocklist HTTP_PROXY integrations used a default HTTP client, so anyone able to create an integration could read loopback services, the VPS provider's metadata service or the Docker bridge through a public API. They now use core.SafeClient, which checks the resolved address at dial time, ignores proxy environment variables and does not follow redirects. The shared blocklist now also covers IPv4-mapped/NAT64/6to4 forms, 0.0.0.0/8, non-link-local metadata addresses and every address of the host itself; HOMECLOUD_DENY_PRIVATE_TARGETS=1 also blocks RFC 1918 and unique-local ranges. --- cli/internal/core/targets.go | 71 ++++++++++++++++++++-- cli/internal/core/targets_test.go | 34 +++++++++++ cli/internal/svc/lambda/apigw_aws_test.go | 1 + cli/internal/svc/lambda/apigw_serve.go | 2 +- cli/internal/svc/lambda/apigw_ssrf_test.go | 29 +++++++++ cli/internal/svc/lambda/lambda.go | 5 +- 6 files changed, 135 insertions(+), 7 deletions(-) create mode 100644 cli/internal/core/targets_test.go create mode 100644 cli/internal/svc/lambda/apigw_ssrf_test.go diff --git a/cli/internal/core/targets.go b/cli/internal/core/targets.go index 1c3a4b92..fa0bb162 100644 --- a/cli/internal/core/targets.go +++ b/cli/internal/core/targets.go @@ -7,6 +7,7 @@ import ( "net" "net/http" "net/url" + "os" "strings" "syscall" "time" @@ -32,13 +33,73 @@ func TargetAction(arn string) string { return "" } -// blockedIP reports addresses outbound webhooks may not reach: loopback, -// link-local (including cloud metadata), unspecified and multicast. +// blockedNets are ranges outbound requests made on behalf of users never reach: +// "this network", the cloud metadata services of VPS providers that sit outside +// link-local space, and IPv6 prefixes that embed IPv4 addresses (NAT64, 6to4), +// which could smuggle a blocked IPv4 address past the checks. +var blockedNets = func() []*net.IPNet { + var out []*net.IPNet + for _, c := range []string{"0.0.0.0/8", "100.100.100.200/32", "192.0.0.192/32", "fd00:ec2::/32", "64:ff9b::/96", "64:ff9b:1::/48", "2002::/16", "::/128"} { + _, n, _ := net.ParseCIDR(c) + out = append(out, n) + } + return out +}() + +// DenyPrivateTargets, when set (HOMECLOUD_DENY_PRIVATE_TARGETS=1), also blocks +// RFC 1918 / unique-local addresses. They stay reachable by default because +// self-hosters legitimately call services on their LAN and in their VPCs. +var DenyPrivateTargets = os.Getenv("HOMECLOUD_DENY_PRIVATE_TARGETS") == "1" + +// blockedIP reports addresses outbound requests made for users may not reach: +// loopback, link-local (including the 169.254.169.254 metadata service), +// unspecified, multicast, the ranges in blockedNets and every address of this +// host itself (its public address and the Docker bridge gateways, through which +// workloads and users would otherwise reach the HomeCloud API and the Docker +// daemon). func blockedIP(ip net.IP) bool { - return ip.IsLoopback() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsUnspecified() || ip.IsMulticast() || ip.IsInterfaceLocalMulticast() + if v4 := ip.To4(); v4 != nil { + ip = v4 // IPv4-mapped IPv6 addresses are IPv4 addresses + } + if ip.IsLoopback() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsUnspecified() || ip.IsMulticast() || ip.IsInterfaceLocalMulticast() { + return true + } + for _, n := range blockedNets { + if n.Contains(ip) { + return true + } + } + if DenyPrivateTargets && ip.IsPrivate() { + return true + } + addrs, _ := net.InterfaceAddrs() + for _, a := range addrs { + if n, ok := a.(*net.IPNet); ok && n.IP.Equal(ip) { + return true + } + } + return false } -var errBlocked = errors.New("destination address is not allowed (loopback, link-local or unspecified)") +var errBlocked = errors.New("destination address is not allowed (loopback, link-local, metadata or this host)") + +// SafeClient is an HTTP client for requests to user-supplied URLs. It refuses +// to connect to blocked addresses at dial time (after DNS resolution, so +// rebinding and redirects cannot reach them), ignores proxy environment +// variables and follows at most maxRedirects redirects. +func SafeClient(timeout time.Duration, maxRedirects int) *http.Client { + c := WebhookClient(timeout) + c.CheckRedirect = func(req *http.Request, via []*http.Request) error { + if len(via) > maxRedirects { + if maxRedirects == 0 { + return http.ErrUseLastResponse + } + return errors.New("too many redirects") + } + return CheckWebhookURL(req.URL.String()) + } + return c +} // CheckWebhookURL validates an outbound webhook URL. func CheckWebhookURL(raw string) error { @@ -70,7 +131,7 @@ func WebhookClient(timeout time.Duration) *http.Client { }} tr := &http.Transport{DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) { return dialer.DialContext(ctx, network, addr) - }, TLSHandshakeTimeout: 10 * time.Second, MaxIdleConns: 10} + }, TLSHandshakeTimeout: 10 * time.Second, MaxIdleConns: 10, Proxy: nil} return &http.Client{Timeout: timeout, Transport: tr, CheckRedirect: func(req *http.Request, via []*http.Request) error { if len(via) >= 3 { return errors.New("too many redirects") diff --git a/cli/internal/core/targets_test.go b/cli/internal/core/targets_test.go new file mode 100644 index 00000000..24d4ca27 --- /dev/null +++ b/cli/internal/core/targets_test.go @@ -0,0 +1,34 @@ +package core + +import ( + "net" + "testing" +) + +func TestBlockedIP(t *testing.T) { + for _, s := range []string{"127.0.0.1", "::1", "169.254.169.254", "::ffff:169.254.169.254", "::ffff:127.0.0.1", "0.0.0.0", "0.1.2.3", + "100.100.100.200", "192.0.0.192", "fd00:ec2::254", "64:ff9b::7f00:1", "2002:7f00:1::", "fe80::1", "224.0.0.1"} { + if !blockedIP(net.ParseIP(s)) { + t.Errorf("%s is reachable", s) + } + } + for _, s := range []string{"8.8.8.8", "93.184.216.34", "2606:4700:4700::1111", "100.64.0.1"} { + if blockedIP(net.ParseIP(s)) { + t.Errorf("%s is blocked", s) + } + } + // Every address of this host is off limits (Docker bridge gateways, public IP). + addrs, _ := net.InterfaceAddrs() + for _, a := range addrs { + if n, ok := a.(*net.IPNet); ok && !blockedIP(n.IP) { + t.Errorf("host address %s is reachable", n.IP) + } + } + DenyPrivateTargets = true + defer func() { DenyPrivateTargets = false }() + for _, s := range []string{"10.0.0.5", "172.17.0.1", "192.168.1.1", "fd12::1"} { + if !blockedIP(net.ParseIP(s)) { + t.Errorf("%s reachable with HOMECLOUD_DENY_PRIVATE_TARGETS", s) + } + } +} diff --git a/cli/internal/svc/lambda/apigw_aws_test.go b/cli/internal/svc/lambda/apigw_aws_test.go index accaffbf..40f13766 100644 --- a/cli/internal/svc/lambda/apigw_aws_test.go +++ b/cli/internal/svc/lambda/apigw_aws_test.go @@ -45,6 +45,7 @@ func gwHarness(t *testing.T, docker bool) (*awstest.Harness, *lambda.Service) { } l := lambda.New(h.Env, cw, v) l.Roles = roles{h.IAM} + l.HTTP = &http.Client{CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }} // tests proxy to 127.0.0.1 l.RegisterAWS() l.Routes(h.Router) return h, l diff --git a/cli/internal/svc/lambda/apigw_serve.go b/cli/internal/svc/lambda/apigw_serve.go index 5a8628d4..f4b7e27d 100644 --- a/cli/internal/svc/lambda/apigw_serve.go +++ b/cli/internal/svc/lambda/apigw_serve.go @@ -350,7 +350,7 @@ func (s *Service) proxyHTTP(c *httpx.Ctx, integ doc, r Route, p string, params m for _, h := range hopHeaders { req.Header.Del(h) } - cli := &http.Client{CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }} + cli := s.HTTP resp, err := cli.Do(req) if err != nil { if tooBig := (*http.MaxBytesError)(nil); errors.As(err, &tooBig) { diff --git a/cli/internal/svc/lambda/apigw_ssrf_test.go b/cli/internal/svc/lambda/apigw_ssrf_test.go new file mode 100644 index 00000000..2bd5cf3a --- /dev/null +++ b/cli/internal/svc/lambda/apigw_ssrf_test.go @@ -0,0 +1,29 @@ +package lambda_test + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/homecloudhq/homecloud/cli/internal/core" +) + +// An HTTP_PROXY integration must not be a way to read loopback services (the +// HomeCloud API, MinIO), the host provider's metadata service or the Docker +// bridge: the API is callable by anyone, so whoever can create an integration +// would otherwise get a read-through proxy into the host. +func TestAPIGatewayProxyRefusesInternalTargets(t *testing.T) { + h, l := gwHarness(t, false) + l.HTTP = core.SafeClient(0, 0) // what the server uses + secret := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Write([]byte("internal-secret")) })) + defer secret.Close() + + for _, target := range []string{secret.URL, "http://169.254.169.254/latest/meta-data/", "http://[::ffff:127.0.0.1]:1/", "http://[::1]:1/", "http://0.0.0.0:1/"} { + api := h.AWSJSON(t, "apigatewayv2", "create-api", "--name", "p", "--protocol-type", "HTTP", "--target", target) + id := api["ApiId"].(string) + code, body := gwCall(t, h, api["ApiEndpoint"].(string), "GET", "/", nil) + if code == http.StatusOK || body == "internal-secret" { + t.Fatalf("%s: proxied to an internal address: %d %q (api %s)", target, code, body, id) + } + } +} diff --git a/cli/internal/svc/lambda/lambda.go b/cli/internal/svc/lambda/lambda.go index 47a11d10..d2fb07c5 100644 --- a/cli/internal/svc/lambda/lambda.go +++ b/cli/internal/svc/lambda/lambda.go @@ -180,6 +180,9 @@ type Service struct { // ResolveImage maps an image URI (e.g. an ECR repository URI) to one the // local Docker engine can pull. ResolveImage func(uri string) string + // HTTP carries HTTP_PROXY integration requests; it refuses loopback, link-local + // and metadata addresses at dial time (tests point it at local backends). + HTTP *http.Client } // Credentials are temporary credentials for a function's execution role. @@ -199,7 +202,7 @@ type RoleSource interface { func New(env *svc.Env, cw *cloudwatch.Service, v *vpc.Service) *Service { s := &Service{env: env, cw: cw, vpc: v, pools: map[string]*pool{}, async: make(chan *asyncEvent, 10000), - urlKey: []byte(core.NewSecret(32)), now: time.Now} + urlKey: []byte(core.NewSecret(32)), now: time.Now, HTTP: core.SafeClient(0, 0)} if v != nil { v.RegisterMembers(s.fwMembers) } From 5d8087bade99f28c1bdc36cc903cc6bf99976092 Mon Sep 17 00:00:00 2001 From: Suryansh Prajapati <58465650+drk1rd@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:11:38 +0530 Subject: [PATCH 04/18] ECS: require iam:PassRole on the resolved role for every task definition registration The native task definition route (and CloudFormation, which uses it) accepted a task role without any PassRole check, so a caller with only ecs:RegisterTaskDefinition and ecs:RunTask could run a container holding any role that trusts ecs-tasks. The check now lives in registerTaskDef and is made against the role's real ARN, so alternative spellings such as role/x-/admin cannot slip past a scoped policy. --- cli/internal/svc/ecs/aws.go | 8 +--- cli/internal/svc/ecs/ecs.go | 25 ++++++++++++ cli/internal/svc/ecs/passrole_test.go | 55 +++++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 6 deletions(-) create mode 100644 cli/internal/svc/ecs/passrole_test.go diff --git a/cli/internal/svc/ecs/aws.go b/cli/internal/svc/ecs/aws.go index 1902dfad..b48b08df 100644 --- a/cli/internal/svc/ecs/aws.go +++ b/cli/internal/svc/ecs/aws.go @@ -500,9 +500,8 @@ func (e *ECS) awsRegisterTaskDefinition(q *awsapi.Req) (any, error) { } } if role := str(raw, "taskRoleArn"); role != "" { - if err := q.Authorize("iam:PassRole", role); err != nil { - return nil, err - } + // PassRole is checked against the resolved role ARN by registerTaskDef: a + // spelling such as role/x-/admin must not dodge a policy on role/admin. if e.Roles != nil { arn, err := e.Roles.TaskRole(role) if err != nil { @@ -513,9 +512,6 @@ func (e *ECS) awsRegisterTaskDefinition(q *awsapi.Req) (any, error) { td.TaskRole = role } if role := str(raw, "executionRoleArn"); role != "" { - if err := q.Authorize("iam:PassRole", role); err != nil { - return nil, err - } td.ExecutionRole = role } td, err := e.registerTaskDef(q.Authorize, td) diff --git a/cli/internal/svc/ecs/ecs.go b/cli/internal/svc/ecs/ecs.go index eda69e77..974fb03a 100644 --- a/cli/internal/svc/ecs/ecs.go +++ b/cli/internal/svc/ecs/ecs.go @@ -704,6 +704,31 @@ func (e *ECS) registerTaskDef(az authz, in TaskDefinition) (TaskDefinition, erro if err := e.authorizeSecrets(az, in); err != nil { return in, err } + // Passing a role to tasks needs iam:PassRole on the role's real ARN, on every + // path that registers task definitions (native API, AWS API, CloudFormation): + // running the task hands the role's credentials to the task's code. + if in.TaskRole != "" { + arn := in.TaskRole + if e.Roles != nil { + var err error + if arn, err = e.Roles.TaskRole(in.TaskRole); err != nil { + return in, core.BadRequest("task role: %v", err) + } + } + if err := az("iam:PassRole", arn); err != nil { + return in, err + } + in.TaskRole = arn + } + if in.ExecutionRole != "" { + arn := core.CanonicalARN(in.ExecutionRole) + if !strings.HasPrefix(arn, "arn:") { + arn = e.env.ARN("iam", "role/"+in.ExecutionRole) + } + if err := az("iam:PassRole", arn); err != nil { + return in, err + } + } e.tdMu.Lock() defer e.tdMu.Unlock() rev := 1 diff --git a/cli/internal/svc/ecs/passrole_test.go b/cli/internal/svc/ecs/passrole_test.go new file mode 100644 index 00000000..a4f793c7 --- /dev/null +++ b/cli/internal/svc/ecs/passrole_test.go @@ -0,0 +1,55 @@ +package ecs_test + +import ( + "strings" + "testing" + "time" + + "github.com/homecloudhq/homecloud/cli/internal/awsapi/awstest" + "github.com/homecloudhq/homecloud/cli/internal/svc/ecs" +) + +// stubRoles resolves every spelling of a role to its real ARN, as IAM does +// (role/dev-/admin and "admin" both name the role "admin"). +type stubRoles struct{ acct string } + +func (r stubRoles) TaskRole(ref string) (string, error) { + name := ref[strings.LastIndex(ref, "/")+1:] + return "arn:aws:iam::" + r.acct + ":role/" + name, nil +} +func (stubRoles) TaskCredentials(string, string, time.Duration) (ecs.Credentials, error) { + return ecs.Credentials{}, nil +} + +// Handing a task role to a task hands its credentials to the task's code, so +// every way of registering a task definition needs iam:PassRole on the role, +// judged on the role's real ARN. +func TestTaskRoleNeedsPassRole(t *testing.T) { + h := awstest.New(t) + e := ecs.New(h.Env, nil, nil, h.Secrets) + e.Roles = stubRoles{h.Env.AccountID} + e.Routes(h.Router) + e.RegisterAWS() + h.Native(t, "POST", "/api/v1/iam/policies", map[string]any{"name": "ecs-dev", "document": map[string]any{"Version": "2012-10-17", + "Statement": []any{ + map[string]any{"Effect": "Allow", "Action": "ecs:*", "Resource": "*"}, + map[string]any{"Effect": "Allow", "Action": "iam:PassRole", "Resource": "arn:aws:iam::" + h.Env.AccountID + ":role/dev-*"}, + }}}) + akid, secret := h.User(t, "dev", "ecs-dev") + + td := func(role string) map[string]any { + return map[string]any{"family": "f", "image": "alpine", "task_role": role} + } + for _, role := range []string{"admin", "arn:aws:iam::" + h.Env.AccountID + ":role/admin", "arn:aws:iam::" + h.Env.AccountID + ":role/dev-/admin"} { + if code, body := h.NativeAs(t, akid, secret, "POST", "/api/v1/ecs/task-definitions", td(role)); code != 403 { + t.Fatalf("native register with task role %q: %d %s", role, code, body) + } + in := `{"family":"g","taskRoleArn":"` + role + `","containerDefinitions":[{"name":"a","image":"alpine"}]}` + if out, err := h.AWSAs(t, akid, secret, "", "ecs", "register-task-definition", "--cli-input-json", in); err == nil || !strings.Contains(out, "iam:PassRole") { + t.Fatalf("AWS register with task role %q: %v %s", role, err, out) + } + } + if code, body := h.NativeAs(t, akid, secret, "POST", "/api/v1/ecs/task-definitions", td("dev-app")); code != 200 { + t.Fatalf("a role the caller may pass was refused: %d %s", code, body) + } +} From 55cd26648a142d5b25e836543f97ddcd8e5d8c46 Mon Sep 17 00:00:00 2001 From: Suryansh Prajapati <58465650+drk1rd@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:18:26 +0530 Subject: [PATCH 05/18] EC2 and Auto Scaling: require iam:PassRole for instance profiles on every launch path The native RunInstances route, launch templates and Auto Scaling groups launching from a template accepted any instance profile without iam:PassRole (only the AWS RunInstances call checked it), letting a caller with ec2:RunInstances boot an instance holding any role's credentials through IMDS. The check is now shared (ec2.PassProfile) and applied on the native route, when a launch template carries a profile, and when a group is created or updated from such a template. --- cli/internal/svc/autoscaling/autoscaling.go | 7 ++ cli/internal/svc/autoscaling/passrole_test.go | 72 +++++++++++++++++++ cli/internal/svc/ec2/ec2.go | 23 ++++++ cli/internal/svc/ec2/launchtemplates.go | 8 +++ 4 files changed, 110 insertions(+) create mode 100644 cli/internal/svc/autoscaling/passrole_test.go diff --git a/cli/internal/svc/autoscaling/autoscaling.go b/cli/internal/svc/autoscaling/autoscaling.go index 142ae114..a8956d61 100644 --- a/cli/internal/svc/autoscaling/autoscaling.go +++ b/cli/internal/svc/autoscaling/autoscaling.go @@ -35,6 +35,9 @@ type LaunchConfig struct { SecurityGroupIDs []string `json:"security_group_ids"` UserData string `json:"user_data,omitempty"` FileSystems []ec2.FSMount `json:"file_systems,omitempty"` + // IAMInstanceProfile is the launch template's instance profile; creating or + // updating a group needs iam:PassRole for its role. + IAMInstanceProfile string `json:"iam_instance_profile,omitempty"` } type Policy struct { @@ -428,6 +431,9 @@ func (s *Service) authorizeLaunch(c authz, l LaunchConfig, targetGroups []string if err := c.Authorize("ec2:RunInstances", "*"); err != nil { return err } + if err := s.ec2.PassProfile(c.Authorize, l.IAMInstanceProfile); err != nil { + return err + } for _, m := range l.FileSystems { if err := c.Authorize("elasticfilesystem:ClientMount", s.env.ARN("elasticfilesystem", "file-system/"+m.FileSystemID)); err != nil { return err @@ -486,6 +492,7 @@ func (s *Service) resolveTemplate(g *Group) error { } g.Template.ID, g.Template.Name = rt.ID, rt.Name g.Launch.ImageID, g.Launch.InstanceType, g.Launch.SecurityGroupIDs, g.Launch.UserData = rt.Input.ImageID, rt.Input.InstanceType, rt.Input.SecurityGroupIDs, rt.Input.UserData + g.Launch.IAMInstanceProfile = rt.Input.IAMInstanceProfile return nil } diff --git a/cli/internal/svc/autoscaling/passrole_test.go b/cli/internal/svc/autoscaling/passrole_test.go new file mode 100644 index 00000000..74663a62 --- /dev/null +++ b/cli/internal/svc/autoscaling/passrole_test.go @@ -0,0 +1,72 @@ +package autoscaling_test + +import ( + "strings" + "testing" + "time" + + "github.com/homecloudhq/homecloud/cli/internal/awsapi/awstest" + "github.com/homecloudhq/homecloud/cli/internal/svc/autoscaling" + "github.com/homecloudhq/homecloud/cli/internal/svc/cloudwatch" + "github.com/homecloudhq/homecloud/cli/internal/svc/ec2" + "github.com/homecloudhq/homecloud/cli/internal/svc/vpc" +) + +// profiles maps instance profile "-prof" to role "". +type profiles struct{ acct string } + +func (p profiles) InstanceProfile(ref string) (arn, id, role string, err error) { + name := strings.TrimSuffix(ref[strings.LastIndex(ref, "/")+1:], "-prof") + return "arn:aws:iam::" + p.acct + ":instance-profile/" + name + "-prof", "AIPA" + name, "arn:aws:iam::" + p.acct + ":role/" + name, nil +} +func (profiles) InstanceCredentials(string, string, time.Duration) (ec2.Credentials, error) { + return ec2.Credentials{}, nil +} + +// An instance profile's role reaches whoever controls the instance, so putting +// one in a launch template, launching with it or creating an Auto Scaling group +// that launches from such a template all need iam:PassRole for the role. +func TestInstanceProfileNeedsPassRole(t *testing.T) { + h := awstest.New(t) + v := vpc.New(h.Env) + e := ec2.New(h.Env, v) + e.Roles = profiles{h.Env.AccountID} + e.RegisterAWS() + e.Routes(h.Router) + cw, err := cloudwatch.New(h.Env) + if err != nil { + t.Fatal(err) + } + a := autoscaling.New(h.Env, e, nil, cw) + a.RegisterAWS() + a.Routes(h.Router) + + h.Native(t, "POST", "/api/v1/iam/policies", map[string]any{"name": "dev", "document": map[string]any{"Version": "2012-10-17", + "Statement": []any{ + map[string]any{"Effect": "Allow", "Action": []string{"ec2:*", "autoscaling:*"}, "Resource": "*"}, + map[string]any{"Effect": "Allow", "Action": "iam:PassRole", "Resource": "arn:aws:iam::" + h.Env.AccountID + ":role/dev-*"}, + }}}) + akid, secret := h.User(t, "dev", "dev") + + // Native launch. + code, body := h.NativeAs(t, akid, secret, "POST", "/api/v1/ec2/instances", map[string]any{"image_id": "ami-x", "iam_instance_profile": "admin-prof"}) + if code != 403 || !strings.Contains(string(body), "iam:PassRole") { + t.Fatalf("native launch with another role's profile: %d %s", code, body) + } + + // Launch templates. + if out, err := h.AWSAs(t, akid, secret, "", "ec2", "create-launch-template", "--launch-template-name", "evil", + "--launch-template-data", `{"IamInstanceProfile":{"Name":"admin-prof"}}`); err == nil || !strings.Contains(out, "iam:PassRole") { + t.Fatalf("template with another role's profile: %v %s", err, out) + } + h.AWSAs(t, akid, secret, "", "ec2", "create-launch-template", "--launch-template-name", "fine", "--launch-template-data", `{"IamInstanceProfile":{"Name":"dev-app-prof"}}`) + + // A template an administrator made with a powerful profile can't be used by + // the developer to start a group. + h.AWS(t, "ec2", "create-launch-template", "--launch-template-name", "admin-lt", "--launch-template-data", `{"IamInstanceProfile":{"Name":"admin-prof"},"InstanceType":"t3.micro"}`) + code, body = h.NativeAs(t, akid, secret, "POST", "/api/v1/autoscaling/groups", + map[string]any{"name": "g", "template": map[string]any{"name": "admin-lt", "version": "$Latest"}, "min_size": 0, "max_size": 1}) + if code != 403 || !strings.Contains(string(body), "iam:PassRole") { + t.Fatalf("group from a template with another role's profile: %d %s", code, body) + } +} diff --git a/cli/internal/svc/ec2/ec2.go b/cli/internal/svc/ec2/ec2.go index 5bcb8955..c8433b83 100644 --- a/cli/internal/svc/ec2/ec2.go +++ b/cli/internal/svc/ec2/ec2.go @@ -391,6 +391,26 @@ func checkTags(t core.Tags) error { return nil } +// PassProfile checks, through az, that the caller may pass the role of the +// instance profile ref (a name or ARN) to instances: launching with a profile +// hands the role's credentials to whoever controls the instance. +func (s *Service) PassProfile(az func(action, resource string) error, ref string) error { + if ref == "" { + return nil + } + if s.Roles == nil { + return core.BadRequest("instance profiles are not available") + } + _, _, role, err := s.Roles.InstanceProfile(ref) + if err != nil { + return err + } + if role == "" { + return nil + } + return az("iam:PassRole", role) +} + func (s *Service) run(c *httpx.Ctx) (any, error) { var in RunInput if err := c.Bind(&in); err != nil { @@ -399,6 +419,9 @@ func (s *Service) run(c *httpx.Ctx) (any, error) { if err := checkTags(in.Tags); err != nil { return nil, err } + if err := s.PassProfile(c.Authorize, in.IAMInstanceProfile); err != nil { + return nil, err + } for _, v := range in.Volumes { if v.VolumeID != "" { if err := c.Authorize("ec2:AttachVolume", s.env.ARN("ec2", "volume/"+v.VolumeID)); err != nil { diff --git a/cli/internal/svc/ec2/launchtemplates.go b/cli/internal/svc/ec2/launchtemplates.go index 5da53519..28ca8376 100644 --- a/cli/internal/svc/ec2/launchtemplates.go +++ b/cli/internal/svc/ec2/launchtemplates.go @@ -272,6 +272,14 @@ func (s *Service) ltData(q *awsapi.Req) (map[string]string, error) { return nil, core.Errf(http.StatusBadRequest, "InvalidGroup.NotFound", "The security group '%s' does not exist", g) } } + // As in AWS, putting an instance profile in a template needs iam:PassRole. + ref := d["IamInstanceProfile.Arn"] + if ref == "" { + ref = d["IamInstanceProfile.Name"] + } + if err := s.PassProfile(q.Check, ref); err != nil { + return nil, err + } return d, nil } From 2f94e06adf293152975f4ef9c6bf028df375e0af Mon Sep 17 00:00:00 2001 From: Suryansh Prajapati <58465650+drk1rd@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:20:10 +0530 Subject: [PATCH 06/18] Judge iam:PassRole on the resolved role and refuse foreign-account target ARNs Permission checks used the resource string exactly as the caller wrote it, so a Deny or scoped Allow on role/admin could be dodged with the bare name or an ARN with another path part (role/dev-/admin) that the IAM lookup still resolved to admin. Principal.Permits now canonicalizes the resource and IAM resolves role references to the real role ARN for iam:PassRole. Delivery to EventBridge, scheduler, CloudWatch and Step Functions targets, and Secrets Manager rotation functions, now rejects ARNs of another account or region instead of delivering to the same-named local resource. --- cli/internal/core/core.go | 13 ++++++ cli/internal/httpx/authz.go | 6 +++ cli/internal/httpx/httpx.go | 4 ++ cli/internal/server/server.go | 2 +- cli/internal/server/targets.go | 11 +++-- cli/internal/server/targets_arn_test.go | 32 ++++++++++++++ cli/internal/svc/iam/iam.go | 2 +- .../svc/iam/passrole_spelling_test.go | 44 +++++++++++++++++++ cli/internal/svc/iam/roles.go | 14 +++++- cli/internal/svc/secrets/rotation.go | 4 ++ 10 files changed, 126 insertions(+), 6 deletions(-) create mode 100644 cli/internal/server/targets_arn_test.go create mode 100644 cli/internal/svc/iam/passrole_spelling_test.go diff --git a/cli/internal/core/core.go b/cli/internal/core/core.go index fc1c0a0b..3e6e614b 100644 --- a/cli/internal/core/core.go +++ b/cli/internal/core/core.go @@ -243,6 +243,19 @@ func CanonicalARN(s string) string { return strings.Join(parts, ":") } +// IsLocalARN reports whether arn names a resource of this deployment: the same +// partition and account, and the deployment's region (or none, for global +// services). Code that delivers to "the resource named by this ARN" by looking +// up only its name must check this first, or an ARN for another account or +// region would be authorized as one resource and served as another. +func IsLocalARN(arn, account string) bool { + parts := strings.SplitN(CanonicalARN(arn), ":", 6) + if len(parts) != 6 || parts[0] != "arn" || parts[1] != Partition || parts[4] != account { + return false + } + return parts[3] == Region || (parts[3] == "" && globalServices[parts[2]]) +} + func Now() time.Time { return time.Now().UTC().Truncate(time.Second) } // Error is an API error with an AWS-style code and an HTTP status. diff --git a/cli/internal/httpx/authz.go b/cli/internal/httpx/authz.go index a6bdc06e..76f10f4f 100644 --- a/cli/internal/httpx/authz.go +++ b/cli/internal/httpx/authz.go @@ -6,6 +6,8 @@ import ( "strconv" "strings" "sync" + + "github.com/homecloudhq/homecloud/cli/internal/core" ) // Decision is the outcome of evaluating identity policies. @@ -132,6 +134,10 @@ func (p *Principal) Permits(action, resource string, extra Access) bool { if p == nil { return PermitsAnonymous(action, resource, extra) } + resource = core.CanonicalARN(resource) + if p.ResolveResource != nil { + resource = p.ResolveResource(action, resource) + } acc := providerAccess(resource) if extra.Policy != "" { acc.Policy, acc.KeyPolicy = extra.Policy, extra.KeyPolicy diff --git a/cli/internal/httpx/httpx.go b/cli/internal/httpx/httpx.go index 7ecae506..366f3f72 100644 --- a/cli/internal/httpx/httpx.go +++ b/cli/internal/httpx/httpx.go @@ -45,6 +45,10 @@ type Principal struct { // as "aws:sourceip" or "aws:username"). IAM fills the identity keys; see // AddRequestContext for the request keys. Context map[string][]string `json:"-"` + // ResolveResource maps the resource a caller named to the one it denotes (IAM + // resolves role names and paths for iam:PassRole), so that a policy on the real + // resource cannot be dodged by another spelling of it. + ResolveResource func(action, resource string) string `json:"-"` } // AddRequestContext records the IAM global condition keys that come from the diff --git a/cli/internal/server/server.go b/cli/internal/server/server.go index ca226905..fb77c416 100644 --- a/cli/internal/server/server.go +++ b/cli/internal/server/server.go @@ -264,7 +264,7 @@ func Run(ctx context.Context, cfg core.Config, opts Options) error { } lambdaSvc.VerifyJWT = cognitoSvc.VerifyToken lambdaSvc.CheckAuthorizer = cognitoSvc.CheckClient - tg := &targets{lambda: lambdaSvc, sqs: sqsSvc, sns: snsSvc, sfn: sfnSvc} + tg := &targets{lambda: lambdaSvc, sqs: sqsSvc, sns: snsSvc, sfn: sfnSvc, account: account} sfnSvc.Tasks = tg sfnSvc.Call = func(ctx context.Context, p *httpx.Principal, service, op string, in any) (json.RawMessage, error) { return awsapi.Call(ctx, p, account, service, op, in) diff --git a/cli/internal/server/targets.go b/cli/internal/server/targets.go index d61c33cf..f47d518b 100644 --- a/cli/internal/server/targets.go +++ b/cli/internal/server/targets.go @@ -21,6 +21,8 @@ type targets struct { sqs *sqs.Service sns *sns.Service sfn *sfn.Service + // account, when set, restricts deliveries to this account's own ARNs. + account string } // Invoke, SendMessage and Publish implement sfn.Tasks. @@ -64,7 +66,10 @@ func (t *targets) Publish(topic, subject, message string) (map[string]any, error } // parse splits "arn:aws::::". -func parse(arn string) (service, resource string, ok bool) { +func (t *targets) parse(arn string) (service, resource string, ok bool) { + if t.account != "" && !core.IsLocalARN(arn, t.account) { + return "", "", false // another account or region: not a resource this server can deliver to + } parts := strings.SplitN(core.CanonicalARN(arn), ":", 6) if len(parts) != 6 || parts[0] != "arn" || parts[1] != core.Partition { return "", "", false @@ -73,7 +78,7 @@ func parse(arn string) (service, resource string, ok bool) { } func (t *targets) exists(arn string) bool { - svc, res, ok := parse(arn) + svc, res, ok := t.parse(arn) if !ok { return false } @@ -98,7 +103,7 @@ func denied(src core.Source, target string) error { } func (t *targets) deliver(ctx context.Context, arn string, payload []byte) error { - svc, res, ok := parse(arn) + svc, res, ok := t.parse(arn) if !ok { return fmt.Errorf("malformed ARN %q", arn) } diff --git a/cli/internal/server/targets_arn_test.go b/cli/internal/server/targets_arn_test.go new file mode 100644 index 00000000..7ae8e379 --- /dev/null +++ b/cli/internal/server/targets_arn_test.go @@ -0,0 +1,32 @@ +package server + +import ( + "testing" + + "github.com/homecloudhq/homecloud/cli/internal/core" +) + +// A target ARN of another account or region must not be treated as a local +// resource of the same name: IAM would authorize it as the foreign ARN while the +// delivery went to the local queue. +func TestTargetsRefuseForeignARNs(t *testing.T) { + tg := &targets{account: "111122223333"} + if _, _, ok := tg.parse(core.ARN("111122223333", "sqs", "q")); !ok { + t.Fatal("a local ARN was refused") + } + if _, _, ok := tg.parse("arn:hc:sqs:local-1:111122223333:q"); !ok { + t.Fatal("a legacy-form local ARN was refused") + } + for _, arn := range []string{ + "arn:aws:sqs:" + core.Region + ":999999999999:q", + "arn:aws:sqs:eu-north-9:111122223333:q", + "arn:aws:sqs::111122223333:q", + } { + if _, _, ok := tg.parse(arn); ok { + t.Errorf("%s was accepted", arn) + } + if tg.exists(arn) { + t.Errorf("%s exists", arn) + } + } +} diff --git a/cli/internal/svc/iam/iam.go b/cli/internal/svc/iam/iam.go index 58cda2fe..a23f4f8c 100644 --- a/cli/internal/svc/iam/iam.go +++ b/cli/internal/svc/iam/iam.go @@ -411,7 +411,7 @@ func (s *Service) userContext(u User) CondContext { } func (s *Service) principal(u User, keyID string) *httpx.Principal { - p := &httpx.Principal{AccountID: s.env.AccountID, UserName: u.Name, ARN: u.ARN, Root: u.Root, AccessKey: keyID, Context: s.userContext(u)} + p := &httpx.Principal{AccountID: s.env.AccountID, UserName: u.Name, ARN: u.ARN, Root: u.Root, AccessKey: keyID, Context: s.userContext(u), ResolveResource: s.resolveResource} if u.Root { p.Can = func(string, string) bool { return true } p.Identity = func(string, string, map[string][]string) httpx.Decision { return httpx.Allowed } diff --git a/cli/internal/svc/iam/passrole_spelling_test.go b/cli/internal/svc/iam/passrole_spelling_test.go new file mode 100644 index 00000000..19db7e70 --- /dev/null +++ b/cli/internal/svc/iam/passrole_spelling_test.go @@ -0,0 +1,44 @@ +package iam_test + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/homecloudhq/homecloud/cli/internal/awsapi/awstest" + "github.com/homecloudhq/homecloud/cli/internal/httpx" +) + +// A scoped iam:PassRole policy must judge the role the caller really names: a +// bare name, or an ARN whose path part matches an Allow pattern, is still the +// role "admin" and must hit the Deny on it. +func TestPassRoleIsJudgedOnTheResolvedRole(t *testing.T) { + h := awstest.New(t) + acct := h.Env.AccountID + trust := `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"Service":"lambda.amazonaws.com"},"Action":"sts:AssumeRole"}]}` + h.AWS(t, "iam", "create-role", "--role-name", "admin", "--assume-role-policy-document", trust) + h.AWS(t, "iam", "create-role", "--role-name", "dev-app", "--assume-role-policy-document", trust) + h.Native(t, "POST", "/api/v1/iam/policies", map[string]any{"name": "pass-dev", "document": map[string]any{"Version": "2012-10-17", + "Statement": []any{ + map[string]any{"Effect": "Allow", "Action": "iam:PassRole", "Resource": "arn:aws:iam::" + acct + ":role/dev-*"}, + map[string]any{"Effect": "Deny", "Action": "iam:PassRole", "Resource": "arn:aws:iam::" + acct + ":role/admin"}, + map[string]any{"Effect": "Allow", "Action": "iam:PassRole", "Resource": "*"}, + }}}) + akid, secret := h.User(t, "dev", "pass-dev") + r := httptest.NewRequest(http.MethodGet, "/", nil) + r.Header.Set("Authorization", "Bearer "+akid+":"+secret) + p, err := h.IAM.Authenticate(r) + if err != nil { + t.Fatal(err) + } + for _, res := range []string{"admin", "arn:aws:iam::" + acct + ":role/admin", "arn:aws:iam::" + acct + ":role/dev-/admin", "arn:aws:iam::" + acct + ":role/x/admin"} { + if p.Permits("iam:PassRole", res, httpx.Access{}) { + t.Errorf("iam:PassRole on %q was allowed despite the Deny on role/admin", res) + } + } + for _, res := range []string{"dev-app", "arn:aws:iam::" + acct + ":role/dev-app"} { + if !p.Permits("iam:PassRole", res, httpx.Access{}) { + t.Errorf("iam:PassRole on %q was refused", res) + } + } +} diff --git a/cli/internal/svc/iam/roles.go b/cli/internal/svc/iam/roles.go index a30b51fd..39707ff3 100644 --- a/cli/internal/svc/iam/roles.go +++ b/cli/internal/svc/iam/roles.go @@ -98,6 +98,18 @@ func roleName(ref string) string { return ref } +// resolveResource maps the role a caller named in an iam:PassRole check (a bare +// name, or an ARN with a wrong path or account part) to the role's real ARN, so +// an Allow or Deny written for the real ARN applies whatever the spelling. +func (s *Service) resolveResource(action, resource string) string { + if action == "iam:PassRole" { + if r, err := s.GetRole(resource); err == nil { + return r.ARN + } + } + return resource +} + // GetRole returns a role by name or ARN. func (s *Service) GetRole(ref string) (Role, error) { r, err := store.Get[Role](s.env.Store, cRoles, roleName(ref)) @@ -141,7 +153,7 @@ func (s *Service) rolePrincipal(r Role, t tempCred) *httpx.Principal { AccountID: s.env.AccountID, UserName: "assumed-role/" + r.Name + "/" + t.SessionName, ARN: s.env.ARN("sts", "assumed-role/"+r.Name+"/"+t.SessionName), AccessKey: t.AccessKeyID, RoleName: r.Name, SessionName: t.SessionName, - Context: s.roleContext(r, t), + Context: s.roleContext(r, t), ResolveResource: s.resolveResource, } p.Can = func(action, resource string) bool { return decide(docs, boundary, action, resource, CondContext(p.Context)) == allow diff --git a/cli/internal/svc/secrets/rotation.go b/cli/internal/svc/secrets/rotation.go index fbb3cbca..07c841de 100644 --- a/cli/internal/svc/secrets/rotation.go +++ b/cli/internal/svc/secrets/rotation.go @@ -109,6 +109,10 @@ func (s *Service) rotate(az Authz, ref string, in RotateInput) (Secret, string, if !strings.HasPrefix(fn, "arn:") { fn = s.env.ARN("lambda", "function:"+fn) } + fn = core.CanonicalARN(fn) + if !core.IsLocalARN(fn, s.env.AccountID) { + return sec, "", core.Errf(http.StatusBadRequest, "InvalidParameterException", "The rotation function must be a Lambda function of this account and region.") + } // Secrets Manager invokes the function on the caller's behalf. if err := az("lambda:InvokeFunction", fn); err != nil { return sec, "", err From 3d80520979931ace4fd4f28458780174071369f8 Mon Sep 17 00:00:00 2001 From: Suryansh Prajapati <58465650+drk1rd@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:21:49 +0530 Subject: [PATCH 07/18] SNS: require sqs:SendMessage on a subscription's dead-letter queue A RedrivePolicy only had to name an existing queue, so a caller with sns:SetSubscriptionAttributes could make failed deliveries write attacker-chosen messages into any queue, including ones they cannot send to. Naming a dead-letter queue now needs sqs:SendMessage on it, like subscribing a queue. --- cli/internal/svc/sns/aws.go | 2 +- cli/internal/svc/sns/aws_test.go | 18 ++++++++++++++++++ cli/internal/svc/sns/sns.go | 13 +++++++++---- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/cli/internal/svc/sns/aws.go b/cli/internal/svc/sns/aws.go index 7511ecb5..286e4823 100644 --- a/cli/internal/svc/sns/aws.go +++ b/cli/internal/svc/sns/aws.go @@ -549,7 +549,7 @@ func (s *Service) awsSetSubscriptionAttributes(q *awsapi.Req) (any, error) { return nil, err } name, value := q.Param("AttributeName"), q.Param("AttributeValue") - _, err = store.Update(s.env.Store, cSubs, sub.ARN, func(x *Subscription) error { return s.setSubAttribute(x, name, value) }) + _, err = store.Update(s.env.Store, cSubs, sub.ARN, func(x *Subscription) error { return s.setSubAttribute(x, name, value, q.Check) }) return nil, err } diff --git a/cli/internal/svc/sns/aws_test.go b/cli/internal/svc/sns/aws_test.go index faeecc64..aa5c75ea 100644 --- a/cli/internal/svc/sns/aws_test.go +++ b/cli/internal/svc/sns/aws_test.go @@ -469,6 +469,24 @@ func TestIAMDenied(t *testing.T) { } } +// A subscription's dead-letter queue receives messages on the subscriber's +// behalf, so naming one needs sqs:SendMessage on it, as subscribing a queue does. +func TestRedrivePolicyNeedsSendMessageOnTheQueue(t *testing.T) { + h, _, _ := harness(t) + acct := h.Env.AccountID + arn := h.AWSJSON(t, "sns", "create-topic", "--name", "t")["TopicArn"].(string) + h.AWS(t, "sqs", "create-queue", "--queue-name", "victim") + h.AWS(t, "sqs", "create-queue", "--queue-name", "mine") + sub := h.AWSJSON(t, "sns", "subscribe", "--topic-arn", arn, "--protocol", "sqs", "--notification-endpoint", "arn:aws:sqs:us-east-1:"+acct+":mine")["SubscriptionArn"].(string) + akid, secret := h.User(t, "topics-only", "SNSFullAccess") + rp := `{"deadLetterTargetArn":"arn:aws:sqs:us-east-1:` + acct + `:victim"}` + out, err := h.AWSAs(t, akid, secret, "", "sns", "set-subscription-attributes", "--subscription-arn", sub, "--attribute-name", "RedrivePolicy", "--attribute-value", rp) + if err == nil || !strings.Contains(out, "AuthorizationError") { + t.Fatalf("redrive policy naming a queue the caller may not write to: %v %s", err, out) + } + h.AWS(t, "sns", "set-subscription-attributes", "--subscription-arn", sub, "--attribute-name", "RedrivePolicy", "--attribute-value", rp) // root may +} + // TestNativeAPI checks the native routes and their interplay with the AWS API. func TestNativeAPI(t *testing.T) { h, _, _ := harness(t) diff --git a/cli/internal/svc/sns/sns.go b/cli/internal/svc/sns/sns.go index 819b4998..ebd30c7d 100644 --- a/cli/internal/svc/sns/sns.go +++ b/cli/internal/svc/sns/sns.go @@ -355,7 +355,7 @@ type subscribeInput struct { var phoneRe = regexp.MustCompile(`^\+?[0-9]{5,15}$`) // setSubAttribute validates and applies one subscription attribute. -func (s *Service) setSubAttribute(sub *Subscription, name, value string) error { +func (s *Service) setSubAttribute(sub *Subscription, name, value string, authorize func(action, resource string) error) error { switch name { case "RawMessageDelivery": if value != "true" && value != "false" { @@ -400,9 +400,14 @@ func (s *Service) setSubAttribute(sub *Subscription, name, value string) error { if json.Unmarshal([]byte(value), &rp) != nil || !strings.HasPrefix(core.CanonicalARN(rp.DeadLetterTargetArn), "arn:"+core.Partition+":sqs:") { return errInvalid("RedrivePolicy: deadLetterTargetArn must be an Amazon SQS queue ARN") } - if _, ok := s.sqs.QueueARN(sqs.NameFromARN(rp.DeadLetterTargetArn)); !ok { + dlq, ok := s.sqs.QueueARN(sqs.NameFromARN(rp.DeadLetterTargetArn)) + if !ok || !core.IsLocalARN(rp.DeadLetterTargetArn, s.env.AccountID) { return errInvalid("RedrivePolicy: dead-letter queue %s does not exist", rp.DeadLetterTargetArn) } + // Failed deliveries are written to the queue on the subscriber's behalf. + if err := authorize("sqs:SendMessage", dlq); err != nil { + return err + } sub.RedrivePolicy = value case "DeliveryPolicy": if value != "" { @@ -497,7 +502,7 @@ func (s *Service) subscribe(t Topic, in subscribeInput, authorize func(action, r if k == "FilterPolicyScope" { continue } - if err := s.setSubAttribute(&sub, k, v); err != nil { + if err := s.setSubAttribute(&sub, k, v, authorize); err != nil { return Subscription{}, err } } @@ -1349,7 +1354,7 @@ func (s *Service) updateSub(c *httpx.Ctx) (any, error) { } for _, k := range []string{"RawMessageDelivery", "FilterPolicy", "FilterPolicyScope"} { if v, ok := attrs[k]; ok { - if err := s.setSubAttribute(x, k, v); err != nil { + if err := s.setSubAttribute(x, k, v, c.Authorize); err != nil { return err } } From 04349a78f3810317484106225a5bde5ba4f3878c Mon Sep 17 00:00:00 2001 From: Suryansh Prajapati <58465650+drk1rd@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:22:53 +0530 Subject: [PATCH 08/18] Require ECR pull permissions to run images from the local registry Lambda container functions and ECS task definitions pulled from HomeCloud's registry (ECR-style or localhost URIs) without any ecr: check, so any user able to create a function or task could run, and read the contents of, another team's private repository. Both now need ecr:BatchGetImage and ecr:GetDownloadUrlForLayer on the repository. --- cli/internal/core/targets.go | 29 +++++++++++++++++++++++++++ cli/internal/core/targets_test.go | 15 ++++++++++++++ cli/internal/svc/ecs/ecs.go | 10 +++++++++ cli/internal/svc/ecs/passrole_test.go | 10 +++++++++ cli/internal/svc/lambda/aws.go | 22 ++++++++++++++++++++ 5 files changed, 86 insertions(+) diff --git a/cli/internal/core/targets.go b/cli/internal/core/targets.go index fa0bb162..4a0a815f 100644 --- a/cli/internal/core/targets.go +++ b/cli/internal/core/targets.go @@ -8,6 +8,7 @@ import ( "net/http" "net/url" "os" + "regexp" "strings" "syscall" "time" @@ -33,6 +34,34 @@ func TargetAction(arn string) string { return "" } +var ecrHostedImage = regexp.MustCompile(`^[0-9]{12}\.dkr\.ecr\.[a-z0-9-]+\.amazonaws\.com/`) + +// LocalImageRepo reports which repository of HomeCloud's own registry an image +// reference points at: an AWS-style ECR URI (".dkr.ecr..amazonaws.com/app:tag") +// or the registry's local address. Images from elsewhere return ok == false. +func LocalImageRepo(image string, ecrPort int) (repo string, ok bool) { + rest := "" + if loc := ecrHostedImage.FindStringIndex(image); loc != nil { + rest = image[loc[1]:] + } else { + for _, h := range []string{"localhost", "127.0.0.1"} { + if r, found := strings.CutPrefix(image, fmt.Sprintf("%s:%d/", h, ecrPort)); found { + rest = r + } + } + } + if rest == "" { + return "", false + } + if i := strings.IndexByte(rest, '@'); i >= 0 { + rest = rest[:i] + } + if i := strings.LastIndexByte(rest, ':'); i > strings.LastIndexByte(rest, '/') { + rest = rest[:i] + } + return rest, rest != "" +} + // blockedNets are ranges outbound requests made on behalf of users never reach: // "this network", the cloud metadata services of VPS providers that sit outside // link-local space, and IPv6 prefixes that embed IPv4 addresses (NAT64, 6to4), diff --git a/cli/internal/core/targets_test.go b/cli/internal/core/targets_test.go index 24d4ca27..99dec7dc 100644 --- a/cli/internal/core/targets_test.go +++ b/cli/internal/core/targets_test.go @@ -5,6 +5,21 @@ import ( "testing" ) +func TestLocalImageRepo(t *testing.T) { + for in, want := range map[string]string{ + "localhost:5500/app:1": "app", + "127.0.0.1:5500/team/app": "team/app", + "123456789012.dkr.ecr.us-east-1.amazonaws.com/app@sha256:ab": "app", + "123456789012.dkr.ecr.us-east-1.amazonaws.com/a/b:v2": "a/b", + "nginx:alpine": "", "localhost:5501/app": "", "docker.io/library/nginx": "", + } { + got, ok := LocalImageRepo(in, 5500) + if got != want || ok != (want != "") { + t.Errorf("%s: %q %v", in, got, ok) + } + } +} + func TestBlockedIP(t *testing.T) { for _, s := range []string{"127.0.0.1", "::1", "169.254.169.254", "::ffff:169.254.169.254", "::ffff:127.0.0.1", "0.0.0.0", "0.1.2.3", "100.100.100.200", "192.0.0.192", "fd00:ec2::254", "64:ff9b::7f00:1", "2002:7f00:1::", "fe80::1", "224.0.0.1"} { diff --git a/cli/internal/svc/ecs/ecs.go b/cli/internal/svc/ecs/ecs.go index 974fb03a..3f034a21 100644 --- a/cli/internal/svc/ecs/ecs.go +++ b/cli/internal/svc/ecs/ecs.go @@ -729,6 +729,16 @@ func (e *ECS) registerTaskDef(az authz, in TaskDefinition) (TaskDefinition, erro return in, err } } + // Running the image discloses its contents to the task: pulling from the local + // registry needs the same permissions as pulling from ECR. + if repo, ok := core.LocalImageRepo(in.Image, e.env.Cfg.ECRPort); ok { + arn := e.env.ARN("ecr", "repository/"+repo) + for _, action := range []string{"ecr:BatchGetImage", "ecr:GetDownloadUrlForLayer"} { + if err := az(action, arn); err != nil { + return in, err + } + } + } e.tdMu.Lock() defer e.tdMu.Unlock() rev := 1 diff --git a/cli/internal/svc/ecs/passrole_test.go b/cli/internal/svc/ecs/passrole_test.go index a4f793c7..212ddd5c 100644 --- a/cli/internal/svc/ecs/passrole_test.go +++ b/cli/internal/svc/ecs/passrole_test.go @@ -49,6 +49,16 @@ func TestTaskRoleNeedsPassRole(t *testing.T) { t.Fatalf("AWS register with task role %q: %v %s", role, err, out) } } + // Images of the local registry need the ECR pull permissions. + for _, image := range []string{"localhost:5500/secret:1", "123456789012.dkr.ecr.us-east-1.amazonaws.com/secret@sha256:abc"} { + h.Env.Cfg.ECRPort = 5500 + if code, body := h.NativeAs(t, akid, secret, "POST", "/api/v1/ecs/task-definitions", map[string]any{"family": "img", "image": image}); code != 403 || !strings.Contains(string(body), "ecr:BatchGetImage") { + t.Fatalf("image %s: %d %s", image, code, body) + } + } + if code, body := h.NativeAs(t, akid, secret, "POST", "/api/v1/ecs/task-definitions", map[string]any{"family": "img", "image": "nginx:alpine"}); code != 200 { + t.Fatalf("a public image was refused: %d %s", code, body) + } if code, body := h.NativeAs(t, akid, secret, "POST", "/api/v1/ecs/task-definitions", td("dev-app")); code != 200 { t.Fatalf("a role the caller may pass was refused: %d %s", code, body) } diff --git a/cli/internal/svc/lambda/aws.go b/cli/internal/svc/lambda/aws.go index a3297b5e..513d1048 100644 --- a/cli/internal/svc/lambda/aws.go +++ b/cli/internal/svc/lambda/aws.go @@ -342,6 +342,22 @@ func (s *Service) s3Code(q *awsapi.Req, bucket, key, version string) ([]byte, er // ---- functions ---- +// authorizeImage requires the caller to be allowed to pull an image of +// HomeCloud's own registry: the function runs it, and its code can read it. +func (s *Service) authorizeImage(q *awsapi.Req, image string) error { + repo, ok := core.LocalImageRepo(image, s.env.Cfg.ECRPort) + if !ok { + return nil + } + arn := s.env.ARN("ecr", "repository/"+repo) + for _, action := range []string{"ecr:BatchGetImage", "ecr:GetDownloadUrlForLayer"} { + if err := q.Check(action, arn); err != nil { + return err + } + } + return nil +} + func (s *Service) awsCreateFunction(q *awsapi.Req, _ map[string]string) error { var in struct { awsConfigIn @@ -363,6 +379,9 @@ func (s *Service) awsCreateFunction(q *awsapi.Req, _ map[string]string) error { if err := q.Authorize("lambda:CreateFunction", s.fnARN(in.FunctionName)); err != nil { return err } + if err := s.authorizeImage(q, in.Code.ImageUri); err != nil { + return err + } spec := createSpec{Name: in.FunctionName, configInput: in.config(), PackageType: in.PackageType, ImageURI: in.Code.ImageUri, Publish: in.Publish} spec.Tags = in.Tags switch { @@ -544,6 +563,9 @@ func (s *Service) awsUpdateCode(q *awsapi.Req, p map[string]string) error { if err := q.Authorize("lambda:UpdateFunctionCode", s.fnARN(name)); err != nil { return err } + if err := s.authorizeImage(q, in.ImageUri); err != nil { + return err + } spec := codeSpec{ImageURI: in.ImageUri, Publish: in.Publish, Revision: in.RevisionId} switch { case in.ZipFile != nil: From 0170ac66071669ca0cac7d9c06fe90921d7a1426 Mon Sep 17 00:00:00 2001 From: Suryansh Prajapati <58465650+drk1rd@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:24:32 +0530 Subject: [PATCH 09/18] Stop inline object views from reading the console session token; add clickjacking headers The console opens an object inline with ?access_token= in the URL, and the response was sandboxed with allow-scripts, so script inside an uploaded HTML object could read its own location and send the viewer's session token away. Inline views now run no scripts and load nothing from elsewhere, API responses send Referrer-Policy: no-referrer, and the console itself is served with X-Frame-Options/frame-ancestors so it cannot be framed. --- cli/internal/server/sandbox_test.go | 31 +++++++++++++++++++++++++++++ cli/internal/server/server.go | 15 ++++++++++++-- cli/internal/web/web.go | 18 ++++++++++++++++- cli/internal/web/web_test.go | 19 ++++++++++++++++++ 4 files changed, 80 insertions(+), 3 deletions(-) create mode 100644 cli/internal/server/sandbox_test.go create mode 100644 cli/internal/web/web_test.go diff --git a/cli/internal/server/sandbox_test.go b/cli/internal/server/sandbox_test.go new file mode 100644 index 00000000..c08fa3dc --- /dev/null +++ b/cli/internal/server/sandbox_test.go @@ -0,0 +1,31 @@ +package server + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// The console opens an object inline with the viewer's session token in the URL +// (?access_token=), so a script inside the object could read the token from its +// own location. Inline views must therefore run no scripts. +func TestInlineObjectViewsRunNoScripts(t *testing.T) { + h := sandboxUserContent(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + r := httptest.NewRequest(http.MethodGet, "/api/v1/s3/b/object?key=x.html&inline=true&access_token=hcs_secret", nil) + w := httptest.NewRecorder() + h.ServeHTTP(w, r) + csp := w.Header().Get("Content-Security-Policy") + if !strings.HasPrefix(csp, "sandbox;") || strings.Contains(csp, "allow-scripts") || !strings.Contains(csp, "default-src 'none'") { + t.Fatalf("CSP %q lets an object run scripts or load resources", csp) + } + if w.Header().Get("Referrer-Policy") != "no-referrer" { + t.Fatal("the token-bearing URL would be sent as a Referer") + } + // Hosted websites keep their scripts (they are sandboxed to an opaque origin and carry no token). + w = httptest.NewRecorder() + h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/website/b/index.html", nil)) + if !strings.Contains(w.Header().Get("Content-Security-Policy"), "allow-scripts") { + t.Fatal("website sandbox lost allow-scripts") + } +} diff --git a/cli/internal/server/server.go b/cli/internal/server/server.go index fb77c416..9fc38f53 100644 --- a/cli/internal/server/server.go +++ b/cli/internal/server/server.go @@ -453,6 +453,11 @@ func Run(ctx context.Context, cfg core.Config, opts Options) error { func vpcList(st *store.Store) []vpc.VPC { return store.List[vpc.VPC](st, "vpc_vpcs") } +// inlineObjectCSP is the policy of object bytes shown from the native API: an +// opaque origin, no scripts, forms or navigation, and only inline styles and +// data:/same-origin media. +const inlineObjectCSP = "sandbox; default-src 'none'; style-src 'unsafe-inline'; img-src data: 'self'; media-src data: 'self'; font-src data:; form-action 'none'; base-uri 'none'" + // sandboxUserContent isolates responses whose bytes come from users (static // websites, function URLs, HTTP APIs and inline object views). They share an // origin with the console, so without a sandbox a page could read the console's @@ -460,13 +465,19 @@ func vpcList(st *store.Store) []vpc.VPC { return store.List[vpc.VPC](st, "vpc_vp func sandboxUserContent(h http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { p := r.URL.Path - if strings.HasPrefix(p, "/website/") || strings.HasPrefix(p, "/lambda-url/") || strings.HasPrefix(p, "/apigw/") || - (strings.HasPrefix(p, "/api/v1/s3/") && strings.HasSuffix(p, "/object")) { + if strings.HasPrefix(p, "/api/v1/s3/") && strings.HasSuffix(p, "/object") { + // The console opens objects with ?access_token= in the URL, which + // a script in the object could read from its own location and send away. + // So inline views run no scripts, load nothing from elsewhere and send no Referer. + w.Header().Set("Content-Security-Policy", inlineObjectCSP) + w.Header().Set("X-Content-Type-Options", "nosniff") + } else if strings.HasPrefix(p, "/website/") || strings.HasPrefix(p, "/lambda-url/") || strings.HasPrefix(p, "/apigw/") { w.Header().Set("Content-Security-Policy", "sandbox allow-scripts allow-forms allow-popups allow-modals allow-downloads") w.Header().Set("X-Content-Type-Options", "nosniff") } if strings.HasPrefix(p, "/api/") { w.Header().Set("X-Content-Type-Options", "nosniff") + w.Header().Set("Referrer-Policy", "no-referrer") } h.ServeHTTP(w, r) }) diff --git a/cli/internal/web/web.go b/cli/internal/web/web.go index aa21f0c4..bd421c1e 100644 --- a/cli/internal/web/web.go +++ b/cli/internal/web/web.go @@ -14,7 +14,23 @@ var dist embed.FS // Handler serves the console. Unknown paths fall back to their .html page or // to index.html so client-side routes survive a reload. -func Handler() http.Handler { +func Handler() http.Handler { return withSecurityHeaders(handler()) } + +// withSecurityHeaders forbids framing the console (clickjacking of a signed-in +// administrator), plugins and foreign form targets, and keeps the console's +// URLs out of Referer headers. +func withSecurityHeaders(h http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hd := w.Header() + hd.Set("X-Frame-Options", "DENY") + hd.Set("Content-Security-Policy", "frame-ancestors 'none'; object-src 'none'; base-uri 'self'; form-action 'self'") + hd.Set("X-Content-Type-Options", "nosniff") + hd.Set("Referrer-Policy", "no-referrer") + h.ServeHTTP(w, r) + }) +} + +func handler() http.Handler { root, _ := fs.Sub(dist, "dist") files := http.FileServerFS(root) if _, err := fs.Stat(root, "index.html"); err != nil { diff --git a/cli/internal/web/web_test.go b/cli/internal/web/web_test.go new file mode 100644 index 00000000..ae3f60a9 --- /dev/null +++ b/cli/internal/web/web_test.go @@ -0,0 +1,19 @@ +package web + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func TestConsoleCannotBeFramed(t *testing.T) { + w := httptest.NewRecorder() + Handler().ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/", nil)) + if w.Header().Get("X-Frame-Options") != "DENY" || !strings.Contains(w.Header().Get("Content-Security-Policy"), "frame-ancestors 'none'") { + t.Fatalf("console can be framed: %v", w.Header()) + } + if w.Header().Get("Referrer-Policy") != "no-referrer" { + t.Fatal("console sends Referer headers") + } +} From e4bf457a7a86456b536d4ac357e50c6dcc31e8f8 Mon Sep 17 00:00:00 2001 From: Suryansh Prajapati <58465650+drk1rd@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:25:08 +0530 Subject: [PATCH 10/18] Keep access key secrets out of URLs and stop session tokens renewing themselves ?access_token= accepted a full accessKeyId:secret, which would land in access logs and browser history; only console session tokens may travel in the query string now. GetSessionToken also refuses temporary credentials, as AWS does, so a stolen session cannot be renewed indefinitely. --- cli/internal/svc/iam/iam.go | 7 ++++- cli/internal/svc/iam/login_test.go | 42 ++++++++++++++++++++++++++++++ cli/internal/svc/iam/roles.go | 7 +++-- 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/cli/internal/svc/iam/iam.go b/cli/internal/svc/iam/iam.go index a23f4f8c..e656395a 100644 --- a/cli/internal/svc/iam/iam.go +++ b/cli/internal/svc/iam/iam.go @@ -331,7 +331,12 @@ func (s *Service) Authenticate(r *http.Request) (*httpx.Principal, error) { if h := r.Header.Get("Authorization"); strings.HasPrefix(h, "Bearer ") { token = strings.TrimSpace(strings.TrimPrefix(h, "Bearer ")) } else if r.Method == http.MethodGet { - token = r.URL.Query().Get("access_token") // lets the console link downloads directly + // Lets the console link downloads directly. URLs end up in logs and history, + // so only a (revocable, expiring) console session may travel in one, never an + // access key secret. + if t := r.URL.Query().Get("access_token"); strings.HasPrefix(t, "hcs_") { + token = t + } } if token == "" { return nil, core.Errf(http.StatusUnauthorized, "MissingAuthenticationToken", "request is not signed: send 'Authorization: Bearer :'") diff --git a/cli/internal/svc/iam/login_test.go b/cli/internal/svc/iam/login_test.go index 4dc03fb8..69112afb 100644 --- a/cli/internal/svc/iam/login_test.go +++ b/cli/internal/svc/iam/login_test.go @@ -2,6 +2,7 @@ package iam_test import ( "bytes" + "encoding/json" "net/http" "strings" "sync" @@ -33,6 +34,47 @@ func TestLoginThrottleHoldsUnderParallelBurst(t *testing.T) { } } +// Query-string credentials end up in access logs and browser history: only a +// console session may be sent that way, never an access key secret. +func TestQueryTokenAcceptsSessionsOnly(t *testing.T) { + h := awstest.New(t) + get := func(tok string) int { + resp, err := http.Get(h.URL + "/api/v1/auth/whoami?access_token=" + tok) + if err != nil { + t.Fatal(err) + } + resp.Body.Close() + return resp.StatusCode + } + if st := get(h.AccessKeyID + ":" + h.SecretKey); st != http.StatusUnauthorized { + t.Fatalf("an access key secret in the query string was accepted: %d", st) + } + if err := h.IAM.SetRootPassword("a-chosen-passphrase"); err != nil { + t.Fatal(err) + } + resp, err := http.Post(h.URL+"/api/v1/auth/login", "application/json", strings.NewReader(`{"username":"root","password":"a-chosen-passphrase"}`)) + if err != nil { + t.Fatal(err) + } + var out struct{ Token string } + _ = json.NewDecoder(resp.Body).Decode(&out) + resp.Body.Close() + if st := get(out.Token); st != http.StatusOK { + t.Fatalf("a session token in the query string: %d", st) + } +} + +// Temporary credentials cannot mint new ones: a stolen session token would +// otherwise be renewable forever, surviving the revocation of the user's keys. +func TestSessionTokensCannotBeRenewed(t *testing.T) { + h := awstest.New(t) + c := h.AWSJSON(t, "sts", "get-session-token")["Credentials"].(map[string]any) + out, err := h.AWSAs(t, c["AccessKeyId"].(string), c["SecretAccessKey"].(string), c["SessionToken"].(string), "sts", "get-session-token") + if err == nil || !strings.Contains(out, "AccessDenied") { + t.Fatalf("renewing a session token: %v %s", err, out) + } +} + func TestLoginRejectsHugeBodies(t *testing.T) { h := awstest.New(t) body := `{"username":"root","password":"` + strings.Repeat("a", 200<<10) + `"}` diff --git a/cli/internal/svc/iam/roles.go b/cli/internal/svc/iam/roles.go index 39707ff3..0db01de6 100644 --- a/cli/internal/svc/iam/roles.go +++ b/cli/internal/svc/iam/roles.go @@ -297,8 +297,11 @@ func (s *Service) ServiceRolePrincipal(ref, service, sessionName string) (*httpx // SessionToken issues temporary credentials carrying a user's own permissions. func (s *Service) SessionToken(p *httpx.Principal, seconds int) (Credentials, error) { - if p.RoleName != "" { - return Credentials{}, core.Errf(http.StatusForbidden, "AccessDenied", "GetSessionToken cannot be called with temporary role credentials") + if p.RoleName != "" || strings.HasPrefix(p.AccessKey, tempKeyPrefix) { + // As in AWS, only long-term credentials can mint session tokens; otherwise a + // stolen session could be renewed forever, outliving the revocation of the + // user's access keys. + return Credentials{}, core.Errf(http.StatusForbidden, "AccessDenied", "GetSessionToken cannot be called with temporary credentials") } ttl, err := stsTTL(seconds, nil) if err != nil { From bc9c52b2b030fba540fc71d8ef50bc207f6808d5 Mon Sep 17 00:00:00 2001 From: Suryansh Prajapati <58465650+drk1rd@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:26:02 +0530 Subject: [PATCH 11/18] Lambda: refuse decompression bombs and oversized package listings at upload Packages were only checked for being a valid zip, then unpacked in memory at every cold start (and copied a second time into a tar), so a tiny archive of zeros could make each invocation allocate hundreds of megabytes. checkZip now sums the declared uncompressed sizes against the 250 MB limit and caps the entry count, and unzip reads no more than the remaining budget per file. --- cli/internal/svc/lambda/lambda.go | 28 +++++++++-- cli/internal/svc/lambda/zipbomb_test.go | 65 +++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 4 deletions(-) create mode 100644 cli/internal/svc/lambda/zipbomb_test.go diff --git a/cli/internal/svc/lambda/lambda.go b/cli/internal/svc/lambda/lambda.go index d2fb07c5..0e1a1fcc 100644 --- a/cli/internal/svc/lambda/lambda.go +++ b/cli/internal/svc/lambda/lambda.go @@ -274,10 +274,29 @@ func (ci codeInput) zip() ([]byte, error) { return zipFiles(ci.Files) } +// maxUnzipped and maxZipEntries bound what a package may expand to. The zip is +// unpacked in memory on every cold start, so a small archive of zeros must be +// refused when it is uploaded, not when a function first runs. +const ( + maxUnzipped = 5 * maxCodeBytes + maxZipEntries = 20000 +) + func checkZip(b []byte) error { - if _, err := zip.NewReader(bytes.NewReader(b), int64(len(b))); err != nil { + zr, err := zip.NewReader(bytes.NewReader(b), int64(len(b))) + if err != nil { return core.BadRequest("Could not unzip uploaded file. Please check your file, then try to upload again. (%v)", err) } + if len(zr.File) > maxZipEntries { + return core.BadRequest("The package has more than %d files.", maxZipEntries) + } + var total uint64 + for _, f := range zr.File { + total += f.UncompressedSize64 + if total > maxUnzipped { + return core.BadRequest("Unzipped size must be smaller than %d bytes", int64(maxUnzipped)) + } + } return nil } @@ -325,14 +344,15 @@ func unzip(b []byte) (map[string][]byte, error) { if err != nil { return nil, err } - data, err := io.ReadAll(io.LimitReader(rc, 4*maxCodeBytes+1)) + // A header may understate a file's size: read no more than the budget left. + data, err := io.ReadAll(io.LimitReader(rc, maxUnzipped-total+1)) rc.Close() if err != nil { return nil, err } total += int64(len(data)) - if total > 5*maxCodeBytes { - return nil, core.BadRequest("Unzipped size must be smaller than %d bytes", 5*maxCodeBytes) + if total > maxUnzipped { + return nil, core.BadRequest("Unzipped size must be smaller than %d bytes", int64(maxUnzipped)) } out[clean] = data } diff --git a/cli/internal/svc/lambda/zipbomb_test.go b/cli/internal/svc/lambda/zipbomb_test.go new file mode 100644 index 00000000..c4a5c7ab --- /dev/null +++ b/cli/internal/svc/lambda/zipbomb_test.go @@ -0,0 +1,65 @@ +package lambda + +import ( + "archive/zip" + "bytes" + "io" + "strings" + "testing" +) + +type zeros int64 + +func (z *zeros) Read(p []byte) (int, error) { + if *z <= 0 { + return 0, io.EOF + } + n := int64(len(p)) + if n > int64(*z) { + n = int64(*z) + } + clear(p[:n]) + *z -= zeros(n) + return int(n), nil +} + +// A few kilobytes of zip can expand to gigabytes; the package is unpacked in +// memory at every cold start, so the expansion must be refused at upload. +func TestCheckZipRefusesDecompressionBombs(t *testing.T) { + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + w, _ := zw.Create("big.bin") + z := zeros(maxUnzipped + 1<<20) + if _, err := io.Copy(w, &z); err != nil { + t.Fatal(err) + } + zw.Close() + if buf.Len() > maxCodeBytes { + t.Fatalf("test archive is %d bytes", buf.Len()) + } + if err := checkZip(buf.Bytes()); err == nil || !strings.Contains(err.Error(), "Unzipped size") { + t.Fatalf("a bomb of %d bytes (zipped %d) was accepted: %v", maxUnzipped+1<<20, buf.Len(), err) + } + if _, err := unzip(buf.Bytes()); err == nil { + t.Fatal("unzip expanded a bomb") + } + + many := new(bytes.Buffer) + zw = zip.NewWriter(many) + for i := 0; i <= maxZipEntries; i++ { + zw.Create(string(rune('a'+i%26)) + strings.Repeat("x", i%7) + "/" + strings.Repeat("y", i/26%50) + string(rune(0x4e00+i))) + } + zw.Close() + if err := checkZip(many.Bytes()); err == nil { + t.Fatal("an archive with too many entries was accepted") + } + + var ok bytes.Buffer + zw = zip.NewWriter(&ok) + w, _ = zw.Create("index.py") + w.Write([]byte("def handler(e, c): return 1")) + zw.Close() + if err := checkZip(ok.Bytes()); err != nil { + t.Fatal(err) + } +} From 7b076522532827a69b45ee24cc3f042cc8250429 Mon Sep 17 00:00:00 2001 From: Suryansh Prajapati <58465650+drk1rd@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:26:47 +0530 Subject: [PATCH 12/18] Console: close the open redirect after sign-in (/\host and tab-split paths) safeNext refused "//host" but let "/\\evil.example" and "//evil.example" through; browsers read both as "//evil.example", so a crafted sign-in link sent the user to another site right after entering their password. The path is now rejected on backslashes and control characters and must resolve to this origin. --- console/app/login/page.tsx | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/console/app/login/page.tsx b/console/app/login/page.tsx index 419994e5..535130cf 100644 --- a/console/app/login/page.tsx +++ b/console/app/login/page.tsx @@ -10,8 +10,21 @@ import { Label } from "@/components/ui/label" import { Logo } from "@/components/console/topbar" import { BASE_PATH, errorMessage, getSession, login } from "@/lib/api" +// safeNext only lets a same-site path through. Browsers read "\" as "/" and drop +// tabs and newlines inside URLs, so "/\evil.example" and "//evil.example" +// both mean "//evil.example": refuse them by character, then confirm by resolving +// the path against this origin. function safeNext(next: string | null): string { if (!next || !next.startsWith("/") || next.startsWith("//") || next.startsWith("/login")) return "/" + // eslint-disable-next-line no-control-regex + if (/[\\\u0000-\u001f\u007f]/.test(next)) return "/" + if (typeof window !== "undefined") { + try { + if (new URL(next, window.location.origin).origin !== window.location.origin) return "/" + } catch { + return "/" + } + } return next } From abdff417a41bb06eac0b5219800c6fc5d867b44b Mon Sep 17 00:00:00 2001 From: Suryansh Prajapati <58465650+drk1rd@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:27:21 +0530 Subject: [PATCH 13/18] Do not return internal error text to clients or the audit trail Unexpected errors (file paths, Docker daemon output, database errors) were copied into 500 responses, SQS batch results and, through the error message of the trail, into other users' CloudTrail LookupEvents. Clients now get a fixed message and the detail stays in the server log. --- cli/internal/awsapi/awsapi.go | 2 +- cli/internal/awsapi/preauth_test.go | 14 ++++++++++++++ cli/internal/core/core.go | 5 +++++ cli/internal/httpx/httpx.go | 2 +- cli/internal/svc/sqs/aws.go | 4 +++- console/node_modules | 1 + 6 files changed, 25 insertions(+), 3 deletions(-) create mode 120000 console/node_modules diff --git a/cli/internal/awsapi/awsapi.go b/cli/internal/awsapi/awsapi.go index 2bfaeaea..15f6a578 100644 --- a/cli/internal/awsapi/awsapi.go +++ b/cli/internal/awsapi/awsapi.go @@ -262,7 +262,7 @@ func toError(s *Service, err error) *Error { ce = core.Errf(http.StatusConflict, "Conflict", "the resource was modified concurrently; retry") default: log.Printf("aws %s: internal error: %v", s.Name, err) - ce = core.Errf(http.StatusInternalServerError, "InternalError", "%v", err) + ce = core.Errf(http.StatusInternalServerError, "InternalError", "%s", core.InternalErrorMessage) } code := "" if s != nil && s.ErrorCode != nil { diff --git a/cli/internal/awsapi/preauth_test.go b/cli/internal/awsapi/preauth_test.go index 48e79acf..57ab1d67 100644 --- a/cli/internal/awsapi/preauth_test.go +++ b/cli/internal/awsapi/preauth_test.go @@ -67,6 +67,20 @@ func TestUnsignedBodyIsCapped(t *testing.T) { } } +// Unexpected failures carry file paths and daemon output: they belong in the +// server log, not in the response, the audit trail or other users' LookupEvents. +func TestInternalErrorsAreNotEchoed(t *testing.T) { + e := toError(&Service{Name: "x"}, errors.New("open /home/svc/.homecloud/state.json: permission denied")) + if e.Code != "InternalFailure" || strings.Contains(e.Message, "/home/svc") { + t.Fatalf("internal error leaked: %+v", e) + } + w := httptest.NewRecorder() + httpx.WriteError(w, errors.New("docker: dial unix /var/run/docker.sock: connect: permission denied")) + if w.Code != 500 || strings.Contains(w.Body.String(), "docker.sock") { + t.Fatalf("native API leaked an internal error: %d %s", w.Code, w.Body) + } +} + func TestHostAndTargetMustBeSigned(t *testing.T) { for _, signed := range []string{"x-amz-date", "host;x-amz-date"} { if _, err := ParseSignature(signedReq(t, signed, strings.NewReader(""))); err == nil { diff --git a/cli/internal/core/core.go b/cli/internal/core/core.go index 3e6e614b..32d17242 100644 --- a/cli/internal/core/core.go +++ b/cli/internal/core/core.go @@ -243,6 +243,11 @@ func CanonicalARN(s string) string { return strings.Join(parts, ":") } +// InternalErrorMessage is what clients are told when a request fails for a +// reason that is not a *Error: the real error (file paths, Docker daemon output, +// database errors) goes to the server log, not to the caller or the audit trail. +const InternalErrorMessage = "an internal error occurred; details are in the server log" + // IsLocalARN reports whether arn names a resource of this deployment: the same // partition and account, and the deployment's region (or none, for global // services). Code that delivers to "the resource named by this ARN" by looking diff --git a/cli/internal/httpx/httpx.go b/cli/internal/httpx/httpx.go index 366f3f72..337139e0 100644 --- a/cli/internal/httpx/httpx.go +++ b/cli/internal/httpx/httpx.go @@ -274,7 +274,7 @@ func WriteError(w http.ResponseWriter, err error) { ce = core.Errf(http.StatusNotFound, "ResourceNotFound", "resource not found") default: log.Printf("internal error: %v", err) - ce = core.Errf(http.StatusInternalServerError, "InternalError", "%v", err) + ce = core.Errf(http.StatusInternalServerError, "InternalError", "%s", core.InternalErrorMessage) } WriteJSON(w, ce.Status, map[string]any{"error": ce}) } diff --git a/cli/internal/svc/sqs/aws.go b/cli/internal/svc/sqs/aws.go index 3823da84..80aa2e76 100644 --- a/cli/internal/svc/sqs/aws.go +++ b/cli/internal/svc/sqs/aws.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "log" "net/http" "net/url" "reflect" @@ -867,7 +868,8 @@ func entryError(id string, err error) batchError { if errors.As(e, &ae) { return batchError{Id: id, SenderFault: ae.Status < 500, Code: ae.Code, Message: ae.Message} } - return batchError{Id: id, Code: "InternalError", Message: err.Error()} + log.Printf("sqs: batch entry %s: %v", id, err) + return batchError{Id: id, Code: "InternalError", Message: core.InternalErrorMessage} } func isNoQueue(err error) bool { diff --git a/console/node_modules b/console/node_modules new file mode 120000 index 00000000..2a560b7b --- /dev/null +++ b/console/node_modules @@ -0,0 +1 @@ +/Users/drk1rd/Documents/GitHub/homecloud/console/node_modules \ No newline at end of file From 9e8896b0617a0130bba5e2bfb6d63853c86ac36e Mon Sep 17 00:00:00 2001 From: Suryansh Prajapati <58465650+drk1rd@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:32:22 +0530 Subject: [PATCH 14/18] S3: serve websites per key by policy, check DeleteObjects keys, keep /lambda-code/ out of S3 Website hosting decided that a bucket was public by looking for the strings "*" and s3:GetObject in MinIO's copy of the policy, then served every key with the server's storage credentials: a policy granting one prefix, or carrying a Deny or a condition, exposed the whole bucket. Each requested key is now evaluated against the bucket policy as an anonymous caller. DeleteObjects body keys get the same dot-segment and length checks as URL keys, and /lambda-code/ and /_s3/ are never claimed by anonymous S3 routing (a bucket named lambda-code broke code downloads). --- cli/internal/svc/s3/aws.go | 11 +++- cli/internal/svc/s3/s3.go | 28 +++++--- cli/internal/svc/s3/website_security_test.go | 67 ++++++++++++++++++++ 3 files changed, 97 insertions(+), 9 deletions(-) create mode 100644 cli/internal/svc/s3/website_security_test.go diff --git a/cli/internal/svc/s3/aws.go b/cli/internal/svc/s3/aws.go index 6f0f32e0..476c27e9 100644 --- a/cli/internal/svc/s3/aws.go +++ b/cli/internal/svc/s3/aws.go @@ -150,7 +150,7 @@ func (s *Service) claimsUnsigned(r *http.Request) bool { return v2 || s.knownBucket(b) } p := r.URL.Path - for _, pre := range []string{"/api/", "/website/", "/lambda-url/", "/apigw/", "/cognito/", "/_next/"} { + for _, pre := range []string{"/api/", "/website/", "/lambda-url/", "/apigw/", "/cognito/", "/_next/", "/lambda-code/", "/_s3/"} { if strings.HasPrefix(p, pre) { return false } @@ -993,6 +993,15 @@ func (s *Service) deleteObjects(a *s3req) error { return awsapi.Errorf(http.StatusBadRequest, "MalformedXML", "the XML you provided was not well-formed") } for _, o := range in.Objects { + // Keys in the body are forwarded as they are, so they need the same check + // as the key in the URL: "allowed/../secret" is authorized as a key under + // allowed/ but may name another object. + if err := checkKey(o.Key); err != nil { + return err + } + if len(o.Key) > 1024 || strings.ContainsRune(o.Key, 0) { + return awsapi.Errorf(http.StatusBadRequest, "KeyTooLongError", "the object key is invalid") + } act := "s3:DeleteObject" if o.VersionID != "" { act = "s3:DeleteObjectVersion" diff --git a/cli/internal/svc/s3/s3.go b/cli/internal/svc/s3/s3.go index 89069f65..ba2200a2 100644 --- a/cli/internal/svc/s3/s3.go +++ b/cli/internal/svc/s3/s3.go @@ -858,19 +858,31 @@ func (s *Service) website(c *httpx.Ctx) (any, error) { if err != nil { return nil, err } - if !s.isPublic(c.R.Context(), cl, bucket) { - return nil, core.Errf(http.StatusForbidden, "AccessDenied", "bucket %q hosts a website but does not allow public reads", bucket) - } + // The site is served with the server's own storage credentials, so each key + // must be one the bucket policy lets an anonymous caller read: a policy that + // merely mentions "*" and s3:GetObject (for one prefix, with a Deny or a + // condition) does not make the whole bucket public. + policy, _ := s.bucketPolicy(c.R.Context(), bucket) + acc := httpx.Access{Policy: policy, Keys: httpx.RequestContext(c.R)} + public := func(k string) bool { + return policy != "" && httpx.PermitsAnonymous("s3:GetObject", "arn:aws:s3:::"+bucket+"/"+k, acc) + } + denied := core.Errf(http.StatusForbidden, "AccessDenied", "bucket %q hosts a website but does not allow public reads of this object", bucket) if key == "" || strings.HasSuffix(key, "/") { key += m.IndexDocument } + if !public(key) { + return nil, denied + } if _, err := cl.StatObject(c.R.Context(), bucket, key, minio.StatObjectOptions{}); err != nil { - if _, err2 := cl.StatObject(c.R.Context(), bucket, key+"/"+m.IndexDocument, minio.StatObjectOptions{}); err2 == nil { - http.Redirect(c.W, c.R, c.R.URL.Path+"/", http.StatusFound) - c.MarkWritten() - return nil, nil + if public(key + "/" + m.IndexDocument) { + if _, err2 := cl.StatObject(c.R.Context(), bucket, key+"/"+m.IndexDocument, minio.StatObjectOptions{}); err2 == nil { + http.Redirect(c.W, c.R, c.R.URL.Path+"/", http.StatusFound) + c.MarkWritten() + return nil, nil + } } - if m.ErrorDocument != "" { + if m.ErrorDocument != "" && public(m.ErrorDocument) { return nil, s.stream(c, cl, bucket, m.ErrorDocument, "", false, http.StatusNotFound) } return nil, core.Errf(http.StatusNotFound, "NoSuchKey", "%s not found", key) diff --git a/cli/internal/svc/s3/website_security_test.go b/cli/internal/svc/s3/website_security_test.go new file mode 100644 index 00000000..7762b39c --- /dev/null +++ b/cli/internal/svc/s3/website_security_test.go @@ -0,0 +1,67 @@ +package s3 + +import ( + "io" + "net/http" + "os" + "strings" + "testing" +) + +func getStatus(t *testing.T, url string) (int, string) { + t.Helper() + resp, err := http.Get(url) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + b, _ := io.ReadAll(resp.Body) + return resp.StatusCode, string(b) +} + +// The website endpoint reads with the server's own storage credentials, so it +// may only serve keys the bucket policy lets an anonymous caller read: a policy +// that grants one prefix, or carries a Deny, does not publish the whole bucket. +func TestWebsiteServesOnlyWhatThePolicyAllows(t *testing.T) { + h, _ := newHarness(t) + b := bucketName() + h.AWS(t, "s3", "mb", "s3://"+b) + dir := t.TempDir() + _ = os.WriteFile(dir+"/f", []byte("content"), 0o600) + h.AWS(t, "s3", "cp", dir+"/f", "s3://"+b+"/pub/index.html") + h.AWS(t, "s3", "cp", dir+"/f", "s3://"+b+"/pub/secret.txt") + h.AWS(t, "s3", "cp", dir+"/f", "s3://"+b+"/private/data.txt") + h.AWS(t, "s3api", "put-bucket-website", "--bucket", b, "--website-configuration", `{"IndexDocument":{"Suffix":"index.html"}}`) + h.AWS(t, "s3api", "put-bucket-policy", "--bucket", b, "--policy", `{"Version":"2012-10-17","Statement":[ + {"Effect":"Allow","Principal":"*","Action":"s3:GetObject","Resource":"arn:aws:s3:::`+b+`/pub/*"}, + {"Effect":"Deny","Principal":"*","Action":"s3:GetObject","Resource":"arn:aws:s3:::`+b+`/pub/secret.txt"}]}`) + + if st, body := getStatus(t, h.URL+"/website/"+b+"/pub/index.html"); st != 200 || body != "content" { + t.Fatalf("allowed page: %d %q", st, body) + } + for _, p := range []string{"/private/data.txt", "/pub/secret.txt", "/index.html"} { + if st, body := getStatus(t, h.URL+"/website/"+b+p); st != 403 || strings.Contains(body, "content") { + t.Fatalf("%s: %d %q", p, st, body) + } + } +} + +// Keys in a DeleteObjects body are forwarded as they are, so "allowed/../x" must +// not be authorized as a key under allowed/. +func TestDeleteObjectsRejectsDotSegments(t *testing.T) { + h, _ := newHarness(t) + b := bucketName() + h.AWS(t, "s3", "mb", "s3://"+b) + dir := t.TempDir() + _ = os.WriteFile(dir+"/f", []byte("x"), 0o600) + h.AWS(t, "s3", "cp", dir+"/f", "s3://"+b+"/keep") + ak, sk := h.User(t, "deleter") + h.Native(t, "PUT", "/api/v1/iam/users/deleter/inline-policies/one", map[string]any{ + "Version": "2012-10-17", "Statement": []any{map[string]any{"Effect": "Allow", "Action": "s3:*", + "Resource": []string{"arn:aws:s3:::" + b, "arn:aws:s3:::" + b + "/allowed/*"}}}}) + out, err := h.AWSAs(t, ak, sk, "", "s3api", "delete-objects", "--bucket", b, "--delete", `{"Objects":[{"Key":"allowed/../keep"}]}`) + mustFail(t, out, err, "InvalidArgument") + if out := h.AWS(t, "s3", "ls", "s3://"+b); !strings.Contains(out, "keep") { + t.Fatalf("object deleted through a dot segment: %s", out) + } +} From a25334d55375355dd32a3ebab6fe46a6cfce9f43 Mon Sep 17 00:00:00 2001 From: Suryansh Prajapati <58465650+drk1rd@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:32:45 +0530 Subject: [PATCH 15/18] Pin the sandbox headers of function URL, HTTP API and website responses Functions and HTTP_PROXY upstreams choose their own response headers, and respond() copied them over the sandbox Content-Security-Policy, so a function could serve a page with script on the console's origin and take over the session of an administrator who opened its URL. The sandbox and nosniff headers are now re-applied when the response is written. --- cli/internal/server/sandbox_test.go | 17 ++++++++++++++++ cli/internal/server/server.go | 30 +++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/cli/internal/server/sandbox_test.go b/cli/internal/server/sandbox_test.go index c08fa3dc..b7f61314 100644 --- a/cli/internal/server/sandbox_test.go +++ b/cli/internal/server/sandbox_test.go @@ -10,6 +10,23 @@ import ( // The console opens an object inline with the viewer's session token in the URL // (?access_token=), so a script inside the object could read the token from its // own location. Inline views must therefore run no scripts. +// A function (or an HTTP integration's upstream) picks its own response headers; +// it must not be able to replace the sandbox and run script on the console origin. +func TestUserContentCannotReplaceTheSandbox(t *testing.T) { + h := sandboxUserContent(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Security-Policy", "default-src *") + w.Header().Set("X-Content-Type-Options", "") + w.Write([]byte("")) + })) + for _, p := range []string{"/lambda-url/f/", "/apigw/x/y", "/website/b/index.html"} { + w := httptest.NewRecorder() + h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, p, nil)) + if csp := w.Header().Get("Content-Security-Policy"); !strings.HasPrefix(csp, "sandbox ") || w.Header().Get("X-Content-Type-Options") != "nosniff" { + t.Fatalf("%s: response replaced the sandbox: %q", p, csp) + } + } +} + func TestInlineObjectViewsRunNoScripts(t *testing.T) { h := sandboxUserContent(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) r := httptest.NewRequest(http.MethodGet, "/api/v1/s3/b/object?key=x.html&inline=true&access_token=hcs_secret", nil) diff --git a/cli/internal/server/server.go b/cli/internal/server/server.go index 9fc38f53..a695a97f 100644 --- a/cli/internal/server/server.go +++ b/cli/internal/server/server.go @@ -479,10 +479,40 @@ func sandboxUserContent(h http.Handler) http.Handler { w.Header().Set("X-Content-Type-Options", "nosniff") w.Header().Set("Referrer-Policy", "no-referrer") } + if csp := w.Header().Get("Content-Security-Policy"); csp != "" { + // Functions and HTTP integrations choose their own response headers: pin + // the sandbox so they cannot replace it and run on the console's origin. + w = &pinnedHeaders{ResponseWriter: w, pins: map[string]string{"Content-Security-Policy": csp, "X-Content-Type-Options": "nosniff"}} + } h.ServeHTTP(w, r) }) } +// pinnedHeaders re-applies fixed headers just before the response is written, +// whatever the handler set in between. +type pinnedHeaders struct { + http.ResponseWriter + pins map[string]string +} + +func (p *pinnedHeaders) pin() { + for k, v := range p.pins { + p.Header().Set(k, v) + } +} +func (p *pinnedHeaders) WriteHeader(code int) { p.pin(); p.ResponseWriter.WriteHeader(code) } +func (p *pinnedHeaders) Write(b []byte) (int, error) { + p.pin() + return p.ResponseWriter.Write(b) +} +func (p *pinnedHeaders) Flush() { + p.pin() + if f, ok := p.ResponseWriter.(http.Flusher); ok { + f.Flush() + } +} +func (p *pinnedHeaders) Unwrap() http.ResponseWriter { return p.ResponseWriter } + // withCORS allows the console dev server and other origins to call the API. // Credentials travel in the Authorization header, never cookies, so a wildcard is safe. func withCORS(h http.Handler) http.Handler { From 57f5f7ce8f1304db794f2834b6e6e55103e8822b Mon Sep 17 00:00:00 2001 From: Suryansh Prajapati <58465650+drk1rd@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:34:15 +0530 Subject: [PATCH 16/18] Cognito: bound password guesses per account and self sign-ups per pool The sign-in throttle was keyed by pool, user and source address, so rotating addresses had no per-account limit, and self sign-up (a bcrypt hash and a store write per call) had none at all. Password and SRP sign-in now also count attempts per account across addresses, and each pool accepts a bounded number of self sign-ups per window. --- cli/internal/svc/cognito/cognito.go | 21 ++++++++++++--- cli/internal/svc/cognito/signup_limit_test.go | 27 +++++++++++++++++++ cli/internal/svc/cognito/srp.go | 2 +- 3 files changed, 46 insertions(+), 4 deletions(-) create mode 100644 cli/internal/svc/cognito/signup_limit_test.go diff --git a/cli/internal/svc/cognito/cognito.go b/cli/internal/svc/cognito/cognito.go index ee51f7e4..8fd41b39 100644 --- a/cli/internal/svc/cognito/cognito.go +++ b/cli/internal/svc/cognito/cognito.go @@ -976,7 +976,17 @@ func (s *Service) client(p Pool, id, secret string) (Client, error) { // attempt records a sign-in attempt for key and reports whether it is allowed. // Attempts are counted before the password check so parallel guesses cannot // slip past the limit; a success clears the record. -func (s *Service) attempt(key string) bool { +func (s *Service) attempt(key string) bool { return s.attemptLimit(key, 10) } + +const ( + // maxAccountAttempts bounds password guesses per user across all addresses in + // the window; maxSignUps bounds self sign-ups per pool (each costs a bcrypt hash). + maxAccountAttempts = 40 + maxSignUps = 120 +) + +// attemptLimit is attempt with a limit of max attempts per window. +func (s *Service) attemptLimit(key string, max int) bool { s.mu.Lock() defer s.mu.Unlock() recent := s.fails[key][:0] @@ -985,7 +995,7 @@ func (s *Service) attempt(key string) bool { recent = append(recent, t) } } - if len(recent) >= 10 { + if len(recent) >= max { s.fails[key] = recent return false } @@ -1045,6 +1055,9 @@ func (s *Service) selfSignUp(poolID string, in authInput) (User, error) { if _, err := s.client(p, in.ClientID, in.ClientSecret); err != nil { return User{}, err } + if !s.attemptLimit("signup/"+p.ID, maxSignUps) { + return User{}, core.Errf(http.StatusTooManyRequests, "TooManyRequestsException", "too many sign-ups; try again later") + } status := "UNCONFIRMED" if p.AutoConfirm { status = "CONFIRMED" @@ -1130,7 +1143,9 @@ func (s *Service) authenticate(poolID string, in authInput, ip string) (any, err switch in.Flow { case "", "USER_PASSWORD_AUTH": tk := p.ID + "/" + strings.ToLower(in.Username) + "/" + ip - if !s.attempt(tk) { + // The per-address limit alone is bypassed by rotating addresses: also bound + // the guesses an account can receive from everywhere together. + if !s.attemptLimit(p.ID+"/acct/"+strings.ToLower(in.Username), maxAccountAttempts) || !s.attempt(tk) { return nil, core.Errf(http.StatusTooManyRequests, "TooManyRequestsException", "too many failed attempts; try again later") } u, err := store.Get[User](s.env.Store, cUsers, userKey(p.ID, in.Username)) diff --git a/cli/internal/svc/cognito/signup_limit_test.go b/cli/internal/svc/cognito/signup_limit_test.go new file mode 100644 index 00000000..969b1ac9 --- /dev/null +++ b/cli/internal/svc/cognito/signup_limit_test.go @@ -0,0 +1,27 @@ +package cognito_test + +import ( + "encoding/json" + "fmt" + "testing" +) + +// Self sign-up is open to anyone and costs a bcrypt hash and a store write each: +// a pool must stop accepting a flood of them. +func TestSelfSignUpIsRateLimited(t *testing.T) { + h, _ := newCognito(t) + var pool, cl map[string]any + _ = json.Unmarshal(h.Native(t, "POST", "/api/v1/cognito/user-pools", map[string]any{"name": "open"}), &pool) + pid := pool["id"].(string) + _ = json.Unmarshal(h.Native(t, "POST", "/api/v1/cognito/user-pools/"+pid+"/clients", map[string]any{"name": "c"}), &cl) + limited := 0 + for i := 0; i < 130; i++ { + st, _ := post(t, h.URL+"/cognito/"+pid+"/sign-up", map[string]any{"client_id": cl["id"], "username": fmt.Sprintf("u%d", i), "password": "pass1234"}, "") + if st == 429 { + limited++ + } + } + if limited < 5 { + t.Fatalf("only %d of 130 sign-ups were refused", limited) + } +} diff --git a/cli/internal/svc/cognito/srp.go b/cli/internal/svc/cognito/srp.go index ce26e99e..3ce43d95 100644 --- a/cli/internal/svc/cognito/srp.go +++ b/cli/internal/svc/cognito/srp.go @@ -167,7 +167,7 @@ func (s *Service) srpInitiate(p Pool, cl Client, username, srpA, ip string) (map return nil, invalid("Invalid SRP_A") } rk := p.ID + "/srp/" + strings.ToLower(username) + "/" + ip - if !s.attempt(rk) { + if !s.attemptLimit(p.ID+"/acct/"+strings.ToLower(username), maxAccountAttempts) || !s.attempt(rk) { return nil, core.Errf(http.StatusTooManyRequests, "TooManyRequestsException", "too many failed attempts; try again later") } st := &srpPending{pool: p.ID, client: cl.ID, username: username, rateKey: rk, a: a, b: randInt(512), expires: time.Now().Add(srpChallengeTTL)} From 0fd25bde0af22626502b26b49c3d55dbb766c173 Mon Sep 17 00:00:00 2001 From: Suryansh Prajapati <58465650+drk1rd@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:36:23 +0530 Subject: [PATCH 17/18] S3: refuse unsigned sub-resources on SigV2 URLs and authorize object-lock headers A SigV2 signature covers only the sub-resources SigV2 defines, so whoever held a presigned URL could append ?retention, ?publicAccessBlock and similar and have the signer's permissions applied to the changed operation; those requests are now refused. Uploads that set retention or a legal hold through headers now need s3:PutObjectRetention / s3:PutObjectLegalHold, as in AWS. --- cli/internal/svc/s3/aws.go | 20 ++++++++++ cli/internal/svc/s3/sigv2.go | 13 +++++++ cli/internal/svc/s3/website_security_test.go | 41 ++++++++++++++++++++ 3 files changed, 74 insertions(+) diff --git a/cli/internal/svc/s3/aws.go b/cli/internal/svc/s3/aws.go index 476c27e9..971a4ada 100644 --- a/cli/internal/svc/s3/aws.go +++ b/cli/internal/svc/s3/aws.go @@ -466,6 +466,26 @@ func (s *Service) serveAWS(a *s3req) error { return err } } + // Uploads can set retention or a legal hold through headers: AWS needs the + // matching permissions for that, not just s3:PutObject. + if op.object && (op.action == "s3:PutObject" || op.name == "CopyObject" || op.name == "CreateMultipartUpload") { + h := q.R.Header + need := "" + if h.Get("X-Amz-Object-Lock-Mode") != "" || h.Get("X-Amz-Object-Lock-Retain-Until-Date") != "" { + need = "s3:PutObjectRetention" + } + if h.Get("X-Amz-Object-Lock-Legal-Hold") != "" { + need = "s3:PutObjectLegalHold" + } + if need != "" { + if err := s.authorize(a, need, res); err != nil { + return err + } + if err := s.authorize(a, op.action, res); err != nil { // recorded as the call's action + return err + } + } + } if _, err := s.cl(); err != nil { return err } diff --git a/cli/internal/svc/s3/sigv2.go b/cli/internal/svc/s3/sigv2.go index d4a22ce1..432e6a8a 100644 --- a/cli/internal/svc/s3/sigv2.go +++ b/cli/internal/svc/s3/sigv2.go @@ -97,6 +97,19 @@ func (s *Service) sigV2(a *s3req) (bool, error) { return true, awsapi.Errorf(http.StatusForbidden, "SignatureDoesNotMatch", "The request signature we calculated does not match the signature you provided. Check your key and signing method.") } + // SigV2 only signs the sub-resources listed above. Any other one the request + // carries (?retention, ?publicAccessBlock, ...) is not covered by the signature, + // so whoever holds a presigned URL could append it to change the operation. + for k := range a.query { + if v2SubResources[k] || valueParams[k] { + continue + } + for _, sub := range append(append([]subOps{}, bucketSubs...), objectSubs...) { + if sub.sub == k { + return true, awsapi.Errorf(http.StatusForbidden, "AccessDenied", "the ?%s sub-resource cannot be signed with signature version 2: use signature version 4", k) + } + } + } a.q.P = p return true, nil } diff --git a/cli/internal/svc/s3/website_security_test.go b/cli/internal/svc/s3/website_security_test.go index 7762b39c..9b0c8daa 100644 --- a/cli/internal/svc/s3/website_security_test.go +++ b/cli/internal/svc/s3/website_security_test.go @@ -46,6 +46,47 @@ func TestWebsiteServesOnlyWhatThePolicyAllows(t *testing.T) { } } +// A SigV2 presigned URL signs only the sub-resources SigV2 knows; appending +// another one (?retention, ?publicAccessBlock, ...) must not reuse the signature. +func TestSigV2PresignedURLCannotGainSubresources(t *testing.T) { + h, _ := newHarness(t) + b := bucketName() + h.AWS(t, "s3", "mb", "s3://"+b) + out := h.Python(t, ` +s3 = boto3.client("s3", config=botocore.config.Config(signature_version="s3")) +print(s3.generate_presigned_url("get_object", Params={"Bucket": "`+b+`", "Key": "f"}, ExpiresIn=300)) +`) + u := strings.TrimSpace(out) + if !strings.Contains(u, "AWSAccessKeyId=") { + t.Skipf("not a SigV2 URL: %s", u) + } + for _, sub := range []string{"retention", "publicAccessBlock", "legal-hold"} { + st, body := getStatus(t, u+"&"+sub) + if st != 403 || !strings.Contains(body, "signature version 2") { + t.Fatalf("?%s appended to a SigV2 URL: %d %s", sub, st, body) + } + } +} + +// Locking an object through upload headers needs the retention / legal hold +// permissions, not just s3:PutObject. +func TestObjectLockHeadersNeedTheirOwnPermissions(t *testing.T) { + h, _ := newHarness(t) + b := bucketName() + h.AWS(t, "s3", "mb", "s3://"+b) + dir := t.TempDir() + _ = os.WriteFile(dir+"/f", []byte("x"), 0o600) + ak, sk := h.User(t, "uploader") + h.Native(t, "PUT", "/api/v1/iam/users/uploader/inline-policies/one", map[string]any{ + "Version": "2012-10-17", "Statement": []any{map[string]any{"Effect": "Allow", "Action": "s3:PutObject", + "Resource": []string{"arn:aws:s3:::" + b + "/*"}}}}) + out, err := h.AWSAs(t, ak, sk, "", "s3api", "put-object", "--bucket", b, "--key", "k", "--body", dir+"/f", "--object-lock-legal-hold-status", "ON") + mustFail(t, out, err, "s3:PutObjectLegalHold") + out, err = h.AWSAs(t, ak, sk, "", "s3api", "put-object", "--bucket", b, "--key", "k", "--body", dir+"/f", + "--object-lock-mode", "COMPLIANCE", "--object-lock-retain-until-date", "2999-01-01T00:00:00Z") + mustFail(t, out, err, "s3:PutObjectRetention") +} + // Keys in a DeleteObjects body are forwarded as they are, so "allowed/../x" must // not be authorized as a key under allowed/. func TestDeleteObjectsRejectsDotSegments(t *testing.T) { From 6292386abf6e6764c67ff694987335bbf775e816 Mon Sep 17 00:00:00 2001 From: Suryansh Prajapati <58465650+drk1rd@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:36:49 +0530 Subject: [PATCH 18/18] Add the October 2026 security audit report and server security notes --- docs/install-server.md | 9 ++ docs/security-audit-2026-10.md | 257 +++++++++++++++++++++++++++++++++ 2 files changed, 266 insertions(+) create mode 100644 docs/security-audit-2026-10.md diff --git a/docs/install-server.md b/docs/install-server.md index abdf60e7..78060b09 100644 --- a/docs/install-server.md +++ b/docs/install-server.md @@ -483,6 +483,15 @@ that door open: Console sign-in is throttled after 10 failures per client IP in 5 minutes, but that is no substitute. - Keep the API on loopback behind a proxy where you can, and firewall the ports HomeCloud publishes ([Firewall](#7-firewall)). +- **Outbound requests made for users** (SNS and alarm webhooks, API Gateway `HTTP_PROXY` integrations) + never reach loopback, link-local (including the provider's `169.254.169.254` metadata service), this + host's own addresses or other metadata endpoints, checked when the connection is made. Private LAN + ranges stay reachable because homelabs call their own services; on a multi-user VPS set + `HOMECLOUD_DENY_PRIVATE_TARGETS=1` in the service environment to block RFC 1918 and unique-local + addresses too. +- **`homecloud:CreateBackup` is account-root.** A backup contains `master.key` and everything it + decrypts, so grant that action (and `homecloud:*`) only to administrators, and encrypt backups before + they leave the machine. - Keep the host patched (`unattended-upgrades` on Ubuntu and Debian) and upgrade HomeCloud regularly. ## 14. Troubleshooting diff --git a/docs/security-audit-2026-10.md b/docs/security-audit-2026-10.md new file mode 100644 index 00000000..7d8884a2 --- /dev/null +++ b/docs/security-audit-2026-10.md @@ -0,0 +1,257 @@ +# Security audit, October 2026 + +Pre-launch review of HomeCloud before it is exposed on public VPSes. Scope, in priority order: +authentication (SigV4, sessions, sign-in, unsigned operations), authorization (actions and resources, +`iam:PassRole`, confused deputies), SSRF, container and host isolation, secrets handling and the web +console. Every fix below has a regression test and its own commit on the `security-audit` branch. + +Findings marked *fixed* are closed. *Accepted* findings are left on purpose, with the reasoning. Nothing +in the VM code (`svc/ec2/vm*.go`, `svc/ec2/vm/`) was reviewed or changed. + +## Summary + +| # | Severity | Finding | Status | +| --- | --- | --- | --- | +| 1 | High | AWS handler buffered up to 100 MB of request body before checking the signature | Fixed | +| 2 | High | API Gateway `HTTP_PROXY` integrations were an unrestricted, response-reading proxy into the host | Fixed | +| 3 | High | Inline object views ran scripts with the viewer's session token in the URL | Fixed | +| 4 | High | Native ECS task definitions (and CloudFormation) skipped `iam:PassRole` | Fixed | +| 5 | High | Native EC2 launch, launch templates and Auto Scaling skipped `iam:PassRole` for instance profiles | Fixed | +| 6 | Medium | Sign-in throttle could be bypassed with parallel requests; user existence observable by timing | Fixed | +| 7 | Medium | Open redirect after console sign-in (`/\host`, `//host`) | Fixed | +| 8 | Medium | `iam:PassRole` and target ARNs judged on the caller's spelling, dodging Deny statements | Fixed | +| 9 | Medium | Outbound webhook blocklist missed IPv6-embedded IPv4, metadata and host-local addresses | Fixed | +| 10 | Medium | Lambda packages could decompress to gigabytes at every cold start | Fixed | +| 11 | Medium | Unexpected errors returned Go error text (paths, Docker output) to clients and CloudTrail | Fixed | +| 12 | Medium | Lambda and ECS could run images of any private ECR repository without `ecr:` permissions | Fixed | +| 13 | Low | SNS subscription dead-letter queue needed no `sqs:SendMessage` | Fixed | +| 14 | Low | `?access_token=` accepted access key secrets; temporary credentials could mint new ones | Fixed | +| 15 | Low | SigV4 accepted requests that did not sign `host` / `x-amz-target` | Fixed | +| 16 | Low | Console could be framed (clickjacking); public routes accepted 64 MB bodies | Fixed | +| 17 | Info | `homecloud:CreateBackup` is account-root (the archive contains `master.key`) | Accepted | +| 18 | Info | Sealed secrets use one key with no per-purpose AAD | Accepted | +| 19 | Info | SigV4 requests can be replayed inside the 15 minute skew window | Accepted | +| 20 | Info | Step Functions machines without a role act as a trusted principal | Accepted | +| 21 | Info | RFC 1918 targets stay reachable by default for webhooks and proxy integrations | Accepted | +| 22 | Info | Docker pulls user-chosen images from arbitrary registries | Accepted | +| 23 | High | Website hosting served any key of a bucket whose policy merely mentioned `*` and `s3:GetObject` | Fixed | +| 24 | High | Function / HTTP API responses could replace the sandbox CSP and run script on the console origin | Fixed | +| 25 | Medium | `DeleteObjects` body keys skipped the `..` check | Fixed | +| 26 | Medium | Cognito: no per-account limit on password guesses, none on self sign-up | Fixed | +| 27 | Medium | SigV2 presigned URLs accepted unsigned sub-resources (`?retention`, ...) | Fixed | +| 28 | Low | Object-lock upload headers needed only `s3:PutObject` | Fixed | +| 29 | Low | A bucket named `lambda-code` broke `/lambda-code/` downloads | Fixed | +| 30 | Medium | New Cognito user pools allow open, auto-confirmed self sign-up | Accepted | +| 31 | Low | S3 Block Public Access settings are stored but not enforced | Accepted | +| 32 | Low | Cognito `ForgotPassword`/`ConfirmSignUp` reveal whether a user exists | Accepted | +| 33 | Low | SNS unsubscribe link needs only the subscription ARN | Accepted | +| 34 | Low | Self-signed TLS bootstrap can leave a certificate without its key after a crash | Accepted | +| 35 | Low | `CopyObject` source parsing may differ from MinIO's (not confirmed) | Accepted | + +## Fixed + +### 1. Body buffered before authentication (awsapi) + +`awsapi.Handler` read up to `maxBody` (100 MB) into memory and hashed it before it looked at the +signature. Anyone who could reach the port could hold 100 MB per connection open with an unsigned +request or a signature naming a made-up access key. The handler now checks the signature's freshness and +the access key first, verifies the signature before reading the body whenever the client declared +`X-Amz-Content-Sha256` (and always for presigned URLs), and caps unsigned (public operation) bodies at +1 MB. Native public routes (sign-in, Cognito) use a 64 KB cap instead of 64 MB. Servers also got an idle +timeout and a header size limit. Tests: `awsapi/preauth_test.go`, `svc/iam/login_test.go`. + +### 2. API Gateway HTTP_PROXY SSRF + +`proxyHTTP` used a default `http.Client`, so a caller with `apigateway:*` on an API could point an +integration at loopback services, the host provider's metadata service or the Docker bridge and read +the responses through the public API route. It now uses `core.SafeClient`: the destination is checked +after DNS resolution at dial time (so rebinding and redirects cannot get around it), proxy environment +variables are ignored and redirects are not followed. Test: `svc/lambda/apigw_ssrf_test.go`. + +### 9. Webhook blocklist + +`core.blockedIP` now also covers IPv4-mapped, NAT64 and 6to4 forms, `0.0.0.0/8`, `100.100.100.200`, +`192.0.0.192`, `fd00:ec2::/32` and every address of the host itself (its public address and the Docker +bridge gateways, through which the HomeCloud API and Docker daemon are reachable). See accepted risk 21 +for private ranges. Test: `core/targets_test.go`. + +### 3. Script in inline object views could read the session token + +The console opens an object with `?access_token=` in the URL and the response was sandboxed with +`allow-scripts`, so an uploaded HTML object could read `location.search` and send the viewer's session +away: any user who can write to a bucket could take over an administrator who previewed the object. +Inline views now use `sandbox; default-src 'none'` (no scripts, no outbound loads) and API responses send +`Referrer-Policy: no-referrer`. Hosted websites, function URLs and HTTP APIs keep their scripts; their +URLs carry no token. Test: `server/sandbox_test.go`. + +### 4 and 5. iam:PassRole on native routes + +The PassRole check existed only on the AWS wire API. The native ECS task definition route, the native +EC2 launch route, launch templates carrying an instance profile and Auto Scaling groups launching from +such a template did not check it, so a user with `ecs:RegisterTaskDefinition` + `ecs:RunTask`, or +`ec2:RunInstances`, could run code holding any role that trusts the service and read its credentials. +CloudFormation uses the same native routes. The check now lives in `registerTaskDef` and +`ec2.PassProfile` and is applied on every path. Tests: `svc/ecs/passrole_test.go`, +`svc/autoscaling/passrole_test.go`. + +### 8. Resource spelling + +Policies were evaluated against the resource string as the caller wrote it. `role/dev-/admin` or the bare +name `admin` matched an Allow on `role/dev-*` and missed a Deny on `role/admin`, while IAM resolved +both to the role `admin`. `Principal.Permits` now canonicalizes the resource and IAM resolves role +references to the real ARN for `iam:PassRole`. Deliveries to EventBridge, scheduler, CloudWatch and Step +Functions targets and Secrets Manager rotation functions now reject ARNs of another account or region +instead of delivering to the same-named local resource. Tests: `svc/iam/passrole_spelling_test.go`, +`server/targets_arn_test.go`. + +### 6. Sign-in throttle + +The per-IP throttle counted a failure only after bcrypt had finished, so a burst of parallel guesses all +passed the check. The slot is now reserved before the password check and returned on success. Unknown +users pay the same bcrypt cost as known ones, so response time no longer reveals which user names exist. +Test: `svc/iam/login_test.go`. + +### 7. Console open redirect + +The post-sign-in `next` parameter refused `//host` but accepted `/\host` and `//host`, which browsers +treat as `//host`. It is now rejected on backslashes and control characters and must resolve to the +console's own origin (`console/app/login/page.tsx`). The console has no test runner; the predicate was +checked against the crafted values with Node and `tsc --noEmit` passes. + +### 10. Lambda decompression bombs + +Packages were only checked to be valid zips and then unpacked in memory at every cold start, with a second +copy when loaded into the container. `checkZip` now sums the declared uncompressed sizes against the 250 MB +limit and caps the entry count, and `unzip` never reads more than the budget left. Test: +`svc/lambda/zipbomb_test.go`. + +### 11. Internal error text + +500 responses, SQS batch results and, through the error message, CloudTrail `LookupEvents` carried raw Go +errors (file paths, Docker daemon output, database errors). Clients now get a fixed message; the detail +stays in the server log. Test: `awsapi/preauth_test.go`. + +### 12. ECR image pulls + +Lambda container functions and ECS task definitions pulled from the local registry with no `ecr:` check, +so any user able to create a function or task could run and read another team's private repository. Both +now need `ecr:BatchGetImage` and `ecr:GetDownloadUrlForLayer` on the repository. Test: +`svc/ecs/passrole_test.go`, `core/targets_test.go`. + +### 13. SNS dead-letter queue + +A `RedrivePolicy` only had to name an existing queue; failed deliveries then wrote into it. Naming one now +needs `sqs:SendMessage` on it. Test: `svc/sns/aws_test.go`. + +### 14. Token handling + +`?access_token=` accepted `accessKeyId:secret`; only console session tokens (revocable, expiring) are accepted +in a URL now. `sts:GetSessionToken` refuses temporary credentials, as AWS does, so a stolen session cannot +be renewed forever. Tests: `svc/iam/login_test.go`. + +### 15. SigV4 signed headers + +Requests whose `SignedHeaders` omit `host`, or `x-amz-target` when it is sent, are rejected, as AWS does. +Test: `awsapi/preauth_test.go`. + +### 16. Console headers + +The console is served with `X-Frame-Options: DENY`, `frame-ancestors 'none'`, `object-src 'none'`, +`base-uri`/`form-action 'self'` and `Referrer-Policy: no-referrer`. A script-src policy is not set: the +statically exported Next.js pages rely on inline bootstrap scripts. Test: `web/web_test.go`. + +### 23. Website hosting and bucket policies + +`isPublic` looked for the strings `"*"` and `s3:GetObject` in MinIO's copy of the bucket policy and then +served every key with the server's storage credentials. A policy granting one prefix, or with a Deny or a +condition, exposed the whole bucket. Each requested key (index, redirect probe, error document) is now +evaluated against the bucket policy as an anonymous caller. Test: `svc/s3/website_security_test.go`. + +### 24. Sandbox headers overridden by functions + +`respond` copied the function's headers over the sandbox `Content-Security-Policy`, and the HTTP_PROXY relay +copied upstream headers, so a function author could serve script on the console origin to an administrator +who opened its URL. The sandbox and `nosniff` headers are re-applied when the response is written. Test: +`server/sandbox_test.go`. + +### 25-29. S3 and Cognito hardening + +`DeleteObjects` body keys get the `..`/length checks URL keys already had (25). Password and SRP sign-in count +attempts per account across all addresses, and each pool bounds self sign-ups per window (26). SigV2 URLs +carrying a sub-resource outside SigV2's signed set are refused (27). Retention and legal-hold upload headers +need `s3:PutObjectRetention` / `s3:PutObjectLegalHold` (28). `/lambda-code/` and `/_s3/` are never claimed by +anonymous S3 routing (29). Tests: `svc/s3/website_security_test.go`, `svc/cognito/signup_limit_test.go`. + +## Accepted risks + +**30. Open Cognito sign-up by default.** New pools auto-confirm self sign-ups so the emulator works out of the +box, like AWS's own developer setups. Anyone who knows a client ID can register and obtain tokens that verify +for the pool; an API Gateway JWT authorizer with no audience therefore accepts any of them. On a public server +turn off self sign-up (`SelfSignUp=false`) or auto-confirm for pools that guard real APIs, and set the +authorizer's audience. + +**31. Block Public Access is cosmetic.** The four settings are stored and returned, but public bucket policies +still take effect. Do not rely on them; review bucket policies instead. + +**32. Cognito existence signals.** Recovery and confirmation calls distinguish unknown users. Sign-in itself +does not, and the per-account and per-address limits bound enumeration. + +**33. SNS unsubscribe links.** As in AWS, the link is authorized by knowing the subscription ARN. + +**34. TLS bootstrap.** `--tls-self-signed` writes the certificate before the key; a crash in between needs +the `tls/` directory removed. No security impact. + +**35. CopyObject source parsing.** HomeCloud and MinIO may decode `+` and `%3F` in `x-amz-copy-source` +differently. The difference could not be confirmed in MinIO's code from this repository. Exposure is limited +to principals who can already read some source object, and the cross-bucket test in `aws_security_test.go` +still passes. + +**17. `homecloud:CreateBackup` is account-root.** `GET /api/v1/system/backup` streams an archive containing +`master.key`, the state file (sealed secrets) and volumes. It needs `homecloud:CreateBackup`, which only +administrator policies carry. Requiring the root user would break administrators who are not root, so the +install guide now says to treat the action as root-equivalent and to encrypt backups off the machine. + +**18. One sealing key, no AAD.** `secrets.Seal` uses AES-256-GCM with random 96-bit nonces under one key for +every purpose. Swapping ciphertexts between fields needs write access to the state file, which already means +host compromise (and the key file next to it). Changing the format needs a migration; tracked for a later release. + +**19. Replay.** As in AWS, a captured SigV4 request can be replayed within 15 minutes (presigned URLs until +they expire). The mitigation is TLS, which the install guide requires for anything off a trusted network. + +**20. Role-less Step Functions machines.** They run as a trusted principal; each task's permission was checked +against the creator when the definition was saved, and changing the definition re-checks it against the editor. +Anyone with `states:StartExecution` can start it. AWS requires a role, which HomeCloud deliberately relaxes for +small setups. Use a role (and `iam:PassRole`) for machines that matter. + +**21. Private ranges stay reachable.** Webhooks and proxy integrations to RFC 1918 and unique-local addresses +work by default because the typical user calls their own LAN and VPC. On a shared VPS set +`HOMECLOUD_DENY_PRIVATE_TARGETS=1`. + +**22. Image pulls.** The Docker daemon pulls user-chosen image names from any registry. Registries are +contacted over HTTPS (plain HTTP only for loopback), the response is not returned to the caller, and the +daemon already runs with the privileges of the administrator who set it up. + +## Reviewed, not an issue + +- **Sessions and CSRF.** The console authenticates with `Authorization: Bearer` from local storage; there are + no cookies, so there is no session fixation and no CSRF on native routes. Sessions are stored hashed, expire, + and end on password change or user deletion. CORS `*` is safe for the same reason. +- **Passwords and keys.** bcrypt for console passwords; access key secrets stored as SHA-256 plus a sealed + copy, compared in constant time; `master.key` from `crypto/rand`, mode 0600 in a 0700 directory. +- **CloudTrail records** hold method, path (no query), action, resource, source and error only: no request + parameters or response elements, so no secret values or presigned URLs. +- **Path traversal.** Function, layer and snapshot names are validated before any path join; zip extraction + rejects `..`, never creates symlinks and stages through memory; backup restore rejects `..` and absolute + names; S3 keys never touch the host file system. +- **Container isolation.** Workloads get named volumes only: no bind mounts, `--privileged`, Docker socket, + host network/PID or devices; ECS volumes and mount points are rejected. +- **CloudFormation `TemplateURL`** only extracts a bucket and key and reads through the caller's own + credentials; the URL host is never dialed. +- **Console XSS.** React escapes all rendered names, tags and log lines; the only `dangerouslySetInnerHTML` + is the chart theme built from static configuration. + +## Reported for the VM worker (not touched) + +None. The VM code was excluded from this audit and has not been reviewed; it needs its own pass before +VM-backed instances are offered on public servers. `svc/ec2/imds.go` (the metadata service, outside the VM +files) is guarded by a per-process key held by the helper container and looks correct.