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

verify signature of bundles downloaded from mirror.openshift.com #3493

Closed
anjannath opened this issue Jan 31, 2023 · 12 comments · Fixed by #3605
Closed

verify signature of bundles downloaded from mirror.openshift.com #3493

anjannath opened this issue Jan 31, 2023 · 12 comments · Fixed by #3605
Assignees
Labels
kind/task Workable task

Comments

@anjannath
Copy link
Member

Currently we manually update the bundle hashes, which is error prone. two things needs to happen when bundle version is updated: OPENSHIFT_VERSION in the Makefile needs to be updated and the hashes in the following file needs to be updated.

var bundleLocations = map[string]bundlesDownloadInfo{
"amd64": {
"darwin": {
preset.OpenShift: download.NewRemoteFile("https://mirror.openshift.com/pub/openshift-v4/clients/crc/bundles/openshift/4.12.0/crc_vfkit_4.12.0_amd64.crcbundle",
"d19c80e53f5c593908a09eb9b3f43ebd908db60ca2e54a01d87a8b09c208557f"),
},
"linux": {
preset.OpenShift: download.NewRemoteFile("https://mirror.openshift.com/pub/openshift-v4/clients/crc/bundles/openshift/4.12.0/crc_libvirt_4.12.0_amd64.crcbundle",
"cbc75023e63fb33ce4a571ba2047c813acc04f68d6e51879e6a3b238913f54bb"),
},
"windows": {
preset.OpenShift: download.NewRemoteFile("https://mirror.openshift.com/pub/openshift-v4/clients/crc/bundles/openshift/4.12.0/crc_hyperv_4.12.0_amd64.crcbundle",
"a780aed82eea3a023b67247598f57ecf16c3d6fd80375cc3e2f1b564d2b9bc71"),
},
},
"arm64": {
"darwin": {
preset.OpenShift: download.NewRemoteFile("https://mirror.openshift.com/pub/openshift-v4/clients/crc/bundles/openshift/4.12.0/crc_vfkit_4.12.0_arm64.crcbundle",
"59ec291480daf9e0a92978473b573f06fbb38ff5e1839ee7029aa77d2abea93a"),
},
},
}

this was suggested in: #3486 (review)

@anjannath anjannath added the kind/task Workable task label Jan 31, 2023
@anjannath anjannath moved this to Scheduled in Project planning: crc Jan 31, 2023
@cfergeau
Copy link
Contributor

The bundles released in https://developers.redhat.com/content-gateway/rest/mirror2/pub/openshift-v4/clients/crc/bundles/openshift/4.12.0/ will be GPG-signed in the future, similarly to what is done for https://developers.redhat.com/content-gateway/rest/mirror2/pub/openshift-v4/clients/crc/2.13.1/ (signed sha256sum.txt).
crc could also be taught to download this sha256sum, check its GPG signature and use this instead of the hardcoded hashs.

@anjannath
Copy link
Member Author

anjannath commented Feb 1, 2023

there are two signature files in the mirror for sha256sum.txt: sha256sum.txt.gpg and sha256sum.txt.sig both are same it seems, one is armoured and the other in binary format. we can verify and see the content using the Red Hat, Inc. (release key 2) pub key:

❯ gpg --decrypt sha256sum.txt.gpg
4f0edb2b9dc9bed2830e0751ec5e4cc3340207f7ab2a0fb97e26ab3833ef3178  crc-linux-amd64.tar.xz
9403edcb8dceea4005864c04cac5fd344d185c647106967a02a26e9103d9b57e  crc-macos-installer.pkg
92686a17ef49328a190edacb40016ea6c4549d1116e6a1d5a5da6096c659160a  crc-windows-installer.zip
5f0709bad22f0d593a149a7d4a2b150ab0b6a4cc4f48abd1e1dae2853d6aa692  release-info.json
gpg: Signature made Wed Jan 25 15:42:52 2023 IST
gpg:                using RSA key 199E2F91FD431D51
gpg: Good signature from "Red Hat, Inc. (release key 2) <security@redhat.com>" [unknown]
gpg: WARNING: This key is not certified with a trusted signature!
gpg:          There is no indication that the signature belongs to the owner.
Primary key fingerprint: 567E 347A D004 4ADE 55BA  8A5F 199E 2F91 FD43 1D51

❯ gpg --decrypt sha256sum.txt.sig
4f0edb2b9dc9bed2830e0751ec5e4cc3340207f7ab2a0fb97e26ab3833ef3178  crc-linux-amd64.tar.xz
9403edcb8dceea4005864c04cac5fd344d185c647106967a02a26e9103d9b57e  crc-macos-installer.pkg
92686a17ef49328a190edacb40016ea6c4549d1116e6a1d5a5da6096c659160a  crc-windows-installer.zip
5f0709bad22f0d593a149a7d4a2b150ab0b6a4cc4f48abd1e1dae2853d6aa692  release-info.json
gpg: Signature made Wed Jan 25 15:42:51 2023 IST
gpg:                using RSA key 199E2F91FD431D51
gpg: Good signature from "Red Hat, Inc. (release key 2) <security@redhat.com>" [unknown]
gpg: WARNING: This key is not certified with a trusted signature!
gpg:          There is no indication that the signature belongs to the owner.
Primary key fingerprint: 567E 347A D004 4ADE 55BA  8A5F 199E 2F91 FD43 1D51

So we have to first download either of these files and match the sha256sum of the downloaded bundle to the one from the signed sha256sum.txt.gpg or sha256sum.txt.sig file

when i use the --verify flag it says this is not a detached signature:

❯ gpg --verify sha256sum.txt.sig
gpg: Signature made Wed Jan 25 15:42:51 2023 IST
gpg:                using RSA key 199E2F91FD431D51
gpg: Good signature from "Red Hat, Inc. (release key 2) <security@redhat.com>" [unknown]
gpg: WARNING: This key is not certified with a trusted signature!
gpg:          There is no indication that the signature belongs to the owner.
Primary key fingerprint: 567E 347A D004 4ADE 55BA  8A5F 199E 2F91 FD43 1D51
gpg: WARNING: not a detached signature; file 'sha256sum.txt' was NOT verified!

@cfergeau
Copy link
Contributor

cfergeau commented Feb 1, 2023

Yeah, the check is a bit annoying, I don't know why there is this indirection instead of directly signing each file :) crc already has some gpg signature checking code when downloading bundles from quay.

@cfergeau
Copy link
Contributor

cfergeau commented Feb 1, 2023

Ah and you are correct, these are not detached gpg signatures for the sha256sum.txt file, but sha256sum.txt content followed by a signature.

@anjannath
Copy link
Member Author

yeah, and the library we are using currently for checking signature on the bundles from quay doesn't have helpers for verifying non-detached signatures, https://pkg.go.dev/github.com/ProtonMail/go-crypto@v0.0.0-20230131201316-b9d3a0f1b21b/openpgp

@cfergeau
Copy link
Contributor

cfergeau commented Feb 1, 2023

Maybe https://pkg.go.dev/github.com/ProtonMail/go-crypto@v0.0.0-20230131201316-b9d3a0f1b21b/openpgp#ReadMessage could be used

@anjannath anjannath changed the title use go generate to populate the bundle hashes file verify signature of bundles downloaded from mirror.openshift.com Feb 13, 2023
@anjannath
Copy link
Member Author

I tried to use the ReadMessage method to verify but it gives an error saying the message is not properly formatted.

Since the signtaure is clearsign i tried to use the https://pkg.go.dev/github.com/ProtonMail/go-crypto@v0.0.0-20230214155104-81033d7f4442/openpgp/clearsign#Block.VerifySignature but this is not working for the signed files on mirror

i did some testing with keys generated locally and its working for that (https://github.com/anjannath/go-crypto-openpgp-test/blob/1f1c3a83c4c9dad4f1909aab80e7b22dbc741eda/verify.go#L13), now am not sure why its not working on the files from mirror

i've also asked for detached sign of the individual bundles.

@anjannath
Copy link
Member Author

This is a v3 signature, which we don't support anymore. Probably the error message could be improved here, basically it just means it couldn't find a (parseable) signature signed by one of the passed keys.

from ProtonMail/go-crypto#149 (comment)

@anjannath
Copy link
Member Author

one possible workaround is to perform the signing ourselves with keys we control (which'd produce a v4 signature) we can perform the signing as part of the bundle generation process

longer term if the bundles are published on quay this'll not be needed as we can already verify bundles pulled from quay.

@cfergeau
Copy link
Contributor

cfergeau commented Apr 4, 2023

Maybe it's just me, but I consider a crc signature to be weaker/not as good as a RH signature, ideally we'd have everything signed and verifiable with the RH key.

@anjannath
Copy link
Member Author

yeah, agreed not as good as a RH signature, i haven't given up on that yet, tried to use the golang.org/x/crypto/openpgp which has V3 signature support still (its not maintained but gets security updates.. )

now with that library hitting another issue, openpgp: invalid signature: hash tag doesn't match, i checked that by default it sets the hashing algo to sha256 which matches with the hash algorithm in the signature we have,

small reproducer: https://go.dev/play/p/6p9h0Z0BSIm?v=1.19

@anjannath
Copy link
Member Author

anjannath commented Apr 5, 2023

the issue was that the signed msg was not in expected format, the library doesn't transform the decoded signed msg to the needed format.
from: https://datatracker.ietf.org/doc/html/rfc4880#section-5.2.1

0x01: Signature of a canonical text document.
This means the signer owns it, created it, or certifies that it
has not been modified. The signature is calculated over the text
data with its line endings converted to <CR><LF>.

and the gopenpgp lib has two helpers that does the transformation,

https://github.com/ProtonMail/gopenpgp/blob/58dbea76e750c3110dbbb0631418fc915f189a36/internal/common.go#L10-L22

once those transformations are applied to the signed msg, openpgp.CheckDetachedSignature() is recognising it!
working example: https://go.dev/play/p/Cg7aS80AfjA?v=goprev

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/task Workable task
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants