[RFE] SNMP driver does not implement security features

Bug #1710850 reported by Ilya Etingof on 2017-08-15
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Ironic
In Progress
Wishlist
Ilya Etingof

Bug Description

The Ironic SNMP driver does not leverage the security features that SNMPv3 offers. Supporting them would improve the security of the management network whenever SNMP is used for power management.

Since the underlying library (pysnmp) does all the SNMPv3 heavy lifting, at the level of the Ironic SNMP driver it is merely a matter of adding additional configuration parameters to utilize all the SNMPv3 security features. Then the operator could configure SNMP driver to use either SNMP v1, v2c or v3 by way of setting the `snmp_version` option (which is already present).

SNMPv3-related options to be introduced would be:

* snmp_usm_user: user name (string)
* snmp_usm_auth_protocol: tokens like md5, sha
* snmp_usm_auth_key: ascii string
* snmp_usm_priv_protocol: tokens like des, aes
* snmp_usm_priv_key: ascii string
* snmp_context_name: SNMP context (string) to address possibly many instances of the same MIB behind a single SNMP agent

Implementation-wise, we would not need to fork Ironic SNMP driver or introduce new hardware type or migrate existing SNMPv1/v2c configuration by way of adding SNMPv3 support to the SNMP driver.

The same applies to the VirtualPDU tool -- we will need to teach it picking up the options above [1], no backward incompatible changes anticipated.

The proposed change would introduce the indirect dependency on the `pycryptodomex` package which pysnmp (4.4.1+) uses for the low-level crypto operations.

1. https://github.com/openstack/virtualpdu/blob/master/virtualpdu/main.py#L51

Dmitry Tantsur (divius) on 2017-08-21
tags: added: snmp
Changed in ironic:
status: New → Confirmed
importance: Undecided → Wishlist
Ilya Etingof (etingof) on 2017-08-21
description: updated
Ruby Loo (rloo) wrote :

Do we need a spec for this? I'd be fine if the additional config parameters were described here and they aren't controversial :)

wrt 'pycryptodome', I see this note [1], but I don't think we are using pycrypto? (someone should verify of course)

[1] https://github.com/openstack/requirements/blob/a5892e231bdf80f6b36348b8a1dee0ae09d4fb15/global-requirements.txt#L224

Ilya Etingof (etingof) wrote :

I'd be fine if we implement this feature without a dedicated spec.

SNMPv3-related options would be:

* snmp_usm_user: user name (string)
* snmp_usm_auth_protocol: tokens like md5, sha
* snmp_usm_auth_key: ascii string
* snmp_usm_priv_protocol: tokens like des, aes
* snmp_usm_priv_key: ascii string
* snmp_context_name: SNMP context (string) to address possibly many instances of the same MIB behind a single SNMP agent

I do not see that any of the Ironic projects rely on PyCrypto. But there is the PyCryptodomex (notice `x` at the end) which installs under its own name (Cryptodome), not under the name of Crypto like PyCryptodome (without x) does being a drop-in replacement for PyCrypto.

Since the newer pysnmp (4.4.x) has switched to PyCryptodomex, Ironic can theoretically introduce the new PyCryptodomex dependency which could co-exist with PyCrypto/PyCryptodome [1] so that the rest of the OpenStack projects to eventually migrate to PyCryptodomex.

1. https://www.pycryptodome.org/en/latest/src/installation.html

Ruby Loo (rloo) wrote :

This was discussed in the ironic meeting [1] and Dmitry, Julia are good with approving this without a spec, although the issues discussed in the meeting need to be mentioned here. I suggest updating the description to include the answers to the questions we asked, then ping us to approve this RFE. Thanks!

[1] http://eavesdrop.openstack.org/meetings/ironic/2017/ironic.2017-11-20-17.00.log.html#l-212

Ilya Etingof (etingof) on 2017-11-22
description: updated
Ruby Loo (rloo) wrote :

Approving this. Thanks!

Note that we need this tested in gate.

tags: added: rfe-approved
removed: rfe
Ilya Etingof (etingof) on 2017-11-24
Changed in ironic:
assignee: nobody → Ilya Etingof (etingof)
Ilya Etingof (etingof) on 2018-02-16
description: updated

Fix proposed to branch: master
Review: https://review.openstack.org/547999

Changed in ironic:
status: Confirmed → In Progress
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers