When creating uuid-based entities we can duplicate UUIDs

Bug #1758057 reported by Chris Dent
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Fix Released
Undecided
Mike Chen

Bug Description

It is possible to create two different resource providers (and probably other entities) with the same UUID by creating one with '-' and the other without. This is because in both json schema and ovo validate UUIDs using the same route (different code but same concept): with or without - is okay.

Then, we save these strings into a column in the database which is not a uuid type, instead it is varchar 36.

Thus we can make this happen (gabbi format):

-=-=-
# Some tests to see if different representations of the
# same uuid result in different resource providers.

fixtures:
    - APIFixture

defaults:
    request_headers:
        x-auth-token: admin
        accept: application/json
        content-type: application/json
        openstack-api-version: placement latest

tests:
- name: create dashed
  POST: /resource_providers
  data:
      name: dashed
      uuid: b7c31381-0cd6-421c-a2d2-009d645615dc

- name: create not dashed
  POST: /resource_providers
  data:
      name: not dashed
      uuid: b7c313810cd6421ca2d2009d645615dc

- name: check length
  GET: /resource_providers
  response_json_paths:
      # This may be should be 1 but on current master is 2
      $.resource_providers.`len`: 1
-=-=-

We might be able to get away with this not being a problem except that there is one place where we expected a dashed uuid for resource providers: in the JSON schema for PUTting allocations in the dict format: https://github.com/openstack/nova/blob/master/nova/api/openstack/placement/schemas/allocation.py#L80

This happened because I couldn't figure out how to use a format checker for a PatternProperties and wrote a pattern only accepting a 36 length UUID.

This means we've got at least two potential problems:

* we can create a resource provider for which we can't write allocations (unless we use the older list style)
* clients have the potential to think they are using the same UUID when the placement server thinks they are not

We can solve this in a few different ways, this list is not mutually exclusive:

* do nothing, expect people to do the right thing
* change the PatternProperty on allocation put to make dash optional
* continue accepting non-dashed input, but always dash them early in processing
* reject non-dashed input everywhere

And I haven't looked into consumer uuids, but I suspect there's some ambiguity there too.

The root issue here, in case it is not clear, is that code in the wild that we don't control that is creating and stringifying UUID may be creating the non-dashed format.

Revision history for this message
Eric Fried (efried) wrote :

My take is that we should start by fixing this in a way that doesn't require a microversion. Namely, continue accepting what we accept (no schema changes etc.) but normalize the values as they come in.

Given that it is pretty unlikely that this issue has manifested in the wild, don't do anything to try to clean it up until/unless someone demands it.

tags: added: low-hanging-fruit
Rajat Sharma (tajar29)
Changed in nova:
assignee: nobody → Rajat Sharma (tajar29)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (master)

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

Changed in nova:
status: Triaged → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/581137

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (master)

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

commit a644b220569064169ac77ef3b8d804d41dce881f
Author: Eric Fried <email address hidden>
Date: Mon Jul 9 16:06:37 2018 -0500

    Test for unsanitized consumer UUID

    Add a gabbi test case demonstrating that the consumer with UUID
    {8}-{4}-{4}-{4}-{12} is not the same as the consumer with UUID
    {8}{4}{4}{4}{12}.

    Change-Id: I1852ac6004fedc5dfa9dd3de187c937ab385d1b5
    Related-Bug: #1758057

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/591863

Changed in nova:
assignee: Rajat Sharma (tajar29) → Chen (chenn2)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (master)

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

commit fa66d9a730072869ac13797a37d3224855653f1e
Author: Eric Fried <email address hidden>
Date: Tue Aug 14 17:30:04 2018 -0500

    [placement] Regex consts for placement schema

    Factor out some regexes used across multiple schemata into a new
    nova.api.openstack.placement.schemas.common module.

    The UUID part of this is in preparation for fixing the related bug.

    Change-Id: If62bfeeb32d0ad77dd1205116ee4e5e844bb07e4
    Related-Bug: #1758057

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (master)

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

commit d27d8e44a3845349990fca81de805538f01480bf
Author: rajat29 <email address hidden>
Date: Wed May 9 16:15:20 2018 +0530

    Normalize dashless 'resource provider create' uuid

    When creating resource provider with '--uuid' argument, nova
    accept uuid without dash('-') too, which some time results in,
    resource provider with same uuid i.e one with dash and one without.

    This patch attempts to fix it by transforming dashless UUID into
    dashed one before inserting it into the database.

    Co-Authored-By: Chen <email address hidden>

    Change-Id: I2685eb65907adbd22b2d09264b110692e100eaf9
    Closes-Bug: #1758057

Changed in nova:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/nova 19.0.0.0rc1

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

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

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.