Promote continuation token schema to v1

Change the filtering logic to require a leading path and clean that
afterwards.
pull/6/head
Clayton Coleman 2017-09-24 18:09:54 -04:00
parent da7124e5e5
commit ac8808b792
No known key found for this signature in database
GPG Key ID: 3D16906B4F1C5CB3
3 changed files with 22 additions and 16 deletions

View File

@ -18,6 +18,7 @@ go_test(
"//vendor/github.com/coreos/etcd/clientv3:go_default_library", "//vendor/github.com/coreos/etcd/clientv3:go_default_library",
"//vendor/github.com/coreos/etcd/etcdserver/api/v3rpc/rpctypes:go_default_library", "//vendor/github.com/coreos/etcd/etcdserver/api/v3rpc/rpctypes:go_default_library",
"//vendor/github.com/coreos/etcd/integration:go_default_library", "//vendor/github.com/coreos/etcd/integration:go_default_library",
"//vendor/github.com/coreos/pkg/capnslog:go_default_library",
"//vendor/golang.org/x/net/context:go_default_library", "//vendor/golang.org/x/net/context:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/testing:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/testing:go_default_library",

View File

@ -429,25 +429,26 @@ func decodeContinue(continueValue, keyPrefix string) (fromKey string, rv int64,
return "", 0, fmt.Errorf("continue key is not valid: %v", err) return "", 0, fmt.Errorf("continue key is not valid: %v", err)
} }
switch c.APIVersion { switch c.APIVersion {
case "v1alpha1": case "meta.k8s.io/v1":
if c.ResourceVersion == 0 { if c.ResourceVersion == 0 {
return "", 0, fmt.Errorf("continue key is not valid: incorrect encoded start resourceVersion (version v1alpha1)") return "", 0, fmt.Errorf("continue key is not valid: incorrect encoded start resourceVersion (version meta.k8s.io/v1)")
} }
if len(c.StartKey) == 0 { if len(c.StartKey) == 0 {
return "", 0, fmt.Errorf("continue key is not valid: encoded start key empty (version v1alpha1)") return "", 0, fmt.Errorf("continue key is not valid: encoded start key empty (version meta.k8s.io/v1)")
} }
// defend against path traversal attacks by clients - path.Clean will ensure that startKey cannot // 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 // be at a higher level of the hierarchy, and so when we append the key prefix we will end up with
// continue start key that is fully qualified and cannot range over anything less specific than // continue start key that is fully qualified and cannot range over anything less specific than
// keyPrefix. // keyPrefix.
cleaned := path.Clean(c.StartKey) key := c.StartKey
if cleaned != c.StartKey || cleaned == "." || cleaned == "/" { if !strings.HasPrefix(key, "/") {
return "", 0, fmt.Errorf("continue key is not valid: %s", cleaned) key = "/" + key
} }
if len(cleaned) == 0 { cleaned := path.Clean(key)
return "", 0, fmt.Errorf("continue key is not valid: encoded start key empty (version 0)") if cleaned != key {
return "", 0, fmt.Errorf("continue key is not valid: %s", c.StartKey)
} }
return keyPrefix + cleaned, c.ResourceVersion, nil return keyPrefix + cleaned[1:], c.ResourceVersion, nil
default: default:
return "", 0, fmt.Errorf("continue key is not valid: server does not recognize this encoded version %q", c.APIVersion) return "", 0, fmt.Errorf("continue key is not valid: server does not recognize this encoded version %q", c.APIVersion)
} }
@ -459,7 +460,7 @@ func encodeContinue(key, keyPrefix string, resourceVersion int64) (string, error
if nextKey == key { if nextKey == key {
return "", fmt.Errorf("unable to encode next field: the key and key prefix do not match") return "", fmt.Errorf("unable to encode next field: the key and key prefix do not match")
} }
out, err := json.Marshal(&continueToken{APIVersion: "v1alpha1", ResourceVersion: resourceVersion, StartKey: nextKey}) out, err := json.Marshal(&continueToken{APIVersion: "meta.k8s.io/v1", ResourceVersion: resourceVersion, StartKey: nextKey})
if err != nil { if err != nil {
return "", err return "", err
} }

View File

@ -29,7 +29,9 @@ import (
"github.com/coreos/etcd/clientv3" "github.com/coreos/etcd/clientv3"
"github.com/coreos/etcd/integration" "github.com/coreos/etcd/integration"
"github.com/coreos/pkg/capnslog"
"golang.org/x/net/context" "golang.org/x/net/context"
apierrors "k8s.io/apimachinery/pkg/api/errors" apierrors "k8s.io/apimachinery/pkg/api/errors"
apitesting "k8s.io/apimachinery/pkg/api/testing" apitesting "k8s.io/apimachinery/pkg/api/testing"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@ -55,6 +57,8 @@ func init() {
metav1.AddToGroupVersion(scheme, metav1.SchemeGroupVersion) metav1.AddToGroupVersion(scheme, metav1.SchemeGroupVersion)
example.AddToScheme(scheme) example.AddToScheme(scheme)
examplev1.AddToScheme(scheme) examplev1.AddToScheme(scheme)
capnslog.SetGlobalLogLevel(capnslog.CRITICAL)
} }
// prefixTransformer adds and verifies that all data has the correct prefix on its way in and out. // prefixTransformer adds and verifies that all data has the correct prefix on its way in and out.
@ -1205,16 +1209,16 @@ func Test_decodeContinue(t *testing.T) {
wantRv int64 wantRv int64
wantErr bool wantErr bool
}{ }{
{name: "valid", args: args{continueValue: encodeContinueOrDie("v1alpha1", 1, "key"), keyPrefix: "/test/"}, wantRv: 1, wantFromKey: "/test/key"}, {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: "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: "invalid version", args: args{continueValue: encodeContinueOrDie("v1", 1, "key"), keyPrefix: "/test/"}, wantErr: true},
{name: "path traversal - parent", args: args{continueValue: encodeContinueOrDie("v1alpha", 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("v1alpha", 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("v1alpha", 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("v1alpha", 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: "path traversal - separator", args: args{continueValue: encodeContinueOrDie("v1alpha", 1, "/"), keyPrefix: "/test/"}, wantErr: true},
} }
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {