Re: Collecting ideas for audit2allow improvement

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




----- Original Message -----
> From: "Daniel Walsh" <dwalsh@xxxxxxxxxx>
> To: selinux@xxxxxxxxxxxxx
> Sent: Monday, June 19, 2017 11:45:53 AM
> Subject: Re: Collecting ideas for audit2allow improvement
> 
> On 06/16/2017 12:08 PM, Dominick Grift wrote:
> > On Fri, Jun 16, 2017 at 08:21:25AM -0400, Daniel Walsh wrote:
> >> On 06/14/2017 10:47 AM, Dominick Grift wrote:
> >>> On Wed, Jun 14, 2017 at 04:35:41PM +0200, Dominick Grift wrote:
> >>>> On Wed, Jun 14, 2017 at 10:30:25AM -0400, Stephen Smalley wrote:
> >>>>> On Wed, 2017-06-14 at 09:01 -0400, Jan Zarsky wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> I would like to improve SELinux audit2allow tool as my bachelor
> >>>>>> thesis.
> >>>>>> I collected ideas from my colleagues from RedHat SELinux team and I
> >>>>>> would also
> >>>>>> like to hear your ideas - what would you improve to make audit2allow
> >>>>>> smarter or
> >>>>>> easier to use.
> >>>>>>
> >>>>>> Ideas collected so far:
> >>>>>>
> >>>>>>     * offer dac_read_search when sufficient instead of dac_override
> >>>>>>       (see <https://github.com/SELinuxProject/selinux/issues/31>)
> >>>>> The hard part here is knowing when it is sufficient.  Might require
> >>>>> further kernel patches to get to the point where it is completely
> >>>>> unambiguous from the audit messages alone.  You could perhaps default
> >>>>> to only allowing dac_read_search and only allow dac_override if you see
> >>>>> that dac_read_search is already allowed and you are still getting a
> >>>>> dac_override denial.
> >>>> This should not be an issue anymore. Becuase now (linux v4.12)
> >>>> dac_read_search is first checked
> >>>> so translating a rule for dac_read_search will always be the most secure
> >>>> option. It might not be enough but youll notice if it isnt. Just rerun
> >>>> it again and you'll end up with dac_override if needed.
> >>>>
> >>>>>>     * offer multiple solutions to a problem (example: 1) add allow
> >>>>>>     rule
> >>>>>> for
> >>>>>>       execute + execute_no_trans or 2) add allow rule for execute
> >>>>>>       + type_transition rule)
> >>>>> Is this type_transition to an existing type that already exists, or
> >>>>> defining a new type and transitioning to it, or both?  Generating new
> >>>>> domains and types dynamically is one of the major gaps in current
> >>>>> audit2allow, and to date has only been supported in separate tools like
> >>>>> sepolicy generate.
> >>>> One should be extremely careful here. "execute" does not automatically
> >>>> imply "execute_no_trans". That assumption can lead to disaster
> >>>> (https://www.cvedetails.com/cve/CVE-2015-1815/)
> >>>>>>     * interactive mode: ask questions and choose best solution
> >>>>>>     * warn when solution touches trusted computing base (rules you
> >>>>>> should not be
> >>>>>>       adding)
> >>>>>>     * suggest alternate labels for content, example: httpd_t not
> >>>>>> allowed to write
> >>>>>>       to user_home_t, might suggest that changing the label to
> >>>>>>       httpd_user_content_t
> >>>>> This seems more along the lines of what setroubleshoot does, and tends
> >>>>> to be policy-specific. We need to preserve the usability of audit2allow
> >>>>> for other policies (e.g. Android, DSSP), so any policy-specific logic
> >>>>> needs to be encapsulated, configuration-driven, and optional.
> >>>> setroubleshoot is not smart enough to suggest httpd_user_content_t,
> >>>> instead it suggest you allow full access to /home/* by toggling the
> >>>> boolean that grants access to user home content.
> >>>>
> >>>> I know this from experience. There was a bug in fedora's
> >>>> apache_user_content_template for more than a year and hardly anyone
> >>>> noticed it, probably because setroubleshoot would have just suggested
> >>>> you allow full access to user home content.
> >>> https://bugzilla.redhat.com/show_bug.cgi?id=1457406
> >> I actually think the suggest a accessible type is  the best improvement.
> >> audit2allow has defaulted for years to suggesting adding allow rules,
> >> usually the worst solution.  Later we added boolean support, which was
> >> better but as you point out the not a great solution in a lot of cases.
> > The "best" improvement. I agree, but that say's more about the overal state
> > of things than about the quality of this particular improvement.
> >
> > The guy in the tweet was overwhelmed and intimidated by the large list of
> > "accessible" types returned by setroubleshoot (and audit2allow would
> > probably do the same)
> > Also things arent as simple, its not just about "what type grants the
> > access needed to my domain". Other domains access to that type should also
> > be considered.
> Which is why I suggested we filter it down to a list that has the same
> prefix.  Also I think we should at least stick in the beginning to
> permissions that can effect the content of data, rather then just reveal
> the content.
> 
> write, append, add_name unlink setattr, remove_name rmdir link create ...
> 
> I think anything we can do to making it easier for the user to get a
> proper type, would be really helpful.
> >> Most AVC's are caused by mislabeled files or processes.  Having
> >> audit2allow
> >> attempt to examine the policy and figure out which types would have been
> >> allowed with some simple sorting rules might be a big improvement.
> > audit2allow INHO shouldnt "examine" the policy (at least not in any
> > significant way) because it cannot do it right. audit2allow should just
> > stick with the basics + audit2why.
> >
> > audit2why is the only functionality that works reasonably well. provided
> > that some of the more recent "improvements" to audit2why get removed.
> audit2why does nothing to reveal the crutch of the problem, SELinux is a
> Labeling system, if things go wrong, most of the time the labels are
> wrong.  audit2allow/audit2why does not consider this. THis is why you
> get a boolean suggestion as the first outcome.
> 
> I believe the checks should be
> 
> 1. Is the target mislabeled?  If yes, suggest restorecon, ( I believe it
> already does this)
> 2. If this is a modification permission, suggest a type if one exists
> that matches the the prefix of the source type.  This helps a lot with
> people using default_t types, or types labeled var_log_t or var_lib_t.
> Somewhat with user_home_t.
> 3. Look for a boolean to satisfy the needs.
> 4. Suggest audit2allow -M mypol

audit2allow along with semodule including a set priority like sealert does 

# audit2allow -M mypol
# semodule -X 300 -i mypol.pp

> 
> There are probably a couple of other things that could happen.
> 
> 
> >> Avc denying httpd_t ability to write to httpd_sys_content_t.
> >>
> >> audit2allow should examine policy and figure out which domains httpd_t can
> >> write to with the current policy settings.  Then it should sort the
> >> solutions based on the characters of the sources context.  Perhaps
> >> chopping
> >> a characters off, maybe down to 3.
> >>
> >>   sesearch -A -s httpd_t -p write -c | cut -f1 -d ":"  | cut -d" " -f 3 |
> >> sort -u | grep httpd
> >> httpd_cache_t
> >> httpdcontent
> >> httpd_lock_t
> >> httpd_squirrelmail_t
> >> httpd_sys_rw_content_t
> >> httpd_t
> >> httpd_tmpfs_t
> >> httpd_tmp_t
> >> httpd_user_rw_content_t
> >> httpd_var_lib_t
> >> httpd_var_run_t
> >>
> >> Now eliminate the non file types, removes httpd_t.
> >>
> >> That would give the user a much better chance of solving the problem
> >> without
> >> becoming policy specific.
> > It cannot do this right. If you look to be bugzilla i pasted above:
> >
> > All that was needed was to allow the httpd_t domain access to read dirs
> > with the httpd_sys_content_t type. sealert couldnt figure this out
> > and instead it suggested that he toggle the "httpd_use_user_content"
> > boolean instead, which would grant httpd_t broad access to user home
> > content, and defeat the purpose of "httpd_user_content_t"
> >
> >> Now if you wanted to become a little more smart, you could look at the
> >> second field of the label, and have a table that says "var" indicates the
> >> path begins with /var and "tmp" indicates a directory that ends with tmp.
> >> But users could probably determine this.
> >>
> >>
> >>
> >>
> >> Sort by types starting with httpd, http, htt
> > The audience that uses this functionality get intimidated by the large
> > lists of types that are returned and various "solutions" returned.
> > plus often times the solutions returned are sub-optimal, but the user
> > doesnt know this.
> >
> > Seasoned users will only use audit2allow for simple things like translation
> > of avc to raw rules (maybe resolving of interfaces but this is prone to
> > bad results as well). quickly determining whether its a missing TE rule,
> > boolean, or constraint. These users will use setools to do further
> > analysis
> >
> 
> 

-- 
Simon Sekidde
gpg: 5848 958E 73BA 04D3 7C06 F096 1BA1 2DBF 94BC 377E





[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux