On Fri, Sep 4, 2020 at 5:35 PM Casey Schaufler <casey(a)schaufler-ca.com> wrote:
On 9/4/2020 1:08 PM, Paul Moore wrote:
> On Wed, Aug 26, 2020 at 11:07 AM Casey Schaufler <casey(a)schaufler-ca.com>
wrote:
>> Change the data used in UDS SO_PEERSEC processing from a
>> secid to a more general struct lsmblob. Update the
>> security_socket_getpeersec_dgram() interface to use the
>> lsmblob. There is a small amount of scaffolding code
>> that will come out when the security_secid_to_secctx()
>> code is brought in line with the lsmblob.
>>
>> Signed-off-by: Casey Schaufler <casey(a)schaufler-ca.com>
>> ---
>> include/linux/security.h | 7 +++++--
>> include/net/af_unix.h | 2 +-
>> include/net/scm.h | 8 +++++---
>> net/ipv4/ip_sockglue.c | 8 +++++---
>> net/unix/af_unix.c | 6 +++---
>> security/security.c | 18 +++++++++++++++---
>> 6 files changed, 34 insertions(+), 15 deletions(-)
> ...
>
>> diff --git a/include/net/af_unix.h b/include/net/af_unix.h
>> index f42fdddecd41..a86da0cb5ec1 100644
>> --- a/include/net/af_unix.h
>> +++ b/include/net/af_unix.h
>> @@ -36,7 +36,7 @@ struct unix_skb_parms {
>> kgid_t gid;
>> struct scm_fp_list *fp; /* Passed files */
>> #ifdef CONFIG_SECURITY_NETWORK
>> - u32 secid; /* Security ID */
>> + struct lsmblob lsmblob; /* Security LSM data */
> As mentioned in a previous revision, I remain concerned that this is
> going to become a problem due to the size limit on unix_skb_parms. I
> would need to redo the math to be certain, but if I recall correctly
> this would limit us to five LSMs assuming both that we don't need to
> grow the per-LSM size of lsmblob *and* the netdev folks don't decide
> to add more fields to the unix_skb_parms.
>
> I lost track of that earlier discussion so I'm not sure where it ended
> up, but if there is a viable alternative it might be a good idea to
> pursue it.
Stephen had concerns about the lifecycle management involved. He also
pointed out that I had taken a cowards way out when allocations failed.
That could result in unexpected behavior when an allocation failed.
Fixing that would have required a major re-write of the currently simple
UDS attribute code, which I suspect would be as hard a sell to netdev as
expanding the secid to a lsmblob. I also thought I'd gotten netdev on the
CC: for this patch, but it looks like I missed it.
I did start on the UDS attribute re-do, and discovered that I was going
to have to introduce new failure paths, and that it might not be possible
to maintain compatibility for all cases because of the new possibilities
of failure.
... and you're hoping to not be responsible for this code by the time
this becomes a limiting issue? ;)
I understand the concerns you mention, they are all valid as far as
I'm concerned, but I think we are going to get burned by this code as
it currently stands.
--
paul moore
www.paul-moore.com