HADOOP-10701. NFS should not validate the access premission only based on the user's primary group. Contributed by Harsh J.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-2@1606043 13f79535-47bb-0310-9956-ffa450edef68

hadoop-common-project/hadoop-nfs/dev-support/findbugsExcludeFile.xml

@@ -0,0 +1,28 @@
+<!--
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+-->
+<FindBugsFilter>
+  <!--
+    FindBugs is complaining about CredentialsSys#getAuxGIDs(...) returning
+    a mutable array, but it is alright in our case, and copies would be
+    more expensive instead.
+  -->
+  <Match>
+      <Class name="org.apache.hadoop.oncrpc.security.CredentialsSys"/>
+      <Method name="getAuxGIDs" params="" returns="int[]"/>
+      <Bug code="EI"/>
+  </Match>
+</FindBugsFilter>

hadoop-common-project/hadoop-nfs/pom.xml

@@ -93,6 +93,18 @@
     </dependency>
   </dependencies>
 
+  <build>
+    <plugins>
+      <plugin>
+        <groupId>org.codehaus.mojo</groupId>
+        <artifactId>findbugs-maven-plugin</artifactId>
+        <configuration>
+          <excludeFilterFile>${basedir}/dev-support/findbugsExcludeFile.xml
+          </excludeFilterFile>
+        </configuration>
+      </plugin>
+    </plugins>
+  </build>
 
   <profiles>
     <profile>

hadoop-common-project/hadoop-nfs/src/main/java/org/apache/hadoop/oncrpc/security/CredentialsSys.java

@@ -58,6 +58,10 @@ public class CredentialsSys extends Credentials {
     return mUID;
   }
 
+  public int[] getAuxGIDs() {
+    return mAuxGIDs;
+  }
+
   public void setGID(int gid) {
     this.mGID = gid;
   }
