[wildfly-dev] 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 <jperkins at redhat.com> 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.
>
> [1]:
> http://word-bits.flurg.com/presentations/security-manager/index.html#/
> [2]:
> http://word-bits.flurg.com/presentations/security-manager/index.html#/safe-privilege
>
> On Tue, Jan 8, 2019 at 10:15 AM Brian Stansberry <
> brian.stansberry at redhat.com> 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/basic/src/test/java/org/wildfly/test/integration/microprofile/config/smallrye/app/MicroProfileConfigTestCase.java#L71
>>
>> 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/basic/src/test/java/org/jboss/as/test/integration/jpa/hibernate/classfiletransformertest/ClassFileTransformerTestCase.java#L70
>>
>> 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 at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>
>
>
> --
> James R. Perkins
> JBoss by Red Hat
> _______________________________________________
> wildfly-dev mailing list
> wildfly-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/wildfly-dev



-- 
Brian Stansberry
Manager, Senior Principal Software Engineer
Red Hat
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/wildfly-dev/attachments/20190108/de0f0725/attachment.html