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

[Christophe TROESTLER]

On Tue, 08 Jul 2008 20:29:31 -0500, John Arbash Meinel wrote:
> 
> We specifically preserve the bits that were set on the .bzr/
> directory. (...)  So if your top level .bzr/ directory doesn't have
> +w, then none of the directories underneath will. (...)
> 
> The only question is when there isn't a .bzr/ directory to stat. I think we
> try to stat its containing directory, but I'm not positive if that is still
> the case.

Thanks for the clarification.  In my case, the g+w bit was set when
the umask was 0002 and not when it was 0022 -- hence my question.

> First thing I see.. "stat.st_mode > 01000", but usually the fact the
> object is a file or directory is stored in the upper bits, so you
> should be masking those off.

You are right, I corrected this oversight (with Harald suggestion).

On Wed, 09 Jul 2008 11:10:42 +0200, Harald Meland wrote:
> 
> If I've understood the sftp behaviour correctly, these bits will be
> cleared by the chmod() done immediately below this warning.  

Correct.

>     warning('About to chmod %s over sftp, which will result in its'
>             ' suid or sgid bits being cleared.  If you want to'
>             ' preserve those bits, change your environment on the'
>             ' server to use umask ...')

I have adopted your message.

Patch attached.

Cheers,
ChriS

-------------- next part --------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: christophe.troestler at umh.ac.be-20080709093751-\
#   l5yht55owvjshi7f
# target_branch: file:///home/devel/bzr/bzr.dev/
# testament_sha1: 27a5456e271c10d58a3cffa30e2d36273639f86a
# timestamp: 2008-07-09 11:41:04 +0200
# base_revision_id: pqm at pqm.ubuntu.com-20080709054822-jrzq8pdw4w7ob493
# 
# 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 09:36:41 +0000
@@ -519,7 +519,21 @@
         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 & 06000:
