Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 14 Jul 2023 21:00:29 -0700
From:   Fan Wu <wufan@linux.microsoft.com>
To:     Paul Moore <paul@paul-moore.com>
Cc:     corbet@lwn.net, zohar@linux.ibm.com, jmorris@namei.org,
        serge@hallyn.com, tytso@mit.edu, ebiggers@kernel.org,
        axboe@kernel.dk, agk@redhat.com, snitzer@kernel.org,
        eparis@redhat.com, linux-doc@vger.kernel.org,
        linux-integrity@vger.kernel.org,
        linux-security-module@vger.kernel.org,
        linux-fscrypt@vger.kernel.org, linux-block@vger.kernel.org,
        dm-devel@redhat.com, audit@vger.kernel.org,
        roberto.sassu@huawei.com, linux-kernel@vger.kernel.org,
        Deven Bowers <deven.desai@linux.microsoft.com>
Subject: Re: [PATCH RFC v10 9/17] ipe: add permissive toggle
Message-ID: <20230715040029.GH15267@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
References: <1687986571-16823-10-git-send-email-wufan@linux.microsoft.com>
 <85af33c02638ebb501b40fd0f3785b12.paul@paul-moore.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <85af33c02638ebb501b40fd0f3785b12.paul@paul-moore.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: bulk

On Sat, Jul 08, 2023 at 12:23:06AM -0400, Paul Moore wrote:
> On Jun 28, 2023 Fan Wu <wufan@linux.microsoft.com> wrote:
> > 
> > IPE, like SELinux, supports a permissive mode. This mode allows policy
> > authors to test and evaluate IPE policy without it effecting their
> > programs. When the mode is changed, a 1404 AUDIT_MAC_STATUS
> > be reported.
> > 
> > This patch adds the following audit records:
> > 
> >     audit: MAC_STATUS enforcing=0 old_enforcing=1 auid=4294967295
> >       ses=4294967295 enabled=1 old-enabled=1 lsm=ipe res=1
> >     audit: MAC_STATUS enforcing=1 old_enforcing=0 auid=4294967295
> >       ses=4294967295 enabled=1 old-enabled=1 lsm=ipe res=1
> > 
> > The audit record only emit when the value from the user input is
> > different from the current enforce value.
> > 
> > Signed-off-by: Deven Bowers <deven.desai@linux.microsoft.com>
> > Signed-off-by: Fan Wu <wufan@linux.microsoft.com>
> > ---
> >  security/ipe/audit.c | 22 ++++++++++++++
> >  security/ipe/audit.h |  1 +
> >  security/ipe/eval.c  |  9 ++++++
> >  security/ipe/eval.h  |  1 +
> >  security/ipe/fs.c    | 69 ++++++++++++++++++++++++++++++++++++++++++++
> >  5 files changed, 102 insertions(+)
> 
> ...
> 
> > diff --git a/security/ipe/fs.c b/security/ipe/fs.c
> > index 6bd2aa84831b..1761d39e4d04 100644
> > --- a/security/ipe/fs.c
> > +++ b/security/ipe/fs.c
> > @@ -16,6 +16,7 @@ static struct dentry *np __ro_after_init;
> >  static struct dentry *root __ro_after_init;
> >  struct dentry *policy_root __ro_after_init;
> >  static struct dentry *audit_node __ro_after_init;
> > +static struct dentry *enforce_node __ro_after_init;
> >  
> >  /**
> >   * setaudit - Write handler for the securityfs node, "ipe/success_audit"
> > @@ -68,6 +69,61 @@ static ssize_t getaudit(struct file *f, char __user *data,
> >  	return simple_read_from_buffer(data, len, offset, result, 1);
> >  }
> >  
> > +/**
> > + * setenforce - Write handler for the securityfs node, "ipe/enforce"
> > + * @f: Supplies a file structure representing the securityfs node.
> > + * @data: Supplies a buffer passed to the write syscall.
> > + * @len: Supplies the length of @data.
> > + * @offset: unused.
> > + *
> > + * Return:
> > + * * >0	- Success, Length of buffer written
> > + * * <0	- Error
> > + */
> > +static ssize_t setenforce(struct file *f, const char __user *data,
> > +			  size_t len, loff_t *offset)
> > +{
> > +	int rc = 0;
> > +	bool new_value, old_value;
> > +
> > +	if (!file_ns_capable(f, &init_user_ns, CAP_MAC_ADMIN))
> > +		return -EPERM;
> > +
> > +	old_value = READ_ONCE(enforce);
> > +	new_value = old_value;
> 
> Why set @new_value equal to @old_value here?
> 
Sorry this mistake is the same as the one for audit switch. 
kstrtobool_from_user will return error if new_value is not set,
I will remove the above line.

-Fan
> > +	rc = kstrtobool_from_user(data, len, &new_value);
> > +	if (rc)
> > +		return rc;
> > +
> > +	if (new_value != old_value) {
> > +		ipe_audit_enforce(new_value, old_value);
> > +		WRITE_ONCE(enforce, new_value);
> > +	}
> > +
> > +	return len;
> > +}
> 
> --
> paul-moore.com