On Wed, Jan 21, 2015 at 03:45:32PM -0800, David Lutterkort wrote: > On Wed, Jan 21, 2015 at 1:15 PM, Richard W.M. Jones <[email protected]> > wrote: > > > On Wed, Jan 21, 2015 at 12:59:40PM -0800, David Lutterkort wrote: > > > The thing that makes me nervous the most about this change is that > > > it changes the paths that people get back from Augeas, especially > > > from aug_match. > > > > [I think the first thing to say is that we only care about the API, not > > about augtool.] > > > > I've probably not understood the full implications of this. > > > > Programs like libvirt and virt-v2v use the aug_match a lot, and in > > some cases pass those strings back to aug_get, aug_set, aug_rm. There > > are many examples of this in the following file (search for > > "aug_match"): > > > > With this change, taking paths returned from aug_match and feeding them > into aug_get, aug_set, aug_rm etc. now actually becomes safe: before, > aug_match would return paths that really needed escaping, but weren't so > that passing them to aug_get could fail. This will no longer be an issue. > > https://github.com/libguestfs/libguestfs/blob/master/v2v/convert_linux.ml > > > > For example: > > > > let expr = > > sprintf "/file/etc/sysconfig/kernel/%s/value[. = '%s']" > > var xen_mod in > > let entries = g#aug_match expr in > > let entries = Array.to_list entries in > > if entries <> [] then ( > > List.iter (fun e -> ignore (g#aug_rm e)) entries; > > modified := true > > ) > > > > Yes, this will now work, assuming you replace 'var' in there with > '(g#aug_escape_name var)' > > > > Our real concern is where strings get interpolated into an Augeas > > expression, especially if those string come from untrusted user input > > (which is not the case in that file, but could be in general). > > > > As long as these strings go through aug_escape_name before sticking them > into a path, all should be well. > > > > > While those can now be directly fed to aug_get, they can no longer > > > be used to find the underlying file directly. Not sure if that will > > > cause problems for anybody. > > > > I'm not really sure what this means. What is "underlying file" in > > this context? Would it affect code like the above? Can you give an > > example of a problem case? > > > Sorry, this was a bit obtuse: before this change, you could take a path > returned by aug_match, strip off '/files' from the beginning and use the > rest as the path to the actual file in the filesystem. After this change, > that's no longer the case. If you have a file '/etc/weird-[a]' in the > filesystem, an aug_match for something in that file will return > '/files/etc/weird-\[a\]' - you can now pass that back to aug_get, but you > can't use that to open the actual file (because of the extra '\' in there).
All sounds good to me. Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html _______________________________________________ augeas-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/augeas-devel
