CellMappingPayload in select_destinations versioned notification sends sensitive database_connection and transport_url information

Bug #1823104 reported by Matt Riedemann on 2019-04-04
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
High
Matt Riedemann
Stein
High
Eric Fried

Bug Description

As of this change in Stein:

https://review.openstack.org/#/c/508506/28/nova/notifications/objects/request_spec.py@334

Which is not yet officially released, but is in the 19.0.0.0rc1, the select_destinations versioned notification payload during a move operation (resize, cold/live migrate, unshelve, evacuate) will send the cell database_connection URL and MQ transport_url information which contains credentials to connect directly to the cell DB and MQ, which even though notifications are meant to be internal within openstack services, seems like a pretty bad idea. IOW, just because it's internal to openstack doesn't mean nova needs to give ceilometer the keys to it's cell databases.

There seems to be no justification in the change for *why* this information was needed in the notification payload, it seemed to be added simply for completeness.

Matt Riedemann (mriedem) on 2019-04-04
Changed in nova:
assignee: nobody → Matt Riedemann (mriedem)

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

Changed in nova:
status: Triaged → In Progress
Jeremy Stanley (fungi) wrote :

Thanks for spotting, it would be great to get this into stable/stein before release.

Reviewed: https://review.openstack.org/649775
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=3301449e736af81fdbd96fd3f396dd01b02beaf2
Submitter: Zuul
Branch: master

commit 3301449e736af81fdbd96fd3f396dd01b02beaf2
Author: Matt Riedemann <email address hidden>
Date: Wed Apr 3 22:09:52 2019 -0400

    Remove CellMappingPayload database_connection and transport_url fields

    Change I019e88fabd1d386c0d6395a7b1969315873485fd in Stein, which
    is not yet officially released, exposes the unencrypted
    database_connection URL and MQ transport_url to a CellMapping in
    the select_destinations versioned notification CellMappingPayload.

    While notifications are not meant to be consumed by end users of
    the cloud but only internal services of the deployment, it still
    seems like a bad idea to give the keys to the nova cell DB and MQ
    to an external-to-nova service like ceilometer.

    This change removes the fields from the CellMappingPayload and
    bumps the major version to 2.0 to signal the change to consumers,
    although I don't expect anything is consuming this yet but we should
    follow standard versioning procedure anyway.

    Note that notification consumers do not request a specific payload
    version nor do they get a schema to perform their own backporting,
    they just get what they get, so after this there should be no worry
    about needing to support the 1.0 format for this payload.

    Change-Id: Ib5edea32d15db01000e6730aebceaf119daf8c5c
    Closes-Bug: #1823104

Changed in nova:
status: In Progress → Fix Released

Reviewed: https://review.openstack.org/650020
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=88560094450bb4935909793965866b542d9ef3fe
Submitter: Zuul
Branch: master

commit 88560094450bb4935909793965866b542d9ef3fe
Author: Matt Riedemann <email address hidden>
Date: Thu Apr 4 10:20:32 2019 -0400

    Add docs on what not to include in notifications

    Based on bug 1823104 it's clear we should have some
    explicit wording in the notification reference docs
    about what not to include in versioned notification
    payloads, so this change attempts to start that with
    the most obvious thing - don't expose access credentials
    to the nova deployment.

    This also adds a reminder to think about what is being
    added / mirrored from internal objects and determine if
    consumers really need it and if they aren't asking, opt
    to not including it until requested.

    Change-Id: I326aa39d963091282a5d0b70ba222abfe8ccfdac
    Related-Bug: #1823104

Reviewed: https://review.openstack.org/650043
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=afe5f0d4de6d30f119c89c2cfd1bdf0f7c5b3de9
Submitter: Zuul
Branch: stable/stein

commit afe5f0d4de6d30f119c89c2cfd1bdf0f7c5b3de9
Author: Matt Riedemann <email address hidden>
Date: Wed Apr 3 22:09:52 2019 -0400

    Remove CellMappingPayload database_connection and transport_url fields

    Change I019e88fabd1d386c0d6395a7b1969315873485fd in Stein, which
    is not yet officially released, exposes the unencrypted
    database_connection URL and MQ transport_url to a CellMapping in
    the select_destinations versioned notification CellMappingPayload.

    While notifications are not meant to be consumed by end users of
    the cloud but only internal services of the deployment, it still
    seems like a bad idea to give the keys to the nova cell DB and MQ
    to an external-to-nova service like ceilometer.

    This change removes the fields from the CellMappingPayload and
    bumps the major version to 2.0 to signal the change to consumers,
    although I don't expect anything is consuming this yet but we should
    follow standard versioning procedure anyway.

    Note that notification consumers do not request a specific payload
    version nor do they get a schema to perform their own backporting,
    they just get what they get, so after this there should be no worry
    about needing to support the 1.0 format for this payload.

    Change-Id: Ib5edea32d15db01000e6730aebceaf119daf8c5c
    Closes-Bug: #1823104
    (cherry picked from commit 3301449e736af81fdbd96fd3f396dd01b02beaf2)

This issue was fixed in the openstack/nova 19.0.0.0rc2 release candidate.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers