)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"814ce4c9a39fef83f55292cd1a3913f5ee68b243","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"00f44af6_0be0a207","updated":"2023-03-29 08:18:58.000000000","message":"2023.2 is open now but I still have comments for improvement so changing vote to -1.","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d9d0fd7395c16addc4e33c48851d293a5711acfc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c3ede84e_6dd18399","updated":"2022-12-05 03:31:03.000000000","message":"Few doubts and suggestions inline.","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"75f289bd842ba2be20ba882fe85537ebcac1ff9b","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"171086e0_1c99226b","updated":"2022-11-24 10:20:35.000000000","message":"I am wondering about how Fernet would work for the backup chunks and potential side effect. Maybe we can discuss this during the midcycle?","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"000cb9d7537031b16252f4716a6290f8457fb81e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"076a4c50_26908aa4","updated":"2022-12-09 05:10:40.000000000","message":"I think we can go a little further than Christian R.\u0027s comments do. Consider this. If the assume that the argument for having static keys is solid, and the rotation is mandatory, having just two keys may not be enough if the keys are used to encrypt the data. The data gravity makes it infeasible to re-encrypt everything when keys roll.\n\nSo, as Christian said, we need to consider encrypting only a small amount with static keys - a metadata or a superblock if you will - and then store actual data keys there. Then, it may be possible to update these metadata or superblocks when keys roll. Note that we must supply the redundancy for the list of data keys. Also, it must be encrypted in a smart way: for example, not have a magic number that can provide a known plaintext to an attacker. It must have a checksum that allows us to know if/when the supplied static key decrypts correctly.\n\nOh we can go with Barbician from the start.","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"0f4140deca6a856529ac478262a046c0d6c81f6b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"f4d9b577_353b2952","updated":"2023-07-19 16:43:50.000000000","message":"I\u0027d like to work on this.  I think Christian\u0027s suggestion for primary/secondary keys as is done in keystone makes the most sense.  Gorka: do you mind if I post a spec update to address the existing comments to try to reach consensus?","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":30615,"name":"Tushar Trambak Gite","email":"tushargite96@gmail.com","username":"tushargite96"},"change_message_id":"f2a49f5b74569e07e85f4db82a674aa4119a3ddf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"3a47f6b0_63315973","updated":"2022-11-24 04:30:31.000000000","message":"LGTM","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"fed189f4ff2af679749b2ff9322a06787e85ea9d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"133129ed_9552529d","updated":"2022-11-03 13:44:34.000000000","message":"Looks good to me","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"4e96b947da75128637415f344468e838780f3dd4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"beff4c1f_201641c5","updated":"2023-11-22 14:16:31.000000000","message":"Need to be reproposed to 2024.1 spec dir","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"8f2a02eaa3ded484e33533cb2e4c2b09a7fde0be","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"ea2b5f8c_04a95bb9","updated":"2022-11-24 10:28:53.000000000","message":"Pete I added you to the list of reviewers as I am wondering about your ideas on using streaming I/O for backups.","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"08b3f3612907ac65a067ef2947e15c22b0d994ee","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"78101a5a_c454269c","updated":"2023-01-05 14:25:45.000000000","message":"This needs further discussion on the fernet token part and we are past spec deadline (23 December, 2022). As per discussion in the meeting yesterday[1], adding a procedural -2.\n\n[1] https://meetings.opendev.org/meetings/cinder/2023/cinder.2023-01-04-14.00.log.html#l-22","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"2d7c57704aca30f017264f9e3cf13541e8331f3d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"3bfca4fa_4e2a5a8b","updated":"2023-03-13 16:05:41.000000000","message":"Will you resubmit this for the 2023.2 release?\nHaving an \n\na) performant (worked on in other changes)\nb) encrypted (this)\n\nchunked based backup enables many more use cases / backup strategies.","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"5ca832e663ccb3fe46cd45a903fa8bb394ce4d87","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"f7c23e07_a8af716b","in_reply_to":"beff4c1f_201641c5","updated":"2023-11-22 20:00:07.000000000","message":"Will do.","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"73d24c470044c8c40d471c79d1f67bf272c7965c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"779797eb_e83f6928","in_reply_to":"f4d9b577_353b2952","updated":"2023-07-26 13:24:09.000000000","message":"Thanks Jon, I would appreciate if you could take over the spec writing effort, thanks.","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"003e6b6d1dd26bdcb8b13f82271dfccc14403bb0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ad732b25_77bc8f3e","updated":"2024-06-08 01:50:28.000000000","message":"(I\u0027m only commenting to clear my drafts ahead of the takeover)","commit_id":"0cfece2ed6af598b97959ae23024344f4817809f"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"302730c6d35fa043dba5bfaf4640ecec94cb1bed","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f94cd577_f456daa5","updated":"2023-12-07 18:52:06.000000000","message":"Gah! Work in progress, not workflow, sorry!","commit_id":"0cfece2ed6af598b97959ae23024344f4817809f"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"af016f5340f8982a1a8267b7e3984dd637f33ebb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e1b06b79_f897a297","updated":"2024-02-08 14:42:40.000000000","message":"How does this interact with the backup import/export functionality?  Does it work at all?  Is it allowed for encrypted backups?","commit_id":"0cfece2ed6af598b97959ae23024344f4817809f"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"062a9c64a1a48735791b45182516fd16b289bb78","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f13f3722_2fb71629","updated":"2023-12-07 18:51:36.000000000","message":"I still need to update the spec to address above comments.","commit_id":"0cfece2ed6af598b97959ae23024344f4817809f"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"6b4020ac2306917791c306eeb76af99794950867","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"8e181359_8e2b7181","updated":"2024-02-17 02:02:23.000000000","message":"I think that my own, Rajat\u0027s, and Christian\u0027s comments on the patchset 1 were somewhat useful. My comments this time (set 2) are partially redundant with set 1. But hopefully not as verbose.\n\nI heard no argument against allowing key rotation. This time I\u0027m a little more specific about the new configuration parameters. Note that I avoided an array. Not sure if this is really necessary.\n\nI wish to avoid a new column in the DB if at all possible.\n\nI\u0027m going to start with a pilot implementation now, and we can have both it and the spec adjusted as discussion progresses.","commit_id":"0cfece2ed6af598b97959ae23024344f4817809f"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"835f2c787195926cee32de24e6ecd1f38072d2e1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"04b932ca_52e2c1bd","updated":"2023-12-20 13:13:55.000000000","message":"No comments from the last PS are addressed.","commit_id":"0cfece2ed6af598b97959ae23024344f4817809f"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"c85011260ca76bb4a3b7c8a5c6c48591ea2b574d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c80779ee_03981fbd","updated":"2024-05-02 15:02:40.000000000","message":"The spec should mention AES-CTR and why that mode is being chosen (per the posted code in https://review.opendev.org/c/openstack/cinder/+/915192 ).","commit_id":"0cfece2ed6af598b97959ae23024344f4817809f"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"985cb1d0eb3bef40c07a192b675a49846c9133f8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"91ba9d12_7f1747cc","in_reply_to":"e1b06b79_f897a297","updated":"2024-02-19 22:01:20.000000000","message":"I think that oddly enough, everything should work. Every backup is assigned one static key. And the management of these keys is placed on sysadmins. As long as the export copies the encrypted data without damaging it to the new place, it should be possible to restore it if the administrator preserved the key. Just another reason allow more keys than one: the backup could be imported into a cloud that already has encrypted backups of its own.","commit_id":"0cfece2ed6af598b97959ae23024344f4817809f"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"58b3a697681a9cf8a8a7aa0cf7670fd354375c12","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"8e250aab_006ec6cc","updated":"2024-06-10 23:33:14.000000000","message":"Should this be re-targeted to 2024.2?","commit_id":"dcce0d325a5e11eca686afdbcf46e70114edcd21"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"f0335a6aaff6e4dac6cb1f0647f421da15a497ac","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"ca97afa1_a514abc5","updated":"2024-07-30 19:18:39.000000000","message":"Having implemented the spec (see the latest pilot [1]), it is now my opinion that specifying Fernet was a mistake, for 3 reasons:\n\n1) Fernet always returns base64, so the size of backups is unnecessarily inflated.\n2) Fernet insists on random IV, so unit tests cannot examine the ciphertext without extensive monkey-patching.\n3) Fernet is not compatible with any kind of streaming, and I planned to introduce streaming compression.\n\n1: https://review.opendev.org/c/openstack/cinder/+/915192","commit_id":"7fcc717532f886978c26a6a6ecac93145c716296"},{"author":{"_account_id":37328,"name":"Nimesh Desai","display_name":"Nimesh Desai","email":"nimesh.desai@ibm.com","username":"nimeshdesai"},"change_message_id":"56ef237a5eb2cd2e2ced27ef229426f561537d82","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"ff897011_9a0d0e36","updated":"2025-12-05 15:04:36.000000000","message":"Review the text for consistent grammar and standardised formatting for clarity.","commit_id":"7fcc717532f886978c26a6a6ecac93145c716296"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"b1bede6b4a45d727542debf1dc7356853b5265a2","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"b73ea1aa_d1d0c179","in_reply_to":"ca97afa1_a514abc5","updated":"2024-07-31 07:00:07.000000000","message":"See my comments about Fernet https://review.opendev.org/c/openstack/cinder-specs/+/862601/comment/971a68a1_9de3c7e1/","commit_id":"7fcc717532f886978c26a6a6ecac93145c716296"}],"specs/2023.1/chunked-backup-encryption.rst":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d9d0fd7395c16addc4e33c48851d293a5711acfc","unresolved":true,"context_lines":[{"line_number":10,"context_line":""},{"line_number":11,"context_line":"https://blueprints.launchpad.net/cinder/+spec/encrypted-backups"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Backups inheriting from the Chunked Backup Driver always store data unencrypted"},{"line_number":14,"context_line":"which may not be undesirable in some scenarios."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"c67f07d0_90037de6","line":13,"range":{"start_line":13,"start_character":68,"end_line":13,"end_character":79},"updated":"2022-12-05 03:31:03.000000000","message":"in unencrypted form?","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"75f289bd842ba2be20ba882fe85537ebcac1ff9b","unresolved":true,"context_lines":[{"line_number":11,"context_line":"https://blueprints.launchpad.net/cinder/+spec/encrypted-backups"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Backups inheriting from the Chunked Backup Driver always store data unencrypted"},{"line_number":14,"context_line":"which may not be undesirable in some scenarios."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Problem description"}],"source_content_type":"text/x-rst","patch_set":1,"id":"46a29a19_efd831ad","line":14,"updated":"2022-11-24 10:20:35.000000000","message":"nit: \"which may not be desirable\" (double negation)","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d9d0fd7395c16addc4e33c48851d293a5711acfc","unresolved":true,"context_lines":[{"line_number":11,"context_line":"https://blueprints.launchpad.net/cinder/+spec/encrypted-backups"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Backups inheriting from the Chunked Backup Driver always store data unencrypted"},{"line_number":14,"context_line":"which may not be undesirable in some scenarios."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Problem description"}],"source_content_type":"text/x-rst","patch_set":1,"id":"fe1d5f0c_54bedaf5","line":14,"range":{"start_line":14,"start_character":6,"end_line":14,"end_character":28},"updated":"2022-12-05 03:31:03.000000000","message":"sounds like a double negative. can be either of two:\nmay be undesirable\nmay not be desirable","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"000cb9d7537031b16252f4716a6290f8457fb81e","unresolved":true,"context_lines":[{"line_number":22,"context_line":"forced to trust that the provider has tight control over its access."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"This is not the best policy with sensitive information, and it is recommended"},{"line_number":25,"context_line":"to encrypt sensitive data."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Unfortunately in OpenStack all public cloud drivers, GCS and S3, do not support"},{"line_number":28,"context_line":"encryption, which means that anyone with access to the OpenStack backup"}],"source_content_type":"text/x-rst","patch_set":1,"id":"c15560cf_4126d871","line":25,"updated":"2022-12-09 05:10:40.000000000","message":"I am against judgements like \"not a best policy\" and appeals to unspecified authority (recommended by whom?). I prefer sticking to facts with something like: \"Encrypting data stored in a public cloud adds an additional layer of protection beyond the access controls of the provider.\"","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"000cb9d7537031b16252f4716a6290f8457fb81e","unresolved":true,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"As a system administrator I want to configure my OpenStack deployment to make"},{"line_number":37,"context_line":"my backups to S3 without having to worry about having sensitive data readable"},{"line_number":38,"context_line":"for anyone that has access to the S3 account, authorized or not."},{"line_number":39,"context_line":""},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"Proposed change"}],"source_content_type":"text/x-rst","patch_set":1,"id":"10163479_6ed7e96c","line":38,"updated":"2022-12-09 05:10:40.000000000","message":"Rather than decreasing the worry, I would rather decrease the risk due to specific attack vectors such as the compromise at the provider. The existing wording about \"anyone having data readable\" is sufficient, although I would replace \"sensitive\" with \"backup\". If you want to link the sensitivity level with decision-making, it needs to be explicit (e.g. sensitive data - encrypt it; not sensitive - dump it naked with 1 layer of protection by authorization).","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"000cb9d7537031b16252f4716a6290f8457fb81e","unresolved":true,"context_lines":[{"line_number":42,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"The proposal is to support encrypted backups for the S3 and GCS Cinder backup"},{"line_number":45,"context_line":"drivers using a single static encryption key for all encrypted backups."},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"Users would **not** be allowed to select whether they want encryption or not,"},{"line_number":48,"context_line":"as this would be configured by the system administrator that configures the"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5118719e_b0616472","line":45,"updated":"2022-12-09 05:10:40.000000000","message":"I don\u0027t think the single key is going to fly. It is essential to change keys. See the comments by Christian Rohmann about primary/secondary, and most importantly, only encrypting a metadata store (which can be re-encrypted upon a key roll-over).","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d9d0fd7395c16addc4e33c48851d293a5711acfc","unresolved":true,"context_lines":[{"line_number":49,"context_line":"Cinder services and knows where backups are being stored."},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"Note:"},{"line_number":52,"context_line":"  The RBD driver would not support encryption."},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"The reason to use a single key instead of per backup encryption keys using"},{"line_number":55,"context_line":"Barbican, like Cinder does for volumes, is because the primary objective of"}],"source_content_type":"text/x-rst","patch_set":1,"id":"2fc6989b_a99a328e","line":52,"range":{"start_line":52,"start_character":17,"end_line":52,"end_character":22},"updated":"2022-12-05 03:31:03.000000000","message":"nit: will","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"000cb9d7537031b16252f4716a6290f8457fb81e","unresolved":true,"context_lines":[{"line_number":49,"context_line":"Cinder services and knows where backups are being stored."},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"Note:"},{"line_number":52,"context_line":"  The RBD driver would not support encryption."},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"The reason to use a single key instead of per backup encryption keys using"},{"line_number":55,"context_line":"Barbican, like Cinder does for volumes, is because the primary objective of"}],"source_content_type":"text/x-rst","patch_set":1,"id":"20cba31b_ed26922d","line":52,"range":{"start_line":52,"start_character":17,"end_line":52,"end_character":22},"in_reply_to":"2fc6989b_a99a328e","updated":"2022-12-09 05:10:40.000000000","message":"There\u0027s a similar \"would\" one paragraph up as well.","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"000cb9d7537031b16252f4716a6290f8457fb81e","unresolved":true,"context_lines":[{"line_number":53,"context_line":""},{"line_number":54,"context_line":"The reason to use a single key instead of per backup encryption keys using"},{"line_number":55,"context_line":"Barbican, like Cinder does for volumes, is because the primary objective of"},{"line_number":56,"context_line":"encrypting backups is to prevent people that has access to S3 from reading the"},{"line_number":57,"context_line":"contents of the backups, and in a disaster scenario where Barbican store is"},{"line_number":58,"context_line":"also lost, all backups would be rendered unusable by the loss of the keys."},{"line_number":59,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"68b00754_0479bf3f","line":56,"range":{"start_line":56,"start_character":45,"end_line":56,"end_character":48},"updated":"2022-12-09 05:10:40.000000000","message":"have","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d9d0fd7395c16addc4e33c48851d293a5711acfc","unresolved":true,"context_lines":[{"line_number":58,"context_line":"also lost, all backups would be rendered unusable by the loss of the keys."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"The proposal is to have 3 new configuration options ``backup_encryption_mode``"},{"line_number":61,"context_line":"and ``backup_encryption_algorithm``, ``backup_encryption_key`` to configure how"},{"line_number":62,"context_line":"chunked backup drivers would handle encryption."},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"The ``backup_encryption_mode`` would accept 3 different options: ``off``,"}],"source_content_type":"text/x-rst","patch_set":1,"id":"dbec4713_617e8d55","line":61,"range":{"start_line":61,"start_character":35,"end_line":61,"end_character":36},"updated":"2022-12-05 03:31:03.000000000","message":"nit: and","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d9d0fd7395c16addc4e33c48851d293a5711acfc","unresolved":true,"context_lines":[{"line_number":58,"context_line":"also lost, all backups would be rendered unusable by the loss of the keys."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"The proposal is to have 3 new configuration options ``backup_encryption_mode``"},{"line_number":61,"context_line":"and ``backup_encryption_algorithm``, ``backup_encryption_key`` to configure how"},{"line_number":62,"context_line":"chunked backup drivers would handle encryption."},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"The ``backup_encryption_mode`` would accept 3 different options: ``off``,"}],"source_content_type":"text/x-rst","patch_set":1,"id":"7acc0635_a740d257","line":61,"range":{"start_line":61,"start_character":0,"end_line":61,"end_character":3},"updated":"2022-12-05 03:31:03.000000000","message":"nit: comma","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d9d0fd7395c16addc4e33c48851d293a5711acfc","unresolved":true,"context_lines":[{"line_number":59,"context_line":""},{"line_number":60,"context_line":"The proposal is to have 3 new configuration options ``backup_encryption_mode``"},{"line_number":61,"context_line":"and ``backup_encryption_algorithm``, ``backup_encryption_key`` to configure how"},{"line_number":62,"context_line":"chunked backup drivers would handle encryption."},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"The ``backup_encryption_mode`` would accept 3 different options: ``off``,"},{"line_number":65,"context_line":"``always``, and ``auto``, where:"}],"source_content_type":"text/x-rst","patch_set":1,"id":"57d4f444_2c8605af","line":62,"range":{"start_line":62,"start_character":0,"end_line":62,"end_character":22},"updated":"2022-12-05 03:31:03.000000000","message":"In the proposed change, we mention only GCS and S3 to support this encryption but here we mentioned chunked backup drivers. Posix driver also inherits from ChunkedBackupDriver and NFS driver inherits from Posix driver.\nMy confusion is, are we supporting this for any driver inheriting from ChunkedBackupDriver or specifically for GCS and S3? I understand GCS and S3 backups are stored remotely so we need this extra layer of protection but trying to understand the implementation.","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d9d0fd7395c16addc4e33c48851d293a5711acfc","unresolved":true,"context_lines":[{"line_number":62,"context_line":"chunked backup drivers would handle encryption."},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"The ``backup_encryption_mode`` would accept 3 different options: ``off``,"},{"line_number":65,"context_line":"``always``, and ``auto``, where:"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"- ``off`` would disable encryption, just like we do for the"},{"line_number":68,"context_line":"  ``backup_compression_algorithm``, for example if we were using our own Swift"}],"source_content_type":"text/x-rst","patch_set":1,"id":"2d1d5ea8_ee786ce5","line":65,"range":{"start_line":65,"start_character":2,"end_line":65,"end_character":8},"updated":"2022-12-05 03:31:03.000000000","message":"below (L#71) we mention \"on\" and here we are describing \"always\", i think \"on\" goes better with \"off\" so we can modify this to \"on\"","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d9d0fd7395c16addc4e33c48851d293a5711acfc","unresolved":true,"context_lines":[{"line_number":68,"context_line":"  ``backup_compression_algorithm``, for example if we were using our own Swift"},{"line_number":69,"context_line":"  cluster."},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"- ``on`` would force all backup to be encrypted."},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"- ``auto`` would only encrypt unencrypted volumes and would backup already"},{"line_number":74,"context_line":"  encrypted volumes as they are, since double encryption would not provide"}],"source_content_type":"text/x-rst","patch_set":1,"id":"86b035be_ce9a0747","line":71,"range":{"start_line":71,"start_character":25,"end_line":71,"end_character":31},"updated":"2022-12-05 03:31:03.000000000","message":"nit: backups","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"2c9db249f57b658e4f661719fd86ee2e14dfd61f","unresolved":true,"context_lines":[{"line_number":74,"context_line":"  encrypted volumes as they are, since double encryption would not provide"},{"line_number":75,"context_line":"  additional security."},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"The ``backup_encryption_key`` would be a string configuration option."},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"For simplicity this initial implementation will only support the ``Fernet``"},{"line_number":80,"context_line":"algorithm from the ``cryptography`` python module, that Cinder already"}],"source_content_type":"text/x-rst","patch_set":1,"id":"971a68a1_9de3c7e1","line":77,"updated":"2022-11-07 10:24:02.000000000","message":"I\u0027d align the Fernet key management with how Keystone does it (https://docs.openstack.org/keystone/zed/admin/fernet-token-faq.html#what-are-the-different-types-of-keys) and have a primary and also a secondary key to allow for key rotation.\n\nAlso having the keys as dedicated files (like Keystone) might make it easier \"sync\" or otherwise handle them in config management.","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"75f289bd842ba2be20ba882fe85537ebcac1ff9b","unresolved":true,"context_lines":[{"line_number":74,"context_line":"  encrypted volumes as they are, since double encryption would not provide"},{"line_number":75,"context_line":"  additional security."},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"The ``backup_encryption_key`` would be a string configuration option."},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"For simplicity this initial implementation will only support the ``Fernet``"},{"line_number":80,"context_line":"algorithm from the ``cryptography`` python module, that Cinder already"}],"source_content_type":"text/x-rst","patch_set":1,"id":"ecd43ba5_cb47853b","line":77,"in_reply_to":"971a68a1_9de3c7e1","updated":"2022-11-24 10:20:35.000000000","message":"1. I still stand with my argument to allow for a possible key rollover. Otherwise a single key would have to live forever. A big no no in cryptography.\n\n2. But I am wondering if Fernet is really a good fit for this use case and how it would actually be applied for backups and their chunks?\n \n a) Fernet is meant for \"tokens\", so small pieces of data. That data is not only AES encrypted but also receives an HMAC (SHA256) and will then be base64 encoded (see https://cryptography.io/en/latest/fernet/#cryptography.fernet.Fernet.encrypt)\n\n While the HMAC might be a addition to ensure the integrity and authenticity of the  remotely stored data, base64 encoding will create lots of additional overhead.\n\n See this issue with performance numbers included:  https://github.com/pyca/cryptography/issues/4953\n\n b) Fernet documents that the message has to fit into memory (see: https://cryptography.io/en/latest/fernet/#limitations), this is somewhat limiting the chunk size and is likely not very efficient. Gorka mentioned that Pete Zaitcev was looking into using streaming IO for backups - something I have looked into as well. Using Fernet encrypt() method would also hinder these kind of improvements, compared to using a \"streaming\" AES encryption for example.\n \n c) Or am I misunderstanding how you intended to apply Fernet?","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"000cb9d7537031b16252f4716a6290f8457fb81e","unresolved":true,"context_lines":[{"line_number":74,"context_line":"  encrypted volumes as they are, since double encryption would not provide"},{"line_number":75,"context_line":"  additional security."},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"The ``backup_encryption_key`` would be a string configuration option."},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"For simplicity this initial implementation will only support the ``Fernet``"},{"line_number":80,"context_line":"algorithm from the ``cryptography`` python module, that Cinder already"}],"source_content_type":"text/x-rst","patch_set":1,"id":"541715cb_2c240cf8","line":77,"in_reply_to":"d438f727_89378830","updated":"2022-12-09 05:10:40.000000000","message":"I haven\u0027t looked into Fernet\u0027s implications for streaming of backups yet. In fact, I didn\u0027t account for the upcoming encryption work.","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"5cd0831fa0fc0e68f58b92ad549a2c18cce00a68","unresolved":true,"context_lines":[{"line_number":74,"context_line":"  encrypted volumes as they are, since double encryption would not provide"},{"line_number":75,"context_line":"  additional security."},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"The ``backup_encryption_key`` would be a string configuration option."},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"For simplicity this initial implementation will only support the ``Fernet``"},{"line_number":80,"context_line":"algorithm from the ``cryptography`` python module, that Cinder already"}],"source_content_type":"text/x-rst","patch_set":1,"id":"d438f727_89378830","line":77,"in_reply_to":"ecd43ba5_cb47853b","updated":"2022-11-30 15:29:33.000000000","message":"Maybe just using Fernet and their keys to encrypt the remotely stored metadata would work. The metadata could then contain the actual (symetric, AES) key used to encrypt the backup data. It\u0027s stored with the metadata when it\u0027s uploaded and it\u0027s read back (and decrypted via Fernet) on restore.\n\nThis is somewhat similar to how Restic does it (see https://restic.readthedocs.io/en/stable/100_references.html#keys-encryption-and-mac) storing \"JSON documents which contain all data that is needed to derive the repository’s master encryption and message authentication keys from a user’s password\".","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d9d0fd7395c16addc4e33c48851d293a5711acfc","unresolved":true,"context_lines":[{"line_number":96,"context_line":"volume is already encrypted."},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"Backup drivers supporting encryption shall fail to start if the"},{"line_number":99,"context_line":"``backup_encryption_algorithm`` has an incorrect value or if"},{"line_number":100,"context_line":"``backup_encryption_mode`` is not ``off`` and there is no valid"},{"line_number":101,"context_line":"``backup_encryption_key``."},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"The new code should be able to support doing an incremental encrypted backup"},{"line_number":104,"context_line":"even if the parent backup is non encrypted, and even if the parent was made by"}],"source_content_type":"text/x-rst","patch_set":1,"id":"0669f9a4_35862e44","line":101,"range":{"start_line":99,"start_character":0,"end_line":101,"end_character":26},"updated":"2022-12-05 03:31:03.000000000","message":"took me some time to understand but basically we\u0027ve 2 conditions,\n1) ``backup_encryption_algorithm`` has an incorrect value\n2)  ``backup_encryption_mode`` is ``on`` or ``auto`` AND we\u0027ve no valid ``backup_encryption_key``\n\nwould be better to rephrase this in  2 points for clarity","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"7f3a462d91763093b476e0d2649b94089422d89e","unresolved":true,"context_lines":[{"line_number":107,"context_line":"method based on each backup\u0027s metadata, and not assume that all backups from a"},{"line_number":108,"context_line":"backup chain are the same."},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"In terms of code this is quite easy, in the ``restore`` method it just needs to"},{"line_number":111,"context_line":"move the assignment of the ``restore_func`` to the ``while`` loop after the"},{"line_number":112,"context_line":"``backup1`` metadata has been read."},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"When compression is enabled with the ``backup_compression_algorithm`` the"},{"line_number":115,"context_line":"compression should be done prior to the encryption, since encryption turns the"}],"source_content_type":"text/x-rst","patch_set":1,"id":"381e49fe_7f5133ae","line":112,"range":{"start_line":110,"start_character":0,"end_line":112,"end_character":35},"updated":"2022-11-30 11:53:42.000000000","message":"Out of curiosity, Isn\u0027t it necessary to pass the Fernet token to the restore method?","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d9d0fd7395c16addc4e33c48851d293a5711acfc","unresolved":true,"context_lines":[{"line_number":107,"context_line":"method based on each backup\u0027s metadata, and not assume that all backups from a"},{"line_number":108,"context_line":"backup chain are the same."},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"In terms of code this is quite easy, in the ``restore`` method it just needs to"},{"line_number":111,"context_line":"move the assignment of the ``restore_func`` to the ``while`` loop after the"},{"line_number":112,"context_line":"``backup1`` metadata has been read."},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"When compression is enabled with the ``backup_compression_algorithm`` the"},{"line_number":115,"context_line":"compression should be done prior to the encryption, since encryption turns the"}],"source_content_type":"text/x-rst","patch_set":1,"id":"0f1fece0_98ab7e29","line":112,"range":{"start_line":110,"start_character":0,"end_line":112,"end_character":35},"updated":"2022-12-05 03:31:03.000000000","message":"which while loop are we referring to? and what is backup1? just a normal backup we are trying to restore or anything specific to code?\nI don\u0027t mind if we don\u0027t include the low level details but if mentioned, would be better to describe it using link references.","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"000cb9d7537031b16252f4716a6290f8457fb81e","unresolved":true,"context_lines":[{"line_number":124,"context_line":"whether backups are encrypted or not for them, but since this information could"},{"line_number":125,"context_line":"be useful for system administrators it will need to make it visible for them."},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"For this purpose a new DB column called ``encrypted`` will be added to the"},{"line_number":128,"context_line":"``backups`` table to reflect whether the data in that specific backup is"},{"line_number":129,"context_line":"encrypted or not.  A backup will be encrypted whenever an encrypted volume is"},{"line_number":130,"context_line":"backed up or when an unencrypted volume is backup up and the chunked backup"},{"line_number":131,"context_line":"driver encrypts it."},{"line_number":132,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"61b22484_1f67e04c","line":129,"range":{"start_line":127,"start_character":0,"end_line":129,"end_character":17},"updated":"2022-12-09 05:10:40.000000000","message":"Wait a moment, we spent so much effort on justifying static keys and now we need DB to operate? I think we ought to mention that class Backup will have a new member (that corresponds to the column in the DB), and that this class can be found in the metadata and de-serialized in the disaster recovery case.","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d9d0fd7395c16addc4e33c48851d293a5711acfc","unresolved":true,"context_lines":[{"line_number":126,"context_line":""},{"line_number":127,"context_line":"For this purpose a new DB column called ``encrypted`` will be added to the"},{"line_number":128,"context_line":"``backups`` table to reflect whether the data in that specific backup is"},{"line_number":129,"context_line":"encrypted or not.  A backup will be encrypted whenever an encrypted volume is"},{"line_number":130,"context_line":"backed up or when an unencrypted volume is backup up and the chunked backup"},{"line_number":131,"context_line":"driver encrypts it."},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"It\u0027s important to reflect in the documentation that this field only reflects"}],"source_content_type":"text/x-rst","patch_set":1,"id":"42d6d790_022505c6","line":130,"range":{"start_line":129,"start_character":19,"end_line":130,"end_character":9},"updated":"2022-12-05 03:31:03.000000000","message":"IIUC, this is already the case and we are not modifying anything to achieve this right?","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"000cb9d7537031b16252f4716a6290f8457fb81e","unresolved":true,"context_lines":[{"line_number":126,"context_line":""},{"line_number":127,"context_line":"For this purpose a new DB column called ``encrypted`` will be added to the"},{"line_number":128,"context_line":"``backups`` table to reflect whether the data in that specific backup is"},{"line_number":129,"context_line":"encrypted or not.  A backup will be encrypted whenever an encrypted volume is"},{"line_number":130,"context_line":"backed up or when an unencrypted volume is backup up and the chunked backup"},{"line_number":131,"context_line":"driver encrypts it."},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"It\u0027s important to reflect in the documentation that this field only reflects"}],"source_content_type":"text/x-rst","patch_set":1,"id":"4cadc0c9_735d6a34","line":130,"range":{"start_line":129,"start_character":19,"end_line":130,"end_character":9},"in_reply_to":"42d6d790_022505c6","updated":"2022-12-09 05:10:40.000000000","message":"Also, this description contradicts to what the backup_encryption_mode\u003dauto does, it looks to me.","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"000cb9d7537031b16252f4716a6290f8457fb81e","unresolved":true,"context_lines":[{"line_number":133,"context_line":"It\u0027s important to reflect in the documentation that this field only reflects"},{"line_number":134,"context_line":"that specific backup in a chain of backups, so we could have an encrypted"},{"line_number":135,"context_line":"incremental backup that states that it is encrypted but its parent is"},{"line_number":136,"context_line":"unencrypted."},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"Alternatives"},{"line_number":139,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"a90aa012_96150aae","line":136,"updated":"2022-12-09 05:10:40.000000000","message":"IMHO the requirement to encrypt incrementals for an unencrypted full backup needs to be discussed. I\u0027m not convinced it\u0027s necessary.","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d9d0fd7395c16addc4e33c48851d293a5711acfc","unresolved":true,"context_lines":[{"line_number":145,"context_line":"-----------------"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"This change will require the addition of a new DB column called ``encrypted``"},{"line_number":148,"context_line":"in the ``backups`` table that defaults to ``False`` to be able to detect ."},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"Besides the DB change in ``cinder/db/sqlalchemy/models.py`` the corresponding"},{"line_number":151,"context_line":"``Backup`` Oslo Versioned Object (OVO) in ``cinder/objects/backup.py`` will"}],"source_content_type":"text/x-rst","patch_set":1,"id":"8deccdc0_d83140c2","line":148,"range":{"start_line":148,"start_character":63,"end_line":148,"end_character":74},"updated":"2022-12-05 03:31:03.000000000","message":"is this an unfinished sentence?","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"000cb9d7537031b16252f4716a6290f8457fb81e","unresolved":true,"context_lines":[{"line_number":177,"context_line":""},{"line_number":178,"context_line":"A new REST API microversion is needed since admins will now be able to see the"},{"line_number":179,"context_line":"``encrypted`` field for backups."},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"Security impact"},{"line_number":182,"context_line":"---------------"},{"line_number":183,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"e27106d3_70d95a8c","line":180,"updated":"2022-12-09 05:10:40.000000000","message":"I\u0027m not sure this is true. The version of Backup is bumped, but all the RPC arguments are the same.","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"000cb9d7537031b16252f4716a6290f8457fb81e","unresolved":true,"context_lines":[{"line_number":181,"context_line":"Security impact"},{"line_number":182,"context_line":"---------------"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"Security will be increased when using external public clouds for Cinder"},{"line_number":185,"context_line":"backups."},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"Active/Active HA impact"}],"source_content_type":"text/x-rst","patch_set":1,"id":"bbbe26d4_8ae02cc3","line":184,"range":{"start_line":184,"start_character":38,"end_line":184,"end_character":60},"updated":"2022-12-09 05:10:40.000000000","message":"The \"external public\" seems redundant.","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d9d0fd7395c16addc4e33c48851d293a5711acfc","unresolved":true,"context_lines":[{"line_number":221,"context_line":""},{"line_number":222,"context_line":"- ``backup_encryption_mode``"},{"line_number":223,"context_line":"- ``backup_encryption_algorithm``"},{"line_number":224,"context_line":"- ``backup_encryption_key``"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"Developer impact"},{"line_number":227,"context_line":"----------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"029093ea_ebcb9f07","line":224,"range":{"start_line":224,"start_character":4,"end_line":224,"end_character":25},"updated":"2022-12-05 03:31:03.000000000","message":"I\u0027m confused about this parameter. what will be the value of this? If this is a static key and someone creates a backup with it and changes this to another key, then while restoring/decrypting, we won\u0027t have the original key available so won\u0027t this be a problem?","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"000cb9d7537031b16252f4716a6290f8457fb81e","unresolved":true,"context_lines":[{"line_number":221,"context_line":""},{"line_number":222,"context_line":"- ``backup_encryption_mode``"},{"line_number":223,"context_line":"- ``backup_encryption_algorithm``"},{"line_number":224,"context_line":"- ``backup_encryption_key``"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"Developer impact"},{"line_number":227,"context_line":"----------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"42c65878_f11715f5","line":224,"range":{"start_line":224,"start_character":4,"end_line":224,"end_character":25},"in_reply_to":"029093ea_ebcb9f07","updated":"2022-12-09 05:10:40.000000000","message":"Indeed this is a key question.","commit_id":"5ac61f754e0cba46de18c871e4f59b2dc038fcc9"}],"specs/2024.1/chunked-backup-encryption.rst":[{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"6b4020ac2306917791c306eeb76af99794950867","unresolved":true,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Backups inheriting from the Chunked Backup Driver always store data unencrypted"},{"line_number":14,"context_line":"which may not be undesirable in some scenarios."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Problem description"},{"line_number":18,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":2,"id":"9cc5334d_cb1bd072","line":15,"updated":"2024-02-17 02:02:23.000000000","message":"\"may be undesirable\" surely","commit_id":"0cfece2ed6af598b97959ae23024344f4817809f"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"003e6b6d1dd26bdcb8b13f82271dfccc14403bb0","unresolved":false,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Backups inheriting from the Chunked Backup Driver always store data unencrypted"},{"line_number":14,"context_line":"which may not be undesirable in some scenarios."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Problem description"},{"line_number":18,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":2,"id":"3ac42d14_dd07b484","line":15,"in_reply_to":"9cc5334d_cb1bd072","updated":"2024-06-08 01:50:28.000000000","message":"Done","commit_id":"0cfece2ed6af598b97959ae23024344f4817809f"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"6b4020ac2306917791c306eeb76af99794950867","unresolved":true,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Keeping data in public cloud storage services, such as GCS and S3, means that"},{"line_number":21,"context_line":"access control to that information is only partially in our control, and we are"},{"line_number":22,"context_line":"forced to trust that the provider has tight control over its access."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"This is not the best policy with sensitive information, and it is recommended"},{"line_number":25,"context_line":"to encrypt sensitive data."}],"source_content_type":"text/x-rst","patch_set":2,"id":"ce1d6e9a_2775f621","line":22,"updated":"2024-02-17 02:02:23.000000000","message":"just \"access to that information\" sounds better","commit_id":"0cfece2ed6af598b97959ae23024344f4817809f"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"003e6b6d1dd26bdcb8b13f82271dfccc14403bb0","unresolved":false,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Keeping data in public cloud storage services, such as GCS and S3, means that"},{"line_number":21,"context_line":"access control to that information is only partially in our control, and we are"},{"line_number":22,"context_line":"forced to trust that the provider has tight control over its access."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"This is not the best policy with sensitive information, and it is recommended"},{"line_number":25,"context_line":"to encrypt sensitive data."}],"source_content_type":"text/x-rst","patch_set":2,"id":"95fd273b_20e19474","line":22,"in_reply_to":"ce1d6e9a_2775f621","updated":"2024-06-08 01:50:28.000000000","message":"Done","commit_id":"0cfece2ed6af598b97959ae23024344f4817809f"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"6b4020ac2306917791c306eeb76af99794950867","unresolved":true,"context_lines":[{"line_number":23,"context_line":""},{"line_number":24,"context_line":"This is not the best policy with sensitive information, and it is recommended"},{"line_number":25,"context_line":"to encrypt sensitive data."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Unfortunately in OpenStack all public cloud drivers, GCS and S3, do not support"},{"line_number":28,"context_line":"encryption, which means that anyone with access to the OpenStack backup"},{"line_number":29,"context_line":"location will be able to access the raw data of all my unencrypted Cinder"}],"source_content_type":"text/x-rst","patch_set":2,"id":"5c44f6a2_80aeb23f","line":26,"updated":"2024-02-17 02:02:23.000000000","message":"No need to appeal to an undefined recommendation authority. Just plainly state that encrypting backups removes the hazard of direct access to raw backups at provider, and therefore we should implement it.","commit_id":"0cfece2ed6af598b97959ae23024344f4817809f"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"003e6b6d1dd26bdcb8b13f82271dfccc14403bb0","unresolved":false,"context_lines":[{"line_number":23,"context_line":""},{"line_number":24,"context_line":"This is not the best policy with sensitive information, and it is recommended"},{"line_number":25,"context_line":"to encrypt sensitive data."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Unfortunately in OpenStack all public cloud drivers, GCS and S3, do not support"},{"line_number":28,"context_line":"encryption, which means that anyone with access to the OpenStack backup"},{"line_number":29,"context_line":"location will be able to access the raw data of all my unencrypted Cinder"}],"source_content_type":"text/x-rst","patch_set":2,"id":"dc75dbe8_c1cc4578","line":26,"in_reply_to":"5c44f6a2_80aeb23f","updated":"2024-06-08 01:50:28.000000000","message":"Done","commit_id":"0cfece2ed6af598b97959ae23024344f4817809f"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"6b4020ac2306917791c306eeb76af99794950867","unresolved":true,"context_lines":[{"line_number":29,"context_line":"location will be able to access the raw data of all my unencrypted Cinder"},{"line_number":30,"context_line":"volumes."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"Use Cases"},{"line_number":34,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":35,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"70aa7eac_0f056afc","line":32,"updated":"2024-02-17 02:02:23.000000000","message":"Overall, the problem definiton is much too verbose, IMHO. But it is correct.","commit_id":"0cfece2ed6af598b97959ae23024344f4817809f"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"003e6b6d1dd26bdcb8b13f82271dfccc14403bb0","unresolved":false,"context_lines":[{"line_number":29,"context_line":"location will be able to access the raw data of all my unencrypted Cinder"},{"line_number":30,"context_line":"volumes."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"Use Cases"},{"line_number":34,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":35,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"af79f7f7_82065953","line":32,"in_reply_to":"70aa7eac_0f056afc","updated":"2024-06-08 01:50:28.000000000","message":"Acknowledged","commit_id":"0cfece2ed6af598b97959ae23024344f4817809f"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"6b4020ac2306917791c306eeb76af99794950867","unresolved":true,"context_lines":[{"line_number":43,"context_line":""},{"line_number":44,"context_line":"The proposal is to support encrypted backups for the S3 and GCS Cinder backup"},{"line_number":45,"context_line":"drivers using a single static encryption key for all encrypted backups."},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"Users would **not** be allowed to select whether they want encryption or not,"},{"line_number":48,"context_line":"as this would be configured by the system administrator that configures the"},{"line_number":49,"context_line":"Cinder services and knows where backups are being stored."}],"source_content_type":"text/x-rst","patch_set":2,"id":"6e57743f_4e7d75b0","line":46,"updated":"2024-02-17 02:02:23.000000000","message":"This was mentioned in the patchset 1 comments: not having a key rotation is a non-starter, IMHO. Static keys offer advantages in DR scenario, but let\u0027s at least have 2, at least for the restore side. I\u0027m concerned about a shift in a government mandate would require a weird workaround like running 2 Cinder backup services simultaneously.","commit_id":"0cfece2ed6af598b97959ae23024344f4817809f"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"6b4020ac2306917791c306eeb76af99794950867","unresolved":true,"context_lines":[{"line_number":60,"context_line":"The proposal is to have 3 new configuration options ``backup_encryption_mode``"},{"line_number":61,"context_line":"and ``backup_encryption_algorithm``, ``backup_encryption_key`` to configure how"},{"line_number":62,"context_line":"chunked backup drivers would handle encryption."},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"The ``backup_encryption_mode`` would accept 3 different options: ``off``,"},{"line_number":65,"context_line":"``always``, and ``auto``, where:"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"b56855a3_1e2b8999","line":63,"updated":"2024-02-17 02:02:23.000000000","message":"I think we\u0027ll need to have backup_decryption_key_a and backup_decryption_key_b, with a syntax \"fernet:NNNNNNNN\". Other equivalent configuration parameters are possible, I\u0027m not wedded to the specifics.","commit_id":"0cfece2ed6af598b97959ae23024344f4817809f"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"003e6b6d1dd26bdcb8b13f82271dfccc14403bb0","unresolved":false,"context_lines":[{"line_number":60,"context_line":"The proposal is to have 3 new configuration options ``backup_encryption_mode``"},{"line_number":61,"context_line":"and ``backup_encryption_algorithm``, ``backup_encryption_key`` to configure how"},{"line_number":62,"context_line":"chunked backup drivers would handle encryption."},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"The ``backup_encryption_mode`` would accept 3 different options: ``off``,"},{"line_number":65,"context_line":"``always``, and ``auto``, where:"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"ec8ce7ec_8d49733e","line":63,"in_reply_to":"b56855a3_1e2b8999","updated":"2024-06-08 01:50:28.000000000","message":"I\u0027m working on a more refined proposal than key_a and key_b. Stay tuned for an update.","commit_id":"0cfece2ed6af598b97959ae23024344f4817809f"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"6b4020ac2306917791c306eeb76af99794950867","unresolved":true,"context_lines":[{"line_number":84,"context_line":""},{"line_number":85,"context_line":"To support encrypted backup we need a new version of the *metadata* file format"},{"line_number":86,"context_line":"to be able to store the encryption algorithm to mark the backup as encrypted."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"First we would bump the ``DRIVER_VERSION`` and update the"},{"line_number":89,"context_line":"``DRIVER_VERSION_MAPPING`` to include the restore method for the new version,"},{"line_number":90,"context_line":"which would support decrypting volumes."}],"source_content_type":"text/x-rst","patch_set":2,"id":"a905c3bb_8ad6dea0","line":87,"updated":"2024-02-17 02:02:23.000000000","message":"In that case you can save a fingerprint of the key in the metadata too. Sounds like good news for my demand of supporting 2 or more decryption keys.","commit_id":"0cfece2ed6af598b97959ae23024344f4817809f"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"003e6b6d1dd26bdcb8b13f82271dfccc14403bb0","unresolved":false,"context_lines":[{"line_number":84,"context_line":""},{"line_number":85,"context_line":"To support encrypted backup we need a new version of the *metadata* file format"},{"line_number":86,"context_line":"to be able to store the encryption algorithm to mark the backup as encrypted."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"First we would bump the ``DRIVER_VERSION`` and update the"},{"line_number":89,"context_line":"``DRIVER_VERSION_MAPPING`` to include the restore method for the new version,"},{"line_number":90,"context_line":"which would support decrypting volumes."}],"source_content_type":"text/x-rst","patch_set":2,"id":"32c36d38_896fb98e","line":87,"in_reply_to":"a905c3bb_8ad6dea0","updated":"2024-06-08 01:50:28.000000000","message":"Acknowledged","commit_id":"0cfece2ed6af598b97959ae23024344f4817809f"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"6b4020ac2306917791c306eeb76af99794950867","unresolved":true,"context_lines":[{"line_number":128,"context_line":"``backups`` table to reflect whether the data in that specific backup is"},{"line_number":129,"context_line":"encrypted or not.  A backup will be encrypted whenever an encrypted volume is"},{"line_number":130,"context_line":"backed up or when an unencrypted volume is backup up and the chunked backup"},{"line_number":131,"context_line":"driver encrypts it."},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"It\u0027s important to reflect in the documentation that this field only reflects"},{"line_number":134,"context_line":"that specific backup in a chain of backups, so we could have an encrypted"}],"source_content_type":"text/x-rst","patch_set":2,"id":"c78b7c62_a645e0db","line":131,"updated":"2024-02-17 02:02:23.000000000","message":"I\u0027m not sure if this is necessary. Keeping this information in DB allows operators to list backups and obtain the information about the encryption. So that much is good (although it needs to be weighed against accessing the metadata in S3 and getting the truth from the source). But do we not have a better space to keep this than a whole new column?","commit_id":"0cfece2ed6af598b97959ae23024344f4817809f"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"6b4020ac2306917791c306eeb76af99794950867","unresolved":true,"context_lines":[{"line_number":170,"context_line":"data migrations when updating the schema,  a new online data migration will"},{"line_number":171,"context_line":"need to be added to the ``cinder/cmd/manage.py`` CLI and to the ``Backup`` OVO"},{"line_number":172,"context_line":"when a record is read from the DB and it doesn\u0027t have the ``encrypted`` value"},{"line_number":173,"context_line":"set."},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"REST API impact"},{"line_number":176,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"ca986e6a_636776b8","line":173,"updated":"2024-02-17 02:02:23.000000000","message":"The impact upon migrations and upgrades is why I wish to explore ways to live without schema changes, and the consequences (e.g. extra access to S3).","commit_id":"0cfece2ed6af598b97959ae23024344f4817809f"}],"specs/2024.2/chunked-backup-encryption.rst":[{"author":{"_account_id":37328,"name":"Nimesh Desai","display_name":"Nimesh Desai","email":"nimesh.desai@ibm.com","username":"nimeshdesai"},"change_message_id":"56ef237a5eb2cd2e2ced27ef229426f561537d82","unresolved":true,"context_lines":[{"line_number":65,"context_line":"- ``off`` disables encryption, just like we do for the"},{"line_number":66,"context_line":"  ``backup_compression_algorithm``. This is useful for backups stored"},{"line_number":67,"context_line":"  in a secure, private back-end, and delivers the best performance."},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"- ``on`` forces all backups to be encrypted."},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"- ``auto`` configures the service to encrypt unencrypted volumes only,"},{"line_number":72,"context_line":"  and backup already encrypted volumes as they are, since double encryption"}],"source_content_type":"text/x-rst","patch_set":4,"id":"cc0dcef3_24ab5099","line":69,"range":{"start_line":68,"start_character":0,"end_line":69,"end_character":43},"updated":"2025-12-05 15:04:36.000000000","message":"this option should be \u0027always\u0027","commit_id":"7fcc717532f886978c26a6a6ecac93145c716296"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"1bec0276c4519ec1b4cc4739192f0180587c97bf","unresolved":true,"context_lines":[{"line_number":74,"context_line":""},{"line_number":75,"context_line":"The ``backup_encryption_key`` would be a string configuration option."},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"For simplicity this initial implementation will only support the ``Fernet``"},{"line_number":78,"context_line":"algorithm from the ``cryptography`` python module, that Cinder already"},{"line_number":79,"context_line":"requires, which is a symmetric authenticated cryptography.  It will be the"},{"line_number":80,"context_line":"default algorithm and will be configured with the value ``fernet`` in the"}],"source_content_type":"text/x-rst","patch_set":4,"id":"f0afcdf5_866a6745","line":77,"updated":"2024-07-31 14:06:52.000000000","message":"I think this spec needs to explain why we are choosing Fernet instead of just AES etc. encryption.  This seems rather unclear.","commit_id":"7fcc717532f886978c26a6a6ecac93145c716296"},{"author":{"_account_id":37328,"name":"Nimesh Desai","display_name":"Nimesh Desai","email":"nimesh.desai@ibm.com","username":"nimeshdesai"},"change_message_id":"56ef237a5eb2cd2e2ced27ef229426f561537d82","unresolved":true,"context_lines":[{"line_number":260,"context_line":"Testing"},{"line_number":261,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"Besides the standard unit tests one of the tempest CI jobs should be modified"},{"line_number":264,"context_line":"to run with encryption enabled and the backup tempest tests should be made to"},{"line_number":265,"context_line":"look for the value of the new ``encrypted`` field in the admin REST API"},{"line_number":266,"context_line":"responses and confirm its not present for normal users."}],"source_content_type":"text/x-rst","patch_set":4,"id":"6f1201f2_d7ff4091","line":263,"range":{"start_line":263,"start_character":21,"end_line":263,"end_character":31},"updated":"2025-12-05 15:04:36.000000000","message":"Should it also mention about negative tests?\n- wrong/missing restore key etc","commit_id":"7fcc717532f886978c26a6a6ecac93145c716296"}]}
