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>
> ---
> 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