opentelemetry-collector icon indicating copy to clipboard operation
opentelemetry-collector copied to clipboard

[otlpreceiver] OpenTelemetry Collector accepts non-utf8 data

Open povilasv opened this issue 1 year ago • 1 comments

Describe the bug

Because opentelemetry-collector uses deprecated gogo protobuf library OpenTelemetry Collector is able to accept invalid protobuf data.

In protocol buffers string fields cannot contain utf-8 data:

https://protobuf.dev/programming-guides/proto3/#scalar

string A string must always contain UTF-8 encoded or 7-bit ASCII text, and cannot be longer than 232.

The issue here that spans containing invalid data get processed and batched up together with valid data, then it fails on the ingestion side since Ingestion actually follows the Protocol buffer spec. The batching up with good data makes it so some valid data gets lost

Example issue: https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/35723

My proposal is to basically deny bad data coming to Collector. We could do that by basically switching either to official, non deprecated protocol buffer library or find alternative non-deprecated that has support for utf8 checks. Alternatively we could modify otlp receivers and add checks for utf8 ourselves.

Relevant issue: https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/35723

Steps to reproduce

func main() {
	// Create a new trace with non-UTF-8 data
	traces := ptrace.NewTraces()
	rs := traces.ResourceSpans().AppendEmpty()
	ss := rs.ScopeSpans().AppendEmpty()
	span := ss.Spans().AppendEmpty()

	span.SetName("example-span")
	span.Attributes().PutStr("valid_utf8", "hello")
	span.Attributes().PutStr("invalid_utf8", string([]byte{0xff, 0xfe, 0xfd}))

	// Set up gRPC client
	conn, err := grpc.Dial("localhost:4317", grpc.WithTransportCredentials(insecure.NewCredentials()))
	if err != nil {
		log.Fatalf("Failed to connect to gRPC server: %v", err)
	}
	defer conn.Close()

	client := ptraceotlp.NewGRPCClient(conn)

	// Send the trace to the collector
	exportRequest := ptraceotlp.NewExportRequestFromTraces(traces)
	_, err = client.Export(context.Background(), exportRequest)
	if err != nil {
		log.Fatalf("Failed to send trace to collector: %v", err)
	}

	fmt.Println("Span sent successfully")
}

What did you expect to see?

I expect the request to fail.

What did you see instead?

Request passed

What version did you use?

v0.111.0

What config did you use?

Environment

Additional context

povilasv avatar Oct 15 '24 08:10 povilasv