> -----Original Message----- > From: Aaron Conole [mailto:[email protected]] > Sent: Tuesday, April 12, 2016 1:12 AM > To: Ben Pfaff <[email protected]> > Cc: [email protected]; Flavio Leitner <[email protected]>; Traynor, > Kevin <[email protected]>; Panu Matilainen <[email protected]>; > Wojciechowicz, RobertX <[email protected]>; Mooney, Sean K > <[email protected]>; Andy Zhou <[email protected]>; Daniele Di > Proietto <[email protected]>; Zoltan Kiss <[email protected]>; > Christian Ehrhardt <[email protected]> > Subject: Re: [PATCH v11 2/8] util: Add a path canonicalizer > > Hi Ben, > > Ben Pfaff <[email protected]> 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 <[email protected]> > > > > 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. [Mooney, Sean K] currently it is possible to use any dir for the vhost-socket dir if the absolute path is specified on the ovs-vswitchd command line correct? I know for a time we used /tmp/ for the sockets. I have also seen /dev/vhostuser/ used. The run dir (/var/run/openvswitch or /usr/var/run/openvswitch) makes the most sense To me as a default.
More of a clarifying question but are you proposing restricting the localtions where the vhost-user sockets can be stored or constraining the path format to allow/deny relative path specifiers such as "." and ".." and converting that path in an canonical absolute path? I don't have a strong preference but just wanted to make sure I understand what is being proposed And what flexibility we would be gaining/losing. > > I do agree it's heavy. > > > Thanks, > > Thanks for the review! > > > Ben. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
