ssh_key_contents isn't masked when using the ssh power driver

[Derek Higgins]

When getting node details, in most drivers the password/keys are masked to prevent them being displayed to the console and appearing in logs

When using the ssh power driver this isn't the case, on a development environment where virtual nodes are being used, the ssh private keys are logged in various places at various debug levels and when running "ironic node-show <uuid>" e.g.

$ ironic node-show baremetal-0
+------------------------+-----------------------------------------------------------------------+
| Property | Value |
+------------------------+-----------------------------------------------------------------------+
| chassis_uuid | |
| clean_step | {} |
| console_enabled | False |
| created_at | 2016-11-02T14:31:34+00:00 |
| driver | pxe_ssh |
| driver_info | {u'ssh_username': u'root', u'deploy_kernel': |
| | u'b6e8a5e6-90d0-4471-bc00-363db8d7705f', u'deploy_ramdisk': |
| | u'2b280e67-d3a0-42f6-b95b-417a5417eb2f', u'ssh_key_contents': u'----- |
| | BEGIN RSA PRIVATE KEY----- |
| | ............................................. |
| | ............................................. |
| | ............................................. |
| | ..........Removed for bug report............. |
| | ............................................. |
| | ............................................. |
| | ............................................. |
| | -----END RSA PRIVATE KEY-----', u'ssh_virt_type': |
| | u'virsh', u'ssh_address': u'192.168.XX.XX'} |
| driver_internal_info | {} |
| extra | {} |
| inspection_finished_at | None |
| inspection_started_at | None |
| instance_info | {} |
| instance_uuid | None |
| last_error | None |
| maintenance | False |
| maintenance_reason | None |
| name | baremetal-0 |
| network_interface | |
| power_state | power off |
| properties | {u'memory_mb': u'6144', u'cpu_arch': u'x86_64', u'local_gb': u'41', |
| | u'cpus': u'1', u'capabilities': u'boot_option:local'} |
| provision_state | available |
| provision_updated_at | 2016-11-02T14:32:07+00:00 |
| raid_config | |
| reservation | None |
| resource_class | |
| target_power_state | None |
| target_provision_state | None |
| target_raid_config | |
| updated_at | 2016-11-02T14:32:07+00:00 |
| uuid | 9a7b89d5-51c4-4017-8f63-6b0505a58242 |
+------------------------+-----------------------------------------------------------------------+

Flagging this as a security vulnerability as a precaution, but I'd imagine it doesn't need to be kept private as it would only effect development environments and its already reported publicly here
https://bugzilla.redhat.com/show_bug.cgi?id=1346089

[Jay Faulkner (jason-oldos)]

I believe the ssh_password key in driver_info for ssh driver would also be subject to this information disclosure and should also be masked.

[Jay Faulkner (jason-oldos)]

Thoughts on how to fix:

https://github.com/openstack/ironic/blob/c101fce33ab36605c6826e4ac354d27f0930cb8c/ironic/api/controllers/v1/node.py#L837 is an example of where we already block out an ironic-specific key from an API response. While I'm aware we use Oslo.utils mask_dict_password() to block out most keys, in this case for ease of backporting, and also keeping in mind that the ssh driver is being deprecated, I think we should just add if statements similar to what we do for image_url into node.py. It would be a simple, easy to backport change that would resolve this.

[Derek Higgins (derekh)]

Thanks Jay, I'll put together a patch for this, given that this has already been reported publicly does it need to be kept private?

[Jim Rollenhagen (jim-rollenhagen)]

Looks like a class B3 to me: https://security.openstack.org/vmt-process.html#incident-report-taxonomy, which doesn't require an advisory. Also, since it's already public, no need to keep it private here.

Confirmed this with fungi.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/400273
Committed: https://git.openstack.org/cgit/openstack/ironic/commit/?id=ca585bec9d6973a89f6f6b5788c783affe9621a3
Submitter: Jenkins
Branch: master

commit ca585bec9d6973a89f6f6b5788c783affe9621a3
Author: Derek Higgins <email address hidden>
Date: Mon Nov 21 13:57:20 2016 +0000

    mask private keys for the ssh power driver.

    As this driver is deprecated masking here (opposed to strutils)
    is simpler, and easier to backport. This can be removed along with
    support for the ssh power driver.

    Change-Id: I107f2ce4ee2cd22558455de7ed595c2b3a7c6845
    Closes-Bug: #1638596

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/newton
Review: https://review.openstack.org/401160

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/401160
Committed: https://git.openstack.org/cgit/openstack/ironic/commit/?id=94f132e71144210ac0dbc8b71f6c741de7dc98fd
Submitter: Jenkins
Branch: stable/newton

commit 94f132e71144210ac0dbc8b71f6c741de7dc98fd
Author: Derek Higgins <email address hidden>
Date: Mon Nov 21 13:57:20 2016 +0000

    mask private keys for the ssh power driver.

    As this driver is deprecated masking here (opposed to strutils)
    is simpler, and easier to backport. This can be removed along with
    support for the ssh power driver.

    Closes-Bug: #1638596
    Change-Id: I107f2ce4ee2cd22558455de7ed595c2b3a7c6845
    (cherry picked from commit ca585bec9d6973a89f6f6b5788c783affe9621a3)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/ironic 7.0.0 release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/ironic 6.2.3 release.