)]}'
{"swift/common/middleware/encrypter.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"02adb84893701884a7cc624db8102e6e4004be88","unresolved":false,"context_lines":[{"line_number":27,"context_line":"from swift.common.utils import get_logger, config_true_value"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"EMPTY_ETAG \u003d md5().hexdigest()"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"def encrypt_header_val(crypto, value, key, iv_base\u003dNone):"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aa08908_4613010e","line":30,"updated":"2016-06-09 16:46:11.000000000","message":"I thought we had this defined already somewhere, but looks like it\u0027s over in the unit tests, which may not get packaged. Makes sense that we\u0027d want it here, too.","commit_id":"52480263a89fc22d10d899dd47efe9a893e46d79"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"02adb84893701884a7cc624db8102e6e4004be88","unresolved":false,"context_lines":[{"line_number":148,"context_line":"                # crypto parameters appended. Don\u0027t encrypt an EMPTY_ETAG"},{"line_number":149,"context_line":"                # unless there actually was some body content, in which case"},{"line_number":150,"context_line":"                # the container-listing etag is possibly conveying some"},{"line_number":151,"context_line":"                # non-obvious information."},{"line_number":152,"context_line":"                val, crypto_meta \u003d encrypt_header_val("},{"line_number":153,"context_line":"                    self.crypto, container_listing_etag,"},{"line_number":154,"context_line":"                    self.keys[\u0027container\u0027],"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aa08908_26e87516","line":151,"updated":"2016-06-09 16:46:11.000000000","message":"So if I later implement the SLO-etag fix I\u0027ve been thinking about... and someone uploads a manifest of \"[]\"... the container listing etag would be EMPTY_ETAG, but we\u0027d still encrypt container_listing_etag because there\u0027d be a plaintext_etag. That seems bad -- the content-type would say (via swift_bytes) that the object is an empty SLO, so an attacker would know the plaintext was EMPTY_ETAG.\n\nI don\u0027t like the idea of *just* checking for EMPTY_ETAG, though, in case someone uploads something whose hash happens to collide with EMPTY_ETAG. I was thinking about having us check container-update-override-size instead, but that doesn\u0027t really help the SLO case. Maybe we do that + make this aware of swift_bytes in content-type?\n\nOr maybe it\u0027s not practical for me to do the SLO-etag fix. It just *really* feels weird that there\u0027s this etag mismatch between container listing and object GET/HEAD :-/","commit_id":"52480263a89fc22d10d899dd47efe9a893e46d79"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"68a366cea779b7c7468bf1a3468343281fcb58fc","unresolved":false,"context_lines":[{"line_number":148,"context_line":"                # crypto parameters appended. Don\u0027t encrypt an EMPTY_ETAG"},{"line_number":149,"context_line":"                # unless there actually was some body content, in which case"},{"line_number":150,"context_line":"                # the container-listing etag is possibly conveying some"},{"line_number":151,"context_line":"                # non-obvious information."},{"line_number":152,"context_line":"                val, crypto_meta \u003d encrypt_header_val("},{"line_number":153,"context_line":"                    self.crypto, container_listing_etag,"},{"line_number":154,"context_line":"                    self.keys[\u0027container\u0027],"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aa08908_914845ce","line":151,"in_reply_to":"7aa08908_26e87516","updated":"2016-06-16 16:02:17.000000000","message":"hmm, nice catch - so you could infer that the plaintext was EMPTY_ETAG and you know also the IV :(\nWe could learn about swift_bytes here - it is pretty pervasive - proxy, obj and container servers already know about it.\n\nIdeally swift_bytes would be a header (a new style override header even) but there\u0027s a whole mess of backwards compatibility around changing it, given that old-style headers could be lingering in async pending files etc","commit_id":"52480263a89fc22d10d899dd47efe9a893e46d79"}],"test/unit/common/middleware/test_encrypter.py":[{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"222765b095985abafc1cdea2d2e269c5f0efdde3","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        self.assertNotIn(\u0027X-Object-Sysmeta-Crypto-Etag\u0027, req_hdrs)"},{"line_number":187,"context_line":"        # verify there is no container update override for etag"},{"line_number":188,"context_line":"        self.assertNotIn("},{"line_number":189,"context_line":"            \u0027X-Object-Sysmeta-Container-Update-Override-Etag\u0027, req_hdrs)"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        # user meta is still encrypted"},{"line_number":192,"context_line":"        self.assertEqual("}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_76d83980","line":189,"updated":"2016-07-09 19:31:58.000000000","message":"This assertion and behavior still exists on master: an \"empty etag\" is not encrypted, and there is no container override.  The only case we are talking about here is when another middleware container override was set in footers and is supposed to override the value used.  Let me know if that is not correct.\n\nI added these tests onto master locally to verify that we would still encrypt such an override, although it was an empty etag.  It seems that we still do.  So, it is still an inconsistency (even though its an empty etag, we perform all the encryption anyway in this rare case), but I agree that there is no security risk since the iv is now unique.  \n\nI could live with this possible inconsistency since it is a case that may never happen anyway.","commit_id":"52480263a89fc22d10d899dd47efe9a893e46d79"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"222765b095985abafc1cdea2d2e269c5f0efdde3","unresolved":false,"context_lines":[{"line_number":449,"context_line":"            \u0027Etag\u0027: EMPTY_ETAG,"},{"line_number":450,"context_line":"            \u0027X-Object-Sysmeta-Other\u0027: \u0027other sysmeta\u0027,"},{"line_number":451,"context_line":"            \u0027X-Object-Sysmeta-Container-Update-Override-Etag\u0027:"},{"line_number":452,"context_line":"                \u0027other override\u0027}"},{"line_number":453,"context_line":"        env.update({\u0027swift.callback.update_footers\u0027:"},{"line_number":454,"context_line":"                    lambda footers: footers.update(other_footers)})"},{"line_number":455,"context_line":"        req \u003d Request.blank(\u0027/v1/a/c/o\u0027, environ\u003denv, body\u003d\u0027\u0027, headers\u003dhdrs)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_b6b90118","line":452,"updated":"2016-07-09 19:31:58.000000000","message":"On master, we are still using an etag that doesn\u0027t match a zero size body scenario, and the \u0027Backend\u0027 value is set.  If it was intended to test the sysmeta setting, then this test fails in the assertion later because it is encrypted.  the etag value would have to be popped and not checked.","commit_id":"52480263a89fc22d10d899dd47efe9a893e46d79"}]}
