fix: [processor/signingprocessor] sign the log body whatever its type

.chloggen/signingprocessor-sign-non-string-body.yaml

@@ -0,0 +1,24 @@
+change_type: breaking
+
+component: processor/signing
+
+note: Include the log record body in the signed payload whatever its type. Previously only a string body was signed, so a record with a structured body and a record with no body produced identical canonical bytes and shared a signature.
+
+issues: [50911]
+
+subtext: |
+  The body is now routed through the same `valueToInterface` conversion as
+  attributes, which handles every `pcommon.Value` type, enforces the existing
+  nesting depth cap, and preserves the UTF-8 validation that previously guarded
+  the string-only path. An unset body is still omitted rather than encoded as
+  null, so records without a body are unaffected.
+
+  This changes the canonical form for any record carrying a non-string body, so
+  signatures produced by earlier versions over such records will not reproduce.
+
+  Two new failure modes follow from the body no longer being discarded: a record
+  whose structured body pushes the canonical payload past the 2 MiB limit, or
+  whose body nests deeper than 128 levels, now fails to sign where it previously
+  signed with the body silently omitted.
+
+change_logs: []

processor/signingprocessor/coverage_test.go

@@ -20,6 +20,7 @@ import (
 	"math/big"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
@@ -585,6 +586,11 @@ func TestSerializeLogRecordNonStringBody(t *testing.T) {
 	if len(b) == 0 {
 		t.Error("expected non-empty serialized payload")
 	}
+	// A non-empty payload is not enough: the body has to be in it. Without this
+	// the record would sign identically to one carrying no body at all.
+	if !strings.Contains(string(b), `"body":99`) {
+		t.Errorf("non-string body missing from signed payload: %s", b)
+	}
 }
 
 // ---------------------------------------------------------------------------

processor/signingprocessor/processor.go

@@ -212,10 +212,10 @@ func (p *signingProcessor) serializeLogRecord(lr plog.LogRecord) ([]byte, error)
 		data["event_name"] = lr.EventName()
 	}
 
