diff --git a/staging/src/k8s.io/apiserver/pkg/storage/continue.go b/staging/src/k8s.io/apiserver/pkg/storage/continue.go index ad3bb6b2da2..fcd95af9b3d 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/continue.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/continue.go @@ -19,11 +19,19 @@ package storage import ( "encoding/base64" "encoding/json" + "errors" "fmt" "path" "strings" ) +var ( + ErrInvalidStartRV = errors.New("continue key is not valid: incorrect encoded start resourceVersion (version meta.k8s.io/v1)") + ErrEmptyStartKey = errors.New("continue key is not valid: encoded start key empty (version meta.k8s.io/v1)") + ErrGenericInvalidKey = errors.New("continue key is not valid") + ErrUnrecognizedEncodedVersion = errors.New("continue key is not valid: server does not recognize this encoded version") +) + // continueToken is a simple structured object for encoding the state of a continue token. // TODO: if we change the version of the encoded from, we can't start encoding the new version // until all other servers are upgraded (i.e. we need to support rolling schema) @@ -39,19 +47,19 @@ type continueToken struct { func DecodeContinue(continueValue, keyPrefix string) (fromKey string, rv int64, err error) { data, err := base64.RawURLEncoding.DecodeString(continueValue) if err != nil { - return "", 0, fmt.Errorf("continue key is not valid: %v", err) + return "", 0, fmt.Errorf("%w: %v", ErrGenericInvalidKey, err) } var c continueToken if err := json.Unmarshal(data, &c); err != nil { - return "", 0, fmt.Errorf("continue key is not valid: %v", err) + return "", 0, fmt.Errorf("%w: %v", ErrGenericInvalidKey, err) } switch c.APIVersion { case "meta.k8s.io/v1": if c.ResourceVersion == 0 { - return "", 0, fmt.Errorf("continue key is not valid: incorrect encoded start resourceVersion (version meta.k8s.io/v1)") + return "", 0, ErrInvalidStartRV } if len(c.StartKey) == 0 { - return "", 0, fmt.Errorf("continue key is not valid: encoded start key empty (version meta.k8s.io/v1)") + return "", 0, ErrEmptyStartKey } // defend against path traversal attacks by clients - path.Clean will ensure that startKey cannot // be at a higher level of the hierarchy, and so when we append the key prefix we will end up with @@ -63,11 +71,11 @@ func DecodeContinue(continueValue, keyPrefix string) (fromKey string, rv int64, } cleaned := path.Clean(key) if cleaned != key { - return "", 0, fmt.Errorf("continue key is not valid: %s", c.StartKey) + return "", 0, fmt.Errorf("%w: %v", ErrGenericInvalidKey, c.StartKey) } return keyPrefix + cleaned[1:], c.ResourceVersion, nil default: - return "", 0, fmt.Errorf("continue key is not valid: server does not recognize this encoded version %q", c.APIVersion) + return "", 0, fmt.Errorf("%w %v", ErrUnrecognizedEncodedVersion, c.APIVersion) } } diff --git a/staging/src/k8s.io/apiserver/pkg/storage/continue_test.go b/staging/src/k8s.io/apiserver/pkg/storage/continue_test.go index 3a8afb6a5c2..1006be7a2ae 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/continue_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/continue_test.go @@ -19,6 +19,7 @@ package storage import ( "encoding/base64" "encoding/json" + "errors" "testing" ) @@ -40,23 +41,65 @@ func Test_decodeContinue(t *testing.T) { args args wantFromKey string wantRv int64 - wantErr bool + wantErr error }{ - {name: "valid", args: args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, "key"), keyPrefix: "/test/"}, wantRv: 1, wantFromKey: "/test/key"}, - {name: "root path", args: args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, "/"), keyPrefix: "/test/"}, wantRv: 1, wantFromKey: "/test/"}, - - {name: "empty version", args: args{continueValue: encodeContinueOrDie("", 1, "key"), keyPrefix: "/test/"}, wantErr: true}, - {name: "invalid version", args: args{continueValue: encodeContinueOrDie("v1", 1, "key"), keyPrefix: "/test/"}, wantErr: true}, - - {name: "path traversal - parent", args: args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, "../key"), keyPrefix: "/test/"}, wantErr: true}, - {name: "path traversal - local", args: args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, "./key"), keyPrefix: "/test/"}, wantErr: true}, - {name: "path traversal - double parent", args: args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, "./../key"), keyPrefix: "/test/"}, wantErr: true}, - {name: "path traversal - after parent", args: args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, "key/../.."), keyPrefix: "/test/"}, wantErr: true}, + { + name: "valid", + args: args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, "key"), keyPrefix: "/test/"}, + wantRv: 1, + wantFromKey: "/test/key", + }, + { + name: "root path", + args: args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, "/"), keyPrefix: "/test/"}, + wantRv: 1, + wantFromKey: "/test/", + }, + { + name: "empty version", + args: args{continueValue: encodeContinueOrDie("", 1, "key"), keyPrefix: "/test/"}, + wantErr: ErrUnrecognizedEncodedVersion, + }, + { + name: "invalid version", + args: args{continueValue: encodeContinueOrDie("v1", 1, "key"), keyPrefix: "/test/"}, + wantErr: ErrUnrecognizedEncodedVersion, + }, + { + name: "invalid RV", + args: args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 0, "key"), keyPrefix: "/test/"}, + wantErr: ErrInvalidStartRV, + }, + { + name: "no start Key", + args: args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, ""), keyPrefix: "/test/"}, + wantErr: ErrEmptyStartKey, + }, + { + name: "path traversal - parent", + args: args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, "../key"), keyPrefix: "/test/"}, + wantErr: ErrGenericInvalidKey, + }, + { + name: "path traversal - local", + args: args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, "./key"), keyPrefix: "/test/"}, + wantErr: ErrGenericInvalidKey, + }, + { + name: "path traversal - double parent", + args: args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, "./../key"), keyPrefix: "/test/"}, + wantErr: ErrGenericInvalidKey, + }, + { + name: "path traversal - after parent", + args: args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, "key/../.."), keyPrefix: "/test/"}, + wantErr: ErrGenericInvalidKey, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { gotFromKey, gotRv, err := DecodeContinue(tt.args.continueValue, tt.args.keyPrefix) - if (err != nil) != tt.wantErr { + if !errors.Is(err, tt.wantErr) { t.Errorf("decodeContinue() error = %v, wantErr %v", err, tt.wantErr) return }