Unverified Commit 28d36389 authored by Minseong Choi's avatar Minseong Choi 💬
Browse files

fix(nano): stop following redirects from upstream Yggdrasil roots 

authHTTPClient kept net/http's default redirect policy, so a configured
third-party root that answered hasJoined with a 3xx made this host fetch
whatever URL it named, up to ten hops. That is a blind SSRF into
anything the host can reach, and it includes the multiplexer's own
listener: a root that redirects back to /session/minecraft/hasJoined
re-enters the handler, which queries Mojang and every source again and
gets redirected again, until the outer 5s client timeout fires. With a
50ms Mojang stub, one login produced 86 nested handler calls and 86
Mojang requests from this host's egress IP. The loopback default does
not help, because the redirect target is resolved from this host.

Return the 3xx as the response instead. resolveHasJoined already skips
any non-200 answer and closes its body, so a redirecting source is now
treated like one that is down, and the next source gets its turn. The
same probe now makes one handler call and one Mojang request.
Neither Mojang's nor LittleSkin's hasJoined redirects.

The new subtest puts a redirecting root ahead of an honest one and
checks that the redirect target is never contacted and the honest
source's player is returned. The pre-fix handler fails it.
parent 07bafebf

internal/api/handlers_hasjoined.go

+8 −1
Changes for internal/api/handlers_hasjoined.go: 8 added lines, 1 removed line.
Original line number Diff line number Diff line
@@ -43,7 +43,14 @@ var felisAuthNS = uuid.NewSHA1(uuid.NameSpaceURL, []byte("nano.felis.lolicon.bes 
// against a hung source; the resolver moves on to the next source on any failure.

// ponytail: one shared client, sequential priority scan — a third-party login costs one

// wasted Mojang round-trip; add parallel fan-out only if login latency bites.

var authHTTPClient = &http.Client{Timeout: 5 * time.Second}

var authHTTPClient = &http.Client{

	Timeout: 5 * time.Second,

	// A redirect is not a hasJoined answer. Following one would let a configured root point

	// this host at any URL it can reach — this listener included, where each hop re-runs the

	// whole source scan inside the same login's timeout. The 3xx is returned as-is and the

	// resolver skips that source like any other non-200.

	CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse },

}



// AuthSource is one upstream Yggdrasil root in the multiplexer's priority list (config

// order = priority). URL is the full hasJoined endpoint the query string is appended to.

internal/api/handlers_hasjoined_test.go

+32 −0
Changes for internal/api/handlers_hasjoined_test.go: 32 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -7,6 +7,7 @@ import ( 
	"net/http/httptest"

	"path"

	"strings"

	"sync/atomic"

	"testing"



	"github.com/google/uuid"
@@ -188,6 +189,37 @@ func TestHasJoined(t *testing.T) { 
		}

	})



	// A root that answers with a redirect is skipped, not followed: following it lets that

	// root aim this host at arbitrary URLs, including its own hasJoined route, which re-enters

	// the scan and multiplies the upstream traffic one login causes.

	t.Run("redirecting source is skipped, not followed", func(t *testing.T) {

		stubMojangNames(t)

		var followed atomic.Bool

		target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

			followed.Store(true)

			_ = json.NewEncoder(w).Encode(map[string]any{"id": notchMojangID, "name": "Notch"})

		}))

		t.Cleanup(target.Close)

		redirector := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

			http.Redirect(w, r, target.URL+"/hasJoined?"+r.URL.RawQuery, http.StatusFound)

		}))

		t.Cleanup(redirector.Close)

		honest := fakeYgg(t, "0123456789abcdef0123456789abcdef", "Steve0")

		api := newTestAPI(newFakeRepo(), newFakeCluster())

		api.AuthSources = []AuthSource{

			{Tag: "evil", Prefix: "EV", URL: redirector.URL},

			{Tag: "littleskin", Prefix: "LS", URL: honest.URL},

		}



		w := getHasJoined(api.InternalHandler(), "Steve0", "abc")

		if followed.Load() {

			t.Fatal("the redirect was followed")

		}

		if w.Code != http.StatusOK || profileOf(t, w).Name != "Steve0" {

			t.Fatalf("code = %d body = %q, want the next source's player", w.Code, w.Body.String())

		}

	})



	// The reused bar gate: a barred CANONICAL UUID is rejected at the resolver, so a

	// reclaimed squatter stays out even on a consumer with no limbo plugin. Keyed on the

	// dashed canonical (post-rewrite), the same form Repo.ReclaimUsername stores.