+                        warning('About to chmod %s over sftp, which will result'
+                                ' in its suid or sgid bits being cleared.  If'
+                                ' you want to preserve those bits, change your '
+                                ' environment on the server to use umask 0%03o.'
+                                % (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
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWdM97pEABJr/gAAURAR75///
eiIMAL////BgCE4vuz57wAAzYKGtqOg3TUAyJMmptKYag8o2aptTepABk9QAABERPTFG9UaepkMh
oxAaAYQBkaMc0xMBGmBGEYAAAAmEYCRIjIppowE0yeqaaeU0Bo0aepoZNACKREbUwRMmKnmIZUNh
pIepp6ho0NH6oJJARkATU9Iymp+kDQ9TUbRAABpvV1ru6BVmU+ev2HeFwFo0Q+P1HrTHLJiDIDN9
plIFJCIMyaOrNfJ38krRQ1vaCLWQqzpOzRY92wEgqkGING/hCzg/Hq04qecX1OQY22H4K5951vzI
6KcVw1E9MzmlAaufXhcqWorLPxPWsHC5iZED/TyRKTHugUJyt0rPa71sPoNqVLg/OqWmJx5HuaUn
SFOrW1vVRBWmms9WhgQ1eSIQsziXsMwMX3hSjVt/eEWva5ppSX85ZOyAGWKogi1bFAEEil/hgSpC
9QFuHMe0yLiMOJ+qF/F3FpeM3AKRJqpBKMVEzGN08kogAOc6c3lpINVXWRscGTIaih6AkaFE5GAj
wi0TuvQUXdnMyUA17LmMGakjeEf60EhhH7BPNCA3Gt9GoMza4+OOjqlF2jfhbfFbsLhHwEbfgNJb
M46mipqHWO3ZVKURHBJX9OfL6FcLMNDGc+j92qZuTs2r5VqIkCCY0nCH2RCJkM6gWnRx5ntXSgNU
TldIghJy5FcFGYkpjywQoD0KBVD0XYHoIDjTIiXouo5BNBaFK3A54frrIgI8lFxllwxVVQu8kOWB
inbZiOqCpkqEd78QyPBZEcUWFunyeEi4Y6CKZhFfIEG/A4bT2Tdyliomz47BxHosQXHEO4kHE1LR
xce1B619RGojyXCZR1zk20A3C7JXzfExHQYtQQYMTdBYWVCRIkMXoLiBjnG23Rie8SKMCSCBanTI
YEHQO0YwMziUsWgcWUUaDjItPqmlMdg64TUMAoUHL5roXA8Ocyy5Ug5UNSa4F8S1e0GpsczMgkPg
d5Ybl5JBEL1oHMQ/HBBF7RRjRBOhE0J32l5gXEiZwMhYjDyZFXzQlhNWjHYHBWGCsBxxGoUTPHDr
gMhxIYqEhjAMCKDsCZuvqnVIsNeZOeklBdhwxUoBAyJHpX2R2gVSHBZOAhnA5bDIVWNSdj0Ko45W
GAykamIxcRIGpgTHnAqRLDa893J1rBRGCo9kTAcjFEUEZi90OTCB7sLuPne37CwZSbGUmDr84YgZ
kBHA/giH0fwZtTIasjdNqK2PUbrRH2xriICBbflRcFK9ZWUa96OMRcZRoGoDH2SMjGMoDxHMv+HI
7DvG3JDj3/EIHgfRaCF3nd0Nj2GyXNB1NuKN1KKgbuXUeeLUuAIBWXQKIYUU05e7A7T2GY4vD6Jj
3mJ3BzJkRxzNxC5GNQYCp8RH86dFVfcKZ7EjlAQqDpenEeQNDUYzJz1Jleo4PSQLjqF5Ex9F+54e
Ca9Ha3wMwbNMOBnDIK46K2gmtak2iwauHIlGnQbC0WomBC2khKo41KCINC6pwPiGgdEZHmmteran
aHqY9a4ve92bvkecpftETTcOjYYqQySGiIVhAofOnbmRLTiHQlq1ln2BE6mi9SMsxccA96bQgg1w
fcGhXsnCWhzcD+a24lS7OZxQMJdzVQTmoo14aqAjZ9cugGBwG0uXGHtRZ1pp0mFSDtQwWxZldwZf
4bHlEfqd5DxOZ4nAtIEhjkYmYcz1GMtJEwGTMEcqOBuOhdj6dkAPVtSXqWpBNGECYIfdKFeHQ86D
/Tx474YeIjiyyCRya3pAoELJ296D1oKhKoqf0UJIjuDaIobEmibGvTKOt09SQRsl1i4fm5h+n1N3
FLhPAdRlGs/FCrqn2aA8OLW1uKDcnGZCDei+QTwkyjuccChz3tYmptCt7A5Nbz2ofbmMrQITgNMD
BE1h7RnK7F1GBqiDsALTWAJYiLrMDccYLwQazYJrb3kNNsRxiR/cGDimssvhiGVUhStJJUJNv1zX
aJXe6Qy2FTSCFMsC0GSEpnXt7oD16R8W41oEKvfwQwvYYFPk1i5kW6skh8pgc0mh2cDV6FI5bbfi
FFdDVU+D4O8CxyJ7MrkE7ZOfd0H3oRBcFzcMPePlahtZFoOkmGLH0yIHzt2BFYgkmhKaDMs6lWtl
F1y3AwDK4bJdU4zTCHhUn4iOaErgKBQw8xMkjDLdYCrULxNi9LFfJB3q8u8oh4NyopCafQBpuujt
zRtqSQeDyDhYGoZEgk7UWI5QlyoGZc4Ad25ycm8DvYefCtmi2ot6Hi1BVN5eMjfoKrE/CykzirMt
qRJbAl3K1TFQ1qhvCAIdTiGVtsZqYWjIZ2IskgUMyHcr2FU9SpRJDZAEmBkNC0aM8g+tdQZmM2Y2
CMbBqyE2cs+YvvQlVDznWg+wZ8H3bQ6HlJLQuKIaupDm+LUtt2cNbn+AzZsNgXvzEyTE7RPWmLc0
df/i7kinChIaZ73SIA==