stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8344183: (zipfs) SecurityManager cleanup in the ZipFS area

Browse Source 
Reviewed-by: mullan, lancea
This commit is contained in:
Eirik Bjørsnøs 2024-11-15 07:48:15 +00:00
parent 857f68c60f
commit bfee766f03
3 changed files with 28 additions and 82 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

58
src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java
View File

@ -44,10 +44,6 @@ import java.nio.channels.WritableByteChannel;
import java.nio.file.*; import java.nio.file.*;
import java.nio.file.attribute.*; import java.nio.file.attribute.*;
import java.nio.file.spi.FileSystemProvider; import java.nio.file.spi.FileSystemProvider;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
import java.util.*; import java.util.*;
import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock;
@ -82,10 +78,8 @@ import static jdk.nio.zipfs.ZipUtils.*;
*/ */
class ZipFileSystem extends FileSystem { class ZipFileSystem extends FileSystem {
// statics // statics
@SuppressWarnings("removal") private static final boolean isWindows = System.getProperty("os.name")
private static final boolean isWindows = AccessController.doPrivileged( .startsWith("Windows");
(PrivilegedAction<Boolean>)()->System.getProperty("os.name")
.startsWith("Windows"));
private static final byte[] ROOTPATH = new byte[] { '/' }; private static final byte[] ROOTPATH = new byte[] { '/' };
private static final String PROPERTY_POSIX = "enablePosixFileAttributes"; private static final String PROPERTY_POSIX = "enablePosixFileAttributes";
private static final String PROPERTY_DEFAULT_OWNER = "defaultOwner"; private static final String PROPERTY_DEFAULT_OWNER = "defaultOwner";
@ -168,9 +162,7 @@ class ZipFileSystem extends FileSystem {
} }
// sm and existence check // sm and existence check
zfpath.getFileSystem().provider().checkAccess(zfpath, AccessMode.READ); zfpath.getFileSystem().provider().checkAccess(zfpath, AccessMode.READ);
@SuppressWarnings("removal") boolean writeable = Files.isWritable(zfpath);
boolean writeable = AccessController.doPrivileged(
(PrivilegedAction<Boolean>)()->Files.isWritable(zfpath));
this.readOnly = !writeable; this.readOnly = !writeable;
this.zc = ZipCoder.get(nameEncoding); this.zc = ZipCoder.get(nameEncoding);
this.rootdir = new ZipPath(this, new byte[]{'/'}); this.rootdir = new ZipPath(this, new byte[]{'/'});
@ -244,23 +236,14 @@ class ZipFileSystem extends FileSystem {
// If not specified in env, it is the owner of the archive. If no owner can // If not specified in env, it is the owner of the archive. If no owner can
// be determined, we try to go with system property "user.name". If that's not // be determined, we try to go with system property "user.name". If that's not
// accessible, we return "<zipfs_default>". // accessible, we return "<zipfs_default>".
@SuppressWarnings("removal")
private UserPrincipal initOwner(Path zfpath, Map<String, ?> env) throws IOException { private UserPrincipal initOwner(Path zfpath, Map<String, ?> env) throws IOException {
Object o = env.get(PROPERTY_DEFAULT_OWNER); Object o = env.get(PROPERTY_DEFAULT_OWNER);
if (o == null) { if (o == null) {
try { try {
PrivilegedExceptionAction<UserPrincipal> pa = ()->Files.getOwner(zfpath); return Files.getOwner(zfpath);
return AccessController.doPrivileged(pa); } catch (UnsupportedOperationException | NoSuchFileException e) {
} catch (UnsupportedOperationException | PrivilegedActionException e) { String userName = System.getProperty("user.name");
if (e instanceof UnsupportedOperationException || return ()->userName;
e.getCause() instanceof NoSuchFileException)
{
PrivilegedAction<String> pa = ()->System.getProperty("user.name");
String userName = AccessController.doPrivileged(pa);
return ()->userName;
} else {
throw new IOException(e);
}
} }
} }
if (o instanceof String) { if (o instanceof String) {
@ -282,7 +265,6 @@ class ZipFileSystem extends FileSystem {
// If not specified in env, we try to determine the group of the zip archive itself. // If not specified in env, we try to determine the group of the zip archive itself.
// If this is not possible/unsupported, we will return a group principal going by // If this is not possible/unsupported, we will return a group principal going by
// the same name as the default owner. // the same name as the default owner.
@SuppressWarnings("removal")
private GroupPrincipal initGroup(Path zfpath, Map<String, ?> env) throws IOException { private GroupPrincipal initGroup(Path zfpath, Map<String, ?> env) throws IOException {
Object o = env.get(PROPERTY_DEFAULT_GROUP); Object o = env.get(PROPERTY_DEFAULT_GROUP);
if (o == null) { if (o == null) {
@ -291,16 +273,9 @@ class ZipFileSystem extends FileSystem {
if (zfpv == null) { if (zfpv == null) {
return defaultOwner::getName; return defaultOwner::getName;
} }
PrivilegedExceptionAction<GroupPrincipal> pa = ()->zfpv.readAttributes().group(); return zfpv.readAttributes().group();
return AccessController.doPrivileged(pa); } catch (UnsupportedOperationException | NoSuchFileException e) {
} catch (UnsupportedOperationException | PrivilegedActionException e) { return defaultOwner::getName;
if (e instanceof UnsupportedOperationException ||
e.getCause() instanceof NoSuchFileException)
{
return defaultOwner::getName;
} else {
throw new IOException(e);
}
} }
} }
if (o instanceof String) { if (o instanceof String) {
@ -462,7 +437,6 @@ class ZipFileSystem extends FileSystem {
return (path)->pattern.matcher(path.toString()).matches(); return (path)->pattern.matcher(path.toString()).matches();
} }
@SuppressWarnings("removal")
@Override @Override
public void close() throws IOException { public void close() throws IOException {
beginWrite(); beginWrite();
@ -480,13 +454,9 @@ class ZipFileSystem extends FileSystem {
} }
beginWrite(); // lock and sync beginWrite(); // lock and sync
try { try {
AccessController.doPrivileged((PrivilegedExceptionAction<Void>)() -> { sync();
sync(); return null;
});
ch.close(); // close the ch just in case no update ch.close(); // close the ch just in case no update
// and sync didn't close the ch // and sync didn't close the ch
} catch (PrivilegedActionException e) {
throw (IOException)e.getException();
} finally { } finally {
endWrite(); endWrite();
} }
@ -512,10 +482,8 @@ class ZipFileSystem extends FileSystem {
synchronized (tmppaths) { synchronized (tmppaths) {
for (Path p : tmppaths) { for (Path p : tmppaths) {
try { try {
AccessController.doPrivileged( Files.deleteIfExists(p);
(PrivilegedExceptionAction<Boolean>)() -> Files.deleteIfExists(p)); } catch (IOException x) {
} catch (PrivilegedActionException e) {
IOException x = (IOException)e.getException();
if (ioe == null) if (ioe == null)
ioe = x; ioe = x;
else else

15
src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystemProvider.java
View File

@ -1,5 +1,5 @@
/* /*
* Copyright (c) 2009, 2022, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2009, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
* *
* This code is free software; you can redistribute it and/or modify it * This code is free software; you can redistribute it and/or modify it
@ -39,9 +39,6 @@ import java.nio.file.attribute.BasicFileAttributes;
import java.nio.file.attribute.FileAttribute; import java.nio.file.attribute.FileAttribute;
import java.nio.file.attribute.FileAttributeView; import java.nio.file.attribute.FileAttributeView;
import java.nio.file.spi.FileSystemProvider; import java.nio.file.spi.FileSystemProvider;
import java.security.AccessController;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
import java.util.HashMap; import java.util.HashMap;
import java.util.Map; import java.util.Map;
import java.util.Set; import java.util.Set;
@ -317,17 +314,9 @@ public class ZipFileSystemProvider extends FileSystemProvider {
} }
////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////
@SuppressWarnings("removal")
void removeFileSystem(Path zfpath, ZipFileSystem zfs) throws IOException { void removeFileSystem(Path zfpath, ZipFileSystem zfs) throws IOException {
synchronized (filesystems) { synchronized (filesystems) {
Path tempPath = zfpath; filesystems.remove(zfpath.toRealPath(), zfs);
PrivilegedExceptionAction<Path> action = tempPath::toRealPath;
try {
zfpath = AccessController.doPrivileged(action);
} catch (PrivilegedActionException e) {
throw (IOException) e.getException();
}
filesystems.remove(zfpath, zfs);
} }
} }
} }

37
test/jdk/jdk/nio/zipfs/TestPosix.java
View File

@ -29,10 +29,6 @@ import java.nio.ByteBuffer;
import java.nio.ByteOrder; import java.nio.ByteOrder;
import java.nio.file.*; import java.nio.file.*;
import java.nio.file.attribute.*; import java.nio.file.attribute.*;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
import java.time.Instant; import java.time.Instant;
import java.util.*; import java.util.*;
import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicInteger;
@ -219,35 +215,28 @@ public class TestPosix {
private static String expectedDefaultOwner(Path zf) { private static String expectedDefaultOwner(Path zf) {
try { try {
try { return Files.getOwner(zf).getName();
PrivilegedExceptionAction<String> pa = ()->Files.getOwner(zf).getName(); } catch (UnsupportedOperationException e) {
return AccessController.doPrivileged(pa); // if we can't get the owner of the file, we fall back to system property user.name
} catch (UnsupportedOperationException e) { return System.getProperty("user.name");
// if we can't get the owner of the file, we fall back to system property user.name } catch (IOException e) {
PrivilegedAction<String> pa = ()->System.getProperty("user.name");
return AccessController.doPrivileged(pa);
}
} catch (PrivilegedActionException | SecurityException e) {
System.out.println("Caught " + e.getClass().getName() + "(" + e.getMessage() + System.out.println("Caught " + e.getClass().getName() + "(" + e.getMessage() +
") when running a privileged operation to get the default owner."); ") when getting the default owner.");
return null; return null;
} }
} }
private static String expectedDefaultGroup(Path zf, String defaultOwner) { private static String expectedDefaultGroup(Path zf, String defaultOwner) {
try { try {
try { PosixFileAttributeView zfpv = Files.getFileAttributeView(zf, PosixFileAttributeView.class);
PosixFileAttributeView zfpv = Files.getFileAttributeView(zf, PosixFileAttributeView.class); if (zfpv == null) {
if (zfpv == null) {
return defaultOwner;
}
PrivilegedExceptionAction<String> pa = ()->zfpv.readAttributes().group().getName();
return AccessController.doPrivileged(pa);
} catch (UnsupportedOperationException e) {
return defaultOwner; return defaultOwner;
} }
} catch (PrivilegedActionException | SecurityException e) { return zfpv.readAttributes().group().getName();
System.out.println("Caught an exception when running a privileged operation to get the default group."); } catch (UnsupportedOperationException e) {
return defaultOwner;
} catch (IOException e) {
System.out.println("Caught an exception when getting the default group.");
e.printStackTrace(); e.printStackTrace();
return null; return null;
} }