-
Notifications
You must be signed in to change notification settings - Fork 429
Update rpm package to have 256bit digests #1411
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
base: main
Are you sure you want to change the base?
Conversation
a60b5f4 to
3de3949
Compare
|
/cherry-pick release-1.18 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL - FPM (Effing Package Management) supports 256-bit digests for RPM packages
LGTM
It is my understanding that |
This change uses fpm to update the built rpm packages to have 256bit digests without changing their contents. Signed-off-by: Evan Lezar <[email protected]>
Yeah, that's why I put |
|
It seems that drivers built with centos8-x86_64 works fine, so this is only needed for drivers built with centos7. Can this step be made to only run for centos7? |
Could you clarify what you mean? For our publicly released packages, we currently build on |
Got it, we started building artifacts in parallel for centos8 so that they would work. This is a separate containerfile that is not used for building so its a NOOP. |
|
@antheas as a matter of interest, do you have problems using our public artifacts on centos8? The intent is that these should work as expected, since the primary reason for us using centos7 to build the artifacts is to ensure an as broad as possible glibc support. (These same artifacts are used in other RPM-based distributions too and we don't see any problems there). |
I am using them on F43. I found using centos 8 to produce them creates the correct signature and I did not want to modify the code further. We started pre-building the nvidia drivers to match them with the kernel, so I also added the nvidia container toolkit to that. |
|
Thanks for the clarification. With these changes here, you should be able to consume the packages from upstream. As a note, the SHA-tagged packaging image (e.g. ghcr.io/nvidia/container-toolkit:85cfb7f4-packaging for the latest commit here) can be used to extract the packages for a given SHA. We use this artifact to propagate the packages (that are built only once) through our release pipeline. |
I can look into doing that in the future. Would you be interested in adding a github attestation to your action? One of the reasons we did this was to get an extra artifact having SLSA L3 |
| mkdir -p /artifacts/packages/centos7/${arch}; \ | ||
| cd /artifacts/packages/centos7/${arch}; \ | ||
| # For each package in the source folder we recreate a package to update | ||
| # the digests to 256-bit since we're running in a ubi9 container. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello! I got confused by the logic communicated by this sentence: we update the digests because we run in an ubi9 container?
Just glancing over this whole topic... is the following true?
We update the centos 7 RPM packages to contain 256 bit digests because of <some-reason-probably-compliance> and we do so by processing these RPM files in a UBI9 (rhel generation 9) container because only there we have the required tooling available.
This is for my own understanding, I am not asking to change the patch :).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we originally build the packages, we do so in a centos7 container (for glibc reasons). The version of rpmbuild available in this container is:
# rpmbuild --version
RPM version 4.11.3
which only generates SHA1 and MD5 digests and does not support 256-bit digests.
According to https://stackoverflow.com/questions/71942026/add-sha256-digests-to-rpm-packages, support for SHA256 digests was added in 4.14.3 (although it may not have been the default).
What we do in this case is us an intermediate container (rpmdigests) based on nvcr.io/nvidia/cuda:13.0.1-base-ubi9 (currently) with:
# rpmbuild --version
RPM version 4.16.1.3
Use fpm to "recreate" the packages. What I assume happens is that fpm extracts the packages and creates new packages using rpmbuild. Since the version in the container supports SHA256 digests by default these new packages will include these. Note that the exising SHA1 and MD5 digests are maintained.
And yes, the reason that we do this is "compliance". There are programs like FIPS that require this and certain newer distributions (see #1307) also require this by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have updated the description with the motivation.
This change uses fpm to update the built rpm packages to have
256bit digests without changing their contents.
This is required in cases where
SHA1orMD5digests are not supported such as FIPS or newer RPM-based distributions. (See #1307).For the v1.18.0 packages:
With these changes: