8344034: Remove security manager dependency in Serialization #22041

RogerRiggs · 2024-11-12T15:09:23Z

After JDK-8338411, Serialization implementation dependencies on SecurityManager, doPrivildged, and AccessController are removed.
Some refactoring to cleanup the remaining code is expected.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8344034: Remove security manager dependency in Serialization (Bug - P4)

Reviewers

Sean Mullan (@seanjmullan - Reviewer)
Alan Bateman (@AlanBateman - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/22041/head:pull/22041
$ git checkout pull/22041

Update a local copy of the PR:
$ git checkout pull/22041
$ git pull https://git.openjdk.org/jdk.git pull/22041/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 22041

View PR using the GUI difftool:
$ git pr show -t 22041

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/22041.diff

Using Webrev

Link to Webrev Comment

Co-authored-by: Sean Mullan <[email protected]> Co-authored-by: Alan Bateman <[email protected]> Co-authored-by: Weijun Wang <[email protected]> Co-authored-by: Aleksei Efimov <[email protected]> Co-authored-by: Brian Burkhalter <[email protected]> Co-authored-by: Daniel Fuchs <[email protected]> Co-authored-by: Harshitha Onkar <[email protected]> Co-authored-by: Joe Wang <[email protected]> Co-authored-by: Jorn Vernee <[email protected]> Co-authored-by: Justin Lu <[email protected]> Co-authored-by: Kevin Walls <[email protected]>> Co-authored-by: Lance Andersen <[email protected]> Co-authored-by: Naoto Sato <[email protected]> Co-authored-by: Roger Riggs <[email protected]> Co-authored-by: Brent Christian <[email protected]>

…java, not useful without SM

…stance.

setInitialContextFactoryBuilder and setObjectFactoryBuilder methods in javax.naming.spi.NamingManager.

permission cannot be used anymore to control access.

description.

…sion checks of the Class.getNestHost and getNestMembers methods, which no longer apply.

…ested by other tests

…ava, useless without SM

ModuleDescriptor.location().

…attacker to create new instances

…Class<?> clazz, int which)

turbanoff · 2024-11-13T06:37:30Z

src/java.base/share/classes/java/io/ObjectInputFilter.java

Should all usages of SecurityManager be removed from this class?

AlanBateman · 2024-11-13T11:28:26Z

ObjectInputFilter.setSerialFilter and setSerialFilterFactory, have they been missed?

…StreamClass

src/java.base/share/classes/java/io/ObjectStreamField.java

seanjmullan · 2024-11-14T13:58:25Z

src/java.base/share/classes/java/io/ObjectOutputStream.java

-        java.security.AccessController.doPrivileged(
-            new sun.security.action.GetBooleanAction(
-                "sun.io.serialization.extendedDebugInfo")).booleanValue();
+    private static final boolean extendedDebugInfo = Boolean.getBoolean("sun.io.serialization.extendedDebugInfo");


Nit: bit of a long line, consider breaking into 2 lines.

seanjmullan

Looks good other than 2 small comments.

src/java.base/share/classes/java/io/ObjectInputStream.java

…tType.

seanjmullan

Updates look good.

RogerRiggs · 2024-11-14T20:30:15Z

Updates passed CI tier1, tier2, tier3

RogerRiggs · 2024-11-18T16:15:49Z

/integrate

openjdk · 2024-11-18T16:17:11Z

Going to push as commit 9b0ab92.
Since your change was applied there have been 67 commits pushed to the master branch:

d52d136: 8344221: Remove calls to SecurityManager and and doPrivileged in java.net.IDN, java.net.URL, java.net.URLConnection, sun.net.util.URLUtil, and java.net.URLStreamHandlerProvider after JEP 486 integration
dfddbca: 8341916: Remove ProtectionDomain related hotspot code and tests
5eb0733: 8344383: Include ZipArchive and JarArchive directly
b8b70c8: 8344379: [s390x] build failure due to missing change from JDK-8339466
5fc4322: 8288298: Resolve multiline message parsing ambiguities in UL
ea8f289: 8344271: Comparison build fails due to difference in doc summary
b9c6ce9: 8344122: IGV: Extend c2 IdealGraphPrinter to send subgraphs to IGV
00ff6a3: 8344105: Remove SecurityManager and related calls from jdk.attach and jdk.hotspot.agent
475feb0: 8344056: Use markdown format for man pages
6c2ae44: 8344204: IGV: Button to enable/disable cutting of long edges
... and 57 more: https://git.openjdk.org/jdk/compare/752e1629555f0ec8630373ec87b049afdd709ea6...master

Your commit was automatically rebased without conflicts.

openjdk · 2024-11-18T16:17:24Z

@RogerRiggs Pushed as commit 9b0ab92.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

AlanBateman and others added 30 commits September 30, 2024 18:02

ServerSocket bind/accept no longer throw SecurityException

591d3d6

Merge

781eecd

hack out Security Manager from LoaderHandler

4035376

Update spec for RMIClassLoader

6d2006d

Fix whitespace.

6c325c2

Restore URL parsing to LoaderHandler::getClassLoader.

463d46b

Merge

439ed26

Remove vmTestbase/nsk/jvmti/AttachOnDemand/attach004/TestDescription.…

53e06d0

…java, not useful without SM

Remove leftover SecurityException text from RMIConnection.getObjectIn…

9e1cec8

…stance.

Remove leftover "@see java.lang.SecurityManager#checkSetFactory" from

7cf5824

