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

[John Ferlan]

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 at redhat.com>
>> ---
>>  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.

John