)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"732d27020df3777d1650524b3e638e9455c25533","unresolved":true,"context_lines":[{"line_number":9,"context_line":"This patch fixes the Castellan secret store use of SecretDTO objects,"},{"line_number":10,"context_line":"which require that the \"secret\" member be base64 encoded. [1]"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Prior to this fix all secrets that wre generated were stored in"},{"line_number":13,"context_line":"plaintext, but secrets coming in through the API were base64 encoded"},{"line_number":14,"context_line":"before beign stored in the backend."},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"07a66be1_52cec5da","line":12,"range":{"start_line":12,"start_character":35,"end_line":12,"end_character":38},"updated":"2021-07-26 16:27:18.000000000","message":"nit: were","commit_id":"0405e10ea5f17da732e83d5f55b1ea53c923a810"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"732d27020df3777d1650524b3e638e9455c25533","unresolved":true,"context_lines":[{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Prior to this fix all secrets that wre generated were stored in"},{"line_number":13,"context_line":"plaintext, but secrets coming in through the API were base64 encoded"},{"line_number":14,"context_line":"before beign stored in the backend."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"On secret retreival the Castellan plugin wrongly assumed everything in"},{"line_number":17,"context_line":"the backend was encoded, so attempts to retrieve generated keys failed."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"33dc7adb_798b3a20","line":14,"range":{"start_line":14,"start_character":7,"end_line":14,"end_character":12},"updated":"2021-07-26 16:27:18.000000000","message":"nit: being","commit_id":"0405e10ea5f17da732e83d5f55b1ea53c923a810"}],"barbican/plugin/castellan_secret_store.py":[{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"d8baa7574340e6075e561ba871ff6878dd481895","unresolved":true,"context_lines":[{"line_number":85,"context_line":"                self.context,"},{"line_number":86,"context_line":"                secret_ref)"},{"line_number":87,"context_line":"            # TODO(dmendiza):  We need to check secret_meta version"},{"line_number":88,"context_line":"            # here to handle secrets that were stored before this"},{"line_number":89,"context_line":"            # fix."},{"line_number":90,"context_line":"            b64_secret \u003d base64.b64encode(secret.get_encoded())"},{"line_number":91,"context_line":"            return ss.SecretDTO(secret_type, b64_secret,"},{"line_number":92,"context_line":"                                ss.KeySpec(), secret_metadata[\u0027content_type\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"6e36973f_6f9cb6b4","line":89,"range":{"start_line":88,"start_character":54,"end_line":89,"end_character":17},"updated":"2021-06-21 14:47:30.000000000","message":"I\u0027m not sure it\u0027s quite that simple - before the fix most secrets are b64 encoded, but keys generated by castellan are not.","commit_id":"c49903b27c081196b90e9a4c493bddf2fc4514ea"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"3ee94dfde46d09cb17a5d18222a1eb60463e7639","unresolved":true,"context_lines":[{"line_number":85,"context_line":"                self.context,"},{"line_number":86,"context_line":"                secret_ref)"},{"line_number":87,"context_line":"            # TODO(dmendiza):  We need to check secret_meta version"},{"line_number":88,"context_line":"            # here to handle secrets that were stored before this"},{"line_number":89,"context_line":"            # fix."},{"line_number":90,"context_line":"            b64_secret \u003d base64.b64encode(secret.get_encoded())"},{"line_number":91,"context_line":"            return ss.SecretDTO(secret_type, b64_secret,"},{"line_number":92,"context_line":"                                ss.KeySpec(), secret_metadata[\u0027content_type\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"7996e16a_7659cffd","line":89,"range":{"start_line":88,"start_character":54,"end_line":89,"end_character":17},"in_reply_to":"6e36973f_6f9cb6b4","updated":"2021-06-21 15:30:52.000000000","message":"I think it\u0027s a fairly straightforward check here.  If the meta doesn\u0027t have a version, then we know it was stored prior to this fix.\n\nFor most secrets, that means the payload is already base64 encoded, so we can just stuff it into the DTO (which must be encoded).  The tricky part is figuring out which ones are not base64 encoded because they were generated by Castellan.\n\nOne important detail that can help us tell the difference is that orders require the bit length of the key you\u0027re generating.  So, generated secrets always have a bit length.\n\nWith that in mind, the logic should be:\n\n* if it has no version, and no bit length, then it was not generated and it is already b64 encoded.\n\n* if it has no version, and does have a bit length, then it may or may not be encoded.  By definition base64 encoding expands the data.  So, if the actual length of the payload is longer than the bit_length specified, then it is already base64 encoded.  If the actual length matches the bit_length, then we know we have plaintext and need to b64-encode before putting it in the DTO.","commit_id":"c49903b27c081196b90e9a4c493bddf2fc4514ea"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"0b2c3cb87a3fe8585e6bb11ce37d1d6c5c8c7626","unresolved":true,"context_lines":[{"line_number":85,"context_line":"                self.context,"},{"line_number":86,"context_line":"                secret_ref)"},{"line_number":87,"context_line":"            # TODO(dmendiza):  We need to check secret_meta version"},{"line_number":88,"context_line":"            # here to handle secrets that were stored before this"},{"line_number":89,"context_line":"            # fix."},{"line_number":90,"context_line":"            b64_secret \u003d base64.b64encode(secret.get_encoded())"},{"line_number":91,"context_line":"            return ss.SecretDTO(secret_type, b64_secret,"},{"line_number":92,"context_line":"                                ss.KeySpec(), secret_metadata[\u0027content_type\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"72656b9b_7ce2e117","line":89,"range":{"start_line":88,"start_character":54,"end_line":89,"end_character":17},"in_reply_to":"7996e16a_7659cffd","updated":"2021-06-21 16:03:26.000000000","message":"Ok, I think that should work.","commit_id":"c49903b27c081196b90e9a4c493bddf2fc4514ea"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"732d27020df3777d1650524b3e638e9455c25533","unresolved":true,"context_lines":[{"line_number":61,"context_line":"        for Story 2008335 are base64 encoded."},{"line_number":62,"context_line":"        \"\"\""},{"line_number":63,"context_line":"        payload \u003d secret.get_encoded()"},{"line_number":64,"context_line":"        if meta_bit_length is not None:"},{"line_number":65,"context_line":"            # Only asymmetric keys were stored with bit_length in the meta"},{"line_number":66,"context_line":"            # dict.  They are not base64 encoded."},{"line_number":67,"context_line":"            LOG.debug(\"Encoding legacy asymmetric key\")"},{"line_number":68,"context_line":"            return base64.b64encode(payload)"},{"line_number":69,"context_line":"        if hasattr(secret, \u0027bit_length\u0027):"},{"line_number":70,"context_line":"            # Only generated symmetric keys have a bit_length in Castellan."},{"line_number":71,"context_line":"            # They are not base64 encoded."},{"line_number":72,"context_line":"            LOG.debug(\"Encoding legacy symmetric key\")"},{"line_number":73,"context_line":"            return base64.b64encode(payload)"},{"line_number":74,"context_line":"        else:"},{"line_number":75,"context_line":"            # secret is likely already base64 encoded so we check that here to"},{"line_number":76,"context_line":"            # make sure."}],"source_content_type":"text/x-python","patch_set":11,"id":"f8052e44_14b72f17","line":73,"range":{"start_line":64,"start_character":0,"end_line":73,"end_character":44},"updated":"2021-07-26 16:27:18.000000000","message":"Why not check the type here rather than trying to infer from the bit length?","commit_id":"0405e10ea5f17da732e83d5f55b1ea53c923a810"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"6551fd9f7b8ea9d6dbcaa3d61fe591cc1b295d69","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        for Story 2008335 are base64 encoded."},{"line_number":62,"context_line":"        \"\"\""},{"line_number":63,"context_line":"        payload \u003d secret.get_encoded()"},{"line_number":64,"context_line":"        if meta_bit_length is not None:"},{"line_number":65,"context_line":"            # Only asymmetric keys were stored with bit_length in the meta"},{"line_number":66,"context_line":"            # dict.  They are not base64 encoded."},{"line_number":67,"context_line":"            LOG.debug(\"Encoding legacy asymmetric key\")"},{"line_number":68,"context_line":"            return base64.b64encode(payload)"},{"line_number":69,"context_line":"        if hasattr(secret, \u0027bit_length\u0027):"},{"line_number":70,"context_line":"            # Only generated symmetric keys have a bit_length in Castellan."},{"line_number":71,"context_line":"            # They are not base64 encoded."},{"line_number":72,"context_line":"            LOG.debug(\"Encoding legacy symmetric key\")"},{"line_number":73,"context_line":"            return base64.b64encode(payload)"},{"line_number":74,"context_line":"        else:"},{"line_number":75,"context_line":"            # secret is likely already base64 encoded so we check that here to"},{"line_number":76,"context_line":"            # make sure."}],"source_content_type":"text/x-python","patch_set":11,"id":"d64762a8_cc9d6265","line":73,"range":{"start_line":64,"start_character":0,"end_line":73,"end_character":44},"in_reply_to":"f8052e44_14b72f17","updated":"2021-09-13 20:39:27.000000000","message":"Finally had some time to discuss with Ade (the other active core),  he agrees that type checking would be better here.  I\u0027ll update the patch with type checks here instead.","commit_id":"0405e10ea5f17da732e83d5f55b1ea53c923a810"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"732d27020df3777d1650524b3e638e9455c25533","unresolved":true,"context_lines":[{"line_number":129,"context_line":"                #   the backend.  We need to base64 encode them for the DTO."},{"line_number":130,"context_line":"                data \u003d base64.b64encode(secret.get_encoded())"},{"line_number":131,"context_line":"            else:"},{"line_number":132,"context_line":"                # Unknown version - maybe a future version?"},{"line_number":133,"context_line":"                # assume future version will not be encoded."},{"line_number":134,"context_line":"                LOG.warning(\"Unknown metadata version\")"},{"line_number":135,"context_line":"                data \u003d base64.b64encode(secret.get_encoded())"},{"line_number":136,"context_line":"            return ss.SecretDTO(secret_type, data, ss.KeySpec(), None)"},{"line_number":137,"context_line":"        except Exception as e:"},{"line_number":138,"context_line":"            LOG.exception(\"Error retrieving secret {}: {}\".format("}],"source_content_type":"text/x-python","patch_set":11,"id":"0c5aa625_c530a38b","line":135,"range":{"start_line":132,"start_character":0,"end_line":135,"end_character":61},"updated":"2021-07-26 16:27:18.000000000","message":"Probably safer to bail here?","commit_id":"0405e10ea5f17da732e83d5f55b1ea53c923a810"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"6551fd9f7b8ea9d6dbcaa3d61fe591cc1b295d69","unresolved":false,"context_lines":[{"line_number":129,"context_line":"                #   the backend.  We need to base64 encode them for the DTO."},{"line_number":130,"context_line":"                data \u003d base64.b64encode(secret.get_encoded())"},{"line_number":131,"context_line":"            else:"},{"line_number":132,"context_line":"                # Unknown version - maybe a future version?"},{"line_number":133,"context_line":"                # assume future version will not be encoded."},{"line_number":134,"context_line":"                LOG.warning(\"Unknown metadata version\")"},{"line_number":135,"context_line":"                data \u003d base64.b64encode(secret.get_encoded())"},{"line_number":136,"context_line":"            return ss.SecretDTO(secret_type, data, ss.KeySpec(), None)"},{"line_number":137,"context_line":"        except Exception as e:"},{"line_number":138,"context_line":"            LOG.exception(\"Error retrieving secret {}: {}\".format("}],"source_content_type":"text/x-python","patch_set":11,"id":"45233941_7c10c015","line":135,"range":{"start_line":132,"start_character":0,"end_line":135,"end_character":61},"in_reply_to":"0c5aa625_c530a38b","updated":"2021-09-13 20:39:27.000000000","message":"Talked to Ade about this as well.  We probably don\u0027t need to worry about this scenario, as this patch will ensure that the meta_dict will either have the metadata or not.  We can revisit this if/when we add a new metadata dict version in the future.  I\u0027m going to remove the last clause from this if statement.","commit_id":"0405e10ea5f17da732e83d5f55b1ea53c923a810"},{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"d88b5f9fdf48d18db6c81fa190b5835d487a4471","unresolved":true,"context_lines":[{"line_number":117,"context_line":"            if meta_version is None:"},{"line_number":118,"context_line":"                # Secrets without a metadata version were stored prior to fix"},{"line_number":119,"context_line":"                # for Story 2008335.  They may or may not be base64-encoded."},{"line_number":120,"context_line":"                LOG.debug(\"Retreiving legacy secret\")"},{"line_number":121,"context_line":"                data \u003d self._ensure_legacy_base64(secret)"},{"line_number":122,"context_line":"            else:"},{"line_number":123,"context_line":"                # Version 1 - secret payload data is stored in plaintext in"}],"source_content_type":"text/x-python","patch_set":12,"id":"851b1a98_ca53eb49","line":120,"range":{"start_line":120,"start_character":27,"end_line":120,"end_character":37},"updated":"2021-09-15 03:26:25.000000000","message":"nit: typo","commit_id":"b49520930fabc868d8e9cc831586d35f05e96592"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"9f7a81325eda8091b4efd0a41e1e2d8b280bc6c7","unresolved":false,"context_lines":[{"line_number":117,"context_line":"            if meta_version is None:"},{"line_number":118,"context_line":"                # Secrets without a metadata version were stored prior to fix"},{"line_number":119,"context_line":"                # for Story 2008335.  They may or may not be base64-encoded."},{"line_number":120,"context_line":"                LOG.debug(\"Retreiving legacy secret\")"},{"line_number":121,"context_line":"                data \u003d self._ensure_legacy_base64(secret)"},{"line_number":122,"context_line":"            else:"},{"line_number":123,"context_line":"                # Version 1 - secret payload data is stored in plaintext in"}],"source_content_type":"text/x-python","patch_set":12,"id":"09126006_0613bdb6","line":120,"range":{"start_line":120,"start_character":27,"end_line":120,"end_character":37},"in_reply_to":"851b1a98_ca53eb49","updated":"2021-09-15 14:43:13.000000000","message":"Fixed","commit_id":"b49520930fabc868d8e9cc831586d35f05e96592"}],"barbican/tests/plugin/test_castellan_secret_store.py":[{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"732d27020df3777d1650524b3e638e9455c25533","unresolved":true,"context_lines":[{"line_number":176,"context_line":"            secret_dto"},{"line_number":177,"context_line":"        )"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"    def test_get_secret(self):"},{"line_number":180,"context_line":"        secret_metadata \u003d self.plugin._meta_dict(key_ref1, 256, \u0027AES\u0027)"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"        response \u003d self.plugin.get_secret("},{"line_number":183,"context_line":"            ss.SecretType.SYMMETRIC,"},{"line_number":184,"context_line":"            secret_metadata"},{"line_number":185,"context_line":"        )"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"        self.assertIsInstance(response, ss.SecretDTO)"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"        plaintext \u003d base64.b64decode(response.secret)"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        self.assertEqual(ss.SecretType.SYMMETRIC, response.type)"},{"line_number":192,"context_line":"        self.assertEqual(mock_key, plaintext)"},{"line_number":193,"context_line":"        self.plugin.key_manager.get.assert_called_once_with("},{"line_number":194,"context_line":"            mock.ANY,"},{"line_number":195,"context_line":"            key_ref1"},{"line_number":196,"context_line":"        )"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    def test_get_secret_throws_exception(self):"},{"line_number":199,"context_line":"        secret_metadata \u003d self.plugin._meta_dict(key_ref1, 256, \u0027AES\u0027)"}],"source_content_type":"text/x-python","patch_set":11,"id":"8a908bcd_63d5163e","line":196,"range":{"start_line":179,"start_character":0,"end_line":196,"end_character":9},"updated":"2021-07-26 16:27:18.000000000","message":"More of the logic could be covered here, including the legacy cases.","commit_id":"0405e10ea5f17da732e83d5f55b1ea53c923a810"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"6551fd9f7b8ea9d6dbcaa3d61fe591cc1b295d69","unresolved":true,"context_lines":[{"line_number":176,"context_line":"            secret_dto"},{"line_number":177,"context_line":"        )"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"    def test_get_secret(self):"},{"line_number":180,"context_line":"        secret_metadata \u003d self.plugin._meta_dict(key_ref1, 256, \u0027AES\u0027)"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"        response \u003d self.plugin.get_secret("},{"line_number":183,"context_line":"            ss.SecretType.SYMMETRIC,"},{"line_number":184,"context_line":"            secret_metadata"},{"line_number":185,"context_line":"        )"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"        self.assertIsInstance(response, ss.SecretDTO)"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"        plaintext \u003d base64.b64decode(response.secret)"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        self.assertEqual(ss.SecretType.SYMMETRIC, response.type)"},{"line_number":192,"context_line":"        self.assertEqual(mock_key, plaintext)"},{"line_number":193,"context_line":"        self.plugin.key_manager.get.assert_called_once_with("},{"line_number":194,"context_line":"            mock.ANY,"},{"line_number":195,"context_line":"            key_ref1"},{"line_number":196,"context_line":"        )"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    def test_get_secret_throws_exception(self):"},{"line_number":199,"context_line":"        secret_metadata \u003d self.plugin._meta_dict(key_ref1, 256, \u0027AES\u0027)"}],"source_content_type":"text/x-python","patch_set":11,"id":"434c1238_d21bedbf","line":196,"range":{"start_line":179,"start_character":0,"end_line":196,"end_character":9},"in_reply_to":"8a908bcd_63d5163e","updated":"2021-09-13 20:39:27.000000000","message":"I think we would need way too many mocks to unit-test the legacy cases, and I\u0027m not sure how useful that would be.\n\nIdeally we should be testing that using something like Grenade, so that we can store secrets with an old stable version, and then try to retrieve them using the latest codebase.","commit_id":"0405e10ea5f17da732e83d5f55b1ea53c923a810"}],"releasenotes/notes/fix-story-2008335-a253190d0fa799a0.yaml":[{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"732d27020df3777d1650524b3e638e9455c25533","unresolved":true,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Fixed Story 2008335: Fixed a data encoding issue in the Hashicorp Vault"},{"line_number":5,"context_line":"    backend that was causing errors when retrieving keys that were generated"},{"line_number":6,"context_line":"    by Vault."}],"source_content_type":"text/x-yaml","patch_set":11,"id":"b6d799e9_5d7c011c","line":6,"range":{"start_line":6,"start_character":7,"end_line":6,"end_character":12},"updated":"2021-07-26 16:27:18.000000000","message":"nit: The keys are generated by the Castellan Vault key manager.","commit_id":"0405e10ea5f17da732e83d5f55b1ea53c923a810"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"6551fd9f7b8ea9d6dbcaa3d61fe591cc1b295d69","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Fixed Story 2008335: Fixed a data encoding issue in the Hashicorp Vault"},{"line_number":5,"context_line":"    backend that was causing errors when retrieving keys that were generated"},{"line_number":6,"context_line":"    by Vault."}],"source_content_type":"text/x-yaml","patch_set":11,"id":"6a9793cd_0e306105","line":6,"range":{"start_line":6,"start_character":7,"end_line":6,"end_character":12},"in_reply_to":"b6d799e9_5d7c011c","updated":"2021-09-13 20:39:27.000000000","message":"Fixed","commit_id":"0405e10ea5f17da732e83d5f55b1ea53c923a810"}]}