From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lan.nucleusys.com ([92.247.61.126]:58982 "EHLO zztop.nucleusys.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751929AbcBIHsY (ORCPT ); Tue, 9 Feb 2016 02:48:24 -0500 Date: Tue, 9 Feb 2016 09:47:42 +0200 From: Petko Manolov To: Mimi Zohar Cc: Dmitry Kasatkin , "linux-security-module@vger.kernel.org" , "Luis R. Rodriguez" , "kexec@lists.infradead.org" , "linux-modules@vger.kernel.org" , "fsdevel@vger.kernel.org" , David Howells , David Woodhouse , Kees Cook , Dmitry Torokhov , Dmitry Kasatkin , Eric Biederman , Rusty Russell , Dmitry Kasatkin Subject: Re: [PATCH v3 20/22] ima: load policy using path Message-ID: <20160209074742.GB2912@bender.nucleusys.com> References: <1454526390-19792-1-git-send-email-zohar@linux.vnet.ibm.com> <1454526390-19792-21-git-send-email-zohar@linux.vnet.ibm.com> <20160207195945.GG17321@localhost> <20160208103505.GA7931@bender.nucleusys.com> <1454965936.3037.34.camel@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <1454965936.3037.34.camel@linux.vnet.ibm.com> Sender: owner-linux-modules@vger.kernel.org List-ID: On 16-02-08 16:12:16, Mimi Zohar wrote: > On Mon, 2016-02-08 at 10:45 +0000, Dmitry Kasatkin wrote: > > > > > @@ -286,9 +322,12 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf, > > > > result = mutex_lock_interruptible(&ima_write_mutex); > > > > if (result < 0) > > > > goto out_free; > > > > - result = ima_parse_add_rule(data); > > > > - mutex_unlock(&ima_write_mutex); > > > > > > > > + if (data[0] == '/') > > > > > > >It seems that if we feed relative path to ima_policy the update will fail... > > > > > > Yes, i think it is always a good idea to pass absolute path. > > > > What if we at least emit a warning so people know what's wrong? > > The next patch "ima: measure and appraise the IMA policy itself" adds > the following. Is a failure message enough? That would be the wrong message. The above code does not handle relative paths so any attempt to load the policy by "./ima_policy_file" or "../../ima_policy_file" will fail. Isn't there a kernel function that checks if given string is a path-name? > + else if (ima_appraise & IMA_APPRAISE_POLICY) { > + pr_err("IMA: signed policy required\n"); > + integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL, > + "policy_update", "signed policy > required", > + 1, 0); > + if (ima_appraise & IMA_APPRAISE_ENFORCE) > + result = -EACCES; > + } else > result = ima_parse_add_rule(data); > > > > > Petko > > > > DK: May be a good idea to print that loading policy by path or not. > > Are we including the pathname? Are you suggesting a log or audit message? I guess log is more appropriate. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from lan.nucleusys.com ([92.247.61.126] helo=zztop.nucleusys.com) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1aT32T-00059q-Cv for kexec@lists.infradead.org; Tue, 09 Feb 2016 07:48:46 +0000 Date: Tue, 9 Feb 2016 09:47:42 +0200 From: Petko Manolov Subject: Re: [PATCH v3 20/22] ima: load policy using path Message-ID: <20160209074742.GB2912@bender.nucleusys.com> References: <1454526390-19792-1-git-send-email-zohar@linux.vnet.ibm.com> <1454526390-19792-21-git-send-email-zohar@linux.vnet.ibm.com> <20160207195945.GG17321@localhost> <20160208103505.GA7931@bender.nucleusys.com> <1454965936.3037.34.camel@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1454965936.3037.34.camel@linux.vnet.ibm.com> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "kexec" Errors-To: kexec-bounces+dwmw2=infradead.org@lists.infradead.org To: Mimi Zohar Cc: Rusty Russell , Kees Cook , "fsdevel@vger.kernel.org" , Dmitry Kasatkin , "Luis R. Rodriguez" , Dmitry Torokhov , "kexec@lists.infradead.org" , Dmitry Kasatkin , David Howells , "linux-security-module@vger.kernel.org" , Eric Biederman , Dmitry Kasatkin , David Woodhouse , "linux-modules@vger.kernel.org" On 16-02-08 16:12:16, Mimi Zohar wrote: > On Mon, 2016-02-08 at 10:45 +0000, Dmitry Kasatkin wrote: > > > > > @@ -286,9 +322,12 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf, > > > > result = mutex_lock_interruptible(&ima_write_mutex); > > > > if (result < 0) > > > > goto out_free; > > > > - result = ima_parse_add_rule(data); > > > > - mutex_unlock(&ima_write_mutex); > > > > > > > > + if (data[0] == '/') > > > > > > >It seems that if we feed relative path to ima_policy the update will fail... > > > > > > Yes, i think it is always a good idea to pass absolute path. > > > > What if we at least emit a warning so people know what's wrong? > > The next patch "ima: measure and appraise the IMA policy itself" adds > the following. Is a failure message enough? That would be the wrong message. The above code does not handle relative paths so any attempt to load the policy by "./ima_policy_file" or "../../ima_policy_file" will fail. Isn't there a kernel function that checks if given string is a path-name? > + else if (ima_appraise & IMA_APPRAISE_POLICY) { > + pr_err("IMA: signed policy required\n"); > + integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL, > + "policy_update", "signed policy > required", > + 1, 0); > + if (ima_appraise & IMA_APPRAISE_ENFORCE) > + result = -EACCES; > + } else > result = ima_parse_add_rule(data); > > > > > Petko > > > > DK: May be a good idea to print that loading policy by path or not. > > Are we including the pathname? Are you suggesting a log or audit message? I guess log is more appropriate. _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec