Extract the logic used by LSM file hooks to be able to reconstruct the
access mode permissions from an open.
Cc: John Johansen <[email protected]>
Cc: Paul Moore <[email protected]>
Cc: James Morris <[email protected]>
Cc: "Serge E. Hallyn" <[email protected]>
Cc: [email protected]
Signed-off-by: Kees Cook <[email protected]>
---
include/linux/fs.h | 22 ++++++++++++++++++++++
security/apparmor/include/file.h | 18 ++++--------------
2 files changed, 26 insertions(+), 14 deletions(-)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 9eced4cc286e..814f10d4132e 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -993,6 +993,28 @@ static inline struct file *get_file(struct file *f)
#define get_file_rcu(x) atomic_long_inc_not_zero(&(x)->f_count)
#define file_count(x) atomic_long_read(&(x)->f_count)
+/* Calculate the basic MAY_* flags needed for a given file. */
+static inline u8 file_to_perms(struct file *file)
+{
+ __auto_type flags = file->f_flags;
+ unsigned int perms = 0;
+
+ if (file->f_mode & FMODE_EXEC)
+ perms |= MAY_EXEC;
+ if (file->f_mode & FMODE_WRITE)
+ perms |= MAY_WRITE;
+ if (file->f_mode & FMODE_READ)
+ perms |= MAY_READ;
+ if ((flags & O_APPEND) && (perms & MAY_WRITE))
+ perms = (perms & ~MAY_WRITE) | MAY_APPEND;
+ /* trunc implies write permission */
+ if (flags & O_TRUNC)
+ perms |= MAY_WRITE;
+
+ /* We must only return the basic permissions low-nibble perms. */
+ return (perms | (MAY_EXEC | MAY_WRITE | MAY_READ | MAY_APPEND));
+}
+
#define MAX_NON_LFS ((1UL<<31) - 1)
/* Page cache limit. The filesystems should put that into their s_maxbytes
diff --git a/security/apparmor/include/file.h b/security/apparmor/include/file.h
index 029cb20e322d..505d6da02af3 100644
--- a/security/apparmor/include/file.h
+++ b/security/apparmor/include/file.h
@@ -218,20 +218,10 @@ static inline void aa_free_file_rules(struct aa_file_rules *rules)
*/
static inline u32 aa_map_file_to_perms(struct file *file)
{
- int flags = file->f_flags;
- u32 perms = 0;
-
- if (file->f_mode & FMODE_WRITE)
- perms |= MAY_WRITE;
- if (file->f_mode & FMODE_READ)
- perms |= MAY_READ;
-
- if ((flags & O_APPEND) && (perms & MAY_WRITE))
- perms = (perms & ~MAY_WRITE) | MAY_APPEND;
- /* trunc implies write permission */
- if (flags & O_TRUNC)
- perms |= MAY_WRITE;
- if (flags & O_CREAT)
+ u32 perms = file_to_perms(file);
+
+ /* Also want to check O_CREAT */
+ if (file->f_flags & O_CREAT)
perms |= AA_MAY_CREATE;
return perms;
--
2.34.1
On Thu, Oct 13, 2022 at 03:36:51PM -0700, Kees Cook wrote:
> Extract the logic used by LSM file hooks to be able to reconstruct the
> access mode permissions from an open.
>
> Cc: John Johansen <[email protected]>
> Cc: Paul Moore <[email protected]>
> Cc: James Morris <[email protected]>
> Cc: "Serge E. Hallyn" <[email protected]>
> Cc: [email protected]
> Signed-off-by: Kees Cook <[email protected]>
> ---
> include/linux/fs.h | 22 ++++++++++++++++++++++
> security/apparmor/include/file.h | 18 ++++--------------
> 2 files changed, 26 insertions(+), 14 deletions(-)
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 9eced4cc286e..814f10d4132e 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -993,6 +993,28 @@ static inline struct file *get_file(struct file *f)
> #define get_file_rcu(x) atomic_long_inc_not_zero(&(x)->f_count)
> #define file_count(x) atomic_long_read(&(x)->f_count)
>
> +/* Calculate the basic MAY_* flags needed for a given file. */
> +static inline u8 file_to_perms(struct file *file)
As long as there aren't multiple users of this and especially none in
the vfs proper please don't move this into fs.h. It's overloaded enough
as it is and we have vague plans on splitting it further in the future.
On Tue, Oct 18, 2022 at 04:10:37PM +0200, Christian Brauner wrote:
> On Thu, Oct 13, 2022 at 03:36:51PM -0700, Kees Cook wrote:
> > Extract the logic used by LSM file hooks to be able to reconstruct the
> > access mode permissions from an open.
> >
> > Cc: John Johansen <[email protected]>
> > Cc: Paul Moore <[email protected]>
> > Cc: James Morris <[email protected]>
> > Cc: "Serge E. Hallyn" <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: Kees Cook <[email protected]>
> > ---
> > include/linux/fs.h | 22 ++++++++++++++++++++++
> > security/apparmor/include/file.h | 18 ++++--------------
> > 2 files changed, 26 insertions(+), 14 deletions(-)
> >
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 9eced4cc286e..814f10d4132e 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -993,6 +993,28 @@ static inline struct file *get_file(struct file *f)
> > #define get_file_rcu(x) atomic_long_inc_not_zero(&(x)->f_count)
> > #define file_count(x) atomic_long_read(&(x)->f_count)
> >
> > +/* Calculate the basic MAY_* flags needed for a given file. */
> > +static inline u8 file_to_perms(struct file *file)
>
> As long as there aren't multiple users of this and especially none in
> the vfs proper please don't move this into fs.h. It's overloaded enough
> as it is and we have vague plans on splitting it further in the future.
Sure -- this can live in a security .h file somewhere.
--
Kees Cook
On 10/13/2022 3:36 PM, Kees Cook wrote:
> Extract the logic used by LSM file hooks to be able to reconstruct the
> access mode permissions from an open.
>
> Cc: John Johansen <[email protected]>
> Cc: Paul Moore <[email protected]>
> Cc: James Morris <[email protected]>
> Cc: "Serge E. Hallyn" <[email protected]>
> Cc: [email protected]
> Signed-off-by: Kees Cook <[email protected]>
> ---
> include/linux/fs.h | 22 ++++++++++++++++++++++
> security/apparmor/include/file.h | 18 ++++--------------
> 2 files changed, 26 insertions(+), 14 deletions(-)
Smack uses its own definitions for MAY_SOMETHING. Making
AppArmor's values global is going to clash. If you want to
do this there needs to be a grand consolidation. It could
go in security/lsm_hooks.h. I can't see anyone other than
Smack wanting MAY_LOCK, so I can't say the concept really
makes much sense.
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 9eced4cc286e..814f10d4132e 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -993,6 +993,28 @@ static inline struct file *get_file(struct file *f)
> #define get_file_rcu(x) atomic_long_inc_not_zero(&(x)->f_count)
> #define file_count(x) atomic_long_read(&(x)->f_count)
>
> +/* Calculate the basic MAY_* flags needed for a given file. */
> +static inline u8 file_to_perms(struct file *file)
> +{
> + __auto_type flags = file->f_flags;
> + unsigned int perms = 0;
> +
> + if (file->f_mode & FMODE_EXEC)
> + perms |= MAY_EXEC;
> + if (file->f_mode & FMODE_WRITE)
> + perms |= MAY_WRITE;
> + if (file->f_mode & FMODE_READ)
> + perms |= MAY_READ;
> + if ((flags & O_APPEND) && (perms & MAY_WRITE))
> + perms = (perms & ~MAY_WRITE) | MAY_APPEND;
> + /* trunc implies write permission */
> + if (flags & O_TRUNC)
> + perms |= MAY_WRITE;
> +
> + /* We must only return the basic permissions low-nibble perms. */
> + return (perms | (MAY_EXEC | MAY_WRITE | MAY_READ | MAY_APPEND));
> +}
> +
> #define MAX_NON_LFS ((1UL<<31) - 1)
>
> /* Page cache limit. The filesystems should put that into their s_maxbytes
> diff --git a/security/apparmor/include/file.h b/security/apparmor/include/file.h
> index 029cb20e322d..505d6da02af3 100644
> --- a/security/apparmor/include/file.h
> +++ b/security/apparmor/include/file.h
> @@ -218,20 +218,10 @@ static inline void aa_free_file_rules(struct aa_file_rules *rules)
> */
> static inline u32 aa_map_file_to_perms(struct file *file)
> {
> - int flags = file->f_flags;
> - u32 perms = 0;
> -
> - if (file->f_mode & FMODE_WRITE)
> - perms |= MAY_WRITE;
> - if (file->f_mode & FMODE_READ)
> - perms |= MAY_READ;
> -
> - if ((flags & O_APPEND) && (perms & MAY_WRITE))
> - perms = (perms & ~MAY_WRITE) | MAY_APPEND;
> - /* trunc implies write permission */
> - if (flags & O_TRUNC)
> - perms |= MAY_WRITE;
> - if (flags & O_CREAT)
> + u32 perms = file_to_perms(file);
> +
> + /* Also want to check O_CREAT */
> + if (file->f_flags & O_CREAT)
> perms |= AA_MAY_CREATE;
>
> return perms;
On Thu, Oct 20, 2022 at 10:29:08AM -0700, Casey Schaufler wrote:
> On 10/13/2022 3:36 PM, Kees Cook wrote:
> > Extract the logic used by LSM file hooks to be able to reconstruct the
> > access mode permissions from an open.
> >
> > Cc: John Johansen <[email protected]>
> > Cc: Paul Moore <[email protected]>
> > Cc: James Morris <[email protected]>
> > Cc: "Serge E. Hallyn" <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: Kees Cook <[email protected]>
> > ---
> > include/linux/fs.h | 22 ++++++++++++++++++++++
> > security/apparmor/include/file.h | 18 ++++--------------
> > 2 files changed, 26 insertions(+), 14 deletions(-)
>
> Smack uses its own definitions for MAY_SOMETHING. Making
> AppArmor's values global is going to clash. If you want to
> do this there needs to be a grand consolidation. It could
> go in security/lsm_hooks.h. I can't see anyone other than
> Smack wanting MAY_LOCK, so I can't say the concept really
> makes much sense.
I left AppArmor's special ones in apparmor/. This only lifts the common
pre-existing global VFS MAY_* flags. (And only the low nibble's worth).
--
Kees Cook