Information Disclosure / Authentication Bypass

PoC (pre-fix behavior): - An unauthenticated client can enumerate and learn about private OpenFeature flags by posting to the OFREP evaluation endpoint and reading flag metadata returned in the response. - attacker can identify existence and properties of private flags (e.g., public: false) via responses from /ofrep/v1/evaluate/flags/{flagKey}. Prereqs: - Grafana instance with OpenFeature (OFPREP) enabled, containing both public and private flags. - No authentication provided in the request. - Access to the OFREP endpoint at /ofrep/v1/evaluate/flags/{flagKey}. Reproduction steps (pre-fix behavior): 1) Unauthenticated HTTP POST to https://<grafana-host>/ofrep/v1/evaluate/flags/secretflag with empty JSON body: curl -s -X POST -H 'Content-Type: application/json' -d '{}' https://<grafana-host>/ofrep/v1/evaluate/flags/secretflag 2) Observe a 200 OK response containing the flag evaluation, including Metadata.public or details about the secret flag (e.g., Metadata: {"public": false}) and Key: "secretflag". 3) Iterate over a list of known private flag keys to enumerate which exist and their public/private status, enabling an information-disclosure path. Expected outcome after fix (PoC steps show difference): - The unauthenticated response should not reveal private flag details. Instead, for non-public flags, the endpoint should respond with not-found (404) or sanitized content, and only public flags should be visible. Minimal PoC script (Python) to illustrate pre-fix exposure (adapt BASE URL as needed): import requests import json BASE = "http://grafana.example.org" # replace with target HEADERS = {"Content-Type": "application/json"} flag_keys = ["publicFlag","secretFlag","privateFlag","anotherFlag"] for k in flag_keys: url = f"{BASE}/ofrep/v1/evaluate/flags/{k}" resp = requests.post(url, json={}, headers=HEADERS, timeout=5) print(k, resp.status_code) try: data = resp.json() print(json.dumps(data, indent=2)) except Exception: print(resp.text) Notes: - In this vulnerability scenario, an attacker leverages unauthenticated requests to discover which flags exist and which are private by inspecting the returned data. The fix ensures unauthenticated responses only expose public flags or not-found indications, reducing information disclosure and preventing auth-bypass via flag enumeration.

