Received: by 2002:a05:6a10:2785:0:0:0:0 with SMTP id ia5csp3191485pxb; Tue, 12 Jan 2021 08:29:12 -0800 (PST) X-Google-Smtp-Source: ABdhPJwLkKbp7kbzHRBYM8MkF7kAT0Wb+D1BElC04z3Bq4M6B/+BjINWDVXnizBw0oJOlOANNSSW X-Received: by 2002:a17:906:5912:: with SMTP id h18mr3726323ejq.261.1610468952091; Tue, 12 Jan 2021 08:29:12 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1610468952; cv=none; d=google.com; s=arc-20160816; b=Uf6ki6+gB2aVGc+/lROf0i5NDugj6u1SDn92CL1vFXlHif2AK4lt1zBf7YXPvsJE63 IN3VvcddqdtMf6j1Cai9xJg9kTdzbH2MJcQoZAIEBIS7BQCeTFtKyVcDl3Dm9H/mE+6a xPyFBTKbAea4PKfu7FjZZoeiIVjF8o0StTBPClMvtz3RsT0F8T2L4DjDq/pyxrb3A5DC 6KTM/306OfXAlkKRpQnPtFu8pUoCihCaL4o6rnWZeO1riiVuMyWGTwZOaytgAYy3Kq6z AUmndBeVaah0yG070uSJcc73SWryrtPJ8+zkSn6pYb8EwBQR5pnN0dQZvJVI0MK8STR7 cPQQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version:dkim-signature; bh=w9XY+SrL1C8QH90qcJqPmDFWdeEzWfdgu1cbhdh73k4=; b=fFY8B62/5JO/lC/1vH/O1gJTrBZJdZ9Eks5QJngIqwrklt6Mx0hWlsfUtgU7zII+8y GvZ1b13ej6eiTRUeSbO4plmOg+/e7MS2itF3nW3rmejtlT7qhG9Ju3jLQXpHWOMe+CDW fDfpaUKGQQY+SL+eguQwjfgekZ14CmdCSH362H/6VZyovEcTfbwsZGmV0MYHtW4LICE1 o+m0j/CjIsfJKJLCSPuAEPBvXcKhw6t3AqvNXbt4RF9PuiJhaoSvkypXTHRJz+lOvfYr lNV9ixLd6j+jh8YQxEXLSCjcEJCmTv55uFPySjnr2zuTUbLVesMZfEi1v6mXF04Ijnip 8XeQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@paul-moore-com.20150623.gappssmtp.com header.s=20150623 header.b="EQ/fT5KT"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id kt6si1259281ejb.378.2021.01.12.08.28.46; Tue, 12 Jan 2021 08:29:12 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@paul-moore-com.20150623.gappssmtp.com header.s=20150623 header.b="EQ/fT5KT"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2406724AbhALQ2I (ORCPT + 99 others); Tue, 12 Jan 2021 11:28:08 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34258 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2406390AbhALQ2I (ORCPT ); Tue, 12 Jan 2021 11:28:08 -0500 Received: from mail-ej1-x62d.google.com (mail-ej1-x62d.google.com [IPv6:2a00:1450:4864:20::62d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 400DCC0617A3 for ; Tue, 12 Jan 2021 08:27:21 -0800 (PST) Received: by mail-ej1-x62d.google.com with SMTP id lt17so4400541ejb.3 for ; Tue, 12 Jan 2021 08:27:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=paul-moore-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=w9XY+SrL1C8QH90qcJqPmDFWdeEzWfdgu1cbhdh73k4=; b=EQ/fT5KTR3MtbCTetUzVn/GWwkD8fDJdMXji5OfxzkRmMlIP58JNlysqqO5iAZ51ta M/v76BBkKQph3OPmuxOLrE56u6UcmQuufbKg0rIHzmYaU9LkndR/x9/rjLDCxLnTDHvK eWFzzz8GVOIFJCCUTh6eMeTaAf6iTx5mqsbkiei8l4Mf1PvTmgPWwwr/4xN1CFMzNgYv Gpk/8WU51A8ksp/8MoHqeJ9c6RmMMVzP1qIxXha4OxZngMLZoQX1Jo1fuC5SZdP5XS9m la+BdHJWU6mKMitROBWVmWqFRKu6fzu0I0CgeXEvwSlZKtn9X0vblx8adBoyLIah5xj9 j+Jg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=w9XY+SrL1C8QH90qcJqPmDFWdeEzWfdgu1cbhdh73k4=; b=F72z6Yv4sBQ5wMwXyhIAVG4HMozwCD6Dotc6xM5ftdafA7KAznDJa/VodzZFztnDDS vubwy5lWySvhmA7TAxjVV2c1ECGk82liA2jd7/IWciBnO9v3Kd0GzdG7Ueme2JCBS+Qr 7weZp/cj46QOllKugqSvqn3unKIe5G+ASfCxj0kfKv86BVmjajJX58mm21RCipgo0AWY pG5zctcFl5VRSazHbqHncdh80/mzf9OMyMU/ny4Abh9TcldHYUIWDYzMo6efsKX95Aru B5ZNAp9lWY0pYExWip6DNpLEf3BnSGjJT0bX7iZjStwYcy/h0jog7d2j3EC+Wr4p3eSB a2dg== X-Gm-Message-State: AOAM531tJxo6BG39fVSHu4oLYBoVBhrv9LGCVS8s/7KuMPFjvWDtISOE O2cw2xlkZEt/JsNpW+AYqeRAuTzm/xzX1xyefGVG X-Received: by 2002:a17:906:3712:: with SMTP id d18mr3920253ejc.178.1610468839776; Tue, 12 Jan 2021 08:27:19 -0800 (PST) MIME-Version: 1.0 References: <20210108040708.8389-1-tusharsu@linux.microsoft.com> <20210108040708.8389-9-tusharsu@linux.microsoft.com> In-Reply-To: <20210108040708.8389-9-tusharsu@linux.microsoft.com> From: Paul Moore Date: Tue, 12 Jan 2021 11:27:08 -0500 Message-ID: Subject: Re: [PATCH v10 8/8] selinux: include a consumer of the new IMA critical data hook To: Tushar Sugandhi Cc: zohar@linux.ibm.com, Stephen Smalley , casey@schaufler-ca.com, agk@redhat.com, snitzer@redhat.com, gmazyland@gmail.com, tyhicks@linux.microsoft.com, sashal@kernel.org, James Morris , nramas@linux.microsoft.com, linux-integrity@vger.kernel.org, selinux@vger.kernel.org, linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org, dm-devel@redhat.com Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jan 7, 2021 at 11:07 PM Tushar Sugandhi wrote: > From: Lakshmi Ramasubramanian > > SELinux stores the active policy in memory, so the changes to this data > at runtime would have an impact on the security guarantees provided > by SELinux. Measuring in-memory SELinux policy through IMA subsystem > provides a secure way for the attestation service to remotely validate > the policy contents at runtime. > > Measure the hash of the loaded policy by calling the IMA hook > ima_measure_critical_data(). Since the size of the loaded policy > can be large (several MB), measure the hash of the policy instead of > the entire policy to avoid bloating the IMA log entry. > > To enable SELinux data measurement, the following steps are required: > > 1, Add "ima_policy=critical_data" to the kernel command line arguments > to enable measuring SELinux data at boot time. > For example, > BOOT_IMAGE=/boot/vmlinuz-5.10.0-rc1+ root=UUID=fd643309-a5d2-4ed3-b10d-3c579a5fab2f ro nomodeset security=selinux ima_policy=critical_data > > 2, Add the following rule to /etc/ima/ima-policy > measure func=CRITICAL_DATA label=selinux > > Sample measurement of the hash of SELinux policy: > > To verify the measured data with the current SELinux policy run > the following commands and verify the output hash values match. > > sha256sum /sys/fs/selinux/policy | cut -d' ' -f 1 > > grep "selinux-policy-hash" /sys/kernel/security/integrity/ima/ascii_runtime_measurements | tail -1 | cut -d' ' -f 6 > > Note that the actual verification of SELinux policy would require loading > the expected policy into an identical kernel on a pristine/known-safe > system and run the sha256sum /sys/kernel/selinux/policy there to get > the expected hash. > > Signed-off-by: Lakshmi Ramasubramanian > Suggested-by: Stephen Smalley > Reviewed-by: Tyler Hicks > --- > Documentation/ABI/testing/ima_policy | 3 +- > security/selinux/Makefile | 2 + > security/selinux/ima.c | 64 ++++++++++++++++++++++++++++ > security/selinux/include/ima.h | 24 +++++++++++ > security/selinux/include/security.h | 3 +- > security/selinux/ss/services.c | 64 ++++++++++++++++++++++++---- > 6 files changed, 149 insertions(+), 11 deletions(-) > create mode 100644 security/selinux/ima.c > create mode 100644 security/selinux/include/ima.h I remain concerned about the possibility of bypassing a measurement by tampering with the time, but I appear to be the only one who is worried about this so I'm not going to block this patch on those grounds. Acked-by: Paul Moore > diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy > index 54fe1c15ed50..8365596cb42b 100644 > --- a/Documentation/ABI/testing/ima_policy > +++ b/Documentation/ABI/testing/ima_policy > @@ -52,8 +52,9 @@ Description: > template:= name of a defined IMA template type > (eg, ima-ng). Only valid when action is "measure". > pcr:= decimal value > - label:= [data_label] > + label:= [selinux]|[data_label] > data_label:= a unique string used for grouping and limiting critical data. > + For example, "selinux" to measure critical data for SELinux. > > default policy: > # PROC_SUPER_MAGIC > diff --git a/security/selinux/Makefile b/security/selinux/Makefile > index 4d8e0e8adf0b..776162444882 100644 > --- a/security/selinux/Makefile > +++ b/security/selinux/Makefile > @@ -16,6 +16,8 @@ selinux-$(CONFIG_NETLABEL) += netlabel.o > > selinux-$(CONFIG_SECURITY_INFINIBAND) += ibpkey.o > > +selinux-$(CONFIG_IMA) += ima.o > + > ccflags-y := -I$(srctree)/security/selinux -I$(srctree)/security/selinux/include > > $(addprefix $(obj)/,$(selinux-y)): $(obj)/flask.h > diff --git a/security/selinux/ima.c b/security/selinux/ima.c > new file mode 100644 > index 000000000000..0b835bdc3aa9 > --- /dev/null > +++ b/security/selinux/ima.c > @@ -0,0 +1,64 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (C) 2021 Microsoft Corporation > + * > + * Author: Lakshmi Ramasubramanian (nramas@linux.microsoft.com) > + * > + * Measure critical data structures maintainted by SELinux > + * using IMA subsystem. > + */ > +#include > +#include > +#include > +#include "security.h" > +#include "ima.h" > + > +/* > + * selinux_ima_measure_state - Measure hash of the SELinux policy > + * > + * @state: selinux state struct > + * > + * NOTE: This function must be called with policy_mutex held. > + */ > +void selinux_ima_measure_state(struct selinux_state *state) > +{ > + struct timespec64 cur_time; > + void *policy = NULL; > + char *policy_event_name = NULL; > + size_t policy_len; > + int rc = 0; > + > + /* > + * Measure SELinux policy only after initialization is completed. > + */ > + if (!selinux_initialized(state)) > + return; > + > + /* > + * Pass a unique "event_name" to the IMA hook so that IMA subsystem > + * will always measure the given data. > + */ > + ktime_get_real_ts64(&cur_time); > + policy_event_name = kasprintf(GFP_KERNEL, "%s-%lld:%09ld", > + "selinux-policy-hash", > + cur_time.tv_sec, cur_time.tv_nsec); > + if (!policy_event_name) { > + pr_err("SELinux: %s: event name for policy not allocated.\n", > + __func__); > + goto out; > + } > + > + rc = security_read_state_kernel(state, &policy, &policy_len); > + if (rc) { > + pr_err("SELinux: %s: failed to read policy %d.\n", __func__, rc); > + goto out; > + } > + > + ima_measure_critical_data("selinux", policy_event_name, > + policy, policy_len, true); > + > + vfree(policy); > + > +out: > + kfree(policy_event_name); > +} > diff --git a/security/selinux/include/ima.h b/security/selinux/include/ima.h > new file mode 100644 > index 000000000000..d69c36611423 > --- /dev/null > +++ b/security/selinux/include/ima.h > @@ -0,0 +1,24 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > +/* > + * Copyright (C) 2021 Microsoft Corporation > + * > + * Author: Lakshmi Ramasubramanian (nramas@linux.microsoft.com) > + * > + * Measure critical data structures maintainted by SELinux > + * using IMA subsystem. > + */ > + > +#ifndef _SELINUX_IMA_H_ > +#define _SELINUX_IMA_H_ > + > +#include "security.h" > + > +#ifdef CONFIG_IMA > +extern void selinux_ima_measure_state(struct selinux_state *selinux_state); > +#else > +static inline void selinux_ima_measure_state(struct selinux_state *selinux_state) > +{ > +} > +#endif > + > +#endif /* _SELINUX_IMA_H_ */ > diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h > index 3cc8bab31ea8..29cae32d3fc5 100644 > --- a/security/selinux/include/security.h > +++ b/security/selinux/include/security.h > @@ -229,7 +229,8 @@ void selinux_policy_cancel(struct selinux_state *state, > struct selinux_policy *policy); > int security_read_policy(struct selinux_state *state, > void **data, size_t *len); > - > +int security_read_state_kernel(struct selinux_state *state, > + void **data, size_t *len); > int security_policycap_supported(struct selinux_state *state, > unsigned int req_cap); > > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c > index 9704c8a32303..cc8dbc4ed8db 100644 > --- a/security/selinux/ss/services.c > +++ b/security/selinux/ss/services.c > @@ -65,6 +65,7 @@ > #include "ebitmap.h" > #include "audit.h" > #include "policycap_names.h" > +#include "ima.h" > > /* Forward declaration. */ > static int context_struct_to_string(struct policydb *policydb, > @@ -2180,6 +2181,7 @@ static void selinux_notify_policy_change(struct selinux_state *state, > selinux_status_update_policyload(state, seqno); > selinux_netlbl_cache_invalidate(); > selinux_xfrm_notify_policyload(); > + selinux_ima_measure_state(state); > } > > void selinux_policy_commit(struct selinux_state *state, > @@ -3875,8 +3877,33 @@ int security_netlbl_sid_to_secattr(struct selinux_state *state, > } > #endif /* CONFIG_NETLABEL */ > > +/** > + * __security_read_policy - read the policy. > + * @policy: SELinux policy > + * @data: binary policy data > + * @len: length of data in bytes > + * > + */ > +static int __security_read_policy(struct selinux_policy *policy, > + void *data, size_t *len) > +{ > + int rc; > + struct policy_file fp; > + > + fp.data = data; > + fp.len = *len; > + > + rc = policydb_write(&policy->policydb, &fp); > + if (rc) > + return rc; > + > + *len = (unsigned long)fp.data - (unsigned long)data; > + return 0; > +} > + > /** > * security_read_policy - read the policy. > + * @state: selinux_state > * @data: binary policy data > * @len: length of data in bytes > * > @@ -3885,8 +3912,6 @@ int security_read_policy(struct selinux_state *state, > void **data, size_t *len) > { > struct selinux_policy *policy; > - int rc; > - struct policy_file fp; > > policy = rcu_dereference_protected( > state->policy, lockdep_is_held(&state->policy_mutex)); > @@ -3898,14 +3923,35 @@ int security_read_policy(struct selinux_state *state, > if (!*data) > return -ENOMEM; > > - fp.data = *data; > - fp.len = *len; > + return __security_read_policy(policy, *data, len); > +} > > - rc = policydb_write(&policy->policydb, &fp); > - if (rc) > - return rc; > +/** > + * security_read_state_kernel - read the policy. > + * @state: selinux_state > + * @data: binary policy data > + * @len: length of data in bytes > + * > + * Allocates kernel memory for reading SELinux policy. > + * This function is for internal use only and should not > + * be used for returning data to user space. > + * > + * This function must be called with policy_mutex held. > + */ > +int security_read_state_kernel(struct selinux_state *state, > + void **data, size_t *len) > +{ > + struct selinux_policy *policy; > > - *len = (unsigned long)fp.data - (unsigned long)*data; > - return 0; > + policy = rcu_dereference_protected( > + state->policy, lockdep_is_held(&state->policy_mutex)); > + if (!policy) > + return -EINVAL; > + > + *len = policy->policydb.len; > + *data = vmalloc(*len); > + if (!*data) > + return -ENOMEM; > > + return __security_read_policy(policy, *data, len); > } > -- > 2.17.1 > -- paul moore www.paul-moore.com