)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"629c0d462de2fd0af55ee2b34f707a60085b6b07","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"ee180f07_ceb68c25","updated":"2024-06-14 15:56:51.000000000","message":"Thanks, Stephen this looks great.\n\n\u0027Prevent\u0027 use of web-download is too strong, but I don\u0027t know how important that is to the document. Everything else I wrote is just context which you\u0027re welcome to use or now.","commit_id":"9ab17a9f3887ee0283833c567c4dfcc2cfe46fb7"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"5e3fa5ebe522e29b1e2647c73d99c85b7cf28915","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"ddda3d9a_6de052d1","updated":"2024-06-25 15:17:24.000000000","message":"I still have doubt over renaming the config option, I am not sure whether we need to follow deprecation policy for it or not!!","commit_id":"65bc7a2973671705148b7ba8610faf774657fc0c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e0cdd49e1d32c6fce9daa165c34c2ab4d61933b1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"220e1eba_1a3f7433","updated":"2024-07-18 15:57:21.000000000","message":"I submitted the fixes as a follow-up since I think the compressed/uncompressed image thing needs more thought.","commit_id":"65bc7a2973671705148b7ba8610faf774657fc0c"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"af9d78b580e3ab5109419744226adf1d77f3588d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"c0f2c82b_452752b3","updated":"2024-06-21 15:35:00.000000000","message":"Nitpicking for a couple of typos, but the proposal looks solid to me.","commit_id":"65bc7a2973671705148b7ba8610faf774657fc0c"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"ccff81ad5f39c36dbd6307592053dce9e3ea22a6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"09698c61_6d91da96","updated":"2024-06-26 05:25:07.000000000","message":"Should we also provide a discovery call to show supported hashing algorithms (similar to import-info or stores-info)?","commit_id":"65bc7a2973671705148b7ba8610faf774657fc0c"},{"author":{"_account_id":19138,"name":"Pranali Deore","email":"pdeore@redhat.com","username":"PranaliD"},"change_message_id":"57140729f5dd3f6dd24e58d1d5affaefb2d0f581","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"15fde10a_8ce06a4d","updated":"2024-07-02 13:29:51.000000000","message":"The proposal looks good to me, but before merging this spec and going forward, I think all the existing comments should be answered and marked as resolved.\n\nThank you!!","commit_id":"65bc7a2973671705148b7ba8610faf774657fc0c"},{"author":{"_account_id":19138,"name":"Pranali Deore","email":"pdeore@redhat.com","username":"PranaliD"},"change_message_id":"a469efccd153effcdd85a60ca50423a912827532","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"3861c486_df9f4553","in_reply_to":"15fde10a_8ce06a4d","updated":"2024-07-18 14:23:42.000000000","message":"Since we have passed the spec freeze deadline and there are no updates here from the spec owner, we have to move this to next cycle unfortunately.","commit_id":"65bc7a2973671705148b7ba8610faf774657fc0c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5b183d30524df334419550691da9403b354055f5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"8db539b9_46e290ce","in_reply_to":"3861c486_df9f4553","updated":"2024-07-18 16:22:44.000000000","message":"As with https://review.opendev.org/c/openstack/glance-specs/+/922116/, I\u0027m hoping that the fact there\u0027s broad agreement on this plus WIP code means we can revisit this. I\u0027m happy to complete this work in D and will set aside time specifically.","commit_id":"65bc7a2973671705148b7ba8610faf774657fc0c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4406976b7c5bb14a9d46aa023b456c56c8e1cec4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"e38c5bef_7ff76833","in_reply_to":"ddda3d9a_6de052d1","updated":"2024-06-25 16:59:03.000000000","message":"We will follow the deprecation policy. We will provide a deprecated alias so that operators can continue to use the old option for a while, but they will get a warning on startup and it will _eventually_ stop working. For example, from nova-cpu logs on a local DevStack environment:\n\n```\nJun 11 13:01:05 stephenfin-devstack nova-compute[102446]: WARNING oslo_config.cfg [None req-a3bed986-6120-4caf-abc6-7b30b1810139 None None] Deprecated: Option \"cpu_model\" from group \"libvirt\" is deprecated. Use option \"cpu_models\" from group \"libvirt\".\n```\n\nWe\u0027ll see the same but for `[DEFAULT] hashing_algorithm` -\u003e `[DEFAULT] default_hashing_algorithm`.","commit_id":"65bc7a2973671705148b7ba8610faf774657fc0c"},{"author":{"_account_id":19138,"name":"Pranali Deore","email":"pdeore@redhat.com","username":"PranaliD"},"change_message_id":"fcf1a0615e5a34b8ec368975dd5fa3351ab00204","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"1566eec5_646fdc64","updated":"2024-07-25 10:48:35.000000000","message":"After discussing with Stephen, I think we can go ahead with this as the implementation patches are ready and up for review and looks possible to get in\nthis cycle.\n\nApproving the spec !!\n\nThanks !","commit_id":"4c4dd06f46f542061e3862ef5c5d00aba17b8309"}],"specs/2024.2/approved/glance/configurable-hash-algorithms.rst":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"629c0d462de2fd0af55ee2b34f707a60085b6b07","unresolved":true,"context_lines":[{"line_number":41,"context_line":"either ask an admin to change the hash algorithm (which affects the entire"},{"line_number":42,"context_line":"deployment and might not be possible, particularly in public cloud"},{"line_number":43,"context_line":"environments) or generate the hash on the client-side before uploading the"},{"line_number":44,"context_line":"image (which prevents use of the ``web-download`` image import mechanism). In"},{"line_number":45,"context_line":"our particular instance, this is one of the issues preventing us from using"},{"line_number":46,"context_line":"``web-download`` to upload OpenShift release images since only SHA-256"},{"line_number":47,"context_line":"signatures are provided for these."}],"source_content_type":"text/x-rst","patch_set":1,"id":"e8fabf2d_0ea07593","line":44,"updated":"2024-06-14 15:56:51.000000000","message":"It doesn\u0027t prevent use of web-download, but it does remove much of the usefulness of it.\n\nWriting my use case as:\n\nI want to create a glance image based on a qcow2 for which I only have metadata, including a download URL. I must be able to verify that glance has correctly created the expected image.\n\nThe benefits web-download gives me:\n* I don\u0027t have to download the image\n* I don\u0027t have to upload the image\n\nIf I have to calculate my own hash value instead of using the one from the metadata I\u0027ve already got, I lose the first benefit because I have to download the image. I still have the second benefit, though, because glance can still download it directly and generate a hash, just not a hash I can use easily.","commit_id":"9ab17a9f3887ee0283833c567c4dfcc2cfe46fb7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4efb1654d24a6f5f1d300ff8781f8690095688d7","unresolved":false,"context_lines":[{"line_number":41,"context_line":"either ask an admin to change the hash algorithm (which affects the entire"},{"line_number":42,"context_line":"deployment and might not be possible, particularly in public cloud"},{"line_number":43,"context_line":"environments) or generate the hash on the client-side before uploading the"},{"line_number":44,"context_line":"image (which prevents use of the ``web-download`` image import mechanism). In"},{"line_number":45,"context_line":"our particular instance, this is one of the issues preventing us from using"},{"line_number":46,"context_line":"``web-download`` to upload OpenShift release images since only SHA-256"},{"line_number":47,"context_line":"signatures are provided for these."}],"source_content_type":"text/x-rst","patch_set":1,"id":"b351b696_c7617398","line":44,"in_reply_to":"e8fabf2d_0ea07593","updated":"2024-06-17 14:49:13.000000000","message":"Done","commit_id":"9ab17a9f3887ee0283833c567c4dfcc2cfe46fb7"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"629c0d462de2fd0af55ee2b34f707a60085b6b07","unresolved":true,"context_lines":[{"line_number":43,"context_line":"environments) or generate the hash on the client-side before uploading the"},{"line_number":44,"context_line":"image (which prevents use of the ``web-download`` image import mechanism). In"},{"line_number":45,"context_line":"our particular instance, this is one of the issues preventing us from using"},{"line_number":46,"context_line":"``web-download`` to upload OpenShift release images since only SHA-256"},{"line_number":47,"context_line":"signatures are provided for these."},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"Proposed change"}],"source_content_type":"text/x-rst","patch_set":1,"id":"897cc8e1_87483d8d","line":46,"updated":"2024-06-14 15:56:51.000000000","message":"Also stock Ubuntu and CentOS images if we want to highlight that the scope of the problem extends beyond OpenShift.","commit_id":"9ab17a9f3887ee0283833c567c4dfcc2cfe46fb7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4efb1654d24a6f5f1d300ff8781f8690095688d7","unresolved":false,"context_lines":[{"line_number":43,"context_line":"environments) or generate the hash on the client-side before uploading the"},{"line_number":44,"context_line":"image (which prevents use of the ``web-download`` image import mechanism). In"},{"line_number":45,"context_line":"our particular instance, this is one of the issues preventing us from using"},{"line_number":46,"context_line":"``web-download`` to upload OpenShift release images since only SHA-256"},{"line_number":47,"context_line":"signatures are provided for these."},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"Proposed change"}],"source_content_type":"text/x-rst","patch_set":1,"id":"da885d04_bae87a5b","line":46,"in_reply_to":"897cc8e1_87483d8d","updated":"2024-06-17 14:49:13.000000000","message":"Done","commit_id":"9ab17a9f3887ee0283833c567c4dfcc2cfe46fb7"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"629c0d462de2fd0af55ee2b34f707a60085b6b07","unresolved":true,"context_lines":[{"line_number":54,"context_line":"hashing_algorithm`` config option to ````[DEFAULT] default_hashing_algorithm``"},{"line_number":55,"context_line":"and add a new config option, ``[DEFAULT] allowed_hashing_algorithms``, which"},{"line_number":56,"context_line":"will be a list of algorithms that are permitted to specified by the user and"},{"line_number":57,"context_line":"will initially default to ``[\u0027sha512\u0027, \u0027sha256\u0027, \u0027md5\u0027]``. In this way, users"},{"line_number":58,"context_line":"can select a hashing algorithm that suits their use case while operators can"},{"line_number":59,"context_line":"still restrict certain algorithms to e.g. maintain regulatory compliance."},{"line_number":60,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"24b9d53b_654510a8","line":57,"updated":"2024-06-14 15:56:51.000000000","message":"Probably worth including SHA1 here as well.\n\nAs noted, both MD5 and SHA1 are known to be insecure so at the very least the user-facing documentation both for `allowed_hashing_algorithms` and any new hash algorithm field on the create image api should be clear that their use is not recommended.\n\nDepending on circumstances, some clouds, e.g. a corporate cloud, may take the view that MD5 and SHA1 are never acceptable. A public cloud may make a different choice: a user may feel that if they only have MD5, then verifying an MD5 signature is better than no verification. Also note that as the field would be in the DB, it would be possible to generate reports of which images are using which hashing algorithms.","commit_id":"9ab17a9f3887ee0283833c567c4dfcc2cfe46fb7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4efb1654d24a6f5f1d300ff8781f8690095688d7","unresolved":false,"context_lines":[{"line_number":54,"context_line":"hashing_algorithm`` config option to ````[DEFAULT] default_hashing_algorithm``"},{"line_number":55,"context_line":"and add a new config option, ``[DEFAULT] allowed_hashing_algorithms``, which"},{"line_number":56,"context_line":"will be a list of algorithms that are permitted to specified by the user and"},{"line_number":57,"context_line":"will initially default to ``[\u0027sha512\u0027, \u0027sha256\u0027, \u0027md5\u0027]``. In this way, users"},{"line_number":58,"context_line":"can select a hashing algorithm that suits their use case while operators can"},{"line_number":59,"context_line":"still restrict certain algorithms to e.g. maintain regulatory compliance."},{"line_number":60,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"e2b0664e_293a7a60","line":57,"in_reply_to":"24b9d53b_654510a8","updated":"2024-06-17 14:49:13.000000000","message":"Done","commit_id":"9ab17a9f3887ee0283833c567c4dfcc2cfe46fb7"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"17fbe55155570a3100b3b63b6dfaff863534ca0e","unresolved":true,"context_lines":[{"line_number":51,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"To resolve this, we propose allowing users to select the hash algorithm to be"},{"line_number":54,"context_line":"used when creating the image. We will rename the ``[DEFAULT]"},{"line_number":55,"context_line":"hashing_algorithm`` config option to ````[DEFAULT] default_hashing_algorithm``"},{"line_number":56,"context_line":"and add a new config option, ``[DEFAULT] allowed_hashing_algorithms``, which"},{"line_number":57,"context_line":"will be a list of algorithms that are permitted to specified by the user and"}],"source_content_type":"text/x-rst","patch_set":2,"id":"282d0ac1_795d8d70","line":54,"updated":"2024-06-18 14:48:59.000000000","message":"Instead of renaming config option (for which we need to follow deprecation policy of 2 cycles) and introducing new config parameter I think we can add `choices` attribute to current hashing_algorithm as ``choices\u003d[\u0027sha512\u0027, \u0027sha256\u0027, \u0027sha1\u0027, \u0027md5\u0027]``.","commit_id":"2dd5df180620195ee5e0235a5c662d26efb3dd9b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5ce9ddd231f0612c8021a888527ce2342d9ec7c1","unresolved":false,"context_lines":[{"line_number":51,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"To resolve this, we propose allowing users to select the hash algorithm to be"},{"line_number":54,"context_line":"used when creating the image. We will rename the ``[DEFAULT]"},{"line_number":55,"context_line":"hashing_algorithm`` config option to ````[DEFAULT] default_hashing_algorithm``"},{"line_number":56,"context_line":"and add a new config option, ``[DEFAULT] allowed_hashing_algorithms``, which"},{"line_number":57,"context_line":"will be a list of algorithms that are permitted to specified by the user and"}],"source_content_type":"text/x-rst","patch_set":2,"id":"5a0168b2_4fd8bf22","line":54,"in_reply_to":"0893375e_d9d86cf0","updated":"2024-06-19 10:09:13.000000000","message":"Updated spec to capture this discussion.","commit_id":"2dd5df180620195ee5e0235a5c662d26efb3dd9b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b8606a691345438f63d6644e5607fbb17a6ea14e","unresolved":true,"context_lines":[{"line_number":51,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"To resolve this, we propose allowing users to select the hash algorithm to be"},{"line_number":54,"context_line":"used when creating the image. We will rename the ``[DEFAULT]"},{"line_number":55,"context_line":"hashing_algorithm`` config option to ````[DEFAULT] default_hashing_algorithm``"},{"line_number":56,"context_line":"and add a new config option, ``[DEFAULT] allowed_hashing_algorithms``, which"},{"line_number":57,"context_line":"will be a list of algorithms that are permitted to specified by the user and"}],"source_content_type":"text/x-rst","patch_set":2,"id":"0893375e_d9d86cf0","line":54,"in_reply_to":"282d0ac1_795d8d70","updated":"2024-06-19 09:57:36.000000000","message":"Choices restricts the permitted choices. We don\u0027t want to do that, since we\u0027ve explicitly said an operator can choose to use any hashing algorithm supported by the underlying hashlib library. From the docs for the `[DEFAULT] hashing_algorithm` config option.\n\n\u003e The value must be a valid secure hash algorithm name recognized by the\n\u003e python \u0027hashlib\u0027 library. ...\n\nPerhaps you meant to convert this from a string opt to an order-sensitive list of strings opt where the first opt (index 0) is the default but anything in the list will be permitted? If so, I did consider that but there were a few issues:\n\n1. It muddles the meaning of the option. The option now has two meanings: it is the option to configure the default hashing algorithm *and* it\u0027s the option to configure the allowed hashing algorithms. I think clarity trumps terseness/\"cleverness\".\n2. If anyone has overridden the default to set it to e.g. `sha256`, the override will now affect both the default hashing algorithm and the allowed hashing algorithms, when the user initially only meant to configure the former.\n1. I don\u0027t know if changing a type like that is permitted by oslo.config or what the knock-on impacts could be.\n\nSo these being the case, I think adding a new, separate option is a better choice.\n\nThe reason I think we should then renaming the existing option is that, now that we have two hashing algorithm-related options, I want to make it clearer what option does what. Renaming an option (or changing its group) is not expensive. We\u0027ve done it loads of times in Nova and the two cycle deprecation policy refers to the time we must wait until we can remove the deprecated alias. During that time though, both options will be supported and the user will merely get a warning if they use the older one. tbh, nova has some deprecated aliases that it has yet to remove some 5 or 6 years later so trust me, it\u0027s unlikely that the removal will be rushed 😉","commit_id":"2dd5df180620195ee5e0235a5c662d26efb3dd9b"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"7417225494b2da69814132f1281bf6da2be4414e","unresolved":true,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"https://blueprints.launchpad.net/glance/+spec/configurable-hash-algorithms"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Allow end-users to specify a hash algorithm to be used when creating a new"},{"line_number":16,"context_line":"image, allowing them to compare existing hashes provided by the creator of the"},{"line_number":17,"context_line":"image with hashes returned by Glance, rather than having to generate the hashes"},{"line_number":18,"context_line":"themselves on the client side."}],"source_content_type":"text/x-rst","patch_set":3,"id":"17a11b8d_0099e8e0","line":15,"updated":"2024-06-20 14:54:27.000000000","message":"We also need to allow the user to specify whether the hash should be calculated on the *data downloaded*, or the *resulting image*. These will be different if any processing is done on the image, for example decompression.\n\nTaking CentOS as an example[1], the project publishes SHA256 of the *compressed* data. In order to be able to use this to verify a glance web-download, we would need glance to compute the SHA256 of the downloaded data before decompression.\n\nFor RHCOS downloads we actually have both, although I suspect that is unusual. In practise I expect hashes of public compressed images to be of the compressed data.\n\n[1] https://cloud.centos.org/centos/9-stream/x86_64/images/CHECKSUM","commit_id":"65bc7a2973671705148b7ba8610faf774657fc0c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e0cdd49e1d32c6fce9daa165c34c2ab4d61933b1","unresolved":false,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"https://blueprints.launchpad.net/glance/+spec/configurable-hash-algorithms"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Allow end-users to specify a hash algorithm to be used when creating a new"},{"line_number":16,"context_line":"image, allowing them to compare existing hashes provided by the creator of the"},{"line_number":17,"context_line":"image with hashes returned by Glance, rather than having to generate the hashes"},{"line_number":18,"context_line":"themselves on the client side."}],"source_content_type":"text/x-rst","patch_set":3,"id":"0d684c25_db871192","line":15,"in_reply_to":"17a11b8d_0099e8e0","updated":"2024-07-18 15:57:21.000000000","message":"Done","commit_id":"65bc7a2973671705148b7ba8610faf774657fc0c"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"af9d78b580e3ab5109419744226adf1d77f3588d","unresolved":true,"context_lines":[{"line_number":56,"context_line":"hashing_algorithm`` config option to ``[DEFAULT] default_hashing_algorithm``"},{"line_number":57,"context_line":"(providing an alias for the older name) and add a new config option,"},{"line_number":58,"context_line":"``[DEFAULT] allowed_hashing_algorithms``, which will be a list of algorithms"},{"line_number":59,"context_line":"that are permitted to specified by the user and will initially default to"},{"line_number":60,"context_line":"``[\u0027sha512\u0027, \u0027sha256\u0027, \u0027sha1\u0027, \u0027md5\u0027]``. In this way, users can select a"},{"line_number":61,"context_line":"hashing algorithm that suits their use case while operators can still restrict"},{"line_number":62,"context_line":"certain algorithms to e.g. maintain regulatory compliance."}],"source_content_type":"text/x-rst","patch_set":3,"id":"ecf28e46_9b1c0369","line":59,"range":{"start_line":59,"start_character":22,"end_line":59,"end_character":31},"updated":"2024-06-21 15:35:00.000000000","message":"to *be* specified","commit_id":"65bc7a2973671705148b7ba8610faf774657fc0c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e0cdd49e1d32c6fce9daa165c34c2ab4d61933b1","unresolved":false,"context_lines":[{"line_number":56,"context_line":"hashing_algorithm`` config option to ``[DEFAULT] default_hashing_algorithm``"},{"line_number":57,"context_line":"(providing an alias for the older name) and add a new config option,"},{"line_number":58,"context_line":"``[DEFAULT] allowed_hashing_algorithms``, which will be a list of algorithms"},{"line_number":59,"context_line":"that are permitted to specified by the user and will initially default to"},{"line_number":60,"context_line":"``[\u0027sha512\u0027, \u0027sha256\u0027, \u0027sha1\u0027, \u0027md5\u0027]``. In this way, users can select a"},{"line_number":61,"context_line":"hashing algorithm that suits their use case while operators can still restrict"},{"line_number":62,"context_line":"certain algorithms to e.g. maintain regulatory compliance."}],"source_content_type":"text/x-rst","patch_set":3,"id":"35ea5ac1_2f18c314","line":59,"range":{"start_line":59,"start_character":22,"end_line":59,"end_character":31},"in_reply_to":"ecf28e46_9b1c0369","updated":"2024-07-18 15:57:21.000000000","message":"Done","commit_id":"65bc7a2973671705148b7ba8610faf774657fc0c"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"af9d78b580e3ab5109419744226adf1d77f3588d","unresolved":true,"context_lines":[{"line_number":74,"context_line":"  database changes, and would increase CPU utilisation due to the need to"},{"line_number":75,"context_line":"  generate multiple hashes for every image."},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"- Instead on introducing a new configuration option, we could modify the"},{"line_number":78,"context_line":"  behavior of the existing ``[DEFAULT] hashing_algorithm`` option such"},{"line_number":79,"context_line":"  that it now accepts a list of allowed hashing algorithms, with the first item"},{"line_number":80,"context_line":"  (index 0) being the default used."}],"source_content_type":"text/x-rst","patch_set":3,"id":"1f65c74f_b2842e7d","line":77,"range":{"start_line":77,"start_character":10,"end_line":77,"end_character":12},"updated":"2024-06-21 15:35:00.000000000","message":"Instead *of*","commit_id":"65bc7a2973671705148b7ba8610faf774657fc0c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e0cdd49e1d32c6fce9daa165c34c2ab4d61933b1","unresolved":false,"context_lines":[{"line_number":74,"context_line":"  database changes, and would increase CPU utilisation due to the need to"},{"line_number":75,"context_line":"  generate multiple hashes for every image."},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"- Instead on introducing a new configuration option, we could modify the"},{"line_number":78,"context_line":"  behavior of the existing ``[DEFAULT] hashing_algorithm`` option such"},{"line_number":79,"context_line":"  that it now accepts a list of allowed hashing algorithms, with the first item"},{"line_number":80,"context_line":"  (index 0) being the default used."}],"source_content_type":"text/x-rst","patch_set":3,"id":"5ea059ee_7275ba51","line":77,"range":{"start_line":77,"start_character":10,"end_line":77,"end_character":12},"in_reply_to":"1f65c74f_b2842e7d","updated":"2024-07-18 15:57:21.000000000","message":"Done","commit_id":"65bc7a2973671705148b7ba8610faf774657fc0c"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"fd4eeb7f7cb1315b36bb005e025402bb232bcfad","unresolved":true,"context_lines":[{"line_number":77,"context_line":"- Instead on introducing a new configuration option, we could modify the"},{"line_number":78,"context_line":"  behavior of the existing ``[DEFAULT] hashing_algorithm`` option such"},{"line_number":79,"context_line":"  that it now accepts a list of allowed hashing algorithms, with the first item"},{"line_number":80,"context_line":"  (index 0) being the default used."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"  This was rejected because it overloads the option to have two purposes - both"},{"line_number":83,"context_line":"  configuring a default and configuring allowed options - which could be"}],"source_content_type":"text/x-rst","patch_set":3,"id":"243deb5b_0bc030f8","line":80,"range":{"start_line":80,"start_character":2,"end_line":80,"end_character":34},"updated":"2024-06-19 17:29:00.000000000","message":"this is not how default works if you provide choices option;\n\nyou can make default any value from the list of choices you provide\n\ne.g.\n\n```\n    cfg.StrOpt(\u0027volume_clear\u0027,\n               default\u003d\u0027zero\u0027,\n               choices\u003d[\u0027none\u0027, \u0027zero\u0027],\n               help\u003d\u0027Method used to wipe old volumes\u0027),\n```","commit_id":"65bc7a2973671705148b7ba8610faf774657fc0c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ce4fee97b57ace38c6bf14934a8aef32627107ac","unresolved":true,"context_lines":[{"line_number":77,"context_line":"- Instead on introducing a new configuration option, we could modify the"},{"line_number":78,"context_line":"  behavior of the existing ``[DEFAULT] hashing_algorithm`` option such"},{"line_number":79,"context_line":"  that it now accepts a list of allowed hashing algorithms, with the first item"},{"line_number":80,"context_line":"  (index 0) being the default used."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"  This was rejected because it overloads the option to have two purposes - both"},{"line_number":83,"context_line":"  configuring a default and configuring allowed options - which could be"}],"source_content_type":"text/x-rst","patch_set":3,"id":"6fbfa3fe_3834e633","line":80,"range":{"start_line":80,"start_character":2,"end_line":80,"end_character":34},"in_reply_to":"243deb5b_0bc030f8","updated":"2024-06-20 10:33:01.000000000","message":"\u003e this is not how default works if you provide choices option;\n\nI\u0027m not using `choices` though. `choices` is used to restrict the allowed values for single value options like `StrOpt`, allowing a way to provide an enum. That\u0027s not what I\u0027m suggesting we could do here. Instead, I am suggesting converting `[DEFAULT] hashing_algorithm` from a single value string opt to a string list opt, e.g.\n\n```\ncfg.ListOpt(\n    \u0027hashing_algorithms\u0027,\n    default\u003d[\u0027sha512\u0027, \u0027sha256\u0027, \u0027sha1\u0027, \u0027md5\u0027],\n    help\u003d(\n        \u0027Supported hashing algorithms. A user can select any of these. \u0027\n        \u0027The first item is the default algorithm used if the user does not \u0027\n        \u0027provide one.\u0027\n    ),\n)\n```\n\nThis would allow the operator to define a list of algorithms that a user can set on the `os_hash_algo` field during `POST /images` calls (i.e. image creation). We would merely treat the first item as special (in code!) if the user didn\u0027t provide something, e.g.\n\n```\nos_hash_algo \u003d properties.get(\u0027os_hash_algo\u0027)\nif os_hash_algo and os_hash_algo not in CONF.hashing_algorithm:\n    raise webob.exc.HTTPBadRequest(\u0027...\u0027)\nelif not os_hash_algo:\n    os_hash_algo \u003d CONF.hashing_algorithm[0]\n```\n\nbut tbc, I don\u0027t think this is a good approach and we\u0027d be better off with two options, as I\u0027ve explained here.\n\nHopefully that makes sense?","commit_id":"65bc7a2973671705148b7ba8610faf774657fc0c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"dd0a3b36fbec89627937a6dad0a14826d24c59f8","unresolved":true,"context_lines":[{"line_number":77,"context_line":"- Instead on introducing a new configuration option, we could modify the"},{"line_number":78,"context_line":"  behavior of the existing ``[DEFAULT] hashing_algorithm`` option such"},{"line_number":79,"context_line":"  that it now accepts a list of allowed hashing algorithms, with the first item"},{"line_number":80,"context_line":"  (index 0) being the default used."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"  This was rejected because it overloads the option to have two purposes - both"},{"line_number":83,"context_line":"  configuring a default and configuring allowed options - which could be"}],"source_content_type":"text/x-rst","patch_set":3,"id":"9eefab05_087a4072","line":80,"range":{"start_line":80,"start_character":2,"end_line":80,"end_character":34},"in_reply_to":"59e4bc86_15188e8d","updated":"2024-06-24 09:47:56.000000000","message":"\u003e I am confused, if you are going to use\n\u003e \n\u003e ```\n\u003e os_hash_algo \u003d CONF.hashing_algorithm[0]\n\u003e ```\n\nI\u0027m **not** 😅 This is an *alternative*. I\u0027m just explaining how the *alternative* would work *if* we used it (which we\u0027re not going to).\n\n\u003e Then what is the use of having \u0027default_hashing_algorithm\u0027 config option.\n\u003e \n\u003e I think you want to define new option hashing_algorithms with some default values to allow users other supported hashing algorithms (shown below) as well, right?\n\u003e \n\u003e ```\n\u003e hashlib.algorithms_guaranteed # recommended by glance\n\u003e \n\u003e {\u0027sha3_256\u0027, \u0027sha3_512\u0027, \u0027shake_256\u0027, \u0027sha384\u0027, \u0027sha224\u0027, \u0027sha512\u0027, \u0027sha3_384\u0027,\n\u003e \n\u003e \u0027blake2s\u0027, \u0027md5\u0027, \u0027shake_128\u0027, \u0027sha1\u0027, \u0027sha256\u0027, \u0027sha3_224\u0027, \u0027blake2b\u0027}\n\u003e ```\n\nYes, we will have the following options:\n\n- `[DEFAULT] default_hashing_algorithm` - specify the default used if a user doesn\u0027t specify an algorithm\n- `[DEFAULT] allowed_hashing_algorithms` - specify the options a user is allowed to specify\n\nWe could use `haslib.algorithms_guaranteed` for the default of the latter but this leaves us subject to changes in the underlying hashlib library. I\u0027d rather we hardcode a smaller list and let the operator tweak this as they wish.","commit_id":"65bc7a2973671705148b7ba8610faf774657fc0c"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"938d4792a76bb9560889650b7d9acb32cfcb1f76","unresolved":true,"context_lines":[{"line_number":77,"context_line":"- Instead on introducing a new configuration option, we could modify the"},{"line_number":78,"context_line":"  behavior of the existing ``[DEFAULT] hashing_algorithm`` option such"},{"line_number":79,"context_line":"  that it now accepts a list of allowed hashing algorithms, with the first item"},{"line_number":80,"context_line":"  (index 0) being the default used."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"  This was rejected because it overloads the option to have two purposes - both"},{"line_number":83,"context_line":"  configuring a default and configuring allowed options - which could be"}],"source_content_type":"text/x-rst","patch_set":3,"id":"59e4bc86_15188e8d","line":80,"range":{"start_line":80,"start_character":2,"end_line":80,"end_character":34},"in_reply_to":"6fbfa3fe_3834e633","updated":"2024-06-21 06:02:21.000000000","message":"I am confused, if you are going to use\n\n```\nos_hash_algo \u003d CONF.hashing_algorithm[0]\n```\n\nThen what is the use of having \u0027default_hashing_algorithm\u0027 config option.\n\nI think you want to define new option hashing_algorithms with some default values to allow users other supported hashing algorithms (shown below) as well, right?\n\n```\nhashlib.algorithms_guaranteed # recommended by glance\n\n{\u0027sha3_256\u0027, \u0027sha3_512\u0027, \u0027shake_256\u0027, \u0027sha384\u0027, \u0027sha224\u0027, \u0027sha512\u0027, \u0027sha3_384\u0027,\n\n\u0027blake2s\u0027, \u0027md5\u0027, \u0027shake_128\u0027, \u0027sha1\u0027, \u0027sha256\u0027, \u0027sha3_224\u0027, \u0027blake2b\u0027}\n```","commit_id":"65bc7a2973671705148b7ba8610faf774657fc0c"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"be5a28fb4667739fe867d09c51cead27eee83148","unresolved":true,"context_lines":[{"line_number":77,"context_line":"- Instead on introducing a new configuration option, we could modify the"},{"line_number":78,"context_line":"  behavior of the existing ``[DEFAULT] hashing_algorithm`` option such"},{"line_number":79,"context_line":"  that it now accepts a list of allowed hashing algorithms, with the first item"},{"line_number":80,"context_line":"  (index 0) being the default used."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"  This was rejected because it overloads the option to have two purposes - both"},{"line_number":83,"context_line":"  configuring a default and configuring allowed options - which could be"}],"source_content_type":"text/x-rst","patch_set":3,"id":"f4756866_0a9e91f4","line":80,"range":{"start_line":80,"start_character":2,"end_line":80,"end_character":34},"in_reply_to":"9eefab05_087a4072","updated":"2024-06-25 15:16:46.000000000","message":":/ I missed that it is from alternatvies section.\n\nNO objection.","commit_id":"65bc7a2973671705148b7ba8610faf774657fc0c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e0cdd49e1d32c6fce9daa165c34c2ab4d61933b1","unresolved":false,"context_lines":[{"line_number":77,"context_line":"- Instead on introducing a new configuration option, we could modify the"},{"line_number":78,"context_line":"  behavior of the existing ``[DEFAULT] hashing_algorithm`` option such"},{"line_number":79,"context_line":"  that it now accepts a list of allowed hashing algorithms, with the first item"},{"line_number":80,"context_line":"  (index 0) being the default used."},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"  This was rejected because it overloads the option to have two purposes - both"},{"line_number":83,"context_line":"  configuring a default and configuring allowed options - which could be"}],"source_content_type":"text/x-rst","patch_set":3,"id":"4c0cfa1a_4832c138","line":80,"range":{"start_line":80,"start_character":2,"end_line":80,"end_character":34},"in_reply_to":"f4756866_0a9e91f4","updated":"2024-07-18 15:57:21.000000000","message":"Done","commit_id":"65bc7a2973671705148b7ba8610faf774657fc0c"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"af9d78b580e3ab5109419744226adf1d77f3588d","unresolved":true,"context_lines":[{"line_number":116,"context_line":"  attack using SHA-256 or SHA-512 has not been publicly demonstrated. This is"},{"line_number":117,"context_line":"  obviously not the case for SHA1 and MD5 but as both algorithms\u0027 lack of"},{"line_number":118,"context_line":"  security is well documented and well known, there should be no expectation of"},{"line_number":119,"context_line":"  security from end-users."},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"- A malicious user could conduct a denial-of-service attack on Glance by"},{"line_number":122,"context_line":"  uploading images using an expensive hashing algorithm."}],"source_content_type":"text/x-rst","patch_set":3,"id":"a7157915_a8d37b17","line":119,"range":{"start_line":119,"start_character":20,"end_line":119,"end_character":25},"updated":"2024-06-21 15:35:00.000000000","message":"Agreed, end users should be using secure hash algorithms.","commit_id":"65bc7a2973671705148b7ba8610faf774657fc0c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e0cdd49e1d32c6fce9daa165c34c2ab4d61933b1","unresolved":false,"context_lines":[{"line_number":116,"context_line":"  attack using SHA-256 or SHA-512 has not been publicly demonstrated. This is"},{"line_number":117,"context_line":"  obviously not the case for SHA1 and MD5 but as both algorithms\u0027 lack of"},{"line_number":118,"context_line":"  security is well documented and well known, there should be no expectation of"},{"line_number":119,"context_line":"  security from end-users."},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"- A malicious user could conduct a denial-of-service attack on Glance by"},{"line_number":122,"context_line":"  uploading images using an expensive hashing algorithm."}],"source_content_type":"text/x-rst","patch_set":3,"id":"c1969490_b8f5670e","line":119,"range":{"start_line":119,"start_character":20,"end_line":119,"end_character":25},"in_reply_to":"a7157915_a8d37b17","updated":"2024-07-18 15:57:21.000000000","message":"Done","commit_id":"65bc7a2973671705148b7ba8610faf774657fc0c"}]}