diff --git a/devenv/frontend-service/goff-flags.yaml b/devenv/frontend-service/goff-flags.yaml index 2fc55b9acb225..6e689d11be84e 100644 --- a/devenv/frontend-service/goff-flags.yaml +++ b/devenv/frontend-service/goff-flags.yaml @@ -4,6 +4,8 @@ react19: disabled: false defaultRule: variation: enabled + metadata: + public: "true" reportRenderBinding: variations: @@ -11,6 +13,8 @@ reportRenderBinding: disabled: false defaultRule: variation: enabled + metadata: + public: "true" compiledBootScript: variations: @@ -18,6 +22,8 @@ compiledBootScript: disabled: false defaultRule: variation: disabled + metadata: + public: "true" grafana.assetSriChecks: variations: @@ -25,6 +31,8 @@ grafana.assetSriChecks: disabled: false defaultRule: variation: disabled + metadata: + public: "true" grafana.meticulousAIMode: variations: @@ -33,6 +41,8 @@ grafana.meticulousAIMode: on-dev-env: 'on-dev-env' defaultRule: variation: off + metadata: + public: "true" frontendService.reducedBootDataAPI: variations: @@ -40,6 +50,8 @@ frontendService.reducedBootDataAPI: disabled: false defaultRule: variation: disabled + metadata: + public: "true" grafana.frontendLegacyAPIHandling: variations: @@ -48,3 +60,5 @@ grafana.frontendLegacyAPIHandling: block: 'block' defaultRule: variation: off + metadata: + public: "true" diff --git a/pkg/registry/apis/ofrep/helpers_test.go b/pkg/registry/apis/ofrep/helpers_test.go new file mode 100644 index 0000000000000..bbbe0dca55018 --- /dev/null +++ b/pkg/registry/apis/ofrep/helpers_test.go @@ -0,0 +1,101 @@ +package ofrep + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "sync" + "testing" + + "github.com/gorilla/mux" + "github.com/grafana/authlib/types" + "github.com/open-feature/go-sdk/openfeature" + "github.com/open-feature/go-sdk/openfeature/memprovider" + "github.com/stretchr/testify/require" + goffmodel "github.com/thomaspoignant/go-feature-flag/cmd/relayproxy/model" + + "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/setting" +) + +var openfeatureTestMutex sync.Mutex + +// setupOpenFeatureFlag sets the global OpenFeature default provider to a static +// in-memory provider serving flag=value, and restores the noop provider on cleanup. +func setupOpenFeatureFlag(t *testing.T, flag string, value bool) { + t.Helper() + openfeatureTestMutex.Lock() + + provider, err := featuremgmt.CreateStaticProviderWithStandardFlags(map[string]memprovider.InMemoryFlag{ + flag: setting.NewInMemoryFlag(flag, value), + }) + require.NoError(t, err) + + err = openfeature.SetProviderAndWait(provider) + require.NoError(t, err) + + t.Cleanup(func() { + _ = openfeature.SetProviderAndWait(openfeature.NoopProvider{}) + openfeatureTestMutex.Unlock() + }) +} + +func newSingleEvalBuilder(t *testing.T, metadata map[string]any) *APIBuilder { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + body, err := json.Marshal(goffmodel.OFREPEvaluateSuccessResponse{ + Key: "flag", Value: true, Reason: "STATIC", Metadata: metadata, + }) + require.NoError(t, err) + _, _ = w.Write(body) + })) + t.Cleanup(srv.Close) + return newTestBuilder(t, srv.URL) +} + +func newBulkEvalBuilder(t *testing.T, flags []goffmodel.OFREPFlagBulkEvaluateSuccessResponse, status int) *APIBuilder { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(status) + if status != http.StatusOK { + return + } + body, err := json.Marshal(goffmodel.OFREPBulkEvaluateSuccessResponse{Flags: flags}) + require.NoError(t, err) + _, _ = w.Write(body) + })) + t.Cleanup(srv.Close) + return newTestBuilder(t, srv.URL) +} + +func newTestBuilder(t *testing.T, upstreamURL string) *APIBuilder { + t.Helper() + u, err := url.Parse(upstreamURL) + require.NoError(t, err) + return &APIBuilder{ + providerType: setting.OFREPProviderType, + url: u, + logger: log.NewNopLogger(), + transport: &http.Transport{}, + } +} + +func newUnauthReq(target string, vars map[string]string) *http.Request { + req := httptest.NewRequest(http.MethodPost, target, bytes.NewBufferString(`{}`)) + req = mux.SetURLVars(req, vars) + ctx := types.WithAuthInfo(req.Context(), &identity.StaticRequester{Type: types.TypeUnauthenticated}) + return req.WithContext(ctx) +} + +func flagKeys(flags []goffmodel.OFREPFlagBulkEvaluateSuccessResponse) []string { + keys := make([]string, 0, len(flags)) + for _, f := range flags { + keys = append(keys, f.Key) + } + return keys +} diff --git a/pkg/registry/apis/ofrep/http_routes.go b/pkg/registry/apis/ofrep/http_routes.go index 9c3d25d072623..2491bbeaf5475 100644 --- a/pkg/registry/apis/ofrep/http_routes.go +++ b/pkg/registry/apis/ofrep/http_routes.go @@ -85,14 +85,6 @@ func (b *APIBuilder) rootOneFlagHandler(w http.ResponseWriter, r *http.Request) isAuthedReq := b.isAuthenticatedRequest(r) span.SetAttributes(attribute.Bool("authenticated", isAuthedReq)) - if !isAuthedReq && !isPublicFlag(flagKey) { - _ = tracing.Errorf(span, "unauthorized to evaluate flag: %s", flagKey) - span.SetAttributes(semconv.HTTPStatusCode(http.StatusUnauthorized)) - b.logger.Error("Unauthorized to evaluate flag", "flagKey", flagKey) - http.Error(w, "unauthorized to evaluate flag", http.StatusUnauthorized) - return - } - if b.providerType == setting.FeaturesServiceProviderType || b.providerType == setting.OFREPProviderType { evalCtx, err := b.readEvalContext(w, r) if err != nil { @@ -151,7 +143,7 @@ func (b *APIBuilder) rootAllFlagsHandler(w http.ResponseWriter, r *http.Request) return } - b.evalAllFlagsStatic(ctx, isAuthedReq, w) + b.evalAllFlagsStatic(ctx, w) } // validateNamespaceIfPresent checks if the namespace in the evaluation context matches the authenticated user's diff --git a/pkg/registry/apis/ofrep/http_routes_test.go b/pkg/registry/apis/ofrep/http_routes_test.go index c6a1fb1063ac7..7a6e2b77843bf 100644 --- a/pkg/registry/apis/ofrep/http_routes_test.go +++ b/pkg/registry/apis/ofrep/http_routes_test.go @@ -3,6 +3,7 @@ package ofrep import ( "bytes" "context" + "encoding/json" "io" "net/http" "net/http/httptest" @@ -12,6 +13,7 @@ import ( "github.com/grafana/authlib/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + goffmodel "github.com/thomaspoignant/go-feature-flag/cmd/relayproxy/model" "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/log" @@ -131,25 +133,62 @@ func TestRootOneFlagHandler_MissingFlagKey(t *testing.T) { assert.Equal(t, http.StatusBadRequest, w.Code) } -func TestRootOneFlagHandler_UnauthedNonPublicFlag(t *testing.T) { - b := &APIBuilder{ - providerType: setting.OFREPProviderType, - logger: log.NewNopLogger(), +func TestOneFlagHandler_Unauth(t *testing.T) { + routes := []struct { + name string + call func(b *APIBuilder, w http.ResponseWriter, r *http.Request) + }{ + {"root", func(b *APIBuilder, w http.ResponseWriter, r *http.Request) { b.rootOneFlagHandler(w, r) }}, + {"namespaced", func(b *APIBuilder, w http.ResponseWriter, r *http.Request) { b.oneFlagHandler(w, r) }}, + } + tests := []struct { + name string + metadata map[string]any + wantStatus int + }{ + {"public flag returns 200", map[string]any{"public": true}, http.StatusOK}, + {"private flag returns 404, indistinguishable from a genuinely missing flag", nil, http.StatusNotFound}, } - req := httptest.NewRequest(http.MethodPost, "/ofrep/v1/evaluate/flags/secretflag", bytes.NewBufferString(`{}`)) - req = mux.SetURLVars(req, map[string]string{"flagKey": "secretflag"}) + for _, rt := range routes { + for _, tt := range tests { + t.Run(rt.name+": "+tt.name, func(t *testing.T) { + b := newSingleEvalBuilder(t, tt.metadata) + w := httptest.NewRecorder() + r := newUnauthReq("/ofrep/v1/evaluate/flags/brandnewflag", map[string]string{"flagKey": "brandnewflag", "namespace": ""}) + rt.call(b, w, r) + assert.Equal(t, tt.wantStatus, w.Code) + }) + } + } +} - // Unauthenticated requester - requester := &identity.StaticRequester{ - Type: types.TypeUnauthenticated, +func TestAllFlagsHandler_Unauth(t *testing.T) { + routes := []struct { + name string + call func(b *APIBuilder, w http.ResponseWriter, r *http.Request) + }{ + {"root", func(b *APIBuilder, w http.ResponseWriter, r *http.Request) { b.rootAllFlagsHandler(w, r) }}, + {"namespaced", func(b *APIBuilder, w http.ResponseWriter, r *http.Request) { b.allFlagsHandler(w, r) }}, + } + upstreamFlags := []goffmodel.OFREPFlagBulkEvaluateSuccessResponse{ + {OFREPEvaluateSuccessResponse: goffmodel.OFREPEvaluateSuccessResponse{Key: "publicFlag", Metadata: map[string]any{"public": true}}}, + {OFREPEvaluateSuccessResponse: goffmodel.OFREPEvaluateSuccessResponse{Key: "privateFlag", Metadata: map[string]any{"public": false}}}, } - ctx := types.WithAuthInfo(req.Context(), requester) - req = req.WithContext(ctx) - w := httptest.NewRecorder() - b.rootOneFlagHandler(w, req) - assert.Equal(t, http.StatusUnauthorized, w.Code) + for _, rt := range routes { + t.Run(rt.name, func(t *testing.T) { + b := newBulkEvalBuilder(t, upstreamFlags, http.StatusOK) + w := httptest.NewRecorder() + r := newUnauthReq("/ofrep/v1/evaluate/flags", map[string]string{"namespace": ""}) + rt.call(b, w, r) + + require.Equal(t, http.StatusOK, w.Code) + var result goffmodel.OFREPBulkEvaluateSuccessResponse + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &result)) + assert.Equal(t, []string{"publicFlag"}, flagKeys(result.Flags)) + }) + } } func TestRootAllFlagsHandler_NamespaceMismatch(t *testing.T) { diff --git a/pkg/registry/apis/ofrep/proxy.go b/pkg/registry/apis/ofrep/proxy.go index 28a26090fd413..4f45887c0e826 100644 --- a/pkg/registry/apis/ofrep/proxy.go +++ b/pkg/registry/apis/ofrep/proxy.go @@ -32,34 +32,37 @@ func (b *APIBuilder) proxyAllFlagReq(ctx context.Context, isAuthedUser bool, nam } proxy.ModifyResponse = func(resp *http.Response) error { - if resp.StatusCode == http.StatusOK && !isAuthedUser { - var result goffmodel.OFREPBulkEvaluateSuccessResponse - if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { - return err - } - _ = resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return nil + } - var filteredFlags []goffmodel.OFREPFlagBulkEvaluateSuccessResponse - for _, f := range result.Flags { - if isPublicFlag(f.Key) { - filteredFlags = append(filteredFlags, f) - } - } + // Unauth is always filtered to public flags. Authed is filtered only when the flag is on. + if isAuthedUser && !bulkFlagEvalFilteringEnabled(ctx) { + return nil + } - result.Flags = filteredFlags - newBodyBytes, err := json.Marshal(result) - if err != nil { - b.logger.Error("Failed to encode filtered result", "error", err) - return err + var result goffmodel.OFREPBulkEvaluateSuccessResponse + if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { + b.logger.Error("Failed to decode bulk eval response", "error", err) + return err + } + _ = resp.Body.Close() + + filteredFlags := make([]goffmodel.OFREPFlagBulkEvaluateSuccessResponse, 0, len(result.Flags)) + for _, f := range result.Flags { + if isPublic(f.Metadata) { + filteredFlags = append(filteredFlags, f) } + } - // Replace the body - resp.Body = io.NopCloser(bytes.NewReader(newBodyBytes)) - resp.ContentLength = int64(len(newBodyBytes)) - resp.Header.Set("Content-Length", strconv.Itoa(len(newBodyBytes))) - resp.Header.Set("Content-Type", "application/json") + result.Flags = filteredFlags + newBodyBytes, err := json.Marshal(result) + if err != nil { + b.logger.Error("Failed to encode filtered result", "error", err) + return err } + rewriteResponse(resp, resp.StatusCode, newBodyBytes, "application/json") return nil } @@ -81,9 +84,44 @@ func (b *APIBuilder) proxyFlagReq(ctx context.Context, flagKey string, isAuthedU } proxy.ModifyResponse = func(resp *http.Response) error { - if resp.StatusCode == http.StatusOK && !isAuthedUser && !isPublicFlag(flagKey) { - writeResponse(http.StatusUnauthorized, struct{}{}, b.logger, w) + // Unauth may only see public flags. Checked here since metadata is only known after eval. + if resp.StatusCode != http.StatusOK || isAuthedUser { + return nil + } + + body, err := io.ReadAll(resp.Body) + if err != nil { + b.logger.Error("Failed to read flag eval response", "key", flagKey, "error", err) + return err } + _ = resp.Body.Close() + + var result goffmodel.OFREPEvaluateSuccessResponse + if err := json.Unmarshal(body, &result); err != nil { + b.logger.Error("Failed to decode flag eval response", "key", flagKey, "error", err) + return err + } + + if isPublic(result.Metadata) { + resp.Body = io.NopCloser(bytes.NewReader(body)) + return nil + } + + // Not public -> respond as if the flag doesn't exist, so an unauthed + // caller can't use the 404-vs-401 distinction to probe which private + // flags exist. + b.logger.Debug("Unauthed request for non-public flag, responding as not-found", "key", flagKey) + notFoundBody, err := json.Marshal(goffmodel.OFREPEvaluateErrorResponse{ + OFREPCommonErrorResponse: goffmodel.OFREPCommonErrorResponse{ + ErrorCode: "FLAG_NOT_FOUND", + ErrorDetails: fmt.Sprintf("Flag %q was not found", flagKey), + }, + Key: flagKey, + }) + if err != nil { + return err + } + rewriteResponse(resp, http.StatusNotFound, notFoundBody, "application/json") return nil } @@ -117,3 +155,14 @@ func namespaceUserAgent(namespace string) string { } return "features-grafana-app/" + namespace } + +// rewriteResponse swaps a proxied response for a new one, so the reverse proxy +// forwards our content instead of the original upstream response. +func rewriteResponse(resp *http.Response, statusCode int, body []byte, contentType string) { + resp.StatusCode = statusCode + resp.Status = fmt.Sprintf("%d %s", statusCode, http.StatusText(statusCode)) + resp.Body = io.NopCloser(bytes.NewReader(body)) + resp.ContentLength = int64(len(body)) + resp.Header.Set("Content-Length", strconv.Itoa(len(body))) + resp.Header.Set("Content-Type", contentType) +} diff --git a/pkg/registry/apis/ofrep/proxy_test.go b/pkg/registry/apis/ofrep/proxy_test.go index aac1629ec4d56..0e9530313aa0e 100644 --- a/pkg/registry/apis/ofrep/proxy_test.go +++ b/pkg/registry/apis/ofrep/proxy_test.go @@ -1,18 +1,18 @@ package ofrep import ( + "encoding/json" "io" "net/http" "net/http/httptest" - "net/url" "strings" "testing" "github.com/stretchr/testify/assert" "github ... [truncated]