Skip to content

Commit 67bc120

Browse files
wecharyuwecharyu
authored
HIVE-29167: Use access() api to check permission for different filesystems (#6047)
1 parent 12a8eac commit 67bc120

File tree

6 files changed

+96
-134
lines changed

6 files changed

+96
-134
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;
@@ -750,8 +749,6 @@ private void open() throws MetaException {
750749
try {
751750
UserGroupInformation ugi = SecurityUtils.getUGI();
752751
client.set_ugi(ugi.getUserName(), Arrays.asList(ugi.getGroupNames()));
753-
} catch (LoginException e) {
754-
LOG.warn("Failed to do login. set_ugi() is not successful, Continuing without it.", e);
755752
} catch (IOException e) {
756753
LOG.warn("Failed to find ugi of client set_ugi() is not successful, Continuing without it.", e);
757754
} catch (TException e) {

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

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -503,20 +503,17 @@ 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+
FileSystem fs = getFs(path);
516+
HdfsUtils.checkFileAccess(fs, path, FsAction.WRITE);
520517
return true;
521518
} catch (FileNotFoundException fnfe){
522519
// 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: 13 additions & 36 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;
@@ -65,57 +64,35 @@ public class HdfsUtils {
6564
/**
6665
* Check the permissions on a file.
6766
* @param fs Filesystem the file is contained in
68-
* @param stat Stat info for the file
67+
* @param path the file path
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(FileSystem fs, Path path, FsAction action)
72+
throws IOException {
73+
checkFileAccess(fs, path, action, SecurityUtils.getUGI());
7674
}
7775

7876
/**
7977
* Check the permissions on a file
8078
* @param fs Filesystem the file is contained in
81-
* @param stat Stat info for the file
79+
* @param path the file path
8280
* @param action action to be performed
8381
* @param ugi user group info for the current user. This is passed in so that tests can pass
8482
* in mock ones.
8583
* @throws IOException If thrown by Hadoop
86-
* @throws AccessControlException if the file cannot be accessed
8784
*/
8885
@VisibleForTesting
89-
static void checkFileAccess(FileSystem fs, FileStatus stat, FsAction action,
86+
static void checkFileAccess(FileSystem fs, Path path, FsAction action,
9087
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;
88+
try {
89+
ugi.doAs((PrivilegedExceptionAction<Void>) () -> {
90+
fs.access(path, action);
91+
return null;
92+
});
93+
} catch (InterruptedException e) {
94+
throw new IOException(e);
11695
}
117-
throw new AccessControlException("action " + action + " not permitted on path "
118-
+ stat.getPath() + " for user " + user);
11996
}
12097

12198
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
@@ -60,7 +60,6 @@
6060
import javax.net.ssl.TrustManagerFactory;
6161
import javax.security.auth.login.AppConfigurationEntry;
6262
import javax.security.auth.login.AppConfigurationEntry.LoginModuleControlFlag;
63-
import javax.security.auth.login.LoginException;
6463
import java.io.IOException;
6564
import java.net.InetSocketAddress;
6665
import java.net.UnknownHostException;
@@ -90,7 +89,7 @@ public class SecurityUtils {
9089
}
9190
}
9291

93-
public static UserGroupInformation getUGI() throws LoginException, IOException {
92+
public static UserGroupInformation getUGI() throws IOException {
9493
String doAs = System.getenv("HADOOP_USER_NAME");
9594
if (doAs != null && doAs.length() > 0) {
9695
/*
@@ -264,12 +263,8 @@ private static Token<DelegationTokenIdentifier> createToken(String tokenStr, Str
264263
* @throws IOException if underlying Hadoop call throws LoginException
265264
*/
266265
public static String getUser() throws IOException {
267-
try {
268-
UserGroupInformation ugi = getUGI();
269-
return ugi.getUserName();
270-
} catch (LoginException le) {
271-
throw new IOException(le);
272-
}
266+
UserGroupInformation ugi = getUGI();
267+
return ugi.getUserName();
273268
}
274269

275270
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: 73 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,8 @@
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;
36+
import java.io.FileNotFoundException;
3737
import java.io.IOException;
3838
import java.util.ArrayList;
3939
import java.util.List;
@@ -65,143 +65,143 @@ private Path createFile(FileSystem fs, FsPermission perms) throws IOException {
6565
return p;
6666
}
6767

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;
68+
private UserGroupInformation ugiInvalidUserValidGroups() throws IOException {
69+
return UserGroupInformation.createUserForTesting("nosuchuser", SecurityUtils.getUGI().getGroupNames());
8070
}
8171

8272
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;
73+
return UserGroupInformation.createUserForTesting("nosuchuser", new String[]{"nosuchgroup"});
8774
}
8875

8976
@Test
90-
public void userReadWriteExecute() throws IOException, LoginException {
91-
FileSystem fs = FileSystem.get(makeConf());
77+
public void userReadWriteExecute() throws IOException {
78+
FileSystem fs = FileSystem.get(new Configuration());
9279
Path p = createFile(fs, new FsPermission(FsAction.ALL, FsAction.NONE, FsAction.NONE));
9380
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);
81+
HdfsUtils.checkFileAccess(fs, p, FsAction.READ, ugi);
82+
HdfsUtils.checkFileAccess(fs, p, FsAction.WRITE, ugi);
83+
HdfsUtils.checkFileAccess(fs, p, FsAction.EXECUTE, ugi);
9784
}
9885

9986
@Test(expected = AccessControlException.class)
100-
public void userNoRead() throws IOException, LoginException {
101-
FileSystem fs = FileSystem.get(makeConf());
87+
public void userNoRead() throws IOException {
88+
FileSystem fs = FileSystem.get(new Configuration());
10289
Path p = createFile(fs, new FsPermission(FsAction.NONE, FsAction.ALL, FsAction.ALL));
10390
UserGroupInformation ugi = SecurityUtils.getUGI();
104-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.READ, ugi);
91+
HdfsUtils.checkFileAccess(fs, p, FsAction.READ, ugi);
10592
}
10693

10794
@Test(expected = AccessControlException.class)
108-
public void userNoWrite() throws IOException, LoginException {
109-
FileSystem fs = FileSystem.get(makeConf());
95+
public void userNoWrite() throws IOException {
96+
FileSystem fs = FileSystem.get(new Configuration());
11097
Path p = createFile(fs, new FsPermission(FsAction.NONE, FsAction.ALL, FsAction.ALL));
11198
UserGroupInformation ugi = SecurityUtils.getUGI();
112-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.WRITE, ugi);
99+
HdfsUtils.checkFileAccess(fs, p, FsAction.WRITE, ugi);
113100
}
114101

115102
@Test(expected = AccessControlException.class)
116-
public void userNoExecute() throws IOException, LoginException {
117-
FileSystem fs = FileSystem.get(makeConf());
103+
public void userNoExecute() throws IOException {
104+
FileSystem fs = FileSystem.get(new Configuration());
118105
Path p = createFile(fs, new FsPermission(FsAction.NONE, FsAction.ALL, FsAction.ALL));
119106
UserGroupInformation ugi = SecurityUtils.getUGI();
120-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.EXECUTE, ugi);
107+
HdfsUtils.checkFileAccess(fs, p, FsAction.EXECUTE, ugi);
121108
}
122109

123110
@Test
124-
public void groupReadWriteExecute() throws IOException, LoginException {
125-
FileSystem fs = FileSystem.get(makeConf());
111+
public void groupReadWriteExecute() throws IOException {
112+
FileSystem fs = FileSystem.get(new Configuration());
126113
Path p = createFile(fs, new FsPermission(FsAction.NONE, FsAction.ALL, FsAction.NONE));
127114
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);
115+
HdfsUtils.checkFileAccess(fs, p, FsAction.READ, ugi);
116+
HdfsUtils.checkFileAccess(fs, p, FsAction.WRITE, ugi);
117+
HdfsUtils.checkFileAccess(fs, p, FsAction.EXECUTE, ugi);
131118
}
132119

133120
@Test(expected = AccessControlException.class)
134-
public void groupNoRead() throws IOException, LoginException {
135-
FileSystem fs = FileSystem.get(makeConf());
121+
public void groupNoRead() throws IOException {
122+
FileSystem fs = FileSystem.get(new Configuration());
136123
Path p = createFile(fs, new FsPermission(FsAction.ALL, FsAction.NONE, FsAction.ALL));
137124
UserGroupInformation ugi = ugiInvalidUserValidGroups();
138-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.READ, ugi);
125+
HdfsUtils.checkFileAccess(fs, p, FsAction.READ, ugi);
139126
}
140127

141128
@Test(expected = AccessControlException.class)
142-
public void groupNoWrite() throws IOException, LoginException {
143-
FileSystem fs = FileSystem.get(makeConf());
129+
public void groupNoWrite() throws IOException {
130+
FileSystem fs = FileSystem.get(new Configuration());
144131
Path p = createFile(fs, new FsPermission(FsAction.ALL, FsAction.NONE, FsAction.ALL));
145132
UserGroupInformation ugi = ugiInvalidUserValidGroups();
146-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.WRITE, ugi);
133+
HdfsUtils.checkFileAccess(fs, p, FsAction.WRITE, ugi);
147134
}
148135

149136
@Test(expected = AccessControlException.class)
150-
public void groupNoExecute() throws IOException, LoginException {
151-
FileSystem fs = FileSystem.get(makeConf());
137+
public void groupNoExecute() throws IOException {
138+
FileSystem fs = FileSystem.get(new Configuration());
152139
Path p = createFile(fs, new FsPermission(FsAction.ALL, FsAction.NONE, FsAction.ALL));
153140
UserGroupInformation ugi = ugiInvalidUserValidGroups();
154-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.EXECUTE, ugi);
141+
HdfsUtils.checkFileAccess(fs, p, FsAction.EXECUTE, ugi);
155142
}
156143

157144
@Test
158-
public void otherReadWriteExecute() throws IOException, LoginException {
159-
FileSystem fs = FileSystem.get(makeConf());
145+
public void otherReadWriteExecute() throws IOException {
146+
FileSystem fs = FileSystem.get(new Configuration());
160147
Path p = createFile(fs, new FsPermission(FsAction.NONE, FsAction.NONE, FsAction.ALL));
161148
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);
149+
HdfsUtils.checkFileAccess(fs, p, FsAction.READ, ugi);
150+
HdfsUtils.checkFileAccess(fs, p, FsAction.WRITE, ugi);
151+
HdfsUtils.checkFileAccess(fs, p, FsAction.EXECUTE, ugi);
165152
}
166153

167154
@Test(expected = AccessControlException.class)
168-
public void otherNoRead() throws IOException, LoginException {
169-
FileSystem fs = FileSystem.get(makeConf());
155+
public void otherNoRead() throws IOException {
156+
FileSystem fs = FileSystem.get(new Configuration());
170157
Path p = createFile(fs, new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.NONE));
171158
UserGroupInformation ugi = ugiInvalidUserInvalidGroups();
172-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.READ, ugi);
159+
HdfsUtils.checkFileAccess(fs, p, FsAction.READ, ugi);
173160
}
174161

175162
@Test(expected = AccessControlException.class)
176-
public void otherNoWrite() throws IOException, LoginException {
177-
FileSystem fs = FileSystem.get(makeConf());
163+
public void otherNoWrite() throws IOException {
164+
FileSystem fs = FileSystem.get(new Configuration());
178165
Path p = createFile(fs, new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.NONE));
179166
UserGroupInformation ugi = ugiInvalidUserInvalidGroups();
180-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.WRITE, ugi);
167+
HdfsUtils.checkFileAccess(fs, p, FsAction.WRITE, ugi);
181168
}
182169

183170
@Test(expected = AccessControlException.class)
184-
public void otherNoExecute() throws IOException, LoginException {
185-
FileSystem fs = FileSystem.get(makeConf());
171+
public void otherNoExecute() throws IOException {
172+
FileSystem fs = FileSystem.get(new Configuration());
186173
Path p = createFile(fs, new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.NONE));
187174
UserGroupInformation ugi = ugiInvalidUserInvalidGroups();
188-
HdfsUtils.checkFileAccess(fs, fs.getFileStatus(p), FsAction.EXECUTE, ugi);
175+
HdfsUtils.checkFileAccess(fs, p, FsAction.EXECUTE, ugi);
189176
}
190177

191-
@Test
192-
public void rootReadWriteExecute() throws IOException, LoginException {
193-
UserGroupInformation ugi = SecurityUtils.getUGI();
178+
@Test (expected = FileNotFoundException.class)
179+
public void nonExistentFile() throws IOException {
194180
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-
}
181+
Path p = new Path("/tmp/nosuchfile");
182+
UserGroupInformation ugi = SecurityUtils.getUGI();
183+
HdfsUtils.checkFileAccess(fs, p, FsAction.READ, ugi);
184+
}
185+
186+
@Test(expected = AccessControlException.class)
187+
public void accessPerssionFromCustomFilesystem() throws IOException {
188+
FileSystem fs = new RawLocalFileSystem() {
189+
@Override
190+
public void access(Path path, FsAction mode) throws AccessControlException, IOException {
191+
if (path.toString().contains("noaccess")) {
192+
throw new AccessControlException("no access");
193+
}
194+
}
195+
196+
@Override
197+
public Configuration getConf() {
198+
return new Configuration();
199+
}
200+
};
201+
202+
UserGroupInformation ugi = SecurityUtils.getUGI();
203+
Path p = new Path("/tmp/noaccess");
204+
HdfsUtils.checkFileAccess(fs, p, FsAction.EXECUTE, ugi);
205205
}
206206

207207
/**

0 commit comments

Comments
 (0)