diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/cbor.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/cbor.go index 4d2f770645e..643d54d5aed 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/cbor.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/cbor.go @@ -118,15 +118,19 @@ func (s *serializer) Encode(obj runtime.Object, w io.Writer) error { } func (s *serializer) encode(mode modes.EncMode, obj runtime.Object, w io.Writer) error { - if _, err := w.Write(selfDescribedCBOR); err != nil { - return err - } - var v interface{} = obj if u, ok := obj.(runtime.Unstructured); ok { v = u.UnstructuredContent() } + if err := modes.RejectCustomMarshalers(v); err != nil { + return err + } + + if _, err := w.Write(selfDescribedCBOR); err != nil { + return err + } + return mode.MarshalTo(v, w) } @@ -237,6 +241,8 @@ func (s *serializer) unmarshal(data []byte, into interface{}) (strict, lax error } }() into = &content + } else if err := modes.RejectCustomMarshalers(into); err != nil { + return nil, err } if !s.options.strict { diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/cbor_test.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/cbor_test.go index db0d7fc4adc..8b8025ea8eb 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/cbor_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/cbor_test.go @@ -207,6 +207,42 @@ func TestEncode(t *testing.T) { } }, }, + { + name: "unsupported marshaler", + in: &textMarshalerObject{}, + assertOnWriter: func() (io.Writer, func(*testing.T)) { + var b bytes.Buffer + return &b, func(t *testing.T) { + if b.Len() != 0 { + t.Errorf("expected no bytes to be written, got %d", b.Len()) + } + } + }, + assertOnError: func(t *testing.T, err error) { + if want := "unable to serialize *cbor.textMarshalerObject: *cbor.textMarshalerObject implements encoding.TextMarshaler without corresponding cbor interface"; err == nil || err.Error() != want { + t.Errorf("expected error %q, got: %v", want, err) + } + }, + }, + { + name: "unsupported marshaler within unstructured content", + in: &unstructured.Unstructured{ + Object: map[string]interface{}{"": textMarshalerObject{}}, + }, + assertOnWriter: func() (io.Writer, func(*testing.T)) { + var b bytes.Buffer + return &b, func(t *testing.T) { + if b.Len() != 0 { + t.Errorf("expected no bytes to be written, got %d", b.Len()) + } + } + }, + assertOnError: func(t *testing.T, err error) { + if want := "unable to serialize map[string]interface {}: cbor.textMarshalerObject implements encoding.TextMarshaler without corresponding cbor interface"; err == nil || err.Error() != want { + t.Errorf("expected error %q, got: %v", want, err) + } + }, + }, } { t.Run(tc.name, func(t *testing.T) { s := NewSerializer(nil, nil) @@ -625,6 +661,19 @@ func TestDecode(t *testing.T) { } }, }, + { + name: "into unsupported marshaler", + data: []byte("\xa0"), + into: &textMarshalerObject{}, + metaFactory: stubMetaFactory{gvk: &schema.GroupVersionKind{}}, + typer: stubTyper{gvks: []schema.GroupVersionKind{{Version: "v", Kind: "k"}}}, + expectedGVK: &schema.GroupVersionKind{Version: "v", Kind: "k"}, + assertOnError: func(t *testing.T, err error) { + if want := "unable to serialize *cbor.textMarshalerObject: *cbor.textMarshalerObject implements encoding.TextMarshaler without corresponding cbor interface"; err == nil || err.Error() != want { + t.Errorf("expected error %q, got: %v", want, err) + } + }, + }, } { t.Run(tc.name, func(t *testing.T) { s := newSerializer(tc.metaFactory, tc.creater, tc.typer, tc.options...) @@ -643,6 +692,20 @@ func TestDecode(t *testing.T) { } } +type textMarshalerObject struct{} + +func (p textMarshalerObject) GetObjectKind() schema.ObjectKind { + return schema.EmptyObjectKind +} + +func (textMarshalerObject) DeepCopyObject() runtime.Object { + panic("unimplemented") +} + +func (textMarshalerObject) MarshalText() ([]byte, error) { + return nil, nil +} + func TestMetaFactoryInterpret(t *testing.T) { mf := &defaultMetaFactory{} _, err := mf.Interpret(nil) diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/custom.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/custom.go new file mode 100644 index 00000000000..858529e958a --- /dev/null +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/custom.go @@ -0,0 +1,422 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package modes + +import ( + "encoding" + "encoding/json" + "errors" + "fmt" + "reflect" + "sync" + + "github.com/fxamacker/cbor/v2" +) + +// Returns a non-nil error if and only if the argument's type (or one of its component types, for +// composite types) implements json.Marshaler or encoding.TextMarshaler without also implementing +// cbor.Marshaler and likewise for the respective Unmarshaler interfaces. +// +// This is a temporary, graduation-blocking restriction and will be removed in favor of automatic +// transcoding between CBOR and JSON/text for these types. This restriction allows CBOR to be +// exercised for in-tree and unstructured types while mitigating the risk of mangling out-of-tree +// types in client programs. +func RejectCustomMarshalers(v interface{}) error { + if v == nil { + return nil + } + rv := reflect.ValueOf(v) + if err := marshalerCache.getChecker(rv.Type()).check(rv, maxDepth); err != nil { + return fmt.Errorf("unable to serialize %T: %w", v, err) + } + if err := unmarshalerCache.getChecker(rv.Type()).check(rv, maxDepth); err != nil { + return fmt.Errorf("unable to serialize %T: %w", v, err) + } + return nil +} + +// Recursion depth is limited as a basic mitigation against cyclic objects. Objects created by the +// decoder shouldn't be able to contain cycles, but practically any object can be passed to the +// encoder. +var errMaxDepthExceeded = errors.New("object depth exceeds limit (possible cycle?)") + +// The JSON encoder begins detecting cycles after depth 1000. Use a generous limit here, knowing +// that it can might deeply nested acyclic objects. The limit will be removed along with the rest of +// this mechanism. +const maxDepth = 2048 + +var marshalerCache = checkers{ + cborInterface: reflect.TypeFor[cbor.Marshaler](), + nonCBORInterfaces: []reflect.Type{ + reflect.TypeFor[json.Marshaler](), + reflect.TypeFor[encoding.TextMarshaler](), + }, +} + +var unmarshalerCache = checkers{ + cborInterface: reflect.TypeFor[cbor.Unmarshaler](), + nonCBORInterfaces: []reflect.Type{ + reflect.TypeFor[json.Unmarshaler](), + reflect.TypeFor[encoding.TextUnmarshaler](), + }, + assumeAddressableValues: true, +} + +// checker wraps a function for dynamically checking a value of a specific type for custom JSON +// behaviors not matched by a custom CBOR behavior. +type checker struct { + // check returns a non-nil error if the given value might be marshalled to or from CBOR + // using the default behavior for its kind, but marshalled to or from JSON using custom + // behavior. + check func(rv reflect.Value, depth int) error + + // safe returns true if all values of this type are safe from mismatched custom marshalers. + safe func() bool +} + +// TODO: stale +// Having a single addressable checker for comparisons lets us prune and collapse parts of the +// object traversal that are statically known to be safe. Depending on the type, it may be +// unnecessary to inspect each value of that type. For example, no value of the built-in type bool +// can implement json.Marshaler (a named type whose underlying type is bool could, but it is a +// distinct type from bool). +var noop = checker{ + safe: func() bool { + return true + }, + check: func(rv reflect.Value, depth int) error { + return nil + }, +} + +type checkers struct { + m sync.Map // reflect.Type => *checker + + cborInterface reflect.Type + nonCBORInterfaces []reflect.Type + + assumeAddressableValues bool +} + +func (cache *checkers) getChecker(rt reflect.Type) checker { + if ptr, ok := cache.m.Load(rt); ok { + return *ptr.(*checker) + } + + return cache.getCheckerInternal(rt, nil) +} + +// linked list node representing the path from a composite type to an element type +type path struct { + Type reflect.Type + Parent *path +} + +func (p path) cyclic(rt reflect.Type) bool { + for ancestor := &p; ancestor != nil; ancestor = ancestor.Parent { + if ancestor.Type == rt { + return true + } + } + return false +} + +func (cache *checkers) getCheckerInternal(rt reflect.Type, parent *path) (c checker) { + // Store a placeholder cache entry first to handle cyclic types. + var wg sync.WaitGroup + wg.Add(1) + defer wg.Done() + c = checker{ + safe: func() bool { + wg.Wait() + return c.safe() + }, + check: func(rv reflect.Value, depth int) error { + wg.Wait() + return c.check(rv, depth) + }, + } + if actual, loaded := cache.m.LoadOrStore(rt, &c); loaded { + // Someone else stored an entry for this type, use it. + return *actual.(*checker) + } + + // Take a nonreflective path for the unstructured container types. They're common and + // usually nested inside one another. + switch rt { + case reflect.TypeFor[map[string]interface{}](), reflect.TypeFor[[]interface{}](): + return checker{ + safe: func() bool { + return false + }, + check: func(rv reflect.Value, depth int) error { + return checkUnstructuredValue(cache, rv.Interface(), depth) + }, + } + } + + // It's possible that one of the relevant interfaces is implemented on a type with a pointer + // receiver, but that a particular value of that type is not addressable. For example: + // + // func (Foo) MarshalText() ([]byte, error) { ... } + // func (*Foo) MarshalCBOR() ([]byte, error) { ... } + // + // Both methods are in the method set of *Foo, but the method set of Foo contains only + // MarshalText. + // + // Both the unmarshaler and marshaler checks assume that methods implementing a JSON or text + // interface with a pointer receiver are always accessible. Only the unmarshaler check + // assumes that CBOR methods with pointer receivers are accessible. + + if rt.Implements(cache.cborInterface) { + return noop + } + for _, unsafe := range cache.nonCBORInterfaces { + if rt.Implements(unsafe) { + err := fmt.Errorf("%v implements %v without corresponding cbor interface", rt, unsafe) + return checker{ + safe: func() bool { + return false + }, + check: func(reflect.Value, int) error { + return err + }, + } + } + } + + if cache.assumeAddressableValues && reflect.PointerTo(rt).Implements(cache.cborInterface) { + return noop + } + for _, unsafe := range cache.nonCBORInterfaces { + if reflect.PointerTo(rt).Implements(unsafe) { + err := fmt.Errorf("%v implements %v without corresponding cbor interface", reflect.PointerTo(rt), unsafe) + return checker{ + safe: func() bool { + return false + }, + check: func(reflect.Value, int) error { + return err + }, + } + } + } + + self := &path{Type: rt, Parent: parent} + + switch rt.Kind() { + case reflect.Array: + ce := cache.getCheckerInternal(rt.Elem(), self) + rtlen := rt.Len() + if rtlen == 0 || (!self.cyclic(rt.Elem()) && ce.safe()) { + return noop + } + return checker{ + safe: func() bool { + return false + }, + check: func(rv reflect.Value, depth int) error { + if depth <= 0 { + return errMaxDepthExceeded + } + for i := 0; i < rtlen; i++ { + if err := ce.check(rv.Index(i), depth-1); err != nil { + return err + } + } + return nil + }, + } + + case reflect.Interface: + // All interface values have to be checked because their dynamic type might + // implement one of the interesting interfaces or be composed of another type that + // does. + return checker{ + safe: func() bool { + return false + }, + check: func(rv reflect.Value, depth int) error { + if rv.IsNil() { + return nil + } + // Unpacking interfaces must count against recursion depth, + // consider this cycle: + // > var i interface{} + // > var p *interface{} = &i + // > i = p + // > rv := reflect.ValueOf(i) + // > for { + // > rv = rv.Elem() + // > } + if depth <= 0 { + return errMaxDepthExceeded + } + rv = rv.Elem() + return cache.getChecker(rv.Type()).check(rv, depth-1) + }, + } + + case reflect.Map: + rtk := rt.Key() + ck := cache.getCheckerInternal(rtk, self) + rte := rt.Elem() + ce := cache.getCheckerInternal(rte, self) + if !self.cyclic(rtk) && !self.cyclic(rte) && ck.safe() && ce.safe() { + return noop + } + return checker{ + safe: func() bool { + return false + }, + check: func(rv reflect.Value, depth int) error { + if depth <= 0 { + return errMaxDepthExceeded + } + iter := rv.MapRange() + rvk := reflect.New(rtk).Elem() + rve := reflect.New(rte).Elem() + for iter.Next() { + rvk.SetIterKey(iter) + if err := ck.check(rvk, depth-1); err != nil { + return err + } + rve.SetIterValue(iter) + if err := ce.check(rve, depth-1); err != nil { + return err + } + } + return nil + }, + } + + case reflect.Pointer: + ce := cache.getCheckerInternal(rt.Elem(), self) + if !self.cyclic(rt.Elem()) && ce.safe() { + return noop + } + return checker{ + safe: func() bool { + return false + }, + check: func(rv reflect.Value, depth int) error { + if rv.IsNil() { + return nil + } + if depth <= 0 { + return errMaxDepthExceeded + } + return ce.check(rv.Elem(), depth-1) + }, + } + + case reflect.Slice: + ce := cache.getCheckerInternal(rt.Elem(), self) + if !self.cyclic(rt.Elem()) && ce.safe() { + return noop + } + return checker{ + safe: func() bool { + return false + }, + check: func(rv reflect.Value, depth int) error { + if depth <= 0 { + return errMaxDepthExceeded + } + for i := 0; i < rv.Len(); i++ { + if err := ce.check(rv.Index(i), depth-1); err != nil { + return err + } + } + return nil + }, + } + + case reflect.Struct: + type field struct { + Index int + Checker checker + } + var fields []field + for i := 0; i < rt.NumField(); i++ { + f := rt.Field(i) + cf := cache.getCheckerInternal(f.Type, self) + if !self.cyclic(f.Type) && cf.safe() { + continue + } + fields = append(fields, field{Index: i, Checker: cf}) + } + if len(fields) == 0 { + return noop + } + return checker{ + safe: func() bool { + return false + }, + check: func(rv reflect.Value, depth int) error { + if depth <= 0 { + return errMaxDepthExceeded + } + for _, fi := range fields { + if err := fi.Checker.check(rv.Field(fi.Index), depth-1); err != nil { + return err + } + } + return nil + }, + } + + default: + // Not a serializable composite type (funcs and channels are composite types but are + // rejected by JSON and CBOR serialization). + return noop + + } +} + +func checkUnstructuredValue(cache *checkers, v interface{}, depth int) error { + switch v := v.(type) { + case nil, bool, int64, float64, string: + return nil + case []interface{}: + if depth <= 0 { + return errMaxDepthExceeded + } + for _, element := range v { + if err := checkUnstructuredValue(cache, element, depth-1); err != nil { + return err + } + } + return nil + case map[string]interface{}: + if depth <= 0 { + return errMaxDepthExceeded + } + for _, element := range v { + if err := checkUnstructuredValue(cache, element, depth-1); err != nil { + return err + } + } + return nil + default: + // Unmarshaling an unstructured doesn't use other dynamic types, but nothing + // prevents inserting values with arbitrary dynamic types into unstructured content, + // as long as they can be marshalled. + rv := reflect.ValueOf(v) + return cache.getChecker(rv.Type()).check(rv, depth) + } +} diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/custom_test.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/custom_test.go new file mode 100644 index 00000000000..80d55537e03 --- /dev/null +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/custom_test.go @@ -0,0 +1,169 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package modes_test + +import ( + "encoding" + "encoding/json" + "testing" + + "k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes" + + "github.com/fxamacker/cbor/v2" +) + +type TextUnmarshalerPointer struct{} + +func (*TextUnmarshalerPointer) UnmarshalText([]byte) error { return nil } + +type Struct[F any] struct { + Field F +} + +type StructCBORMarshalerValue[F any] struct { + Field F +} + +func (StructCBORMarshalerValue[F]) MarshalCBOR() ([]byte, error) { return nil, nil } + +type StructCBORMarshalerPointer[F any] struct { + Field F +} + +func (*StructCBORMarshalerPointer[F]) MarshalCBOR() ([]byte, error) { return nil, nil } + +type StructCBORUnmarshalerPointer[F any] struct { + Field F +} + +func (*StructCBORUnmarshalerPointer[F]) UnmarshalCBOR([]byte) error { return nil } + +type StructCBORUnmarshalerValueWithEmbeddedJSONMarshaler struct { + json.Marshaler +} + +func (StructCBORUnmarshalerValueWithEmbeddedJSONMarshaler) MarshalCBOR() ([]byte, error) { + return nil, nil +} + +type SafeCyclicTypeA struct { + Bs []SafeCyclicTypeB +} + +type SafeCyclicTypeB struct { + As []SafeCyclicTypeA +} + +type UnsafeCyclicTypeA struct { + Bs []UnsafeCyclicTypeB +} + +type UnsafeCyclicTypeB struct { + As []UnsafeCyclicTypeA + + json.Marshaler +} + +func TestCheckUnsupportedMarshalers(t *testing.T) { + t.Run("accepted", func(t *testing.T) { + for _, v := range []interface{}{ + // Unstructured types. + nil, + false, + 0, + 0.0, + "", + []interface{}{nil, false, 0, 0.0, "", []interface{}{}, map[string]interface{}{}}, + map[string]interface{}{ + "nil": nil, + "false": false, + "0": 0, + "0.0": 0.0, + `""`: "", + "[]interface{}{}": []interface{}{}, + "map[string]interface{}{}": map[string]interface{}{}, + }, + // Zero-length array is statically OK, even though its element type is + // json.Marshaler. + [0]json.Marshaler{}, + [3]bool{}, + // Has to be dynamically checked. + [1]interface{}{}, + map[int]interface{}{0: nil}, + Struct[string]{}, + StructCBORMarshalerValue[json.Marshaler]{}, + &StructCBORMarshalerValue[json.Marshaler]{}, + &StructCBORMarshalerPointer[json.Marshaler]{}, + new(string), + map[cbor.Marshaler]cbor.Marshaler{}, + make([]string, 10), + []Struct[interface{}]{{Field: true}}, + &Struct[StructCBORUnmarshalerPointer[json.Unmarshaler]]{}, + // Checked dynamically because the dynamic type of an interface, even + // encoding.TextMarshaler or json.Marshaler, might also implement + // cbor.Marshaler. Accepted because there are no map entries. + map[encoding.TextMarshaler]struct{}{}, + map[string]json.Marshaler{}, + // Methods of json.Marshaler and cbor.Marshaler are both promoted to the + // struct's method set. + struct { + json.Marshaler + StructCBORMarshalerValue[int] + }{}, + + // Embedded field's json.Marshaler implementation is promoted, but the + // containing struct implements cbor.Marshaler. + StructCBORUnmarshalerValueWithEmbeddedJSONMarshaler{}, + + SafeCyclicTypeA{}, + SafeCyclicTypeB{}, + } { + if err := modes.RejectCustomMarshalers(v); err != nil { + t.Errorf("%#v: unexpected non-nil error: %v", v, err) + } + } + }) + + t.Run("rejected", func(t *testing.T) { + for _, v := range []interface{}{ + Struct[json.Marshaler]{}, + StructCBORMarshalerValue[json.Unmarshaler]{}, + Struct[interface{}]{Field: [1]json.Unmarshaler{}}, + // cbor.Marshaler implemented with pointer receiver on non-pointer type. + StructCBORMarshalerPointer[json.Marshaler]{}, + [1]json.Marshaler{}, + [1]interface{}{[1]json.Marshaler{}}, + map[int]interface{}{0: [1]json.Marshaler{}}, + []interface{}{[1]json.Marshaler{}}, + map[string]interface{}{"": [1]json.Marshaler{}}, + []Struct[interface{}]{{Field: [1]json.Marshaler{}}}, + map[encoding.TextMarshaler]struct{}{[1]encoding.TextMarshaler{}[0]: {}}, + map[string]json.Marshaler{"": [1]json.Marshaler{}[0]}, + &Struct[StructCBORMarshalerPointer[json.Marshaler]]{}, + Struct[TextUnmarshalerPointer]{}, + struct { + json.Marshaler + }{}, + UnsafeCyclicTypeA{Bs: []UnsafeCyclicTypeB{{}}}, + UnsafeCyclicTypeB{}, + } { + if err := modes.RejectCustomMarshalers(v); err == nil { + t.Errorf("%#v: unexpected nil error", v) + } + } + }) +}