https://bugzilla.redhat.com/show_bug.cgi?id=1107420

As unusual as the case may be, having a name of all white space
has been allowed. This leads to the obvious problem of how a future
usage would be able to "utilize" that resource since it's not "simple"
to determine what combination of spaces and tabs are being used for
the name.

So add code to various drivers in order to inhibit this for those
resources that would consider using a name for some sort of lookup
type reference. For a majority of drivers, the schema validation also
allowed the name; however, a couple (secrets and nwfilters) the
virt-xml-validate would fail. Still this wouldn't necessarily stop
someone - just slow them down. So for those the test xml file must
use the *-invalid.xml type syntax to inhibit virschematest from
looking at the schema.

Similar validation for node devices and interfaces is not necessary
since those are referencing system named elements which wouldn't have
the oddly named resource.  Also, while it could be checked, avoid the
check for Storage Volumes as there's no way to "inhibit" in the parse
logic algorithm.

This is not for 4.6.0, but figured I'd get it on list for consideration
for 4.7.0. Yes, it's quite unusual and one has to wonder who thinks
up this type of situation, but still in the long run we should attempt
to avoid it anyway. I've only added the extra checking for qemu and
lxc domains, leaving others to those maintainers to choose to utilize.

John Ferlan (11):
  conf: Add @flags to Storage Pool Def processing
  conf: Disallow new storage pools to use all white space as name
  conf: Add @flags to Network Def processing
  conf: Disallow new networks to use all white space as name
  conf: Disallow new qemu,lxc domains to use all white space as name
  conf: Split and rename secretXMLParseNode
  conf: Add @flags to Secret Def processing
  conf: Disallow new secrets to use all white space as usage id
  conf: Add @flags to NWFilter Def processing
  conf: Disallow new nwfilters to use all white space as name
  conf: Disallow new snapshots to use all white space as name

 src/conf/domain_conf.c                        | 11 ++-
 src/conf/domain_conf.h                        |  4 +
 src/conf/network_conf.c                       | 34 +++++---
 src/conf/network_conf.h                       | 19 ++++-
 src/conf/nwfilter_conf.c                      | 32 +++++---
 src/conf/nwfilter_conf.h                      | 16 +++-
 src/conf/secret_conf.c                        | 79 +++++++++++++------
 src/conf/secret_conf.h                        | 16 +++-
 src/conf/snapshot_conf.c                      |  7 ++
 src/conf/snapshot_conf.h                      |  1 +
 src/conf/storage_conf.c                       | 32 +++++---
 src/conf/storage_conf.h                       | 19 ++++-
 src/conf/virnetworkobj.c                      |  4 +-
 src/conf/virnwfilterobj.c                     |  2 +-
 src/conf/virsecretobj.c                       |  2 +-
 src/conf/virstorageobj.c                      |  4 +-
 src/esx/esx_network_driver.c                  |  2 +-
 src/lxc/lxc_driver.c                          |  6 +-
 src/network/bridge_driver.c                   |  6 +-
 src/nwfilter/nwfilter_driver.c                |  3 +-
 src/phyp/phyp_driver.c                        |  2 +-
 src/qemu/qemu_driver.c                        |  9 ++-
 src/qemu/qemu_process.c                       |  2 +-
 src/secret/secret_driver.c                    |  3 +-
 src/storage/storage_driver.c                  |  6 +-
 src/test/test_driver.c                        | 12 +--
 src/vbox/vbox_network.c                       |  2 +-
 .../name_whitespace.xml                       |  3 +
 tests/domainsnapshotxml2xmltest.c             | 38 ++++++---
 tests/networkxml2conftest.c                   |  2 +-
 tests/networkxml2firewalltest.c               |  2 +-
 .../network-whitespace-name.xml               |  6 ++
 tests/networkxml2xmltest.c                    |  4 +-
 tests/networkxml2xmlupdatetest.c              |  2 +-
 tests/nwfilterxml2firewalltest.c              |  2 +-
 .../name-whitespace-invalid.xml               |  4 +
 tests/nwfilterxml2xmltest.c                   |  7 +-
 tests/qemuxml2argvdata/name-whitespace.xml    | 29 +++++++
 tests/qemuxml2argvtest.c                      |  5 +-
 .../usage-whitespace-invalid.xml              |  7 ++
 tests/secretxml2xmltest.c                     | 30 +++++--
 tests/storagebackendsheepdogtest.c            |  4 +-
 .../pool-dir-whitespace-name.xml              | 18 +++++
 tests/storagepoolxml2xmltest.c                | 45 ++++++++---
 tests/storagevolxml2argvtest.c                |  4 +-
 tests/storagevolxml2xmltest.c                 |  2 +-
 46 files changed, 418 insertions(+), 131 deletions(-)
 create mode 100644 tests/domainsnapshotxml2xmlin/name_whitespace.xml
 create mode 100644 tests/networkxml2xmlin/network-whitespace-name.xml
 create mode 100644 tests/nwfilterxml2xmlin/name-whitespace-invalid.xml
 create mode 100644 tests/qemuxml2argvdata/name-whitespace.xml
 create mode 100644 tests/secretxml2xmlin/usage-whitespace-invalid.xml
 create mode 100644 tests/storagepoolxml2xmlin/pool-dir-whitespace-name.xml

