Re: 回复: 回复: [RFC PATCH 0/3] smack: add file label preserve mechanism
Casey Schaufler <[email protected]>
| Newsgroups | org.kernel.vger.linux-security-module |
|---|---|
| Message-ID | <[email protected]> |
On 8/24/2026 2:08 AM, Tang Peter wrote: > Subject: smack: preserve whitelist for the FAT32 copy problem (question) > > Hi Casey, > > I have a question about a device-side change I've been working on, driven > by a concrete FAT32 problem, and I'd value your read on what I'm still > missing. > > The problem: FAT32 has no xattr support, so the only way to label files on > a USB stick is the mount-time default (mount -o smackfsdef=USB_Area). That > labels files while they are on the stick, but the moment a file is copied > off the stick onto the root filesystem, the copy inherits the copying > process's label — the USB_Area provenance is silently lost, and an > "untrusted" file can end up with an executable label. > > I understand transmute is the intended tool, and it does cover copying > into a quarantine directory. But on this device the destination isn't > fixed — the application may land the copy in a variety of directories, so > I can't rely on a transmuting directory always being in the way. > > What I ended up with is a preserve variant gated by a whitelist: only a > small set of untrusted-channel labels (USB / network / serial) is > preserved > on copy; everything else keeps the normal process-label behavior. I'm > aware > of the write-down this still carries — the whitelist narrows the > target but > doesn't remove the primitive — and in this deployment it isn't reachable > (the only subject that can read the sensitive label is the trusted admin, > and the operator doing the copies has no read access to it). > > My question: apart from that write-down, which I accept as a device-side > tradeoff, what other problems do you see in this whitelist approach? > And is > there a Smack-native mechanism for the FAT32 "label doesn't survive the > copy" case that I'm still overlooking? Did you investigate the SMACK64_EXEC solution I suggested? Unless you want to copy to arbitrary locations with a variety of labels it should address the issue. > > This is not a merge request — I'm asking to make sure I understand the > space before I ship it. If it helps to see the exact diff for reference, > I can send it. > > Thanks, > Tang Pengke > ------------------------------------------------------------------------ > *发件人:* Casey Schaufler <[email protected]> > *发送时间:* 2026年8月21日 4:11 > *收件人:* Tang Peter <[email protected]> > *抄送:* [email protected] > <[email protected]>; [email protected] > <[email protected]>; Casey Schaufler <[email protected]> > *主题:* Re: 回复: [RFC PATCH 0/3] smack: add file label preserve mechanism > > > On 8/20/2026 5:05 AM, Tang Peter wrote: > > Subject: Re: [RFC PATCH 0/3] smack: add file label preserve mechanism > > > > Hi Casey, > > > > Thanks again for the earlier guidance. I've dropped the preserve > mechanism > > as discussed — the ecp example made the write-down flaw clear. > > > > Let me restate the concrete problem I'm still trying to solve, because > > it is > > narrower than the original patch and I want to check whether there is a > > cleaner native mechanism I'm overlooking. > > > > Context: I'm designing a channel-classification scheme for an embedded > > device (MIPS, BusyBox) where incoming files are labeled by origin — USB, > > network, and serial each map to an untrusted label (USB_Area, Net_Area, > > Serial_Area). The design goal is that a file's label encodes where > it came > > from, so it stays traceable through its lifetime. > > > > The specific gap is FAT32 USB. FAT32 has no xattr support, so per-file > > labels can't be stored on the stick. The only way to label stick > > content is > > the mount-time default: > > > > mount -t vfat -o smackfsdef=USB_Area /dev/sda1 /mnt/usb > > > > which makes every file on the mount appear as USB_Area. That part works. > > The problem is the moment content leaves the stick: `cp /mnt/usb/f.txt > > /srv/data/f.txt` creates f.txt with the copying process's label, so the > > USB_Area provenance is silently lost. > > > > From your earlier reply I understand transmuting labels are the intended > > tool, and indeed copying into a transmuting quarantine directory > preserves > > the label — our ingress path already does exactly that. The case I can't > > close is the later approval step: when an administrator moves a file > > out of > > the quarantine into a normal data directory, the label is lost again > > unless > > the administrator relabels explicitly. > > > > So the question is narrower than preserve: is there an established SMACK > > pattern for "a file that originated on a smackfsdef mount keeps that > > provenance across a copy or move", beyond explicit chsmack at every > step? > > How about trying a special program cp_usbarea which uses a SMACK64_EXEC > attribute to always run with your USB_Area label? You would need a rule > > USB_Area <label-of-destination> rw > > to allow the program to write in the destination directory. cp_usbarea > would need to be trusted to some level, but not to the extent a root > or CAP_MAC_ADMIN process would require. > > > > > I currently solve it in userspace (a small cp wrapper that copies the > > source > > label with chsmack after the copy, which works because chsmack is > > privileged), but I'd rather not invent a mechanism if SMACK already > > has one > > for this case. > > > > Thanks for your time. > > > > Regards, > > Tang Pengke > > ------------------------------------------------------------------------ > > *发件人:* Casey Schaufler <[email protected]> > > *发送时间:* 2026年8月19日 0:11 > > *收件人:* Tang Peter <[email protected]> > > *抄送:* [email protected] > > <[email protected]>; Casey Schaufler > > <[email protected]> > > *主题:* Re: [RFC PATCH 0/3] smack: add file label preserve mechanism > > > > On 8/18/2026 1:05 AM, Tang Peter wrote: > > > Subject: [RFC PATCH 0/3] smack: add file label preserve mechanism > > > > > > Hi Casey, > > > > > > I'm working on an embedded Linux device (MIPS, BusyBox) that uses > SMACK > > > for mandatory access control, and I ran into a problem that I think is > > > worth your opinion on. > > > > > > Problem > > > ------- > > > When a user copies a file with BusyBox cp, the destination file > inherits > > > the *process* label rather than the *source* file's label. > > > > This is the correct and expected behavior. A process that creates > > a new file has the opportunity to put whatever data it desires in it, > > so the new file has to have the process label, regardless of any > labeling > > of the source information. > > > > Cases like this are why Smack has transmuting labels. Without more > > information > > about the specifics of your directory hierarchy and labeling > strategy it's > > hard to offer specific advice, but if what you have is a limited set > > of files > > that require label preservation you should be able to use that > mechanism. > > > > > On desktop or > > > server systems this is solved in userspace -- GNU cp has > > > --preserve=context, and SELinux has restorecond. But on an embedded > > > device with BusyBox, neither is available, and the result is that the > > > source file's label (e.g. an untrusted "USB" label) is silently lost > > > after the copy, which breaks both access control and audit tracing. > > > > > > Proposed mechanism > > > ------------------ > > > An opt-in "preserve" feature controlled via /sys/fs/smackfs/preserve > > > (default off): > > > > > > 1. smack_file_open(): when a regular file is opened O_RDONLY, > > > record its label into task_smack::smk_preserve. Shared library > > > loads under /lib/ and /usr/lib/ are skipped (path-prefix filter) > > > so ld.so does not pollute the preserve slot. > > > > > > 2. smack_inode_init_security(): if smk_preserve is set, use it as > > > the new inode's label and clear it. TRANSMUTE still takes > > > priority, so existing transmuting-directory behaviour is > unchanged. > > > > > > 3. The slot is one-shot: cleared on use, on fork (cred_prepare), > > > and on exec (bprm_creds_for_exec), so it only applies to a > > > "read then create" sequence within a single process. > > > > > > This makes `cp a b`, `cat a > b`, and similar BusyBox workflows keep > > > the source label without any userspace changes. > > > > I have an evil program "ecp", which uses this mechanism. ecp opens a > > file labeled "_", and creates a new executable file /bin/xyzzy. It then > > opens a file labeled "somethingelse", which contains a malicious > program. > > /bin/xyzzy is filled with the malicious program. Now, processes that > > have no access to somethingelse are exposed to the malicious program. > > > > You could restrict smk_preserve to programs with CAP_MAC_ADMIN, but > > such a process could relabel the file directly, rendering the mechanism > > unnecessary. > > > > > > > > Questions > > > --------- > > > - Does this approach make sense for SMACK, or is there an existing / > > > preferred way to solve this that I've missed? > > > - Any concerns about the one-shot slot semantics, or the /lib/ filter? > > > > > > Thanks, > > > Tang Pengke > > > > > > --- > > > security/smack/smack.h | 2 ++ > > > security/smack/smack_lsm.c | 45 > > > +++++++++++++++++++++++++++++++++++++++++---- > > > security/smack/smackfs.c | 50 > > > ++++++++++++++++++++++++++++++++++++++++++++++++++ > > > 3 files changed, 93 insertions(+), 4 deletions(-) > > > > > > > > > From: Tang Pengke <[email protected]> > > > Subject: [RFC PATCH 1/3] smack: add smk_preserve field to task_smack > > > > > > Add a smk_preserve pointer to struct task_smack for recording > > > the label of the last file opened read-only. Also declare the > > > global smack_preserve toggle variable. > > > > > > Signed-off-by: Tang Pengke <[email protected]> > > > --- > > > security/smack/smack.h | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/security/smack/smack.h b/security/smack/smack.h > > > index 8ad30955e15..c6eea57cf52 100644 > > > --- a/security/smack/smack.h > > > +++ b/security/smack/smack.h > > > @@ -124,6 +124,7 @@ struct task_smack { > > > struct list_head smk_rules; /* per task access rules */ > > > struct mutex smk_rules_lock; /* lock for the > rules */ > > > struct list_head smk_relabel; /* transit allowed labels */ > > > + struct smack_known *smk_preserve; /* label to preserve > > > on create */ > > > }; > > > > > > #define SMK_INODE_INSTANT 0x01 /* inode is instantiated */ > > > @@ -325,6 +326,7 @@ extern struct smack_known *smack_unconfined; > > > #endif > > > extern int smack_ptrace_rule; > > > +extern int smack_preserve; > > > extern struct lsm_blob_sizes smack_blob_sizes; > > > > > > extern struct smack_known smack_known_floor; > > > > > > > > > From: Tang Pengke <[email protected]> > > > Subject: [RFC PATCH 2/3] smack: implement file label preserve > mechanism > > > > > > When smack_preserve is enabled, opening a regular file read-only > > > records its label in task_smack::smk_preserve. On the next file > > > creation (smack_inode_init_security), that label is applied to the > > > new inode -- unless TRANSMUTE takes priority. > > > > > > The preserve is one-shot: > > > - Consumed immediately on new inode creation > > > - Cleared on fork (smack_cred_prepare) > > > - Cleared on exec (smack_bprm_creds_for_exec) > > > > > > Shared library loads under /lib/ and /usr/lib/ are explicitly > > > excluded from recording, so ld.so does not pollute the preserve > > > label during exec. Non-regular files (directories, devices) are > > > also skipped. > > > > > > This enables tools like cp and 'cat > file' to create files that > > > retain the source file's label, without requiring any userspace > > > changes. The feature is disabled by default (preserve=0). > > > > > > Signed-off-by: Tang Pengke <[email protected]> > > > --- > > > security/smack/smack_lsm.c | 45 > > > +++++++++++++++++++++++++++++++++++++++++---- > > > 1 file changed, 41 insertions(+), 4 deletions(-) > > > > > > diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c > > > index 11f238e94..951c79d9e 100644 > > > --- a/security/smack/smack_lsm.c > > > +++ b/security/smack/smack_lsm.c > > > @@ -935,6 +935,7 @@ static int smack_bprm_creds_for_exec(struct > > > linux_binprm *bprm) > > > return -EPERM; > > > > > > bsp->smk_task = isp->smk_task; > > > + bsp->smk_preserve = NULL; > > > bprm->per_clear |= PER_CLEAR_ON_SETID; > > > > > > /* Decide if this is a secure exec. */ > > > @@ -1066,6 +1067,15 @@ static int smack_inode_init_security(struct > > > inode *inode, struct inode *dir, > > > } > > > } > > > > > > + /* > > > + * If a preserve label was recorded on file open, use it > > > + * for the new inode (unless TRANSMUTE already applied above). > > > + */ > > > + if (tsp->smk_preserve != NULL) { > > > + issp->smk_inode = tsp->smk_preserve; > > > + tsp->smk_preserve = NULL; > > > + } > > > + > > > if (rc == 0) > > > if (xattr_dupval(xattrs, xattr_count, > > > XATTR_SMACK_SUFFIX, > > > @@ -2064,6 +2074,27 @@ static int smack_file_open(struct file *file) > > > struct smk_audit_info ad; > > > int rc; > > > > > > + if (smack_preserve && (file->f_flags & O_ACCMODE) == O_RDONLY) { > > > + char *buf; > > > + const char *path; > > > + > > > + if (!S_ISREG(inode->i_mode)) > > > + goto skip_preserve; > > > + > > > + buf = kmalloc(PATH_MAX, GFP_KERNEL); > > > + if (!buf) > > > + goto skip_preserve; > > > + > > > + path = dentry_path_raw(file->f_path.dentry, buf, > PATH_MAX); > > > + if (!IS_ERR(path) && > > > + strncmp(path, "/lib/", 5) != 0 && > > > + strncmp(path, "/usr/lib/", 9) != 0) > > > + tsp->smk_preserve = smk_of_inode(inode); > > > + > > > + kfree(buf); > > > + } > > > +skip_preserve: > > > + > > > smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_PATH); > > > smk_ad_setfield_u_fs_path(&ad, file->f_path); > > > rc = smk_tskacc(tsp, smk_of_inode(inode), MAY_READ, &ad); > > > @@ -2129,6 +2160,7 @@ static int smack_cred_prepare(struct cred *new, > > > const struct cred *old, > > > int rc; > > > > > > init_task_smack(new_tsp, old_tsp->smk_task, old_tsp->smk_task); > > > + new_tsp->smk_preserve = NULL; > > > > > > rc = smk_copy_rules(&new_tsp->smk_rules, &old_tsp->smk_rules, > > gfp); > > > if (rc != 0) > > > > > > > > > From: Tang Pengke <[email protected]> > > > Subject: [RFC PATCH 3/3] smack: add /sys/fs/smackfs/preserve interface > > > > > > Expose the smack_preserve toggle via smackfs. Reading returns '0' > > > or '1'. Writing '0' or '1' toggles the feature (requires > > > CAP_MAC_ADMIN). Default is disabled (0) so existing systems are > > > unaffected. > > > > > > Signed-off-by: Tang Pengke <[email protected]> > > > --- > > > security/smack/smackfs.c | 50 > > > ++++++++++++++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 50 insertions(+) > > > > > > diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c > > > index a3a55a25360..8379ef73cf2 100644 > > > --- a/security/smack/smackfs.c > > > +++ b/security/smack/smackfs.c > > > @@ -62,6 +62,7 @@ enum smk_inos { > > > SMK_NET6ADDR = 23, /* single label IPv6 hosts */ > > > #endif /* CONFIG_IPV6 */ > > > SMK_RELABEL_SELF = 24, /* relabel possible without > > CAP_MAC_ADMIN */ > > > + SMK_PRESERVE = 25, /* label preserve on copy */ > > > }; > > > > > > /* > > > @@ -2047,6 +2048,53 @@ static ssize_t smk_write_onlycap(struct file > > > *file, const char __user *buf, > > > return rc; > > > } > > > > > > +int smack_preserve; > > > + > > > +static ssize_t smk_read_preserve(struct file *filp, char __user *buf, > > > + size_t cn, loff_t *ppos) > > > +{ > > > + char val[2]; > > > + > > > + if (*ppos != 0) > > > + return 0; > > > + > > > + val[0] = smack_preserve ? '1' : '0'; > > > + val[1] = '\n'; > > > + > > > + return simple_read_from_buffer(buf, cn, ppos, val, 2); > > > +} > > > + > > > +static ssize_t smk_write_preserve(struct file *file, const char > > > __user *buf, > > > + size_t count, loff_t *ppos) > > > +{ > > > + char data; > > > + > > > + if (!smack_privileged(CAP_MAC_ADMIN)) > > > + return -EPERM; > > > + > > > + if (*ppos != 0) > > > + return -EINVAL; > > > + > > > + if (copy_from_user(&data, buf, 1) != 0) > > > + return -EFAULT; > > > + > > > + if (data == '0') > > > + smack_preserve = 0; > > > + else if (data == '1') > > > + smack_preserve = 1; > > > + else > > > + return -EINVAL; > > > + > > > + return count; > > > +} > > > + > > > +static const struct file_operations smk_preserve_ops = { > > > + .read = smk_read_preserve, > > > + .write = smk_write_preserve, > > > + .llseek = default_llseek, > > > +}; > > > + > > > static const struct file_operations smk_onlycap_ops = { > > > .open = smk_open_onlycap, > > > .read = seq_read, > > > @@ -2933,6 +2981,8 @@ static int smk_fill_super(struct super_block > > > *sb, struct fs_context *fc) > > > [SMK_RELABEL_SELF] = { > > > "relabel-self", &smk_relabel_self_ops, > > > S_IRUGO|S_IWUGO}, > > > + [SMK_PRESERVE] = { > > > + "preserve", &smk_preserve_ops, S_IRUGO|S_IWUSR}, > > > /* last one */ > > > {""} > > > };