You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2022/11/02 15:10:50 UTC

[GitHub] [cloudstack] damonb123 opened a new issue, #6867: World Writable Mounts Need Sticky Bit

damonb123 opened a new issue, #6867:
URL: https://github.com/apache/cloudstack/issues/6867

   <!--
   Verify first that your issue/request is not already reported on GitHub.
   Also test if the latest release and main branch are affected too.
   Always add information AFTER of these HTML comments, but no need to delete the comments.
   -->
   
   ##### 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
   ```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
   ```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;
       }
   
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] DaanHoogland commented on issue #6867: World Writable Mounts Need Sticky Bit

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #6867:
URL: https://github.com/apache/cloudstack/issues/6867#issuecomment-1301724599

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] DaanHoogland commented on issue #6867: World Writable Mounts Need Sticky Bit

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on issue #6867:
URL: https://github.com/apache/cloudstack/issues/6867#issuecomment-1730970580

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] damonb123 commented on issue #6867: World Writable Mounts Need Sticky Bit

Posted by GitBox <gi...@apache.org>.
damonb123 commented on issue #6867:
URL: https://github.com/apache/cloudstack/issues/6867#issuecomment-1314258547

   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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [cloudstack] DaanHoogland commented on issue #6867: World Writable Mounts Need Sticky Bit

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on issue #6867:
URL: https://github.com/apache/cloudstack/issues/6867#issuecomment-1587513631

   @damonb123 can you review/test #7573 ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org