Re: [libvirt] [PATCH] docs, rng: Allow a pool name to be line domain name

On 09/28/2017 09:47 AM, Peter Krempa wrote: > On Wed, Sep 27, 2017 at 15:07:35 -0400, John Ferlan wrote: >> https://bugzilla.redhat.com/show_bug.cgi?id=1475250 >> >> It's possible to define and start a pool with a '.' in the >> name; however, when trying to add a volume to a domain using >> the storage pool source with a name with a '.' in the name, >> the domain RNG validation fails because RNG uses 'genericName' >> which does not allow a '.' in the name. Pool definition has >> no similar call to virXMLValidateAgainstSchema. Pool name >> validation occurs in storagePoolDefineXML and only calls >> virXMLCheckIllegalChars using the same parameter "\n" as >> qemuDomainDefineXMLFlags would check after the RNG check >> could be succesful. >> >> So in order to resolve this, create a poolName definition >> in the RNG and allow the pool name and the volume source >> pool name to use that definition. >> >> Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; >> --- >> docs/schemas/domaincommon.rng | 2 +- >> docs/schemas/storagecommon.rng | 8 ++++++++ >> docs/schemas/storagepool.rng | 4 ++-- >> 3 files changed, 11 insertions(+), 3 deletions(-) >> >> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng >> index 76852abb3..2cc8dcecf 100644 >> --- a/docs/schemas/domaincommon.rng >> +++ b/docs/schemas/domaincommon.rng >> @@ -1669,7 +1669,7 @@ >> <optional> >> <element name="source"> >> <attribute name="pool"> >> - <ref name="genericName"/> >> + <ref name="poolName"/> >> </attribute> >> <attribute name="volume"> >> <ref name="volName"/> >> diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng >> index 717f3c603..49578312e 100644 >> --- a/docs/schemas/storagecommon.rng >> +++ b/docs/schemas/storagecommon.rng >> @@ -6,6 +6,14 @@ >> <!-- This schema is not designed for standalone use; another file >> must include both this file and basictypes.rng --> >> >> + <define name="poolName"> >> + <data type="string"> >> + <!-- Use literal newline instead of \n for bug in libxml2 2.7.6 --> >> + <param name="pattern">[^ >> +]+</param> >> + </data> >> + </define> >> + >> <define name='encryption'> >> <element name='encryption'> >> <attribute name='format'> >> diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng >> index f0117bd69..52b2044be 100644 >> --- a/docs/schemas/storagepool.rng >> +++ b/docs/schemas/storagepool.rng >> @@ -209,7 +209,7 @@ >> <interleave> >> <optional> >> <element name='name'> >> - <ref name='genericName'/> >> + <ref name='poolName'/> > > This means that a name starting with a dot is invalid according to the > schema, but the user ignored the schema and the code is not doing enough > validation. > > I'm not convinced that this is the correct solution. VMs disallow dots > since the name is used for generating filenames and using '../' as > prefix will allow directory traversal exploits. > > NACK, I think we should disallow pool names with a dot even in the code. > It will be slightly harder since there are no 'validate' callbacks for > them and you can't disallow them in the parser. > As a test I defined a domain and a pool using ".test" and it worked. # virsh list Id Name State ---------------------------------------------------- 1 .f23 running # virsh dumpxml .f23 <domain type='kvm' id='1'> <name>.f23</name> <uuid>e28761fe-ea53-4682-924a-744a4028f146</uuid> <memory unit='KiB'>2097152</memory> ... The problem with disallowing in code after the fact is how do you handle things now that we have allowed it? I have another long standing bug where someone doesn't want a space in the name or even worse a space as the name.