Skip to content

Commit bcf4bb4

Browse files
kevinjwallskevinjwalls
committed
8333344: JMX attaching of Subject does not work when security manager not allowed
Reviewed-by: weijun, dfuchs
1 parent 856931d commit bcf4bb4

File tree

17 files changed

+162
-49
lines changed

17 files changed

+162
-49
lines changed

‎src/java.base/share/classes/module-info.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@
165165
java.desktop,
166166
java.logging,
167167
java.management,
168+
java.management.rmi,
168169
java.naming,
169170
java.rmi,
170171
jdk.charsets,

‎src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectionImpl.java‎

Lines changed: 69 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import javax.management.remote.JMXServerErrorException;
4747
import javax.management.remote.NotificationResult;
4848
import javax.security.auth.Subject;
49+
import jdk.internal.access.SharedSecrets;
4950
import sun.reflect.misc.ReflectUtil;
5051

5152
import static javax.management.remote.rmi.RMIConnector.Util.cast;
@@ -108,14 +109,19 @@ public RMIConnectionImpl(RMIServerImpl rmiServer,
108109
this.rmiServer = rmiServer;
109110
this.connectionId = connectionId;
110111
this.defaultClassLoader = defaultClassLoader;
111-
112112
this.subject = subject;
113+
113114
if (subject == null) {
114115
this.acc = null;
115116
} else {
116117
// An authenticated Subject was provided.
117118
// Subject Delegation has been removed.
118-
this.acc = JMXSubjectDomainCombiner.getContext(subject);
119+
if (SharedSecrets.getJavaLangAccess().allowSecurityManager()) {
120+
// SM is allowed. Will use ACC created with Subject:
121+
this.acc = JMXSubjectDomainCombiner.getContext(subject);
122+
} else {
123+
this.acc = null;
124+
}
119125
}
120126
this.mbeanServer = rmiServer.getMBeanServer();
121127

@@ -1292,10 +1298,21 @@ public NotificationResult run() {
12921298
return getServerNotifFwd().fetchNotifs(csn, t, mn);
12931299
}
12941300
};
1295-
if (acc == null)
1296-
return action.run();
1297-
else
1298-
return AccessController.doPrivileged(action, acc);
1301+
if (!SharedSecrets.getJavaLangAccess().allowSecurityManager()) {
1302+
// Modern case
1303+
if (subject == null) {
1304+
return action.run();
1305+
} else {
1306+
return Subject.doAs(subject, action);
1307+
}
1308+
} else {
1309+
// SM permitted
1310+
if (acc == null) {
1311+
return action.run(); // No Subject or ACC
1312+
} else {
1313+
return AccessController.doPrivileged(action, acc);
1314+
}
1315+
}
12991316
} finally {
13001317
serverCommunicatorAdmin.rspOutgoing();
13011318
}
@@ -1411,16 +1428,36 @@ private Object doPrivilegedOperation(final int operation,
14111428
serverCommunicatorAdmin.reqIncoming();
14121429
try {
14131430
PrivilegedOperation op = new PrivilegedOperation(operation, params);
1414-
if (acc == null) {
1415-
try {
1416-
return op.run();
1417-
} catch (Exception e) {
1418-
if (e instanceof RuntimeException)
1419-
throw (RuntimeException) e;
1420-
throw new PrivilegedActionException(e);
1431+
if (!SharedSecrets.getJavaLangAccess().allowSecurityManager()) {
1432+
// Modern case
1433+
if (subject == null) {
1434+
try {
1435+
return op.run();
1436+
} catch (Exception e) {
1437+
if (e instanceof RuntimeException) {
1438+
throw (RuntimeException) e;
1439+
} else {
1440+
throw new PrivilegedActionException(e);
1441+
}
1442+
}
1443+
} else {
1444+
return Subject.doAs(subject, op);
14211445
}
14221446
} else {
1423-
return AccessController.doPrivileged(op, acc);
1447+
// SM permitted
1448+
if (acc == null) {
1449+
try {
1450+
return op.run();
1451+
} catch (Exception e) {
1452+
if (e instanceof RuntimeException) {
1453+
throw (RuntimeException) e;
1454+
} else {
1455+
throw new PrivilegedActionException(e);
1456+
}
1457+
}
1458+
} else {
1459+
return AccessController.doPrivileged(op, acc);
1460+
}
14241461
}
14251462
} catch (Error e) {
14261463
throw new JMXServerErrorException(e.toString(),e);
@@ -1585,15 +1622,25 @@ private <T> T unwrap(final MarshalledObject<?> mo,
15851622
}
15861623
try {
15871624
final ClassLoader old = AccessController.doPrivileged(new SetCcl(cl));
1588-
try{
1589-
if (acc != null) {
1590-
return AccessController.doPrivileged(
1591-
(PrivilegedExceptionAction<T>) () ->
1592-
wrappedClass.cast(mo.get()), acc);
1593-
}else{
1594-
return wrappedClass.cast(mo.get());
1625+
try {
1626+
if (!SharedSecrets.getJavaLangAccess().allowSecurityManager()) {
1627+
// Modern case
1628+
if (subject != null) {
1629+
return Subject.doAs(subject, (PrivilegedExceptionAction<T>) () -> wrappedClass.cast(mo.get()));
1630+
} else {
1631+
return wrappedClass.cast(mo.get());
1632+
}
1633+
} else {
1634+
// SM permitted
1635+
if (acc != null) {
1636+
return AccessController.doPrivileged(
1637+
(PrivilegedExceptionAction<T>) () ->
1638+
wrappedClass.cast(mo.get()), acc);
1639+
} else {
1640+
return wrappedClass.cast(mo.get());
1641+
}
15951642
}
1596-
}finally{
1643+
} finally {
15971644
AccessController.doPrivileged(new SetCcl(old));
15981645
}
15991646
} catch (PrivilegedActionException pe) {

‎src/java.management/share/classes/com/sun/jmx/remote/internal/ServerNotifForwarder.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2002, 2022, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2002, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -343,10 +343,9 @@ public void terminate() {
343343
//----------------
344344
// PRIVATE METHODS
345345
//----------------
346-
347346
@SuppressWarnings("removal")
348347
private Subject getSubject() {
349-
return Subject.getSubject(AccessController.getContext());
348+
return Subject.current();
350349
}
351350

352351
private void checkState() throws IOException {

‎src/java.management/share/classes/com/sun/jmx/remote/security/MBeanServerFileAccessController.java‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2003, 2022, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2003, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -42,6 +42,7 @@
4242
import javax.management.MBeanServer;
4343
import javax.management.ObjectName;
4444
import javax.security.auth.Subject;
45+
import jdk.internal.access.SharedSecrets;
4546

4647
/**
4748
* <p>An object of this class implements the MBeanServerAccessController
@@ -300,16 +301,19 @@ private static Properties propertiesFromFile(String fname)
300301
}
301302
}
302303

304+
@SuppressWarnings("removal")
303305
private synchronized void checkAccess(AccessType requiredAccess, String arg) {
304-
@SuppressWarnings("removal")
305-
final AccessControlContext acc = AccessController.getContext();
306-
@SuppressWarnings("removal")
307-
final Subject s =
308-
AccessController.doPrivileged(new PrivilegedAction<>() {
309-
public Subject run() {
310-
return Subject.getSubject(acc);
311-
}
306+
Subject s = null;
307+
if (!SharedSecrets.getJavaLangAccess().allowSecurityManager()) {
308+
s = Subject.current();
309+
} else {
310+
final AccessControlContext acc = AccessController.getContext();
311+
s = AccessController.doPrivileged(new PrivilegedAction<>() {
312+
public Subject run() {
313+
return Subject.getSubject(acc);
314+
}
312315
});
316+
}
313317
if (s == null) return; /* security has not been enabled */
314318
final Set<Principal> principals = s.getPrincipals();
315319
String newPropertyValue = null;

‎src/java.management/share/classes/javax/management/monitor/Monitor.java‎

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 1999, 2022, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 1999, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -60,6 +60,8 @@
6060
import javax.management.NotificationBroadcasterSupport;
6161
import javax.management.ObjectName;
6262
import javax.management.ReflectionException;
63+
import javax.security.auth.Subject;
64+
import jdk.internal.access.SharedSecrets;
6365
import static javax.management.monitor.MonitorNotification.*;
6466

6567
/**
@@ -169,8 +171,9 @@ public final synchronized void setDerivedGaugeTimeStamp(
169171
new CopyOnWriteArrayList<>();
170172

171173
/**
172-
* AccessControlContext of the Monitor.start() caller.
174+
* Subject and possibly AccessControlContext of the Monitor.start() caller.
173175
*/
176+
private volatile Subject subject;
174177
@SuppressWarnings("removal")
175178
private static final AccessControlContext noPermissionsACC =
176179
new AccessControlContext(
@@ -713,10 +716,14 @@ void doStart() {
713716
//
714717
cleanupIsComplexTypeAttribute();
715718

716-
// Cache the AccessControlContext of the Monitor.start() caller.
719+
// Cache the Subject or AccessControlContext of the Monitor.start() caller.
717720
// The monitor tasks will be executed within this context.
718721
//
719-
acc = AccessController.getContext();
722+
if (!SharedSecrets.getJavaLangAccess().allowSecurityManager()) {
723+
subject = Subject.current();
724+
} else {
725+
acc = AccessController.getContext();
726+
}
720727

721728
// Start the scheduler.
722729
//
@@ -747,8 +754,9 @@ void doStop() {
747754
//
748755
cleanupFutures();
749756

750-
// Reset the AccessControlContext.
757+
// Reset the Subject and AccessControlContext.
751758
//
759+
subject = null;
752760
acc = noPermissionsACC;
753761

754762
// Reset the complex type attribute information
@@ -1512,9 +1520,11 @@ public Future<?> submit() {
15121520
@SuppressWarnings("removal")
15131521
public void run() {
15141522
final ScheduledFuture<?> sf;
1523+
final Subject s;
15151524
final AccessControlContext ac;
15161525
synchronized (Monitor.this) {
15171526
sf = Monitor.this.schedulerFuture;
1527+
s = Monitor.this.subject;
15181528
ac = Monitor.this.acc;
15191529
}
15201530
PrivilegedAction<Void> action = new PrivilegedAction<>() {
@@ -1531,10 +1541,20 @@ public Void run() {
15311541
return null;
15321542
}
15331543
};
1534-
if (ac == null) {
1535-
throw new SecurityException("AccessControlContext cannot be null");
1544+
if (!SharedSecrets.getJavaLangAccess().allowSecurityManager()) {
1545+
// No SecurityManager permitted:
1546+
if (s == null) {
1547+
action.run();
1548+
} else {
1549+
Subject.doAs(s, action);
1550+
}
1551+
} else {
1552+
if (ac == null) {
1553+
throw new SecurityException("AccessControlContext cannot be null");
1554+
}
1555+
// ACC means SM is permitted.
1556+
AccessController.doPrivileged(action, ac);
15361557
}
1537-
AccessController.doPrivileged(action, ac);
15381558
synchronized (Monitor.this) {
15391559
if (Monitor.this.isActive() &&
15401560
Monitor.this.schedulerFuture == sf) {

‎test/jdk/javax/management/monitor/StartStopTest.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2005, 2018, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2005, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -32,9 +32,17 @@
3232
*
3333
* @run clean StartStopTest
3434
* @run build StartStopTest
35+
*
3536
* @run main/othervm/timeout=300 StartStopTest 1
3637
* @run main/othervm/timeout=300 StartStopTest 2
3738
* @run main/othervm/timeout=300 StartStopTest 3
39+
* @run main/othervm/timeout=300 -Djava.security.manager=allow StartStopTest 1
40+
* @run main/othervm/timeout=300 -Djava.security.manager=allow StartStopTest 2
41+
* @run main/othervm/timeout=300 -Djava.security.manager=allow StartStopTest 3
42+
* @run main/othervm/timeout=300/policy=all.policy StartStopTest 1
43+
* @run main/othervm/timeout=300/policy=all.policy StartStopTest 2
44+
* @run main/othervm/timeout=300/policy=all.policy StartStopTest 3
45+
*
3846
* @run main/othervm/timeout=300 -Djmx.x.monitor.maximum.pool.size=5 StartStopTest 1
3947
* @run main/othervm/timeout=300 -Djmx.x.monitor.maximum.pool.size=5 StartStopTest 2
4048
* @run main/othervm/timeout=300 -Djmx.x.monitor.maximum.pool.size=5 StartStopTest 3

‎test/jdk/javax/management/monitor/ThreadPoolAccTest.java‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,19 @@
3030
*
3131
* @run clean ThreadPoolAccTest
3232
* @run build ThreadPoolAccTest
33+
*
34+
* @run main/othervm ThreadPoolAccTest
3335
* @run main/othervm -Djava.security.manager=allow ThreadPoolAccTest
36+
* @run main/othervm -Djava.security.manager=allow -DThreadPoolAccTest.useGetSubjectACC=true ThreadPoolAccTest
37+
* @run main/othervm/policy=all.policy ThreadPoolAccTest
38+
* @run main/othervm/policy=all.policy -DThreadPoolAccTest.useGetSubjectACC=true ThreadPoolAccTest
3439
*/
3540

3641
import java.security.AccessController;
3742
import java.security.PrivilegedAction;
3843
import java.util.Date;
3944
import java.util.Set;
45+
import java.util.concurrent.Callable;
4046
import javax.management.MBeanServer;
4147
import javax.management.MBeanServerFactory;
4248
import javax.management.ObjectName;
@@ -67,7 +73,9 @@ public String getString() {
6773
return "";
6874
}
6975
private void setPrincipal() {
70-
Subject subject = Subject.getSubject(AccessController.getContext());
76+
// Use Subject.current() unless test Property is set.
77+
Subject subject = Boolean.getBoolean("ThreadPoolAccTest.useGetSubjectACC") ?
78+
Subject.getSubject(AccessController.getContext()) : Subject.current();
7179
Set<JMXPrincipal> principals = subject.getPrincipals(JMXPrincipal.class);
7280
principal = principals.iterator().next().getName();
7381
}
@@ -136,7 +144,9 @@ public Void run() {
136144
return null;
137145
}
138146
};
139-
Subject.doAs(subject, action);
147+
// Subject.doAs(subject, action);
148+
Callable<Void> c = (Callable<Void>) () -> action.run();
149+
Subject.callAs(subject, c);
140150
}
141151

142152
sleep(500); // wait for getX method to be called, which calls setPrincipal

‎test/jdk/javax/management/monitor/all.policy‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
grant {
2+
permission java.security.AllPermission;
3+
};

‎test/jdk/javax/management/remote/mandatory/notif/NotificationAccessControllerTest.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030
* java.management/com.sun.jmx.remote.security
3131
* @run clean NotificationAccessControllerTest
3232
* @run build NotificationAccessControllerTest
33+
*
34+
* @run main/othervm NotificationAccessControllerTest
3335
* @run main/othervm -Djava.security.manager=allow NotificationAccessControllerTest
3436
*/
3537

‎test/jdk/javax/management/remote/mandatory/notif/NotificationEmissionTest.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2005, 2015, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2005, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -31,7 +31,9 @@
3131
*
3232
* @run clean NotificationEmissionTest
3333
* @run build NotificationEmissionTest
34+
*
3435
* @run main NotificationEmissionTest 1
36+
* @run main/othervm -Djava.security.manager=allow NotificationEmissionTest 1
3537
* @run main/othervm -Djava.security.manager=allow NotificationEmissionTest 2
3638
* @run main/othervm -Djava.security.manager=allow NotificationEmissionTest 3
3739
* @run main/othervm -Djava.security.manager=allow NotificationEmissionTest 4

0 commit comments

Comments
 (0)