severity 597778 important thanks On Wed, Sep 22, 2010 at 05:21:15PM -0400, Len Sorensen wrote: > Package: schroot > Version: 1.4.12-1 > Severity: grave > > I just upgraded schroot to the current version in testing, and now I > can't use it anymore because someone got the bright (not) idea that only > alphanumeric, dashes and underscores should be allowed in chroot names. > It was annoying enough when a while ago the config files started having > that (undocumented unless you read the source code and without useful > error message) restriction. > > Put it back to sanity please. > > If I want to name a chroot rr2.1.0 because that's what is in it, then > that's my choice. > > While you are at it, let me choose the names of my config files too. > > Breaking existing users is NOT acceptable so stop doing that.
While this change is clearly an annoyance to yourself, the severity is certainly not grave, so I am changing it accordingly. If the change was causing irretrievable data loss, then the grave severity would have been appropriate. Your comments are also rather aggressive. Please keep to the facts, rather than using unnecessarily inflammatory language. It's not helpful, and is most certainly *not* appreciated. Bug reports should have the goal of improving the software, not abusing its developers. Please bear that in mind, both in any replies to this email, and in any future bug reports. Regarding anything undocumented: if something is undocumented that you need to know about, that's a bug, and if there's an error message that's not clear, that's also a bug. Please do file (separate) bug reports about any such incidence and I'll be happy to improve the documentation. Remember that programs such as schroot are documented by their authors and we already know every aspect of the program inside and out. Quality documentation is of course a goal we have, but it's an unfortunate reality that it's easy to inadvertently miss out details, especially when you are writing for someone who isn't intimately familiar with the program. Making documentation that is easily accessible to newcomers that contains exactly what they need to know is a rare skill, and while I have done a lot of work on the manual pages, I am fully aware that the documentation could be much better. But what I really need to do that is *feedback* about what wasn't there that was needed, and what was there that was unnecessary, etc.. Now, to the main point of your report. schroot imposes certain restrictions upon what it considers to be a permitted filename when reading files under /etc/schroot/chroot.d and also what is permitted to be a valid chroot and/or session name. Since the chroot name is used to create session files bearing the same name, the rules used to validate filenames therefore may also have a concomitant restriction upon session names. Before adding stricter validation of chroot names, it was possible to create names which could lead to sessions which were not possible to end, or which might potentially lead to an exploit (naming your chroot "../../../../etc/xxx/xxx could have some interesting consequences, as a simple example), so the primary motivation behind this change was to improve security. Additional restrictions are also placed by other components we depend upon (for example, if LVM logical volumes and Btrfs subvolumes and snapshots have their own naming restrictions). The imposed restrictions do actually prevent breakage and/or security issues in a number of scenarios. The second motivation behind the change was to provide a single validation mechanism for all names, again with the goal of improving security and reliability. You can find this function in sbuild/sbuild-util.cc, called is_valid_filename(). Having made this change, we have now added a second function called is_valid_sessionname() which is used in places which only need to validate names, which has relaxed requirements compared with the filename checks. static regex file_namespace("^[a-zA-Z0-9][a-zA-Z0-9_-]*$"); Now, the intention has always been to further relax this requirement and allow a wider variety of names. I am well aware that this is in some respects a "regression". However, you must understand that schroot is a suid root executable and the *primary* concern is and always has been security. If you wish to "fix" the naming restrictions, all you need to do is alter that regular expression. I will make that change once I have *fully evaluated the security implications of doing so*, and not before. It may be that "^[^/]$" is sufficient, but we need to be sure of that. The chroot/session name is used in quite a lot of places and for many different purposes, and each of those needs careful consideration. So to summarise: • this change was entirely intentional • it was made to prevent session breakage and security exploits • we have put the infrastructure in place to separately validate filenames and sesssion names • we will allow more permissive chroot naming once the security implications have been throughly evaluated, and the code audited • you are free to modify the source to make this change yourself, at entirely your own risk Regards, Roger -- .''`. Roger Leigh : :' : Debian GNU/Linux http://people.debian.org/~rleigh/ `. `' Printing on GNU/Linux? http://gutenprint.sourceforge.net/ `- GPG Public Key: 0x25BFB848 Please GPG sign your mail.
signature.asc
Description: Digital signature