-	if lr.Body().Type() == pcommon.ValueTypeStr {
-		body := lr.Body().Str()
-		if !utf8.ValidString(body) {
-			return nil, errors.New("log record body contains invalid UTF-8")
+	if lr.Body().Type() != pcommon.ValueTypeEmpty {
+		body, err := p.valueToInterface(lr.Body(), 0)
+		if err != nil {
+			return nil, fmt.Errorf("log record body: %w", err)
 		}
 		data["body"] = body
 	}
@@ -284,13 +284,13 @@ func (*signingProcessor) marshalJCS(v any) ([]byte, error) {
 
 func (p *signingProcessor) valueToInterface(v pcommon.Value, depth int) (any, error) {
 	if depth > jsonMaxDepth {
-		return nil, fmt.Errorf("attribute value exceeds nesting depth limit (%d)", jsonMaxDepth)
+		return nil, fmt.Errorf("value exceeds nesting depth limit (%d)", jsonMaxDepth)
 	}
 	switch v.Type() {
 	case pcommon.ValueTypeStr:
 		s := v.Str()
 		if !utf8.ValidString(s) {
-			return nil, errors.New("attribute string value contains invalid UTF-8")
+			return nil, errors.New("string value contains invalid UTF-8")
 		}
 		return s, nil
 	case pcommon.ValueTypeInt:

processor/signingprocessor/processor_body_test.go

@@ -0,0 +1,152 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package signingprocessor
+
+import (
+	"strings"
+	"testing"
+
+	"go.opentelemetry.io/collector/pdata/pcommon"
+	"go.opentelemetry.io/collector/pdata/plog"
+)
+
+// serializeBody builds a record with the given body and returns its canonical
+// signed payload.
+func serializeBody(t *testing.T, set func(plog.LogRecord)) string {
+	t.Helper()
+	p := &signingProcessor{config: &Config{}}
+	lr := plog.NewLogRecord()
+	lr.SetEventName("e")
+	set(lr)
+	b, err := p.serializeLogRecord(lr)
+	if err != nil {
+		t.Fatalf("serializeLogRecord: %v", err)
+	}
+	return string(b)
+}
+
+// TestBodyIsSignedForEveryType verifies that the log body is part of the canonical
+// payload whatever its type. Before this was fixed only a string body was included,
+// so a record with a structured body and a record with no body at all produced
+// identical bytes and therefore shared a signature.
+func TestBodyIsSignedForEveryType(t *testing.T) {
+	empty := serializeBody(t, func(plog.LogRecord) {})
+
+	tests := []struct {
+		name string
+		set  func(plog.LogRecord)
+		want string
+	}{
+		{
+			name: "string body",
+			set:  func(lr plog.LogRecord) { lr.Body().SetStr("hello") },
+			want: `"body":"hello"`,
+		},
+		{
+			name: "int body",
+			set:  func(lr plog.LogRecord) { lr.Body().SetInt(42) },
+			want: `"body":42`,
+		},
+		{
+			name: "double body",
+			set:  func(lr plog.LogRecord) { lr.Body().SetDouble(1.5) },
+			want: `"body":1.5`,
+		},
+		{
+			name: "bool body",
+			set:  func(lr plog.LogRecord) { lr.Body().SetBool(true) },
+			want: `"body":true`,
+		},
+		{
+			name: "bytes body",
+			set:  func(lr plog.LogRecord) { lr.Body().SetEmptyBytes().Append(0xDE, 0xAD) },
+			want: `"body":"3q0="`,
+		},
+		{
+			name: "slice body",
+			set:  func(lr plog.LogRecord) { lr.Body().SetEmptySlice().AppendEmpty().SetStr("x") },
+			want: `"body":["x"]`,
+		},
+		{
+			name: "map body",
+			set:  func(lr plog.LogRecord) { lr.Body().SetEmptyMap().PutStr("action", "delete-all") },
+			want: `"body":{"action":"delete-all"}`,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got := serializeBody(t, tc.set)
+			if got == empty {
+				t.Fatalf("body is absent from the signed payload: got %s, same as a record with no body", got)
+			}
+			if !strings.Contains(got, tc.want) {
+				t.Errorf("payload does not carry the body:\n  got  %s\n  want it to contain %s", got, tc.want)
+			}
+		})
+	}
+}
+
+// TestBodyDistinguishesDistinctRecords is the property that matters: two records
+// differing only in their body must not share a signature.
+func TestBodyDistinguishesDistinctRecords(t *testing.T) {
+	a := serializeBody(t, func(lr plog.LogRecord) { lr.Body().SetEmptyMap().PutStr("action", "read") })
+	b := serializeBody(t, func(lr plog.LogRecord) { lr.Body().SetEmptyMap().PutStr("action", "delete-all") })
+	if a == b {
+		t.Errorf("records with different map bodies canonicalize identically: %s", a)
+	}
+}
+
+// TestEmptyBodyIsOmitted confirms an unset body stays out of the payload rather
+// than being encoded as a null, so this change does not alter records without one.
+func TestEmptyBodyIsOmitted(t *testing.T) {
+	p := &signingProcessor{config: &Config{}}
+	lr := plog.NewLogRecord()
+	lr.SetEventName("e")
+	if lr.Body().Type() != pcommon.ValueTypeEmpty {
+		t.Fatalf("expected an unset body to be ValueTypeEmpty, got %v", lr.Body().Type())
+	}
+	b, err := p.serializeLogRecord(lr)
+	if err != nil {
+		t.Fatalf("serializeLogRecord: %v", err)
+	}
+	if strings.Contains(string(b), `"body"`) {
+		t.Errorf("unset body should be omitted, got %s", b)
+	}
+}
+
+// TestBodyInvalidUTF8IsRejected confirms the UTF-8 validation that guarded the
+// string-only path still applies now that the body goes through valueToInterface.
+func TestBodyInvalidUTF8IsRejected(t *testing.T) {
+	p := &signingProcessor{config: &Config{}}
+	lr := plog.NewLogRecord()
+	lr.Body().SetStr(string([]byte{0xff, 0xfe}))
+	_, err := p.serializeLogRecord(lr)
+	if err == nil {
+		t.Fatal("expected an error for a body containing invalid UTF-8")
+	}
+	if !strings.Contains(err.Error(), "UTF-8") {
+		t.Errorf("expected a UTF-8 error, got %v", err)
+	}
+}
+
+// TestBodyNestingDepthIsCapped pins the bound the body inherits from
+// valueToInterface. Without it a deeply nested body would reach encoding/json
+// and overflow the stack, which is a fatal error the collector cannot recover
+// from. The entry depth is the assertion a refactor is most likely to get wrong.
+func TestBodyNestingDepthIsCapped(t *testing.T) {
+	p := &signingProcessor{config: &Config{}}
+	lr := plog.NewLogRecord()
+	m := lr.Body().SetEmptyMap()
+	for range jsonMaxDepth + 1 {
+		m = m.PutEmptyMap("n")
+	}
+	_, err := p.serializeLogRecord(lr)
+	if err == nil {
+		t.Fatal("expected a depth-limit error for a deeply nested body")
+	}
+	if !strings.Contains(err.Error(), "nesting depth limit") {
+		t.Errorf("expected a nesting depth error, got %v", err)
+	}
+}

processor/signingprocessor/processor_sign_verify_test.go

@@ -72,8 +72,8 @@ func verifyRecord(t *testing.T, lr plog.LogRecord, pubKey *rsa.PublicKey) {
 	if lr.EventName() != "" {
 		data["event_name"] = lr.EventName()
 	}
-	if lr.Body().Type() == pcommon.ValueTypeStr {
-		data["body"] = lr.Body().Str()
+	if lr.Body().Type() != pcommon.ValueTypeEmpty {
+		data["body"] = rawValue(lr.Body())
 	}
 	if lr.Timestamp() != 0 {
 		data["timestamp"] = lr.Timestamp().AsTime().UnixNano()
@@ -287,3 +287,67 @@ func TestSignVerifyEventName(t *testing.T) {
 		t.Logf("🔍 tampered EventName correctly invalidates signature: %v", err)
 	}
 }
+
+// rawValue converts a pcommon.Value to the shape the signed payload carries.
+// It is written out independently of the processor's own valueToInterface so
+// this helper stays a genuine re-derivation rather than a mirror of the code
+// under test.
+func rawValue(v pcommon.Value) any {
+	switch v.Type() {
+	case pcommon.ValueTypeStr:
+		return v.Str()
+	case pcommon.ValueTypeInt:
+		return v.Int()
+	case pcommon.ValueTypeDouble:
+		return v.Double()
+	case pcommon.ValueTypeBool:
+		return v.Bool()
+	case pcommon.ValueTypeBytes:
+		return base64.StdEncoding.EncodeToString(v.Bytes().AsRaw())
+	case pcommon.ValueTypeSlice:
+		out := make([]any, 0, v.Slice().Len())
+		for i := 0; i < v.Slice().Len(); i++ {
+			out = append(out, rawValue(v.Slice().At(i)))
+		}
+		return out
+	case pcommon.ValueTypeMap:
+		out := make(map[string]any)
+		v.Map().Range(func(k string, mv pcommon.Value) bool {
+			out[k] = rawValue(mv)
+			return true
+		})
+		return out
+	default:
+		return nil
+	}
+}
+
+// TestSignVerifyStructuredBody drives a non-string body through the full
+// sign-then-verify path. It exists so verifyRecord's independent re-derivation
+// of the signed payload stays honest: every other test here feeds it a string
+// body, so a helper that only handled strings would go unnoticed.
+func TestSignVerifyStructuredBody(t *testing.T) {
+	prov := newTestProvider(t)
+	p := &signingProcessor{
+		config:       &Config{Algorithm: "RS256", CertificateRef: CertificateRefFingerprint},
+		provider:     prov,
+		hashFunc:     func() hash.Hash { return crypto.SHA256.New() },
+		jwaAlgorithm: "RS256",
+		certRef:      "sha256:test",
+	}
+
+	lr := plog.NewLogRecord()
+	body := lr.Body().SetEmptyMap()
+	body.PutStr("action", "delete-all")
+	body.PutInt("count", 3)
+	body.PutBool("dry_run", false)
+	body.PutEmptyMap("who").PutStr("id", "u8472")
+	lr.SetTimestamp(pcommon.Timestamp(1714041600000000000))
+	lr.Attributes().PutStr("audit.action", "DELETE")
+
+	if err := p.processLogRecord(lr); err != nil {
+		t.Fatalf("processLogRecord: %v", err)
+	}
+
+	verifyRecord(t, lr, &prov.key.PublicKey)
+}