-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Mon, Jul 30, 2018 at 02:46:37PM -0400, John Ferlan wrote:
>https://bugzilla.redhat.com/show_bug.cgi?id=1107420
>
>As unusual as the case may be, having a name of all white space
>has been allowed. This leads to the obvious problem of how a future
>usage would be able to "utilize" that resource since it's not "simple"
>to determine what combination of spaces and tabs are being used for
>the name.
>

So if the user wants it difficult, they can do so.
What do we gain by forbidding it?

> 46 files changed, 418 insertions(+), 131 deletions(-)
> create mode 100644 tests/domainsnapshotxml2xmlin/name_whitespace.xml
> create mode 100644 tests/networkxml2xmlin/network-whitespace-name.xml
> create mode 100644 tests/nwfilterxml2xmlin/name-whitespace-invalid.xml
> create mode 100644 tests/qemuxml2argvdata/name-whitespace.xml
> create mode 100644 tests/secretxml2xmlin/usage-whitespace-invalid.xml
> create mode 100644 tests/storagepoolxml2xmlin/pool-dir-whitespace-name.xml
>

(apart from extra 300 lines)

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 07/31/2018 04:28 AM, Ján Tomko wrote:
> On Mon, Jul 30, 2018 at 02:46:37PM -0400, John Ferlan wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1107420
>>
>> As unusual as the case may be, having a name of all white space
>> has been allowed. This leads to the obvious problem of how a future
>> usage would be able to "utilize" that resource since it's not "simple"
>> to determine what combination of spaces and tabs are being used for
>> the name.
>>
> 
> So if the user wants it difficult, they can do so.
> What do we gain by forbidding it?

	  	  			   		

> 
>> 46 files changed, 418 insertions(+), 131 deletions(-)
>> create mode 100644 tests/domainsnapshotxml2xmlin/name_whitespace.xml
>> create mode 100644 tests/networkxml2xmlin/network-whitespace-name.xml
>> create mode 100644 tests/nwfilterxml2xmlin/name-whitespace-invalid.xml
>> create mode 100644 tests/qemuxml2argvdata/name-whitespace.xml
>> create mode 100644 tests/secretxml2xmlin/usage-whitespace-invalid.xml
>> create mode 100644
>> tests/storagepoolxml2xmlin/pool-dir-whitespace-name.xml
>>
> 
> (apart from extra 300 lines)
> 
> Jano

Since when has the number of insertions/deletions become the determining
factor of what is and isn't a problem? I could drop the tests and shave
a few off. Skipping a few comments along the way may pick up a few more.

As long as one "knows" to list certain objects using 'virsh xxx-list
--uuid', then they can manipulate the white space named object once they
figure out which UUID is associated with their white space named
resource. Still snapshot's don't provide that, so they'd be a problem.
At least nwfilter and secrets are in your face with the UUID. Domains,
networks, and pools just make you work a bit harder.

If you don't think this is a problem, then grab/own the bz and close it.

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list