)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"50bbf46e7cc12d844e8d2636fb5ad9680ec0f946","unresolved":false,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Currently, the object key cannot change for user-metadata"},{"line_number":21,"context_line":"on a POST.  So for now, this verification also applies to"},{"line_number":22,"context_line":"the key used for user-metadata.  However, if in the future"},{"line_number":23,"context_line":"a POST can use a different key for encryption, then a"},{"line_number":24,"context_line":"different verification would be required."},{"line_number":25,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7f231b9d_a1cb87a3","line":22,"updated":"2017-06-11 21:52:44.000000000","message":"Well, that is already incorrect :-). As this is coded, the check isn\u0027t made unless the body needs to be decrypted for the request.  At first, I had in mind that the user-metadata might have a new persisted value to validate against.  But I have since decided to have this patch only involve checks against the etag HMAC. So, I will apply that the check to the header decryption as well.  I will try to do this in a way that results in the check only being done once if any decryption is necessary among headers or body, and none if there is no decryption.","commit_id":"95770687f7154b2867ef6b43cdc595224bf86a27"}],"swift/common/middleware/crypto/decrypter.py":[{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"50bbf46e7cc12d844e8d2636fb5ad9680ec0f946","unresolved":false,"context_lines":[{"line_number":305,"context_line":"                    \u0027X-Object-Sysmeta-Crypto-Etag-Mac\u0027):"},{"line_number":306,"context_line":"                self.verify_wrapping_key(keys[\u0027object\u0027], decrypted_etag,"},{"line_number":307,"context_line":"                                         self._response_header_value("},{"line_number":308,"context_line":"                                         \u0027X-Object-Sysmeta-Crypto-Etag-Mac\u0027))"},{"line_number":309,"context_line":"            else:"},{"line_number":310,"context_line":"                self.logger.error("},{"line_number":311,"context_line":"                    _(\u0027Missing ETag or ETag HMAC for %(resp_type)s\u0027),"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f231b9d_21bf77fc","line":308,"updated":"2017-06-11 21:52:44.000000000","message":"The call to the verify method might move up earlier in processing the GET (see comments made to commit message).  I would also change the method name in that case.","commit_id":"95770687f7154b2867ef6b43cdc595224bf86a27"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"6de7fadb45f3c334afc9679be940daba442cb260","unresolved":false,"context_lines":[{"line_number":312,"context_line":"                    {\u0027resp_type\u0027: self.server_type})"},{"line_number":313,"context_line":"                raise HTTPInternalServerError("},{"line_number":314,"context_line":"                    body\u003d\u0027Unable to verify encryption key\u0027,"},{"line_number":315,"context_line":"                    content_type\u003d\u0027text/plain\u0027)"},{"line_number":316,"context_line":""},{"line_number":317,"context_line":"            # 2xx response and encrypted body"},{"line_number":318,"context_line":"            body_key \u003d self.get_unwrapped_key(crypto_meta, keys[\u0027object\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f231b9d_eeef2e85","line":315,"updated":"2017-06-11 20:09:19.000000000","message":"I will be working on a unit test to hit this case.","commit_id":"95770687f7154b2867ef6b43cdc595224bf86a27"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"cfe2468c58afd4b9f43f59f12402a1ef85a58c30","unresolved":false,"context_lines":[{"line_number":325,"context_line":"            # don\u0027t decrypt body of unencrypted or non-2xx responses"},{"line_number":326,"context_line":"            resp_iter \u003d app_resp"},{"line_number":327,"context_line":"            self.logger.warning("},{"line_number":328,"context_line":"                \u0027No ETag HMAC available for verification of object key\u0027)"},{"line_number":329,"context_line":""},{"line_number":330,"context_line":"        mod_resp_headers \u003d purge_crypto_sysmeta_headers(mod_resp_headers)"},{"line_number":331,"context_line":"        start_response(self._response_status, mod_resp_headers,"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f231b9d_62640a23","line":328,"updated":"2017-06-13 17:16:06.000000000","message":"I know this warning will happen too frequently left like this.  I\u0027d like a nice way to detect whether or not any metadata indeed needed decrypting, so that the warning will only happen if there was some header decrypting with a key.  I\u0027ll be working on that, but any suggestions for how to do this in a \u0027nice\u0027 way would be welcome :-)\n\nAlong those lines, when this is fixed, in the related test case for no decryption happening on a GET, it needs to be verified that the warning does not get logged.","commit_id":"716334abf35a07279f1fc694b9df006d77aaa5f5"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4545d5f3d2622ea3ad59398336c4b448645bb509","unresolved":false,"context_lines":[{"line_number":116,"context_line":"                \"Missing crypto meta in value %s\" % value)"},{"line_number":117,"context_line":"        return value"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    def decrypt_value(self, value, key, crypto_meta, keys_to_verify\u003d[]):"},{"line_number":120,"context_line":"        \"\"\""},{"line_number":121,"context_line":"        Base64-decode and decrypt a value using the crypto_meta provided."},{"line_number":122,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7f287b81_543cc02d","line":119,"range":{"start_line":119,"start_character":68,"end_line":119,"end_character":70},"updated":"2017-08-30 20:12:57.000000000","message":"This turns into a memory leak:\n\n \u003e\u003e\u003e class A(object):\n ...     def foo(self, val, arr\u003d[]):\n ...         arr.append(val)\n ...         return val, arr\n ...\n \u003e\u003e\u003e x \u003d A()\n \u003e\u003e\u003e x.foo(1234)\n (1234, [1234])\n \u003e\u003e\u003e x.foo(\u0027asdf\u0027)\n (\u0027asdf\u0027, [1234, \u0027asdf\u0027])\n\nIt\u0027s even shared across instances:\n\n \u003e\u003e\u003e y \u003d A()\n \u003e\u003e\u003e y.foo({})\n ({}, [1234, \u0027asdf\u0027, {}])\n\nBetter to go with a default of None and have a\n\n if keys_to_verify is not None:\n\nbefore the append. We\u0027ll need to do similar changes for decrypt_value_with_meta above and _decrypt_header below.","commit_id":"f00017292aef76da3a42cc70101ba78770ed7a4f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4545d5f3d2622ea3ad59398336c4b448645bb509","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        \"\"\""},{"line_number":179,"context_line":"        try:"},{"line_number":180,"context_line":"            return self.decrypt_value_with_meta(value, key, required\u003drequired,"},{"line_number":181,"context_line":"                                                keys_to_verify\u003dkeys_to_verify)"},{"line_number":182,"context_line":"        except EncryptionException as err:"},{"line_number":183,"context_line":"            self.logger.error("},{"line_number":184,"context_line":"                _(\"Error decrypting header %(header)s: %(error)s\"),"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f287b81_d49af011","line":181,"range":{"start_line":181,"start_character":63,"end_line":181,"end_character":77},"updated":"2017-08-30 20:12:57.000000000","message":"Would we ever want something *besides* self.keys_to_verify here? Would it be better to avoid the signature change?","commit_id":"f00017292aef76da3a42cc70101ba78770ed7a4f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4545d5f3d2622ea3ad59398336c4b448645bb509","unresolved":false,"context_lines":[{"line_number":343,"context_line":"            # so no checks are made on which key was needing verification."},{"line_number":344,"context_line":"            if self.keys_to_verify:"},{"line_number":345,"context_line":"                self.logger.warning("},{"line_number":346,"context_line":"                    \u0027No ETag HMAC available for verification of object key\u0027)"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"        mod_resp_headers \u003d purge_crypto_sysmeta_headers(mod_resp_headers)"},{"line_number":349,"context_line":"        start_response(self._response_status, mod_resp_headers,"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f287b81_b4440475","line":346,"updated":"2017-08-30 20:12:57.000000000","message":"This still seems noisy -- won\u0027t it always come up if you turn on encryption in an existing cluster and someone POSTs new meta to an existing object? Or, even if encryption has *always* been on, when someone uploads a zero-byte object?","commit_id":"f00017292aef76da3a42cc70101ba78770ed7a4f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4545d5f3d2622ea3ad59398336c4b448645bb509","unresolved":false,"context_lines":[{"line_number":381,"context_line":"                          required\u003dTrue):"},{"line_number":382,"context_line":"        # assumes the etag should have been HMAC\u0027ed with the object_key"},{"line_number":383,"context_line":""},{"line_number":384,"context_line":"        if plaintext_etag is None:"},{"line_number":385,"context_line":"            if required:"},{"line_number":386,"context_line":"                self.logger.error("},{"line_number":387,"context_line":"                    \u0027Missing ETag for verification of object key\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f287b81_0f6741b9","line":384,"updated":"2017-08-30 20:12:57.000000000","message":"I\u0027m struggling to think of a way for this to trigger -- is it just CYA?","commit_id":"f00017292aef76da3a42cc70101ba78770ed7a4f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4545d5f3d2622ea3ad59398336c4b448645bb509","unresolved":false,"context_lines":[{"line_number":402,"context_line":"                    content_type\u003d\u0027text/plain\u0027)"},{"line_number":403,"context_line":"            else:"},{"line_number":404,"context_line":"                self.logger.warning("},{"line_number":405,"context_line":"                    \u0027No ETag HMAC available for verification of object key\u0027)"},{"line_number":406,"context_line":"            return"},{"line_number":407,"context_line":""},{"line_number":408,"context_line":"        # calculate the MAC of the result using object_key"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f287b81_cf3c9994","line":405,"updated":"2017-08-30 20:12:57.000000000","message":"As in handle_get, this seems noisy.","commit_id":"f00017292aef76da3a42cc70101ba78770ed7a4f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4545d5f3d2622ea3ad59398336c4b448645bb509","unresolved":false,"context_lines":[{"line_number":413,"context_line":"            self.logger.error(\u0027Incorrect object key\u0027)"},{"line_number":414,"context_line":"            raise HTTPInternalServerError("},{"line_number":415,"context_line":"                body\u003d\u0027Incorrect object key\u0027, content_type\u003d\u0027text/plain\u0027)"},{"line_number":416,"context_line":"        self.object_key_verified \u003d True"},{"line_number":417,"context_line":""},{"line_number":418,"context_line":""},{"line_number":419,"context_line":"class DecrypterContContext(BaseDecrypterContext):"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f287b81_cfa019f7","line":416,"range":{"start_line":416,"start_character":13,"end_line":416,"end_character":32},"updated":"2017-08-30 20:12:57.000000000","message":"Do we use this anywhere?","commit_id":"f00017292aef76da3a42cc70101ba78770ed7a4f"}],"test/unit/common/middleware/crypto/crypto_helpers.py":[{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"50bbf46e7cc12d844e8d2636fb5ad9680ec0f946","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    return dec_val"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"def hmac_etag(key, etag):"},{"line_number":47,"context_line":"    \"\"\""},{"line_number":48,"context_line":"    Compute an HMAC-SHA256 using given key and etag."},{"line_number":49,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7f231b9d_01bc7b03","line":46,"updated":"2017-06-11 21:52:44.000000000","message":"The tests could use this method from the crypto_utils, I suppose.","commit_id":"95770687f7154b2867ef6b43cdc595224bf86a27"}],"test/unit/common/middleware/crypto/test_decrypter.py":[{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"50bbf46e7cc12d844e8d2636fb5ad9680ec0f946","unresolved":false,"context_lines":[{"line_number":295,"context_line":"        resp \u003d self._test_bad_key(\u0027HEAD\u0027)"},{"line_number":296,"context_line":"        self.assertEqual(\u0027500 Internal Error\u0027, resp.status)"},{"line_number":297,"context_line":"        self.assertIn(\"Bad key for \u0027object\u0027\","},{"line_number":298,"context_line":"                      self.decrypter.logger.get_lines_for_level(\u0027error\u0027)[0])"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    def test_GET_with_bad_key(self):"},{"line_number":301,"context_line":"        resp \u003d self._test_bad_key(\u0027GET\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f231b9d_c13b239a","line":298,"updated":"2017-06-11 21:52:44.000000000","message":"Again, I am thinking that the check should also be applied even if only headers need decrypting.  So, I would add a test_HEAD_with_incorrect_key to test that verification is happening.","commit_id":"95770687f7154b2867ef6b43cdc595224bf86a27"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"cfe2468c58afd4b9f43f59f12402a1ef85a58c30","unresolved":false,"context_lines":[{"line_number":297,"context_line":"                    \u0027x-object-transient-sysmeta-crypto-meta-test\u0027:"},{"line_number":298,"context_line":"                        base64.b64encode(encrypt(\u0027encrypt me\u0027, key, FAKE_IV)) +"},{"line_number":299,"context_line":"                        \u0027;swift_meta\u003d\u0027 + get_crypto_meta_header(),"},{"line_number":300,"context_line":"                    \u0027x-object-sysmeta-test\u0027: \u0027do not encrypt me\u0027}"},{"line_number":301,"context_line":"        if not encrypted_hdrs:"},{"line_number":302,"context_line":"            hdrs.pop(\u0027x-object-transient-sysmeta-crypto-meta-test\u0027)"},{"line_number":303,"context_line":"            hdrs[\u0027x-object-meta-test\u0027] \u003d \u0027plaintext not encrypted\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f231b9d_e225bac3","line":300,"updated":"2017-06-13 17:16:06.000000000","message":"probably could create a _make_response_headers_body_not_encrypted type of method for this and the other places this type of thing is done.  (similarly for the metadata not encrypted hdrs)","commit_id":"716334abf35a07279f1fc694b9df006d77aaa5f5"}]}