@@ -65,7 +69,7 @@ public class CredentialsSys extends Credentials {
   public void setUID(int uid) {
     this.mUID = uid;
   }
-  
+
   public void setStamp(int stamp) {
     this.mStamp = stamp;
   }

hadoop-common-project/hadoop-nfs/src/main/java/org/apache/hadoop/oncrpc/security/SecurityHandler.java

@@ -60,4 +60,9 @@ public abstract class SecurityHandler {
   public int getGid() {
     throw new UnsupportedOperationException();
   }
+
+  /** Used by AUTH_SYS */
+  public int[] getAuxGids() {
+    throw new UnsupportedOperationException();
+  }
 }

hadoop-common-project/hadoop-nfs/src/main/java/org/apache/hadoop/oncrpc/security/SysSecurityHandler.java

@@ -56,4 +56,9 @@ public class SysSecurityHandler extends SecurityHandler {
   public int getGid() {
     return mCredentialsSys.getGID();
   }
+
+  @Override
+  public int[] getAuxGids() {
+    return mCredentialsSys.getAuxGIDs();
+  }
 }

hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/Nfs3Utils.java

@@ -160,7 +160,7 @@ public class Nfs3Utils {
   }
 
   public static int getAccessRightsForUserGroup(int uid, int gid,
-      Nfs3FileAttributes attr) {
+      int[] auxGids, Nfs3FileAttributes attr) {
     int mode = attr.getMode();
     if (uid == attr.getUid()) {
       return getAccessRights(mode >> 6, attr.getType());
@@ -168,6 +168,14 @@ public class Nfs3Utils {
     if (gid == attr.getGid()) {
       return getAccessRights(mode >> 3, attr.getType());
     }
+    // Check for membership in auxiliary groups
+    if (auxGids != null) {
+      for (int auxGid : auxGids) {
+        if (attr.getGid() == auxGid) {
+          return getAccessRights(mode >> 3, attr.getType());
+        }
+      }
+    }
     return getAccessRights(mode, attr.getType());
   }
   
@@ -191,4 +199,4 @@ public class Nfs3Utils {
     data[7] = (byte) (v >>> 0);
     return data;
   }
-}
+}

hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/RpcProgramNfs3.java

@@ -504,7 +504,8 @@ public class RpcProgramNfs3 extends RpcProgram implements Nfs3Interface {
         return new ACCESS3Response(Nfs3Status.NFS3ERR_STALE);
       }
       int access = Nfs3Utils.getAccessRightsForUserGroup(
-          securityHandler.getUid(), securityHandler.getGid(), attrs);
+          securityHandler.getUid(), securityHandler.getGid(),
+          securityHandler.getAuxGids(), attrs);
       
       return new ACCESS3Response(Nfs3Status.NFS3_OK, attrs, access);
     } catch (RemoteException r) {
@@ -659,7 +660,8 @@ public class RpcProgramNfs3 extends RpcProgram implements Nfs3Interface {
         return new READ3Response(Nfs3Status.NFS3ERR_NOENT);
       }
       int access = Nfs3Utils.getAccessRightsForUserGroup(
-          securityHandler.getUid(), securityHandler.getGid(), attrs);
+          securityHandler.getUid(), securityHandler.getGid(),
+          securityHandler.getAuxGids(), attrs);
       if ((access & Nfs3Constant.ACCESS3_READ) != 0) {
         eof = offset < attrs.getSize() ? false : true;
         return new READ3Response(Nfs3Status.NFS3_OK, attrs, 0, eof,

hadoop-hdfs-project/hadoop-hdfs-nfs/src/test/java/org/apache/hadoop/hdfs/nfs/nfs3/TestNfs3Utils.java

@@ -0,0 +1,72 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hdfs.nfs.nfs3;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import org.junit.Test;
+
+import java.io.IOException;
+
+import org.apache.hadoop.nfs.NfsFileType;
+import org.apache.hadoop.nfs.nfs3.Nfs3FileAttributes;
+
+import org.mockito.Mockito;
+
+public class TestNfs3Utils {
+  @Test
+  public void testGetAccessRightsForUserGroup() throws IOException {
+    Nfs3FileAttributes attr = Mockito.mock(Nfs3FileAttributes.class);
+    Mockito.when(attr.getUid()).thenReturn(2);
+    Mockito.when(attr.getGid()).thenReturn(3);
+    Mockito.when(attr.getMode()).thenReturn(448); // 700
+    Mockito.when(attr.getType()).thenReturn(NfsFileType.NFSREG.toValue());
+    assertEquals("No access should be allowed as UID does not match attribute over mode 700",
+      0, Nfs3Utils.getAccessRightsForUserGroup(3, 3, null, attr));
+    Mockito.when(attr.getUid()).thenReturn(2);
+    Mockito.when(attr.getGid()).thenReturn(3);
+    Mockito.when(attr.getMode()).thenReturn(56); // 070
+    Mockito.when(attr.getType()).thenReturn(NfsFileType.NFSREG.toValue());
+    assertEquals("No access should be allowed as GID does not match attribute over mode 070",
+      0, Nfs3Utils.getAccessRightsForUserGroup(2, 4, null, attr));
+    Mockito.when(attr.getUid()).thenReturn(2);
+    Mockito.when(attr.getGid()).thenReturn(3);
+    Mockito.when(attr.getMode()).thenReturn(7); // 007
+    Mockito.when(attr.getType()).thenReturn(NfsFileType.NFSREG.toValue());
+    assertEquals("Access should be allowed as mode is 007 and UID/GID do not match",
+      61 /* RWX */, Nfs3Utils.getAccessRightsForUserGroup(1, 4, new int[] {5, 6}, attr));
+    Mockito.when(attr.getUid()).thenReturn(2);
+    Mockito.when(attr.getGid()).thenReturn(10);
+    Mockito.when(attr.getMode()).thenReturn(288); // 440
+    Mockito.when(attr.getType()).thenReturn(NfsFileType.NFSREG.toValue());
+    assertEquals("Access should be allowed as mode is 440 and Aux GID does match",
+      1 /* R */, Nfs3Utils.getAccessRightsForUserGroup(3, 4, new int[] {5, 16, 10}, attr));
+    Mockito.when(attr.getUid()).thenReturn(2);
+    Mockito.when(attr.getGid()).thenReturn(10);
+    Mockito.when(attr.getMode()).thenReturn(448); // 700
+    Mockito.when(attr.getType()).thenReturn(NfsFileType.NFSDIR.toValue());
+    assertEquals("Access should be allowed for dir as mode is 700 and UID does match",
+      31 /* Lookup */, Nfs3Utils.getAccessRightsForUserGroup(2, 4, new int[] {5, 16, 10}, attr));
+    assertEquals("No access should be allowed for dir as mode is 700 even though GID does match",
+      0, Nfs3Utils.getAccessRightsForUserGroup(3, 10, new int[] {5, 16, 4}, attr));
+    assertEquals("No access should be allowed for dir as mode is 700 even though AuxGID does match",
+      0, Nfs3Utils.getAccessRightsForUserGroup(3, 20, new int[] {5, 10}, attr));
+  }
+}

hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt

@@ -473,6 +473,9 @@ Release 2.5.0 - UNRELEASED
     HDFS-6475. WebHdfs clients fail without retry because incorrect handling
     of StandbyException. (Yongjun Zhang via atm)
 
+    HADOOP-10701. NFS should not validate the access premission only based on
+    the user's primary group (Harsh J via atm)
+
   BREAKDOWN OF HDFS-2006 SUBTASKS AND RELATED JIRAS
 
     HDFS-6299. Protobuf for XAttr and client-side implementation. (Yi Liu via umamahesh)