[ciq-6.18.y] [FIPS] Multiple patches tested (27 commits)#1291
[ciq-6.18.y] [FIPS] Multiple patches tested (27 commits)#1291ciq-kernel-automation[bot] wants to merge 28 commits into
Conversation
commit-author Herbert Xu <herbert.xu@redhat.com> commit 6892c65 commit-source https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-10 Upstream Status: RHEL only Restore the changes to /dev/random which were reverted after 5.18. This reverts commit 900f11e054896bae7b0146055698656e3d1e20a6. This also brings the code up-to-date with respect to centos-stream commit 9de3a73 so that changes that were made after the kernel-ark revert have been brought in. Signed-off-by: Herbert Xu <herbert.xu@redhat.com> Signed-off-by: Jeremy Allison <jallison@ciq.com>
commit-author Herbert Xu <herbert.xu@redhat.com> commit 8b0beca705b3877e24cccdd672422c66bbd75635 commit-source https://gitlab.com/cki-project/kernel-ark Upstream Status: RHEL only Restore the changes to use the crypto RNG in drivers/char/random which were reverted after 5.18. This reverts commit 297bcb88233101e8d5062729ff3a5f989bad1c3b. This also brings the code up-to-date with respect to centos-stream commit 9de3a73 so that changes that were made after the kernel-ark revert have been brought in. Signed-off-by: Herbert Xu <herbert.xu@redhat.com> Signed-off-by: Jeremy Allison <jallison@ciq.com>
commit-author Herbert Xu <herbert.xu@redhat.com> commit 248b805 commit-source https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-10 In order to ensure that the FIPS-certified RNG is always used, disable the vdso getrandom code by always making it fall back to getrandom(2) when FIPS mode is enabled. Signed-off-by: Herbert Xu <herbert.xu@redhat.com> Signed-off-by: Jeremy Allison <jallison@ciq.com>
commit - commit-source https://build.opensuse.org/public/source/SUSE:SLE-15-SP6:GA/kernel-source/patches.suse.tar.bz2 commit-patch-path patches.suse/crypto-ecdh-implement-FIPS-PCT.patch SP800-56Arev3, 5.6.2.1.4 ("Owner Assurance of Pair-wise Consistency") requires that a pair-wise consistency check needs to be conducted on a keypair. A pair-wise consistency test (PCT) is meant to ensure that a some provided public key is indeed associated with the given private one. As the kernel's ECDH implementation always computes the public key from the private one, this is guaranteed already as per the API. However, in the course of the certification process, there had been a lengthy discussion regarding this topic, with the result that a PCT is nonetheless mandatory. As the only user of the in-kernel ECDH is bluetooth, performance certainly isn't super critical. Simply implement a PCT for ECDH and move on. As mandated by SP800-56Arev3, 5.6.2.1.4, the PCT involves recomputing the public key and comparing it against the one under test. Signed-off-by: Nicolai Stange <nstange@suse.de> Signed-off-by: Jeremy Allison <jallison@ciq.com>
|
Unfortunately I just go an email from Stephan Mueller this morning, we need one more change. Hi Jeremy, unfortunately we need another kernel configuration change: Can you please set CRYPTO_JITTERENTROPY_MEMSIZE_128 (currently Thank you. Reason: The memory size used by the Jitter RNG should be larger than L1 cache Ciao |
Done! |
PlaidCat
left a comment
There was a problem hiding this comment.
There is no signed off by
but what is this subject it feels garbled / truncated / etc
Can I please get some context on this as it was not addressed in my previous review
#1258 (review)
In essiv_aead_setkey(), use the same logic as crypto_authenc_esn_setkey() to zeroize keys on exit. [Sultan: touched up commit message] Signed-off-by: Jason Rodriguez <jrodriguez@ciq.com>
None of the ciphers used by the DRBG have an alignment requirement; thus, they all return 0 from .crypto_init, resulting in inconsistent alignment across all buffers. Align all buffers to at least a cache line to improve performance. This is especially useful when multiple DRBG instances are used, since it prevents false sharing of cache lines between the different instances. Signed-off-by: Sultan Alsawaf <sultan@ciq.com>
Like pin_user_pages_fast(), but with the internal-only FOLL_FAST_ONLY flag. This complements the get_user_pages*() API, which already has get_user_pages_fast_only(). Signed-off-by: Sultan Alsawaf <sultan@ciq.com>
There is no reason this refcount should be a signed int. Convert it to an unsigned int, thereby also making it less likely to ever overflow. Signed-off-by: Sultan Alsawaf <sultan@ciq.com>
Since crypto_devrandom_read_iter() is invoked directly by user tasks and is accessible by every task in the system, there are glaring priority inversions on crypto_reseed_rng_lock and crypto_default_rng_lock. Tasks of arbitrary scheduling priority access crypto_devrandom_read_iter(). When a low-priority task owns one of the mutex locks, higher-priority tasks waiting on that mutex lock are stalled until the low-priority task is done. Fix the priority inversions by converting the mutex locks into rt_mutex locks which have PI support. Signed-off-by: Sultan Alsawaf <sultan@ciq.com>
When the kernel is booted with fips=1, the RNG exposed to userspace is hijacked away from the CRNG and redirects to crypto_devrandom_read_iter(), which utilizes the DRBG. Notably, crypto_devrandom_read_iter() maintains just two global DRBG instances _for the entire system_, and the two instances serve separate request types: one instance for GRND_RANDOM requests (crypto_reseed_rng), and one instance for non-GRND_RANDOM requests (crypto_default_rng). So in essence, for requests of a single type, there is just one global RNG for all CPUs in the entire system, which scales _very_ poorly. To make matters worse, the temporary buffer used to ferry data between the DRBG and userspace is woefully small at only 256 bytes, which doesn't do a good job of maximizing throughput from the DRBG. This results in lost performance when userspace requests >256 bytes; it is observed that DRBG throughput improves by 70% on an i9-13900H when the buffer size is increased to 4096 bytes (one page). Going beyond the size of one page up to the DRBG maximum request limit of 65536 bytes produces diminishing returns of only 3% improved throughput in comparison. And going below the size of one page produces progressively less throughput at each power of 2: there's a 5% loss going from 4096 bytes to 2048 bytes and a 9% loss going from 2048 bytes to 1024 bytes. Thus, this implements per-CPU DRBG instances utilizing a page-sized buffer for each CPU to utilize the DRBG itself more effectively. On top of that, for non-GRND_RANDOM requests, the DRBG's operations now occur under a local lock that disables preemption on non-PREEMPT_RT kernels, which not only keeps each CPU's DRBG instance isolated from another, but also improves temporal cache locality while the DRBG actively generates a new string of random bytes. Prefaulting one user destination page at a time is also employed to prevent a DRBG instance from getting blocked on page faults, thereby maximizing the use of the DRBG so that the only bottleneck is the DRBG itself. Signed-off-by: Sultan Alsawaf <sultan@ciq.com>
commit-author Eric Biggers <ebiggers@kernel.org> commit 04cadb4 Add FIPS cryptographic algorithm self-tests for all SHA-1 and SHA-2 algorithms. Following the "Implementation Guidance for FIPS 140-3" document, to achieve this it's sufficient to just test a single test vector for each of HMAC-SHA1, HMAC-SHA256, and HMAC-SHA512. Just run these tests in the initcalls, following the example of e.g. crypto/kdf_sp800108.c. Note that this should meet the FIPS self-test requirement even in the built-in case, given that the initcalls run before userspace, storage, network, etc. are accessible. This does not fix a regression, seeing as lib/ has had SHA-1 support since 2005 and SHA-256 support since 2018. Neither ever had FIPS self-tests. Moreover, fips=1 support has always been an unfinished feature upstream. However, with lib/ now being used more widely, it's now seeing more scrutiny and people seem to want these now [1][2]. [1] https://lore.kernel.org/r/3226361.1758126043@warthog.procyon.org.uk/ [2] https://lore.kernel.org/r/f31dbb22-0add-481c-aee0-e337a7731f8e@oracle.com/ Reviewed-by: Ard Biesheuvel <ardb@kernel.org> Link: https://lore.kernel.org/r/20251011001047.51886-1-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@kernel.org> Signed-off-by: Jeremy Allison <jallison@ciq.com>
commit-author Eric Biggers <ebiggers@kernel.org> commit c99d307 Add le64_to_cpu_array() and cpu_to_le64_array(). These mirror the corresponding 32-bit functions. These will be used by the BLAKE2b code. Reviewed-by: Ard Biesheuvel <ardb@kernel.org> Link: https://lore.kernel.org/r/20251018043106.375964-6-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@kernel.org> Signed-off-by: Jeremy Allison <jallison@ciq.com>
commit-author Eric Biggers <ebiggers@kernel.org> commit 23a16c9 Add a library API for BLAKE2b, closely modeled after the BLAKE2s API. This will allow in-kernel users such as btrfs to use BLAKE2b without going through the generic crypto layer. In addition, as usual the BLAKE2b crypto_shash algorithms will be reimplemented on top of this. Note: to create lib/crypto/blake2b.c I made a copy of lib/crypto/blake2s.c and made the updates from BLAKE2s => BLAKE2b. This way, the BLAKE2s and BLAKE2b code is kept consistent. Therefore, it borrows the SPDX-License-Identifier and Copyright from lib/crypto/blake2s.c rather than crypto/blake2b_generic.c. The library API uses 'struct blake2b_ctx', consistent with other lib/crypto/ APIs. The existing 'struct blake2b_state' will be removed once the blake2b crypto_shash algorithms are updated to stop using it. Reviewed-by: Ard Biesheuvel <ardb@kernel.org> Link: https://lore.kernel.org/r/20251018043106.375964-7-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@kernel.org> Signed-off-by: Jeremy Allison <jallison@ciq.com>
commit-author Eric Biggers <ebiggers@kernel.org> commit fe11ac1 upstream-diff | NB. This was a back-port not a cherry-pick as some of the underlying code had been changed in prior upstream commits: cc38d17 btrfs: enable large data folio support under CONFIG_BTRFS_EXPERIMENTAL and commit: 62bcbdc btrfs: make btrfs_csum_one_bio() handle bs > ps without large folios and others unknown. These changes were judged too large to bring back from upstream as they are CONFIG_BTRFS_EXPERIMENTAL. btrfs: switch to library APIs for checksums Make btrfs use the library APIs instead of crypto_shash, for all checksum computations. This has many benefits: - Allows future checksum types, e.g. XXH3 or CRC64, to be more easily supported. Only a library API will be needed, not crypto_shash too. - Eliminates the overhead of the generic crypto layer, including an indirect call for every function call and other API overhead. A microbenchmark of btrfs_check_read_bio() with crc32c checksums shows a speedup from 658 cycles to 608 cycles per 4096-byte block. - Decreases the stack usage of btrfs by reducing the size of checksum contexts from 384 bytes to 240 bytes, and by eliminating the need for some functions to declare a checksum context at all. - Increases reliability. The library functions always succeed and return void. In contrast, crypto_shash can fail and return errors. Also, the library functions are guaranteed to be available when btrfs is loaded; there's no longer any need to use module softdeps to try to work around the crypto modules sometimes not being loaded. - Fixes a bug where blake2b checksums didn't work on kernels booted with fips=1. Since btrfs checksums are for integrity only, it's fine for them to use non-FIPS-approved algorithms. Note that with having to handle 4 algorithms instead of just 1-2, this commit does result in a slightly positive diffstat. That being said, this wouldn't have been the case if btrfs had actually checked for errors from crypto_shash, which technically it should have been doing. Reviewed-by: Ard Biesheuvel <ardb@kernel.org> Reviewed-by: Neal Gompa <neal@gompa.dev> Signed-off-by: Eric Biggers <ebiggers@kernel.org> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: David Sterba <dsterba@suse.com> Signed-off-by: Jeremy Allison <jallison@ciq.com>
commit-author Joachim Vandersmissen <git@jvdsn.com> commit - commit-source https://lore.kernel.org/linux-crypto/20260303060509.246038-1-git@jvdsn.com/ xxhash64 is not a cryptographic hash algorithm, but is offered in the same API (shash) as actual cryptographic hash algorithms such as SHA-256. The Cryptographic Module Validation Program (CMVP), managing FIPS certification, believes that this could cause confusion. xxhash64 must therefore be blocked in FIPS mode. The only usage of xxhash64 in the kernel is btrfs. Commit fe11ac1 ("btrfs: switch to library APIs for checksums") recently modified the btrfs code to use the lib/crypto API, avoiding the Kernel Cryptographic API. Consequently, the removal of xxhash64 from the Crypto API in FIPS mode should now have no impact on btrfs usage. Signed-off-by: Joachim Vandersmissen <git@jvdsn.com> Signed-off-by: Jeremy Allison <jallison@ciq.com>
Requested by the lab. Signed-off-by: Jeremy Allison <jallison@ciq.com>
Requested by the lab. Signed-off-by: Jeremy Allison <jallison@ciq.com>
Retquested by the lab. Signed-off-by: Jeremy Allison <jallison@ciq.com>
…ypically implemented after dh_is_pubkey_valid. Requested by lab. Signed-off-by: Jeremy Allison <jallison@ciq.com>
…the digest to be generated - it must be at least 112 bits. Requested by the lab. Signed-off-by: Jeremy Allison <jallison@ciq.com>
Ensure this is initialized correctly based on system state and key length. Requested by lab. Signed-off-by: Jeremy Allison <jallison@ciq.com>
Ensure this is initialized correctly based on system state and key length. Requested by lab. Signed-off-by: Jeremy Allison <jallison@ciq.com>
Ensure this is initialized correctly based on system state and key length. Requested by lab. Signed-off-by: Jeremy Allison <jallison@ciq.com>
Ensure this is initialized correctly based on system state and key length. Requested by lab. Signed-off-by: Jeremy Allison <jallison@ciq.com>
Ensure this is initialized correctly based on system state and key length. Requested by lab. Signed-off-by: Jeremy Allison <jallison@ciq.com>
Requested by lab. Signed-off-by: Jeremy Allison <jallison@ciq.com>
…ciq.6.18.20260531" Requested by lab. Will be changed for rpm builds. Signed-off-by: Jeremy Allison <jallison@ciq.com>
…E_128. Requested by lab. Signed-off-by: Jeremy Allison <jallison@ciq.com>
adb7ce6 to
0ad59df
Compare
We have this patch with a better commit message in rlc-9. Updated
@PlaidCat I got so caught up in adding the headers for the stuff that came from somewhere else I forgot about your suggestion for tagging our own stuff. You are thinking something like this? |
If we cant get the ones commit message cleaned up i'm fine with leaving off our CIQ_ORIGINAL from an adhoc methodology and something we can work out more cleanly in offline. I don't want to hold this up as I see there is another rebase in queue. |
Summary
This is all of the changes from jallison-ciq-6.18.y-fips-patches-on-top, rebased on the latest ciq-6.18.y and with ciq commit headers on all of the commits that came from someplace else.
Commit Message(s)
Test Results
✅ Build Stage
✅ Boot Verification
✅ Kernel Selftests
✅ LTP Results
🤖 This PR was automatically generated by GitHub Actions
Run ID: 26843426190