Bug 11055 - vfs_snapper incorrectly handles multi-byte DBus strings
Summary: vfs_snapper incorrectly handles multi-byte DBus strings
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: VFS Modules (show other bugs)
Version: 4.2.0rc3
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-01-15 13:22 UTC by David Disseldorp
Modified: 2020-12-11 09:42 UTC (History)
5 users (show)

See Also:


Attachments
master based patchset (13.83 KB, patch)
2015-01-20 19:26 UTC, David Disseldorp
no flags Details
unit test - do not push (7.01 KB, patch)
2015-01-20 19:32 UTC, David Disseldorp
no flags Details
patchset for 4.2, cherry picked from master (14.16 KB, patch)
2015-01-22 10:44 UTC, David Disseldorp
jra: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Disseldorp 2015-01-15 13:22:20 UTC
Snapper escapes multi-byte strings prior to sending them in DBus requests and responses. Samba's vfs_snapper module doesn't do the same, causing problems when dealing with multi-byte share paths.

This is easily reproducible with the following procedure:
- Create a multi-byte share path (e.g. /srv/日本国).
- Create a snapper config for the share path.
- allow access to the snapper config
  + snapper -c $config set-config SYNC_ACL=yes
  + snapper -c $config ALLOW_USERS='$windows_user'
- Define the share in smb.conf, with "vfs objects = snapper" configured.
- connect to the share from a Windows client.
- create a test file on the share, with some data.
- request a snapshot locally
  + snapper -c $config create
- modify the test file
- request previous file versions for the test file in Windows Explorer
  + Samba will log a message: "config for base /srv/日本国 not found"
Comment 1 Jeremy Allison 2015-01-15 19:21:06 UTC
What escaping mechanism does snapper use ?
Comment 2 Justin Maggard 2015-01-15 19:31:07 UTC
The snapper documentation describes it as:

Strings are UTF-8. Other characters (e.g. in filenames) must be encoded
hexadecimal as "\x?" or "\x??". As a consequence "\" must be encoded as "\\".
Comment 3 David Disseldorp 2015-01-20 19:26:10 UTC
Created attachment 10638 [details]
master based patchset

Unit test patch to follow.

@Justin, it'd be great if you could give this a spin in your own environment.
Comment 4 David Disseldorp 2015-01-20 19:32:02 UTC
(In reply to Justin Maggard from comment #2)

BTW, I discussed the nonsensical "\x?" token with Arvin (Snapper maintainer). He agreed that it should be treated as an error, given that only characters above 0x7F are encoded (will always be two-char "\x??").
Comment 5 David Disseldorp 2015-01-20 19:32:43 UTC
Created attachment 10639 [details]
unit test - do not push
Comment 6 Justin Maggard 2015-01-21 00:45:01 UTC
Comment on attachment 10638 [details]
master based patchset

Patchset looks good to me, and works great in my environment as well.  Thanks!
Comment 7 David Disseldorp 2015-01-21 17:27:47 UTC
(In reply to Justin Maggard from comment #6)

Thanks for the feedback. I've sent the patch to the samba-technical mailing list for review. Will attach a 4.2 based patch once upstream.
Comment 8 David Disseldorp 2015-01-22 10:44:49 UTC
Created attachment 10649 [details]
patchset for 4.2, cherry picked from master
Comment 9 Jeremy Allison 2015-01-22 18:02:42 UTC
Comment on attachment 10649 [details]
patchset for 4.2, cherry picked from master

LGTM.
Comment 10 Jeremy Allison 2015-01-22 18:04:39 UTC
Re-assigning to Karolin for inclusion in 4.2.0.
Comment 11 David Disseldorp 2015-01-22 18:06:14 UTC
Thanks a bunch for the review Jeremy - much appreciated!
Comment 12 Karolin Seeger 2015-01-24 21:11:36 UTC
Pushed to autobuild-v4-2-test.
Comment 13 Karolin Seeger 2015-01-26 20:15:21 UTC
Pushed to v4-2-test.
Closing out bug report.

Thanks!