| Message ID | 20191213202838.7323-1-sds@tycho.nsa.gov (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [RFC] selinux: randomize layout of key structures | expand |
On Fri, Dec 13, 2019 at 03:28:38PM -0500, Stephen Smalley wrote: > I would have expected that two kernels built with the same config > with this enabled would have yielded different struct layouts in > pahole vmlinux output, but that doesn't appear to be the case. They > do have different seeds. Am I doing something wrong? > Also, does DEBUG_INFO_BTF effectively undermine/negate the benefits of this > change if enabled? There's currently a long-standing bug with the GCC plugin where the generated debug info is in declaration order, not build order (see: [1]). So, to verify it, try looking at the generated machine code. Planned Clang implementation of this feature does not have this bug. [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?format=multiple&id=84052
On 12/14/19 1:50 PM, Dan Aloni wrote: > On Fri, Dec 13, 2019 at 03:28:38PM -0500, Stephen Smalley wrote: >> I would have expected that two kernels built with the same config >> with this enabled would have yielded different struct layouts in >> pahole vmlinux output, but that doesn't appear to be the case. They >> do have different seeds. Am I doing something wrong? >> Also, does DEBUG_INFO_BTF effectively undermine/negate the benefits of this >> change if enabled? > > There's currently a long-standing bug with the GCC plugin where the > generated debug info is in declaration order, not build order (see: > [1]). So, to verify it, try looking at the generated machine code. Thanks for that clarification; I can see in the code that the struct layout has changed between the two kernel builds. > > Planned Clang implementation of this feature does not have this bug. > > [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?format=multiple&id=84052
On Mon, Dec 16, 2019 at 9:21 AM Stephen Smalley <sds@tycho.nsa.gov> wrote: > On 12/14/19 1:50 PM, Dan Aloni wrote: > > On Fri, Dec 13, 2019 at 03:28:38PM -0500, Stephen Smalley wrote: > >> I would have expected that two kernels built with the same config > >> with this enabled would have yielded different struct layouts in > >> pahole vmlinux output, but that doesn't appear to be the case. They > >> do have different seeds. Am I doing something wrong? > >> Also, does DEBUG_INFO_BTF effectively undermine/negate the benefits of this > >> change if enabled? > > > > There's currently a long-standing bug with the GCC plugin where the > > generated debug info is in declaration order, not build order (see: > > [1]). So, to verify it, try looking at the generated machine code. > > Thanks for that clarification; I can see in the code that the struct > layout has changed between the two kernel builds. This likely falls under the category of stupid questions, but I'm assuming it passed the test suite w/o problems and the system generally ran as expected? I've also heard some comments about performance concerns, have you done any testing? I'm guessing that isn't a major concern here because I don't recall any of the structs marked in this patch going through any optimizations, but I could be forgetting something (or missing a performance concern with RANDSTRUCT).
On 12/16/19 6:46 PM, Paul Moore wrote: > On Mon, Dec 16, 2019 at 9:21 AM Stephen Smalley <sds@tycho.nsa.gov> wrote: >> On 12/14/19 1:50 PM, Dan Aloni wrote: >>> On Fri, Dec 13, 2019 at 03:28:38PM -0500, Stephen Smalley wrote: >>>> I would have expected that two kernels built with the same config >>>> with this enabled would have yielded different struct layouts in >>>> pahole vmlinux output, but that doesn't appear to be the case. They >>>> do have different seeds. Am I doing something wrong? >>>> Also, does DEBUG_INFO_BTF effectively undermine/negate the benefits of this >>>> change if enabled? >>> >>> There's currently a long-standing bug with the GCC plugin where the >>> generated debug info is in declaration order, not build order (see: >>> [1]). So, to verify it, try looking at the generated machine code. >> >> Thanks for that clarification; I can see in the code that the struct >> layout has changed between the two kernel builds. > > This likely falls under the category of stupid questions, but I'm > assuming it passed the test suite w/o problems and the system > generally ran as expected? Yes, it tested fine for me. It did require a full rebuild to ensure that the randomized struct layouts were being used consistently throughout, and requires a make mrproper or distclean to remove the random seed before rebuilding if you want to generate a new random seed. > I've also heard some comments about performance concerns, have you > done any testing? I'm guessing that isn't a major concern here > because I don't recall any of the structs marked in this patch going > through any optimizations, but I could be forgetting something (or > missing a performance concern with RANDSTRUCT). I haven't done any specific performance testing, but it will only impact users who enable RANDSTRUCT, so it is entirely opt-in, and as you say, these structs have not been especially optimized in the first place. I think these same structures (or at least significant subsets thereof) are good candidates for write-rare protections if those ever reach mainline. Haven't seen any progress there in a while.
On Fri, Dec 13, 2019 at 03:28:38PM -0500, Stephen Smalley wrote: > Randomize the layout of key selinux data structures. > Initially this is applied to the selinux_state, selinux_ss, > policydb, and task_security_struct data structures. > > NB To test/use this mechanism, one must install the > necessary build-time dependencies, e.g. gcc-plugin-devel on Fedora, > and enable CONFIG_GCC_PLUGIN_RANDSTRUCT in the kernel configuration. > > Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> Reviewed-by: Kees Cook <keescook@chromium.org> -Kees > --- > I would have expected that two kernels built with the same config > with this enabled would have yielded different struct layouts in > pahole vmlinux output, but that doesn't appear to be the case. They > do have different seeds. Am I doing something wrong? > Also, does DEBUG_INFO_BTF effectively undermine/negate the benefits of this > change if enabled? > > security/selinux/include/objsec.h | 2 +- > security/selinux/include/security.h | 2 +- > security/selinux/ss/policydb.h | 2 +- > security/selinux/ss/services.h | 2 +- > 4 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h > index a4a86cbcfb0a..330b7b6d44e0 100644 > --- a/security/selinux/include/objsec.h > +++ b/security/selinux/include/objsec.h > @@ -35,7 +35,7 @@ struct task_security_struct { > u32 create_sid; /* fscreate SID */ > u32 keycreate_sid; /* keycreate SID */ > u32 sockcreate_sid; /* fscreate SID */ > -}; > +} __randomize_layout; > > enum label_initialized { > LABEL_INVALID, /* invalid or not initialized */ > diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h > index 49737087ad33..3ea406ad91b6 100644 > --- a/security/selinux/include/security.h > +++ b/security/selinux/include/security.h > @@ -110,7 +110,7 @@ struct selinux_state { > bool policycap[__POLICYDB_CAPABILITY_MAX]; > struct selinux_avc *avc; > struct selinux_ss *ss; > -}; > +} __randomize_layout; > > void selinux_ss_init(struct selinux_ss **ss); > void selinux_avc_init(struct selinux_avc **avc); > diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h > index bc56b14e2216..98afe52a3d19 100644 > --- a/security/selinux/ss/policydb.h > +++ b/security/selinux/ss/policydb.h > @@ -307,7 +307,7 @@ struct policydb { > > u16 process_class; > u32 process_trans_perms; > -}; > +} __randomize_layout;; > > extern void policydb_destroy(struct policydb *p); > extern int policydb_load_isids(struct policydb *p, struct sidtab *s); > diff --git a/security/selinux/ss/services.h b/security/selinux/ss/services.h > index fc40640a9725..c5896f39e8f6 100644 > --- a/security/selinux/ss/services.h > +++ b/security/selinux/ss/services.h > @@ -31,7 +31,7 @@ struct selinux_ss { > struct selinux_map map; > struct page *status_page; > struct mutex status_lock; > -}; > +} __randomize_layout; > > void services_compute_xperms_drivers(struct extended_perms *xperms, > struct avtab_node *node); > -- > 2.23.0 >
On Tue, Dec 17, 2019 at 07:12:28PM -0500, Wenhui Zhang wrote: > Hi, Sorry for bothering you again. > > However I came into some *evm code* in LSM, which calculates HMAC for data > structures. > It looks like HMAC could protect integrity of LSM data structures. > > IMHO, __randomize_layout and evm might duplicate the work, any instructions > on this please? When does the EVM code perform the checking? I would assume these are complimentary features rather than duplicate. -Kees > > > On Tue, Dec 17, 2019 at 6:50 PM Kees Cook <keescook@chromium.org> wrote: > > > On Fri, Dec 13, 2019 at 03:28:38PM -0500, Stephen Smalley wrote: > > > Randomize the layout of key selinux data structures. > > > Initially this is applied to the selinux_state, selinux_ss, > > > policydb, and task_security_struct data structures. > > > > > > NB To test/use this mechanism, one must install the > > > necessary build-time dependencies, e.g. gcc-plugin-devel on Fedora, > > > and enable CONFIG_GCC_PLUGIN_RANDSTRUCT in the kernel configuration. > > > > > > Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> > > > > Reviewed-by: Kees Cook <keescook@chromium.org> > > > > -Kees > > > > > --- > > > I would have expected that two kernels built with the same config > > > with this enabled would have yielded different struct layouts in > > > pahole vmlinux output, but that doesn't appear to be the case. They > > > do have different seeds. Am I doing something wrong? > > > Also, does DEBUG_INFO_BTF effectively undermine/negate the benefits of > > this > > > change if enabled? > > > > > > security/selinux/include/objsec.h | 2 +- > > > security/selinux/include/security.h | 2 +- > > > security/selinux/ss/policydb.h | 2 +- > > > security/selinux/ss/services.h | 2 +- > > > 4 files changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git a/security/selinux/include/objsec.h > > b/security/selinux/include/objsec.h > > > index a4a86cbcfb0a..330b7b6d44e0 100644 > > > --- a/security/selinux/include/objsec.h > > > +++ b/security/selinux/include/objsec.h > > > @@ -35,7 +35,7 @@ struct task_security_struct { > > > u32 create_sid; /* fscreate SID */ > > > u32 keycreate_sid; /* keycreate SID */ > > > u32 sockcreate_sid; /* fscreate SID */ > > > -}; > > > +} __randomize_layout; > > > > > > enum label_initialized { > > > LABEL_INVALID, /* invalid or not initialized */ > > > diff --git a/security/selinux/include/security.h > > b/security/selinux/include/security.h > > > index 49737087ad33..3ea406ad91b6 100644 > > > --- a/security/selinux/include/security.h > > > +++ b/security/selinux/include/security.h > > > @@ -110,7 +110,7 @@ struct selinux_state { > > > bool policycap[__POLICYDB_CAPABILITY_MAX]; > > > struct selinux_avc *avc; > > > struct selinux_ss *ss; > > > -}; > > > +} __randomize_layout; > > > > > > void selinux_ss_init(struct selinux_ss **ss); > > > void selinux_avc_init(struct selinux_avc **avc); > > > diff --git a/security/selinux/ss/policydb.h > > b/security/selinux/ss/policydb.h > > > index bc56b14e2216..98afe52a3d19 100644 > > > --- a/security/selinux/ss/policydb.h > > > +++ b/security/selinux/ss/policydb.h > > > @@ -307,7 +307,7 @@ struct policydb { > > > > > > u16 process_class; > > > u32 process_trans_perms; > > > -}; > > > +} __randomize_layout;; > > > > > > extern void policydb_destroy(struct policydb *p); > > > extern int policydb_load_isids(struct policydb *p, struct sidtab *s); > > > diff --git a/security/selinux/ss/services.h > > b/security/selinux/ss/services.h > > > index fc40640a9725..c5896f39e8f6 100644 > > > --- a/security/selinux/ss/services.h > > > +++ b/security/selinux/ss/services.h > > > @@ -31,7 +31,7 @@ struct selinux_ss { > > > struct selinux_map map; > > > struct page *status_page; > > > struct mutex status_lock; > > > -}; > > > +} __randomize_layout; > > > > > > void services_compute_xperms_drivers(struct extended_perms *xperms, > > > struct avtab_node *node); > > > -- > > > 2.23.0 > > > > > > > -- > > Kees Cook > > > > > -- > V/R, > > Wenhui Zhang > > Email: wenhui@gwmail.gwu.edu > Telephone: 1-(703) 424 3193
On Tue, Dec 17, 2019 at 07:21:49PM -0500, Wenhui Zhang wrote: > In my understanding, evm is called whenever a security related attribute is > committed to VFS. > It is set as enabled by default. > (ref. > https://github.com/torvalds/linux/blob/8b68150883ca466a23e90902dd4113b22e692f04/security/integrity/evm/evm_main.c > ) Okay, so yes, complimentary. randstruct means attackers need use bugs to expose enough memory from the kernel to determine the order of structure members before they can manipulate them with a kernel bug. -Kees > > On Tue, Dec 17, 2019 at 7:16 PM Kees Cook <keescook@chromium.org> wrote: > > > On Tue, Dec 17, 2019 at 07:12:28PM -0500, Wenhui Zhang wrote: > > > Hi, Sorry for bothering you again. > > > > > > However I came into some *evm code* in LSM, which calculates HMAC for > > data > > > structures. > > > It looks like HMAC could protect integrity of LSM data structures. > > > > > > IMHO, __randomize_layout and evm might duplicate the work, any > > instructions > > > on this please? > > > > When does the EVM code perform the checking? I would assume these are > > complimentary features rather than duplicate. > > > > -Kees > > > > > > > > > > > On Tue, Dec 17, 2019 at 6:50 PM Kees Cook <keescook@chromium.org> wrote: > > > > > > > On Fri, Dec 13, 2019 at 03:28:38PM -0500, Stephen Smalley wrote: > > > > > Randomize the layout of key selinux data structures. > > > > > Initially this is applied to the selinux_state, selinux_ss, > > > > > policydb, and task_security_struct data structures. > > > > > > > > > > NB To test/use this mechanism, one must install the > > > > > necessary build-time dependencies, e.g. gcc-plugin-devel on Fedora, > > > > > and enable CONFIG_GCC_PLUGIN_RANDSTRUCT in the kernel configuration. > > > > > > > > > > Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> > > > > > > > > Reviewed-by: Kees Cook <keescook@chromium.org> > > > > > > > > -Kees > > > > > > > > > --- > > > > > I would have expected that two kernels built with the same config > > > > > with this enabled would have yielded different struct layouts in > > > > > pahole vmlinux output, but that doesn't appear to be the case. They > > > > > do have different seeds. Am I doing something wrong? > > > > > Also, does DEBUG_INFO_BTF effectively undermine/negate the benefits > > of > > > > this > > > > > change if enabled? > > > > > > > > > > security/selinux/include/objsec.h | 2 +- > > > > > security/selinux/include/security.h | 2 +- > > > > > security/selinux/ss/policydb.h | 2 +- > > > > > security/selinux/ss/services.h | 2 +- > > > > > 4 files changed, 4 insertions(+), 4 deletions(-) > > > > > > > > > > diff --git a/security/selinux/include/objsec.h > > > > b/security/selinux/include/objsec.h > > > > > index a4a86cbcfb0a..330b7b6d44e0 100644 > > > > > --- a/security/selinux/include/objsec.h > > > > > +++ b/security/selinux/include/objsec.h > > > > > @@ -35,7 +35,7 @@ struct task_security_struct { > > > > > u32 create_sid; /* fscreate SID */ > > > > > u32 keycreate_sid; /* keycreate SID */ > > > > > u32 sockcreate_sid; /* fscreate SID */ > > > > > -}; > > > > > +} __randomize_layout; > > > > > > > > > > enum label_initialized { > > > > > LABEL_INVALID, /* invalid or not initialized */ > > > > > diff --git a/security/selinux/include/security.h > > > > b/security/selinux/include/security.h > > > > > index 49737087ad33..3ea406ad91b6 100644 > > > > > --- a/security/selinux/include/security.h > > > > > +++ b/security/selinux/include/security.h > > > > > @@ -110,7 +110,7 @@ struct selinux_state { > > > > > bool policycap[__POLICYDB_CAPABILITY_MAX]; > > > > > struct selinux_avc *avc; > > > > > struct selinux_ss *ss; > > > > > -}; > > > > > +} __randomize_layout; > > > > > > > > > > void selinux_ss_init(struct selinux_ss **ss); > > > > > void selinux_avc_init(struct selinux_avc **avc); > > > > > diff --git a/security/selinux/ss/policydb.h > > > > b/security/selinux/ss/policydb.h > > > > > index bc56b14e2216..98afe52a3d19 100644 > > > > > --- a/security/selinux/ss/policydb.h > > > > > +++ b/security/selinux/ss/policydb.h > > > > > @@ -307,7 +307,7 @@ struct policydb { > > > > > > > > > > u16 process_class; > > > > > u32 process_trans_perms; > > > > > -}; > > > > > +} __randomize_layout;; > > > > > > > > > > extern void policydb_destroy(struct policydb *p); > > > > > extern int policydb_load_isids(struct policydb *p, struct sidtab > > *s); > > > > > diff --git a/security/selinux/ss/services.h > > > > b/security/selinux/ss/services.h > > > > > index fc40640a9725..c5896f39e8f6 100644 > > > > > --- a/security/selinux/ss/services.h > > > > > +++ b/security/selinux/ss/services.h > > > > > @@ -31,7 +31,7 @@ struct selinux_ss { > > > > > struct selinux_map map; > > > > > struct page *status_page; > > > > > struct mutex status_lock; > > > > > -}; > > > > > +} __randomize_layout; > > > > > > > > > > void services_compute_xperms_drivers(struct extended_perms *xperms, > > > > > struct avtab_node *node); > > > > > -- > > > > > 2.23.0 > > > > > > > > > > > > > -- > > > > Kees Cook > > > > > > > > > > > > > -- > > > V/R, > > > > > > Wenhui Zhang > > > > > > Email: wenhui@gwmail.gwu.edu > > > Telephone: 1-(703) 424 3193 > > > > -- > > Kees Cook > > > > > -- > V/R, > > Wenhui Zhang > > Email: wenhui@gwmail.gwu.edu > Telephone: 1-(703) 424 3193
On Tue, Dec 17, 2019 at 8:43 AM Stephen Smalley <sds@tycho.nsa.gov> wrote: > On 12/16/19 6:46 PM, Paul Moore wrote: > > On Mon, Dec 16, 2019 at 9:21 AM Stephen Smalley <sds@tycho.nsa.gov> wrote: > >> On 12/14/19 1:50 PM, Dan Aloni wrote: > >>> On Fri, Dec 13, 2019 at 03:28:38PM -0500, Stephen Smalley wrote: > >>>> I would have expected that two kernels built with the same config > >>>> with this enabled would have yielded different struct layouts in > >>>> pahole vmlinux output, but that doesn't appear to be the case. They > >>>> do have different seeds. Am I doing something wrong? > >>>> Also, does DEBUG_INFO_BTF effectively undermine/negate the benefits of this > >>>> change if enabled? > >>> > >>> There's currently a long-standing bug with the GCC plugin where the > >>> generated debug info is in declaration order, not build order (see: > >>> [1]). So, to verify it, try looking at the generated machine code. > >> > >> Thanks for that clarification; I can see in the code that the struct > >> layout has changed between the two kernel builds. > > > > This likely falls under the category of stupid questions, but I'm > > assuming it passed the test suite w/o problems and the system > > generally ran as expected? > > Yes, it tested fine for me. It did require a full rebuild to ensure that > the randomized struct layouts were being used consistently throughout, > and requires a make mrproper or distclean to remove the random seed > before rebuilding if you want to generate a new random seed. > > > I've also heard some comments about performance concerns, have you > > done any testing? I'm guessing that isn't a major concern here > > because I don't recall any of the structs marked in this patch going > > through any optimizations, but I could be forgetting something (or > > missing a performance concern with RANDSTRUCT). > > I haven't done any specific performance testing, but it will only impact > users who enable RANDSTRUCT, so it is entirely opt-in, and as you say, > these structs have not been especially optimized in the first place. > > I think these same structures (or at least significant subsets thereof) > are good candidates for write-rare protections if those ever reach > mainline. Haven't seen any progress there in a while. Okay, thanks. I think it's worth getting this now, even if there is some question about how widely they will be used. At least they will be there and we can use this as a starting point. Thanks for putting this patch together, but as a FYI, I did take the liberty to remove the double semi-colon during the merge ;)
On Tue, Dec 17, 2019 at 08:02:58PM -0500, Wenhui Zhang wrote: > EVM protects while security attributes data in memory and disk. > structrand protects security attributes data in memory. > > It seem like EVM introduces higher overhead than trampoline based > structrand in general. > > So, if we know our disks are reliable, does it make sense if we add some > ifdef options for disabling EVM please? > > However this really depends on which one is faster, the trampoline > function or the encryption hash function. There is no trampoline. It's just a per-compile re-ordering of the structures. It makes attack that depend on knowing structure layouts less reliable. EVM does not check these at memory access times, so they are complimentary. -Kees > > > On Tue, Dec 17, 2019 at 7:47 PM Kees Cook <keescook@chromium.org> wrote: > > > On Tue, Dec 17, 2019 at 07:21:49PM -0500, Wenhui Zhang wrote: > > > In my understanding, evm is called whenever a security related attribute > > is > > > committed to VFS. > > > It is set as enabled by default. > > > (ref. > > > > > https://github.com/torvalds/linux/blob/8b68150883ca466a23e90902dd4113b22e692f04/security/integrity/evm/evm_main.c > > > ) > > > > Okay, so yes, complimentary. randstruct means attackers need use bugs to > > expose enough memory from the kernel to determine the order of structure > > members before they can manipulate them with a kernel bug. > > > > -Kees > > > > > > > > On Tue, Dec 17, 2019 at 7:16 PM Kees Cook <keescook@chromium.org> wrote: > > > > > > > On Tue, Dec 17, 2019 at 07:12:28PM -0500, Wenhui Zhang wrote: > > > > > Hi, Sorry for bothering you again. > > > > > > > > > > However I came into some *evm code* in LSM, which calculates HMAC for > > > > data > > > > > structures. > > > > > It looks like HMAC could protect integrity of LSM data structures. > > > > > > > > > > IMHO, __randomize_layout and evm might duplicate the work, any > > > > instructions > > > > > on this please? > > > > > > > > When does the EVM code perform the checking? I would assume these are > > > > complimentary features rather than duplicate. > > > > > > > > -Kees > > > > > > > > > > > > > > > > > > > On Tue, Dec 17, 2019 at 6:50 PM Kees Cook <keescook@chromium.org> > > wrote: > > > > > > > > > > > On Fri, Dec 13, 2019 at 03:28:38PM -0500, Stephen Smalley wrote: > > > > > > > Randomize the layout of key selinux data structures. > > > > > > > Initially this is applied to the selinux_state, selinux_ss, > > > > > > > policydb, and task_security_struct data structures. > > > > > > > > > > > > > > NB To test/use this mechanism, one must install the > > > > > > > necessary build-time dependencies, e.g. gcc-plugin-devel on > > Fedora, > > > > > > > and enable CONFIG_GCC_PLUGIN_RANDSTRUCT in the kernel > > configuration. > > > > > > > > > > > > > > Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> > > > > > > > > > > > > Reviewed-by: Kees Cook <keescook@chromium.org> > > > > > > > > > > > > -Kees > > > > > > > > > > > > > --- > > > > > > > I would have expected that two kernels built with the same config > > > > > > > with this enabled would have yielded different struct layouts in > > > > > > > pahole vmlinux output, but that doesn't appear to be the case. > > They > > > > > > > do have different seeds. Am I doing something wrong? > > > > > > > Also, does DEBUG_INFO_BTF effectively undermine/negate the > > benefits > > > > of > > > > > > this > > > > > > > change if enabled? > > > > > > > > > > > > > > security/selinux/include/objsec.h | 2 +- > > > > > > > security/selinux/include/security.h | 2 +- > > > > > > > security/selinux/ss/policydb.h | 2 +- > > > > > > > security/selinux/ss/services.h | 2 +- > > > > > > > 4 files changed, 4 insertions(+), 4 deletions(-) > > > > > > > > > > > > > > diff --git a/security/selinux/include/objsec.h > > > > > > b/security/selinux/include/objsec.h > > > > > > > index a4a86cbcfb0a..330b7b6d44e0 100644 > > > > > > > --- a/security/selinux/include/objsec.h > > > > > > > +++ b/security/selinux/include/objsec.h > > > > > > > @@ -35,7 +35,7 @@ struct task_security_struct { > > > > > > > u32 create_sid; /* fscreate SID */ > > > > > > > u32 keycreate_sid; /* keycreate SID */ > > > > > > > u32 sockcreate_sid; /* fscreate SID */ > > > > > > > -}; > > > > > > > +} __randomize_layout; > > > > > > > > > > > > > > enum label_initialized { > > > > > > > LABEL_INVALID, /* invalid or not initialized */ > > > > > > > diff --git a/security/selinux/include/security.h > > > > > > b/security/selinux/include/security.h > > > > > > > index 49737087ad33..3ea406ad91b6 100644 > > > > > > > --- a/security/selinux/include/security.h > > > > > > > +++ b/security/selinux/include/security.h > > > > > > > @@ -110,7 +110,7 @@ struct selinux_state { > > > > > > > bool policycap[__POLICYDB_CAPABILITY_MAX]; > > > > > > > struct selinux_avc *avc; > > > > > > > struct selinux_ss *ss; > > > > > > > -}; > > > > > > > +} __randomize_layout; > > > > > > > > > > > > > > void selinux_ss_init(struct selinux_ss **ss); > > > > > > > void selinux_avc_init(struct selinux_avc **avc); > > > > > > > diff --git a/security/selinux/ss/policydb.h > > > > > > b/security/selinux/ss/policydb.h > > > > > > > index bc56b14e2216..98afe52a3d19 100644 > > > > > > > --- a/security/selinux/ss/policydb.h > > > > > > > +++ b/security/selinux/ss/policydb.h > > > > > > > @@ -307,7 +307,7 @@ struct policydb { > > > > > > > > > > > > > > u16 process_class; > > > > > > > u32 process_trans_perms; > > > > > > > -}; > > > > > > > +} __randomize_layout;; > > > > > > > > > > > > > > extern void policydb_destroy(struct policydb *p); > > > > > > > extern int policydb_load_isids(struct policydb *p, struct sidtab > > > > *s); > > > > > > > diff --git a/security/selinux/ss/services.h > > > > > > b/security/selinux/ss/services.h > > > > > > > index fc40640a9725..c5896f39e8f6 100644 > > > > > > > --- a/security/selinux/ss/services.h > > > > > > > +++ b/security/selinux/ss/services.h > > > > > > > @@ -31,7 +31,7 @@ struct selinux_ss { > > > > > > > struct selinux_map map; > > > > > > > struct page *status_page; > > > > > > > struct mutex status_lock; > > > > > > > -}; > > > > > > > +} __randomize_layout; > > > > > > > > > > > > > > void services_compute_xperms_drivers(struct extended_perms > > *xperms, > > > > > > > struct avtab_node *node); > > > > > > > -- > > > > > > > 2.23.0 > > > > > > > > > > > > > > > > > > > -- > > > > > > Kees Cook > > > > > > > > > > > > > > > > > > > > > -- > > > > > V/R, > > > > > > > > > > Wenhui Zhang > > > > > > > > > > Email: wenhui@gwmail.gwu.edu > > > > > Telephone: 1-(703) 424 3193 > > > > > > > > -- > > > > Kees Cook > > > > > > > > > > > > > -- > > > V/R, > > > > > > Wenhui Zhang > > > > > > Email: wenhui@gwmail.gwu.edu > > > Telephone: 1-(703) 424 3193 > > > > -- > > Kees Cook > > > > > -- > V/R, > > Wenhui Zhang > > Email: wenhui@gwmail.gwu.edu > Telephone: 1-(703) 424 3193
diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h index a4a86cbcfb0a..330b7b6d44e0 100644 --- a/security/selinux/include/objsec.h +++ b/security/selinux/include/objsec.h @@ -35,7 +35,7 @@ struct task_security_struct { u32 create_sid; /* fscreate SID */ u32 keycreate_sid; /* keycreate SID */ u32 sockcreate_sid; /* fscreate SID */ -}; +} __randomize_layout; enum label_initialized { LABEL_INVALID, /* invalid or not initialized */ diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index 49737087ad33..3ea406ad91b6 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -110,7 +110,7 @@ struct selinux_state { bool policycap[__POLICYDB_CAPABILITY_MAX]; struct selinux_avc *avc; struct selinux_ss *ss; -}; +} __randomize_layout; void selinux_ss_init(struct selinux_ss **ss); void selinux_avc_init(struct selinux_avc **avc); diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h index bc56b14e2216..98afe52a3d19 100644 --- a/security/selinux/ss/policydb.h +++ b/security/selinux/ss/policydb.h @@ -307,7 +307,7 @@ struct policydb { u16 process_class; u32 process_trans_perms; -}; +} __randomize_layout;; extern void policydb_destroy(struct policydb *p); extern int policydb_load_isids(struct policydb *p, struct sidtab *s); diff --git a/security/selinux/ss/services.h b/security/selinux/ss/services.h index fc40640a9725..c5896f39e8f6 100644 --- a/security/selinux/ss/services.h +++ b/security/selinux/ss/services.h @@ -31,7 +31,7 @@ struct selinux_ss { struct selinux_map map; struct page *status_page; struct mutex status_lock; -}; +} __randomize_layout; void services_compute_xperms_drivers(struct extended_perms *xperms, struct avtab_node *node);
Randomize the layout of key selinux data structures. Initially this is applied to the selinux_state, selinux_ss, policydb, and task_security_struct data structures. NB To test/use this mechanism, one must install the necessary build-time dependencies, e.g. gcc-plugin-devel on Fedora, and enable CONFIG_GCC_PLUGIN_RANDSTRUCT in the kernel configuration. Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> --- I would have expected that two kernels built with the same config with this enabled would have yielded different struct layouts in pahole vmlinux output, but that doesn't appear to be the case. They do have different seeds. Am I doing something wrong? Also, does DEBUG_INFO_BTF effectively undermine/negate the benefits of this change if enabled? security/selinux/include/objsec.h | 2 +- security/selinux/include/security.h | 2 +- security/selinux/ss/policydb.h | 2 +- security/selinux/ss/services.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-)