World Writable Mounts Need Sticky Bit

[damonb123]

ISSUE TYPE

• Bug Report

COMPONENT NAME

component:api

CLOUDSTACK VERSION

4.17
4.18

OS / ENVIRONMENT

Ubuntu Rocky Linux 8

SUMMARY

In java code, NFS mounts are not consistently set to 1777 to prevent world writable issues.

References to correct setting

./server/src/main/java/org/apache/cloudstack/storage/NfsMountManagerImpl.java: script.add("1777", mountPoint);
./plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java: script.add("1777", mountPoint);

Change 777 to 1777

./services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/LocalNfsSecondaryStorageResource.java

    @Override
    protected void mount(String localRootPath, String remoteDevice, URI uri, String nfsVersion) {
        ensureLocalRootPathExists(localRootPath, uri);

        if (mountExists(localRootPath, uri)) {
            return;
        }

        attemptMount(localRootPath, remoteDevice, uri, nfsVersion);

        // Change permissions for the mountpoint - seems to bypass authentication
        Script script = new Script(true, "chmod", _timeout, s_logger);
        script.add("777", localRootPath);
        String result = script.execute();
        if (result != null) {
            String errMsg = "Unable to set permissions for " + localRootPath + " due to " + result;
            s_logger.error(errMsg);
            throw new CloudRuntimeException(errMsg);
        }
        s_logger.debug("Successfully set 777 permission for " + localRootPath);

        // XXX: Adding the check for creation of snapshots dir here. Might have
        // to move it somewhere more logical later.
        checkForSnapshotsDir(localRootPath);
        checkForVolumesDir(localRootPath);
    }

./plugins/hypervisors/hyperv/src/main/java/com/cloud/hypervisor/hyperv/manager/HypervManagerImpl.java

protected String mount(String path, String parent, String scheme, String query) {
        String mountPoint = setupMountPoint(parent);
        if (mountPoint == null) {
            s_logger.warn("Unable to create a mount point");
            return null;
        }

        Script script = null;
        String result = null;
        if (scheme.equals("cifs")) {
            String user = System.getProperty("user.name");
            Script command = new Script(true, "mount", _timeout, s_logger);
            command.add("-t", "cifs");
            command.add(path);
            command.add(mountPoint);

            if (user != null) {
                command.add("-o", "uid=" + user + ",gid=" + user);
            }

            if (query != null) {
                query = query.replace('&', ',');
                command.add("-o", query);
            }

            result = command.execute();
        }

        if (result != null) {
            s_logger.warn("Unable to mount " + path + " due to " + result);
            File file = new File(mountPoint);
            if (file.exists()) {
                file.delete();
            }
            return null;
        }

        // Change permissions for the mountpoint
        script = new Script(true, "chmod", _timeout, s_logger);
        script.add("-R", "777", mountPoint);
        result = script.execute();
        if (result != null) {
            s_logger.warn("Unable to set permissions for " + mountPoint + " due to " + result);
        }
        return mountPoint;
    }

[DaanHoogland]

@damonb123 have you seen issues with mounts not being writeable due to this? Can you describe the use cases of failures?

[damonb123]

The lack of the sticky bit is a security issue, and shows up as a security violation. Not having sticky bit will allow users other than the owner to delete them.

[DaanHoogland]

@damonb123 can you review/test #7573 ?

[DaanHoogland]

change requested does not have the result desired as per testing. not urgent as the mounts are not publicely exposed but needs more investigation.