sync-images fails with Invalid glance image: <X>. Expected size=<Y> md5=None. Found size=<Y> md5=<Z>

Also visible in this gate:
https://review.opendev.org/c/openstack/charm-octavia-diskimage-retrofit/+/798310

This is still an issue, see this gate:
https://review.opendev.org/c/openstack/charm-octavia-diskimage-retrofit/+/807705

The exception comes from simplestreams library.

The exception is thrown when the md5 checksums don't match[0]:

OSError: Invalid glance image: 3c4b49a4-a4c9-4b84-95eb-dbf0ce3d1e83. Expected size=172883968 md5=None. Found size=172883968 md5=078ff054bceec76f66ffeaa748f9f2e5.

md5=None <- this would mean the variable `new_md5` is None[1], this variable is set to whatever returns GlanceMirror.download_image()[2], the sizes match, so this shouldn't be related to a download issue.

The md5 checksum could come from 2 different places[3], if the class was configured with a "hook" to modify the images or not, when there is no hook the md5sum comes from the structure stored in `flattened_img_data`[4], IIUC this information comes from the published streams that we are using as a source.

[0] https://git.launchpad.net/simplestreams/tree/simplestreams/mirrors/glance.py#n531
[1] https://git.launchpad.net/simplestreams/tree/simplestreams/mirrors/glance.py#n502
[2] https://git.launchpad.net/simplestreams/tree/simplestreams/mirrors/glance.py#n363
[3] https://git.launchpad.net/simplestreams/tree/simplestreams/mirrors/glance.py#n390
[4] https://git.launchpad.net/simplestreams/tree/simplestreams/mirrors/glance.py#n453

I'm wondering if this is because the following does not have md5s published:
http://cloud-images.ubuntu.com/minimal/daily/streams/v1/com.ubuntu.cloud:daily:download.sjson

as compared to the following which does have md5s published:
http://cloud-images.ubuntu.com/daily/streams/v1/com.ubuntu.cloud:daily:download.sjson

If I understand correctly, charm-octavia-diskimage-retrofit test bundles configure both of these. For example:
https://opendev.org/openstack/charm-octavia-diskimage-retrofit/src/branch/master/src/tests/bundles/bionic-ussuri.yaml#L38

Testing that theory here: https://review.opendev.org/c/openstack/charm-octavia-diskimage-retrofit/+/812400

Tests were successful for https://review.opendev.org/c/openstack/charm-octavia-diskimage-retrofit/+/812400.

It's not conclusive but that helps confirm the issue in question, that the minimal mirrors are missing md5 hashes. It at least confirms that the failure is limited to the minimal mirrors.

Fyi this has been failing since at least June 3rd. It looks like our failing test artifacts from before that have been erased so it's hard to tell how long before that it was occurring.

I'm hoping that will be useful to the cloud-images team.

This change disables the minimal mirror for now until this bug is fixed: https://review.opendev.org/c/openstack/charm-octavia-diskimage-retrofit/+/811211

looking at minimal streams, I don't see that CPC has ever published md5 sums. I didn't check every single entry, but sampling the oldest entries for all suites, and newest entries, none have md5. Digging into the code generating streams, I only see calls for generating sha256.

So I'm wondering if something changed in the testing that now specifically demands md5 to be in in the streams. I don't see a change to simplestreams, so the validation has been there for a while. Considering the wide-ranged deprecation of md5, it's probably worth looking at the simplestreams code and updating it to check for md5 and/or sha256

We provide sha256 checksums for all images.

Reviewed: https://review.opendev.org/c/openstack/charm-octavia-diskimage-retrofit/+/811211
Committed: https://opendev.org/openstack/charm-octavia-diskimage-retrofit/commit/068e5ff08739675da03478d16f8fb763db6d44b9
Submitter: "Zuul (22348)"
Branch: master

commit 068e5ff08739675da03478d16f8fb763db6d44b9
Author: Alex Kavanagh <email address hidden>
Date: Mon Sep 27 19:45:03 2021 +0100

    Add xena bundles

    - add non-voting impish-xena bundle
    - add non-voting focal-xena bundle
    - add non-voting focal-wallaby bundle
    - update bionic bundles to replace eoan image with focal
    - rebuild to pick up charm-helpers changes
    - update tox/pip.sh to ensure setuptools<50.0.0
    - drop minimal mirror from tests until
      https://pad.lv/1933966 is fixed

    Change-Id: I9b38c0da71229fbc1a70849d8fcae450e8983b21
    Related-Bug: #1933966

@John, thanks for taking a look. The simplestreams code [1] seems to validate based on md5, so perhaps it should be validating images based on sha256.
[1] https://git.launchpad.net/simplestreams/tree/simplestreams/mirrors/glance.py

It's 2021 and md5 has been insufficient as a cryptographic hash for years;
as of 2008 CERT declared MD5 "cryptographically broken and unsuitable for
further use"[1]. Minimal streams were created after this and therefore md5
sums are intentionally omitted. For standard image streams md5 sums should
be removed before another 10 year LTS is released considering supply chain
security concerns and simplestreams updated to only accept sha256 or
greater. Given the timeframe for LTS support NIST 2020 recommendations[2]
would point to SHA-384 or SHA-512.

