You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-commits@db.apache.org by da...@apache.org on 2011/11/09 10:35:15 UTC

svn commit: r1199673 - in /db/derby/code/trunk/java: engine/org/apache/derby/iapi/services/io/FileUtil.java testing/org/apache/derbyTesting/functionTests/tests/engine/RestrictiveFilePermissionsTest.java

Author: dag
Date: Wed Nov  9 09:35:15 2011
New Revision: 1199673

URL: http://svn.apache.org/viewvc?rev=1199673&view=rev
Log:
DERBY-5492 Restrictive file permissions: permissions removed also for owner on NTFS if Acl does not contain explicit entry for owner

Patch derby-5492-2 which solves this issue plus make one other
adjustment, see item two below.

- Construct a new AclEntry for the owner with all rights, and removed
  existing ones (NTFS). This should handle the error seen in Oracle's
  regressions.

- For Solaris/ZFS and similar file systems which support both Posix
  file attributes view and ACLs, don't touch the ACLs but stick to the
  Posix flags.

 For the latter my rationale is as follows: Principle of least
 surprise: most users never touch the ACLs but use the more familiar
 Posix file masks. It turned out the existing Derby implementation,
 although protecting the file adequately, showed a "+" in the ls(1)
 listing indicating that the settings could not be directly mapped
 onto the Posix model. The reason was that we removed more permissions
 than the plain read,write, and execute. Since ZFS internally builds
 on ACLs, the ls(1) listing would should that Derby had been tinkering
 with the non-mappable ACL permissions. I think it is better to stick
 to the Posix permissions by default. If people are using ACL
 functionality they are likely more than average concerned with
 security and can run with default file permissions and take full
 responsibility of the permissions fo created filed.


Modified:
    db/derby/code/trunk/java/engine/org/apache/derby/iapi/services/io/FileUtil.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/engine/RestrictiveFilePermissionsTest.java

Modified: db/derby/code/trunk/java/engine/org/apache/derby/iapi/services/io/FileUtil.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/iapi/services/io/FileUtil.java?rev=1199673&r1=1199672&r2=1199673&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/iapi/services/io/FileUtil.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/iapi/services/io/FileUtil.java Wed Nov  9 09:35:15 2011
@@ -38,8 +38,10 @@ import org.apache.derby.io.StorageFile;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.ArrayList;
-import java.util.Iterator;
+import java.util.Arrays;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 import org.apache.derby.iapi.reference.Property;
 import org.apache.derby.iapi.services.info.JVMInfo;
 import org.apache.derby.iapi.services.property.PropertyUtil;
