Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

script/seccomp.sh: check tarball sha256 #3482

Merged
merged 1 commit into from
May 27, 2022

Conversation

kolyshkin
Copy link
Contributor

@kolyshkin kolyshkin commented May 24, 2022

Add checking of downloaded tarball checksum.

In case it doesn't match the hardcoded value, the error is like this:

    libseccomp-2.5.4.tar.gz: FAILED
    sha256sum: WARNING: 1 computed checksum did NOT match

In case the checksum for a particular version is not specified in the
script, the error will look like this:

    ./script/seccomp.sh: line 29: SECCOMP_SHA256[${ver}]: unbound variable

In case the the hardcoded value in the file is of wrong format/length,
we'll get:

    sha256sum: 'standard input': no properly formatted SHA256 checksum lines found

In any of these cases, the script aborts (due to set -e).

Addresses #3480 (review)

@kolyshkin kolyshkin added this to the 1.2.0 milestone May 24, 2022
@kolyshkin kolyshkin added backport/1.1-todo A PR in main branch which needs to be backported to release-1.1 backport/1.1-done A PR in main branch which has been backported to release-1.1 and removed backport/1.1-todo A PR in main branch which needs to be backported to release-1.1 labels May 24, 2022
@kolyshkin
Copy link
Contributor Author

1.1 backport: #3481

wget "https://github.com/seccomp/libseccomp/releases/download/v${ver}/${tar}"{,.asc}
if ! sha256sum "${tar}" | grep -E "^${SECCOMP_SHA256[${ver}]}\s+${tar}$"; then
Copy link
Member

@tianon tianon May 24, 2022

Choose a reason for hiding this comment

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

Not a maintainer (so take this for what it's worth), but given this is Bash, I think this would be more reliable something like this:

Suggested change
if ! sha256sum "${tar}" | grep -E "^${SECCOMP_SHA256[${ver}]}\s+${tar}$"; then
if ! sha256sum --strict --check - <<<"${SECCOMP_SHA256[${ver}]} *${tar}"; then

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I agree that's more readable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, thanks @tianon. I have also removed the if since now we have the proper error message printed by sha256 sum itself.

@cyphar
Copy link
Member

cyphar commented May 24, 2022

It'd be nice to also verify the signature (which would be a better check than just the checksum) but it would require us to store the seccomp signing key in our repo (then again that's not too big of a deal).

Add checking of downloaded tarball checksum.

In case it doesn't match the hardcoded value, the error is like this:

	libseccomp-2.5.4.tar.gz: FAILED
	sha256sum: WARNING: 1 computed checksum did NOT match

In case the checksum for a particular version is not specified in the
script, the error will look like this:

	./script/seccomp.sh: line 29: SECCOMP_SHA256[${ver}]: unbound variable

In case the the hardcoded value in the file is of wrong format/length,
we'll get:

	sha256sum: 'standard input': no properly formatted SHA256 checksum lines found

In any of these cases, the script aborts (due to set -e).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Copy link
Member

@tianon tianon left a comment

Choose a reason for hiding this comment

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

👀

@kolyshkin kolyshkin requested review from cyphar and thaJeztah May 26, 2022 19:50
@kolyshkin kolyshkin merged commit 1505379 into opencontainers:main May 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci area/seccomp backport/1.1-done A PR in main branch which has been backported to release-1.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants