| Message ID | 20200121171302.4935-1-nramas@linux.microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | IMA: Turn IMA_MEASURE_ASYMMETRIC_KEYS off by default | expand |
On Tue, 2020-01-21 at 09:13 -0800, Lakshmi Ramasubramanian wrote: > Enabling IMA and ASYMMETRIC_PUBLIC_KEY_SUBTYPE configs will > automatically enable the IMA hook to measure asymmetric keys. Keys > created or updated early in the boot process are queued up whether > or not a custom IMA policy is provided. Although the queued keys will > be freed if a custom IMA policy is not loaded within 5 minutes, it > could still cause significant performance impact on smaller systems. What exactly do you expect distributions to do with this? I can tell you that most of them will take the default option, so this gets set to N and you may as well not have got the patches upstream because you won't be able to use them in any distro with this setting. > This patch turns the config IMA_MEASURE_ASYMMETRIC_KEYS off by > default. Since a custom IMA policy that defines key measurement is > required to measure keys, systems that require key measurement can > enable this config option in addition to providing a custom IMA > policy. Well, no they can't ... it's rather rare nowadays for people to build their own kernels. The vast majority of Linux consumers take what the distros give them. Think carefully before you decide a config option is the solution to this problem. James
On 1/21/20 9:34 AM, James Bottomley wrote: > What exactly do you expect distributions to do with this? I can tell > you that most of them will take the default option, so this gets set to > N and you may as well not have got the patches upstream because you > won't be able to use them in any distro with this setting. I agree - distros that are not sure or don't care about key measurement are anyway not going to choose this option. Only those that really care will opt in. My goal is to not burden the vast majority of the users with this additional overhead if they don't need it - particularly, small systems such as embedded devices, etc. > > Well, no they can't ... it's rather rare nowadays for people to build > their own kernels. The vast majority of Linux consumers take what the > distros give them. Think carefully before you decide a config option > is the solution to this problem. > > James > If you have suggestions for how I can handle it in a different way (other than config option), I'll be happy to try it out. thanks, -lakshmi
On Tue, 2020-01-21 at 09:34 -0800, James Bottomley wrote: > On Tue, 2020-01-21 at 09:13 -0800, Lakshmi Ramasubramanian wrote: > > Enabling IMA and ASYMMETRIC_PUBLIC_KEY_SUBTYPE configs will > > automatically enable the IMA hook to measure asymmetric keys. Keys > > created or updated early in the boot process are queued up whether > > or not a custom IMA policy is provided. Although the queued keys will > > be freed if a custom IMA policy is not loaded within 5 minutes, it > > could still cause significant performance impact on smaller systems. > > What exactly do you expect distributions to do with this? I can tell > you that most of them will take the default option, so this gets set to > N and you may as well not have got the patches upstream because you > won't be able to use them in any distro with this setting. > > > This patch turns the config IMA_MEASURE_ASYMMETRIC_KEYS off by > > default. Since a custom IMA policy that defines key measurement is > > required to measure keys, systems that require key measurement can > > enable this config option in addition to providing a custom IMA > > policy. > > Well, no they can't ... it's rather rare nowadays for people to build > their own kernels. The vast majority of Linux consumers take what the > distros give them. Think carefully before you decide a config option > is the solution to this problem. James, up until now IMA could be configured, but there wouldn't be any performance penalty for enabling IMA until a policy was loaded. With IMA and asymmetric keys enabled, whether or not an IMA policy is loaded, certificates will be queued. My concern is: - changing the expected behavior - really small devices/sensors being able to queue certificates This change permits disabling queueing certificates. Whether the default should be "disabled" is a separate question. I'm open to comments/suggestions. Mimi
On Tue, 2020-01-21 at 14:13 -0500, Mimi Zohar wrote: > On Tue, 2020-01-21 at 09:34 -0800, James Bottomley wrote: > > On Tue, 2020-01-21 at 09:13 -0800, Lakshmi Ramasubramanian wrote: > > > Enabling IMA and ASYMMETRIC_PUBLIC_KEY_SUBTYPE configs will > > > automatically enable the IMA hook to measure asymmetric keys. > > > Keys created or updated early in the boot process are queued up > > > whether or not a custom IMA policy is provided. Although the > > > queued keys will be freed if a custom IMA policy is not loaded > > > within 5 minutes, it could still cause significant performance > > > impact on smaller systems. > > > > What exactly do you expect distributions to do with this? I can > > tell you that most of them will take the default option, so this > > gets set to N and you may as well not have got the patches upstream > > because you won't be able to use them in any distro with this > > setting. > > > > > This patch turns the config IMA_MEASURE_ASYMMETRIC_KEYS off by > > > default. Since a custom IMA policy that defines key measurement > > > is required to measure keys, systems that require key measurement > > > can enable this config option in addition to providing a custom > > > IMA policy. > > > > Well, no they can't ... it's rather rare nowadays for people to > > build their own kernels. The vast majority of Linux consumers take > > what the distros give them. Think carefully before you decide a > > config option is the solution to this problem. > > James, up until now IMA could be configured, but there wouldn't be > any performance penalty for enabling IMA until a policy was loaded. > With IMA and asymmetric keys enabled, whether or not an IMA policy > is loaded, certificates will be queued. > > My concern is: > - changing the expected behavior In general config options for this are a really bad idea because if the tools only cope with one setting, no-one should ever use the other and if they work with everything there's no need for the option. > - really small devices/sensors being able to queue certificates seems like the answer to this one would be don't queue. I realise it's after the submit design, but what about measuring when the key is added if there's a policy otherwise measure the keyring when the policy is added ... that way no queueing. > This change permits disabling queueing certificates. Whether the > default should be "disabled" is a separate question. I'm open to > comments/suggestions. I'm just giving the general rule of thumb for boolean config options. If it's default Y there likely shouldn't be a config option and if it's default N the feature should likely not be in the kernel at all.
On 1/21/2020 11:52 AM, James Bottomley wrote: >> - really small devices/sensors being able to queue certificates > > seems like the answer to this one would be don't queue. I realise it's > after the submit design, but what about measuring when the key is added > if there's a policy otherwise measure the keyring when the policy is > added ... that way no queueing. Without the "deferred key processing" changes, only keys added at runtime were measured (if policy permitted). "deferred key processing" enabled queuing keys added early in the boot process and measured them when the policy is loaded. We can make this (the queuing) optional through a config, but leave the runtime key measurement auto-enabled (as is the config IMA_MEASURE_ASYMMETRIC_KEYS now). -lakshmi
On Tue, 2020-01-21 at 11:52 -0800, James Bottomley wrote: > On Tue, 2020-01-21 at 14:13 -0500, Mimi Zohar wrote: > > This change permits disabling queueing certificates. Whether the > > default should be "disabled" is a separate question. I'm open to > > comments/suggestions. > > I'm just giving the general rule of thumb for boolean config options. > If it's default Y there likely shouldn't be a config option and if it's > default N the feature should likely not be in the kernel at all. Thanks, James. I'll keep this in mind when debating about defining a new Kconfig option. Mimi
On Tue, 2020-01-21 at 12:38 -0800, Lakshmi Ramasubramanian wrote: > On 1/21/2020 11:52 AM, James Bottomley wrote: > > >> - really small devices/sensors being able to queue certificates > > > > seems like the answer to this one would be don't queue. I realise it's > > after the submit design, but what about measuring when the key is added > > if there's a policy otherwise measure the keyring when the policy is > > added ... that way no queueing. > > Without the "deferred key processing" changes, only keys added at > runtime were measured (if policy permitted). > > "deferred key processing" enabled queuing keys added early in the boot > process and measured them when the policy is loaded. > > We can make this (the queuing) optional through a config, but leave the > runtime key measurement auto-enabled (as is the config > IMA_MEASURE_ASYMMETRIC_KEYS now). Thanks, Lakshmi. This requires moving the code around. Instead of doing this on the current code base, I suggest posting a v9 version of the entire "IMA: Deferred measurement of keys". I suggest making the switch from spinlock to mutex, as you had it originally, before posting v9. The commit history will then be a lot cleaner. thanks, Mimi
On 1/22/20 12:02 PM, Mimi Zohar wrote: > > Thanks, Lakshmi. This requires moving the code around. Instead of > doing this on the current code base, I suggest posting a v9 version of > the entire "IMA: Deferred measurement of keys". > > I suggest making the switch from spinlock to mutex, as you had it > originally, before posting v9. The commit history will then be a lot > cleaner. > > thanks, > > Mimi > Sure Mimi - I'll post an update to the patch set shortly. thanks, -lakshmi
On Wed, 2020-01-22 at 12:05 -0800, Lakshmi Ramasubramanian wrote: > On 1/22/20 12:02 PM, Mimi Zohar wrote: > > > > > Thanks, Lakshmi. This requires moving the code around. Instead of > > doing this on the current code base, I suggest posting a v9 version of > > the entire "IMA: Deferred measurement of keys". > > > > I suggest making the switch from spinlock to mutex, as you had it > > originally, before posting v9. The commit history will then be a lot > > cleaner. > > > > thanks, > > > > Mimi > > > > Sure Mimi - I'll post an update to the patch set shortly. I just pushed out the next-integrity-testing branch without the "Deferred measurement of keys" patches. I've kept the "IMA: pre- allocate buffer to hold keyrings string" patch, though it probably isn't required. thanks, Mimi
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig index 355754a6b6ca..8e678219ee9e 100644 --- a/security/integrity/ima/Kconfig +++ b/security/integrity/ima/Kconfig @@ -312,7 +312,19 @@ config IMA_APPRAISE_SIGNED_INIT This option requires user-space init to be signed. config IMA_MEASURE_ASYMMETRIC_KEYS - bool + bool "Enable asymmetric keys measurement on key create or update" depends on IMA depends on ASYMMETRIC_PUBLIC_KEY_SUBTYPE=y - default y + default n + help + This option enables measuring asymmetric keys when the key + is created or updated. Additionally a custom IMA policy that + defines key measurement should also be loaded. + + If this option is enabled, keys created or updated early in + the boot process are queued up. The queued keys are processed + when a custom IMA policy is loaded. But if a custom IMA policy + is not loaded within 5 minutes after IMA subsystem is initialized, + any queued keys are just freed. Keys created or updated after + a custom IMA policy is loaded will be processed immediately and + not queued.
Enabling IMA and ASYMMETRIC_PUBLIC_KEY_SUBTYPE configs will automatically enable the IMA hook to measure asymmetric keys. Keys created or updated early in the boot process are queued up whether or not a custom IMA policy is provided. Although the queued keys will be freed if a custom IMA policy is not loaded within 5 minutes, it could still cause significant performance impact on smaller systems. This patch turns the config IMA_MEASURE_ASYMMETRIC_KEYS off by default. Since a custom IMA policy that defines key measurement is required to measure keys, systems that require key measurement can enable this config option in addition to providing a custom IMA policy. Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com> --- security/integrity/ima/Kconfig | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)