[MERGE] sftp transport: do not chmod a dir when unecessary (fix suid and sgid problems).

[Christophe TROESTLER]

Hi,

After discussions with John Arbash Meinel (who I'd like to thank for
his help) on #bzr, I am submitting this patch.

The problem is that, with the sftp protocol, any chmod will erase the
suid or sgid bits.  This can be a problem when the sgid bit is used to
enable sharing among several developers.  This patch issues chmod _for
directories_ only when it is necessary.  When chmod is necessary but
will erase suid/sgid bits, a warning is issued to the user telling him
to set the proper mask so that chmod is no longer needed.

There is an extra cost of lstat'ing the file which is usually more or
less balanced by the fact that chmod is not needed.  Moreover, only
directory (not file) creation is affected.

Cheers,
ChriS


P.S.  The point of chmod, I was told, is to ensure that the g+w bit is
set (for sharing).  This however does not appears to work when the
server umask does not allow it.  Do we want to warn the user in this
case as well?
-------------- next part --------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: christophe.troestler at umh.ac.be-20080709001159-\
#   2rihz5z2mjbng1yr
# target_branch: file:///home/devel/bzr/bzr.dev/
# testament_sha1: ca35032124be72aea7722b6d2d6f058aa96d0305
# timestamp: 2008-07-09 02:16:48 +0200
# base_revision_id: pqm at pqm.ubuntu.com-20080704171330-ieh195xj7su2k2xq
# 
# Begin patch
=== modified file 'bzrlib/transport/sftp.py'
--- bzrlib/transport/sftp.py	2008-05-08 04:33:38 +0000
+++ bzrlib/transport/sftp.py	2008-07-09 00:11:59 +0000
@@ -519,7 +519,20 @@
         try:
             self._get_sftp().mkdir(abspath, local_mode)
             if mode is not None:
-                self._get_sftp().chmod(abspath, mode=mode)
+                # chmod a dir through sftp will erase any sgid bit set
+                # on the server side.  So, if the bit mode are already
+                # set, avoid the chmod.  If the mode is not fine but
+                # the sgid bit is set, report a warning to the user
+                # with the umask fix.
+                stat = self._get_sftp().lstat(abspath)
+                mode = mode & 0777 # can't set special bits anyway
+                if mode != stat.st_mode & 0777:
+                    if stat.st_mode > 01000:
+                        warning('The server set a suid or sgid bit on '
+                                '%s.  If you want to preserve it, use '
+                                '"umask 0%03o" on the server.'
+                                % (abspath, 0777 - mode))
+                    self._get_sftp().chmod(abspath, mode=mode)
         except (paramiko.SSHException, IOError), e:
             self._translate_io_exception(e, abspath, ': unable to mkdir',
                 failure_exc=FileExists)

# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWUhFa1YAAfd/gAAURAR75///
WiAMAL////BQBM7u53uSvdnrvbvbQD3uEpBT1MmKeUY2hMU2iZMmE0NANGgGUCm2pkeoJpMhk0Gg
NAAaANBkKbSGqPKD1P1RoPUBo9IPUB6mgABihNU/SNTzRJ6E00wmjI2ggDRoxASSAQaE0GpPBSfp
qn6pmSepjUDI2po9R78syvXf8FLuX6pq4Kcy1PRkSQikkVXf6GNAuFOaxeIAS4EwNGVCdfWCMr+k
eUUkN0I9eNfAvQttYc1bmiL1cU8gwVU+UkyoPZeUOtuQ4HohsC6cpM7ppxZsI0x5Naix8t9uJ0XG
c5lIsSnZCE5Noin9dMOoZeKprKgS9/pUtXz6IXRyPo5+VcXTPBdej8FZ6RFx1sENcuxDe5rSVWpR
eOFNTzI61RDbt2szRF6CpayBVQrmkAnEDvxJLDJolmmwUxWm4haJxEy5iq5zSIA0m4LvQ72Ijked
KCV6pmUqySsTkKMGFAeTJ8vj1XG0axMgtWn72xETE51o5T+x2tQVzaaQkRopcDrfVbjU+wlrFVag
6RWcMk7AukqITEVDzoBqd2EFRSrMENqiAF43A4Wi9BwnaDUL4KyibKwzL1hKFS91SRGhCuUYxe80
ktUi5/dA4FTAqONhFYtbQfYHyTHzw3Enu1NCeXEeLUZrQVBoa4KHRbYEKBxWsgONwVBmGRkRQWMY
hQNvkMj/VGbVZyRtMbYzPfgLYQoJibsTSAVGMC8eWxzuojFUECp1RICERkb0ByQZh6aM0RYtKLhM
sgWD5Hhqa5gfMxrrDNbJdOgkvt/q2U+asMxhSgEGc9j1i0weHE19vE2nQ+awS24mBtDyMNRl3xgx
6uPe1onlkd6oDEVKXbodDqXBibjQeMeBrS0nejz8RJsuBNyOyg9qJCt21ZNMQaxpaajCSVQ4yuyI
nVpJkaecpKW4eEWKM82ewgnJoI8DEkge1viGBzLzFHem48aDPczNYmLfzExsoCAoxMNNocPeLYbw
w1OIT7xmlzMreAt0hZLI7LBuU4ClIYFgL3uK2q8nLimAwWlmZe3cBFGOokBcpU1sfc7sD8jrgOPM
0OZxGk4jCojOPXQbFZHrTDQB2tKlCXCfxpcHK2wKLCZix4GOQ/yIYTPseB+hAyXQayxvYKo0FK5U
RRsVh/f305F4uKzmR8QhqyOPJ8sALk45AXgdgfV1cZBo90aIwoOxGcHhmgvpWkU4Fw5ti92eGa1N
wl8xrvsP04XkktewXS7yDebu9nHoxXA20qydByURxBD67XrQVcyhRWBJkUTk82c+4+HrbtEmEfhz
CtzDJfGKLAIMFy8Acuxs/ifSIFdtpz5+xTUhrQWjiunqv1oXF1w3BdlYLUXnBsG3EE5mHMA8NWY/
KfX2uFjdj7n5Mn2T6i6gYK0tr5DC4UgE5qwZjvYLHqmZv9JQGbgtlPzzfvm8D0bIdx3NAJER2ADr
S9ct6l7S9cxvdyiPAuAvDzUB0NVR3TQoH9QrJxCTnjvle8loSOo5Da8CdtE9BfcOKAMMCrN4AnQ2
yDUoihVCopkaiznRCgV11zJSZg7BqpFh6LQAuIbeVptF3wdRibkkMdXIXOZL0BhSDqB2JWR/4u5I
pwoSCQitasA=