[PATCH v15 18/23] NET: Store LSM netlabel data in a lsmblob

[Casey Schaufler]

On 3/6/2020 6:14 PM, Paul Moore wrote:
> On Fri, Feb 14, 2020 at 6:45 PM Casey Schaufler <casey at schaufler-ca.com> wrote:
>> Netlabel uses LSM interfaces requiring an lsmblob and
>> the internal storage is used to pass information between
>> these interfaces, so change the internal data from a secid
>> to a lsmblob. Update the netlabel interfaces and their
>> callers to accommodate the change. This requires that the
>> modules using netlabel use the lsm_id.slot to access the
>> correct secid when using netlabel.
>>
>> Reviewed-by: Kees Cook <keescook at chromium.org>
>> Reviewed-by: John Johansen <john.johansen at canonical.com>
>> Acked-by: Stephen Smalley <sds at tycho.nsa.gov>
>> Signed-off-by: Casey Schaufler <casey at schaufler-ca.com>
>> ---
>>  include/net/netlabel.h              |  8 ++--
>>  net/ipv4/cipso_ipv4.c               | 23 +++++++-----
>>  net/netlabel/netlabel_kapi.c        |  6 +--
>>  net/netlabel/netlabel_unlabeled.c   | 57 +++++++++++------------------
>>  net/netlabel/netlabel_unlabeled.h   |  2 +-
>>  security/selinux/hooks.c            |  2 +-
>>  security/selinux/include/security.h |  1 +
>>  security/selinux/netlabel.c         |  2 +-
>>  security/selinux/ss/services.c      |  4 +-
>>  security/smack/smack.h              |  1 +
>>  security/smack/smack_lsm.c          |  5 ++-
>>  security/smack/smackfs.c            | 10 +++--
>>  12 files changed, 59 insertions(+), 62 deletions(-)
> ...
>
>
>> diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
>> index 376882215919..adb9dffc3952 100644
>> --- a/net/ipv4/cipso_ipv4.c
>> +++ b/net/ipv4/cipso_ipv4.c
>> @@ -106,15 +106,17 @@ int cipso_v4_rbm_strictvalid = 1;
>>  /* Base length of the local tag (non-standard tag).
>>   *  Tag definition (may change between kernel versions)
>>   *
>> - * 0          8          16         24         32
>> - * +----------+----------+----------+----------+
>> - * | 10000000 | 00000110 | 32-bit secid value  |
>> - * +----------+----------+----------+----------+
>> - * | in (host byte order)|
>> - * +----------+----------+
>> - *
>> + * 0          8          16                    16 + sizeof(struct lsmblob)
>> + * +----------+----------+---------------------+
>> + * | 10000000 | 00000110 | LSM blob data       |
>> + * +----------+----------+---------------------+
>> + *
>> + * All secid and flag fields are in host byte order.
>> + * The lsmblob structure size varies depending on which
>> + * Linux security modules are built in the kernel.
>> + * The data is opaque.
>>   */
>> -#define CIPSO_V4_TAG_LOC_BLEN         6
>> +#define CIPSO_V4_TAG_LOC_BLEN         (2 + sizeof(struct lsmblob))
> This isn't as bad as the sk_buff.cb limitation so I'm not going to
> worry too much about it, but just to be safe I think we should put a
> compile-time check to ensure that the local tag is within the bounds
> of the IPv4 option limit.  If we don't put a check I worry that there
> is a chance someone could get a very rude surprise at some point in
> the future (yes, this is highly unlikely, but still possible).

I hadn't thought about it, but that's a splendid idea.


>>  /*
>>   * Helper Functions
>> @@ -1467,7 +1469,8 @@ static int cipso_v4_gentag_loc(const struct cipso_v4_doi *doi_def,
>>
>>         buffer[0] = CIPSO_V4_TAG_LOCAL;
>>         buffer[1] = CIPSO_V4_TAG_LOC_BLEN;
>> -       *(u32 *)&buffer[2] = secattr->attr.secid;
>> +       memcpy(&buffer[2], &secattr->attr.lsmblob,
>> +              sizeof(secattr->attr.lsmblob));
>>
>>         return CIPSO_V4_TAG_LOC_BLEN;
>>  }