Re: [PATCH] Module for tpm2-abrmd

[Chris PeBenito <pebenito@ieee.org>]

On 7/29/19 9:48 PM, Sugar, David wrote:
> 
> 
> On 7/29/19 8:49 PM, Chris PeBenito wrote:
>> On 7/25/19 10:43 AM, Sugar, David wrote:
>>> Signed-off-by: Dave Sugar <dsugar@tresys.com>
>>> ---
>>>    policy/modules/services/tpm2_abrmd.fc |  3 ++
>>>    policy/modules/services/tpm2_abrmd.if | 58 +++++++++++++++++++++++++++
>>>    policy/modules/services/tpm2_abrmd.te | 29 ++++++++++++++
>>>    3 files changed, 90 insertions(+)
>>>    create mode 100644 policy/modules/services/tpm2_abrmd.fc
>>>    create mode 100644 policy/modules/services/tpm2_abrmd.if
>>>    create mode 100644 policy/modules/services/tpm2_abrmd.te
>>
>> I think my only question is about the module name.  Why not tpm2?  I'm
>> not well versed on the Linux TPM stack, but isn't there other components
>> that potentially could need policy, like tss or totp?
>>
>>
> I used this name because the only SELinux policy needed was for the
> daemon 'tpm2-abrmd'.  The processes that interact with the daemon to
> actually use the tpm are using default bin_t label and don't seem to
> need any additional permissions.  In my case I'm using /bin/tpm2_* and
> /bin/clevis.
> 
> I see a tss package but it is only adding libraries.  I don't see a
> package for totp.  I'm working on RHEL7.6 so there could be additional
> binaries on other distributions that will need labeling.
> 
> I'm happy to go either way here.  I don't have a strong feeling if other
> tpm2 related stuff should get grouped into a single module or if they
> should be separate.
> 
> I'm pretty sure anything using a tpm2 will need to use abrmd to access
> the TPM.  I just don't have enough insight to know if those other
> binaries will need labeling and policy.
> 
> If the decision is made to use tpm2.{fc,if,te} would any of the policy
> change?  I feel like the fc and te files are still correct.  Interface
> names might be changed slightly, but also might be OK as is.

I think we should go with just tpm.*.  Renaming modules is painful for 
backwards compat.  The interface names would change, e.g. 
tpm2_enabledisable_abrmd, otherwise it seems ok.


> Dave
> 
>>
>>> diff --git a/policy/modules/services/tpm2_abrmd.fc
>>> b/policy/modules/services/tpm2_abrmd.fc
>>> new file mode 100644
>>> index 00000000..4ccf2f25
>>> --- /dev/null
>>> +++ b/policy/modules/services/tpm2_abrmd.fc
>>> @@ -0,0 +1,3 @@
>>> +/usr/sbin/tpm2-abrmd                                --
>>> gen_context(system_u:object_r:tpm2_abrmd_exec_t,s0)
>>> +
>>> +/usr/lib/systemd/system/[^/]*tpm2-abrmd\.service    --
>>> gen_context(system_u:object_r:tpm2_abrmd_unit_t,s0)
>>> diff --git a/policy/modules/services/tpm2_abrmd.if
>>> b/policy/modules/services/tpm2_abrmd.if
>>> new file mode 100644
>>> index 00000000..dabb4a65
>>> --- /dev/null
>>> +++ b/policy/modules/services/tpm2_abrmd.if
>>> @@ -0,0 +1,58 @@
>>> +## <summary>TPM2 Access Broker and Resource Management daemon.</summary>
>>> +
>>> +########################################
>>> +## <summary>
>>> +##    Allow specified domain to enable/disable tpm2-abrmd unit
>>> +## </summary>
>>> +## <param name="domain">
>>> +##    <summary>
>>> +##    Domain allowed access.
>>> +##    </summary>
>>> +## </param>
>>> +#
>>> +interface(`tpm2_abrmd_enabledisable',`
>>> +    gen_require(`
>>> +        type tpm2_abrmd_unit_t;
>>> +        class service { enable disable };
>>> +    ')
>>> +
>>> +    allow $1 tpm2_abrmd_unit_t:service { enable disable };
>>> +')
>>> +
>>> +########################################
>>> +## <summary>
>>> +##    Allow specified domain to start/stop tpm2-abrmd unit
>>> +## </summary>
>>> +## <param name="domain">
>>> +##    <summary>
>>> +##    Domain allowed access.
>>> +##    </summary>
>>> +## </param>
>>> +#
>>> +interface(`tpm2_abrmd_startstop',`
>>> +    gen_require(`
>>> +        type tpm2_abrmd_unit_t;
>>> +        class service { start stop };
>>> +    ')
>>> +
>>> +    allow $1 tpm2_abrmd_unit_t:service { start stop };
>>> +')
>>> +
>>> +########################################
>>> +## <summary>
>>> +##    Allow specified domain to get status of tpm2-abrmd unit
>>> +## </summary>
>>> +## <param name="domain">
>>> +##    <summary>
>>> +##    Domain allowed access.
>>> +##    </summary>
>>> +## </param>
>>> +#
>>> +interface(`tpm2_abrmd_status',`
>>> +    gen_require(`
>>> +        type tpm2_abrmd_unit_t;
>>> +        class service status;
>>> +    ')
>>> +
>>> +    allow $1 tpm2_abrmd_unit_t:service status;
>>> +')
>>> diff --git a/policy/modules/services/tpm2_abrmd.te
>>> b/policy/modules/services/tpm2_abrmd.te
>>> new file mode 100644
>>> index 00000000..8a8ba5f0
>>> --- /dev/null
>>> +++ b/policy/modules/services/tpm2_abrmd.te
>>> @@ -0,0 +1,29 @@
>>> +policy_module(tpm2_abrmd, 1.0.0)
>>> +
>>> +########################################
>>> +#
>>> +# Declarations
>>> +#
>>> +
>>> +type tpm2_abrmd_t;
>>> +type tpm2_abrmd_exec_t;
>>> +init_daemon_domain(tpm2_abrmd_t, tpm2_abrmd_exec_t)
>>> +
>>> +type tpm2_abrmd_unit_t;
>>> +init_unit_file(tpm2_abrmd_unit_t)
>>> +
>>> +########################################
>>> +#
>>> +# Local policy
>>> +#
>>> +
>>> +allow tpm2_abrmd_t self:process signal;
>>> +allow tpm2_abrmd_t self:unix_stream_socket create_socket_perms;
>>> +
>>> +dev_rw_tpm(tpm2_abrmd_t)
>>> +
>>> +kernel_read_crypto_sysctls(tpm2_abrmd_t)
>>> +kernel_read_system_state(tpm2_abrmd_t)
>>> +
>>> +dbus_system_bus_client(tpm2_abrmd_t)
>>> +dbus_connect_system_bus(tpm2_abrmd_t)
>>>
>>
>>


-- 
Chris PeBenito