On 02/28/2018 03:53 PM, Stephen Smalley wrote:
> On 02/28/2018 02:44 PM, William Roberts wrote:
>> On Wed, Feb 28, 2018 at 11:39 AM, Stephen Smalley <sds@xxxxxxxxxxxxx> wrote:
>>> On 02/28/2018 02:26 PM, William Roberts wrote:
>>>> So peeking through the code base, I see:
>>>>
>>>> int semanage_direct_is_managed(semanage_handle_t * sh)
>>>> {
>>>> if (semanage_check_init(sh, sh->conf->store_root_path))
>>>> goto err;
>>>>
>>>> if (semanage_access_check(sh) < 0)
>>>> return 0;
>>>>
>>>> return 1;
>>>>
>>>> err:
>>>> ERR(sh, "could not check whether policy is managed");
>>>> return STATUS_ERR;
>>>> }
>>>>
>>>> Which semanage_access_check eventually gets down to a raw access check.
>>>>
>>>> Which is only ever used in test_fcontext
>>>>
>>>> semanage_access_check is also the only consumer of semanage_direct_access_check
>>>>
>>>> which is the semanage_store_access_check is only consumed by the
>>>> former and test case and
>>>> the same could be said for semanage_store_access_check
>>>>
>>>> I think this is a good time to roll in patch 4 and drop everything
>>>> relying on semanage_store_access_check.
>>>>
>>>> Thoughts?
>>>
>>> semanage_access_check() is part of the shared library ABI. Can't be
>>> removed. Used by seobject.py via the python bindings. At most, we can
>>> kill all internal users but the ABI has to remain.
>>
>> Ahh yes, duh.
>>
>> Outside of just killing off internal users of it, should we modify it
>> to not use access?
>> So it at least works under setuid?
>
> I don't think it is worthwhile, and it isn't clear how one would test
> the directory writability case. Plus semanage_access_check() is only
> advisory and the caller is free to ignore the result or not call it in
> the first place. It is really only intended to allow a program to save
> itself some work if it isn't going to be able to access the policy store
> at all, not as a security feature. Also, as I've said before, I
> wouldn't warrant libsemanage to be safe if called from a setuid program;
> no one has ever audited it for such to my knowledge.
By the way, please test that these patches do not end up yielding silent
failures or confusing error messages when users run semanage or semodule
commands with insufficient permissions, e.g. semanage login -l or
semodule -l as non-root. That's what semanage_access_check() was for,
to check up front and provide a useful error message before doing any
other work.
