On Fri, Feb 14, 2020 at 6:45 PM Casey Schaufler <casey@xxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxx> > Reviewed-by: John Johansen <john.johansen@xxxxxxxxxxxxx> > Acked-by: Stephen Smalley <sds@xxxxxxxxxxxxx> > Signed-off-by: Casey Schaufler <casey@xxxxxxxxxxxxxxxx> > --- > 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). > /* > * 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; > } -- paul moore www.paul-moore.com
