cinder drivers using ETree for xml parsing

[Nicholas Jones]

Bandit blacklists calls to xml.etree for security purposes
see: http://docs.openstack.org/developer/bandit/blacklists/blacklist_calls.html#b313-b320-xml

Calls to xml.etree are made in the following locations:

Location: cinder/cinder/volume/drivers/fujitsu/eternus_dx_common.py:1102
Location: cinder/cinder/volume/drivers/hitachi/hnas_utils.py:204
Location: cinder/cinder/volume/drivers/huawei/huawei_conf.py:43
Location: cinder/cinder/volume/drivers/huawei/huawei_conf.py:85
Location: cinder/cinder/volume/drivers/qnap.py:562
Location: cinder/cinder/volume/drivers/qnap.py:594
Location: cinder/cinder/volume/drivers/qnap.py:829
Location: cinder/cinder/volume/drivers/qnap.py:902
Location: cinder/cinder/volume/drivers/qnap.py:951
Location: cinder/cinder/volume/drivers/qnap.py:975
Location: cinder/cinder/volume/drivers/qnap.py:999
Location: cinder/cinder/volume/drivers/qnap.py:1028
Location: cinder/cinder/volume/drivers/qnap.py:1036
Location: cinder/cinder/volume/drivers/qnap.py:1060
Location: cinder/cinder/volume/drivers/qnap.py:1084
Location: cinder/cinder/volume/drivers/qnap.py:1105
Location: cinder/cinder/volume/drivers/qnap.py:1124
Location: cinder/cinder/volume/drivers/qnap.py:1142
Location: cinder/cinder/volume/drivers/qnap.py:1161
Location: cinder/cinder/volume/drivers/qnap.py:1203
Location: cinder/cinder/volume/drivers/qnap.py:1236
Location: cinder/cinder/volume/drivers/qnap.py:1253
Location: cinder/cinder/volume/drivers/qnap.py:1278
Location: cinder/cinder/volume/drivers/qnap.py:1306
Location: cinder/cinder/volume/drivers/qnap.py:1334
Location: cinder/cinder/volume/drivers/qnap.py:1375
Location: cinder/cinder/volume/drivers/qnap.py:1408
Location: cinder/cinder/volume/drivers/qnap.py:1428
Location: cinder/cinder/volume/drivers/qnap.py:1453
Location: cinder/cinder/volume/drivers/qnap.py:1490
Location: cinder/cinder/volume/drivers/qnap.py:1523
Location: cinder/cinder/volume/drivers/qnap.py:1531
Location: cinder/cinder/volume/drivers/qnap.py:1548

[Jeremy Stanley (fungi)]

Since this report concerns a possible security risk, an incomplete security advisory task has been added while the core security reviewers for the affected project or projects confirm the bug and discuss the scope of any vulnerability along with potential solutions.

[Jeremy Stanley (fungi)]

This report doesn't include any useful research, just a copy-and-paste report from a static code analyzer. Can you clarify what actual vulnerabilities you've discovered and provide some realistic exploit scenarios? The recommendation to switch to defusedxml has been debated heavily (and rejected) elsewhere in OpenStack as recently as a few months ago: http://lists.openstack.org/pipermail/openstack-dev/2016-September/104630.html

Given that thread explicitly mentions "cinder drivers" I recommend we switch this bug to Public Security since an embargo is pointless.

[Sean McGinnis (sean-mcginnis)]

I agree with Jeremy.

Also, the majority of the use of this is by drivers for parsing XML to and from their device API. So I don't think "parse untrusted XML data is known to be vulnerable to XML attacks" applies here.

[Nicholas Jones (nj762h)]

This is 100% just me deferring to higher powers in an attempt to either address or eliminate issues raised by bandit.

If the XML in the drivers respective configuration files is guaranteed to be secure then there's likely no issue.

[Jeremy Stanley (fungi)]

I've switched the bug to Public Security now so this discussion can continue as publicly as we like.

[Arun Kant (arukant)]

For details of xml attacks and possible replacement, this package can be referred which mentions about xml related various attack vectors.

https://pypi.python.org/pypi/defusedxml

[Tristan Cacqueray (tristan-cacqueray)]

The VMT doesn't usually issue advisory for bug only exposed in the backend network, unless this can be abused by regular user. I propose we treat this as a class C1 report ( https://security.openstack.org/vmt-process.html#incident-report-taxonomy ).

[Jeremy Stanley (fungi)]

Probably more of a class E unless it can be demonstrated that there is any actual vulnerability exposed (or could barely be class D if there's interest from Cinder in anyone working on a "safer" replacement implementation). Also since it's already public I've gone ahead and marked our security advisory task invalid.