Hi Mimi, On 7/15/21 2:26 PM, Mimi Zohar wrote: > Hi Simon, > > On Tue, 2021-07-13 at 14:48 +0000, THOBY Simon wrote: >> Sorry it took me some time before I could write a patch (in addition it's my [snip] >> - with ima_accept_any_hash: permits access, no warnings whatsoever >> Do you want ideas of other configurations that I could test? >> >> I would also like to point out that I'm more than open to suggestions for >> changing the names of the parameters (`ima_allowed_hashes` and >> `ima_accept_any_hash`) and the "cause" in the audit log (currently >> forbidden-hash-algorithm"), because as you know, "naming things is hard". >> >> >> IMA: add an option to restrict the accepted hash algorithms >> >> Adds two command-line parameters to limit the hash algorithms >> allowed for the security.ima xattr. > > Having two boot command line paramaters is a good indication that this > patch needs to be broken up. Please refer to the section "Separate > your changes" in Documentation/process/submitting-patches.rst. The > cover letter provides the motivation for the patch set. > Thanks for the clarification, I will split up the patch if a similar form with two different options is kept. >> This gives users the >> ability to ensure their systems will not accept weak hashes, >> and potentially increase users trust in their IMA configuration, >> because they can ensure only strong collision-resistant hashes >> are employed and files generated otherwise will not be accepted. > > Boot command line options should be minimized as much as possible. > Perhaps without defining new kernel boot paramaters there are some > additional checks that could be added. For example, on a FIPS enabled > system, prevent writing non FIPS allowed file signatures, limit file > signature algorithms to those enabled on the system, define new policy > rules to limit the permitted hash algorithms. I understand the desire to keep the kernel parameter list as minimal as possible, yet I'm not sure FIPS compliance targets the same use case. I see this patch as potentially of interest for embedded products with custom kernels (where "custom" here means a mainline kernel with more security options enabled than in the kernels of most distros), and these systems do not necessarily target FIPS compliance. In addition, FIPS 140-2 is the latest released version of FIPS 140 from what I could gather - I didn't quite understand how far in the standardization process FIPS 140-3 was, but it doesn't seem totally public and/or deployed in the wild yet. And FIPS 140-2 still permits use of SHA1 (not just HMAC-SHA, but the digest algorithm too, see https://docs.oracle.com/cd/E53394_01/html/E54966/fips-refs.html) but one of the points of this patchset was to allow people to enforce stronger algorithms than SHA1 for IMA. This is why I am unsure that restricting this mechanism to FIPS-enabled systems is the best approach. However, it could still be interesting to enforce FIPS 140-2 authorized algorithms in IMA, so maybe that could be a followup patch ? BTW, that may require adding a list of the FIPS allowed algorithms to the kernel, as I haven't found such a list when grepping "fips" through the kernel codebase. Off the top of my head, I see three main alternatives to expose this functionality: 1) Instead of supplying them at runtime in the kernel cmdline, add compile-time toggles that hardcode in the kernel the list of authorized algorithms and the expected behavior (verifying on write and/or verifying on appraisal). It would basically be the same thing as the proposed patch, but without involving new command line parameters. 2) Use the ima_hash paramter as the sole authorized algorithm. I suppose this is what you meant by "limit file signature algorithms to those enabled on the system, unless you meant the algorithms built in the kernel with the CRYPTO_* compile options ? 3) As you suggested, extend the policy DSL to limit the permitted hash algorithms. This yields the added advantage of flexibility: you can decide to restrict hashes for only some category of files/file systems, even though I'm don't see many use cases for such flexibility (but I hold no doubt someone somewhere would find some use to it). I would love to know if you think one of those alternatives would be better suited that the others for IMA, or if you thought of another option. As a side note, I would also like to thank profusely Lakshmi for his remarks on the patch description and the process itself, as they will prove quite valuable when submitting new revisions of this patch! Thanks, Simon
