Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove setting of null security manager #481

Merged
merged 1 commit into from
Sep 24, 2021

Conversation

timja
Copy link
Member

@timja timja commented Sep 16, 2021

Running an agent on Java 17:

WARNING: A terminally deprecated method in java.lang.System has been called
WARNING: System::setSecurityManager has been called by hudson.remoting.jnlp.Main (file:/Users/timja/Downloads/agent.jar)
WARNING: Please consider reporting this to the maintainers of hudson.remoting.jnlp.Main
WARNING: System::setSecurityManager will be removed in a future release

@timja timja added the chore For changelog: A maintenance chore with no functional changes label Sep 16, 2021
@timja timja requested review from jeffret-b and jglick September 16, 2021 21:50
Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

All of these links are dead.

Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

Maybe not a chore, but whatever. Thanks for the patch!

@oleg-nenashev
Copy link
Member

We may merge it in 24 hours if there is no negative feedback. Please see the merge process documentation for more information about the merge process

@timja
Copy link
Member Author

timja commented Sep 24, 2021

@jeffret-b / @oleg-nenashev can we ship this?

@jeffret-b
Copy link
Contributor

@timja , sure, that's fine.

@jeffret-b jeffret-b merged commit 9c73fcb into master Sep 24, 2021
@jeffret-b jeffret-b deleted the remove-setting-of-security-manager branch September 24, 2021 13:29
@timja
Copy link
Member Author

timja commented Oct 13, 2021

Could this be released @jeffret-b @oleg-nenashev ?

@AckermannC
Copy link

I think this change is causing an issue when trying to install the jenkins agent using java webstart.
See this issue: https://issues.jenkins.io/browse/JENKINS-67000

@MarkEWaite
Copy link
Contributor

I think this change is causing an issue when trying to install the jenkins agent using java webstart. See this issue: https://issues.jenkins.io/browse/JENKINS-67000

Based on https://stackoverflow.com/questions/19487407/impact-of-system-setsecuritymanagernull, I think that this was the right change to make. Disabling applet security checks seems like a very risky choice.

@AckermannC
Copy link

I think this change is causing an issue when trying to install the jenkins agent using java webstart. See this issue: https://issues.jenkins.io/browse/JENKINS-67000

Based on https://stackoverflow.com/questions/19487407/impact-of-system-setsecuritymanagernull, I think that this was the right change to make. Disabling applet security checks seems like a very risky choice.

I get it, security is important to you and you don't think time should be wasted on fixing old technology... You are very outspoken on this in case 67000.
But there is still the launch-button offered in jenkins and in some environments it makes the work much easier.
If there is a more secure way to fix this great. It just would be nice if this would be implemented in the next version,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore For changelog: A maintenance chore with no functional changes ready-to-merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants