Bug 1827522
| Summary: | [RHOS16] Create volume failed to copy metadata to volume when upload-to-image | ||
|---|---|---|---|
| Product: | Red Hat OpenStack | Reporter: | bkopilov <bkopilov> |
| Component: | openstack-cinder | Assignee: | Brian Rosmaita <brian.rosmaita> |
| Status: | ASSIGNED --- | QA Contact: | Evelina Shames <eshames> |
| Severity: | high | Docs Contact: | Andy Stillman <astillma> |
| Priority: | medium | ||
| Version: | 16.0 (Train) | CC: | aefrat, aruffin, averi, brian.rosmaita, cylopez, dhill, eharney, gfidente, jgrosso, jhardee, ltoscano, msecaur, nnavarat, omcgonag, pambre, rdiwakar, yocha |
| Target Milestone: | z10 | Keywords: | Triaged |
| Target Release: | 16.1 (Train on RHEL 8.2) | ||
| Hardware: | x86_64 | ||
| OS: | All | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | If docs needed, set a value | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | Type: | Bug | |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
|
Description
bkopilov
2020-04-24 05:17:31 UTC
The short-term workaround for this is to remove the signature_verified metadata from any glance images. This should not be on the image in the first place, it's a marker from cinder that the glance image contained the img_signature* properties [0] and that when the image was written to a volume, the signature was verified. What happens is that if you turn on signature verification in cinder, it will fail volume creation when the img_signature* properties exist but the verification fails, and it will put "signature_verified: false" on volumes created from images that didn't have img_signature* metadata, and put "signature_verified: true" if verification passed. What I'm getting at is that signature_verified is not an attribute of the image, it's an attribute of the volume, so removing it from images will not affect security or data integrity. (Unless an operator happens to be using an image property with that name for some other purpose.) So an automatic short-term workaround could be a cron job removing signature_verified from any glance image that has it. [0] https://docs.openstack.org/glance/latest/user/signature.html Hi, Brian, Having that automated workaround is really helpful. Thank you for that. For our partners who provide this to their customers, though, I'm not sure it's a very reliable workaround longer-term. What needs to happen to make a formal, permanent fix? Thanks! (In reply to Matthew Secaur from comment #11) > What needs to happen to make a formal, permanent fix? I think it has 3 parts: (1) introduce a non_inheritable_image_properties config option (like Nova has) to filter out properties that are in the cinder volume_image_metadata of a volume, but which should not be put on the image when the volume is uploaded as an image to Glance. One of these would be signature_verified, but also any of the img_signature* properties (because in most cases, the image1 -> volume -> image2 transformation will not give you an image2 that can be verified by the signature for image1). Maybe this doesn't need to be a config option, but it would give operators some flexibility for using third-party tools. Or maybe it could be a combination config opt + hard-coded list (which is what Nova has since ussuri). (2) refactor how cinder handles the volume_image_metadata to include (1) but also there's a weird mix in there of accessing the DB directly and using cinder-oslo-versioned-objects to manipulate the image metadata (which is actually the reason for this bug). Additionally it's not clear that we should be storing cinder-specific info (like signature_verified) in the volume_image_metadata as a conceptual matter (though that won't matter so much if (1) is implemented). (3) introduce a filter similar to (1) except that it will filter out image properties that will not be stored in the volume_image_metadata in the first place. This is needed to deal with "legacy" images that have signature_verified on them, but it might also be useful to make it a config option like (1). Not sure about that, though. Hi team, CU is wondering if it is possible not using "signature verified" that before get patch for this issue. Could you help me for answering to CU? Thank you, YoungCheol. (In reply to youngcheol from comment #13) > CU is wondering if it is possible not using "signature verified" that before > get patch for this issue. Yes. In the cinder.conf file used by the cinder volume service, set verify_glance_signatures = disabled What will happen is that if the image has the 'signature_verified' property, it will be included in the volume_image_metadata of the created volume, but we won't hit this bug because cinder will not try to update the value. I was asked to answer a question about the security implications of one of the workarounds for this bug. The workaround is to set verify_glance_signatures=disabled in cinder.conf (which basically turns off the feature). The security implication is that the data downloaded to cinder (and that cinder writes to the volume) is not verified by using the cryptographic signature. But whether this is an issue depends on your use case, because the data *is* verified by other means. Each image downloaded from glance to cinder (since Rocky) is verified using cryptographically strong checksumming (before that, image downloads were verified using md5; that's still the case for "legacy" images in a cloud that were created before Rocky). This is the glance 'multihash' feature, which uses the os_hash_algo and os_hash_value image properties to store the checksum algorithm and hash. The default is sha512. So even with verify_glance_signatures=disabled, we have assurance that the image data originally uploaded to glance (when the os_hash_value was computed), is unchanged when it is consumed by cinder. (The checksumming code, by the way, is not in cinder; it's in the python-glanceclient (since version 2.13.0), which cinder uses to download the image data.) The use case addressed by image signature verification is to allow users to verify that a previously uploaded image has not been modified. This can be done without the signature if a user computes the sha512 hash of the image before upload, and then verifies that the os_hash_value that glance sets on the image is the same. Then you have to trust that cinder and nova are verifying the hash correctly when consuming the image, but you'd also have to trust that they were performing the image signature verification correctly, so I don't think enabling image signature verification removes the cloud infrastructure from the trust loop. So why use the image signature verification at all? It was implemented during the Liberty-Mitaka timeframe, when glance was still using md5 for checksumming. The first md5 collision was produced sometime around 2004, but md5 was still thought circa 2015 to be OK as a quick check that no bits were flipped during data transfer. Image signature verification was implemented *instead* of using a stronger checksumming mechanism in glance to provide data verification, but since it required the deployment and maintenance of a key management service, as well as requiring extra work by the end user to compute the signature and add the appropriate image properties to the image, it didn't catch on, and the secure 'multihash' feature, which doesn't require key management and is as automatic as the old md5 checksum, was added to glance in Rocky. In my opinion, having signature validation disabled doesn't affect the security of image data in a Rocky or later OpenStack cloud. The only issue I can see is if for some reason a workflow requires using a signed image (not because it's better, but because that's what a customer wants). In that case, verify_glance_signatures can be enabled, and the other workaround comes into play, namely, removing the 'signature_verified' metadata from all images in glance. *** Bug 2121573 has been marked as a duplicate of this bug. *** |