I think I'd prefer to avoid path canonicalization. If it proves not flexible enough, we can add it in later.
Thanks, Ben. On Wed, Apr 13, 2016 at 12:08:58PM -0400, Aaron Conole wrote: > Hi Ben, > > I have rebased (due to conflicts) the series and am set to resubmit; > however I'd like to get closure on this issue before I do. > > I have no strong feelings either way - I can manually scan the string > for .. or use the realpath code I've put here. Since this only happens > at initialization time, I figured it didn't matter how heavy it was, > but that may not be the case (especially due to where initialization > occurs). > > Please advise what you would rather see - I can make the changes and test > with them. > > Thanks :-) > -Aaron > > Aaron Conole <acon...@redhat.com> writes: > > > Hi Ben, > > > > Ben Pfaff <b...@ovn.org> writes: > > > >> (Yow, that's a lot of CCs.) > > > > Lots of cooks in the kitchen on this one. > > > >> On Fri, Apr 01, 2016 at 11:31:31AM -0400, Aaron Conole wrote: > >>> This commit adds a new function (ovs_realpath) to perform the role of > >>> realpath on various operating systems. The purpose is to ensure that a > >>> given path to file exists, and to return a completely resolved path (sans > >>> '.' and '..'). > >>> > >>> Signed-off-by: Aaron Conole <acon...@redhat.com> > >> > >> Path canonicalization is a pretty big hammer. In other cases where OVS > >> relies on an absolute path, it uses textual comparison on a prefix of > >> the name (representing a directory) and rejects use of ".." following > >> the prefix. This is pretty easy to get right, and it is not as > >> heavyweight, and it does not have to actually do file system operations > >> (stat, readlink, ...), and its verdict can't change as a result of > >> changes to the file system (e.g. new or modified or deleted symlinks, > >> NFS servers that are down), and so on. > >> > >> Do you think we really need path canonicalization? > > > > I was nervous about a user putting escapes in the code. Unlike with, > > say, vhost user filenames (where we just blanket deny '/' because the > > semantic is of a file) this is not a file specification, but a directory > > specification. That implies that we would have to keep state and test > > for /../, and ../ (at the beginning of the string), at the least. > > > > If you think it's safe to merely test for the presence of these and > > bail, I'm okay with it, but I didn't want to leave any possibility that > > a malicious DB user could escape out of the rundir when changing the > > vhost-socket dir. > > > > I do agree it's heavy. > > > >> Thanks, > > > > Thanks for the review! > > > >> Ben. > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