[1] https://www.kb.cert.org/vuls/id/836068
[2] https://www.keylength.com/en/4/

On Thu, Oct 7, 2021 at 9:56 AM Corey Bryant <email address hidden>
wrote:

> @John, thanks for taking a look. The simplestreams code [1] seems to
> validate based on md5, so perhaps it should be validating images based on
> sha256.
> [1]
> https://git.launchpad.net/simplestreams/tree/simplestreams/mirrors/glance.py
>
> ** Also affects: simplestreams
> Importance: Undecided
> Status: New
>
> --
> You received this bug notification because you are subscribed to cloud-
> images.
> Matching subscriptions: cloud-images
> https://bugs.launchpad.net/bugs/1933966
>
> Title:
> sync-images fails with Invalid glance image: <X>. Expected size=<Y>
> md5=None. Found size=<Y> md5=<Z>
>
> Status in OpenStack glance-simplestreams-sync charm:
> New
> Status in charm-octavia-diskimage-retrofit:
> Triaged
> Status in cloud-images:
> New
> Status in simplestreams:
> New
>
> Bug description:
> Visible in this gate:
>
> https://review.opendev.org/c/openstack/charm-octavia-diskimage-retrofit/+/778995
>
> https://openstack-ci-reports.ubuntu.com/artifacts/d09/778995/5/check/bionic-ussuri/d092848/job-output.txt
>
> zaza.model.ActionFailed: Run of action "sync-images" with parameters
> "<not-set>" on "glance-simplestreams-sync/0" failed with "exit status 1"
> (id=36 status=failed enqueued=2021-06-28T16:55:10Z
> started=2021-06-28T16:55:11Z completed=2021-06-28T16:55:28Z output={'Code':
> '1', 'Stderr':
> '/usr/lib/python2.7/dist-packages/keystoneauth1/adapter.py:179:
> UserWarning: Using keystoneclient sessions has been deprecated. Please
> update your software to use keystoneauth1.\n warnings.warn(\'Using
> keystoneclient sessions has been deprecated. \'\nTraceback (most recent
> call last):
> File "/snap/simplestreams/27/bin/sstream-mirror-glance", line 185, in
> <module>
> main()
> File "/snap/simplestreams/27/bin/sstream-mirror-glance", line 181, in
> main
> tmirror.sync(smirror, args.path)
> File
> "/snap/simplestreams/27/lib/python3.6/site-packages/simplestreams/mirrors/__init__.py",
> line 91, in sync
> return self.sync_index(reader, path, data, content)
> File
> "/snap/simplestreams/27/lib/python3.6/site-packages/simplestreams/mirrors/__init__.py",
> line 254, in sync_index
> self.sync(reader, path=epath)
> File
> "/snap/simplestreams/27/lib/python3.6/site-packages/simplestreams/mirrors/__init__.py",
> line 89, in sync
> return self.sync_products(reader, path, data, content)
> File
> "/snap/simplestreams/27/lib/python3.6/site-packages/simplestreams/mirr...

I've triaged as low for the charms since the fix is needed in simplestreams.

Glance itself is returning an MD5 hash so that would need to change before simplestreams:
https://docs.openstack.org/glance/latest/user/glanceapi.html

The code using the glance api is in the validate_image() function from simplestreams:

    https://git.launchpad.net/simplestreams/tree/simplestreams/mirrors/glance.py#n515

    def validate_image(self, image_id, checksum, size, delete=True):
        """Validate an image's checksum and size after upload.

        Check for expected checksum and size.
        Throw an IOError if they do not match.

        :param image_id: str Glance Image ID
        :param checksum: str Expected MD5 sum of the image
        :param size: int Expected size in bytes of the image
        :returns: None
        """
        if not isinstance(size, util.INTEGER_TYPES):
            raise TypeError("size '%s' is not an integer" % str(size))
        found = self.gclient.images.get(image_id)
        if found.size == size and found.checksum == checksum:
            return
        msg = (
            ("Invalid glance image: %s. " % image_id) +
            ("Expected size=%s md5=%s. Found size=%s md5=%s." %
             (size, checksum, found.size, found.checksum)))
        if delete:
            LOG.warning("Deleting image %s: %s", image_id, msg)
            self.gclient.images.delete(image_id)
        raise IOError(msg)

Added upstream glance to get opinions on moving away from MD5.

Glance has already (in Rocky) introduced a "multihash" (self-describing hash fields that use SHA-512 by default). See the Rocky release notes for details:
https://docs.openstack.org/releasenotes/glance/rocky.html#new-features

The 'checksum' property remains on images for backward compatability.

Glance has already moved on from MD5. The checksum field is there just for backwards compatibility purposes.

multihash_* properties gives solid hash and algorithm used to calculate it.

Please see https://specs.openstack.org/openstack/glance-specs/specs/rocky/implemented/glance/multihash.html