|
| 1 | +From 45e0bb4829c9c4cfb4e9b968f98b992659342b7e Mon Sep 17 00:00:00 2001 |
| 2 | +From: Tim Allclair <tallclair@google.com> |
| 3 | +Date: Mon, 10 Oct 2022 18:15:22 -0700 |
| 4 | +Subject: [PATCH] Validate etcd paths |
| 5 | + |
| 6 | +--- |
| 7 | + .../apiserver/pkg/storage/etcd3/store.go | 127 +++++++++++++----- |
| 8 | + 1 file changed, 92 insertions(+), 35 deletions(-) |
| 9 | + |
| 10 | +diff --git a/vendor/k8s.io/apiserver/pkg/storage/etcd3/store.go b/vendor/k8s.io/apiserver/pkg/storage/etcd3/store.go |
| 11 | +index 67ae9f5..6b3b808 100644 |
| 12 | +--- a/vendor/k8s.io/apiserver/pkg/storage/etcd3/store.go |
| 13 | ++++ b/vendor/k8s.io/apiserver/pkg/storage/etcd3/store.go |
| 14 | +@@ -89,6 +89,14 @@ func New(c *clientv3.Client, codec runtime.Codec, newFunc func() runtime.Object, |
| 15 | + |
| 16 | + func newStore(c *clientv3.Client, codec runtime.Codec, newFunc func() runtime.Object, prefix string, transformer value.Transformer, pagingEnabled bool, leaseManagerConfig LeaseManagerConfig) *store { |
| 17 | + versioner := APIObjectVersioner{} |
| 18 | ++ // for compatibility with etcd2 impl. |
| 19 | ++ // no-op for default prefix of '/registry'. |
| 20 | ++ // keeps compatibility with etcd2 impl for custom prefixes that don't start with '/' |
| 21 | ++ pathPrefix := path.Join("/", prefix) |
| 22 | ++ if !strings.HasSuffix(pathPrefix, "/") { |
| 23 | ++ // Ensure the pathPrefix ends in "/" here to simplify key concatenation later. |
| 24 | ++ pathPrefix += "/" |
| 25 | ++ } |
| 26 | + result := &store{ |
| 27 | + client: c, |
| 28 | + codec: codec, |
| 29 | +@@ -98,9 +106,9 @@ func newStore(c *clientv3.Client, codec runtime.Codec, newFunc func() runtime.Ob |
| 30 | + // for compatibility with etcd2 impl. |
| 31 | + // no-op for default prefix of '/registry'. |
| 32 | + // keeps compatibility with etcd2 impl for custom prefixes that don't start with '/' |
| 33 | +- pathPrefix: path.Join("/", prefix), |
| 34 | +- watcher: newWatcher(c, codec, newFunc, versioner, transformer), |
| 35 | +- leaseManager: newDefaultLeaseManager(c, leaseManagerConfig), |
| 36 | ++ pathPrefix: pathPrefix, |
| 37 | ++ watcher: newWatcher(c, codec, newFunc, versioner, transformer), |
| 38 | ++ leaseManager: newDefaultLeaseManager(c, leaseManagerConfig), |
| 39 | + } |
| 40 | + return result |
| 41 | + } |
| 42 | +@@ -112,9 +120,12 @@ func (s *store) Versioner() storage.Versioner { |
| 43 | + |
| 44 | + // Get implements storage.Interface.Get. |
| 45 | + func (s *store) Get(ctx context.Context, key string, opts storage.GetOptions, out runtime.Object) error { |
| 46 | +- key = path.Join(s.pathPrefix, key) |
| 47 | ++ preparedKey, err := s.prepareKey(key) |
| 48 | ++ if err != nil { |
| 49 | ++ return err |
| 50 | ++ } |
| 51 | + startTime := time.Now() |
| 52 | +- getResp, err := s.client.KV.Get(ctx, key) |
| 53 | ++ getResp, err := s.client.KV.Get(ctx, preparedKey) |
| 54 | + metrics.RecordEtcdRequestLatency("get", getTypeName(out), startTime) |
| 55 | + if err != nil { |
| 56 | + return err |
| 57 | +@@ -127,11 +138,11 @@ func (s *store) Get(ctx context.Context, key string, opts storage.GetOptions, ou |
| 58 | + if opts.IgnoreNotFound { |
| 59 | + return runtime.SetZeroValue(out) |
| 60 | + } |
| 61 | +- return storage.NewKeyNotFoundError(key, 0) |
| 62 | ++ return storage.NewKeyNotFoundError(preparedKey, 0) |
| 63 | + } |
| 64 | + kv := getResp.Kvs[0] |
| 65 | + |
| 66 | +- data, _, err := s.transformer.TransformFromStorage(kv.Value, authenticatedDataString(key)) |
| 67 | ++ data, _, err := s.transformer.TransformFromStorage(kv.Value, authenticatedDataString(preparedKey)) |
| 68 | + if err != nil { |
| 69 | + return storage.NewInternalError(err.Error()) |
| 70 | + } |
| 71 | +@@ -141,6 +152,10 @@ func (s *store) Get(ctx context.Context, key string, opts storage.GetOptions, ou |
| 72 | + |
| 73 | + // Create implements storage.Interface.Create. |
| 74 | + func (s *store) Create(ctx context.Context, key string, obj, out runtime.Object, ttl uint64) error { |
| 75 | ++ preparedKey, err := s.prepareKey(key) |
| 76 | ++ if err != nil { |
| 77 | ++ return err |
| 78 | ++ } |
| 79 | + if version, err := s.versioner.ObjectResourceVersion(obj); err == nil && version != 0 { |
| 80 | + return errors.New("resourceVersion should not be set on objects to be created") |
| 81 | + } |
| 82 | +@@ -151,30 +166,29 @@ func (s *store) Create(ctx context.Context, key string, obj, out runtime.Object, |
| 83 | + if err != nil { |
| 84 | + return err |
| 85 | + } |
| 86 | +- key = path.Join(s.pathPrefix, key) |
| 87 | + |
| 88 | + opts, err := s.ttlOpts(ctx, int64(ttl)) |
| 89 | + if err != nil { |
| 90 | + return err |
| 91 | + } |
| 92 | + |
| 93 | +- newData, err := s.transformer.TransformToStorage(data, authenticatedDataString(key)) |
| 94 | ++ newData, err := s.transformer.TransformToStorage(data, authenticatedDataString(preparedKey)) |
| 95 | + if err != nil { |
| 96 | + return storage.NewInternalError(err.Error()) |
| 97 | + } |
| 98 | + |
| 99 | + startTime := time.Now() |
| 100 | + txnResp, err := s.client.KV.Txn(ctx).If( |
| 101 | +- notFound(key), |
| 102 | ++ notFound(preparedKey), |
| 103 | + ).Then( |
| 104 | +- clientv3.OpPut(key, string(newData), opts...), |
| 105 | ++ clientv3.OpPut(preparedKey, string(newData), opts...), |
| 106 | + ).Commit() |
| 107 | + metrics.RecordEtcdRequestLatency("create", getTypeName(obj), startTime) |
| 108 | + if err != nil { |
| 109 | + return err |
| 110 | + } |
| 111 | + if !txnResp.Succeeded { |
| 112 | +- return storage.NewKeyExistsError(key, 0) |
| 113 | ++ return storage.NewKeyExistsError(preparedKey, 0) |
| 114 | + } |
| 115 | + |
| 116 | + if out != nil { |
| 117 | +@@ -186,12 +200,15 @@ func (s *store) Create(ctx context.Context, key string, obj, out runtime.Object, |
| 118 | + |
| 119 | + // Delete implements storage.Interface.Delete. |
| 120 | + func (s *store) Delete(ctx context.Context, key string, out runtime.Object, preconditions *storage.Preconditions, validateDeletion storage.ValidateObjectFunc) error { |
| 121 | ++ preparedKey, err := s.prepareKey(key) |
| 122 | ++ if err != nil { |
| 123 | ++ return err |
| 124 | ++ } |
| 125 | + v, err := conversion.EnforcePtr(out) |
| 126 | + if err != nil { |
| 127 | + return fmt.Errorf("unable to convert output object to pointer: %v", err) |
| 128 | + } |
| 129 | +- key = path.Join(s.pathPrefix, key) |
| 130 | +- return s.conditionalDelete(ctx, key, out, v, preconditions, validateDeletion) |
| 131 | ++ return s.conditionalDelete(ctx, preparedKey, out, v, preconditions, validateDeletion) |
| 132 | + } |
| 133 | + |
| 134 | + func (s *store) conditionalDelete(ctx context.Context, key string, out runtime.Object, v reflect.Value, preconditions *storage.Preconditions, validateDeletion storage.ValidateObjectFunc) error { |
| 135 | +@@ -239,6 +256,10 @@ func (s *store) conditionalDelete(ctx context.Context, key string, out runtime.O |
| 136 | + func (s *store) GuaranteedUpdate( |
| 137 | + ctx context.Context, key string, out runtime.Object, ignoreNotFound bool, |
| 138 | + preconditions *storage.Preconditions, tryUpdate storage.UpdateFunc, suggestion runtime.Object) error { |
| 139 | ++ preparedKey, err := s.prepareKey(key) |
| 140 | ++ if err != nil { |
| 141 | ++ return err |
| 142 | ++ } |
| 143 | + trace := utiltrace.New("GuaranteedUpdate etcd3", utiltrace.Field{"type", getTypeName(out)}) |
| 144 | + defer trace.LogIfLong(500 * time.Millisecond) |
| 145 | + |
| 146 | +@@ -246,16 +267,15 @@ func (s *store) GuaranteedUpdate( |
| 147 | + if err != nil { |
| 148 | + return fmt.Errorf("unable to convert output object to pointer: %v", err) |
| 149 | + } |
| 150 | +- key = path.Join(s.pathPrefix, key) |
| 151 | + |
| 152 | + getCurrentState := func() (*objState, error) { |
| 153 | + startTime := time.Now() |
| 154 | +- getResp, err := s.client.KV.Get(ctx, key) |
| 155 | ++ getResp, err := s.client.KV.Get(ctx, preparedKey) |
| 156 | + metrics.RecordEtcdRequestLatency("get", getTypeName(out), startTime) |
| 157 | + if err != nil { |
| 158 | + return nil, err |
| 159 | + } |
| 160 | +- return s.getState(getResp, key, v, ignoreNotFound) |
| 161 | ++ return s.getState(getResp, preparedKey, v, ignoreNotFound) |
| 162 | + } |
| 163 | + |
| 164 | + var origState *objState |
| 165 | +@@ -274,9 +294,9 @@ func (s *store) GuaranteedUpdate( |
| 166 | + } |
| 167 | + trace.Step("initial value restored") |
| 168 | + |
| 169 | +- transformContext := authenticatedDataString(key) |
| 170 | ++ transformContext := authenticatedDataString(preparedKey) |
| 171 | + for { |
| 172 | +- if err := preconditions.Check(key, origState.obj); err != nil { |
| 173 | ++ if err := preconditions.Check(preparedKey, origState.obj); err != nil { |
| 174 | + // If our data is already up to date, return the error |
| 175 | + if !mustCheckData { |
| 176 | + return err |
| 177 | +@@ -349,11 +369,11 @@ func (s *store) GuaranteedUpdate( |
| 178 | + |
| 179 | + startTime := time.Now() |
| 180 | + txnResp, err := s.client.KV.Txn(ctx).If( |
| 181 | +- clientv3.Compare(clientv3.ModRevision(key), "=", origState.rev), |
| 182 | ++ clientv3.Compare(clientv3.ModRevision(preparedKey), "=", origState.rev), |
| 183 | + ).Then( |
| 184 | +- clientv3.OpPut(key, string(newData), opts...), |
| 185 | ++ clientv3.OpPut(preparedKey, string(newData), opts...), |
| 186 | + ).Else( |
| 187 | +- clientv3.OpGet(key), |
| 188 | ++ clientv3.OpGet(preparedKey), |
| 189 | + ).Commit() |
| 190 | + metrics.RecordEtcdRequestLatency("update", getTypeName(out), startTime) |
| 191 | + if err != nil { |
| 192 | +@@ -362,8 +382,8 @@ func (s *store) GuaranteedUpdate( |
| 193 | + trace.Step("Transaction committed") |
| 194 | + if !txnResp.Succeeded { |
| 195 | + getResp := (*clientv3.GetResponse)(txnResp.Responses[0].GetResponseRange()) |
| 196 | +- klog.V(4).Infof("GuaranteedUpdate of %s failed because of a conflict, going to retry", key) |
| 197 | +- origState, err = s.getState(getResp, key, v, ignoreNotFound) |
| 198 | ++ klog.V(4).Infof("GuaranteedUpdate of %s failed because of a conflict, going to retry", preparedKey) |
| 199 | ++ origState, err = s.getState(getResp, preparedKey, v, ignoreNotFound) |
| 200 | + if err != nil { |
| 201 | + return err |
| 202 | + } |
| 203 | +@@ -400,7 +420,10 @@ func (s *store) GetToList(ctx context.Context, key string, listOpts storage.List |
| 204 | + |
| 205 | + newItemFunc := getNewItemFunc(listObj, v) |
| 206 | + |
| 207 | +- key = path.Join(s.pathPrefix, key) |
| 208 | ++ preparedKey, err := s.prepareKey(key) |
| 209 | ++ if err != nil { |
| 210 | ++ return err |
| 211 | ++ } |
| 212 | + startTime := time.Now() |
| 213 | + var opts []clientv3.OpOption |
| 214 | + if len(resourceVersion) > 0 && match == metav1.ResourceVersionMatchExact { |
| 215 | +@@ -411,7 +434,7 @@ func (s *store) GetToList(ctx context.Context, key string, listOpts storage.List |
| 216 | + opts = append(opts, clientv3.WithRev(int64(rv))) |
| 217 | + } |
| 218 | + |
| 219 | +- getResp, err := s.client.KV.Get(ctx, key, opts...) |
| 220 | ++ getResp, err := s.client.KV.Get(ctx, preparedKey, opts...) |
| 221 | + metrics.RecordEtcdRequestLatency("get", getTypeName(listPtr), startTime) |
| 222 | + if err != nil { |
| 223 | + return err |
| 224 | +@@ -421,7 +444,7 @@ func (s *store) GetToList(ctx context.Context, key string, listOpts storage.List |
| 225 | + } |
| 226 | + |
| 227 | + if len(getResp.Kvs) > 0 { |
| 228 | +- data, _, err := s.transformer.TransformFromStorage(getResp.Kvs[0].Value, authenticatedDataString(key)) |
| 229 | ++ data, _, err := s.transformer.TransformFromStorage(getResp.Kvs[0].Value, authenticatedDataString(preparedKey)) |
| 230 | + if err != nil { |
| 231 | + return storage.NewInternalError(err.Error()) |
| 232 | + } |
| 233 | +@@ -451,18 +474,21 @@ func getNewItemFunc(listObj runtime.Object, v reflect.Value) func() runtime.Obje |
| 234 | + } |
| 235 | + |
| 236 | + func (s *store) Count(key string) (int64, error) { |
| 237 | +- key = path.Join(s.pathPrefix, key) |
| 238 | ++ preparedKey, err := s.prepareKey(key) |
| 239 | ++ if err != nil { |
| 240 | ++ return 0, err |
| 241 | ++ } |
| 242 | + |
| 243 | + // We need to make sure the key ended with "/" so that we only get children "directories". |
| 244 | + // e.g. if we have key "/a", "/a/b", "/ab", getting keys with prefix "/a" will return all three, |
| 245 | + // while with prefix "/a/" will return only "/a/b" which is the correct answer. |
| 246 | +- if !strings.HasSuffix(key, "/") { |
| 247 | +- key += "/" |
| 248 | ++ if !strings.HasSuffix(preparedKey, "/") { |
| 249 | ++ preparedKey += "/" |
| 250 | + } |
| 251 | + |
| 252 | + startTime := time.Now() |
| 253 | +- getResp, err := s.client.KV.Get(context.Background(), key, clientv3.WithRange(clientv3.GetPrefixRangeEnd(key)), clientv3.WithCountOnly()) |
| 254 | +- metrics.RecordEtcdRequestLatency("listWithCount", key, startTime) |
| 255 | ++ getResp, err := s.client.KV.Get(context.Background(), preparedKey, clientv3.WithRange(clientv3.GetPrefixRangeEnd(preparedKey)), clientv3.WithCountOnly()) |
| 256 | ++ metrics.RecordEtcdRequestLatency("listWithCount", preparedKey, startTime) |
| 257 | + if err != nil { |
| 258 | + return 0, err |
| 259 | + } |
| 260 | +@@ -551,7 +577,11 @@ func (s *store) List(ctx context.Context, key string, opts storage.ListOptions, |
| 261 | + } |
| 262 | + |
| 263 | + if s.pathPrefix != "" { |
| 264 | +- key = path.Join(s.pathPrefix, key) |
| 265 | ++ preparedKey, err := s.prepareKey(key) |
| 266 | ++ if err != nil { |
| 267 | ++ return err |
| 268 | ++ } |
| 269 | ++ key = preparedKey |
| 270 | + } |
| 271 | + // We need to make sure the key ended with "/" so that we only get children "directories". |
| 272 | + // e.g. if we have key "/a", "/a/b", "/ab", getting keys with prefix "/a" will return all three, |
| 273 | +@@ -783,8 +813,11 @@ func (s *store) watch(ctx context.Context, key string, opts storage.ListOptions, |
| 274 | + if err != nil { |
| 275 | + return nil, err |
| 276 | + } |
| 277 | +- key = path.Join(s.pathPrefix, key) |
| 278 | +- return s.watcher.Watch(ctx, key, int64(rev), recursive, opts.ProgressNotify, opts.Predicate) |
| 279 | ++ preparedKey, err := s.prepareKey(key) |
| 280 | ++ if err != nil { |
| 281 | ++ return nil, err |
| 282 | ++ } |
| 283 | ++ return s.watcher.Watch(ctx, preparedKey, int64(rev), recursive, opts.ProgressNotify, opts.Predicate) |
| 284 | + } |
| 285 | + |
| 286 | + func (s *store) getState(getResp *clientv3.GetResponse, key string, v reflect.Value, ignoreNotFound bool) (*objState, error) { |
| 287 | +@@ -896,6 +929,30 @@ func (s *store) validateMinimumResourceVersion(minimumResourceVersion string, ac |
| 288 | + return nil |
| 289 | + } |
| 290 | + |
| 291 | ++func (s *store) prepareKey(key string) (string, error) { |
| 292 | ++ if key == ".." || |
| 293 | ++ strings.HasPrefix(key, "../") || |
| 294 | ++ strings.HasSuffix(key, "/..") || |
| 295 | ++ strings.Contains(key, "/../") { |
| 296 | ++ return "", fmt.Errorf("invalid key: %q", key) |
| 297 | ++ } |
| 298 | ++ if key == "." || |
| 299 | ++ strings.HasPrefix(key, "./") || |
| 300 | ++ strings.HasSuffix(key, "/.") || |
| 301 | ++ strings.Contains(key, "/./") { |
| 302 | ++ return "", fmt.Errorf("invalid key: %q", key) |
| 303 | ++ } |
| 304 | ++ if key == "" || key == "/" { |
| 305 | ++ return "", fmt.Errorf("empty key: %q", key) |
| 306 | ++ } |
| 307 | ++ // We ensured that pathPrefix ends in '/' in construction, so skip any leading '/' in the key now. |
| 308 | ++ startIndex := 0 |
| 309 | ++ if key[0] == '/' { |
| 310 | ++ startIndex = 1 |
| 311 | ++ } |
| 312 | ++ return s.pathPrefix + key[startIndex:], nil |
| 313 | ++} |
| 314 | ++ |
| 315 | + // decode decodes value of bytes into object. It will also set the object resource version to rev. |
| 316 | + // On success, objPtr would be set to the object. |
| 317 | + func decode(codec runtime.Codec, versioner storage.Versioner, value []byte, objPtr runtime.Object, rev int64) error { |
| 318 | +-- |
| 319 | +2.40.4 |
| 320 | + |
0 commit comments