Skip to content

Commit 3347721

Browse files
wecharyuwecharyu
committed
HIVE-29167: Use access() api to check permission for different filesystems
1 parent 63868a9 commit 3347721

File tree

6 files changed

+89
-135
lines changed

6 files changed

+89
-135
lines changed

‎standalone-metastore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/client/ThriftHiveMetaStoreClient.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@
6969
import org.slf4j.Logger;
7070
import org.slf4j.LoggerFactory;
7171

72-
import javax.security.auth.login.LoginException;
7372
import java.io.IOException;
7473
import java.lang.reflect.Constructor;
7574
import java.lang.reflect.InvocationTargetException;
@@ -746,8 +745,6 @@ private void open() throws MetaException {
746745
try {
747746
UserGroupInformation ugi = SecurityUtils.getUGI();
748747
client.set_ugi(ugi.getUserName(), Arrays.asList(ugi.getGroupNames()));
749-
} catch (LoginException e) {
750-
LOG.warn("Failed to do login. set_ugi() is not successful, Continuing without it.", e);
751748
} catch (IOException e) {
752749
LOG.warn("Failed to find ugi of client set_ugi() is not successful, Continuing without it.", e);
753750
} catch (TException e) {

‎standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/Warehouse.java‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -503,20 +503,16 @@ public boolean isEmptyDir(Path path, PathFilter pathFilter)
503503
}
504504
}
505505

506-
public boolean isWritable(Path path) throws IOException {
506+
public boolean isWritable(Path path) {
507507
if (!storageAuthCheck) {
508508
// no checks for non-secure hadoop installations
509509
return true;
510510
}
511511
if (path == null) { //what??!!
512512
return false;
513513
}
514-
final FileStatus stat;
515-
final FileSystem fs;
516514
try {
517-
fs = getFs(path);
518-
stat = fs.getFileStatus(path);
519-
HdfsUtils.checkFileAccess(fs, stat, FsAction.WRITE);
515+
HdfsUtils.checkFileAccess(path, conf, FsAction.WRITE);
520516
return true;
521517
} catch (FileNotFoundException fnfe){
522518
// File named by path doesn't exist; nothing to validate.

‎standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/HdfsUtils.java‎

Lines changed: 15 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@
4444
import org.slf4j.Logger;
4545
import org.slf4j.LoggerFactory;
4646

47-
import javax.security.auth.login.LoginException;
4847
import java.io.FileNotFoundException;
4948
import java.io.IOException;
5049
import java.net.URI;
@@ -64,58 +63,37 @@ public class HdfsUtils {
6463

6564
/**
6665
* Check the permissions on a file.
67-
* @param fs Filesystem the file is contained in
68-
* @param stat Stat info for the file
66+
* @param path the file path
67+
* @param conf the {@link Configuration} used when checking permissions
6968
* @param action action to be performed
7069
* @throws IOException If thrown by Hadoop
71-
* @throws AccessControlException if the file cannot be accessed
7270
*/
73-
public static void checkFileAccess(FileSystem fs, FileStatus stat, FsAction action)
74-
throws IOException, LoginException {
75-
checkFileAccess(fs, stat, action, SecurityUtils.getUGI());
71+
public static void checkFileAccess(Path path, Configuration conf, FsAction action)
72+
throws IOException {
73+
FileSystem fs = path.getFileSystem(conf);
74+
checkFileAccess(fs, path, action, SecurityUtils.getUGI());
7675
}
7776

7877
/**
7978
* Check the permissions on a file
8079
* @param fs Filesystem the file is contained in
81-
* @param stat Stat info for the file
80+
* @param path the file path
8281
* @param action action to be performed
8382
* @param ugi user group info for the current user. This is passed in so that tests can pass
8483
* in mock ones.
8584
* @throws IOException If thrown by Hadoop
86-
* @throws AccessControlException if the file cannot be accessed
8785
*/
8886
@VisibleForTesting
89-
static void checkFileAccess(FileSystem fs, FileStatus stat, FsAction action,
87+
static void checkFileAccess(FileSystem fs, Path path, FsAction action,
9088
UserGroupInformation ugi) throws IOException {
91-
92-
String user = ugi.getShortUserName();
93-
String[] groups = ugi.getGroupNames();
94-
95-
if (groups != null) {
96-
String superGroupName = fs.getConf().get("dfs.permissions.supergroup", "");
97-
if (arrayContains(groups, superGroupName)) {
98-
LOG.debug("User \"" + user + "\" belongs to super-group \"" + superGroupName + "\". " +
99-
"Permission granted for action: " + action + ".");
100-
return;
101-
}
102-
}
103-
104-
FsPermission dirPerms = stat.getPermission();
105-
106-
if (user.equals(stat.getOwner())) {
107-
if (dirPerms.getUserAction().implies(action)) {
108-
return;
109-
}
110-
} else if (arrayContains(groups, stat.getGroup())) {
111-
if (dirPerms.getGroupAction().implies(action)) {
112-
return;
113-
}
114-
} else if (dirPerms.getOtherAction().implies(action)) {
115-
return;
89+
try {
90+
ugi.doAs((PrivilegedExceptionAction<Void>) () -> {
91+
fs.access(path, action);
92+
return null;
93+
});
94+
} catch (InterruptedException e) {
95+
throw new IOException(e);
11696
}
117-
throw new AccessControlException("action " + action + " not permitted on path "
118-
+ stat.getPath() + " for user " + user);
11997
}
12098

12199
public static boolean isPathEncrypted(Configuration conf, URI fsUri, Path path)

‎standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/SecurityUtils.java‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@
5959
import javax.net.ssl.TrustManagerFactory;
6060
import javax.security.auth.login.AppConfigurationEntry;
6161
import javax.security.auth.login.AppConfigurationEntry.LoginModuleControlFlag;
62-
import javax.security.auth.login.LoginException;
6362
import java.io.IOException;
6463
import java.net.InetSocketAddress;
6564
import java.net.UnknownHostException;
@@ -74,7 +73,7 @@
7473
public class SecurityUtils {
7574
private static final Logger LOG = LoggerFactory.getLogger(SecurityUtils.class);
7675

77-
public static UserGroupInformation getUGI() throws LoginException, IOException {
76+
public static UserGroupInformation getUGI() throws IOException {
7877
String doAs = System.getenv("HADOOP_USER_NAME");
7978
if (doAs != null && doAs.length() > 0) {
8079
/*
@@ -212,12 +211,8 @@ private static Token<DelegationTokenIdentifier> createToken(String tokenStr, Str
212211
* @throws IOException if underlying Hadoop call throws LoginException
213212
*/
214213
public static String getUser() throws IOException {
215-
try {
216-
UserGroupInformation ugi = getUGI();
217-
return ugi.getUserName();
218-
} catch (LoginException le) {
219-
throw new IOException(le);
220-
}
214+
UserGroupInformation ugi = getUGI();
215+
return ugi.getUserName();
221216
}
222217

223218
public static TServerSocket getServerSocket(String hiveHost, int portNum) throws TTransportException {

‎standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestHdfsUtils.java‎

Lines changed: 65 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.apache.hadoop.fs.FileSystem;
2424
import org.apache.hadoop.fs.FsShell;
2525
import org.apache.hadoop.fs.Path;
26+
import org.apache.hadoop.fs.RawLocalFileSystem;
2627
import org.apache.hadoop.fs.permission.AclStatus;
2728
import org.apache.hadoop.fs.permission.FsAction;
2829
import org.apache.hadoop.fs.permission.FsPermission;
@@ -31,9 +32,7 @@
3132
import org.apache.hadoop.security.UserGroupInformation;
3233
import org.junit.Test;
3334
import org.junit.experimental.categories.Category;
34-
import org.mockito.Mockito;
3535

36-
import javax.security.auth.login.LoginException;
3736
import java.io.IOException;
3837
import java.util.ArrayList;
3938
import java.util.List;
@@ -65,143 +64,136 @@ private Path createFile(FileSystem fs, FsPermission perms) throws IOException {
6564
return p;
6665
}
6766

68-
private Configuration makeConf() {
69-
// Make sure that the user doesn't happen to be in the super group
70-
Configuration conf = new Configuration();
71-
conf.set("dfs.permissions.supergroup", "ubermensch");
72-
return conf;
73-
}
74-
75-
private UserGroupInformation ugiInvalidUserValidGroups() throws LoginException, IOException {
76-
UserGroupInformation ugi = Mockito.mock(UserGroupInformation.class);
77-
Mockito.when(ugi.getShortUserName()).thenReturn("nosuchuser");
78-
Mockito.when(ugi.getGroupNames()).thenReturn(SecurityUtils.getUGI().getGroupNames());
79-
return ugi;
67+
private UserGroupInformation ugiInvalidUserValidGroups() throws IOException {
68+
return UserGroupInformation.createUserForTesting("nosuchuser", SecurityUtils.getUGI().getGroupNames());
8069
}
8170

8271
private UserGroupInformation ugiInvalidUserInvalidGroups() {
83-
UserGroupInformation ugi = Mockito.mock(UserGroupInformation.class);
84-
Mockito.when(ugi.getShortUserName()).thenReturn("nosuchuser");
85-
Mockito.when(ugi.getGroupNames()).thenReturn(new String[]{"nosuchgroup"});
86-
return ugi;
72+
return UserGroupInformation.createUserForTesting("nosuchuser", new String[]{"nosuchgroup"});
8773
}
8874

8975
@Test
90-
public void userReadWriteExecute() throws IOException, LoginException {
91-
FileSystem fs = FileSystem.get(makeConf());
76+
public void userReadWriteExecute() throws IOException {
77+
FileSystem fs = FileSystem.get(new Configuration());
9278
Path p = createFile(fs, new FsPermission(FsAction.ALL, FsAction.NONE, FsAction.NONE));
9379
UserGroupInformation ugi = SecurityUtils.getUGI();
94-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.READ, ugi);
95-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.WRITE, ugi);
96-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.EXECUTE, ugi);
80+
HdfsUtils.checkFileAccess(fs, p, FsAction.READ, ugi);
81+
HdfsUtils.checkFileAccess(fs, p, FsAction.WRITE, ugi);
82+
HdfsUtils.checkFileAccess(fs, p, FsAction.EXECUTE, ugi);
9783
}
9884

9985
@Test(expected = AccessControlException.class)
100-
public void userNoRead() throws IOException, LoginException {
101-
FileSystem fs = FileSystem.get(makeConf());
86+
public void userNoRead() throws IOException {
87+
FileSystem fs = FileSystem.get(new Configuration());
10288
Path p = createFile(fs, new FsPermission(FsAction.NONE, FsAction.ALL, FsAction.ALL));
10389
UserGroupInformation ugi = SecurityUtils.getUGI();
104-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.READ, ugi);
90+
HdfsUtils.checkFileAccess(fs, p, FsAction.READ, ugi);
10591
}
10692

10793
@Test(expected = AccessControlException.class)
108-
public void userNoWrite() throws IOException, LoginException {
109-
FileSystem fs = FileSystem.get(makeConf());
94+
public void userNoWrite() throws IOException {
95+
FileSystem fs = FileSystem.get(new Configuration());
11096
Path p = createFile(fs, new FsPermission(FsAction.NONE, FsAction.ALL, FsAction.ALL));
11197
UserGroupInformation ugi = SecurityUtils.getUGI();
112-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.WRITE, ugi);
98+
HdfsUtils.checkFileAccess(fs, p, FsAction.WRITE, ugi);
11399
}
114100

115101
@Test(expected = AccessControlException.class)
116-
public void userNoExecute() throws IOException, LoginException {
117-
FileSystem fs = FileSystem.get(makeConf());
102+
public void userNoExecute() throws IOException {
103+
FileSystem fs = FileSystem.get(new Configuration());
118104
Path p = createFile(fs, new FsPermission(FsAction.NONE, FsAction.ALL, FsAction.ALL));
119105
UserGroupInformation ugi = SecurityUtils.getUGI();
120-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.EXECUTE, ugi);
106+
HdfsUtils.checkFileAccess(fs, p, FsAction.EXECUTE, ugi);
121107
}
122108

123109
@Test
124-
public void groupReadWriteExecute() throws IOException, LoginException {
125-
FileSystem fs = FileSystem.get(makeConf());
110+
public void groupReadWriteExecute() throws IOException {
111+
FileSystem fs = FileSystem.get(new Configuration());
126112
Path p = createFile(fs, new FsPermission(FsAction.NONE, FsAction.ALL, FsAction.NONE));
127113
UserGroupInformation ugi = ugiInvalidUserValidGroups();
128-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.READ, ugi);
129-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.WRITE, ugi);
130-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.EXECUTE, ugi);
114+
HdfsUtils.checkFileAccess(fs, p, FsAction.READ, ugi);
115+
HdfsUtils.checkFileAccess(fs, p, FsAction.WRITE, ugi);
116+
HdfsUtils.checkFileAccess(fs, p, FsAction.EXECUTE, ugi);
131117
}
132118

133119
@Test(expected = AccessControlException.class)
134-
public void groupNoRead() throws IOException, LoginException {
135-
FileSystem fs = FileSystem.get(makeConf());
120+
public void groupNoRead() throws IOException {
121+
FileSystem fs = FileSystem.get(new Configuration());
136122
Path p = createFile(fs, new FsPermission(FsAction.ALL, FsAction.NONE, FsAction.ALL));
137123
UserGroupInformation ugi = ugiInvalidUserValidGroups();
138-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.READ, ugi);
124+
HdfsUtils.checkFileAccess(fs, p, FsAction.READ, ugi);
139125
}
140126

141127
@Test(expected = AccessControlException.class)
142-
public void groupNoWrite() throws IOException, LoginException {
143-
FileSystem fs = FileSystem.get(makeConf());
128+
public void groupNoWrite() throws IOException {
129+
FileSystem fs = FileSystem.get(new Configuration());
144130
Path p = createFile(fs, new FsPermission(FsAction.ALL, FsAction.NONE, FsAction.ALL));
145131
UserGroupInformation ugi = ugiInvalidUserValidGroups();
146-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.WRITE, ugi);
132+
HdfsUtils.checkFileAccess(fs, p, FsAction.WRITE, ugi);
147133
}
148134

149135
@Test(expected = AccessControlException.class)
150-
public void groupNoExecute() throws IOException, LoginException {
151-
FileSystem fs = FileSystem.get(makeConf());
136+
public void groupNoExecute() throws IOException {
137+
FileSystem fs = FileSystem.get(new Configuration());
152138
Path p = createFile(fs, new FsPermission(FsAction.ALL, FsAction.NONE, FsAction.ALL));
153139
UserGroupInformation ugi = ugiInvalidUserValidGroups();
154-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.EXECUTE, ugi);
140+
HdfsUtils.checkFileAccess(fs, p, FsAction.EXECUTE, ugi);
155141
}
156142

157143
@Test
158-
public void otherReadWriteExecute() throws IOException, LoginException {
159-
FileSystem fs = FileSystem.get(makeConf());
144+
public void otherReadWriteExecute() throws IOException {
145+
FileSystem fs = FileSystem.get(new Configuration());
160146
Path p = createFile(fs, new FsPermission(FsAction.NONE, FsAction.NONE, FsAction.ALL));
161147
UserGroupInformation ugi = ugiInvalidUserInvalidGroups();
162-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.READ, ugi);
163-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.WRITE, ugi);
164-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.EXECUTE, ugi);
148+
HdfsUtils.checkFileAccess(fs, p, FsAction.READ, ugi);
149+
HdfsUtils.checkFileAccess(fs, p, FsAction.WRITE, ugi);
150+
HdfsUtils.checkFileAccess(fs, p, FsAction.EXECUTE, ugi);
165151
}
166152

167153
@Test(expected = AccessControlException.class)
168-
public void otherNoRead() throws IOException, LoginException {
169-
FileSystem fs = FileSystem.get(makeConf());
154+
public void otherNoRead() throws IOException {
155+
FileSystem fs = FileSystem.get(new Configuration());
170156
Path p = createFile(fs, new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.NONE));
171157
UserGroupInformation ugi = ugiInvalidUserInvalidGroups();
172-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.READ, ugi);
158+
HdfsUtils.checkFileAccess(fs, p, FsAction.READ, ugi);
173159
}
174160

175161
@Test(expected = AccessControlException.class)
176-
public void otherNoWrite() throws IOException, LoginException {
177-
FileSystem fs = FileSystem.get(makeConf());
162+
public void otherNoWrite() throws IOException {
163+
FileSystem fs = FileSystem.get(new Configuration());
178164
Path p = createFile(fs, new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.NONE));
179165
UserGroupInformation ugi = ugiInvalidUserInvalidGroups();
180-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.WRITE, ugi);
166+
HdfsUtils.checkFileAccess(fs, p, FsAction.WRITE, ugi);
181167
}
182168

183169
@Test(expected = AccessControlException.class)
184-
public void otherNoExecute() throws IOException, LoginException {
185-
FileSystem fs = FileSystem.get(makeConf());
170+
public void otherNoExecute() throws IOException {
171+
FileSystem fs = FileSystem.get(new Configuration());
186172
Path p = createFile(fs, new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.NONE));
187173
UserGroupInformation ugi = ugiInvalidUserInvalidGroups();
188-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.EXECUTE, ugi);
174+
HdfsUtils.checkFileAccess(fs, p, FsAction.EXECUTE, ugi);
189175
}
190176

191-
@Test
192-
public void rootReadWriteExecute() throws IOException, LoginException {
177+
178+
@Test(expected = AccessControlException.class)
179+
public void accessPerssionFromCustomFilesystem() throws IOException {
180+
FileSystem fs = new RawLocalFileSystem() {
181+
@Override
182+
public void access(Path path, FsAction mode) throws AccessControlException, IOException {
183+
if (path.toString().contains("noaccess")) {
184+
throw new AccessControlException("no access");
185+
}
186+
}
187+
188+
@Override
189+
public Configuration getConf() {
190+
return new Configuration();
191+
}
192+
};
193+
193194
UserGroupInformation ugi = SecurityUtils.getUGI();
194-
FileSystem fs = FileSystem.get(new Configuration());
195-
String old = fs.getConf().get("dfs.permissions.supergroup");
196-
try {
197-
fs.getConf().set("dfs.permissions.supergroup", ugi.getPrimaryGroupName());
198-
Path p = createFile(fs, new FsPermission(FsAction.NONE, FsAction.NONE, FsAction.NONE));
199-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.READ, ugi);
200-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.WRITE, ugi);
201-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.EXECUTE, ugi);
202-
} finally {
203-
fs.getConf().set("dfs.permissions.supergroup", old);
204-
}
195+
Path p = new Path("/tmp/noaccess");
196+
HdfsUtils.checkFileAccess(fs, p, FsAction.EXECUTE, ugi);
205197
}
206198

207199
/**

0 commit comments

Comments
 (0)