@@ -612,6 +614,7 @@ nextFile:	for (int i = 0; i < list.lengt
     private static Class aclEntryBuilderClz;
     private static Class aclEntryTypeClz;
     private static Class fileStoreClz;
+    private static Class aclEntryPermissionClz;
 
     private static Method get;
     private static Method getFileAttributeView;
@@ -626,7 +629,9 @@ nextFile:	for (int i = 0; i < list.lengt
     private static Method newBuilder;
     private static Method setPrincipal;
     private static Method setType;
-
+    private static Method values;
+    private static Method setPermissions;
+    
     private static Field allow;
     /**
      * Use when creating new files. If running with Java 6 or higher on Unix,
@@ -660,7 +665,7 @@ nextFile:	for (int i = 0; i < list.lengt
             // The property has not been specified. Only proceed if we are
             // running with the network server started from the command line
             // *and* at Java 7 or above
-            if (JVMInfo.JDK_ID >= JVMInfo.J2SE_17 && 
+            if (JVMInfo.JDK_ID >= JVMInfo.J2SE_17 &&
                     (PropertyUtil.getSystemBoolean(
                         Property.SERVER_STARTED_FROM_CMD_LINE, false)) ) {
                 // proceed
@@ -717,6 +722,8 @@ nextFile:	for (int i = 0; i < list.lengt
                         "java.nio.file.attribute.AclEntryType");
                     fileStoreClz = Class.forName(
                         "java.nio.file.FileStore");
+                    aclEntryPermissionClz = Class.forName(
+                            "java.nio.file.attribute.AclEntryPermission");
                     get = pathsClz.getMethod(
                         "get",
                         new Class[]{String.class, stringArrayClz});
@@ -748,6 +755,10 @@ nextFile:	for (int i = 0; i < list.lengt
                                   new Class[]{userPrincipalClz});
                     setType = aclEntryBuilderClz.
                         getMethod("setType", new Class[]{aclEntryTypeClz});
+                    values = aclEntryPermissionClz.
+                        getMethod("values", (Class[]) null);
+                    setPermissions = aclEntryBuilderClz.
+                        getMethod("setPermissions", new Class[] { Set.class });
 
                     allow = aclEntryTypeClz.getField("ALLOW");
 
@@ -904,81 +915,49 @@ nextFile:	for (int i = 0; i < list.lengt
             }
 
 
-            // If we have a posix view, we can use ACLs to interface
-            // the usual Unix permission masks vi the special principals
-            // OWNER@, GROUP@ and EVERYONE@
-
-            // PosixFileAttributeView posixView =
-            // Files.getFileAttributeView(fileP, PosixFileAttributeView.class);
+            // If we have a posix view, just return and fall back on
+            // the JDK 6 approach.
             Object posixView = getFileAttributeView.invoke(
                 null,
                 new Object[]{fileP,
                              posixFileAttributeViewClz,
                              Array.newInstance(linkOptionClz, 0)});
 
+            if (posixView != null) {
+                return false;
+            }
+
+            // Since we have an AclFileAttributeView which is not a
+            // PosixFileAttributeView, we probably have a NTFS file
+            // system.
+
             // UserPrincipal owner = Files.getOwner(fileP);
             Object owner = getOwner.invoke(
                 null,
                 new Object[]{fileP, Array.newInstance(linkOptionClz, 0)});
 
-            // List<AclEntry> oldAcl = view.getAcl();
-            // List<AclEntry> newAcl = new ArrayList<>();
+            //
+            // Remove existing ACEs, build a new one which simply
+            // gives all possible permissions to current owner.
+            //
+            // List<AclEntry>        newAcl = new ArrayList<>();
+            // AclEntryPermissions[] perms = AclEntryPermission.values();
+            // AclEntry.Builder      aceb = AclEntry.newBuilder();
+            //
+            // aceb.setType(AclEntryType.ALLOW);
+            // aceb.setPermissions(new HashSet(Arrays.asList(perms);
+            // newAcl.add(aceb);
 
-            List oldAcl = (List)getAcl.invoke(view, null);
             List newAcl = new ArrayList();
-
-            // for (AclEntry ace : oldAcl) {
-            //     if (posixView != null) {
-            //         if (ace.principal().getName().equals("OWNER@")) {
-            //             // retain permission for owner
-            //             newAcl.add(ace);
-            //         } else if (
-            //             ace.principal().getName().equals("GROUP@") ||
-            //             ace.principal().getName().equals("EVERYONE@")) {
-            //
-            //             AclEntry.Builder aceb = AclEntry.newBuilder();
-            //             aceb.setPrincipal(ace.principal())
-            //                 .setType(AclEntryType.ALLOW);
-            //             // add no permissions for the group and other
-            //             newAcl.add(aceb.build());
-            //         }
-            //     } else {
-            //         // NTFS, hopefully
-            //         if (ace.principal().equals(owner)) {
-            //             newAcl.add(ace);
-            //         }
-            //     }
-            // }
-
-            for (Iterator i = oldAcl.iterator(); i.hasNext();) {
-                Object ace = i.next();
-                Object princ = principal.invoke(ace, null);
-                String princName = (String)getName.invoke(princ, null);
-
-                if (posixView != null) {
-                    if (princName.equals("OWNER@")) {
-                        // retain permission for owner
-                        newAcl.add(ace);
-                    } else if (
-                        princName.equals("GROUP@") ||
-                        princName.equals("EVERYONE@")) {
-
-                        // add ALLOW ACE w/no permissions for group and other
-
-                        Object aceb = newBuilder.invoke(null, null);
-                        Object allowValue = allow.get(aclEntryTypeClz);
-
-                        aceb = setPrincipal.invoke(aceb, new Object[]{princ});
-                        aceb = setType.invoke(aceb, new Object[]{allowValue});
-                        newAcl.add(build.invoke(aceb, null));
-                    }
-                } else {
-                    // NTFS, hopefully
-                    if (princ.equals(owner)) {
-                        newAcl.add(ace);
-                    }
-                }
-            }
+            Object[] perms = (Object[]) values.invoke(null, (Object[]) null);
+            Object aceb = newBuilder.invoke(null, (Object[]) null);
+            Object allowValue = allow.get(aclEntryTypeClz);
+            aceb = setPrincipal.invoke(aceb, new Object[]{owner});
+            aceb = setType.invoke(aceb, new Object[]{allowValue});
+            aceb = setPermissions.invoke(
+                aceb,
+                new Object[] {new HashSet(Arrays.asList(perms))});
+            newAcl.add(build.invoke(aceb, (Object[]) null));
 
             // view.setAcl(newAcl);
             setAcl.invoke(view, new Object[]{newAcl});

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/engine/RestrictiveFilePermissionsTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/engine/RestrictiveFilePermissionsTest.java?rev=1199673&r1=1199672&r2=1199673&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/engine/RestrictiveFilePermissionsTest.java (original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/engine/RestrictiveFilePermissionsTest.java Wed Nov  9 09:35:15 2011
@@ -725,8 +725,9 @@ public class RestrictiveFilePermissionsT
                                 posixFileAttributeViewClz,
                                 Array.newInstance(linkOptionClz, 0)});
 
-                        if (aclsSupported && aclView != null) {
-                            // Windows, Solaris 11
+                        if (aclsSupported && aclView != null &&
+                                posixView == null) {
+                            // Windows
                             Object owner = getOwner.invoke(
                                 null,
                                 new Object[]{
@@ -784,6 +785,7 @@ public class RestrictiveFilePermissionsT
                                 }
                             }
                         } else if (posixView != null) {
+                            // Unixen
                             Object posixFileAttributes =
                                 readAttributes.invoke(posixView,
                                                       new Object[]{});