copier.Put(): clear up os/syscall mode bit confusion

When noting that a non-symlink has setuid/setgid/sticky bits, switch
from using "syscall" package bits and syscall.Chmod() to using "os"
package bits and os.Chmod(), and if the item's a directory, record the
updated mode information in the "directoryModes" map that we'll use to
reset its permissions later.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Signed-off-by: Aditya R <arajan@redhat.com>

copier/copier.go

@@ -1565,15 +1565,15 @@ func copierHandlerPut(bulkReader io.Reader, req request, idMappings *idtools.IDM
 		return nil
 	}
 	makeDirectoryWriteable := func(directory string) error {
+		if _, ok := directoryModes[directory]; !ok {
 			st, err := os.Lstat(directory)
 			if err != nil {
 				return errors.Wrapf(err, "copier: put: error reading permissions of directory %q", directory)
 			}
-		mode := st.Mode() & os.ModePerm
-		if _, ok := directoryModes[directory]; !ok {
+			mode := st.Mode()
 			directoryModes[directory] = mode
 		}
-		if err = os.Chmod(directory, 0o700); err != nil {
+		if err := os.Chmod(directory, 0o700); err != nil {
 			return errors.Wrapf(err, "copier: put: error making directory %q writable", directory)
 		}
 		return nil
@@ -1859,16 +1859,21 @@ func copierHandlerPut(bulkReader io.Reader, req request, idMappings *idtools.IDM
 			// set other bits that might have been reset by chown()
 			if hdr.Typeflag != tar.TypeSymlink {
 				if hdr.Mode&cISUID == cISUID {
-					mode |= syscall.S_ISUID
+					mode |= os.ModeSetuid
 				}
 				if hdr.Mode&cISGID == cISGID {
-					mode |= syscall.S_ISGID
+					mode |= os.ModeSetgid
 				}
 				if hdr.Mode&cISVTX == cISVTX {
-					mode |= syscall.S_ISVTX
+					mode |= os.ModeSticky
 				}
-				if err = syscall.Chmod(path, uint32(mode)); err != nil {
-					return errors.Wrapf(err, "error setting additional permissions on %q to 0%o", path, mode)
+				if hdr.Typeflag == tar.TypeDir {
+					// if/when we do the final setting of permissions on this
+					// directory, make sure to incorporate these bits, too
+					directoryModes[path] = mode
+				}
+				if err = os.Chmod(path, mode); err != nil {
+					return errors.Wrapf(err, "copier: put: setting additional permissions on %q to 0%o", path, mode)
 				}
 			}
 			// set xattrs, including some that might have been reset by chown()

tests/conformance/conformance_test.go

@@ -1872,6 +1872,42 @@ var internalTestCases = []testCase{
 			if _, err = io.Copy(tw, bytes.NewReader([]byte("whatever"))); err != nil {
 				return errors.Wrapf(err, "error writing tar archive content")
 			}
+			hdr = tar.Header{
+				Name:     "setuid-dir",
+				Uid:      0,
+				Gid:      0,
+				Typeflag: tar.TypeDir,
+				Size:     0,
+				Mode:     cISUID | 0755,
+				ModTime:  testDate,
+			}
+			if err = tw.WriteHeader(&hdr); err != nil {
+				return fmt.Errorf("error writing tar archive header: %w", err)
+			}
+			hdr = tar.Header{
+				Name:     "setgid-dir",
+				Uid:      0,
+				Gid:      0,
+				Typeflag: tar.TypeDir,
+				Size:     0,
+				Mode:     cISGID | 0755,
+				ModTime:  testDate,
+			}
+			if err = tw.WriteHeader(&hdr); err != nil {
+				return fmt.Errorf("error writing tar archive header: %w", err)
+			}
+			hdr = tar.Header{
+				Name:     "sticky-dir",
+				Uid:      0,
+				Gid:      0,
+				Typeflag: tar.TypeDir,
+				Size:     0,
+				Mode:     cISVTX | 0755,
+				ModTime:  testDate,
+			}
+			if err = tw.WriteHeader(&hdr); err != nil {
+				return fmt.Errorf("error writing tar archive header: %w", err)
+			}
 			return nil
 		},
 	},