Skip to content

[release/1.6] vendor: github.com/containerd/imgcrypt v1.1.4#6739

Merged
AkihiroSuda merged 1 commit intocontainerd:release/1.6from
thaJeztah:1.6_bump_imgcrypt
Mar 26, 2022
Merged

[release/1.6] vendor: github.com/containerd/imgcrypt v1.1.4#6739
AkihiroSuda merged 1 commit intocontainerd:release/1.6from
thaJeztah:1.6_bump_imgcrypt

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

  • Fixed issue in CheckAuthorization() callpath for images with a ManifestList
  • Updated to ocicrypt 1.1.3
  • Updated to containerd 1.6.1

Comment on lines +325 to +323
# github.com/opencontainers/image-spec v1.0.2-0.20211117181255-693428a734f5
# github.com/opencontainers/image-spec v1.0.2
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm... interesting; why does it roll back this one? opencontainers/image-spec@v1.0.2...693428a

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1.0.2-0.20211117181255-693428a734f5 is actually older than 1.0.2.

https://go.dev/ref/mod#pseudo-versions

vX.Y.(Z+1)-0.yyyymmddhhmmss-abcdefabcdef is used when the base version is a release version like vX.Y.Z. For example, if the base version is v1.2.3, a pseudo-version might be v1.2.4-0.20191109021931-daa7c04131f5.

So 1.0.2-0.[date]-[sha1hex] is 1.0.1 + some changes.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, it's "older" but different branch so they cannot be compared;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go modules only works if all tags are either released from the main branch (so no release branches are used), or if a new tag is created on main (v1.0.3-alpha.0) after a release branch is created (even then, I'm not sure if would do the comparison correctly in this case if it's a pseudo tag, and it's looking at the date)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the situation was something like;

main ---- v1.0.1 ---------------- patch for GHSA-77vh-xpmg-72qh -> 693428a734f5
            |
            |                                                               
            +-------------------- patch for GHSA-77vh-xpmg-72qh -> v1.0.2

But I think the v1 branch was also merged into main

The fun thing is that the commit we use (from the main branch) (opencontainers/image-spec@693428a) looks to be newer;

commit 693428a734f5bab1a84bd2f990d92ef1111cd60c (HEAD)
Merge: 9a7a987 6ced3bd
Author: Vincent Batts <vbatts@hashbangbash.com>
Date:   Wed Nov 17 13:12:55 2021 -0500

    Merge pull request from GHSA-77vh-xpmg-72qh

The v1.0.2 tag (on v1 branch_ (opencontainers/image-spec@67d2d56):

commit 67d2d5658fe0476ab9bf414cec164077ebff3920 (HEAD, tag: v1.0.2)
Author: Vincent Batts <vbatts@hashbangbash.com>
Date:   Tue Nov 9 12:04:38 2021 -0500

    version: release 1.0.2

    Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

go.mod Outdated
Comment on lines +138 to +139
// prevent go mod from rolling this back to the last tagged release
github.com/opencontainers/image-spec => github.com/opencontainers/image-spec v1.0.2-0.20211117181255-693428a734f5
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a possible bug in go modules; this is a direct dependency of containerd, but it ignores the version we specify, and rolls it back to the last tagged release

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or is this an instance of release branches on that repo? and opencontainers/image-spec@693428a...v1.0.2

- Fixed issue in CheckAuthorization() callpath for images with a ManifestList
  - CVE-2022-24778
  - Fix: containerd/imgcrypt@6fdd981
  - Added test case covering this
- Updated to ocicrypt 1.1.3
- Updated to containerd 1.6.1

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AkihiroSuda
Copy link
Copy Markdown
Member

Could you open a main PR then cherry-pick?

@thaJeztah
Copy link
Copy Markdown
Member Author

main branch looks to be already ahead of 1.1.4;

github.com/containerd/imgcrypt v1.1.4-0.20220322210345-7eff50ecc4f6

I also have a branch for 1.5, but need to looks at the diff, as it's pushing various other dependencies as well (so possibly some replace rules are needed)

@AkihiroSuda AkihiroSuda merged commit bb8da7a into containerd:release/1.6 Mar 26, 2022
@thaJeztah thaJeztah deleted the 1.6_bump_imgcrypt branch March 26, 2022 11:34
github.com/moby/sys/symlink v0.2.0
github.com/opencontainers/go-digest v1.0.0
github.com/opencontainers/image-spec v1.0.2-0.20211117181255-693428a734f5
github.com/opencontainers/image-spec v1.0.2 // see replace for the actual version
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right, so that's the merge commit that merged back the v1.0 branch into main, right? opencontainers/image-spec@693428a...c5a74bc

I guess that fixes it, without introducing changes, yes (thanks!)

It's still somewhat ridiculous that these things are needed (especially the "roll back to what we think is more current).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opened #6766

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants