Bug 1370932 - Cinder backup using NFS backend will be overwritten if same container is used
Summary: Cinder backup using NFS backend will be overwritten if same container is used
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat OpenStack
Classification: Red Hat
Component: openstack-cinder
Version: 8.0 (Liberty)
Hardware: All
OS: Linux
urgent
urgent
Target Milestone: zstream
: 8.0 (Liberty)
Assignee: Gorka Eguileor
QA Contact: Avi Avraham
URL:
Whiteboard:
Depends On: 1456381 1456384 1456387
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-08-28 14:48 UTC by Chen
Modified: 2020-08-13 08:34 UTC (History)
20 users (show)

Fixed In Version: openstack-cinder-7.0.3-7.el7ost
Doc Type: Bug Fix
Doc Text:
Previously, backups on the NFS back end shared the same "backup" prefix. As a result, if multiple backups shared the same container, they would end up overwritten. With this update, the backup prefix is now generated from the backup UUID and the volume ID, so that backup data is no longer overwritten.
Clone Of:
: 1456381 1456384 1456387 (view as bug list)
Environment:
Last Closed: 2017-07-12 13:16:54 UTC
Target Upstream Version:
lkuchlan: automate_bug+


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Launchpad 1628768 0 None None None 2016-09-29 16:59:02 UTC
OpenStack gerrit 413753 0 'None' ABANDONED Posix Backup: Open files w/ exclusive mode 2021-01-29 04:14:34 UTC
OpenStack gerrit 465869 0 'None' MERGED NFS Backup: Fix overwritting backups 2021-01-29 04:14:35 UTC
Red Hat Product Errata RHBA-2017:1743 0 normal SHIPPED_LIVE openstack-cinder bug fix advisory 2017-07-12 17:12:34 UTC

Description Chen 2016-08-28 14:48:02 UTC
Description of problem:

cinder backup using NFS will be overwritten if same container is used

Version-Release number of selected component (if applicable):

openstack-cinder-7.0.2-2.el7ost.noarch

How reproducible:

100%

Steps to Reproduce:

1. Attach the volume called rhel_7 to the instance. Create a file called first1.txt in the volume.

2. # cinder backup-create --name rhel_7_1 rhel_7 --force --container volumebackups

3. Create a file called second2.txt in the volume.

4. # cinder backup-create --name rhel_7_2 rhel_7 --force --container volumebackups

5. Create a file called third3.txt in the volume.

6. Detach the volume from the instance and restore the volume from rhel_7_1.

7. Attach the volume again. You will find there are first1.txt and second.txt in the volume. But rhel_7_1 should only contain first1.txt.


Actual results:

Volume backup is overwritten

Expected results:

There should be separate directories for different backups

Additional info:

In Horizon, if I don't specify the container, a default container will be used and volumes will be overwritten.

backup_driver = cinder.backup.drivers.nfs
backup_share = 10.72.32.49:/home/nfsshare

Comment 2 Eric Harney 2016-08-29 20:55:16 UTC
I tested this and can confirm it is a problem.

At this time, container names specified on the command-line for this configuration should be unique rather than "volumebackups".

This should be fixed by either:
a) Making separate directories under this container name for each backup
or
b) At least failing new backups rather than overwriting data.

Comment 3 Junko IKEDA 2016-08-30 04:33:20 UTC
This problem is reproducible with posix driver, it means not only nfs but also glusterfs driver can also probably overwrite on the same container.

It seems that swift driver can create unique prefix for container.

https://github.com/openstack/cinder/blob/stable/mitaka/cinder/backup/drivers/swift.py#L324

Distinguish containers by unique, separated id is absolutely required.
Falling backup operation is also desirable to avoid the possibility of the id conflict.

