2014-06-18 12:41:51

by Chanho Park

[permalink] [raw]
Subject: [PATCH] Smack: separate smackfstransmute and smackfsroot

According to previous commit(e830b39: Smack: Add smkfstransmute mount option),
the smackfstransmute option is the smackfsroot option + transmute option.
I think it can be confused because the transmute option can only have "TRUE".
Before the patch, you cannot use the smackfsroot and the smackfstransmute at
the same time. If you use the options simultaenously, the previous option will
be omitted. In the below example, the smackfsroot option will be omitted.
mount -t tmpfs -o size=128M,smackfsroot=*,smackfstransmute=_ tmpfs /tmp

After the patch, now you can use the smackfstransmute option like below:
mount -t tmpfs -o size=128M,smackfsroot=*,smackfstransmute=TRUE tmpfs /tmp

Signed-off-by: Chanho Park <[email protected]>
---
Documentation/security/Smack.txt | 3 +++
security/smack/smack.h | 2 ++
security/smack/smack_lsm.c | 5 ++---
3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/Documentation/security/Smack.txt b/Documentation/security/Smack.txt
index b6ef7e9..27290d8 100644
--- a/Documentation/security/Smack.txt
+++ b/Documentation/security/Smack.txt
@@ -669,6 +669,9 @@ Smack supports some mount options:
smackfsfloor=label: specifies a label to which all labels set on the
filesystem must have read access. Not yet enforced.

+ smackfstransmute=TRUE: assign a transmute option for the root of the
+ file system if it lacks the Smack extended attribute.
+
These mount options apply to all file system types.

Smack auditing
diff --git a/security/smack/smack.h b/security/smack/smack.h
index 020307e..d4f75e6 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -146,6 +146,8 @@ struct smk_port_label {
#define SMK_FSROOT "smackfsroot="
#define SMK_FSTRANS "smackfstransmute="

+#define SMK_FSTRANS_ON "TRUE"
+
#define SMACK_CIPSO_OPTION "-CIPSO"

/*
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index f2c3080..7d3f40d 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -450,9 +450,8 @@ static int smack_sb_kern_mount(struct super_block *sb, int flags, void *data)
}
} else if (strncmp(op, SMK_FSTRANS, strlen(SMK_FSTRANS)) == 0) {
op += strlen(SMK_FSTRANS);
- nsp = smk_import(op, 0);
- if (nsp != NULL) {
- sp->smk_root = nsp;
+ if (strncmp(op, SMK_FSTRANS_ON,
+ strlen(SMK_FSTRANS_ON)) == 0) {
transmute = 1;
specified = 1;
}
--
1.9.1


2014-06-18 13:26:19

by Casey Schaufler

[permalink] [raw]
Subject: Re: [PATCH] Smack: separate smackfstransmute and smackfsroot

On 6/18/2014 5:41 AM, Chanho Park wrote:
> According to previous commit(e830b39: Smack: Add smkfstransmute mount option),
> the smackfstransmute option is the smackfsroot option + transmute option.
> I think it can be confused because the transmute option can only have "TRUE".
> Before the patch, you cannot use the smackfsroot and the smackfstransmute at
> the same time. If you use the options simultaenously, the previous option will
> be omitted. In the below example, the smackfsroot option will be omitted.
> mount -t tmpfs -o size=128M,smackfsroot=*,smackfstransmute=_ tmpfs /tmp
>
> After the patch, now you can use the smackfstransmute option like below:
> mount -t tmpfs -o size=128M,smackfsroot=*,smackfstransmute=TRUE tmpfs /tmp
>
> Signed-off-by: Chanho Park <[email protected]>

Nacked-by: Casey Schaufler <[email protected]>

The smackfsroot option behaves as intended.
The smackfstransmute option behaves as intended.
There is no case where you want to mark the root inode
transmuting where you do not also want to set the
Smack label of that inode. Making the smackfstransmute
option require the smackfsroot option as well would be
unnecessary and inconvenient.

> ---
> Documentation/security/Smack.txt | 3 +++
> security/smack/smack.h | 2 ++
> security/smack/smack_lsm.c | 5 ++---
> 3 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/security/Smack.txt b/Documentation/security/Smack.txt
> index b6ef7e9..27290d8 100644
> --- a/Documentation/security/Smack.txt
> +++ b/Documentation/security/Smack.txt
> @@ -669,6 +669,9 @@ Smack supports some mount options:
> smackfsfloor=label: specifies a label to which all labels set on the
> filesystem must have read access. Not yet enforced.
>
> + smackfstransmute=TRUE: assign a transmute option for the root of the
> + file system if it lacks the Smack extended attribute.
> +
> These mount options apply to all file system types.
>
> Smack auditing
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index 020307e..d4f75e6 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -146,6 +146,8 @@ struct smk_port_label {
> #define SMK_FSROOT "smackfsroot="
> #define SMK_FSTRANS "smackfstransmute="
>
> +#define SMK_FSTRANS_ON "TRUE"
> +
> #define SMACK_CIPSO_OPTION "-CIPSO"
>
> /*
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index f2c3080..7d3f40d 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -450,9 +450,8 @@ static int smack_sb_kern_mount(struct super_block *sb, int flags, void *data)
> }
> } else if (strncmp(op, SMK_FSTRANS, strlen(SMK_FSTRANS)) == 0) {
> op += strlen(SMK_FSTRANS);
> - nsp = smk_import(op, 0);
> - if (nsp != NULL) {
> - sp->smk_root = nsp;
> + if (strncmp(op, SMK_FSTRANS_ON,
> + strlen(SMK_FSTRANS_ON)) == 0) {
> transmute = 1;
> specified = 1;
> }

2014-06-18 13:36:01

by Chanho Park

[permalink] [raw]
Subject: RE: [PATCH] Smack: separate smackfstransmute and smackfsroot

Hi,

> -----Original Message-----
> From: Casey Schaufler [mailto:[email protected]]
> Sent: Wednesday, June 18, 2014 10:26 PM
> To: Chanho Park
> Cc: [email protected]; [email protected];
> [email protected]; Casey Schaufler
> Subject: Re: [PATCH] Smack: separate smackfstransmute and smackfsroot
>
> On 6/18/2014 5:41 AM, Chanho Park wrote:
> > According to previous commit(e830b39: Smack: Add smkfstransmute mount
> option),
> > the smackfstransmute option is the smackfsroot option + transmute
> option.
> > I think it can be confused because the transmute option can only have
> "TRUE".
> > Before the patch, you cannot use the smackfsroot and the
> smackfstransmute at
> > the same time. If you use the options simultaenously, the previous
> option will
> > be omitted. In the below example, the smackfsroot option will be
> omitted.
> > mount -t tmpfs -o size=128M,smackfsroot=*,smackfstransmute=_ tmpfs
> /tmp
> >
> > After the patch, now you can use the smackfstransmute option like
> below:
> > mount -t tmpfs -o size=128M,smackfsroot=*,smackfstransmute=TRUE tmpfs
> /tmp
> >
> > Signed-off-by: Chanho Park <[email protected]>
>
> Nacked-by: Casey Schaufler <[email protected]>
>
> The smackfsroot option behaves as intended.
> The smackfstransmute option behaves as intended.
> There is no case where you want to mark the root inode
> transmuting where you do not also want to set the
> Smack label of that inode. Making the smackfstransmute
> option require the smackfsroot option as well would be
> unnecessary and inconvenient.

If the smackfstransmute option was like that, we should prevent the two
options at the same time to avoid confusing.
At least, I think we need to provide a documentation for that.

Best Regards,
Chanho Park