| Message ID | 20171003005042.16470-1-kilobyte@angband.pl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Oct 03, 2017 at 02:50:42AM +0200, Adam Borowski wrote: > Anything with bytes 1-31,127 will get -EACCES. > > Especially \n is bad: instead of natural file-per-line, you need an > user-unfriendly feature of -print0 added to every producer and consumer; > a good part of users either don't know or don't feel the need to bother > with escaping this snowflake, thus introducing security holes. > > The rest of control characters, while not as harmful, don't have a > legitimate use nor have any real chance of coming from a tarball (malice > and fooling around excluded). No character set ever supported as a system > locale by glibc, and, TTBMK, by other popular Unices, includes them, thus > it can be assumed no foreign files have such names other than artificially. > > This goes in stark contrast with other characters proposed to be banned: > non-UTF8 is common, and even on my desktop's disk I found examples of all > of: [ ], < >, initial -, initial and final space, ?, *, .., ', ", |, &. > Somehow no \ anywhere. I think I have an idea why no / . > > Another debatable point is whether to -EACCES or to silently rename to an > escaped form such as %0A. I believe the former is better because: > * programs can be confused if a directory has files they didn't just write > * many filesystems already disallow certain characters (like invalid > Unicode), thus returning an error is consistent > > An example of a write-up of this issue can be found at: > https://www.dwheeler.com/essays/fixing-unix-linux-filenames.html That essay is full of shit, and you've even mentioned parts of that just above... NAK; you'd _still_ need proper quoting (or a shell with something resembling an actual syntax, rather than the "more or less what srb had ended up implementing"), so it doesn't really buy you anything. Badly written script will still be exploitable. And since older kernels and other Unices are not going away, you would've created an inconsistently vulnerable set of scripts, on top of the false sense of security.
On Tue, Oct 03, 2017 at 03:07:24AM +0100, Al Viro wrote: > On Tue, Oct 03, 2017 at 02:50:42AM +0200, Adam Borowski wrote: > > Anything with bytes 1-31,127 will get -EACCES. > > > > Especially \n is bad: instead of natural file-per-line, you need an > > user-unfriendly feature of -print0 added to every producer and consumer; > > a good part of users either don't know or don't feel the need to bother > > with escaping this snowflake, thus introducing security holes. > > > > The rest of control characters, while not as harmful, don't have a > > legitimate use nor have any real chance of coming from a tarball (malice > > and fooling around excluded). No character set ever supported as a system > > locale by glibc, and, TTBMK, by other popular Unices, includes them, thus > > it can be assumed no foreign files have such names other than artificially. > > > > This goes in stark contrast with other characters proposed to be banned: > > non-UTF8 is common, and even on my desktop's disk I found examples of all > > of: [ ], < >, initial -, initial and final space, ?, *, .., ', ", |, &. > > Somehow no \ anywhere. I think I have an idea why no / . > > > > Another debatable point is whether to -EACCES or to silently rename to an > > escaped form such as %0A. I believe the former is better because: > > * programs can be confused if a directory has files they didn't just write > > * many filesystems already disallow certain characters (like invalid > > Unicode), thus returning an error is consistent > > > > An example of a write-up of this issue can be found at: > > https://www.dwheeler.com/essays/fixing-unix-linux-filenames.html > > That essay is full of shit, and you've even mentioned parts of that just > above... I used it as a list of problems, not solutions. > NAK; you'd _still_ need proper quoting (or a shell with something resembling an > actual syntax, rather than the "more or less what srb had ended up implementing"), > so it doesn't really buy you anything. Well, what about just \n then? Unlike all the others which are relatively straightforward, \n requires -print0 which not all programs implement, and way too many people consider too burdensome to use. > Badly written script will still be exploitable. Yeah, but we'd kill a major exploit avenue. > And since older kernels and other Unices are not going away, you would've > created an inconsistently vulnerable set of scripts, on top of the false > sense of security. That shouldn't stop us from improving new kernels -- scripts that have -print0 won't lose it, those that don't will have a vulnerability fixed. Same as with any other kind of hardening. As for other Unices: Theo de Raadt is not someone to object to a trivial security patch, FreeBSD would follow, OSX is too hostile to developers for me to care. Thus, the only concern is new userland on old kernels. But distributions don't support such combinations for long, unlike the other way around. As for people writing their own scripts: they already tend to be vulnerable. I for example, when writing an ad-hoc pipeline, tend to first make it display files that'd be processed; switching that to -print0 back and forth would be really tedious thus I usually remain vulnerable to \n (unless the script is meant for external use -- but it's too easy to forget). And how do you propose to process a list of files with grep or sed if there are newlines involved? Basic quotes make it trivial to handle everything but two snowflakes: \n and initial -; the latter you need to remember about but ./* or -- aren't hard. This leaves \n. Thus, would you consider banning just newlines? Meow!
On Tue, Oct 03, 2017 at 03:07:24AM +0100, Al Viro wrote: > That essay is full of shit, and you've even mentioned parts of that just above... > NAK; you'd _still_ need proper quoting (or a shell with something resembling an > actual syntax, rather than the "more or less what srb had ended up implementing"), > so it doesn't really buy you anything. Badly written script will still be > exploitable. And since older kernels and other Unices are not going away, > you would've created an inconsistently vulnerable set of scripts, on top of > the false sense of security. Banning certain characters is certainly not a panacea, and there are a lot of best practices that you have to follow when writing good scripts which most people don't follow, and so it's arguable that benefits are being overstated. That being said the costs of suppressing certain bytes from appearing in pathnames seem fairly low. Would this be more palatable if the ban on control characters were made into a compile-time or mount-time option? - Ted
On Tue, Oct 03, 2017 at 12:40:12PM -0400, Theodore Ts'o wrote: > On Tue, Oct 03, 2017 at 03:07:24AM +0100, Al Viro wrote: > > That essay is full of shit, and you've even mentioned parts of that just above... > > NAK; you'd _still_ need proper quoting (or a shell with something resembling an > > actual syntax, rather than the "more or less what srb had ended up implementing"), > > so it doesn't really buy you anything. Badly written script will still be > > exploitable. And since older kernels and other Unices are not going away, > > you would've created an inconsistently vulnerable set of scripts, on top of > > the false sense of security. > > Banning certain characters is certainly not a panacea, and there are a > lot of best practices that you have to follow when writing good > scripts which most people don't follow, and so it's arguable that > benefits are being overstated. Yeah, it's "most people don't follow" which is the main issue here. And it appears to me that it's easy to plug the worst issues, especially \n. > That being said the costs of suppressing certain bytes from appearing > in pathnames seem fairly low. There are three categories of problematic bytes/byte sequences: * those that don't appear in the wild, require a bit of knowledge to input (inserting a control char is no rocket science but is beyond an average user's skill), and also have potential to cause damage - This is why I picked 1-31,127 in the initial patch. * stuff that's unwanted but not unknown in the wild * stuff that's used by every "normal" user, and considered problematic only by us traditional Unix folks > Would this be more palatable if the ban on control characters were made > into a compile-time or mount-time option? For malformed Unicode or such, it'd make sense, yeah. But Al has a good point that if most people were protected, they won't bother escaping badness anymore -- leaving those whose systems allow control chars vulnerable if they run a script that doesn't do quoting. Thus, it'd be good to make at least worst cases non-configurable. I went bold and submitted 1-31,127, as those have very low cost to block; but if that's not conservative enough, blocking just \n has both very low cost and a high benefit (special burdensome quoting required). Discussing a configurable policy (perhaps here in vfs, perhaps as a LSM, a seccomp hack or even LD_PRELOAD) would be interesting, but for the above reason I'd want \n hard-banned. Meow!
On Tue, Oct 03, 2017 at 07:32:15PM +0200, Adam Borowski wrote: > > But Al has a good point that if most people were protected, they won't > bother escaping badness anymore -- leaving those whose systems allow control > chars vulnerable if they run a script that doesn't do quoting. If we look at the attitude used by the kernel-hardening efforts, it's all about adding layers of protection. We can optionally enable features like KASLR, but does that mean that people can afford to be careless with pointers? Not hardly! And that's a pretty good worked example where adding various classes of kernel-hardening protections has *not* resulted in people saying, "Great! I can be careless in the patches we submit to LKML". > I went bold and submitted 1-31,127, as those have very low cost to block; > but if that's not conservative enough, blocking just \n has both very low > cost and a high benefit (special burdensome quoting required). I would have suggested 1-31, since that's in line with what Windows has banned. But whether we include DEL is my mind not a big deal. The argument for making it be configurable is that if it does break things in way we can't foresee, it's a lot easier to back it out. And like what we've done with relatime, if the distro's all run with it as the default for a couple of years, it then becomes easier to make the case for making it be the default. > Discussing a configurable policy (perhaps here in vfs, perhaps as a LSM, a > seccomp hack or even LD_PRELOAD) would be interesting, but for the above > reason I'd want \n hard-banned. Perhaps doing this as an LSM makes the most amount of sense. That makes it be configurable/optional, and I think the security folks will be much more willing to accept the functionality, if we decide we don't want to make it a core VFS restriction. - Ted
On 10/3/2017 11:58 AM, Theodore Ts'o wrote: > On Tue, Oct 03, 2017 at 07:32:15PM +0200, Adam Borowski wrote: >> But Al has a good point that if most people were protected, they won't >> bother escaping badness anymore -- leaving those whose systems allow control >> chars vulnerable if they run a script that doesn't do quoting. > If we look at the attitude used by the kernel-hardening efforts, it's > all about adding layers of protection. We can optionally enable > features like KASLR, but does that mean that people can afford to be > careless with pointers? Not hardly! > > And that's a pretty good worked example where adding various classes > of kernel-hardening protections has *not* resulted in people saying, > "Great! I can be careless in the patches we submit to LKML". > >> I went bold and submitted 1-31,127, as those have very low cost to block; >> but if that's not conservative enough, blocking just \n has both very low >> cost and a high benefit (special burdensome quoting required). > I would have suggested 1-31, since that's in line with what Windows > has banned. But whether we include DEL is my mind not a big deal. > > The argument for making it be configurable is that if it does break > things in way we can't foresee, it's a lot easier to back it out. And > like what we've done with relatime, if the distro's all run with it as > the default for a couple of years, it then becomes easier to make the > case for making it be the default. > >> Discussing a configurable policy (perhaps here in vfs, perhaps as a LSM, a >> seccomp hack or even LD_PRELOAD) would be interesting, but for the above >> reason I'd want \n hard-banned. > Perhaps doing this as an LSM makes the most amount of sense. That > makes it be configurable/optional, and I think the security folks will > be much more willing to accept the functionality, if we decide we > don't want to make it a core VFS restriction. The resistance to using LSMs for things other than access control is pretty well gone at this point. An LSM implementation makes sense, and I'm pretty sure I saw one proposed recently. I can't find the details, unfortunately. > > - Ted > .
On Tue, Oct 3, 2017 at 5:22 AM, Adam Borowski <kilobyte@angband.pl> wrote: > Well, what about just \n then? Unlike all the others which are relatively > straightforward, \n requires -print0 which not all programs implement, and > way too many people consider too burdensome to use. If you don't use -print0, you're vulnerable to spaces. Go on, try to disallow spaces in file names, I'll pass the popcorn. OG.
> For malformed Unicode or such, it'd make sense, yeah.
Not really. It's legitimate to have bad unicode in a directory, or have a
file system where some users are still in 8bit Russian encoding and some
are unicode for example.
The fix for this has always been the same - don't use shell script and
similar things (php for example) where incorrect quoting causes you to
execute random attacker code.
As most of the waya to attack a shell script are printable symbols like
$, ; ` and * you aren't going to save anyone by adding hacks to the VFS.
Alan
On Tue, Oct 03, 2017 at 02:58:52PM -0400, Theodore Ts'o wrote: > The argument for making it be configurable is that if it does break > things in way we can't foresee, it's a lot easier to back it out. And > like what we've done with relatime, if the distro's all run with it as > the default for a couple of years, it then becomes easier to make the > case for making it be the default. I find it hard to believe that any general-purpose distro could turn on something like this without breaking a gazillion things for users. > > Discussing a configurable policy (perhaps here in vfs, perhaps as a LSM, a > > seccomp hack or even LD_PRELOAD) would be interesting, but for the above > > reason I'd want \n hard-banned. > > Perhaps doing this as an LSM makes the most amount of sense. That > makes it be configurable/optional, and I think the security folks will > be much more willing to accept the functionality, if we decide we > don't want to make it a core VFS restriction. Making this something you can turn on and off seems likely to create all sorts of surprises for users when filenames written under one kernel can't be read under another. This kind of restriction sounds more like a permanent feature of the filesystem--something you'd set at mkfs time. We already have filesystems with these kinds of restrictions, don't we? It'd seem trivial to add a "disallow weird characters on this superblock" flag to ext4. --b.
On Thu, Oct 05, 2017 at 12:16:19PM -0400, J. Bruce Fields wrote: > This kind of restriction sounds more like a permanent feature of the > filesystem--something you'd set at mkfs time. > > We already have filesystems with these kinds of restrictions, don't we? In general, no. Filename storage typically defined in the filesystem on-disk formats as an opaque string of bytes - the filesystem has no business parsing them to determine validity of the bytes. Think encrypted filenames and the like - control characters in the on-disk format are most definitely necessary and therefore must be legal. > It'd seem trivial to add a "disallow weird characters on this > superblock" flag to ext4. It seems that way until you consider the scope of work it would involve: to be an effective restrictive mechanism, we'd have to add it to the on-disk format of every supported filesystem, not just ext4. And then, because it has become part of the defined on disk format, every userspace utility for each filesystem has to support it - mkfs, fsck, etc. Filesystem on-disk format documentation needs to be updated. And checking filenames for validity under this new scheme and "fixing" them if they are invalid (i.e. corrupt) needs to be added to fsck, online scrubbers, etc. Then there's all the test infrastructure that is needed around this, too, so we can ensure that every filesystem implements the exact same semantics and behaviour. And if it changes the way directories are formatted on disk for a filesystem, then you've got to do non-obvious stuff like /patch grub/ so it can parse the new format from the bootloader context. Nothing is trivial or simple when you start needing to add on-disk features to filesystems... Cheers, Dave.
On Fri, Oct 06, 2017 at 01:09:42PM +1100, Dave Chinner wrote: > On Thu, Oct 05, 2017 at 12:16:19PM -0400, J. Bruce Fields wrote: > > This kind of restriction sounds more like a permanent feature of the > > filesystem--something you'd set at mkfs time. > > > > We already have filesystems with these kinds of restrictions, don't we? > > In general, no. Filename storage typically defined in the > filesystem on-disk formats as an opaque string of bytes - the > filesystem has no business parsing them to determine validity of the > bytes. Think encrypted filenames and the like - control characters > in the on-disk format are most definitely necessary and therefore > must be legal. I was thinking of non-unixy filesystems (FAT?). > > It'd seem trivial to add a "disallow weird characters on this > > superblock" flag to ext4. > > It seems that way until you consider the scope of work it would > involve: to be an effective restrictive mechanism, we'd have to add > it to the on-disk format of every supported filesystem, not just > ext4. Right, I was assuming it's something you'd do one filesystem at a time. > And then, because it has become part of the defined on disk format, > every userspace utility for each filesystem has to support it - > mkfs, fsck, etc. Filesystem on-disk format documentation needs to be > updated. And checking filenames for validity under this new scheme > and "fixing" them if they are invalid (i.e. corrupt) needs to be > added to fsck, online scrubbers, etc. Then there's all the test > infrastructure that is needed around this, too, so we can ensure > that every filesystem implements the exact same semantics and > behaviour. > > And if it changes the way directories are formatted on disk for a > filesystem, then you've got to do non-obvious stuff like /patch > grub/ so it can parse the new format from the bootloader context. > > Nothing is trivial or simple when you start needing to add > on-disk features to filesystems... Fair enough. I'm not convinced it's a workable idea anyway. But if we had to do it I'd rather see it in the filesystem just because otherwise it's too easy for the user to create "bad" filenames (because they mounted from a different kernel, or turned off the relevant LSM, or whatever). And then we get to decide whether we'd rather remap those names somehow or fail readdir. --b.
On Thu, Oct 05, 2017 at 12:07:57PM +0200, Olivier Galibert wrote: > On Tue, Oct 3, 2017 at 5:22 AM, Adam Borowski <kilobyte@angband.pl> wrote: > > Well, what about just \n then? Unlike all the others which are relatively > > straightforward, \n requires -print0 which not all programs implement, and > > way too many people consider too burdensome to use. > > If you don't use -print0, you're vulnerable to spaces. Go on, try to > disallow spaces in file names, I'll pass the popcorn. The David Wheeler argument for this is to change IFS to "\n\t" from " \n\t". Personally, I think it's easier to use -print0.
On Fri, Oct 06, 2017 at 01:09:42PM +1100, Dave Chinner wrote: > On Thu, Oct 05, 2017 at 12:16:19PM -0400, J. Bruce Fields wrote: > > This kind of restriction sounds more like a permanent feature of the > > filesystem--something you'd set at mkfs time. > > > > We already have filesystems with these kinds of restrictions, don't we? > > In general, no. Filename storage typically defined in the > filesystem on-disk formats as an opaque string of bytes - the > filesystem has no business parsing them to determine validity of the > bytes. Think encrypted filenames and the like - control characters > in the on-disk format are most definitely necessary and therefore > must be legal. Umm. But filenames still can't have / or \0 in them, so your encryption already has to avoid at least two special characters. I agree with your main point though; there is no advantage to doing this in each individual filesystem.
On Fri, Oct 06, 2017 at 07:57:01AM -0700, Matthew Wilcox wrote: > > Umm. But filenames still can't have / or \0 in them, so your encryption > already has to avoid at least two special characters. > > I agree with your main point though; there is no advantage to doing this > in each individual filesystem. One advantage of doing it in an LSM is you can simply make this a ban on the *creation* of new files that match some particular "bad character set". This might be all characters between 1 and 31, not just for security reasons, but also if you are running a filer where the files need to be accessible by Windows sytems, and Windows doesn't allow file names containing control characters. Ultimately, the cost/benefit ratio of this is small. Forbidding newlines doesn't actually buy you that much. It is true that there are some shell progamming constructs which will deal with embedded spaces in file names, but not with embedded newlines --- but there are many more constructs that will fail to deal with spaces, and there enough other potential gotchas that if a shell script progammer isn't using something like shellcheck, they are going to be tempting fate. At the same time, the _cost_ of forbidding control chracaters is tiny. And while the risk of embedding an entertaining escape sequence into a filename and waiting for a root user to list the directory is low --- what's the likelihood we will be crimping a user or shell script's style by forbidding the escape sequence in a filename? Most of these problems are really only an issue on time-sharing systems, so for people who are worried about these attacks --- they can enable an LSM, just as people who are willing to deal with the pain of SELinux can enable SELinux. :-) - Ted
On Fri, Oct 06, 2017 at 07:57:01AM -0700, Matthew Wilcox wrote: > On Fri, Oct 06, 2017 at 01:09:42PM +1100, Dave Chinner wrote: > > On Thu, Oct 05, 2017 at 12:16:19PM -0400, J. Bruce Fields wrote: > > > This kind of restriction sounds more like a permanent feature of the > > > filesystem--something you'd set at mkfs time. > > > > > > We already have filesystems with these kinds of restrictions, don't we? > > > > In general, no. Filename storage typically defined in the > > filesystem on-disk formats as an opaque string of bytes - the > > filesystem has no business parsing them to determine validity of the > > bytes. Think encrypted filenames and the like - control characters > > in the on-disk format are most definitely necessary and therefore > > must be legal. > > Umm. But filenames still can't have / or \0 in them, so your encryption > already has to avoid at least two special characters. Filesystems can have those characters on disk without any problems. Most filesytsems do not null terminate dirents on disk - instead they keep a dirent length on disk to determine the length of the entry. "Opaque" means null is a valid character, not an "end of string" delimiter. Keep in mind that "/" is an OS dependent special character - other OS use different directory delimiters so have a different set of "special characters". This reinforces the fact that it is not the filesystem's job to police what is stored on disk - the filesysetm is just a GIGO filename storage mechanism - you get out exactly what you put in... > I agree with your main point though; there is no advantage to doing this > in each individual filesystem. Yup, especially when we consider filesystems that get mounted and written by different OS and independent filesystem implementations.... Cheers, Dave.
diff --git a/fs/namei.c b/fs/namei.c index c75ea03ca147..8c6ed785fd49 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -2817,6 +2817,18 @@ static int may_delete(struct inode *dir, struct dentry *victim, bool isdir) return 0; } +/* Check for banned characters in file name. Called only on creation, as + * we need to allow removal/renaming/reading of existing stuff. + */ +static int is_bad_name(const char *name) +{ + const char *c; + for (c = name; *c; c++) + if ((1 <= *c && *c <= 31) || *c == 127) + return 1; + return 0; +} + /* Check whether we can create an object with dentry child in directory * dir. * 1. We can't do it if child already exists (open has special treatment for @@ -2834,6 +2846,8 @@ static inline int may_create(struct inode *dir, struct dentry *child) return -EEXIST; if (IS_DEADDIR(dir)) return -ENOENT; + if (is_bad_name(child->d_name.name)) + return -EACCES; s_user_ns = dir->i_sb->s_user_ns; if (!kuid_has_mapping(s_user_ns, current_fsuid()) || !kgid_has_mapping(s_user_ns, current_fsgid())) @@ -2996,6 +3010,9 @@ static int may_o_create(const struct path *dir, struct dentry *dentry, umode_t m if (error) return error; + if (is_bad_name(dentry->d_name.name)) + return -EACCES; + s_user_ns = dir->dentry->d_sb->s_user_ns; if (!kuid_has_mapping(s_user_ns, current_fsuid()) || !kgid_has_mapping(s_user_ns, current_fsgid()))
Anything with bytes 1-31,127 will get -EACCES. Especially \n is bad: instead of natural file-per-line, you need an user-unfriendly feature of -print0 added to every producer and consumer; a good part of users either don't know or don't feel the need to bother with escaping this snowflake, thus introducing security holes. The rest of control characters, while not as harmful, don't have a legitimate use nor have any real chance of coming from a tarball (malice and fooling around excluded). No character set ever supported as a system locale by glibc, and, TTBMK, by other popular Unices, includes them, thus it can be assumed no foreign files have such names other than artificially. This goes in stark contrast with other characters proposed to be banned: non-UTF8 is common, and even on my desktop's disk I found examples of all of: [ ], < >, initial -, initial and final space, ?, *, .., ', ", |, &. Somehow no \ anywhere. I think I have an idea why no / . Another debatable point is whether to -EACCES or to silently rename to an escaped form such as %0A. I believe the former is better because: * programs can be confused if a directory has files they didn't just write * many filesystems already disallow certain characters (like invalid Unicode), thus returning an error is consistent An example of a write-up of this issue can be found at: https://www.dwheeler.com/essays/fixing-unix-linux-filenames.html Signed-off-by: Adam Borowski <kilobyte@angband.pl> --- I really care mostly about \n but went bold and submitted a version with all ASCII control characters. If you guys disagree, please consider banning at least \n. fs/namei.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)