Comment 4 Masaki Furuta ( RH ) 2016-09-16 10:33:36 UTC
(In reply to Junko IKEDA from comment #3)
> This problem is reproducible with posix driver, it means not only nfs but
> also glusterfs driver can also probably overwrite on the same container.
> 
> It seems that swift driver can create unique prefix for container.
> 
> https://github.com/openstack/cinder/blob/stable/mitaka/cinder/backup/drivers/
> swift.py#L324
> 
> Distinguish containers by unique, separated id is absolutely required.
> Falling backup operation is also desirable to avoid the possibility of the
> id conflict.

Dear IKEDA-san,

Sorry for jumping in, I've also checked upstream code.

In my understanding, for me it looks this NFS driver looks overriding Posix driver in __init__ with backup_path and Posix driver overrides chunk driver basically (and Posix driver still doesn't have it).

I've checked this further, but as far as I've checked, I could not find any of BP for this feature yet too.

If my current understanding is true, since we need fix in the upstream code in the first place then we'll backport it into downstream, so I think you may have to wait for upstream fix before requesting fix on rhbz and we'd do recommend to track upstream changes first.


Kind Regards,


Reference:

- 
https://github.com/openstack/cinder/blob/stable/mitaka/cinder/backup/chunkeddriver.py
- https://github.com/openstack/cinder/blob/stable/mitaka/cinder/backup/drivers/posix.py#L74,#L78

- https://github.com/openstack/cinder/blob/stable/mitaka/cinder/backup/drivers/nfs.py#L55,L57,L58

- https://specs.openstack.org/openstack/cinder-specs/specs/kilo/nfs-backup.html

Comment 5 Masaki Furuta ( RH ) 2016-09-16 10:50:46 UTC
All,

Changing summary based on my understanding at #4 ,please feel free to revert back if I had a wrong idea and please provide your concern or upstream fix or anything else related to the fix if you'd have.

Comment 6 Masaki Furuta ( RH ) 2016-09-26 09:42:27 UTC
(In reply to Masaki Furuta from comment #5)
> All,
> 
> Changing summary based on my understanding at #4 ,please feel free to revert
> back if I had a wrong idea and please provide your concern or upstream fix
> or anything else related to the fix if you'd have.

Hi Eric,

We got additional concern from Customer as follows;

"""
As for this problem, we did additional test internally on our side and we found that the code doesn't guarantee namespace uniqueness for backup name and therefore the backup name wouldn't be unique per tenant.

This means, we can not avoid this command will override backup from other tenants (and by other users).

Furthermore, if this is true, we think this is sort of vulnerability that a malicious user can override existing backup by taking backup files with random name to try to delete existing backup data.
"""

Can I get your voice?, I really appreciate if we have answer to this customer's concern.

Comment 8 Elise Gafford 2016-11-01 17:08:17 UTC
Moving to 10.0.z per discussion with eharney.

Comment 9 Chen 2016-12-08 07:55:18 UTC
Hi Eric,

Can I get any progress on this bug ? Shall we backport it in OSP8 ?

Best Regards,
Chen

Comment 10 Chen 2017-01-09 07:29:30 UTC
Hi,

Can I get any progress on this bug ? Both OSP10 and OSP8 will have the fix ?

Best Regards,
Chen

Comment 11 Eric Harney 2017-01-12 17:15:03 UTC
The fix for this bug is still under development upstream.

Can we advise the customer to not use the "--container" option for now?  Is it a requirement here?

Comment 12 Chen 2017-01-13 01:32:31 UTC
Hi Eric,

Thank you for your reply.

As I said in the bug description, 

- In Horizon, if I don't specify the container, a default container will be used and volumes will be overwritten.

This seems to be a security issue to the customer and you can not ask every end users to input a unique container name when backing up the volume.

Best Regards,
Chen

Comment 13 Chen 2017-03-15 14:18:34 UTC
Hi,

Do we have any progress about this bug ?

Best Regards,
Chen

Comment 14 Eric Harney 2017-03-15 14:26:41 UTC
Patch is in progress and linked in the external trackers field of this bug.

Comment 27 Chen 2017-06-22 00:46:13 UTC
Hi Paul,

Thank you for your reply.

The Solution Architect informed me that "the customer requests to have the hotfix for OSP10, since they currently test OSP10." So it would be much appreciated if you could help us with the hotfix with OSP10 first and then OSP8 later.

Thank you very much !

Best Regards,
Chen

Comment 28 Avi Avraham 2017-06-22 06:02:05 UTC
The fix was verified according to the steps described in the bug description 
all 3 different backups restored successfully

Comment 29 lkuchlan 2017-06-25 07:59:33 UTC
Tested and automated downstream

Comment 33 Tzach Shefi 2017-06-29 14:47:08 UTC
Verified, 

Version openstack-cinder-7.0.3-7.el7ost.noarch
Reproduction steps provided expected results. 
Only first1.txt file was found on a restored volume.

Comment 36 errata-xmlrpc 2017-07-12 13:16:54 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2017:1743


Note You need to log in before you can comment on or make changes to this bug.