1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
487
488
489
490
491
492
493
494
495
496
497
498
499
500
501
502
503
504
505
506
507
508
509
510
511
512
513
514
515
516
517
518
519
520
521
522
523
524
525
526
527
528
529
530
531
532
533
534
535
536
537
538
539
540
541
542
543
544
545
546
547
548
549
550
551
552
553
554
555
556
557
558
559
560
561
562
563
564
565
566
567
568
569
570
571
572
573
574
575
576
577
578
579
580
581
582
583
584
585
586
587
588
589
590
591
592
593
594
595
596
597
598
599
600
601
602
603
604
605
606
607
608
609
610
611
612
613
614
615
616
617
618
619
620
621
622
623
624
625
626
627
628
629
630
631
632
633
634
635
636
637
638
639
640
641
642
643
644
645
646
647
648
649
650
651
652
653
654
655
656
657
658
659
660
661
662
663
664
665
666
667
668
669
670
671
672
673
674
675
676
677
678
679
680
681
682
683
684
685
686
687
688
689
690
691
692
693
694
695
696
697
698
699
700
701
702
703
704
705
706
707
708
709
710
711
712
713
714
715
716
717
718
719
720
721
722
723
724
725
726
727
728
729
730
731
732
733
734
735
736
737
738
739
740
741
742
743
744
745
746
747
748
749
750
751
752
753
754
755
756
757
758
759
760
761
762
763
764
765
766
767
768
769
770
771
772
773
774
775
776
777
778
779
780
781
782
783
784
785
786
787
788
789
790
791
792
793
794
795
796
797
798
799
800
801
802
803
804
805
806
807
808
809
810
811
812
813
814
815
816
817
818
819
820
821
822
823
824
825
826
827
828
829
830
831
832
833
834
835
836
837
838
839
840
841
842
843
844
845
846
847
848
849
850
851
852
853
854
|
From 668b22a4a92ce7f842a247c38dcf5010338716dd Mon Sep 17 00:00:00 2001
From: Clay Gerrard <clay.gerrard@gmail.com>
Date: Thu, 23 Jul 2015 22:36:21 -0700
Subject: [PATCH 1/2] Disallow unsafe tempurl operations to point to
unauthorized data
Do not allow PUT tempurls to create pointers to other data. Specifically
disallow the creation of DLO object manifests by returning an error if a
non-safe tempurl request includes an X-Object-Manifest header regardless of
the value of the header.
This prevents discoverability attacks which can use any PUT tempurl to probe
for private data by creating a DLO object manifest and then using the PUT
tempurl to head the object which would 404 if the prefix does not match any
object data or form a valid DLO HEAD response if it does.
This also prevents a tricky and potentially unexpected consequence of PUT
tempurls which would make it unsafe to allow a user to download objects
created by tempurl (even if they just created them) because the result of
reading the object created via tempurl may not be the data which was uploaded.
Co-Authored-By: Kota Tsuyuzaki <tsuyuzaki.kota@lab.ntt.co.jp>
Change-Id: I91161dfb0f089c3990aca1b4255b520299ef73c8
---
swift/common/middleware/tempurl.py | 31 ++++++++++++++++++++++++-
test/functional/tests.py | 36 +++++++++++++++++++++++++++++
test/unit/common/middleware/test_tempurl.py | 19 +++++++++++++++
3 files changed, 85 insertions(+), 1 deletion(-)
diff --git a/swift/common/middleware/tempurl.py b/swift/common/middleware/tempurl.py
index 3dd1448..659121e 100644
--- a/swift/common/middleware/tempurl.py
+++ b/swift/common/middleware/tempurl.py
@@ -122,11 +122,13 @@ from urllib import urlencode
from urlparse import parse_qs
from swift.proxy.controllers.base import get_account_info, get_container_info
-from swift.common.swob import HeaderKeyDict, HTTPUnauthorized
+from swift.common.swob import HeaderKeyDict, HTTPUnauthorized, HTTPBadRequest
from swift.common.utils import split_path, get_valid_utf8_str, \
register_swift_info, get_hmac, streq_const_time, quote
+DISALLOWED_INCOMING_HEADERS = 'x-object-manifest'
+
#: Default headers to remove from incoming requests. Simply a whitespace
#: delimited list of header names and names can optionally end with '*' to
#: indicate a prefix match. DEFAULT_INCOMING_ALLOW_HEADERS is a list of
@@ -230,6 +232,10 @@ class TempURL(object):
#: The methods allowed with Temp URLs.
self.methods = methods
+ self.disallowed_headers = set(
+ 'HTTP_' + h.upper().replace('-', '_')
+ for h in DISALLOWED_INCOMING_HEADERS.split())
+
headers = DEFAULT_INCOMING_REMOVE_HEADERS
if 'incoming_remove_headers' in conf:
headers = conf['incoming_remove_headers']
@@ -323,6 +329,13 @@ class TempURL(object):
for hmac in hmac_vals)
if not is_valid_hmac:
return self._invalid(env, start_response)
+ # disallowed headers prevent accidently allowing upload of a pointer
+ # to data that the PUT tempurl would not otherwise allow access for.
+ # It should be safe to provide a GET tempurl for data that an
+ # untrusted client just uploaded with a PUT tempurl.
+ resp = self._clean_disallowed_headers(env, start_response)
+ if resp:
+ return resp
self._clean_incoming_headers(env)
env['swift.authorize'] = lambda req: None
env['swift.authorize_override'] = True
@@ -465,6 +478,22 @@ class TempURL(object):
body = '401 Unauthorized: Temp URL invalid\n'
return HTTPUnauthorized(body=body)(env, start_response)
+ def _clean_disallowed_headers(self, env, start_response):
+ """
+ Validate the absense of disallowed headers for "unsafe" operations.
+
+ :returns: None for safe operations or swob.HTTPBadResponse if the
+ request includes disallowed headers.
+ """
+ if env['REQUEST_METHOD'] in ('GET', 'HEAD', 'OPTIONS'):
+ return
+ for h in env:
+ if h in self.disallowed_headers:
+ return HTTPBadRequest(
+ body='The header %r is not allowed in this tempurl' %
+ h[len('HTTP_'):].title().replace('_', '-'))(
+ env, start_response)
+
def _clean_incoming_headers(self, env):
"""
Removes any headers from the WSGI environment as per the
diff --git a/test/functional/tests.py b/test/functional/tests.py
index 95f168e..800d070 100644
--- a/test/functional/tests.py
+++ b/test/functional/tests.py
@@ -2732,6 +2732,42 @@ class TestTempurl(Base):
self.assert_(new_obj.info(parms=put_parms,
cfg={'no_auth_token': True}))
+ def test_PUT_manifest_access(self):
+ new_obj = self.env.container.file(Utils.create_name())
+
+ # give out a signature which allows a PUT to new_obj
+ expires = int(time.time()) + 86400
+ sig = self.tempurl_sig(
+ 'PUT', expires, self.env.conn.make_path(new_obj.path),
+ self.env.tempurl_key)
+ put_parms = {'temp_url_sig': sig,
+ 'temp_url_expires': str(expires)}
+
+ # try to create manifest pointing to some random container
+ try:
+ new_obj.write('', {
+ 'x-object-manifest': '%s/foo' % 'some_random_container'
+ }, parms=put_parms, cfg={'no_auth_token': True})
+ except ResponseError as e:
+ self.assertEqual(e.status, 400)
+ else:
+ self.fail('request did not error')
+
+ # create some other container
+ other_container = self.env.account.container(Utils.create_name())
+ if not other_container.create():
+ raise ResponseError(self.conn.response)
+
+ # try to create manifest pointing to new container
+ try:
+ new_obj.write('', {
+ 'x-object-manifest': '%s/foo' % other_container
+ }, parms=put_parms, cfg={'no_auth_token': True})
+ except ResponseError as e:
+ self.assertEqual(e.status, 400)
+ else:
+ self.fail('request did not error')
+
def test_HEAD(self):
expires = int(time.time()) + 86400
sig = self.tempurl_sig(
diff --git a/test/unit/common/middleware/test_tempurl.py b/test/unit/common/middleware/test_tempurl.py
index e665563..ba42ace 100644
--- a/test/unit/common/middleware/test_tempurl.py
+++ b/test/unit/common/middleware/test_tempurl.py
@@ -649,6 +649,25 @@ class TestTempURL(unittest.TestCase):
self.assertTrue('Temp URL invalid' in resp.body)
self.assertTrue('Www-Authenticate' in resp.headers)
+ def test_disallowed_header_object_manifest(self):
+ self.tempurl = tempurl.filter_factory({})(self.auth)
+ method = 'PUT'
+ expires = int(time() + 86400)
+ path = '/v1/a/c/o'
+ key = 'abc'
+ hmac_body = '%s\n%s\n%s' % (method, expires, path)
+ sig = hmac.new(key, hmac_body, sha1).hexdigest()
+ req = self._make_request(
+ path, method='PUT', keys=[key],
+ headers={'x-object-manifest': 'private/secret'},
+ environ={'QUERY_STRING': 'temp_url_sig=%s&temp_url_expires=%s' % (
+ sig, expires)})
+ resp = req.get_response(self.tempurl)
+ self.assertEquals(resp.status_int, 400)
+ self.assertTrue('header' in resp.body)
+ self.assertTrue('not allowed' in resp.body)
+ self.assertTrue('X-Object-Manifest' in resp.body)
+
def test_removed_incoming_header(self):
self.tempurl = tempurl.filter_factory({
'incoming_remove_headers': 'x-remove-this'})(self.auth)
--
2.4.6
From fdd96d85dab7655649c75d5c6f6df5639c742daf Mon Sep 17 00:00:00 2001
From: Samuel Merritt <sam@swiftstack.com>
Date: Tue, 11 Aug 2015 09:10:13 -0500
Subject: [PATCH 2/2] Better scoping for tempurls, especially container
tempurls
It used to be that a GET of a tempurl referencing a large object would
let you download that large object regardless of where its segments
lived. However, this led to some violated user expectations around
container tempurls.
(Note on shorthand: all tempurls reference objects. However, "account
tempurl" and "container tempurl" are shorthand meaning tempurls
generated using a key on the account or container, respectively.)
Let's say an application is given tempurl keys to a particular
container, and it does all its work therein using those keys. The user
expects that, if the application is compromised, then the attacker
only gains access to the "compromised-container". However, with the old
behavior, the attacker could read data from *any* container like so:
1) Choose a "victim-container" to download
2) Create PUT and GET tempurl for any object name within the
"compromised-container". The object doesn't need to exist;
we'll create it.
3) Using the PUT tempurl, upload a DLO manifest with
"X-Object-Manifest: /victim-container/"
4) Using the GET tempurl, download the object created in step 3. The
result will be the concatenation of all objects in the
"victim-container".
Step 3 need not be for all objects in the "victim-container"; for
example, a value "X-Object-Manifest: /victim-container/abc" would only
be the concatenation of all objects whose names begin with "abc". By
probing for object names in this way, individual objects may be found
and extracted.
A similar bug would exist for manifests referencing other accounts
except that neither the X-Object-Manifest (DLO) nor the JSON manifest
document (SLO) have a way of specifying a different account.
This change makes it so that a container tempurl only grants access to
objects within its container, *including* large-object segments. This
breaks backward compatibility for container tempurls that may have
pointed to cross container *LO's, but (a) there are security
implications, and (b) container tempurls are a relatively new feature.
This works by having the tempurl middleware install an authorization
callback ('swift.authorize' in the WSGI environment) that limits the
scope of any requests to the account or container from which the key
came.
This requires swift.authorize to persist for both the manifest request
and all segment requests; this is done by having the proxy server
restore it to the WSGI environment prior to returning from __call__.
Co-Authored-By: Clay Gerrard <clayg@swiftstack.com>
Co-Authored-By: Alistair Coles <alistair.coles@hp.com>
Co-Authored-By: Christian Schwede <cschwede@redhat.com>
Co-Authored-By: Matthew Oliver <matt@oliver.net.au>
Change-Id: I11078af178cb9acdd9039388282fcd0db165ba7a
---
swift/common/middleware/tempurl.py | 105 +++++++++++++----
swift/proxy/server.py | 11 +-
test/functional/tests.py | 114 +++++++++++++++++++
test/unit/common/middleware/test_tempurl.py | 171 +++++++++++++++++++++-------
4 files changed, 333 insertions(+), 68 deletions(-)
diff --git a/swift/common/middleware/tempurl.py b/swift/common/middleware/tempurl.py
index 659121e..fb8bb01 100644
--- a/swift/common/middleware/tempurl.py
+++ b/swift/common/middleware/tempurl.py
@@ -152,6 +152,10 @@ DEFAULT_OUTGOING_REMOVE_HEADERS = 'x-object-meta-*'
DEFAULT_OUTGOING_ALLOW_HEADERS = 'x-object-meta-public-*'
+CONTAINER_SCOPE = 'container'
+ACCOUNT_SCOPE = 'account'
+
+
def get_tempurl_keys_from_metadata(meta):
"""
Extracts the tempurl keys from metadata.
@@ -172,6 +176,38 @@ def disposition_format(filename):
quote(filename, safe=' /'), quote(filename))
+def authorize_same_account(account_to_match):
+
+ def auth_callback_same_account(req):
+ try:
+ _ver, acc, _rest = req.split_path(2, 3, True)
+ except ValueError:
+ return HTTPUnauthorized(request=req)
+
+ if acc == account_to_match:
+ return None
+ else:
+ return HTTPUnauthorized(request=req)
+
+ return auth_callback_same_account
+
+
+def authorize_same_container(account_to_match, container_to_match):
+
+ def auth_callback_same_container(req):
+ try:
+ _ver, acc, con, _rest = req.split_path(3, 4, True)
+ except ValueError:
+ return HTTPUnauthorized(request=req)
+
+ if acc == account_to_match and con == container_to_match:
+ return None
+ else:
+ return HTTPUnauthorized(request=req)
+
+ return auth_callback_same_container
+
+
class TempURL(object):
"""
WSGI Middleware to grant temporary URLs specific access to Swift
@@ -304,10 +340,10 @@ class TempURL(object):
return self.app(env, start_response)
if not temp_url_sig or not temp_url_expires:
return self._invalid(env, start_response)
- account = self._get_account(env)
+ account, container = self._get_account_and_container(env)
if not account:
return self._invalid(env, start_response)
- keys = self._get_keys(env, account)
+ keys = self._get_keys(env)
if not keys:
return self._invalid(env, start_response)
if env['REQUEST_METHOD'] == 'HEAD':
@@ -322,11 +358,16 @@ class TempURL(object):
else:
hmac_vals = self._get_hmacs(env, temp_url_expires, keys)
- # While it's true that any() will short-circuit, this doesn't affect
- # the timing-attack resistance since the only way this will
- # short-circuit is when a valid signature is passed in.
- is_valid_hmac = any(streq_const_time(temp_url_sig, hmac)
- for hmac in hmac_vals)
+ is_valid_hmac = False
+ hmac_scope = None
+ for hmac, scope in hmac_vals:
+ # While it's true that we short-circuit, this doesn't affect the
+ # timing-attack resistance since the only way this will
+ # short-circuit is when a valid signature is passed in.
+ if streq_const_time(temp_url_sig, hmac):
+ is_valid_hmac = True
+ hmac_scope = scope
+ break
if not is_valid_hmac:
return self._invalid(env, start_response)
# disallowed headers prevent accidently allowing upload of a pointer
@@ -337,7 +378,12 @@ class TempURL(object):
if resp:
return resp
self._clean_incoming_headers(env)
- env['swift.authorize'] = lambda req: None
+
+ if hmac_scope == ACCOUNT_SCOPE:
+ env['swift.authorize'] = authorize_same_account(account)
+ else:
+ env['swift.authorize'] = authorize_same_container(account,
+ container)
env['swift.authorize_override'] = True
env['REMOTE_USER'] = '.wsgi.tempurl'
qs = {'temp_url_sig': temp_url_sig,
@@ -378,22 +424,23 @@ class TempURL(object):
return self.app(env, _start_response)
- def _get_account(self, env):
+ def _get_account_and_container(self, env):
"""
- Returns just the account for the request, if it's an object
- request and one of the configured methods; otherwise, None is
+ Returns just the account and container for the request, if it's an
+ object request and one of the configured methods; otherwise, None is
returned.
:param env: The WSGI environment for the request.
- :returns: Account str or None.
+ :returns: (Account str, container str) or (None, None).
"""
if env['REQUEST_METHOD'] in self.methods:
try:
ver, acc, cont, obj = split_path(env['PATH_INFO'], 4, 4, True)
except ValueError:
- return None
+ return (None, None)
if ver == 'v1' and obj.strip('/'):
- return acc
+ return (acc, cont)
+ return (None, None)
def _get_temp_url_info(self, env):
"""
@@ -423,18 +470,23 @@ class TempURL(object):
inline = True
return temp_url_sig, temp_url_expires, filename, inline
- def _get_keys(self, env, account):
+ def _get_keys(self, env):
"""
Returns the X-[Account|Container]-Meta-Temp-URL-Key[-2] header values
- for the account or container, or an empty list if none are set.
+ for the account or container, or an empty list if none are set. Each
+ value comes as a 2-tuple (key, scope), where scope is either
+ CONTAINER_SCOPE or ACCOUNT_SCOPE.
Returns 0-4 elements depending on how many keys are set in the
account's or container's metadata.
:param env: The WSGI environment for the request.
- :param account: Account str.
- :returns: [X-Account-Meta-Temp-URL-Key str value if set,
- X-Account-Meta-Temp-URL-Key-2 str value if set]
+ :returns: [
+ (X-Account-Meta-Temp-URL-Key str value, ACCOUNT_SCOPE) if set,
+ (X-Account-Meta-Temp-URL-Key-2 str value, ACCOUNT_SCOPE if set,
+ (X-Container-Meta-Temp-URL-Key str value, CONTAINER_SCOPE) if set,
+ (X-Container-Meta-Temp-URL-Key-2 str value, CONTAINER_SCOPE if set,
+ ]
"""
account_info = get_account_info(env, self.app, swift_source='TU')
account_keys = get_tempurl_keys_from_metadata(account_info['meta'])
@@ -443,25 +495,28 @@ class TempURL(object):
container_keys = get_tempurl_keys_from_metadata(
container_info.get('meta', []))
- return account_keys + container_keys
+ return ([(ak, ACCOUNT_SCOPE) for ak in account_keys] +
+ [(ck, CONTAINER_SCOPE) for ck in container_keys])
- def _get_hmacs(self, env, expires, keys, request_method=None):
+ def _get_hmacs(self, env, expires, scoped_keys, request_method=None):
"""
:param env: The WSGI environment for the request.
:param expires: Unix timestamp as an int for when the URL
expires.
- :param keys: Key strings, from the X-Account-Meta-Temp-URL-Key[-2] of
- the account.
+ :param scoped_keys: (key, scope) tuples like _get_keys() returns
:param request_method: Optional override of the request in
the WSGI env. For example, if a HEAD
does not match, you may wish to
override with GET to still allow the
HEAD.
+
+ :returns: a list of (hmac, scope) 2-tuples
"""
if not request_method:
request_method = env['REQUEST_METHOD']
- return [get_hmac(
- request_method, env['PATH_INFO'], expires, key) for key in keys]
+ return [
+ (get_hmac(request_method, env['PATH_INFO'], expires, key), scope)
+ for (key, scope) in scoped_keys]
def _invalid(self, env, start_response):
"""
diff --git a/swift/proxy/server.py b/swift/proxy/server.py
index b631542..8ff4956 100644
--- a/swift/proxy/server.py
+++ b/swift/proxy/server.py
@@ -378,6 +378,7 @@ class Application(object):
allowed_methods = getattr(controller, 'allowed_methods', set())
return HTTPMethodNotAllowed(
request=req, headers={'Allow': ', '.join(allowed_methods)})
+ old_authorize = None
if 'swift.authorize' in req.environ:
# We call authorize before the handler, always. If authorized,
# we remove the swift.authorize hook so isn't ever called
@@ -388,7 +389,7 @@ class Application(object):
if not resp and not req.headers.get('X-Copy-From-Account') \
and not req.headers.get('Destination-Account'):
# No resp means authorized, no delayed recheck required.
- del req.environ['swift.authorize']
+ old_authorize = req.environ['swift.authorize']
else:
# Response indicates denial, but we might delay the denial
# and recheck later. If not delayed, return the error now.
@@ -398,7 +399,13 @@ class Application(object):
# gets mutated during handling. This way logging can display the
# method the client actually sent.
req.environ['swift.orig_req_method'] = req.method
- return handler(req)
+ try:
+ if old_authorize:
+ req.environ.pop('swift.authorize', None)
+ return handler(req)
+ finally:
+ if old_authorize:
+ req.environ['swift.authorize'] = old_authorize
except HTTPException as error_response:
return error_response
except (Exception, Timeout):
diff --git a/test/functional/tests.py b/test/functional/tests.py
index 800d070..1c342f0 100644
--- a/test/functional/tests.py
+++ b/test/functional/tests.py
@@ -2714,6 +2714,59 @@ class TestTempurl(Base):
contents = self.env.obj.read(parms=parms, cfg={'no_auth_token': True})
self.assertEqual(contents, "obj contents")
+ def test_GET_DLO_inside_container(self):
+ seg1 = self.env.container.file(
+ "get-dlo-inside-seg1" + Utils.create_name())
+ seg2 = self.env.container.file(
+ "get-dlo-inside-seg2" + Utils.create_name())
+ seg1.write("one fish two fish ")
+ seg2.write("red fish blue fish")
+
+ manifest = self.env.container.file("manifest" + Utils.create_name())
+ manifest.write(
+ '',
+ hdrs={"X-Object-Manifest": "%s/get-dlo-inside-seg" %
+ (self.env.container.name,)})
+
+ expires = int(time.time()) + 86400
+ sig = self.tempurl_sig(
+ 'GET', expires, self.env.conn.make_path(manifest.path),
+ self.env.tempurl_key)
+ parms = {'temp_url_sig': sig,
+ 'temp_url_expires': str(expires)}
+
+ contents = manifest.read(parms=parms, cfg={'no_auth_token': True})
+ self.assertEqual(contents, "one fish two fish red fish blue fish")
+
+ def test_GET_DLO_outside_container(self):
+ seg1 = self.env.container.file(
+ "get-dlo-outside-seg1" + Utils.create_name())
+ seg2 = self.env.container.file(
+ "get-dlo-outside-seg2" + Utils.create_name())
+ seg1.write("one fish two fish ")
+ seg2.write("red fish blue fish")
+
+ container2 = self.env.account.container(Utils.create_name())
+ container2.create()
+
+ manifest = container2.file("manifest" + Utils.create_name())
+ manifest.write(
+ '',
+ hdrs={"X-Object-Manifest": "%s/get-dlo-outside-seg" %
+ (self.env.container.name,)})
+
+ expires = int(time.time()) + 86400
+ sig = self.tempurl_sig(
+ 'GET', expires, self.env.conn.make_path(manifest.path),
+ self.env.tempurl_key)
+ parms = {'temp_url_sig': sig,
+ 'temp_url_expires': str(expires)}
+
+ # cross container tempurl works fine for account tempurl key
+ contents = manifest.read(parms=parms, cfg={'no_auth_token': True})
+ self.assertEqual(contents, "one fish two fish red fish blue fish")
+ self.assert_status([200])
+
def test_PUT(self):
new_obj = self.env.container.file(Utils.create_name())
@@ -3042,6 +3095,67 @@ class TestContainerTempurl(Base):
'Container TempURL key-2 found, should not be visible '
'to readonly ACLs')
+ def test_GET_DLO_inside_container(self):
+ seg1 = self.env.container.file(
+ "get-dlo-inside-seg1" + Utils.create_name())
+ seg2 = self.env.container.file(
+ "get-dlo-inside-seg2" + Utils.create_name())
+ seg1.write("one fish two fish ")
+ seg2.write("red fish blue fish")
+
+ manifest = self.env.container.file("manifest" + Utils.create_name())
+ manifest.write(
+ '',
+ hdrs={"X-Object-Manifest": "%s/get-dlo-inside-seg" %
+ (self.env.container.name,)})
+
+ expires = int(time.time()) + 86400
+ sig = self.tempurl_sig(
+ 'GET', expires, self.env.conn.make_path(manifest.path),
+ self.env.tempurl_key)
+ parms = {'temp_url_sig': sig,
+ 'temp_url_expires': str(expires)}
+
+ contents = manifest.read(parms=parms, cfg={'no_auth_token': True})
+ self.assertEqual(contents, "one fish two fish red fish blue fish")
+
+ def test_GET_DLO_outside_container(self):
+ container2 = self.env.account.container(Utils.create_name())
+ container2.create()
+ seg1 = container2.file(
+ "get-dlo-outside-seg1" + Utils.create_name())
+ seg2 = container2.file(
+ "get-dlo-outside-seg2" + Utils.create_name())
+ seg1.write("one fish two fish ")
+ seg2.write("red fish blue fish")
+
+ manifest = self.env.container.file("manifest" + Utils.create_name())
+ manifest.write(
+ '',
+ hdrs={"X-Object-Manifest": "%s/get-dlo-outside-seg" %
+ (container2.name,)})
+
+ expires = int(time.time()) + 86400
+ sig = self.tempurl_sig(
+ 'GET', expires, self.env.conn.make_path(manifest.path),
+ self.env.tempurl_key)
+ parms = {'temp_url_sig': sig,
+ 'temp_url_expires': str(expires)}
+
+ # cross container tempurl does not work for container tempurl key
+ try:
+ manifest.read(parms=parms, cfg={'no_auth_token': True})
+ except ResponseError as e:
+ self.assertEqual(e.status, 401)
+ else:
+ self.fail('request did not error')
+ try:
+ manifest.info(parms=parms, cfg={'no_auth_token': True})
+ except ResponseError as e:
+ self.assertEqual(e.status, 401)
+ else:
+ self.fail('request did not error')
+
class TestContainerTempurlUTF8(Base2, TestContainerTempurl):
set_up = False
diff --git a/test/unit/common/middleware/test_tempurl.py b/test/unit/common/middleware/test_tempurl.py
index ba42ace..f153147 100644
--- a/test/unit/common/middleware/test_tempurl.py
+++ b/test/unit/common/middleware/test_tempurl.py
@@ -29,6 +29,7 @@
# limitations under the License.
import hmac
+import itertools
import unittest
from hashlib import sha1
from time import time
@@ -44,10 +45,13 @@ class FakeApp(object):
self.calls = 0
self.status_headers_body_iter = status_headers_body_iter
if not self.status_headers_body_iter:
- self.status_headers_body_iter = iter([('404 Not Found', {
- 'x-test-header-one-a': 'value1',
- 'x-test-header-two-a': 'value2',
- 'x-test-header-two-b': 'value3'}, '')])
+ self.status_headers_body_iter = iter(
+ itertools.repeat((
+ '404 Not Found', {
+ 'x-test-header-one-a': 'value1',
+ 'x-test-header-two-a': 'value2',
+ 'x-test-header-two-b': 'value3'},
+ '')))
self.request = None
def __call__(self, env, start_response):
@@ -69,16 +73,18 @@ class TestTempURL(unittest.TestCase):
self.auth = tempauth.filter_factory({'reseller_prefix': ''})(self.app)
self.tempurl = tempurl.filter_factory({})(self.auth)
- def _make_request(self, path, environ=None, keys=(), **kwargs):
+ def _make_request(self, path, environ=None, keys=(), container_keys=None,
+ **kwargs):
if environ is None:
environ = {}
_junk, account, _junk, _junk = utils.split_path(path, 2, 4)
- self._fake_cache_environ(environ, account, keys)
+ self._fake_cache_environ(environ, account, keys,
+ container_keys=container_keys)
req = Request.blank(path, environ=environ, **kwargs)
return req
- def _fake_cache_environ(self, environ, account, keys):
+ def _fake_cache_environ(self, environ, account, keys, container_keys=None):
"""
Fake out the caching layer for get_account_info(). Injects account data
into environ such that keys are the tempurl keys, if set.
@@ -96,8 +102,13 @@ class TestTempURL(unittest.TestCase):
'bytes': '0',
'meta': meta}
+ meta = {}
+ for i, key in enumerate(container_keys or []):
+ meta_name = 'Temp-URL-key' + (("-%d" % (i + 1) if i else ""))
+ meta[meta_name] = key
+
container_cache_key = 'swift.container/' + account + '/c'
- environ.setdefault(container_cache_key, {'meta': {}})
+ environ.setdefault(container_cache_key, {'meta': meta})
def test_passthrough(self):
resp = self._make_request('/v1/a/c/o').get_response(self.tempurl)
@@ -581,6 +592,81 @@ class TestTempURL(unittest.TestCase):
self.assertTrue('Temp URL invalid' in resp.body)
self.assertTrue('Www-Authenticate' in resp.headers)
+ def test_authorize_limits_scope(self):
+ req_other_object = Request.blank("/v1/a/c/o2")
+ req_other_container = Request.blank("/v1/a/c2/o2")
+ req_other_account = Request.blank("/v1/a2/c2/o2")
+
+ key_kwargs = {
+ 'keys': ['account-key', 'shared-key'],
+ 'container_keys': ['container-key', 'shared-key'],
+ }
+
+ # A request with the account key limits the pre-authed scope to the
+ # account level.
+ method = 'GET'
+ expires = int(time() + 86400)
+ path = '/v1/a/c/o'
+
+ hmac_body = '%s\n%s\n%s' % (method, expires, path)
+ sig = hmac.new('account-key', hmac_body, sha1).hexdigest()
+ qs = '?temp_url_sig=%s&temp_url_expires=%s' % (sig, expires)
+
+ # make request will setup the environ cache for us
+ req = self._make_request(path + qs, **key_kwargs)
+ resp = req.get_response(self.tempurl)
+ self.assertEquals(resp.status_int, 404) # sanity check
+
+ authorize = req.environ['swift.authorize']
+ # Requests for other objects happen if, for example, you're
+ # downloading a large object or creating a large-object manifest.
+ oo_resp = authorize(req_other_object)
+ self.assertEqual(oo_resp, None)
+ oc_resp = authorize(req_other_container)
+ self.assertEqual(oc_resp, None)
+ oa_resp = authorize(req_other_account)
+ self.assertEqual(oa_resp.status_int, 401)
+
+ # A request with the container key limits the pre-authed scope to
+ # the container level; a different container in the same account is
+ # out of scope and thus forbidden.
+ hmac_body = '%s\n%s\n%s' % (method, expires, path)
+ sig = hmac.new('container-key', hmac_body, sha1).hexdigest()
+ qs = '?temp_url_sig=%s&temp_url_expires=%s' % (sig, expires)
+
+ req = self._make_request(path + qs, **key_kwargs)
+ resp = req.get_response(self.tempurl)
+ self.assertEquals(resp.status_int, 404) # sanity check
+
+ authorize = req.environ['swift.authorize']
+ oo_resp = authorize(req_other_object)
+ self.assertEqual(oo_resp, None)
+ oc_resp = authorize(req_other_container)
+ self.assertEqual(oc_resp.status_int, 401)
+ oa_resp = authorize(req_other_account)
+ self.assertEqual(oa_resp.status_int, 401)
+
+ # If account and container share a key (users set these, so this can
+ # happen by accident, stupidity, *or* malice!), limit the scope to
+ # account level. This prevents someone from shrinking the scope of
+ # account-level tempurls by reusing one of the account's keys on a
+ # container.
+ hmac_body = '%s\n%s\n%s' % (method, expires, path)
+ sig = hmac.new('shared-key', hmac_body, sha1).hexdigest()
+ qs = '?temp_url_sig=%s&temp_url_expires=%s' % (sig, expires)
+
+ req = self._make_request(path + qs, **key_kwargs)
+ resp = req.get_response(self.tempurl)
+ self.assertEquals(resp.status_int, 404) # sanity check
+
+ authorize = req.environ['swift.authorize']
+ oo_resp = authorize(req_other_object)
+ self.assertEqual(oo_resp, None)
+ oc_resp = authorize(req_other_container)
+ self.assertEqual(oc_resp, None)
+ oa_resp = authorize(req_other_account)
+ self.assertEqual(oa_resp.status_int, 401)
+
def test_changed_path_invalid(self):
method = 'GET'
expires = int(time() + 86400)
@@ -828,35 +914,38 @@ class TestTempURL(unittest.TestCase):
self.assertTrue('x-conflict-header-test' in resp.headers)
self.assertEqual(resp.headers['x-conflict-header-test'], 'value')
- def test_get_account(self):
- self.assertEquals(self.tempurl._get_account({
- 'REQUEST_METHOD': 'HEAD', 'PATH_INFO': '/v1/a/c/o'}), 'a')
- self.assertEquals(self.tempurl._get_account({
- 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a/c/o'}), 'a')
- self.assertEquals(self.tempurl._get_account({
- 'REQUEST_METHOD': 'PUT', 'PATH_INFO': '/v1/a/c/o'}), 'a')
- self.assertEquals(self.tempurl._get_account({
- 'REQUEST_METHOD': 'POST', 'PATH_INFO': '/v1/a/c/o'}), 'a')
- self.assertEquals(self.tempurl._get_account({
- 'REQUEST_METHOD': 'DELETE', 'PATH_INFO': '/v1/a/c/o'}), 'a')
- self.assertEquals(self.tempurl._get_account({
- 'REQUEST_METHOD': 'UNKNOWN', 'PATH_INFO': '/v1/a/c/o'}), None)
- self.assertEquals(self.tempurl._get_account({
- 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a/c/'}), None)
- self.assertEquals(self.tempurl._get_account({
- 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a/c//////'}), None)
- self.assertEquals(self.tempurl._get_account({
- 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a/c///o///'}), 'a')
- self.assertEquals(self.tempurl._get_account({
- 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a/c'}), None)
- self.assertEquals(self.tempurl._get_account({
- 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a//o'}), None)
- self.assertEquals(self.tempurl._get_account({
- 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1//c/o'}), None)
- self.assertEquals(self.tempurl._get_account({
- 'REQUEST_METHOD': 'GET', 'PATH_INFO': '//a/c/o'}), None)
- self.assertEquals(self.tempurl._get_account({
- 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v2/a/c/o'}), None)
+ def test_get_account_and_container(self):
+ self.assertEquals(self.tempurl._get_account_and_container({
+ 'REQUEST_METHOD': 'HEAD', 'PATH_INFO': '/v1/a/c/o'}), ('a', 'c'))
+ self.assertEquals(self.tempurl._get_account_and_container({
+ 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a/c/o'}), ('a', 'c'))
+ self.assertEquals(self.tempurl._get_account_and_container({
+ 'REQUEST_METHOD': 'PUT', 'PATH_INFO': '/v1/a/c/o'}), ('a', 'c'))
+ self.assertEquals(self.tempurl._get_account_and_container({
+ 'REQUEST_METHOD': 'POST', 'PATH_INFO': '/v1/a/c/o'}), ('a', 'c'))
+ self.assertEquals(self.tempurl._get_account_and_container({
+ 'REQUEST_METHOD': 'DELETE', 'PATH_INFO': '/v1/a/c/o'}), ('a', 'c'))
+ self.assertEquals(self.tempurl._get_account_and_container({
+ 'REQUEST_METHOD': 'UNKNOWN', 'PATH_INFO': '/v1/a/c/o'}),
+ (None, None))
+ self.assertEquals(self.tempurl._get_account_and_container({
+ 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a/c/'}), (None, None))
+ self.assertEquals(self.tempurl._get_account_and_container({
+ 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a/c//////'}),
+ (None, None))
+ self.assertEquals(self.tempurl._get_account_and_container({
+ 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a/c///o///'}),
+ ('a', 'c'))
+ self.assertEquals(self.tempurl._get_account_and_container({
+ 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a/c'}), (None, None))
+ self.assertEquals(self.tempurl._get_account_and_container({
+ 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a//o'}), (None, None))
+ self.assertEquals(self.tempurl._get_account_and_container({
+ 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1//c/o'}), (None, None))
+ self.assertEquals(self.tempurl._get_account_and_container({
+ 'REQUEST_METHOD': 'GET', 'PATH_INFO': '//a/c/o'}), (None, None))
+ self.assertEquals(self.tempurl._get_account_and_container({
+ 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v2/a/c/o'}), (None, None))
def test_get_temp_url_info(self):
s = 'f5d5051bddf5df7e27c628818738334f'
@@ -908,13 +997,13 @@ class TestTempURL(unittest.TestCase):
self.assertEquals(
self.tempurl._get_hmacs(
{'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a/c/o'},
- 1, ['abc']),
- ['026d7f7cc25256450423c7ad03fc9f5ffc1dab6d'])
+ 1, [('abc', 'account')]),
+ [('026d7f7cc25256450423c7ad03fc9f5ffc1dab6d', 'account')])
self.assertEquals(
self.tempurl._get_hmacs(
{'REQUEST_METHOD': 'HEAD', 'PATH_INFO': '/v1/a/c/o'},
- 1, ['abc'], request_method='GET'),
- ['026d7f7cc25256450423c7ad03fc9f5ffc1dab6d'])
+ 1, [('abc', 'account')], request_method='GET'),
+ [('026d7f7cc25256450423c7ad03fc9f5ffc1dab6d', 'account')])
def test_invalid(self):
--
2.4.6
|