diff --git a/hadoop-common-project/hadoop-nfs/dev-support/findbugsExcludeFile.xml b/hadoop-common-project/hadoop-nfs/dev-support/findbugsExcludeFile.xml new file mode 100644 index 0000000000..d76cc77a32 --- /dev/null +++ b/hadoop-common-project/hadoop-nfs/dev-support/findbugsExcludeFile.xml @@ -0,0 +1,28 @@ + + + + + + + + + diff --git a/hadoop-common-project/hadoop-nfs/pom.xml b/hadoop-common-project/hadoop-nfs/pom.xml index 56250660bc..e30a482edf 100644 --- a/hadoop-common-project/hadoop-nfs/pom.xml +++ b/hadoop-common-project/hadoop-nfs/pom.xml @@ -93,6 +93,18 @@ + + + + org.codehaus.mojo + findbugs-maven-plugin + + ${basedir}/dev-support/findbugsExcludeFile.xml + + + + + diff --git a/hadoop-common-project/hadoop-nfs/src/main/java/org/apache/hadoop/oncrpc/security/CredentialsSys.java b/hadoop-common-project/hadoop-nfs/src/main/java/org/apache/hadoop/oncrpc/security/CredentialsSys.java index 9ba12b8990..997ce35c60 100644 --- a/hadoop-common-project/hadoop-nfs/src/main/java/org/apache/hadoop/oncrpc/security/CredentialsSys.java +++ b/hadoop-common-project/hadoop-nfs/src/main/java/org/apache/hadoop/oncrpc/security/CredentialsSys.java @@ -58,6 +58,10 @@ public int getUID() { return mUID; } + public int[] getAuxGIDs() { + return mAuxGIDs; + } + public void setGID(int gid) { this.mGID = gid; } @@ -65,7 +69,7 @@ public void setGID(int gid) { public void setUID(int uid) { this.mUID = uid; } - + public void setStamp(int stamp) { this.mStamp = stamp; } diff --git a/hadoop-common-project/hadoop-nfs/src/main/java/org/apache/hadoop/oncrpc/security/SecurityHandler.java b/hadoop-common-project/hadoop-nfs/src/main/java/org/apache/hadoop/oncrpc/security/SecurityHandler.java index 40004d0e78..063082e4c4 100644 --- a/hadoop-common-project/hadoop-nfs/src/main/java/org/apache/hadoop/oncrpc/security/SecurityHandler.java +++ b/hadoop-common-project/hadoop-nfs/src/main/java/org/apache/hadoop/oncrpc/security/SecurityHandler.java @@ -60,4 +60,9 @@ public int getUid() { public int getGid() { throw new UnsupportedOperationException(); } + + /** Used by AUTH_SYS */ + public int[] getAuxGids() { + throw new UnsupportedOperationException(); + } } diff --git a/hadoop-common-project/hadoop-nfs/src/main/java/org/apache/hadoop/oncrpc/security/SysSecurityHandler.java b/hadoop-common-project/hadoop-nfs/src/main/java/org/apache/hadoop/oncrpc/security/SysSecurityHandler.java index 196d3d8cbb..74237764c5 100644 --- a/hadoop-common-project/hadoop-nfs/src/main/java/org/apache/hadoop/oncrpc/security/SysSecurityHandler.java +++ b/hadoop-common-project/hadoop-nfs/src/main/java/org/apache/hadoop/oncrpc/security/SysSecurityHandler.java @@ -56,4 +56,9 @@ public int getUid() { public int getGid() { return mCredentialsSys.getGID(); } + + @Override + public int[] getAuxGids() { + return mCredentialsSys.getAuxGIDs(); + } } diff --git a/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/Nfs3Utils.java b/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/Nfs3Utils.java index b351fda68f..5c30f16bc9 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/Nfs3Utils.java +++ b/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/Nfs3Utils.java @@ -160,7 +160,7 @@ public static int getAccessRights(int mode, int type) { } 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 static int getAccessRightsForUserGroup(int uid, int gid, 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 static byte[] longToByte(long v) { data[7] = (byte) (v >>> 0); return data; } -} \ No newline at end of file +} diff --git a/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/RpcProgramNfs3.java b/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/RpcProgramNfs3.java index 446e722a21..96a4c49498 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/RpcProgramNfs3.java +++ b/hadoop-hdfs-project/hadoop-hdfs-nfs/src/main/java/org/apache/hadoop/hdfs/nfs/nfs3/RpcProgramNfs3.java @@ -504,7 +504,8 @@ public ACCESS3Response access(XDR xdr, RpcInfo info) { 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 @@ READ3Response read(XDR xdr, SecurityHandler securityHandler, 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, diff --git a/hadoop-hdfs-project/hadoop-hdfs-nfs/src/test/java/org/apache/hadoop/hdfs/nfs/nfs3/TestNfs3Utils.java b/hadoop-hdfs-project/hadoop-hdfs-nfs/src/test/java/org/apache/hadoop/hdfs/nfs/nfs3/TestNfs3Utils.java new file mode 100644 index 0000000000..b5f0cd4c53 --- /dev/null +++ b/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)); + } +} diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index e3ca235b33..a4220b0962 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -725,6 +725,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)