CI testing with the security manager enabled

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

CI testing with the security manager enabled

Brian Stansberry
FYI about some small changes in how we test PRs...

tl;dr; We are running the entire testsuite with a security manager enabled as one of the CI jobs run when PRs are tested. If a PR fails this test, there will need to be some form of correction before merging.


For quite a while now one of the jobs that runs when you submit or update a PR turns on the JVM security manager for the WF processes being tested. But until this year that job only ran a relatively small portion of the overall testsuite. Now it has been modified to run the entire testsuite.

The effect of this is PRs that introduce failures when the security manager is enabled will be detected before merge. For a long time my colleagues in Red Hat QE have run the entire suite with the security manager enabled and filed bug reports for issues. Now we'll catch these before merging, a much more efficient way of dealing with them.

The 3,000 meter overview of how WildFly works with a security manager installed is the code WildFly ships (including libraries from other projects) is intended to have full permissions to do what it wants, but code in deployments needs to be granted permissions. Whether a particular call that results in a security manager check results in the deployment code needing a permission depends on whether the call stack involves deployment code, and if does, whether WildFly code higher in the stack used a privileged block to limit the part of the stack the security manager evaluates for permissions.

If your PR has a failure in the security manager job, here's what you can do:

Do some thinking about whether the failure is because a deployment you are introducing in the test is doing something for which it's reasonable that the deployment would need permissions. For example, it's directly opening an HttpClient, or is directly reading a system property or is directly reading a file.

1) If the answer is CLEARLY YES, then

a) modify the test to stop doing that or
b) use the utils in the WildFly testsuite codebase to add a permissions.xml to the test deployment such that it gets the permissions it needs.[1]

2) If the answer is CLEARLY NO, then WildFly should have been using a privileged block for something, or is doing something unnecessary, so

a) if it was your PR that introduced the problem, fix the PR.
b) else file a WFLY or WFCORE JIRA showing the problem, including a stack trace of the security manager failure. You can then use a utility in the WildFly testsuite code to ignore the test in the security manager test.[2] Include a note in the JIRA that you've done this so removing the ignore can be part of resolving the JIRA.

3) If the answer is NOT SURE, do not just add a permission in a permissions.xml! Don't sweep a possible problem under the rug. Instead, try to have a discussion with the PR reviewers or in chat or here to get some feedback, and once you've gone as far as you're willing with that, file a JIRA for the problem and ignore the test when the security manager is enabled[2].


My thanks to the many folks over the years who've worked to get all but a tiny fraction of our thousands of tests passing with a security manager. And especially to James Perkins and Martin Choma who've really pushed on this this year.

[1] For example,


This grants the deployment the permission to read a lot of system properties. That looks scary but it's just because the test wants to read a lot of properties.

[2] For example, 


Please include the comment showing the issue that was filed; that helps make it easier to find and remove these once the JIRA is resolved.

Best regards,
Brian

_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: CI testing with the security manager enabled

James Perkins
Seems like a good time to bring this back up :) David did some slides [1] a while back now with useful information about the security manager and when/how to use privileged blocks. Reviewing [2] is specifically useful when you're writing a privileged block.

On Tue, Jan 8, 2019 at 10:15 AM Brian Stansberry <[hidden email]> wrote:
FYI about some small changes in how we test PRs...

tl;dr; We are running the entire testsuite with a security manager enabled as one of the CI jobs run when PRs are tested. If a PR fails this test, there will need to be some form of correction before merging.


For quite a while now one of the jobs that runs when you submit or update a PR turns on the JVM security manager for the WF processes being tested. But until this year that job only ran a relatively small portion of the overall testsuite. Now it has been modified to run the entire testsuite.

The effect of this is PRs that introduce failures when the security manager is enabled will be detected before merge. For a long time my colleagues in Red Hat QE have run the entire suite with the security manager enabled and filed bug reports for issues. Now we'll catch these before merging, a much more efficient way of dealing with them.

The 3,000 meter overview of how WildFly works with a security manager installed is the code WildFly ships (including libraries from other projects) is intended to have full permissions to do what it wants, but code in deployments needs to be granted permissions. Whether a particular call that results in a security manager check results in the deployment code needing a permission depends on whether the call stack involves deployment code, and if does, whether WildFly code higher in the stack used a privileged block to limit the part of the stack the security manager evaluates for permissions.

If your PR has a failure in the security manager job, here's what you can do:

Do some thinking about whether the failure is because a deployment you are introducing in the test is doing something for which it's reasonable that the deployment would need permissions. For example, it's directly opening an HttpClient, or is directly reading a system property or is directly reading a file.

1) If the answer is CLEARLY YES, then

a) modify the test to stop doing that or
b) use the utils in the WildFly testsuite codebase to add a permissions.xml to the test deployment such that it gets the permissions it needs.[1]

2) If the answer is CLEARLY NO, then WildFly should have been using a privileged block for something, or is doing something unnecessary, so

