Re: [wildfly-dev] CI testing with the security manager enabled

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. [1]: http://word-bits.flurg.com/presentations/security-manager/index.html#/ [2]: http://word-bits.flurg.com/presentations/security-manager/index.html#/saf... On Tue, Jan 8, 2019 at 10:15 AM Brian Stansberry < brian.stansberry(a)redhat.com&gt; 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, > > > https://github.com/wildfly/wildfly/blob/master/testsuite/integration/basi... > > 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, > > > https://github.com/wildfly/wildfly/blob/master/testsuite/integration/basi... > > 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 > wildfly-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/wildfly-dev -- James R. Perkins JBoss by Red Hat _______________________________________________ wildfly-dev mailing list wildfly-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/wildfly-dev