Location reported in notifications

[Stuart McLaren]

If I create an image, eg:

$ glance --os-username glance --os-password <redacted> --os-tenant-name service --os-auth-url http://localhost:5000/v2.0 image-create --disk-format=qcow2 --container-format=bare --name="Image2" < /etc/motd

while having notification enabled, eg in glance-api.conf:

notifier_strategy = rabbit

And then query ceilometer:

curl http://127.0.0.1:8777/v1/resources/bf8c239a-2e8a-49b0-ac6a-b67b997b394b/meters/image

    {
      "counter_name": "image",
 .
.
.
      "counter_volume": 1,
      "resource_metadata": {
        "status": "active",
        "name": "Image2",
 .
.
.
        "location": "swift+http://service%3Aglance:<redacted>@10.7.101.5:5000/v2.0/glance/f9f40980-03fa-4151-aaac-d37a80490b96",
.
.
.
        "size": 667
      },
      "counter_type": "gauge"
    }

The location (with credentials) is shown in plaintext.

This is the case whether or not encryption of the location is enabled via the "metadata_encryption_key" config parameter, ie here is the relevant database entry showing the location is encrypted:

| id | name | size | status | is_public | location | created_at | updated_at | deleted_at | deleted | disk_format | container_format | checksum | owner | min_disk | min_ram | protected |

| f9f40980-03fa-4151-aaac-d37a80490b96 | Image2 | 667 | active | 0 | GDII5JDRVEedM9J0PrjmoIM3r-_DyQlKZ4hWQCIjYKhj5yJpSz8gq66XZ-5mJBJA2CjVrvHIh_ZJuJn-O1TcRBk6jdA6owyHbHYcQky5NYmf7tT5ut9oY_Y1h7Q_uOj8AjJye4Xqk32jN1qlkM6Mvgi5vaFPXjKvwFNUJCr5bAGEK0YS3Ko1lDH9VS7V-Vkt | 2013-02-14 11:49:49 | 2013-02-14 11:49:50 | NULL | 0 | qcow2 | bare | 84de8695202d116bbe0e0fb06c6e020b | e2906b40a41747fbbfa6c66e01bc1f51 | 0 | 0 | 0 |

[Stuart McLaren (stuart-mclaren)]

Similarly if notification is set to file, ie:

notifier_strategy = logging

a6c66e01bc1f51] {'event_type': 'image.activate', 'timestamp': '2013-02-14 12:34:47.570814', 'message_id': 'b3a41d9f-68bf-4777-8d9a-5d5779d4d3e8', 'priority': 'INFO', 'publisher_id': 'server-1360836694-az-2-region-a-geo-1', 'payload': {u'status': u'active', u'name': u'Image3', u'deleted': False, u'container_format': u'bare', u'created_at': u'2013-02-14T12:34:46', u'disk_format': u'qcow2', u'updated_at': u'2013-02-14T12:34:47', u'properties': {}, u'min_disk': 0, u'protected': False, u'id': u'9a0f1074-8196-4f7d-a3c3-c5e131b9ce79', u'location': 'swift+http://service%3Aglance:<redacted>@10.7.101.5:5000/v2.0/glance/9a0f1074-8196-4f7d-a3c3-c5e131b9ce79', u'checksum': u'84de8695202d116bbe0e0fb06c6e020b', u'owner': u'e2906b40a41747fbbfa6c66e01bc1f51', u'is_public': False, u'deleted_at': None, u'min_ram': 0, u'size': 667}}

[Stuart McLaren (stuart-mclaren)]

Options I see here are:

1) remove the location parameter altogether
2) leave it in but ensure it is encrypted when the metadata encryption is set on
3) leave it in and just remove the value (set to "redacted" or "****")

Possibly 3 may be the best option since it doesn't change the existing format and is secure whether metadata encryption is on or off.

[Thierry Carrez (ttx)]

Adding PTL

If I understand this right, the location is only leaked in operator-visible notifications. It should definitely be fixed (and backported if need be), but I would not issue a public advisory for this.

[Brian Waldon (bcwaldon)]

I think I will support option 3, but with the caveat that we use the same format as the config secret sanitizer.

[Stuart McLaren (stuart-mclaren)]

Attaching patch 2 -- should I just push this up to review.openstack.org?

[Thierry Carrez (ttx)]

@Stuart: yes. Let me open this bug so that you can reference it in the commit message.

[OpenStack Infra (hudson-openstack)]

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

[Flavio Percoco (flaper87)]

This bug is similar to[0][1] in terms of "design flaw" and perhaps should be addressed by the same fix which requires some refactoring on how locations are handled and what data is being stored in the database. I think it is better to change the way credentials are being handled (perhaps we should create a bp since it would hit many stores) instead of fixing every single bug.

[0] https://bugs.launchpad.net/glance/+bug/1100220

[1] https://bugs.launchpad.net/glance/+bug/1098962

[Iccha Sethi (iccha-sethi)]

Yeah I agree with Flavio in terms of looking for a more long term solution for this problem.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/22298
Committed: http://github.com/openstack/glance/commit/7bdc4eb94a0df6c6edca73bc2c47c05efb22e025
Submitter: Jenkins
Branch: master

commit 7bdc4eb94a0df6c6edca73bc2c47c05efb22e025
Author: Stuart McLaren <email address hidden>
Date: Tue Feb 19 12:52:38 2013 +0000

    Redact location from notifications

    Redact the value of 'location' in notifications since it may contain
    credentials.

    Fix for bug 1125130.

    Change-Id: Ic0baee259ac46479f8992eca2b02c307fcaf70ac