Skip to content

Commit 38f42e7

Browse files
authored
[exporter/opensearchexporter] Validate attribute values in dynamic index names (#49362)
#### Description Validates attribute values before they are substituted into a `%{placeholder}` index name in the dynamic index resolver to mitigate #49225. Rejecting leading-dot (`.`) and (`..`) keeps a value from resolving to a system index (e.g. `.kibana`) or a different index via a `..` segment. A value that is invalid to OpenSearch (uppercase, spaces, forbidden characters) remains untouched and surfaces as an index-time (server-side) error rather than being silently rerouted to the configured fallback. The leading-dot escalation requires the placeholder to lead the index name (e.g. `traces_index: "%{tenant}"`); a static prefix like `logs-%{tenant}` already keeps the result out of the system namespace. **What this does not cover.** A well-formed but unauthorized value still resolves. With `traces_index: "%{tenant}"`, an attacker setting `tenant=team-b` still produces the `team-b` index, because `team-b` is a valid index segment. Character validation can't distinguish an authorized tenant from an unauthorized one; cross-tenant isolation needs tenant allowlisting or an immutable prefix. #### Link to tracking issue - related to #49225. #### Testing - `go test ./...` in `exporter/opensearchexporter/`. - New `index_resolver_test.go` cases cover path traversal (`../../system`), `..`, leading dot, values substituted verbatim (`/`, `\`, space, uppercase), and fall-through from an invalid higher-precedence attribute to a valid lower-precedence one. - Verified end-to-end against OpenSearch 2.17.1 and 3.7.0 (shell output below). <details> <summary>Full repro steps</summary> Two OpenSearch nodes (security plugin off for plain HTTP): ```yaml # docker-compose.yaml services: opensearch2: image: opensearchproject/opensearch:2.17.1 environment: [discovery.type=single-node, "OPENSEARCH_JAVA_OPTS=-Xms512m -Xmx512m", DISABLE_SECURITY_PLUGIN=true, DISABLE_INSTALL_DEMO_CONFIG=true] ports: ["9201:9200"] opensearch3: image: opensearchproject/opensearch:3.7.0 environment: [discovery.type=single-node, "OPENSEARCH_JAVA_OPTS=-Xms512m -Xmx512m", DISABLE_SECURITY_PLUGIN=true, DISABLE_INSTALL_DEMO_CONFIG=true] ports: ["9202:9200"] ``` ```sh docker-compose up -d ``` Build a collector with this branch's exporter. `make otelcontribcol` from the repo root is the official path (its `genotelcontribcol` step adds a local `replace` for every module). For faster single-exporter iteration, a minimal module wiring the OTLP receiver, debug exporter, and this exporter with a `replace` to the local tree works too. Collector config. `traces_index: "%{tenant}"` makes the index name fully attacker-controlled, which is what exposes the leading-dot system-index case: ```yaml receivers: otlp: protocols: http: endpoint: 0.0.0.0:4328 exporters: debug: { verbosity: basic } opensearch: http: endpoint: http://localhost:9202 # or :9201 for the 2.x node traces_index: "%{tenant}" traces_index_fallback: "rejected-traces" mapping: mode: ss4o service: pipelines: traces: receivers: [otlp] exporters: [debug, opensearch] ``` Send a span carrying the `tenant` value under test (swap the value per case): ```json { "resourceSpans": [ { "resource": { "attributes": [ { "key": "service.name", "value": { "stringValue": "victim" } }, { "key": "tenant", "value": { "stringValue": "../../system" } } ] }, "scopeSpans": [ { "scope": { "name": "repro" }, "spans": [ { "traceId": "5b8efff798038103d269b633813fc60c", "spanId": "eee19b7ec3c1b174", "name": "span", "kind": 1, "startTimeUnixNano": "1700000000000000000", "endTimeUnixNano": "1700000000000000010" } ] } ] } ] } ``` Send the span (swap the `tenant` value per case), then list indices: ```sh curl -s -X POST http://localhost:4328/v1/traces -H 'Content-Type: application/json' -d @span.json curl -s "http://localhost:9202/_cat/indices?h=index,docs.count" ``` Sending `team-a`, `team-b`, `.kibana_x`, `../../system` in turn, without the fix: ```text tenant=team-a HTTP 200 tenant=team-b HTTP 200 tenant=.kibana_x HTTP 200 tenant=../../system HTTP 500 -- indices -- .kibana_x 1 # leading dot created a system-namespace index team-a 1 team-b 1 # cross-tenant write ``` `../../system` returns HTTP 500; the exporter logs `invalid_index_name_exception` (the `/` is a forbidden index char) and drops the span. The same four with the fix: ```text tenant=team-a HTTP 200 tenant=team-b HTTP 200 tenant=.kibana_x HTTP 200 tenant=../../system HTTP 200 -- indices -- rejected-traces 2 # .kibana_x and ../../system both routed here team-a 1 team-b 1 # still resolves: cross-tenant gap (see #49225) ``` </details> #### Documentation - README dynamic-indexing section documents the value restriction and fall-through behavior. - `.chloggen/opensearchexporter-sanitize-index-placeholders.yaml` added. --- - [x] I, a human, wrote this pull request description myself.
1 parent bb9c3e4 commit 38f42e7

4 files changed

Lines changed: 116 additions & 3 deletions

File tree

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. receiver/filelog)
7+
component: exporter/opensearch
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Reject attribute values substituted into dynamic index names when they would redirect writes to a system index or another index via a `.` prefix or `..` segment.
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [49225]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext: |
19+
A value substituted into a `%{placeholder}` index name that is empty, starts with `.`, or contains
20+
`..` is skipped in favor of the next attribute in the precedence order and then the configured
21+
fallback, matching the existing missing-key behavior. Values that are merely invalid to OpenSearch
22+
(for example uppercase) are still substituted as-is and rejected by OpenSearch at index time, so that
23+
failure remains visible rather than being silently rerouted to the fallback index.
24+
25+
# If your change doesn't affect end users or the exported elements of any package,
26+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
27+
# Optional: The change log or logs in which this entry should be included.
28+
# e.g. '[user]' or '[user, api]'
29+
# Include 'user' if the change is relevant to end users.
30+
# Include 'api' if there is a change to a library API.
31+
# Default: '[user]'
32+
change_logs: [user]

exporter/opensearchexporter/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ The OpenSearch exporter supports dynamic index names for both logs and traces us
5151
- The value is looked up from item attributes (log/span), scope attributes, and resource attributes (in that precedence order)
5252
- If the key is missing, the fallback value is used
5353
- Generated index names must adhere to [OpenSearch index naming restrictions](https://docs.opensearch.org/latest/api-reference/index-apis/create-index/#index-naming-restrictions)
54+
- A substituted attribute value that is empty, starts with `.`, or contains `..` is skipped as if the key were missing, so the next attribute in the precedence order (then the fallback) is used. This keeps a value from resolving to a system index (e.g. `.kibana`) or a different index via a `..` segment. Other values are substituted as-is; a value OpenSearch rejects (uppercase, spaces, forbidden characters) surfaces as an indexing error rather than being silently rerouted. The configured fallback value is trusted and is not validated.
5455

5556
**Time Suffix Format:**
5657

exporter/opensearchexporter/index_resolver.go

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,13 +103,13 @@ func (r *indexResolver) resolveIndexName(indexPattern, fallback string, itemAttr
103103
}
104104
indexName := r.placeholderPattern.ReplaceAllStringFunc(indexPattern, func(match string) string {
105105
key := r.placeholderPattern.FindStringSubmatch(match)[1]
106-
if val, ok := itemAttributes[key]; ok && val != "" {
106+
if val, ok := itemAttributes[key]; ok && isSafeIndexSegment(val) {
107107
return val
108108
}
109-
if val, ok := scopeAttributes[key]; ok && val != "" {
109+
if val, ok := scopeAttributes[key]; ok && isSafeIndexSegment(val) {
110110
return val
111111
}
112-
if val, ok := resourceAttributes[key]; ok && val != "" {
112+
if val, ok := resourceAttributes[key]; ok && isSafeIndexSegment(val) {
113113
return val
114114
}
115115
if fallback != "" {
@@ -129,6 +129,18 @@ func (*indexResolver) calculateTimeSuffix(timeFormat string, timestamp time.Time
129129
return ""
130130
}
131131

132+
// isSafeIndexSegment reports whether an attribute value can be substituted into
133+
// an index name. It rejects only values that would target a different index than
134+
// intended: empty values, a leading dot (system indices, e.g. `.kibana`), and any
135+
// `..` sequence. Values that are invalid to OpenSearch (uppercase, spaces,
136+
// forbidden characters) are left to OpenSearch to reject at index time so the
137+
// failure stays visible rather than silently rerouting to the fallback index.
138+
func isSafeIndexSegment(val string) bool {
139+
return val != "" &&
140+
!strings.HasPrefix(val, ".") &&
141+
!strings.Contains(val, "..")
142+
}
143+
132144
// convertGoTimeFormat converts a Java-style date format to Go's time format
133145
func convertGoTimeFormat(format string) string {
134146
// Support yyyy, yy, MM, dd, HH, mm, ss -> 2006, 06, 01, 02, 15, 04, 05

exporter/opensearchexporter/index_resolver_test.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,74 @@ func TestIndexResolver_ResolveIndex(t *testing.T) {
138138
itemAttrs: map[string]string{},
139139
expected: "test-1.0.0",
140140
},
141+
{
142+
name: "path traversal falls back to configured fallback",
143+
indexPattern: "otel-%{tenant}",
144+
fallback: "default-tenant",
145+
timeFormat: "",
146+
resourceAttrs: map[string]string{"tenant": "../../system"},
147+
expected: "otel-default-tenant",
148+
},
149+
{
150+
name: "double dot falls back to unknown",
151+
indexPattern: "otel-%{tenant}",
152+
fallback: "",
153+
timeFormat: "",
154+
resourceAttrs: map[string]string{"tenant": ".."},
155+
expected: "otel-unknown",
156+
},
157+
{
158+
name: "leading dot targets system index and is rejected",
159+
indexPattern: "otel-%{tenant}",
160+
fallback: "fallback",
161+
timeFormat: "",
162+
resourceAttrs: map[string]string{"tenant": ".kibana"},
163+
expected: "otel-fallback",
164+
},
165+
{
166+
// OpenSearch-invalid: substituted verbatim and left for OpenSearch
167+
// to reject at index time so the failure stays visible.
168+
name: "forbidden slash is substituted verbatim",
169+
indexPattern: "otel-%{tenant}",
170+
fallback: "fallback",
171+
timeFormat: "",
172+
resourceAttrs: map[string]string{"tenant": "my/tenant"},
173+
expected: "otel-my/tenant",
174+
},
175+
{
176+
name: "backslash is substituted verbatim",
177+
indexPattern: "otel-%{tenant}",
178+
fallback: "fallback",
179+
timeFormat: "",
180+
resourceAttrs: map[string]string{"tenant": "my\\tenant"},
181+
expected: "otel-my\\tenant",
182+
},
183+
{
184+
name: "space is substituted verbatim",
185+
indexPattern: "otel-%{tenant}",
186+
fallback: "fallback",
187+
timeFormat: "",
188+
resourceAttrs: map[string]string{"tenant": "my tenant"},
189+
expected: "otel-my tenant",
190+
},
191+
{
192+
name: "uppercase is substituted verbatim",
193+
indexPattern: "otel-%{tenant}",
194+
fallback: "fallback",
195+
timeFormat: "",
196+
resourceAttrs: map[string]string{"tenant": "MyTenant"},
197+
expected: "otel-MyTenant",
198+
},
199+
{
200+
name: "invalid item value falls through to valid scope value",
201+
indexPattern: "otel-%{tenant}",
202+
fallback: "fallback",
203+
timeFormat: "",
204+
resourceAttrs: map[string]string{"tenant": "resource-tenant"},
205+
scopeAttrs: map[string]string{"tenant": "scope-tenant"},
206+
itemAttrs: map[string]string{"tenant": "../../system"},
207+
expected: "otel-scope-tenant",
208+
},
141209
}
142210

143211
for _, tt := range tests {

0 commit comments

Comments
 (0)