)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b6ddfc3d64605f11389e85b789c4f334664127bc","unresolved":false,"context_lines":[{"line_number":10,"context_line":"with keymaster provided object key and persist wrapped key in"},{"line_number":11,"context_line":"object sysmeta header for crypto meta."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Wrap using AES CTR mode with IV, not aes_wrap_key algorithm,"},{"line_number":14,"context_line":"for performance."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: I4c2ba12dd35bb3f4951dce65bc7f488a332e0c18"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"dab17558_af71cc79","line":13,"range":{"start_line":13,"start_character":15,"end_line":13,"end_character":18},"updated":"2016-05-12 22:08:10.000000000","message":"Since the body key is random, is there any downside to using ECB? It\u0027d be one less thing to store (no IV), and we already know everything will line up on block boundaries.\n\nOn the other hand, there\u0027s definitely something to be said for using a single mode throughout...","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c8cfec4e27bc7754816fbd92f72a14b3a8713219","unresolved":false,"context_lines":[{"line_number":10,"context_line":"with keymaster provided object key and persist wrapped key in"},{"line_number":11,"context_line":"object sysmeta header for crypto meta."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Wrap using AES CTR mode with IV, not aes_wrap_key algorithm,"},{"line_number":14,"context_line":"for performance."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: I4c2ba12dd35bb3f4951dce65bc7f488a332e0c18"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"dab17558_17372fb1","line":13,"range":{"start_line":13,"start_character":15,"end_line":13,"end_character":18},"in_reply_to":"dab17558_af71cc79","updated":"2016-05-13 12:54:28.000000000","message":"That crossed my mind too. I added a trello card to discuss this.\n\nhttps://trello.com/c/GH375UNr/99-check-could-we-use-ecb-cipher-mode-when-wrapping-keys","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"}],"swift/common/crypto_utils.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"cf798761f06094a7141cb54873f182e0a24c0947","unresolved":false,"context_lines":[{"line_number":74,"context_line":""},{"line_number":75,"context_line":"        return keys"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"def dump_crypto_meta(crypto_meta):"},{"line_number":79,"context_line":"    \"\"\""},{"line_number":80,"context_line":"    Serialize crypto meta to a form suitable for including in a header value."}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_09ebd4b1","line":77,"updated":"2016-05-12 10:51:36.000000000","message":"fixed the doc strings as per comment on patchset 1 - the functions moved to this module","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7c080b82590ad96adc834663378eeb36ca01f9ad","unresolved":false,"context_lines":[{"line_number":91,"context_line":"    return urllib.quote_plus(json.dumps({"},{"line_number":92,"context_line":"        name: (base64.b64encode(value).decode() if name in (\u0027iv\u0027, \u0027key\u0027)"},{"line_number":93,"context_line":"               else value)"},{"line_number":94,"context_line":"        for name, value in crypto_meta.items()}, sort_keys\u003dTrue))"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"def load_crypto_meta(value):"}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_17356eda","line":94,"updated":"2016-05-13 12:54:35.000000000","message":"Tim suggested this before - sort_keys makes testing simpler, particularly now there are possibly 3 keys in the dict making 6 possible key orders.\n\nNo significant performance overhead AFAICT","commit_id":"202ea07c3c85418e547fafae3105e1f3d119fdff"},{"author":{"_account_id":9625,"name":"Thiago da Silva","email":"thiagodasilva@gmail.com","username":"thiago"},"change_message_id":"8d4e2c7e900896908fe8592c0bbc8ab955bda048","unresolved":false,"context_lines":[{"line_number":91,"context_line":"    return urllib.quote_plus(json.dumps({"},{"line_number":92,"context_line":"        name: (base64.b64encode(value).decode() if name in (\u0027iv\u0027, \u0027key\u0027)"},{"line_number":93,"context_line":"               else value)"},{"line_number":94,"context_line":"        for name, value in crypto_meta.items()}, sort_keys\u003dTrue))"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"def load_crypto_meta(value):"}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_808e2dce","line":94,"in_reply_to":"dab17558_17356eda","updated":"2016-05-13 15:40:09.000000000","message":"can you please add the reasoning as comment? might help in the future when someone wonders \"why are we sorting these keys?\"","commit_id":"202ea07c3c85418e547fafae3105e1f3d119fdff"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"28e748ddc09d062d7624df346bf0194dc95a2f78","unresolved":false,"context_lines":[{"line_number":91,"context_line":"    return urllib.quote_plus(json.dumps({"},{"line_number":92,"context_line":"        name: (base64.b64encode(value).decode() if name in (\u0027iv\u0027, \u0027key\u0027)"},{"line_number":93,"context_line":"               else value)"},{"line_number":94,"context_line":"        for name, value in crypto_meta.items()}, sort_keys\u003dTrue))"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"def load_crypto_meta(value):"}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_4f02ae3f","line":94,"in_reply_to":"dab17558_808e2dce","updated":"2016-05-13 17:07:05.000000000","message":"Done","commit_id":"202ea07c3c85418e547fafae3105e1f3d119fdff"}],"swift/common/middleware/crypto.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b6ddfc3d64605f11389e85b789c4f334664127bc","unresolved":false,"context_lines":[{"line_number":85,"context_line":"    def _get_derived_iv(self, base):"},{"line_number":86,"context_line":"        target_length \u003d self.get_required_iv_length()"},{"line_number":87,"context_line":"        if len(base) \u003c target_length:"},{"line_number":88,"context_line":"            return base.zfill(target_length)"},{"line_number":89,"context_line":"        elif len(base) \u003e target_length:"},{"line_number":90,"context_line":"            hash \u003d md5()"},{"line_number":91,"context_line":"            hash.update(base)"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_af242cc6","line":88,"range":{"start_line":88,"start_character":24,"end_line":88,"end_character":29},"updated":"2016-05-12 22:08:10.000000000","message":"TIL about zfill.\n\nOff-topic: Any reason to prefer \u00270\u0027 over \u0027\\x00\u0027, or padding on the left instead of the right?","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c8cfec4e27bc7754816fbd92f72a14b3a8713219","unresolved":false,"context_lines":[{"line_number":85,"context_line":"    def _get_derived_iv(self, base):"},{"line_number":86,"context_line":"        target_length \u003d self.get_required_iv_length()"},{"line_number":87,"context_line":"        if len(base) \u003c target_length:"},{"line_number":88,"context_line":"            return base.zfill(target_length)"},{"line_number":89,"context_line":"        elif len(base) \u003e target_length:"},{"line_number":90,"context_line":"            hash \u003d md5()"},{"line_number":91,"context_line":"            hash.update(base)"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_d27b02f3","line":88,"range":{"start_line":88,"start_character":24,"end_line":88,"end_character":29},"in_reply_to":"dab17558_af242cc6","updated":"2016-05-13 12:54:28.000000000","message":"Not sure. we have trello task to add justification/explanation comment on this method.\n\nhttps://trello.com/c/Ib7A3iFU/82-add-justification-for-derived-iv-for-etag-as-a-comment-in-code","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"6b9dc036ea38d2fe7e32eec921e23e8237758a73","unresolved":false,"context_lines":[{"line_number":85,"context_line":"    def _get_derived_iv(self, base):"},{"line_number":86,"context_line":"        target_length \u003d self.get_required_iv_length()"},{"line_number":87,"context_line":"        if len(base) \u003c target_length:"},{"line_number":88,"context_line":"            return base.zfill(target_length)"},{"line_number":89,"context_line":"        elif len(base) \u003e target_length:"},{"line_number":90,"context_line":"            hash \u003d md5()"},{"line_number":91,"context_line":"            hash.update(base)"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_9c595a5e","line":88,"range":{"start_line":88,"start_character":24,"end_line":88,"end_character":29},"in_reply_to":"dab17558_d27b02f3","updated":"2016-05-15 03:22:41.000000000","message":"I dont think that the padding or what type of pad matters in this case.  We just need an iv that is as unique as the path is.","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b6ddfc3d64605f11389e85b789c4f334664127bc","unresolved":false,"context_lines":[{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    def create_random_key(self):"},{"line_number":132,"context_line":"        # helper method to create random key of correct length"},{"line_number":133,"context_line":"        return os.urandom(32)"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    def wrap_key(self, wrapping_key, key_to_wrap, iv):"},{"line_number":136,"context_line":"        # we don\u0027t use an aes_wrap_key because it\u0027s slower and we have"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_6f8244ca","line":133,"range":{"start_line":133,"start_character":26,"end_line":133,"end_character":28},"updated":"2016-05-12 22:08:10.000000000","message":"I feel like we should make this a constant somewhere. Maybe as\n\n max(algorithms.AES.key_sizes) / 8\n\n?","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c8cfec4e27bc7754816fbd92f72a14b3a8713219","unresolved":false,"context_lines":[{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    def create_random_key(self):"},{"line_number":132,"context_line":"        # helper method to create random key of correct length"},{"line_number":133,"context_line":"        return os.urandom(32)"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    def wrap_key(self, wrapping_key, key_to_wrap, iv):"},{"line_number":136,"context_line":"        # we don\u0027t use an aes_wrap_key because it\u0027s slower and we have"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_728976ca","line":133,"range":{"start_line":133,"start_character":26,"end_line":133,"end_character":28},"in_reply_to":"dab17558_6f8244ca","updated":"2016-05-13 12:54:28.000000000","message":"Yes, I agree, but I\u0027d prefer not to depend on the key_sizes never changing, just hard code 32.\n\nAlso I won\u0027t change the tests to use the constant - that way tests will fail if someone changes the constant.","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"810759f610ede6846b89d0bbd3a21cd930b6093d","unresolved":false,"context_lines":[{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    def create_random_key(self):"},{"line_number":132,"context_line":"        # helper method to create random key of correct length"},{"line_number":133,"context_line":"        return os.urandom(32)"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    def wrap_key(self, wrapping_key, key_to_wrap, iv):"},{"line_number":136,"context_line":"        # we don\u0027t use an aes_wrap_key because it\u0027s slower and we have"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_e8e27700","line":133,"range":{"start_line":133,"start_character":26,"end_line":133,"end_character":28},"in_reply_to":"dab17558_728976ca","updated":"2016-05-13 18:22:23.000000000","message":"\u003e I\u0027d prefer not to depend on the key_sizes never changing, just hard code 32.\n\nFair enough.\n\n\u003e I won\u0027t change the tests to use the constant\n\nI like that plan.","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b6ddfc3d64605f11389e85b789c4f334664127bc","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        return os.urandom(32)"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    def wrap_key(self, wrapping_key, key_to_wrap, iv):"},{"line_number":136,"context_line":"        # we don\u0027t use an aes_wrap_key because it\u0027s slower and we have"},{"line_number":137,"context_line":"        # iv material readily available so don\u0027t need a deterministic algorithm"},{"line_number":138,"context_line":"        encryptor \u003d Cipher(algorithms.AES(wrapping_key), modes.CTR(iv),"},{"line_number":139,"context_line":"                           backend\u003ddefault_backend()).encryptor()"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_ea19027b","line":136,"range":{"start_line":136,"start_character":26,"end_line":136,"end_character":38},"updated":"2016-05-12 22:08:10.000000000","message":"TIL about RFC 3394 - and yeah, if we can avoid 12x as many encrypt/decrypt operations, so much the better.","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c8cfec4e27bc7754816fbd92f72a14b3a8713219","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        return os.urandom(32)"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    def wrap_key(self, wrapping_key, key_to_wrap, iv):"},{"line_number":136,"context_line":"        # we don\u0027t use an aes_wrap_key because it\u0027s slower and we have"},{"line_number":137,"context_line":"        # iv material readily available so don\u0027t need a deterministic algorithm"},{"line_number":138,"context_line":"        encryptor \u003d Cipher(algorithms.AES(wrapping_key), modes.CTR(iv),"},{"line_number":139,"context_line":"                           backend\u003ddefault_backend()).encryptor()"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_1deabbac","line":136,"range":{"start_line":136,"start_character":26,"end_line":136,"end_character":38},"in_reply_to":"dab17558_ea19027b","updated":"2016-05-13 12:54:28.000000000","message":"I\u0027ll add ref to the rfc in the comment","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b6ddfc3d64605f11389e85b789c4f334664127bc","unresolved":false,"context_lines":[{"line_number":143,"context_line":"        decryptor \u003d Cipher(algorithms.AES(wrapping_key), modes.CTR(iv),"},{"line_number":144,"context_line":"                           backend\u003ddefault_backend()).decryptor()"},{"line_number":145,"context_line":"        key \u003d decryptor.update(wrapped_key)"},{"line_number":146,"context_line":"        self.check_key(key)"},{"line_number":147,"context_line":"        return key"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_8ad036cb","line":146,"updated":"2016-05-12 22:08:10.000000000","message":"We could even do this check *before* decrypting, no?","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c8cfec4e27bc7754816fbd92f72a14b3a8713219","unresolved":false,"context_lines":[{"line_number":143,"context_line":"        decryptor \u003d Cipher(algorithms.AES(wrapping_key), modes.CTR(iv),"},{"line_number":144,"context_line":"                           backend\u003ddefault_backend()).decryptor()"},{"line_number":145,"context_line":"        key \u003d decryptor.update(wrapped_key)"},{"line_number":146,"context_line":"        self.check_key(key)"},{"line_number":147,"context_line":"        return key"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_72021640","line":146,"in_reply_to":"dab17558_8ad036cb","updated":"2016-05-13 12:54:28.000000000","message":"I like that suggestion.","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7c080b82590ad96adc834663378eeb36ca01f9ad","unresolved":false,"context_lines":[{"line_number":147,"context_line":"        key \u003d decryptor.update(wrapped_key)"},{"line_number":148,"context_line":"        return key"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"    def check_key(self, key):"},{"line_number":151,"context_line":"        if len(key) !\u003d KEY_LENGTH:"},{"line_number":152,"context_line":"            raise ValueError(\"Key must be length %s bytes\" % KEY_LENGTH)"},{"line_number":153,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_f7a42a09","line":150,"updated":"2016-05-13 12:54:35.000000000","message":"relocated method closer to other key related methods","commit_id":"202ea07c3c85418e547fafae3105e1f3d119fdff"}],"swift/common/middleware/decrypter.py":[{"author":{"_account_id":9625,"name":"Thiago da Silva","email":"thiagodasilva@gmail.com","username":"thiago"},"change_message_id":"4a6307ace243bfddde636261b92b2b37d1397997","unresolved":false,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    Note that json.loads always produces unicode strings, to ensure the"},{"line_number":43,"context_line":"    resultant crypto_meta matches the original object cast all key and value"},{"line_number":44,"context_line":"    data (other then the iv) to a str. This will work in py3 as well where all"},{"line_number":45,"context_line":"    strings are unicode implying the cast is effectively a no-op."},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    :param value: a string serialization of a crypto meta dict"}],"source_content_type":"text/x-python","patch_set":1,"id":"dab17558_a375a432","line":44,"range":{"start_line":44,"start_character":10,"end_line":44,"end_character":27},"updated":"2016-05-12 00:16:42.000000000","message":"iv and key","commit_id":"a962d4d7046fbef35a6f0eeeb4e7a6c340ba0ec3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"6466b0910138c4410516bf7e239a71a1de7f0b2e","unresolved":false,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    Note that json.loads always produces unicode strings, to ensure the"},{"line_number":43,"context_line":"    resultant crypto_meta matches the original object cast all key and value"},{"line_number":44,"context_line":"    data (other then the iv) to a str. This will work in py3 as well where all"},{"line_number":45,"context_line":"    strings are unicode implying the cast is effectively a no-op."},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    :param value: a string serialization of a crypto meta dict"}],"source_content_type":"text/x-python","patch_set":1,"id":"dab17558_5ac4895e","line":44,"range":{"start_line":44,"start_character":10,"end_line":44,"end_character":27},"in_reply_to":"dab17558_a375a432","updated":"2016-05-12 10:51:56.000000000","message":"Done","commit_id":"a962d4d7046fbef35a6f0eeeb4e7a6c340ba0ec3"},{"author":{"_account_id":9625,"name":"Thiago da Silva","email":"thiagodasilva@gmail.com","username":"thiago"},"change_message_id":"4a6307ace243bfddde636261b92b2b37d1397997","unresolved":false,"context_lines":[{"line_number":227,"context_line":"                                              body_crypto_meta[\u0027key\u0027],"},{"line_number":228,"context_line":"                                              body_crypto_meta[\u0027iv\u0027])"},{"line_number":229,"context_line":"            return self.crypto.create_decryption_ctxt("},{"line_number":230,"context_line":"                body_key, body_crypto_meta.get(\u0027iv\u0027), offset)"},{"line_number":231,"context_line":"        except (KeyError, ValueError, EncryptionException) as err:"},{"line_number":232,"context_line":"            msg \u003d \u0027Error creating decryption context for object body\u0027"},{"line_number":233,"context_line":"            self.logger.error(\u0027%s: %s\u0027 % (msg, str(err)))"}],"source_content_type":"text/x-python","patch_set":1,"id":"dab17558_83ece0bb","line":230,"range":{"start_line":230,"start_character":26,"end_line":230,"end_character":52},"updated":"2016-05-12 00:16:42.000000000","message":"why use .get(\u0027iv\u0027) here and not when calling unwrap_key?","commit_id":"a962d4d7046fbef35a6f0eeeb4e7a6c340ba0ec3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"6466b0910138c4410516bf7e239a71a1de7f0b2e","unresolved":false,"context_lines":[{"line_number":227,"context_line":"                                              body_crypto_meta[\u0027key\u0027],"},{"line_number":228,"context_line":"                                              body_crypto_meta[\u0027iv\u0027])"},{"line_number":229,"context_line":"            return self.crypto.create_decryption_ctxt("},{"line_number":230,"context_line":"                body_key, body_crypto_meta.get(\u0027iv\u0027), offset)"},{"line_number":231,"context_line":"        except (KeyError, ValueError, EncryptionException) as err:"},{"line_number":232,"context_line":"            msg \u003d \u0027Error creating decryption context for object body\u0027"},{"line_number":233,"context_line":"            self.logger.error(\u0027%s: %s\u0027 % (msg, str(err)))"}],"source_content_type":"text/x-python","patch_set":1,"id":"dab17558_ba9f7564","line":230,"range":{"start_line":230,"start_character":26,"end_line":230,"end_character":52},"in_reply_to":"dab17558_83ece0bb","updated":"2016-05-12 10:51:56.000000000","message":"Done","commit_id":"a962d4d7046fbef35a6f0eeeb4e7a6c340ba0ec3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"cf798761f06094a7141cb54873f182e0a24c0947","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        self.crypto.check_crypto_meta(crypto_meta)"},{"line_number":58,"context_line":"        return crypto_meta"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    def get_unwrapped_key(self, crypto_meta, wrapping_key):"},{"line_number":61,"context_line":"        \"\"\""},{"line_number":62,"context_line":"        Get a wrapped key from crypto-meta and unwrap it using the provided"},{"line_number":63,"context_line":"        wrapping key."}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_2925f85b","line":60,"updated":"2016-05-12 10:51:36.000000000","message":"new in this version","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"cf798761f06094a7141cb54873f182e0a24c0947","unresolved":false,"context_lines":[{"line_number":283,"context_line":"                self.logger.error(\u0027%s:  %s\u0027, msg, err)"},{"line_number":284,"context_line":"                raise HTTPInternalServerError("},{"line_number":285,"context_line":"                    body\u003dmsg, content_type\u003d\u0027text/plain\u0027)"},{"line_number":286,"context_line":""},{"line_number":287,"context_line":"            if not crypto_meta:"},{"line_number":288,"context_line":"                self.logger.debug(\"No sysmeta-crypto-meta for body.\")"},{"line_number":289,"context_line":"                resp_iter \u003d app_resp"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_e9b810eb","line":286,"updated":"2016-05-12 10:51:36.000000000","message":"new in this version - changes in the else clauses below here while rebasing","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"}],"swift/common/middleware/encrypter.py":[{"author":{"_account_id":9625,"name":"Thiago da Silva","email":"thiagodasilva@gmail.com","username":"thiago"},"change_message_id":"4a6307ace243bfddde636261b92b2b37d1397997","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    \"\"\""},{"line_number":32,"context_line":"    Set the crypto-meta associated to the obj body."},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    The IV value is random bytes and as a result needs to be encoded before"},{"line_number":35,"context_line":"    sending over the wire. Do this by wrapping the crypto meta in a json object"},{"line_number":36,"context_line":"    and encode the iv value. Base64 encoding returns a bytes object in py3, to"},{"line_number":37,"context_line":"    future proof the code, decode this data to produce a string, which is what"}],"source_content_type":"text/x-python","patch_set":1,"id":"dab17558_23b5347d","line":34,"updated":"2016-05-12 00:16:42.000000000","message":"update comment to include key","commit_id":"a962d4d7046fbef35a6f0eeeb4e7a6c340ba0ec3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"6466b0910138c4410516bf7e239a71a1de7f0b2e","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    \"\"\""},{"line_number":32,"context_line":"    Set the crypto-meta associated to the obj body."},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    The IV value is random bytes and as a result needs to be encoded before"},{"line_number":35,"context_line":"    sending over the wire. Do this by wrapping the crypto meta in a json object"},{"line_number":36,"context_line":"    and encode the iv value. Base64 encoding returns a bytes object in py3, to"},{"line_number":37,"context_line":"    future proof the code, decode this data to produce a string, which is what"}],"source_content_type":"text/x-python","patch_set":1,"id":"dab17558_40401836","line":34,"in_reply_to":"dab17558_23b5347d","updated":"2016-05-12 10:51:56.000000000","message":"Done, also clarified \"encoded\" to be base64 encoded.","commit_id":"a962d4d7046fbef35a6f0eeeb4e7a6c340ba0ec3"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"21e5c235f824b6a7ad436be5cd222b3dd094cea2","unresolved":false,"context_lines":[{"line_number":119,"context_line":"                    self.keys[\u0027object\u0027],"},{"line_number":120,"context_line":"                    self.body_key,"},{"line_number":121,"context_line":"                    self.body_crypto_meta[\u0027iv\u0027]"},{"line_number":122,"context_line":"                )"},{"line_number":123,"context_line":"                footers[\u0027X-Object-Sysmeta-Crypto-Meta\u0027] \u003d _dump_crypto_meta("},{"line_number":124,"context_line":"                    self.body_crypto_meta)"},{"line_number":125,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"dab17558_200f862f","line":122,"updated":"2016-05-09 06:49:04.000000000","message":"Overview crypto doc says crypto meta is stored in the following way:\nX-Object-Sysmeta-Crypto-Meta:\n      {\u0027iv\u0027: body_iv,\n       \u0027key_id\u0027: H(object_key),\n       \u0027body_key\u0027: E(body_key, object_key, body_iv)}\n\nBut what we\u0027re actually doing here is:\nX-Object-Sysmeta-Crypto-Meta:\n      {\u0027iv\u0027: body_iv,\n       \u0027cipher\u0027: \u0027AES_CTR_256\u0027\n       \u0027key\u0027: E(body_key, object_key, body_iv)}\n\nIs that intentional?","commit_id":"a962d4d7046fbef35a6f0eeeb4e7a6c340ba0ec3"},{"author":{"_account_id":9625,"name":"Thiago da Silva","email":"thiagodasilva@gmail.com","username":"thiago"},"change_message_id":"4a6307ace243bfddde636261b92b2b37d1397997","unresolved":false,"context_lines":[{"line_number":119,"context_line":"                    self.keys[\u0027object\u0027],"},{"line_number":120,"context_line":"                    self.body_key,"},{"line_number":121,"context_line":"                    self.body_crypto_meta[\u0027iv\u0027]"},{"line_number":122,"context_line":"                )"},{"line_number":123,"context_line":"                footers[\u0027X-Object-Sysmeta-Crypto-Meta\u0027] \u003d _dump_crypto_meta("},{"line_number":124,"context_line":"                    self.body_crypto_meta)"},{"line_number":125,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"dab17558_f8f473f1","line":122,"in_reply_to":"dab17558_200f862f","updated":"2016-05-12 00:16:42.000000000","message":"is the hash of the key going to be added in a follow-up patch?","commit_id":"a962d4d7046fbef35a6f0eeeb4e7a6c340ba0ec3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"6466b0910138c4410516bf7e239a71a1de7f0b2e","unresolved":false,"context_lines":[{"line_number":119,"context_line":"                    self.keys[\u0027object\u0027],"},{"line_number":120,"context_line":"                    self.body_key,"},{"line_number":121,"context_line":"                    self.body_crypto_meta[\u0027iv\u0027]"},{"line_number":122,"context_line":"                )"},{"line_number":123,"context_line":"                footers[\u0027X-Object-Sysmeta-Crypto-Meta\u0027] \u003d _dump_crypto_meta("},{"line_number":124,"context_line":"                    self.body_crypto_meta)"},{"line_number":125,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"dab17558_6030bce9","line":122,"in_reply_to":"dab17558_f8f473f1","updated":"2016-05-12 10:51:56.000000000","message":"1. key_id - yes it would be a hash of the key and we *may* add it in a follow-up. I added a note to the doc review to revisit that. It is not required for data key wrapping.\n2. cipher - it\u0027s missing from the doc.","commit_id":"a962d4d7046fbef35a6f0eeeb4e7a6c340ba0ec3"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b6ddfc3d64605f11389e85b789c4f334664127bc","unresolved":false,"context_lines":[{"line_number":68,"context_line":"    def _init_encryption_context(self):"},{"line_number":69,"context_line":"        # do this once when body is first read"},{"line_number":70,"context_line":"        if self.body_crypto_ctxt is None:"},{"line_number":71,"context_line":"            self.body_crypto_meta \u003d self.crypto.create_crypto_meta()"},{"line_number":72,"context_line":"            self.body_key \u003d self.crypto.create_random_key()"},{"line_number":73,"context_line":"            self.body_crypto_ctxt \u003d self.crypto.create_encryption_ctxt("},{"line_number":74,"context_line":"                self.body_key, self.body_crypto_meta.get(\u0027iv\u0027))"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_855789c9","line":71,"updated":"2016-05-12 22:08:10.000000000","message":"OK, since:\n- we only use body_crypto_meta in footers_callback, and\n- even then only if body_crypto_ctxt is not None, and\n- body_crypto_ctxt is intialized here,\n\nit seems safe to move this. This does make me wonder if it\u0027d be worth having the crypto_meta in some way be a property of the crypto_ctxt, though.","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c8cfec4e27bc7754816fbd92f72a14b3a8713219","unresolved":false,"context_lines":[{"line_number":68,"context_line":"    def _init_encryption_context(self):"},{"line_number":69,"context_line":"        # do this once when body is first read"},{"line_number":70,"context_line":"        if self.body_crypto_ctxt is None:"},{"line_number":71,"context_line":"            self.body_crypto_meta \u003d self.crypto.create_crypto_meta()"},{"line_number":72,"context_line":"            self.body_key \u003d self.crypto.create_random_key()"},{"line_number":73,"context_line":"            self.body_crypto_ctxt \u003d self.crypto.create_encryption_ctxt("},{"line_number":74,"context_line":"                self.body_key, self.body_crypto_meta.get(\u0027iv\u0027))"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_fd9417e9","line":71,"in_reply_to":"dab17558_855789c9","updated":"2016-05-13 12:54:28.000000000","message":"Yes, I agree, would like to do just that in a later patch.","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b6ddfc3d64605f11389e85b789c4f334664127bc","unresolved":false,"context_lines":[{"line_number":90,"context_line":"                footers[\u0027X-Object-Sysmeta-Crypto-Etag\u0027] \u003d val"},{"line_number":91,"context_line":"                footers[\u0027X-Object-Sysmeta-Crypto-Meta-Etag\u0027] \u003d \\"},{"line_number":92,"context_line":"                    dump_crypto_meta(etag_crypto_meta)"},{"line_number":93,"context_line":"                # wrap the body key with object key re-using body iv"},{"line_number":94,"context_line":"                self.body_crypto_meta[\u0027key\u0027] \u003d self.crypto.wrap_key("},{"line_number":95,"context_line":"                    self.keys[\u0027object\u0027],"},{"line_number":96,"context_line":"                    self.body_key,"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_450481c5","line":93,"range":{"start_line":93,"start_character":52,"end_line":93,"end_character":68},"updated":"2016-05-12 22:08:10.000000000","message":"Hrm... there we go wanting to re-use IVs again... \n\nReminds me of the discussions about using hash(path_info) as an IV; still makes me a little nervous :-/\n\nThat said, I still haven\u0027t found anything to refute Janie\u0027s assertion that its safe to re-use IVs provided they\u0027re used with different keys. So I think it\u0027s fine?","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"810759f610ede6846b89d0bbd3a21cd930b6093d","unresolved":false,"context_lines":[{"line_number":90,"context_line":"                footers[\u0027X-Object-Sysmeta-Crypto-Etag\u0027] \u003d val"},{"line_number":91,"context_line":"                footers[\u0027X-Object-Sysmeta-Crypto-Meta-Etag\u0027] \u003d \\"},{"line_number":92,"context_line":"                    dump_crypto_meta(etag_crypto_meta)"},{"line_number":93,"context_line":"                # wrap the body key with object key re-using body iv"},{"line_number":94,"context_line":"                self.body_crypto_meta[\u0027key\u0027] \u003d self.crypto.wrap_key("},{"line_number":95,"context_line":"                    self.keys[\u0027object\u0027],"},{"line_number":96,"context_line":"                    self.body_key,"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_a586dad8","line":93,"range":{"start_line":93,"start_character":52,"end_line":93,"end_character":68},"in_reply_to":"dab17558_2ac5983c","updated":"2016-05-13 18:22:23.000000000","message":"\u003e re-using the same (key, iv) combination is a no-no\n\nDefinitely; you\u0027d be able to xor two ciphertexts together to get the xor of two plaintexts.\n\n\u003e The body iv is randomly chosen for each PUT\n\n...which makes me not overly worried about re-using it (with different keys) in exactly two encrypt operations. Particularly since one uses a random body and the other uses a random key.\n\n\u003e etags are random enough\n\nThis may be a dangerous assumption, though. Etags have a decent bit of entropy, but they\u0027re also fairly constrained:\n- fixed length (32 chars)\n- small alphabet (always lower-case hex)\n\n\u003e there is no \"guessable\" plaintext in an etag\n\nThis *very much* depends on the attack vectors we\u0027re looking at. Even if we can ignore chosen-plaintext attacks, there\u0027s a very high probability of finding \"d41d8cd98f00b204e9800998ecf8427e\" somewhere in a container, particularly when you consider DLOs.","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c8cfec4e27bc7754816fbd92f72a14b3a8713219","unresolved":false,"context_lines":[{"line_number":90,"context_line":"                footers[\u0027X-Object-Sysmeta-Crypto-Etag\u0027] \u003d val"},{"line_number":91,"context_line":"                footers[\u0027X-Object-Sysmeta-Crypto-Meta-Etag\u0027] \u003d \\"},{"line_number":92,"context_line":"                    dump_crypto_meta(etag_crypto_meta)"},{"line_number":93,"context_line":"                # wrap the body key with object key re-using body iv"},{"line_number":94,"context_line":"                self.body_crypto_meta[\u0027key\u0027] \u003d self.crypto.wrap_key("},{"line_number":95,"context_line":"                    self.keys[\u0027object\u0027],"},{"line_number":96,"context_line":"                    self.body_key,"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_9d2a2b8f","line":93,"range":{"start_line":93,"start_character":52,"end_line":93,"end_character":68},"in_reply_to":"dab17558_450481c5","updated":"2016-05-13 12:54:28.000000000","message":"comment is there deliberately to flag up re-use to reviewers with greater crypto knowledge than me","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":9625,"name":"Thiago da Silva","email":"thiagodasilva@gmail.com","username":"thiago"},"change_message_id":"f2a0eb49e275a70c3aa53048f111707176f62b70","unresolved":false,"context_lines":[{"line_number":90,"context_line":"                footers[\u0027X-Object-Sysmeta-Crypto-Etag\u0027] \u003d val"},{"line_number":91,"context_line":"                footers[\u0027X-Object-Sysmeta-Crypto-Meta-Etag\u0027] \u003d \\"},{"line_number":92,"context_line":"                    dump_crypto_meta(etag_crypto_meta)"},{"line_number":93,"context_line":"                # wrap the body key with object key re-using body iv"},{"line_number":94,"context_line":"                self.body_crypto_meta[\u0027key\u0027] \u003d self.crypto.wrap_key("},{"line_number":95,"context_line":"                    self.keys[\u0027object\u0027],"},{"line_number":96,"context_line":"                    self.body_key,"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_c08af519","line":93,"range":{"start_line":93,"start_character":52,"end_line":93,"end_character":68},"in_reply_to":"dab17558_9d2a2b8f","updated":"2016-05-13 15:40:19.000000000","message":"i\u0027m no crypto expert, but please bear with me...\n\"According to wikipedia, re-using iv is a big no no\", but in the case of metadata of the same object, it seems ok, if the IV is always some random data for a PUT request.\n\nI\u0027m more concerned about the idea of using hash(path_info) as the IV. Is it possible an attacker could keep PUTting different data to the same object and since the IV will always be the same they could start learning something about the key?","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a519c541a59b84dce9caaf32f7ed18afe77b35bc","unresolved":false,"context_lines":[{"line_number":90,"context_line":"                footers[\u0027X-Object-Sysmeta-Crypto-Etag\u0027] \u003d val"},{"line_number":91,"context_line":"                footers[\u0027X-Object-Sysmeta-Crypto-Meta-Etag\u0027] \u003d \\"},{"line_number":92,"context_line":"                    dump_crypto_meta(etag_crypto_meta)"},{"line_number":93,"context_line":"                # wrap the body key with object key re-using body iv"},{"line_number":94,"context_line":"                self.body_crypto_meta[\u0027key\u0027] \u003d self.crypto.wrap_key("},{"line_number":95,"context_line":"                    self.keys[\u0027object\u0027],"},{"line_number":96,"context_line":"                    self.body_key,"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_a47ea667","line":93,"range":{"start_line":93,"start_character":52,"end_line":93,"end_character":68},"in_reply_to":"dab17558_a586dad8","updated":"2016-05-16 11:16:56.000000000","message":"Re DLO/zero sized object etags, this patch may help:\n\nhttps://review.openstack.org/316732 crypto - Don\u0027t encrypt zero size object etags","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9eca9b23fb84ee9f84d91f14005e17389bc8a99c","unresolved":false,"context_lines":[{"line_number":90,"context_line":"                footers[\u0027X-Object-Sysmeta-Crypto-Etag\u0027] \u003d val"},{"line_number":91,"context_line":"                footers[\u0027X-Object-Sysmeta-Crypto-Meta-Etag\u0027] \u003d \\"},{"line_number":92,"context_line":"                    dump_crypto_meta(etag_crypto_meta)"},{"line_number":93,"context_line":"                # wrap the body key with object key re-using body iv"},{"line_number":94,"context_line":"                self.body_crypto_meta[\u0027key\u0027] \u003d self.crypto.wrap_key("},{"line_number":95,"context_line":"                    self.keys[\u0027object\u0027],"},{"line_number":96,"context_line":"                    self.body_key,"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_2ac5983c","line":93,"range":{"start_line":93,"start_character":52,"end_line":93,"end_character":68},"in_reply_to":"dab17558_c08af519","updated":"2016-05-13 17:05:56.000000000","message":"1. My understanding is that re-using the same (key, iv) combination is a no-no. Here we use different keys with same iv.\n\nThe body iv is randomly chosen for each PUT, we use it once to encrypt the body with body_key and once to encrypt the body_key with object_key.\n\nI\u0027ll add a note to this trello todo [1] to reconsider this - we could in fact use another iv for the wrapped key, it just means storing more metadata but that\u0027s no big deal. Or we could use ECB with no iv.\n\n\n2. When the etag is encrypted, we do use an iv derive form the path (in some cases a hash of the path). We need to do that to be apply the same encryption to If-Match etag values. Janie felt that was ok because the etags are random enough to mitigate the risk of the attack known as \"crib-dragging\" - there is no \"guessable\" plaintext in an etag.\n\nWe have a todo on trello [2] to add comments to explain/justify the derived iv for etags.\n\n[1] task https://trello.com/c/GH375UNr/99-check-should-we-use-unique-iv-or-ecb-mode-when-wrapping-keys\n\n[2] https://trello.com/c/Ib7A3iFU/82-add-justification-\nfor-derived-iv-for-etag-as-a-comment-in-code","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":9625,"name":"Thiago da Silva","email":"thiagodasilva@gmail.com","username":"thiago"},"change_message_id":"fcdaf78472a33e0a34d8dd3ecf62486962901383","unresolved":false,"context_lines":[{"line_number":91,"context_line":"                footers[\u0027X-Object-Sysmeta-Crypto-Meta-Etag\u0027] \u003d \\"},{"line_number":92,"context_line":"                    dump_crypto_meta(etag_crypto_meta)"},{"line_number":93,"context_line":"                # wrap the body key with object key re-using body iv"},{"line_number":94,"context_line":"                self.body_crypto_meta[\u0027key\u0027] \u003d self.crypto.wrap_key("},{"line_number":95,"context_line":"                    self.keys[\u0027object\u0027],"},{"line_number":96,"context_line":"                    self.body_key,"},{"line_number":97,"context_line":"                    self.body_crypto_meta[\u0027iv\u0027]"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_36e89d16","line":94,"updated":"2016-05-12 19:04:04.000000000","message":"wondering why not do the key wrapping part in _init_encryption_context","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c8cfec4e27bc7754816fbd92f72a14b3a8713219","unresolved":false,"context_lines":[{"line_number":91,"context_line":"                footers[\u0027X-Object-Sysmeta-Crypto-Meta-Etag\u0027] \u003d \\"},{"line_number":92,"context_line":"                    dump_crypto_meta(etag_crypto_meta)"},{"line_number":93,"context_line":"                # wrap the body key with object key re-using body iv"},{"line_number":94,"context_line":"                self.body_crypto_meta[\u0027key\u0027] \u003d self.crypto.wrap_key("},{"line_number":95,"context_line":"                    self.keys[\u0027object\u0027],"},{"line_number":96,"context_line":"                    self.body_key,"},{"line_number":97,"context_line":"                    self.body_crypto_meta[\u0027iv\u0027]"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_78267517","line":94,"in_reply_to":"dab17558_36e89d16","updated":"2016-05-13 12:54:28.000000000","message":"Agree. It groups all the actions around the meta into one place. _init_encryption_context is only called if the body is read so same workload either way.","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"}],"test/unit/common/middleware/test_crypto.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b6ddfc3d64605f11389e85b789c4f334664127bc","unresolved":false,"context_lines":[{"line_number":207,"context_line":"        # verify that ValueError is raised if unwrapped key is invalid"},{"line_number":208,"context_line":"        wrapping_key \u003d os.urandom(32)"},{"line_number":209,"context_line":"        iv \u003d os.urandom(16)"},{"line_number":210,"context_line":"        for length in (16, 31, 33):"},{"line_number":211,"context_line":"            key_to_wrap \u003d os.urandom(length)"},{"line_number":212,"context_line":"            wrapped \u003d self.crypto.wrap_key(wrapping_key, key_to_wrap, iv)"},{"line_number":213,"context_line":"            with self.assertRaises(ValueError) as cm:"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_85d029cb","line":210,"updated":"2016-05-12 22:08:10.000000000","message":"Maybe also worth adding 24, since it\u0027s another valid AES key size? I could go either way.","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c8cfec4e27bc7754816fbd92f72a14b3a8713219","unresolved":false,"context_lines":[{"line_number":207,"context_line":"        # verify that ValueError is raised if unwrapped key is invalid"},{"line_number":208,"context_line":"        wrapping_key \u003d os.urandom(32)"},{"line_number":209,"context_line":"        iv \u003d os.urandom(16)"},{"line_number":210,"context_line":"        for length in (16, 31, 33):"},{"line_number":211,"context_line":"            key_to_wrap \u003d os.urandom(length)"},{"line_number":212,"context_line":"            wrapped \u003d self.crypto.wrap_key(wrapping_key, key_to_wrap, iv)"},{"line_number":213,"context_line":"            with self.assertRaises(ValueError) as cm:"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_5868d121","line":210,"in_reply_to":"dab17558_85d029cb","updated":"2016-05-13 12:54:28.000000000","message":"Done","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"}],"test/unit/common/middleware/test_crypto_utils.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2bf8204243650d7853e4798ae0dc9030ba948b62","unresolved":false,"context_lines":[{"line_number":202,"context_line":"        self.assertEqual(\u0027abc\u0027, val)"},{"line_number":203,"context_line":"        self.assertIsNone(meta)"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"        # other param names will be ignored"},{"line_number":206,"context_line":"        val, meta \u003d crypto_utils.extract_crypto_meta(\u0027abc; foo\u003dbar\u0027)"},{"line_number":207,"context_line":"        self.assertEqual(\u0027abc\u0027, val)"},{"line_number":208,"context_line":"        self.assertIsNone(meta)"}],"source_content_type":"text/x-python","patch_set":4,"id":"dab17558_48bf43fb","line":205,"updated":"2016-05-13 18:22:31.000000000","message":"Now there\u0027s a good point. One more time: this *only ever* is used for etags, right?","commit_id":"c5cf7a22f4cab5f016f0f814983f07cd4a6255d7"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8bdec71b66396c9ddbac09bad9e7740cdd29490d","unresolved":false,"context_lines":[{"line_number":202,"context_line":"        self.assertEqual(\u0027abc\u0027, val)"},{"line_number":203,"context_line":"        self.assertIsNone(meta)"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"        # other param names will be ignored"},{"line_number":206,"context_line":"        val, meta \u003d crypto_utils.extract_crypto_meta(\u0027abc; foo\u003dbar\u0027)"},{"line_number":207,"context_line":"        self.assertEqual(\u0027abc\u0027, val)"},{"line_number":208,"context_line":"        self.assertIsNone(meta)"}],"source_content_type":"text/x-python","patch_set":4,"id":"dab17558_2fc29834","line":205,"in_reply_to":"dab17558_48bf43fb","updated":"2016-05-16 11:19:05.000000000","message":"Hmmm. Currently, yes. But if we were to encrypt content-type (part of the original scope but deferred for time being) then we would add a similar param to the content-type value.\n\nSo...should we rename \"meta\" to something slighlty less generic, perhaps \"swift_meta\" similar to SLO\u0027s \"swift_bytes\"?? Or as i think Tim suggested before, \"crypto_meta\"??","commit_id":"c5cf7a22f4cab5f016f0f814983f07cd4a6255d7"}],"test/unit/common/middleware/test_decrypter.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b6ddfc3d64605f11389e85b789c4f334664127bc","unresolved":false,"context_lines":[{"line_number":35,"context_line":"def get_crypto_meta_header(crypto_meta\u003dNone):"},{"line_number":36,"context_line":"    if crypto_meta is None:"},{"line_number":37,"context_line":"        crypto_meta \u003d fake_get_crypto_meta()"},{"line_number":38,"context_line":"    return urllib.quote_plus("},{"line_number":39,"context_line":"        json.dumps({key: (base64.b64encode(value).decode()"},{"line_number":40,"context_line":"                          if key in (\u0027iv\u0027, \u0027key\u0027) else value)"},{"line_number":41,"context_line":"                    for key, value in crypto_meta.items()}))"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_e502d50a","line":38,"updated":"2016-05-12 22:08:10.000000000","message":"This return is taken practically verbatim from dump_crypto_meta; should we just have some tests for dump_crypto_meta and use that here?","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c8cfec4e27bc7754816fbd92f72a14b3a8713219","unresolved":false,"context_lines":[{"line_number":35,"context_line":"def get_crypto_meta_header(crypto_meta\u003dNone):"},{"line_number":36,"context_line":"    if crypto_meta is None:"},{"line_number":37,"context_line":"        crypto_meta \u003d fake_get_crypto_meta()"},{"line_number":38,"context_line":"    return urllib.quote_plus("},{"line_number":39,"context_line":"        json.dumps({key: (base64.b64encode(value).decode()"},{"line_number":40,"context_line":"                          if key in (\u0027iv\u0027, \u0027key\u0027) else value)"},{"line_number":41,"context_line":"                    for key, value in crypto_meta.items()}))"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_280f7689","line":38,"in_reply_to":"dab17558_e502d50a","updated":"2016-05-13 12:54:28.000000000","message":"Done. We *did* need some tests for load/dump crypto meta.","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"cf798761f06094a7141cb54873f182e0a24c0947","unresolved":false,"context_lines":[{"line_number":296,"context_line":"                      self.decrypter.logger.get_lines_for_level(\u0027error\u0027)[0])"},{"line_number":297,"context_line":""},{"line_number":298,"context_line":"    def test_GET_with_bad_iv_for_object_body(self):"},{"line_number":299,"context_line":"        body_key \u003d encrypt(os.urandom(32), os.urandom(32), os.urandom(16))"},{"line_number":300,"context_line":"        bad_crypto_meta \u003d fake_get_crypto_meta(key\u003dbody_key)"},{"line_number":301,"context_line":"        bad_crypto_meta[\u0027iv\u0027] \u003d \u0027bad_iv\u0027"},{"line_number":302,"context_line":"        self._test_GET_with_bad_crypto_meta_for_object_body(bad_crypto_meta)"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_e9b490d5","line":299,"updated":"2016-05-12 10:51:36.000000000","message":"new in this version - fully populate the test meta dict","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c8cfec4e27bc7754816fbd92f72a14b3a8713219","unresolved":false,"context_lines":[{"line_number":296,"context_line":"                      self.decrypter.logger.get_lines_for_level(\u0027error\u0027)[0])"},{"line_number":297,"context_line":""},{"line_number":298,"context_line":"    def test_GET_with_bad_iv_for_object_body(self):"},{"line_number":299,"context_line":"        body_key \u003d encrypt(os.urandom(32), os.urandom(32), os.urandom(16))"},{"line_number":300,"context_line":"        bad_crypto_meta \u003d fake_get_crypto_meta(key\u003dbody_key)"},{"line_number":301,"context_line":"        bad_crypto_meta[\u0027iv\u0027] \u003d \u0027bad_iv\u0027"},{"line_number":302,"context_line":"        self._test_GET_with_bad_crypto_meta_for_object_body(bad_crypto_meta)"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_23f67ef9","line":299,"in_reply_to":"dab17558_60a9b3df","updated":"2016-05-13 12:54:28.000000000","message":"Done","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b6ddfc3d64605f11389e85b789c4f334664127bc","unresolved":false,"context_lines":[{"line_number":296,"context_line":"                      self.decrypter.logger.get_lines_for_level(\u0027error\u0027)[0])"},{"line_number":297,"context_line":""},{"line_number":298,"context_line":"    def test_GET_with_bad_iv_for_object_body(self):"},{"line_number":299,"context_line":"        body_key \u003d encrypt(os.urandom(32), os.urandom(32), os.urandom(16))"},{"line_number":300,"context_line":"        bad_crypto_meta \u003d fake_get_crypto_meta(key\u003dbody_key)"},{"line_number":301,"context_line":"        bad_crypto_meta[\u0027iv\u0027] \u003d \u0027bad_iv\u0027"},{"line_number":302,"context_line":"        self._test_GET_with_bad_crypto_meta_for_object_body(bad_crypto_meta)"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_60a9b3df","line":299,"in_reply_to":"dab17558_e9b490d5","updated":"2016-05-12 22:08:10.000000000","message":"Why not just\n\n body_key \u003d os.urandom(32)\n\n?","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"cf798761f06094a7141cb54873f182e0a24c0947","unresolved":false,"context_lines":[{"line_number":304,"context_line":"                      self.decrypter.logger.get_lines_for_level(\u0027error\u0027)[0])"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"    def test_GET_with_missing_iv_for_object_body(self):"},{"line_number":307,"context_line":"        body_key \u003d encrypt(os.urandom(32), os.urandom(32), os.urandom(16))"},{"line_number":308,"context_line":"        bad_crypto_meta \u003d fake_get_crypto_meta(key\u003dbody_key)"},{"line_number":309,"context_line":"        bad_crypto_meta.pop(\u0027iv\u0027)"},{"line_number":310,"context_line":"        self._test_GET_with_bad_crypto_meta_for_object_body(bad_crypto_meta)"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_29afb87e","line":307,"updated":"2016-05-12 10:51:36.000000000","message":"new in this version - fully populate the test meta dict","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"cf798761f06094a7141cb54873f182e0a24c0947","unresolved":false,"context_lines":[{"line_number":310,"context_line":"        self._test_GET_with_bad_crypto_meta_for_object_body(bad_crypto_meta)"},{"line_number":311,"context_line":"        self.assertIn("},{"line_number":312,"context_line":"            \u0027iv\u0027, self.decrypter.logger.get_lines_for_level(\u0027error\u0027)[0])"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"    def test_GET_with_bad_body_key_for_object_body(self):"},{"line_number":315,"context_line":"        bad_crypto_meta \u003d fake_get_crypto_meta(key\u003d\u0027wrapped too short key\u0027)"},{"line_number":316,"context_line":"        self._test_GET_with_bad_crypto_meta_for_object_body(bad_crypto_meta)"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_298178fc","line":313,"updated":"2016-05-12 10:51:36.000000000","message":"two new tests...","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b6ddfc3d64605f11389e85b789c4f334664127bc","unresolved":false,"context_lines":[{"line_number":321,"context_line":"        bad_crypto_meta \u003d fake_get_crypto_meta()  # no key by default"},{"line_number":322,"context_line":"        self._test_GET_with_bad_crypto_meta_for_object_body(bad_crypto_meta)"},{"line_number":323,"context_line":"        self.assertIn("},{"line_number":324,"context_line":"            \u0027key\u0027, self.decrypter.logger.get_lines_for_level(\u0027error\u0027)[0])"},{"line_number":325,"context_line":""},{"line_number":326,"context_line":"    def test_HEAD_success(self):"},{"line_number":327,"context_line":"        env \u003d {\u0027REQUEST_METHOD\u0027: \u0027HEAD\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_c0dcdf76","line":324,"range":{"start_line":324,"start_character":13,"end_line":324,"end_character":16},"updated":"2016-05-12 22:08:10.000000000","message":"This feels like a weak assertion; can we do better?","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"810759f610ede6846b89d0bbd3a21cd930b6093d","unresolved":false,"context_lines":[{"line_number":321,"context_line":"        bad_crypto_meta \u003d fake_get_crypto_meta()  # no key by default"},{"line_number":322,"context_line":"        self._test_GET_with_bad_crypto_meta_for_object_body(bad_crypto_meta)"},{"line_number":323,"context_line":"        self.assertIn("},{"line_number":324,"context_line":"            \u0027key\u0027, self.decrypter.logger.get_lines_for_level(\u0027error\u0027)[0])"},{"line_number":325,"context_line":""},{"line_number":326,"context_line":"    def test_HEAD_success(self):"},{"line_number":327,"context_line":"        env \u003d {\u0027REQUEST_METHOD\u0027: \u0027HEAD\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_28ff5fab","line":324,"range":{"start_line":324,"start_character":13,"end_line":324,"end_character":16},"in_reply_to":"dab17558_3e35f545","updated":"2016-05-13 18:22:23.000000000","message":"I\u0027m definitely on board with assertIn for log messages and assertEqual for response bodies. My point was purely about how short and relatively common the string we were searching for was.","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c8cfec4e27bc7754816fbd92f72a14b3a8713219","unresolved":false,"context_lines":[{"line_number":321,"context_line":"        bad_crypto_meta \u003d fake_get_crypto_meta()  # no key by default"},{"line_number":322,"context_line":"        self._test_GET_with_bad_crypto_meta_for_object_body(bad_crypto_meta)"},{"line_number":323,"context_line":"        self.assertIn("},{"line_number":324,"context_line":"            \u0027key\u0027, self.decrypter.logger.get_lines_for_level(\u0027error\u0027)[0])"},{"line_number":325,"context_line":""},{"line_number":326,"context_line":"    def test_HEAD_success(self):"},{"line_number":327,"context_line":"        env \u003d {\u0027REQUEST_METHOD\u0027: \u0027HEAD\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_3e35f545","line":324,"range":{"start_line":324,"start_character":13,"end_line":324,"end_character":16},"in_reply_to":"dab17558_c0dcdf76","updated":"2016-05-13 12:54:28.000000000","message":"Done.\n\nI\u0027m using assertIn because I saw a review recently which suggested that using assertEqual(\"blah\", log_string) created a maintenance burden when small tweaks were made to log messages in future. Maybe I misunderstood, I took it to mean that it is better to assert that some essential content is in the log rather than be precise. I\u0027m happy to be corrected and use assertEqual.\n\nIn this case though I agree the essential content should be more than currently asserted. In fact, I have added \u0027Missing \u0027 to the log string since all we were getting was \u0027key\u0027.\n\n500 response bodies on the contrary are tested with assertEqual because I think we should be certain what is going to clients.","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"}],"test/unit/common/middleware/test_encrypter.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b6ddfc3d64605f11389e85b789c4f334664127bc","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        self.app.register(\u0027PUT\u0027, \u0027/v1/a/c/o\u0027, HTTPCreated, {})"},{"line_number":62,"context_line":"        with mock.patch("},{"line_number":63,"context_line":"                \u0027swift.common.middleware.crypto.Crypto.create_random_key\u0027,"},{"line_number":64,"context_line":"                lambda *args: body_key):"},{"line_number":65,"context_line":"            resp \u003d req.get_response(self.encrypter)"},{"line_number":66,"context_line":"        self.assertEqual(\u0027201 Created\u0027, resp.status)"},{"line_number":67,"context_line":"        self.assertEqual(plaintext_etag, resp.headers[\u0027Etag\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_c01fbf07","line":64,"updated":"2016-05-12 22:08:10.000000000","message":"Any reason to prefer the lambda over\n\n return_value\u003dbody_key\n\n?","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c8cfec4e27bc7754816fbd92f72a14b3a8713219","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        self.app.register(\u0027PUT\u0027, \u0027/v1/a/c/o\u0027, HTTPCreated, {})"},{"line_number":62,"context_line":"        with mock.patch("},{"line_number":63,"context_line":"                \u0027swift.common.middleware.crypto.Crypto.create_random_key\u0027,"},{"line_number":64,"context_line":"                lambda *args: body_key):"},{"line_number":65,"context_line":"            resp \u003d req.get_response(self.encrypter)"},{"line_number":66,"context_line":"        self.assertEqual(\u0027201 Created\u0027, resp.status)"},{"line_number":67,"context_line":"        self.assertEqual(plaintext_etag, resp.headers[\u0027Etag\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_7e893dca","line":64,"in_reply_to":"dab17558_c01fbf07","updated":"2016-05-13 12:54:28.000000000","message":"Done","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"}],"test/unit/common/middleware/test_encrypter_decrypter.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b6ddfc3d64605f11389e85b789c4f334664127bc","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        crypto_meta[\u0027iv\u0027] \u003d base64.b64decode(crypto_meta[\u0027iv\u0027])"},{"line_number":70,"context_line":"        exp_enc_body \u003d encrypt("},{"line_number":71,"context_line":"            body, body_key, crypto_meta[\u0027iv\u0027])"},{"line_number":72,"context_line":"        self.assertNotEqual(body, encrypt_get_resp.body)  # sanity check"},{"line_number":73,"context_line":"        self.assertEqual(exp_enc_body, encrypt_get_resp.body)"},{"line_number":74,"context_line":"        self.assertNotEqual(body, encrypt_get_resp.body)  # sanity check"},{"line_number":75,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_40b90fec","line":72,"updated":"2016-05-12 22:08:10.000000000","message":"Isn\u0027t this duplicated below?","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c8cfec4e27bc7754816fbd92f72a14b3a8713219","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        crypto_meta[\u0027iv\u0027] \u003d base64.b64decode(crypto_meta[\u0027iv\u0027])"},{"line_number":70,"context_line":"        exp_enc_body \u003d encrypt("},{"line_number":71,"context_line":"            body, body_key, crypto_meta[\u0027iv\u0027])"},{"line_number":72,"context_line":"        self.assertNotEqual(body, encrypt_get_resp.body)  # sanity check"},{"line_number":73,"context_line":"        self.assertEqual(exp_enc_body, encrypt_get_resp.body)"},{"line_number":74,"context_line":"        self.assertNotEqual(body, encrypt_get_resp.body)  # sanity check"},{"line_number":75,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_7235f1d9","line":72,"in_reply_to":"dab17558_40b90fec","updated":"2016-05-13 12:54:28.000000000","message":"Done","commit_id":"453fe0e36219fd6f370b08f532160a3c619cb5a0"}]}