setInitialContextFactoryBuilder and setObjectFactoryBuilder methods in javax.naming.spi.NamingManager.

Remove unnecessary comma.

77b58bf

Merge

b5f2ca3

Fix typo.

938737a

Remove text about granting permission and add api note indicating

2612400

permission cannot be used anymore to control access.

Remove text about implementers using Security Manager checks from class

4688f78

description.

Remove Security Manager text from EXIT_ON_CLOSE.

34da8c7

Removed TestSecurityManagerChecks.java - this test was testing permis…

c62c4ad

…sion checks of the Class.getNestHost and getNestMembers methods, which no longer apply.

Drop reference to permissions from Font.getFont

d8e15dd

Merge

29c0bb2

Update record tests to not require SM

23e0f09

Remove tests for repeating annotations with SM set, no SM case well t…

bd27286

…ested by other tests

Change reflect/Nestmates/TestReflectionAPI to not test with a SM set

7f451ea

Remove test/langtools/tools/javac/api/ToolProvider/ToolProviderTest.j…

c9ab253

…ava, useless without SM

Remove text about granting permissions to module from

36522ed

ModuleDescriptor.location().

Remove text about Proxy objects being granted AllPermission.

c769f2d

Remove text about needing ReflectPermission to access AccessibleObject.

79b4b21

JDK-8003881: Prevent lambda implementing inner classes from allowing …

e46d99c

…attacker to create new instances

JDK-8007035: deprecate public void SecurityManager.checkMemberAccess(…

422c489

…Class<?> clazz, int which)

Update copyright header

1f4be6f

RogerRiggs added 2 commits November 12, 2024 14:14

Merge branch 'master' into 8344034-sm-removal-serialization

1317f47

Cleanup of suppress warnings and caller sensitive from review comments.

3f85083

openjdk bot removed the merge-conflict Pull request has merge conflict with target branch label Nov 12, 2024

turbanoff reviewed Nov 13, 2024

View reviewed changes

src/java.base/share/classes/java/io/ObjectInputFilter.java

Copy link

Member

turbanoff Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should all usages of SecurityManager be removed from this class?

Remove overlooked SecurityManager use in ObjectInputFilter and Object…

3e56ef0

…StreamClass

AlanBateman reviewed Nov 13, 2024

View reviewed changes

src/java.base/share/classes/java/io/ObjectStreamField.java Show resolved Hide resolved

RogerRiggs added 2 commits November 13, 2024 10:27

Remove CallerSensitive and suppress warnings

8c1fc41

Remove unused imports

054b410

seanjmullan reviewed Nov 14, 2024

View reviewed changes

seanjmullan approved these changes Nov 14, 2024

View reviewed changes

src/java.base/share/classes/java/io/ObjectInputStream.java Outdated Show resolved Hide resolved

openjdk bot added the ready Pull request is ready to be integrated label Nov 14, 2024

RogerRiggs added 2 commits November 14, 2024 10:25

Merge branch 'master' into 8344034-sm-removal-serialization

6e40d2c

Fix test to longer check for CallerSensistive on ObjectStreamField.ge…

b8e8d42

…tType.

openjdk bot removed the ready Pull request is ready to be integrated label Nov 14, 2024

RogerRiggs added 2 commits November 14, 2024 12:10

Wrap long lines

bd82304

Wrap more long lines

c67a212

seanjmullan approved these changes Nov 14, 2024

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Nov 14, 2024

AlanBateman approved these changes Nov 16, 2024

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Nov 18, 2024

openjdk bot closed this Nov 18, 2024

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Nov 18, 2024

eirbjo mentioned this pull request Nov 18, 2024

8344365: SecurityManager cleanups in java.sql and java.sql.rowset modules #22185

Closed

3 tasks

RogerRiggs deleted the 8344034-sm-removal-serialization branch February 24, 2025 14:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

8344034: Remove security manager dependency in Serialization #22041

8344034: Remove security manager dependency in Serialization #22041

Uh oh!

RogerRiggs commented Nov 12, 2024 •

edited by openjdk bot

Loading

Uh oh!

turbanoff Nov 13, 2024

Uh oh!

AlanBateman commented Nov 13, 2024

Uh oh!

Uh oh!

seanjmullan Nov 14, 2024

Uh oh!

seanjmullan left a comment

Uh oh!

Uh oh!

seanjmullan left a comment

Uh oh!

RogerRiggs commented Nov 14, 2024

Uh oh!

RogerRiggs commented Nov 18, 2024

Uh oh!

openjdk bot commented Nov 18, 2024

Uh oh!

openjdk bot commented Nov 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

13 participants

8344034: Remove security manager dependency in Serialization #22041

8344034: Remove security manager dependency in Serialization #22041

Uh oh!

Conversation

RogerRiggs commented Nov 12, 2024 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

turbanoff Nov 13, 2024

Choose a reason for hiding this comment

Uh oh!

AlanBateman commented Nov 13, 2024

Uh oh!

Uh oh!

seanjmullan Nov 14, 2024

Choose a reason for hiding this comment

Uh oh!

seanjmullan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

seanjmullan left a comment

Choose a reason for hiding this comment

Uh oh!

RogerRiggs commented Nov 14, 2024

Uh oh!

RogerRiggs commented Nov 18, 2024

Uh oh!

openjdk bot commented Nov 18, 2024

Uh oh!

openjdk bot commented Nov 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

13 participants

RogerRiggs commented Nov 12, 2024 •

edited by openjdk bot

Loading