a) if it was your PR that introduced the problem, fix the PR.
b) else file a WFLY or WFCORE JIRA showing the problem, including a stack trace of the security manager failure. You can then use a utility in the WildFly testsuite code to ignore the test in the security manager test.[2] Include a note in the JIRA that you've done this so removing the ignore can be part of resolving the JIRA.

3) If the answer is NOT SURE, do not just add a permission in a permissions.xml! Don't sweep a possible problem under the rug. Instead, try to have a discussion with the PR reviewers or in chat or here to get some feedback, and once you've gone as far as you're willing with that, file a JIRA for the problem and ignore the test when the security manager is enabled[2].


My thanks to the many folks over the years who've worked to get all but a tiny fraction of our thousands of tests passing with a security manager. And especially to James Perkins and Martin Choma who've really pushed on this this year.

[1] For example,


This grants the deployment the permission to read a lot of system properties. That looks scary but it's just because the test wants to read a lot of properties.

[2] For example, 


Please include the comment showing the issue that was filed; that helps make it easier to find and remove these once the JIRA is resolved.

Best regards,
Brian
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev


--
James R. Perkins
JBoss by Red Hat

_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: CI testing with the security manager enabled

Brian Stansberry
Thanks, James. +1 to being careful about privileged blocks.

If one of these tests fail because some call path was missing permissions, that's not so much a security problem as it is a usability problem with a workaround. The software is harder to use. Probably the worst thing to do in such a case is to fix the usability problem by introducing an incorrect privileged block, as that converts the usability problem into a security problem.

On Tue, Jan 8, 2019 at 12:33 PM James Perkins <[hidden email]> wrote:
Seems like a good time to bring this back up :) David did some slides [1] a while back now with useful information about the security manager and when/how to use privileged blocks. Reviewing [2] is specifically useful when you're writing a privileged block.

On Tue, Jan 8, 2019 at 10:15 AM Brian Stansberry <[hidden email]> wrote:
FYI about some small changes in how we test PRs...

tl;dr; We are running the entire testsuite with a security manager enabled as one of the CI jobs run when PRs are tested. If a PR fails this test, there will need to be some form of correction before merging.


For quite a while now one of the jobs that runs when you submit or update a PR turns on the JVM security manager for the WF processes being tested. But until this year that job only ran a relatively small portion of the overall testsuite. Now it has been modified to run the entire testsuite.

The effect of this is PRs that introduce failures when the security manager is enabled will be detected before merge. For a long time my colleagues in Red Hat QE have run the entire suite with the security manager enabled and filed bug reports for issues. Now we'll catch these before merging, a much more efficient way of dealing with them.

The 3,000 meter overview of how WildFly works with a security manager installed is the code WildFly ships (including libraries from other projects) is intended to have full permissions to do what it wants, but code in deployments needs to be granted permissions. Whether a particular call that results in a security manager check results in the deployment code needing a permission depends on whether the call stack involves deployment code, and if does, whether WildFly code higher in the stack used a privileged block to limit the part of the stack the security manager evaluates for permissions.

If your PR has a failure in the security manager job, here's what you can do:

Do some thinking about whether the failure is because a deployment you are introducing in the test is doing something for which it's reasonable that the deployment would need permissions. For example, it's directly opening an HttpClient, or is directly reading a system property or is directly reading a file.

1) If the answer is CLEARLY YES, then

a) modify the test to stop doing that or
b) use the utils in the WildFly testsuite codebase to add a permissions.xml to the test deployment such that it gets the permissions it needs.[1]

2) If the answer is CLEARLY NO, then WildFly should have been using a privileged block for something, or is doing something unnecessary, so

a) if it was your PR that introduced the problem, fix the PR.
b) else file a WFLY or WFCORE JIRA showing the problem, including a stack trace of the security manager failure. You can then use a utility in the WildFly testsuite code to ignore the test in the security manager test.[2] Include a note in the JIRA that you've done this so removing the ignore can be part of resolving the JIRA.

3) If the answer is NOT SURE, do not just add a permission in a permissions.xml! Don't sweep a possible problem under the rug. Instead, try to have a discussion with the PR reviewers or in chat or here to get some feedback, and once you've gone as far as you're willing with that, file a JIRA for the problem and ignore the test when the security manager is enabled[2].


My thanks to the many folks over the years who've worked to get all but a tiny fraction of our thousands of tests passing with a security manager. And especially to James Perkins and Martin Choma who've really pushed on this this year.

[1] For example,


This grants the deployment the permission to read a lot of system properties. That looks scary but it's just because the test wants to read a lot of properties.

[2] For example, 


Please include the comment showing the issue that was filed; that helps make it easier to find and remove these once the JIRA is resolved.

Best regards,
Brian
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev


--
James R. Perkins
JBoss by Red Hat
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev


--
Brian Stansberry
Manager, Senior Principal Software Engineer
Red Hat

_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev