Messages in this thread | | | Date | Tue, 26 Oct 2021 12:03:24 -0700 | From | Deven Bowers <> | Subject | Re: [RFC PATCH v7 07/16] ipe: add auditing support |
| |
On 10/15/2021 12:50 PM, Randy Dunlap wrote: > On 10/15/21 12:25 PM, Deven Bowers wrote: >> On 10/13/2021 3:54 PM, Randy Dunlap wrote: >>> Hi, >>> >>> On 10/13/21 12:06 PM, deven.desai@linux.microsoft.com wrote: >>>> diff --git a/security/ipe/Kconfig b/security/ipe/Kconfig >>>> index c4503083e92d..ef556b66e674 100644 >>>> --- a/security/ipe/Kconfig >>>> +++ b/security/ipe/Kconfig >>>> @@ -17,3 +17,55 @@ menuconfig SECURITY_IPE >>>> requirements on the fly. >>>> If unsure, answer N. >>>> + >>>> +if SECURITY_IPE >>>> + >>>> +choice >>>> + prompt "Hash algorithm used in auditing policies" >>>> + default IPE_AUDIT_HASH_SHA1 >>>> + depends on AUDIT >>>> + help >>>> + Specify the hash algorithm used when auditing policies. >>>> + The hash is used to uniquely identify a policy from other >>>> + policies on the system. >>>> + >>>> + If unsure, leave default. >>>> + >>>> + config IPE_AUDIT_HASH_SHA1 >>>> + bool "sha1" >>>> + depends on CRYPTO_SHA1 >>>> + help >>>> + Use the SHA128 algorithm to hash policies >>>> + in the audit records. >>>> + >>>> + config IPE_AUDIT_HASH_SHA256 >>>> + bool "sha256" >>>> + depends on CRYPTO_SHA256 >>>> + help >>>> + Use the SHA256 algorithm to hash policies >>>> + in the audit records. >>>> + >>>> + config IPE_AUDIT_HASH_SHA384 >>>> + bool "sha384" >>>> + depends on CRYPTO_SHA512 >>>> + help >>>> + Use the SHA384 algorithm to hash policies >>>> + in the audit records >>>> + >>>> + config IPE_AUDIT_HASH_SHA512 >>>> + bool "sha512" >>>> + depends on CRYPTO_SHA512 >>>> + help >>>> + Use the SHA512 algorithm to hash policies >>>> + in the audit records >>>> +endchoice >>>> + >>>> +config IPE_AUDIT_HASH_ALG >>>> + string >>>> + depends on AUDIT >>>> + default "sha1" if IPE_AUDIT_HASH_SHA1 >>>> + default "sha256" if IPE_AUDIT_HASH_SHA256 >>>> + default "sha384" if IPE_AUDIT_HASH_SHA384 >>>> + default "sha512" if IPE_AUDIT_HASH_SHA512 >>>> + >>>> +endif >>> >>> Please follow coding-style for Kconfig files: >>> >>> (from Documentation/process/coding-style.rst, section 10): >>> >>> For all of the Kconfig* configuration files throughout the source tree, >>> the indentation is somewhat different. Lines under a ``config`` >>> definition >>> are indented with one tab, while help text is indented an additional >>> two >>> spaces. >>> >> Oof. That's embarrassing. Sorry, I'll fix this for v8. >> >> While I'm at it, is the help text required for choice configs? >> checkpatch --strict complains with a warning without them, but >> I see other places in the tree where help text is omitted for >> these configs attached to a choice. > > Does checkpatch complain about what you have above > or did you add that help text to keep it from complaining?
I added the help text to keep it from complaining (and added it incorrectly, clearly).
> > >> Documentation/process/* doesn't seem to have any guidance, nor >> Documentation/kbuild/* on whether it is safe to ignore that >> checkpatch warning. > > Yeah, I don't think that we have any good guidance on that. > > I would say that if the choice prompt provides good/adequate > help info, then each 'config' inside the choice block does not > need help text. OTOH, if the choice prompt has little/no help > info, then each 'config' under it should have some useful info. > > I only looked in arch/x86/Kconfig, init/Kconfig, and lib/Kconfig.debug, > but you can see either help text method being used in those. > > And then if the help text is adequate in either one of those > methods, I would just ignore the checkpatch complaints. > It's just a guidance tool.
Alright. I think the choice guidance is pretty clear:
Specify the hash algorithm used when auditing policies. The hash is used to uniquely identify a policy from other policies on the system.
So I'll remove the help text for these choices.
At worst, I can make the choice text more clear. > > HTH. >
| |