diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 000000000..48ad4ab77 --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1 @@ +* @jenkinsci/script-security-plugin-developers diff --git a/.github/dependabot.yml b/.github/dependabot.yml index f3f604548..2e90055d3 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -4,10 +4,6 @@ updates: directory: "/" schedule: interval: "daily" - ignore: - # Caffeine 3.x requires Java 11, so we cannot update until Jenkins requires Java 11 - - dependency-name: "com.github.ben-manes.caffeine:caffeine" - versions: ["[3.0.0,)"] - package-ecosystem: github-actions directory: / schedule: diff --git a/.github/release-drafter.yml b/.github/release-drafter.yml deleted file mode 100644 index 0d0b1c994..000000000 --- a/.github/release-drafter.yml +++ /dev/null @@ -1 +0,0 @@ -_extends: .github diff --git a/.github/workflows/cd.yaml b/.github/workflows/cd.yaml index 8fa03bb92..0279984d7 100644 --- a/.github/workflows/cd.yaml +++ b/.github/workflows/cd.yaml @@ -8,52 +8,8 @@ on: - completed jobs: - validate: - runs-on: ubuntu-latest - outputs: - should_release: ${{ steps.verify-ci-status.outputs.result == 'success' && steps.interesting-categories.outputs.interesting == 'true' }} - steps: - - name: Verify CI status - uses: jenkins-infra/verify-ci-status-action@v1.2.0 - id: verify-ci-status - with: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - output_result: true - - - name: Release Drafter - uses: release-drafter/release-drafter@v5 - if: steps.verify-ci-status.outputs.result == 'success' - with: - name: next - tag: next - version: next - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - - name: Check interesting categories - uses: jenkins-infra/interesting-category-action@v1.0.0 - id: interesting-categories - if: steps.verify-ci-status.outputs.result == 'success' - with: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - release: - runs-on: ubuntu-latest - needs: [validate] - if: needs.validate.outputs.should_release == 'true' - steps: - - name: Check out - uses: actions/checkout@v3 - with: - fetch-depth: 0 - - name: Set up JDK 8 - uses: actions/setup-java@v3 - with: - distribution: 'temurin' - java-version: 8 - - name: Release - uses: jenkins-infra/jenkins-maven-cd-action@v1.2.0 - with: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - MAVEN_USERNAME: ${{ secrets.MAVEN_USERNAME }} - MAVEN_TOKEN: ${{ secrets.MAVEN_TOKEN }} + maven-cd: + uses: jenkins-infra/github-reusable-workflows/.github/workflows/maven-cd.yml@v1 + secrets: + MAVEN_USERNAME: ${{ secrets.MAVEN_USERNAME }} + MAVEN_TOKEN: ${{ secrets.MAVEN_TOKEN }} diff --git a/.github/workflows/jenkins-security-scan.yml b/.github/workflows/jenkins-security-scan.yml new file mode 100644 index 000000000..c7b41fc29 --- /dev/null +++ b/.github/workflows/jenkins-security-scan.yml @@ -0,0 +1,21 @@ +name: Jenkins Security Scan + +on: + push: + branches: + - master + pull_request: + types: [ opened, synchronize, reopened ] + workflow_dispatch: + +permissions: + security-events: write + contents: read + actions: read + +jobs: + security-scan: + uses: jenkins-infra/jenkins-security-scan/.github/workflows/jenkins-security-scan.yaml@v2 + with: + java-cache: 'maven' # Optionally enable use of a build dependency cache. Specify 'maven' or 'gradle' as appropriate. + # java-version: 21 # Optionally specify what version of Java to set up for the build, or remove to use a recent default. diff --git a/.gitignore b/.gitignore index 75bad940c..d055f499a 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,8 @@ target work - +/.classpath +/.project +/.settings/ .idea *.iml .*.sw* diff --git a/.mvn/extensions.xml b/.mvn/extensions.xml index a65d82e1b..4e0774d51 100644 --- a/.mvn/extensions.xml +++ b/.mvn/extensions.xml @@ -2,6 +2,6 @@ io.jenkins.tools.incrementals git-changelist-maven-extension - 1.3 + 1.8 diff --git a/CHANGELOG.md b/CHANGELOG.md deleted file mode 100644 index ce33869b3..000000000 --- a/CHANGELOG.md +++ /dev/null @@ -1,704 +0,0 @@ -# Changelog - -From of version 1.77 see [GitHub Releases](https://github.com/jenkinsci/script-security-plugin/releases) - -## Version 1.76 - -Release date: 2021-02-01 - -* Improvement: Add the following to the default list of approved signatures ([PR #308](https://github.com/jenkinsci/script-security-plugin/pull/308), [PR #310](https://github.com/jenkinsci/script-security-plugin/pull/310)): - * All static methods and fields in `java.lang.Math` - * All methods related to `java.lang.StringBuilder` and `java.lang.StringBuffer` - * All methods related to `java.lang.CharSequence` and `java.lang.String` apart from `String.intern()` - * All static methods and fields in `java.nio.charset.Charset` - * All methods related to `java.util.Base64`, `java.util.Base64.Decoder`, and `java.util.Base64.Encoder` -* Internal: Update dependencies and parent POM ([PR #311](https://github.com/jenkinsci/script-security-plugin/pull/311), [PR #313](https://github.com/jenkinsci/script-security-plugin/pull/313), [PR #314](https://github.com/jenkinsci/script-security-plugin/pull/314), [PR #316](https://github.com/jenkinsci/script-security-plugin/pull/316), [PR #317](https://github.com/jenkinsci/script-security-plugin/pull/317), [PR #321](https://github.com/jenkinsci/script-security-plugin/pull/321), [PR #323](https://github.com/jenkinsci/script-security-plugin/pull/323), [PR #324](https://github.com/jenkinsci/script-security-plugin/pull/324), [PR #326](https://github.com/jenkinsci/script-security-plugin/pull/326)) - -## Version 1.75 - -Release date: 2020-09-23 - -* [Fix sandbox bypass vulnerability](https://jenkins.io/security/advisory/2020-09-23/#SECURITY-2020) -* Improvement: Add the following to the default list of approved signatures: - * `DefaultGroovyMethods.and(Boolean, Boolean)` - * `DefaultGroovyMethods.toBoolean(Boolean)` - * `DefaultGroovyMethods.toDouble(String)` - * `StringGroovyMethods.toBoolean(String)` - * `StringGroovyMethods.toDouble(CharSequence)` - * `StringGroovyMethods.toDouble(String)` - * `StringGroovyMethods.toInteger(CharSequence)` - * `StringGroovyMethods.toInteger(String)` - -## Version 1.74 - -Release date: 2020-06-30 - -* Improvement: On the Manage Jenkins page in Jenkins 2.226 and newer, display the link to the In-process Script Approval page under "Security" instead of "Uncategorized". ([PR 302](https://github.com/jenkinsci/script-security-plugin/pull/302)) -* Improvement: Add the following to the list of approved Jenkins-related signatures: - * `BallColor.getHtmlBaseColor` - * `Result.color` - * `Result.fromString(String)` - -## Version 1.73 - -Release date: 2020-06-03 - -* Fix security vulnerability. ([SECURITY-1866](https://www.jenkins.io/security/advisory/2020-06-03/#SECURITY-1866)) - -## Version 1.72 - -Release date: 2020-05-11 - -* This plugin now requires Jenkins 2.176.4 or newer. -* Improvement: Add various methods to the default list of approved signatures: ([JENKINS-61952](https://issues.jenkins-ci.org/browse/JENKINS-61952), [PR 242](https://github.com/jenkinsci/script-security-plugin/pull/242), [PR 295](https://github.com/jenkinsci/script-security-plugin/pull/295), [PR 296](https://github.com/jenkinsci/script-security-plugin/pull/296)) - * Remaining `java.util.regex.Matcher` methods - * Methods related to `java.time.Instant` - * Methods and fields defined on `java.text.DateFormat` - * Most methods defined on `java.text.Format` - * Methods and fields defined on `java.util.Calendar` - * `Boolean.booleanValue` - * `Collection.containsAll(Collection)` - * `List.indexOf(Object)` - * Various extension methods defined in `DefaultGroovyMethods` -* Improvement: Make `SecureGroovyScript` and `ClasspathEntry` serializable so that they can be used by Active Choices Plugin. ([JENKINS-39742](https://issues.jenkins-ci.org/browse/JENKINS-39742)) -* Fix: Clear static field signatures correctly when signature approvals are reset. ([PR 290](https://github.com/jenkinsci/script-security-plugin/pull/290)) -* Internal: Update parent POM and minimum required Jenkins version to fix build errors when testing against new versions of Jenkins. ([PR 293](https://github.com/jenkinsci/script-security-plugin/pull/293)) -* Internal: Update caffeine dependency to 2.8.2. ([PR 294](https://github.com/jenkinsci/script-security-plugin/pull/294)) - -## Version 1.71 - -Release date: 2020-03-09 - -* [Fix sandbox bypass vulnerability](https://jenkins.io/security/advisory/2020-03-09/#SECURITY-1754) - -## Version 1.70 - -Release date: 2020-03-18 - -* [Fix sandbox bypass vulnerability](https://jenkins.io/security/advisory/2020-02-12/#SECURITY-1713) - -## Version 1.69 - -Release date: 2020-01-27 - -* Improvement: Add various methods to the default list of approved signatures: ([PR 280](https://github.com/jenkinsci/script-security-plugin/pull/280), [PR 281](https://github.com/jenkinsci/script-security-plugin/pull/281), [PR 283](https://github.com/jenkinsci/script-security-plugin/pull/283)) - * All remaining static methods in the `java.util.Collections` class - * Groovy's `List.getAt(Collection)` extension method - * Groovy's `List.transpose()` extension method - * `Integer.parse(String, int)` - * All of the fields in the `java.time.DayOfWeek` enum -* Internal: Add better logging for issues encountered in tests, update test-scope dependencies. ([PR 279](https://github.com/jenkinsci/script-security-plugin/pull/279), [PR 284](https://github.com/jenkinsci/script-security-plugin/pull/284)) - -## Version 1.68 - -Release date: 2019-11-21 - -* [Fix sandbox bypass vulnerability](https://jenkins.io/security/advisory/2019-11-21/#SECURITY-1658) - -## Version 1.67 - -Release date: 2019-11-13 - -* Fix: Remove approved signatures that did not correspond to real signatures. ([PR 268](https://github.com/jenkinsci/script-security-plugin/pull/268)) -* Improvement: Add the following to the default list of approved signatures: - * `Object[].getAt(IntRange)` - * All remaining methods in the `java.util.regex` package - * Getters/setters on `Date` - * Various extension methods defined in `DateGroovyMethods` -* Internal: Migrate Wiki content to GitHub. ([PR 264](https://github.com/jenkinsci/script-security-plugin/pull/264)) - -## Version 1.66 - -Release date: 2019-10-01 - -* [JENKINS-59587](https://issues.jenkins-ci.org/browse/JENKINS-59587) - Fix issue that caused a cache used by the class loader for sandboxed Groovy scripts to be cleared out by the garbage collector when it should not have been. This could lead to performance issues for complex sandboxed scripts. - -## Version 1.65 - -Release date: 2019-10-01 - -* [Fix sandbox bypass vulnerability](https://jenkins.io/security/advisory/2019-10-01/#SECURITY-1579) - -## Version 1.64 - -Release date: 2019-09-13 - -* [JENKINS-57563](https://issues.jenkins-ci.org/browse/JENKINS-57563) - Add support for configuring script approvals using [Jenkins Configuration as Code Plugin](https://plugins.jenkins.io/configuration-as-code). - -## Version 1.63 - -Release date: 2019-09-12 - -* [Fix sandbox bypass security vulnerabilities](https://jenkins.io/security/advisory/2019-09-12/#SECURITY-1538) - -## Version 1.62 - -Release date: 2019-07-31 - -* [Fix sandbox bypass security vulnerabilities](https://jenkins.io/security/advisory/2019-07-31/) - -## Version 1.61 - -Release date: 2019-07-05 - -* [JENKINS-56682](https://issues.jenkins-ci.org/browse/JENKINS-56682) - Fix the use of script-level initializers in sandboxed Groovy scripts, which was a regression from version 1.54. -* [JENKINS-47430](https://issues.jenkins-ci.org/browse/JENKINS-47430) - Replace Guava cache used in for sandbox class loading with Caffeine to fix some performance issues and deadlocks. -* Add the following methods to the default list of approved signatures: - * `Number.times(Closure)` - * `new PrintWriter(Writer)` - * `Reader.read()` - * `Reader.read(char[])` - * `Reader.read(char[], int, int)` - * `Reader.reset()` - * `Reader.skip(long)` - * `Writer.write(char[])` - * `Writer.write(char[], int, int)` - * `Writer.write(int)` - * `Writer.write(String)` - * `Writer.write(String, int, int)` - * `Appendable.append(char)` - * `Appendable.append(CharSequence)` - * `Appendable.append(CharSequence, int, int)` - * `AutoCloseable.close()` - * `Flushable.flush()` - * `new LinkedHashSet()` - * `List.add(int, Object)` - * `Matcher.find()` - * `DefaultGroovyMethods.getAt(Object[], Range)` - * `DefaultGroovyMethods.reverse(List)` - -## Version 1.60 - -Release date: 2019-05-31 - -* SandboxResolvingClassLoader.parentClassCache could leak loaders in a different way ([PR 253](https://github.com/jenkinsci/script-security-plugin/pull/253)) - -## Version 1.59 - -Release date: 2019-04-18 - -* SandboxResolvingClassLoader.parentClassCache could leak loaders ([PR 252](https://github.com/jenkinsci/script-security-plugin/pull/252))  -* [JENKINS-57299](https://issues.jenkins-ci.org/browse/JENKINS-57299) - Add the following methods to the default list of approved signatures: - * `DefaultGroovyMethods.drop(Iterable, int)` - * `DefaultGroovyMethods.drop(List, int)` - * `DefaultGroovyMethods.dropRight(Iterable, int)` - * `DefaultGroovyMethods.dropRight(List, int)` - * `DefaultGroovyMethods.take(List, int)` - * `DefaultGroovyMethods.takeRight(Iterable, int)` - * `DefaultGroovyMethods.takeRight(List, int)` - -## Version 1.58 - -Release date: 2019-04-18 - -* Always block `System.exit(int)` , `Runtime#halt(int)` , and `Runtime#exit(int)`  -* [JENKINS-34973](https://issues.jenkins-ci.org/browse/JENKINS-34973) - Add script approvals from within `try/catch`  blocks. - -## Version 1.57 - -Release date: 2019-04-11 - -* Add the following methods to the default list of approved signatures: - * `Map.getOrDefault(Object, Object)` - * `Map.putIfAbsent(Object, Object)` - * `Map.replace(Object, Object)` - * `Map.replace(Object, Object, Object)` - -## Version 1.56 - -Release date: 2019-03-25 - -* [Fix security issue](https://jenkins.io/security/advisory/2019-03-25/#SECURITY-1353) - -## Version 1.55 - -Release date: 2019-03-18 - -* [JENKINS-55303](https://issues.jenkins-ci.org/browse/JENKINS-55303) - Internal: Update tests and test-scope dependencies so that the plugin can build with all tests passing on Java 11. - -## Version 1.54 - -Release date: 2019-03-06 - -* [Fix security issue](https://jenkins.io/security/advisory/2019-03-06/#SECURITY-1336%20(1)) - -## Version 1.53 - -Release date: 2019-02-19 - -* [Fix security issue](https://jenkins.io/security/advisory/2019-02-19/#SECURITY-1320) - -## Version 1.52 - -Release date: 2019-02-13 - -* Add the following methods to the default list of approved signatures: - * `DateTimeFormatter.ofPattern(String)` - * `Iterable.take(int)` - * `List.subList(int, int)` - -## Version 1.51 - -Release date: 2019-01-28 - -* [Fix security issue](https://jenkins.io/security/advisory/2019-01-28/) - -## Version 1.50 - -Release date: 2019-01-08 - -* [Fix security vulnerability](https://jenkins.io/security/advisory/2019-01-08/) - -## Version 1.49 - -Release date: 2018-11-30 - -* Make sure expensive log lines are only created if the appropriate logging level is enabled ([PR #232](https://github.com/jenkinsci/script-security-plugin/pull/232)) -* Add the following methods to the default list of approved signatures: - - * `String#indexOf(int)` - * `String#indexOf(int, int)` - * `String#indexOf(String, int)` - * `String#lastIndexOf(int)` - * `String#lastIndexOf(int, int)` - * `String#lastIndexOf(String, int)` - -## Version 1.48 - -Release date: 2018-10-29 - -* [Fix security issue](https://jenkins.io/security/advisory/2018-10-29/) - -## Version 1.47 - -Release date: 2018-10-17 - -* Add the following methods to the default list of approved signatures: - * `DefaultGroovyMethods#leftShift(Writer, Object)` - * `Class#isInstance(Object)` - * `Throwable#getCause()` - * `Arrays#asList(Object[])` - * `Matcher#group(String)` - * `DefaultGroovyMethods#minus(List, Collection)` - * `DefaultGroovyMethods#asBoolean(CharSequence)` - * Various methods in the `java.time` package -* Thanks, open source contributors TobiX, haridsv, kevinkjt2000! - -## Version 1.46 - -Release date: 2018-09-05 - -* [JENKINS-53420](https://issues.jenkins-ci.org/browse/JENKINS-53420) - Fix `MissingPropertyException` when executing Pipeline steps. - -## Version 1.45 - -Release date: 2018-09-04 - -* [JENKINS-50843](https://issues.jenkins-ci.org/browse/JENKINS-50843) - Allow calling `Closure` elements of a `Map` as methods. -* [JENKINS-51332](https://issues.jenkins-ci.org/browse/JENKINS-51332) - Add `Calendar` constants for days of the week and months (such as `MONDAY` and `APRIL`) to the default list of approved signatures. -* [JENKINS-50906](https://issues.jenkins-ci.org/browse/JENKINS-50906) - Allow `this.foo()` for closure variables. -* Downgrade logging level for message about slow class loading increase threshold from 250ms to 1s. -* Add the following methods to the default list of approved signatures: - - * `DefaultGroovyMethods#addAll(Collection, Object[])` - * `DefaultGroovyMethods#asImmutable(Map)` - * `DefaultGroovyMethods#flatten(List)` - * `DefaultGroovyMethods#getAt(List, Range)` - * `DefaultGroovyMethods#subMap(Map, Object[])` - * `DefaultGroovyMethods#subMap(Map, Collection)` - -## Version 1.44 - -Release date: 2018-04-27 - -* Add `DefaultGroovyMethods.toLong(String)` to the default list of approved signatures. -* [JENKINS-50470](https://issues.jenkins-ci.org/browse/JENKINS-50470) - fix handling of `ArrayList.someField` to behave as a spread operation. -* [JENKINS-46882](https://issues.jenkins-ci.org/browse/JENKINS-46882) - Add `new Exception(String)` to the default list of approved signatures. - -## Version 1.43 - -Release date: 2018-03-28 - -* Add `DefaultGroovyMethods.collate` methods to the default list of approved signatures. -* [JENKINS-50380](https://issues.jenkins-ci.org/browse/JENKINS-50380) - Stop going through `checkedCast` process for objects that can be assigned to the target class and just return them instead. -* Add `Collection#remove(int)` and `List#remove(int)` to the default list of approved signatures. -* Add `DefaultGroovyMethods` for `sort`, `toSorted`, `unique`, `max`, `min`, and `abs` to the default list of approved signatures. Note that using these (other than `abs`) in Pipeline code will not work until [JENKINS-44924](https://issues.jenkins-ci.org/browse/JENKINS-44924) is resolved. -* Slightly improved error messages replacing `unclassified ...` for cases where we couldn't find a method, field, constructor, etc matching the signature. - -## Version 1.42 - -Release date: 2018-03-12 - -* [JENKINS-45982](https://issues.jenkins-ci.org/browse/JENKINS-45982) - Fix an issue with calling `super` for a CPS-transformed method. -* [JENKINS-49542](https://issues.jenkins-ci.org/browse/JENKINS-49542) - add `Map#isEmpty()` to the default list of approved signatures. -* Add `DefaultGroovyMethods.multiply(String,Number)`, `DefaultGroovyMethods.with(Object,Closure)`, `Object#hashCode()`, `Objects.hash(Object[])`, `DefaultGroovyMethods.first(...)`, and `DefaultGroovyMethods.last(...)` to the default list of approved signatures. - -## Version 1.41 - -Release date: 2018-02-08 - -* **Major improvement**: greatly reduce time required to check whether signatures are approved for some implementations of `Whitelist` -* **Major improvement**: allow permission checks to multithread - elliminate lock contention with concurrent calls -* Improve UX for clearing dangerous signatures [JENKINS-22660](https://issues.jenkins-ci.org/browse/JENKINS-22660) -* Add Integer.toString(int, int) to the default list of approved signatures -* Add DefaultGroovyMethods toListString and toMapString to the default list of approved signatures - -## Version 1.40 - -Release date: 2018-01-10 - -* Block `System.getNanoTime()` to prevent Spectre/Meltdown exploits. -* Add `DefaultGroovyMethods#contains(Iterable,Object)` to the default list of approved signatures. - -## Version 1.39 - -Release date: 2017-12-12 - -* [JENKINS-48501](https://issues.jenkins-ci.org/browse/JENKINS-48501) - Fix NPE regression caused by fix for JENKINS-48364 and JENKINS-46213. - -## Version 1.38 - -Release date: 2017-12-11 - -* [JENKINS-46764](https://issues.jenkins-ci.org/browse/JENKINS-46764) - Log useful message when `scriptApproval.xml` is malformed. -* [JENKINS-48364](https://issues.jenkins-ci.org/browse/JENKINS-48364) - Treat null first vararg param properly. -* [JENKINS-46213](https://issues.jenkins-ci.org/browse/JENKINS-46213) - Treat trailing array parameters as varargs when appropriate. - -## Version 1.37 - -Release date: 2017-12-11 - -* [Fix security issue](https://jenkins.io/security/advisory/2017-12-11/) - -## Version 1.36 - -Release date: 2017-11-29 - -* [JENKINS-47159](https://issues.jenkins-ci.org/browse/JENKINS-47159), [JENKINS-47893](https://issues.jenkins-ci.org/browse/JENKINS-47893) - Fix two issues with varargs handling. -* Add more collection methods to the default list of approved signatures. -* Hide `ScriptApproval` link if there are no pending or approved signatures. -* Introduced support for `SystemCommandLanguage` - -## Version 1.35 - -Release date: 2017-11-02 - -* [JENKINS-47758](https://issues.jenkins-ci.org/browse/JENKINS-47758) -  New feature: plugins using the SecureGroovyScript.evaluate method are automatically protected against Groovy memory leaks (most plugins) - - * Notable plugin exceptions: email-ext, matrix-project, ontrack (may be covered by a later enhancement), job-dsl (needs a bespoke implementation) and splunk-devops plugins (can't cover - doesn't use enough script-security APIs) - * Pipeline offered its own leak protection mechanism (this is based on that) -* [JENKINS-35294](https://issues.jenkins-ci.org/browse/JENKINS-35294) - VarArgs support for enums -* Add map.get, List, minus, padLeft and padRight to the default list of approved signatures (thanks to community contributions from Github users [ryankillory](https://github.com/ryankillory), [Ignition](https://github.com/Ignition), and [andrey-fomin](https://github.com/andrey-fomin) !) -* [JENKINS-47666](https://issues.jenkins-ci.org/browse/JENKINS-47666) - Add math.max and math.min to the default list of approved signatures -* [JENKINS-44557](https://issues.jenkins-ci.org/browse/JENKINS-44557) - Properly cast GString (Groovy dynamic/templated string) in varargs - -## Version 1.34 - -Release date: 2017-09-05 - -* [JENKINS-46391](https://issues.jenkins-ci.org/browse/JENKINS-46391) - Properly handle `~/foo/` regexp declarations and some other `Pattern` methods. -* [JENKINS-46358](https://issues.jenkins-ci.org/browse/JENKINS-46358) - Add `StringGroovyMethods` including `replaceAll`, and `findAll` to the default list of approved signatures. - -## Version 1.33 - -Release date: 2017-08-16 - -* [JENKINS-46088](https://issues.jenkins-ci.org/browse/JENKINS-46088) Fix problems caused by double sandbox transformation of right-hand-side of declarations. -* [JENKINS-33468](https://issues.jenkins-ci.org/browse/JENKINS-33468) Allow use of `it` implicit closure parameter. -* [JENKINS-45776](https://issues.jenkins-ci.org/browse/JENKINS-45776) Better handling of scoping of closure local variables. -* [JENKINS-46191](https://issues.jenkins-ci.org/browse/JENKINS-46191) Fix compilation of empty declarations, such as `String foo;`, in sandbox. - -## Version 1.32 - -Release date: 2017-08-16 - -* Failed release due to repository permissions issues; replaced by 1.33. - -## Version 1.31 - -Release date: 2017-08-07 - -* [Multiple security fixes](https://jenkins.io/security/advisory/2017-08-07/) - -## Version 1.30 - -Release date: 2017-07-25 - -Now requires Jenkins 2.7.x or later, i.e., versions of Jenkins running Groovy 2.x. - -* Add signatures to the lists of approved and dangerous signatures. -* [JENKINS-42563](https://issues.jenkins-ci.org/browse/JENKINS-42563) Handling `super` calls to methods. - -* Be explicit about classpath directory rejection reason. -* [JENKINS-45117](https://issues.jenkins-ci.org/browse/JENKINS-45117) Apply specificity comparisons to constructors, not just methods. - -* [JENKINS-37129](https://issues.jenkins-ci.org/browse/JENKINS-37129) Throw a more helpful `MissingMethodException` rather than an “unclassified” error. - -* Cleanup of math operations. -* [JENKINS-34599](https://issues.jenkins-ci.org/browse/JENKINS-34599) Allow `final` fields to be set. - -* [JENKINS-45629](https://issues.jenkins-ci.org/browse/JENKINS-45629) Field initializers could produce a `NullPointerException` during script transformation. - -## Version 1.29.1 - -Release date: 2017-07-10 - -* [Fix security issue](https://jenkins.io/security/advisory/2017-07-10/) - -## Version 1.29 - -Release date: 2017-06-15 - -* Add various signatures to the default list of approved signatures, particularly for `DefaultGroovyMethods`. - -## Version 1.28 - -Release date: 2017-06-05 - -* [JENKINS-34741](https://issues.jenkins-ci.org/browse/JENKINS-34741) Unclassified error when using Groovy struct constructors. - -* Update the default list of approved signatures. - -## Version 1.27 - -Release date: 2017-02-27 - -* [JENKINS-41797](https://issues.jenkins-ci.org/browse/JENKINS-41797) Race condition could corrupt internal metadata used to check whether signatures are approved. -* [JENKINS-39159](https://issues.jenkins-ci.org/browse/JENKINS-39159) File handle leak when using custom script classpath could lead to unwanted locks on Windows or NFS. -* Update the default list of approved signatures. - -## Version 1.26 - -Release date: 2017-02-13 - -* Update the default list of approved signatures. - -## Version 1.25 - -Release date: 2017-01-03 - -* Update the lists of approved and dangerous signatures. -* Display a warning about previously approved signatures which are now in the list of dangerous signatures. - -## Version 1.24 - -Release date: 2016-10-20 - -* [JENKINS-38908](https://issues.jenkins-ci.org/browse/JENKINS-38908) Improper handling of some varargs methods. -* Update the default list of approved signatures. - -## Version 1.23 - -Release date: 2016-09-21 - -* Better report [JENKINS-37599](https://issues.jenkins-ci.org/browse/JENKINS-37599), a bug in core tickled by the [Promoted Builds Plugin](https://wiki.jenkins.io/display/JENKINS/Promoted+Builds+Plugin). -* Update the lists of approved and dangerous signatures. - -## Version 1.22 - -Release date: 2016-08-15 - -* Introduce a class loader caching layer for the Groovy sandbox to work around core performance limitations such as [JENKINS-23784](https://issues.jenkins-ci.org/browse/JENKINS-23784). -* [JENKINS-37344](https://issues.jenkins-ci.org/browse/JENKINS-37344) Add collection-related signatures to the default list of approved signatures. - -## Version 1.21 - -Release date: 2016-07-11 - -* Add build changelog-related signatures to the default list of approved Jenkins-related signatures ([JENKINS-30412](https://issues.jenkins-ci.org/browse/JENKINS-30412)). - -## Version 1.20 - -Release date: 2016-06-20 - -* Update the default list of approved signatures. -* [JENKINS-34739](https://issues.jenkins-ci.org/browse/JENKINS-34739) Support for varargs methods. -* [JENKINS-33023](https://issues.jenkins-ci.org/browse/JENKINS-33023) `enum` initializer fixes. -* Add `RunWrapper.getRawBuild` to the list of dangerous signatures. - -## Version 1.19 - -Release date: 2016-04-26 - -* [JENKINS-24399](https://issues.jenkins-ci.org/browse/JENKINS-24399) Prohibit class directories from being approved classpath entries. -* [JENKINS-33023](https://issues.jenkins-ci.org/browse/JENKINS-33023) Support `enum` initializers. -* Permit metaclass methods to be run. -* Update the lists of approved and dangerous signatures. - -## Version 1.18.1 - -Release date: 2016-04-11 - -* Security release (CVE-2016-3102). [advisory](https://wiki.jenkins-ci.org/display/SECURITY/Jenkins+Security+Advisory+2016-04-11) - -## Version 1.18 - -Release date: 2016-04-04 - -* Groovy prefers a getter/setter to a field access, so act accordingly, particularly when suggesting signatures to approve. -* [JENKINS-27725](https://issues.jenkins-ci.org/browse/JENKINS-27725) Various fixes to handling of GDK methods. -* Update the lists of approved and dangerous signatures. -* [JENKINS-26481](https://issues.jenkins-ci.org/browse/JENKINS-26481) Supporting fix to GDK method handling necessary to support calls such as `Object.each(Closure)` from `groovy-cps` Pipeline. - -## Version 1.17 - -Release date: 2016-01-25 - -* `obj.prop` should interpret `boolean TheClass.isProp()`, not just `boolean TheClass.getProp()`. - -## Version 1.16 - -Release date: 2016-01-19 - -* Update the default list of approved signatures, including standard Groovy operators and GDK methods. -* [JENKINS-30432](https://issues.jenkins-ci.org/browse/JENKINS-30432) Warn about dangerous signatures. -* [JENKINS-31234](https://issues.jenkins-ci.org/browse/JENKINS-31234) Groovy allows `Singleton.instance` as an alias for `Singleton.getInstance()`; handled. -* [JENKINS-31701](https://issues.jenkins-ci.org/browse/JENKINS-31701) Misclassification of a method taking `long` and being passed an `int`. - -## Version 1.15 - -Release date: 2015-08-20 - -* Update the default list of approved signatures. -* Properly classify pseudofields of a `Map`. -* [JENKINS-29541](https://issues.jenkins-ci.org/browse/JENKINS-29541) Methods on a `GString` may really be called on a `String`. -* Corrected classification of methods ambiguous between `GroovyDefaultMethods` and `invokeMethod`. -* [JENKINS-28586](https://issues.jenkins-ci.org/browse/JENKINS-28586) Corrected handling of receivers inside a `Closure`. -* [JENKINS-28154](https://issues.jenkins-ci.org/browse/JENKINS-28154) Fixing handling of Groovy operators. - -## Version 1.14 - -Release date: 2015-04-22 - -* Better error message when you mistype a method name on a Groovy class. -* Default to using sandbox mode when the current user is not an administrator. - -## Version 1.13 - -Release date: 2015-02-02 - -* Testability fix only. - -## Version 1.12 - -Release date: 2014-12-04 - -* [JENKINS-25914](https://issues.jenkins-ci.org/browse/JENKINS-25914) Allow `env` in Pipeline plugins with a special implementation of `Whitelist`. -* Add `Collection.contains` to the default list of approved signatures. - -## Version 1.11 - -Release date: 2014-12-03 - -* Handling some more Groovy constructs, such as the `=~` operator, and GDK methods like `Iterable.join(String)`. - -## Version 1.10 - -Release date: 2014-11-14 - -* [JENKINS-25524](https://issues.jenkins-ci.org/browse/JENKINS-25524) Handle ambiguous method overloads better. - -## Version 1.9 - -Release date: 2014-11-04 - -* Code can escape sandbox if there are multiple copies of `groovy-sandbox.jar` in Jenkins ([JENKINS-25348](https://issues.jenkins-ci.org/browse/JENKINS-25348)) - -## Version 1.8 - -Release date: 2014-10-29 - -* `groovy-sandbox` 1.8 has a few fixes. - -## Version 1.7 - -Release date: 2014-10-13 - -* [JENKINS-25118](https://issues.jenkins-ci.org/browse/JENKINS-25118) Handle methods with primitive arguments. - -## Version 1.6 - -Release date: 2014-10-02 - -* Handle `GroovyObject.invokeMethod(String,Object)` correctly during call site selecction. - -## Version 1.5 - -Release date: 2014-08-19 - -* [JENKINS-22834](https://issues.jenkins-ci.org/browse/JENKINS-22834) Added support for custom classpaths. - -## Version 1.4 - -Release date: 2014-06-08 - -* Do not bother enforcing whole-script approval when Jenkins is unsecured anyway. -* Some changes to make writing acceptance tests easier. - -## Version 1.3 - -Release date: 2014-05-13 - -* Fixing some regressions from 1.2. - -## Version 1.2 - -Release date: 2014-05-13 - -* Updated Groovy sandbox library for better language coverage. - -## Version 1.1 - -Release date: 2014-05-06 - -* Making it possible to use Groovy functions with `def` syntax. -* Added `GroovySandbox.run` so that methods defined in the script itself are always allowed. - -## Version 1.0 - -Release date: 2014-04-15 - -* String concatenation fix in sandbox. -* Preapprove the empty script. -* Support for static fields in sandbox. -* Changed package of `AbstractWhitelist`. - -## Version 1.0 beta 6 - -Release date: 2014-03-31 - -* Added `SecureGroovyScript` convenience class. - -## Version 1.0 beta 5 - -Release date: 2014-03-13 - -* Fixed various bugs in the Groovy sandbox. -* Added `AbstractWhitelist`. - -## Version 1.0 beta 4 - -Release date: 2014-03-12 - -* Refactored `Whitelist` to support `GString` and more - -## Version 1.0 beta 3 - -Release date: 2014-03-01 - -* Reverted GString fix for now - -## Version 1.0 beta 2 - -Release date: 2014-02-28 - -* @Whitelisted -* initialization bug fix -* Groovy GString fix - -## Version 1.0 beta 1 - -Release date: 2014-02-28 - -* Initial version. - diff --git a/Jenkinsfile b/Jenkinsfile index 3765128a6..52525eb3a 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -1,4 +1,6 @@ -buildPlugin(useAci: true, configurations: [ - [ platform: "windows", jdk: "11" ], - [ platform: "linux", jdk: "11" ] +buildPlugin( + useContainerAgent: true, + configurations: [ + [platform: 'linux', jdk: 21], + [platform: 'windows', jdk: 17], ]) diff --git a/README.md b/README.md index 41bc7f93d..00c335a39 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,5 @@ # Script Security Plugin -[![Jenkins Plugin](https://img.shields.io/jenkins/plugin/v/script-security)](https://plugins.jenkins.io/script-security) -[![Changelog](https://img.shields.io/github/v/tag/jenkinsci/script-security-plugin?label=changelog)](https://github.com/jenkinsci/script-security-plugin/releases) -[![Jenkins Plugin Installs](https://img.shields.io/jenkins/plugin/i/script-security?color=blue)](https://plugins.jenkins.io/script-security) ## User’s guide (adapted from information on [Template plugin in CloudBees Plugins guide](https://docs.cloudbees.com/docs/admin-resources/latest/plugins/template#template-sect-script-approval)) @@ -25,17 +22,17 @@ The first, and simpler, security system is to allow any kind of script to be run with an administrator’s approval. There is a globally maintained list of approved scripts which are judged to not perform any malicious actions. -When an administrator saves some kind of configuration (for example, a job), those scripts -that were edited by admin are automatically approved and are ready to run with no further -intervention. For scripts that were submitted by lower privileged users there will be +When a user saves some kind of configuration (for example, a job), there will be appropriate warnings indicating that approval is required. Administrators may approve those -scripts using the Script Approval configuration page or by editing the script and saving it. -In previous versions of Script Security Plugin, administrators could automatically approve -scripts submitted by unprivileged users by saving them without making any changes, but this -functionality was disabled to prevent social engineering-based attacks. (“Saving” usually -means from the web UI, but could also mean uploading a new XML configuration via REST or CLI.) - -When a non-administrator saves a template configuration, a check is done whether any +scripts using the Script Approval configuration page or following the approval link in the +configuration. +In previous versions of Script Security Plugin, scripts saved by administrators where +automatically approved when saving them, but this functionality was disabled to prevent +a variety of social engineering-based attacks. (“Saving” usually means from the web UI, but +could also mean uploading a new XML configuration via REST or CLI.) or merely by creating a +new item copying an existing one. + +When a user saves a template configuration, a check is done whether any contained scripts have been edited from an approved text. (More precisely, whether the requested content has ever been approved before.) If it has not been approved, a request for approval of this script is added to a queue. (A warning is also displayed in the @@ -193,4 +190,6 @@ whitelist methods used by your tests -- either generally for real users, or usin ## Version history -See [the changelog](CHANGELOG.md) + +From of version 1.77 see [GitHub Releases](https://github.com/jenkinsci/script-security-plugin/releases). +[Archives](https://github.com/jenkinsci/script-security-plugin/blob/71755e3bd5cf0f04acfcb5afc27b5a29252fca7e/CHANGELOG.md) diff --git a/pom.xml b/pom.xml index 783f0ea2d..a519762d2 100644 --- a/pom.xml +++ b/pom.xml @@ -4,20 +4,18 @@ org.jenkins-ci.plugins plugin - 4.47 - + 4.87 + script-security ${changelist} hpi Script Security Plugin - Allows Jenkins administrators to control what in-process scripts can be run by less-privileged users. https://github.com/jenkinsci/${project.artifactId}-plugin 999999-SNAPSHOT - - 2.366-rc32795.df5b_49c75b_0e + 2.440.3 jenkinsci/${project.artifactId}-plugin @@ -51,8 +49,8 @@ io.jenkins.tools.bom - bom-2.361.x - 1607.va_c1576527071 + bom-2.440.x + 3334.v18e2a_2f48356 import pom @@ -63,7 +61,7 @@ org.kohsuke groovy-sandbox - 1.27 + 1.34 org.codehaus.groovy diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/ClassLoaderWhitelist.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/ClassLoaderWhitelist.java index 517f7a145..cab90aa19 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/ClassLoaderWhitelist.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/ClassLoaderWhitelist.java @@ -1,10 +1,12 @@ package org.jenkinsci.plugins.scriptsecurity.sandbox.groovy; +import edu.umd.cs.findbugs.annotations.NonNull; import org.jenkinsci.plugins.scriptsecurity.sandbox.Whitelist; import java.lang.reflect.Constructor; import java.lang.reflect.Field; import java.lang.reflect.Method; +import java.lang.reflect.Modifier; /** * {@link Whitelist} that allows everything defined from a specific classloader. @@ -22,31 +24,82 @@ private boolean permits(Class declaringClass) { return declaringClass.getClassLoader() == scriptLoader; } - @Override public boolean permitsMethod(Method method, Object receiver, Object[] args) { - return permits(method.getDeclaringClass()); + @Override public boolean permitsMethod(@NonNull Method method, @NonNull Object receiver, @NonNull Object[] args) { + return permits(method.getDeclaringClass()) && !isIllegalSyntheticMethod(method); } - @Override public boolean permitsConstructor(Constructor constructor, Object[] args) { - return permits(constructor.getDeclaringClass()); + @Override public boolean permitsConstructor(@NonNull Constructor constructor, @NonNull Object[] args) { + return permits(constructor.getDeclaringClass()) && !isIllegalSyntheticConstructor(constructor); } - @Override public boolean permitsStaticMethod(Method method, Object[] args) { - return permits(method.getDeclaringClass()); + @Override public boolean permitsStaticMethod(@NonNull Method method, @NonNull Object[] args) { + return permits(method.getDeclaringClass()) && !isIllegalSyntheticMethod(method); } - @Override public boolean permitsFieldGet(Field field, Object receiver) { - return permits(field.getDeclaringClass()); + @Override public boolean permitsFieldGet(@NonNull Field field, @NonNull Object receiver) { + return permits(field.getDeclaringClass()) && !isIllegalSyntheticField(field); } - @Override public boolean permitsFieldSet(Field field, Object receiver, Object value) { - return permits(field.getDeclaringClass()); + @Override public boolean permitsFieldSet(@NonNull Field field, @NonNull Object receiver, Object value) { + return permits(field.getDeclaringClass()) && !isIllegalSyntheticField(field); } - @Override public boolean permitsStaticFieldGet(Field field) { - return permits(field.getDeclaringClass()); + @Override public boolean permitsStaticFieldGet(@NonNull Field field) { + return permits(field.getDeclaringClass()) && !isIllegalSyntheticField(field); } - @Override public boolean permitsStaticFieldSet(Field field, Object value) { - return permits(field.getDeclaringClass()); + @Override public boolean permitsStaticFieldSet(@NonNull Field field, Object value) { + return permits(field.getDeclaringClass()) && !isIllegalSyntheticField(field); + } + + /** + * Checks whether a given field was synthetically created by the Groovy compiler and should be inaccessible even if + * it is declared by a class defined by the specified class loader. + */ + private static boolean isIllegalSyntheticField(Field field) { + if (!field.isSynthetic()) { + return false; + } + Class declaringClass = field.getDeclaringClass(); + Class enclosingClass = declaringClass.getEnclosingClass(); + if (field.getType() == enclosingClass && field.getName().startsWith("this$")) { + // Synthetic field added to inner classes to reference the outer class. + return false; + } else if (declaringClass.isEnum() && Modifier.isStatic(field.getModifiers()) && field.getName().equals("$VALUES")) { + // Synthetic field added by Groovy to enum classes to hold the enum constants. + return false; + } + return true; + } + + /** + * Checks whether a given method was synthetically created by the Groovy compiler and should be inaccessible even + * if it is declared by a class defined by the specified class loader. + */ + private static boolean isIllegalSyntheticMethod(Method method) { + if (!method.isSynthetic()) { + return false; + } else if (Modifier.isStatic(method.getModifiers()) && method.getDeclaringClass().isEnum() && method.getName().equals("$INIT")) { + // Synthetic method added by Groovy to enum classes used to initialize the enum constants. + return false; + } + return true; + } + + /** + * Checks whether a given constructor was created by the Groovy compiler (or groovy-sandbox) and + * should be inaccessible even if it is declared by a class defined by the specified class loader. + */ + private static boolean isIllegalSyntheticConstructor(Constructor constructor) { + if (!constructor.isSynthetic()) { + return false; + } + Class declaringClass = constructor.getDeclaringClass(); + Class enclosingClass = declaringClass.getEnclosingClass(); + if (enclosingClass != null && constructor.getParameters().length > 0 && constructor.getParameterTypes()[0] == enclosingClass) { + // Synthetic constructor added by Groovy to anonymous classes. + return false; + } + return true; } } diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyCallSiteSelector.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyCallSiteSelector.java index 0c3a04f2b..768f7c570 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyCallSiteSelector.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyCallSiteSelector.java @@ -31,9 +31,12 @@ import java.lang.reflect.Constructor; import java.lang.reflect.Field; import java.lang.reflect.Method; +import java.util.ArrayList; import java.util.LinkedHashSet; +import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.Predicate; import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; import org.apache.commons.lang.ClassUtils; @@ -43,7 +46,8 @@ * Assists in determination of which method or other JVM element is actually about to be called by Groovy. * Most of this just duplicates what {@link java.lang.invoke.MethodHandles.Lookup} and {@link java.lang.invoke.MethodHandle#asType} do, * but {@link org.codehaus.groovy.vmplugin.v7.TypeTransformers} shows that there are Groovy-specific complications. - * Comments in https://github.com/kohsuke/groovy-sandbox/issues/7 note that it would be great for the sandbox itself to just tell us what the call site is so we would not have to guess. + * Comments in groovy-sandbox #7 note that it would be + * great for the sandbox itself to just tell us what the call site is so we would not have to guess. */ class GroovyCallSiteSelector { @@ -198,6 +202,41 @@ private static boolean isInstancePrimitive(@NonNull Class type, @NonNull Obje return findMatchingMethod(receiver, method, args); } + /** + * Like {@link #method}, but returns all methods with the given name that match the given predicate. + */ + public static List methods(@NonNull Object receiver, @NonNull String method, Predicate filter) { + Set> types = types(receiver); + if (types.contains(GroovyInterceptable.class) && !"invokeMethod".equals(method)) { + return methods(receiver, "invokeMethod", m -> true); + } + List candidates = new ArrayList<>(); + for (Class c : types) { + for (Method candidate : c.getDeclaredMethods()) { + if (candidate.getName().equals(method) && filter.test(candidate)) { + candidates.add(candidate); + } + } + } + if (receiver instanceof GString) { // cf. GString.invokeMethod + candidates.addAll(methods(String.class, method, filter)); + } + return candidates; + } + + /** + * Like {@link #staticMethod}, but returns all methods with the given name that match the given predicate. + */ + public static List staticMethods(@NonNull Class receiver, @NonNull String method, Predicate filter) { + List candidates = new ArrayList<>(); + for (Method candidate : receiver.getDeclaredMethods()) { + if (candidate.getName().equals(method) && filter.test(candidate)) { + candidates.add(candidate); + } + } + return candidates; + } + private static Method findMatchingMethod(@NonNull Class receiver, @NonNull String method, @NonNull Object[] args) { Method candidate = null; diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovySandbox.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovySandbox.java index 58a85dd17..9323071fe 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovySandbox.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovySandbox.java @@ -26,20 +26,28 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import groovy.grape.GrabAnnotationTransformation; +import groovy.lang.Binding; import groovy.lang.GroovyClassLoader; +import groovy.lang.GroovyCodeSource; +import groovy.lang.GroovyObject; +import groovy.lang.GroovyRuntimeException; import groovy.lang.GroovyShell; import static groovy.lang.GroovyShell.DEFAULT_CODE_BASE; +import groovy.lang.MissingPropertyException; import groovy.lang.Script; import hudson.ExtensionList; import hudson.model.RootAction; import hudson.model.TaskListener; import hudson.util.FormValidation; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.net.MalformedURLException; import java.net.URL; import java.security.CodeSource; import java.security.cert.Certificate; import java.util.Collections; import java.util.HashSet; +import java.util.Map; import java.util.concurrent.Callable; import java.util.logging.Level; import java.util.logging.Logger; @@ -49,6 +57,8 @@ import org.codehaus.groovy.control.CompilationUnit; import org.codehaus.groovy.control.CompilerConfiguration; import org.codehaus.groovy.control.Phases; +import org.codehaus.groovy.runtime.InvokerHelper; +import org.codehaus.groovy.syntax.Types; import org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException; import org.jenkinsci.plugins.scriptsecurity.sandbox.Whitelist; import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.ProxyWhitelist; @@ -58,6 +68,7 @@ import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApprovalNote; import org.kohsuke.groovy.sandbox.GroovyInterceptor; import org.kohsuke.groovy.sandbox.SandboxTransformer; +import org.kohsuke.groovy.sandbox.impl.Checker; /** * Allows Groovy scripts (including Groovy Templates) to be run inside a sandbox. @@ -143,6 +154,17 @@ public Scope enter() { @FunctionalInterface public interface Scope extends AutoCloseable { + /** + * Variant of {@link GroovyShell#parse(String)} that intercepts potentially unsafe calls when the script is created. + * + *

{@link GroovySandbox#runScript(GroovyShell, String)} should be used instead of this method in most cases. + */ + default Script parse(GroovyShell shell, GroovyCodeSource codeSource) { + Class scriptClass = shell.getClassLoader().parseClass(codeSource, false); + Script script = checkedCreateScript(scriptClass, shell.getContext()); + return script; + } + @Override void close(); } @@ -150,24 +172,96 @@ public interface Scope extends AutoCloseable { /** * Compiles and runs a script within the sandbox. * @param shell the shell to be used; see {@link #createSecureCompilerConfiguration} and similar methods - * @param script the script to run + * @param scriptText the script to run * @return the return value of the script */ - public Object runScript(@NonNull GroovyShell shell, @NonNull String script) { + public Object runScript(@NonNull GroovyShell shell, @NonNull String scriptText) { GroovySandbox derived = new GroovySandbox(). withApprovalContext(context). withTaskListener(listener). withWhitelist(new ProxyWhitelist(new ClassLoaderWhitelist(shell.getClassLoader()), whitelist())); try (Scope scope = derived.enter()) { - return shell.parse(script).run(); + // GroovyShell does not expose any public APIs that allow us to access the generated Script class before InvokerHelper.createScript is called. + String scriptFileName = "Script0.groovy"; + try { + Method generateScriptName = shell.getClass().getDeclaredMethod("generateScriptName"); + generateScriptName.setAccessible(true); // Method is protected. + scriptFileName = (String) generateScriptName.invoke(shell); + } catch (ReflectiveOperationException e) { + LOGGER.log(Level.WARNING, null, e); + } + GroovyCodeSource codeSource = new GroovyCodeSource(scriptText, scriptFileName, DEFAULT_CODE_BASE); + Script script = scope.parse(shell, codeSource); + return script.run(); + } + } + + /** + * Variant of {@link InvokerHelper#createScript} that intercepts potentially unsafe reflective behaviors. + * + *

{@link GroovySandbox#runScript(GroovyShell, String)} or {@link Scope#parse} should be used instead of this method in most cases. + * + * @see InvokerHelper#createScript + */ + private static Script checkedCreateScript(Class scriptClass, Binding context) { + Script script; + try { + if (Script.class.isAssignableFrom(scriptClass)) { + try { + script = InvokerHelper.newScript(scriptClass, context); + } catch (InvocationTargetException e) { + throw e.getCause(); // Typically RejectedAccessException. + } + } else { + // TODO: Could we just throw a SecurityException instead and tell users to extend Script or not have a + // class definition as the only top-level content in their script? + final GroovyObject object = (GroovyObject) scriptClass.newInstance(); + // it could just be a class, so let's wrap it in a Script + // wrapper; though the bindings will be ignored + script = new Script(context) { + public Object run() { + Object[] argsToPass = new Object[0]; + try { + Object args = getProperty("args"); + if (args instanceof String[]) { + argsToPass = (String[])args; + } + } catch (MissingPropertyException e) { + // They'll get empty args since none exist in the context. + } + try { + Checker.checkedCall(object, false, false, "main", argsToPass); + } catch (Throwable t) { + throw new GroovyRuntimeException(t); + } + return null; + } + }; + Map variables = context.getVariables(); + for (Object o : variables.entrySet()) { + Map.Entry entry = (Map.Entry) o; + String key = entry.getKey().toString(); + // assume underscore variables are for the wrapper script + try { + Checker.checkedSetProperty(key.startsWith("_") ? script : object, key, true, false, Types.ASSIGN, entry.getValue()); + } catch (MissingPropertyException e) { + // Swallow MissingPropertyException to match Groovy behavior in this case. + } + } + } + } catch (Throwable e) { + throw new GroovyRuntimeException( + "Failed to create Script instance for class: " + + scriptClass + ". Reason: " + e, e); } + return script; } /** * Prepares a compiler configuration the sandbox. * - *

CAUTION

*

+ * CAUTION: * When creating {@link GroovyShell} with this {@link CompilerConfiguration}, * you also have to use {@link #createSecureClassLoader(ClassLoader)} to wrap * a classloader of your choice into sandbox-aware one. diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptor.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptor.java index f5a67ec1c..4a46bf5b1 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptor.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptor.java @@ -30,23 +30,26 @@ import groovy.lang.MissingMethodException; import groovy.lang.MissingPropertyException; import groovy.lang.Script; -import hudson.Functions; import java.lang.reflect.Constructor; import java.lang.reflect.Field; import java.lang.reflect.Method; +import java.lang.reflect.Parameter; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; +import java.lang.reflect.Modifier; import org.codehaus.groovy.runtime.DateGroovyMethods; import org.codehaus.groovy.runtime.DefaultGroovyMethods; import org.codehaus.groovy.runtime.EncodingGroovyMethods; import org.codehaus.groovy.runtime.InvokerHelper; +import org.codehaus.groovy.runtime.MetaClassHelper; import org.codehaus.groovy.runtime.ProcessGroovyMethods; import org.codehaus.groovy.runtime.SqlGroovyMethods; import org.codehaus.groovy.runtime.StringGroovyMethods; @@ -54,12 +57,14 @@ import org.codehaus.groovy.runtime.XmlGroovyMethods; import org.codehaus.groovy.runtime.metaclass.ClosureMetaMethod; import org.codehaus.groovy.runtime.typehandling.NumberMathModificationInfo; +import org.codehaus.groovy.syntax.Types; import org.codehaus.groovy.tools.DgmConverter; import org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException; import org.jenkinsci.plugins.scriptsecurity.sandbox.Whitelist; import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.EnumeratingWhitelist; import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.StaticWhitelist; import org.kohsuke.groovy.sandbox.GroovyInterceptor; +import org.kohsuke.groovy.sandbox.impl.Checker; @SuppressWarnings("rawtypes") final class SandboxInterceptor extends GroovyInterceptor { @@ -131,7 +136,7 @@ final class SandboxInterceptor extends GroovyInterceptor { Script s = (Script) receiver; if (s.getBinding().hasVariable(method)) { Object var = s.getBinding().getVariable(method); - if (!InvokerHelper.getMetaClass(var).respondsTo(var, "call", (Object[]) args).isEmpty()) { + if (!InvokerHelper.getMetaClass(var).respondsTo(var, "call", args).isEmpty()) { return onMethodCall(invoker, var, "call", args); } } @@ -154,12 +159,12 @@ final class SandboxInterceptor extends GroovyInterceptor { throw new MissingMethodException(method, receiver.getClass(), args); } else if (StaticWhitelist.isPermanentlyBlacklistedMethod(m)) { throw StaticWhitelist.rejectMethod(m); - } else if (whitelist.permitsMethod(m, receiver, args)) { + } else if (permitsMethod(whitelist, m, receiver, args)) { return super.onMethodCall(invoker, receiver, method, args); } else if (method.equals("invokeMethod") && args.length == 2 && args[0] instanceof String && args[1] instanceof Object[]) { throw StaticWhitelist.rejectMethod(m, EnumeratingWhitelist.getName(receiver.getClass()) + " " + args[0] + printArgumentTypes((Object[]) args[1])); } else { - throw StaticWhitelist.rejectMethod(m); + throw rejectMethod(m); } } @@ -170,6 +175,24 @@ final class SandboxInterceptor extends GroovyInterceptor { } else if (StaticWhitelist.isPermanentlyBlacklistedConstructor(c)) { throw StaticWhitelist.rejectNew(c); } else if (whitelist.permitsConstructor(c, args)) { + if (c.getParameterCount() == 0 && args.length == 1 && args[0] instanceof Map) { + // c.f. https://github.com/apache/groovy/blob/41b990d0a20e442f29247f0e04cbed900f3dcad4/src/main/groovy/lang/MetaClassImpl.java#L1728-L1738 + // We replace the arguments that the invoker will use to construct the object with the empty array to + // bypass Groovy's default handling for implicit map constructors. + Object newInstance = super.onNewInstance(invoker, receiver, new Object[0]); + if (newInstance == null) { + // We were called by Checker.preCheckedCast. Our options here are limited, so we just reject everything. + throw new UnsupportedOperationException("Groovy map constructors may only be invoked using the 'new' keyword in the sandbox (attempted to construct " + receiver + " via a Groovy cast)"); + } + // The call to Map.entrySet below may be on a user-defined type, which could be a problem if we iterated + // over it here to pre-check the property assignments and then let Groovy iterate over it again to + // actually perform them, so we only iterate over it once and perform the property assignments + // ourselves using sandbox-aware methods. + for (Map.Entry entry : ((Map) args[0]).entrySet()) { + Checker.checkedSetProperty(newInstance, entry.getKey(), false, false, Types.ASSIGN, entry.getValue()); + } + return newInstance; + } return super.onNewInstance(invoker, receiver, args); } else { throw StaticWhitelist.rejectNew(c); @@ -197,36 +220,46 @@ final class SandboxInterceptor extends GroovyInterceptor { Rejector rejector = null; // avoid creating exception objects unless and until thrown // https://github.com/kohsuke/groovy-sandbox/issues/7 need to explicitly check for getters and setters: Object[] valueArg = new Object[] {value}; - String setter = "set" + Functions.capitalize(property); - final Method setterMethod = GroovyCallSiteSelector.method(receiver, setter, valueArg); + String setter = "set" + MetaClassHelper.capitalize(property); + List setterMethods = GroovyCallSiteSelector.methods(receiver, setter, m -> m.getParameterCount() == 1); + final Method setterMethod = setterMethods.size() == 1 + ? setterMethods.get(0) // If there is only a single setter, the argument will be cast to match the declared parameter type. + : GroovyCallSiteSelector.method(receiver, setter, valueArg); // If there are multiple setters, MultipleSetterProperty just calls invokeMethod. if (setterMethod != null) { - if (whitelist.permitsMethod(setterMethod, receiver, valueArg)) { + if (permitsMethod(whitelist, setterMethod, receiver, valueArg)) { + preCheckArgumentCasts(setterMethod, valueArg); return super.onSetProperty(invoker, receiver, property, value); } else if (rejector == null) { - rejector = () -> StaticWhitelist.rejectMethod(setterMethod); + rejector = () -> rejectMethod(setterMethod); } } Object[] propertyValueArgs = new Object[] {property, value}; final Method setPropertyMethod = GroovyCallSiteSelector.method(receiver, "setProperty", propertyValueArgs); - if (setPropertyMethod != null) { + if (setPropertyMethod != null && !isSyntheticMethod(receiver, setPropertyMethod)) { if (whitelist.permitsMethod(setPropertyMethod, receiver, propertyValueArgs)) { + preCheckArgumentCasts(setPropertyMethod, propertyValueArgs); return super.onSetProperty(invoker, receiver, property, value); } else if (rejector == null) { rejector = () -> StaticWhitelist.rejectMethod(setPropertyMethod, receiver.getClass().getName() + "." + property); } } - final Field instanceField = GroovyCallSiteSelector.field(receiver, property); - if (instanceField != null) { - if (whitelist.permitsFieldSet(instanceField, receiver, value)) { + final Field field = GroovyCallSiteSelector.field(receiver, property); + if (field != null) { + if (permitsFieldSet(whitelist, field, receiver, value)) { + Checker.preCheckedCast(field.getType(), value, false, false, false); return super.onSetProperty(invoker, receiver, property, value); } else if (rejector == null) { - rejector = () -> StaticWhitelist.rejectField(instanceField); + rejector = () -> rejectField(field); } } if (receiver instanceof Class) { - final Method staticSetterMethod = GroovyCallSiteSelector.staticMethod((Class) receiver, setter, valueArg); + List staticSetterMethods = GroovyCallSiteSelector.staticMethods((Class) receiver, setter, m -> m.getParameterCount() == 1); + final Method staticSetterMethod = staticSetterMethods.size() == 1 + ? staticSetterMethods.get(0) // If there is only a single setter, the value will be cast to match the declared parameter type. + : GroovyCallSiteSelector.staticMethod((Class) receiver, setter, valueArg); // If there are multiple setters, MultipleSetterProperty just calls invokeMethod. if (staticSetterMethod != null) { if (whitelist.permitsStaticMethod(staticSetterMethod, valueArg)) { + preCheckArgumentCasts(staticSetterMethod, valueArg); return super.onSetProperty(invoker, receiver, property, value); } else if (rejector == null) { rejector = () -> StaticWhitelist.rejectStaticMethod(staticSetterMethod); @@ -235,6 +268,7 @@ final class SandboxInterceptor extends GroovyInterceptor { final Field staticField = GroovyCallSiteSelector.staticField((Class) receiver, property); if (staticField != null) { if (whitelist.permitsStaticFieldSet(staticField, value)) { + Checker.preCheckedCast(staticField.getType(), value, false, false, false); return super.onSetProperty(invoker, receiver, property, value); } else if (rejector == null) { rejector = () -> StaticWhitelist.rejectStaticField(staticField); @@ -259,22 +293,22 @@ final class SandboxInterceptor extends GroovyInterceptor { } Rejector rejector = null; Object[] noArgs = new Object[] {}; - String getter = "get" + Functions.capitalize(property); + String getter = "get" + MetaClassHelper.capitalize(property); final Method getterMethod = GroovyCallSiteSelector.method(receiver, getter, noArgs); if (getterMethod != null) { - if (whitelist.permitsMethod(getterMethod, receiver, noArgs)) { + if (permitsMethod(whitelist, getterMethod, receiver, noArgs)) { return super.onGetProperty(invoker, receiver, property); } else if (rejector == null) { - rejector = () -> StaticWhitelist.rejectMethod(getterMethod); + rejector = () -> rejectMethod(getterMethod); } } - String booleanGetter = "is" + Functions.capitalize(property); + String booleanGetter = "is" + MetaClassHelper.capitalize(property); final Method booleanGetterMethod = GroovyCallSiteSelector.method(receiver, booleanGetter, noArgs); if (booleanGetterMethod != null && booleanGetterMethod.getReturnType() == boolean.class) { - if (whitelist.permitsMethod(booleanGetterMethod, receiver, noArgs)) { + if (permitsMethod(whitelist, booleanGetterMethod, receiver, noArgs)) { return super.onGetProperty(invoker, receiver, property); } else if (rejector == null) { - rejector = () -> StaticWhitelist.rejectMethod(booleanGetterMethod); + rejector = () -> rejectMethod(booleanGetterMethod); } } // look for GDK methods @@ -297,18 +331,18 @@ final class SandboxInterceptor extends GroovyInterceptor { } } } - final Field instanceField = GroovyCallSiteSelector.field(receiver, property); - if (instanceField != null) { - if (whitelist.permitsFieldGet(instanceField, receiver)) { + final Field field = GroovyCallSiteSelector.field(receiver, property); + if (field != null) { + if (permitsFieldGet(whitelist, field, receiver)) { return super.onGetProperty(invoker, receiver, property); } else if (rejector == null) { - rejector = () -> StaticWhitelist.rejectField(instanceField); + rejector = () -> rejectField(field); } } // GroovyObject property access Object[] propertyArg = new Object[] {property}; final Method getPropertyMethod = GroovyCallSiteSelector.method(receiver, "getProperty", propertyArg); - if (getPropertyMethod != null) { + if (getPropertyMethod != null && !isSyntheticMethod(receiver, getPropertyMethod)) { if (whitelist.permitsMethod(getPropertyMethod, receiver, propertyArg)) { return super.onGetProperty(invoker, receiver, property); } else if (rejector == null) { @@ -364,8 +398,8 @@ public Object onSuperCall(Invoker invoker, Class senderType, Object receiver, St } } - private static RejectedAccessException unclassifiedField(Object receiver, String property) { - return new RejectedAccessException("No such field found: field " + EnumeratingWhitelist.getName(receiver.getClass()) + " " + property); + private static MissingPropertyException unclassifiedField(Object receiver, String property) { + return new MissingPropertyException("No such field found: field " + EnumeratingWhitelist.getName(receiver.getClass()) + " " + property); } // TODO Java 8: @FunctionalInterface @@ -374,25 +408,51 @@ private interface Rejector { } @Override public Object onGetAttribute(Invoker invoker, Object receiver, String attribute) throws Throwable { + Rejector rejector = null; Field field = GroovyCallSiteSelector.field(receiver, attribute); - if (field == null) { - throw unclassifiedField(receiver, attribute); - } else if (whitelist.permitsFieldGet(field, receiver)) { - return super.onGetAttribute(invoker, receiver, attribute); - } else { - throw StaticWhitelist.rejectField(field); + if (field != null) { + if (permitsFieldGet(whitelist, field, receiver)) { + return super.onGetAttribute(invoker, receiver, attribute); + } else { + rejector = () -> rejectField(field); + } } + if (receiver instanceof Class) { + Field staticField = GroovyCallSiteSelector.staticField((Class)receiver, attribute); + if (staticField != null) { + if (whitelist.permitsStaticFieldGet(staticField)) { + return super.onGetAttribute(invoker, receiver, attribute); + } else { + rejector = () -> StaticWhitelist.rejectStaticField(staticField); + } + } + } + throw rejector != null ? rejector.reject() : unclassifiedField(receiver, attribute); } @Override public Object onSetAttribute(Invoker invoker, Object receiver, String attribute, Object value) throws Throwable { + Rejector rejector = null; Field field = GroovyCallSiteSelector.field(receiver, attribute); - if (field == null) { - throw unclassifiedField(receiver, attribute); - } else if (whitelist.permitsFieldSet(field, receiver, value)) { - return super.onSetAttribute(invoker, receiver, attribute, value); - } else { - throw StaticWhitelist.rejectField(field); + if (field != null) { + if (permitsFieldSet(whitelist, field, receiver, value)) { + Checker.preCheckedCast(field.getType(), value, false, false, false); + return super.onSetAttribute(invoker, receiver, attribute, value); + } else { + rejector = () -> rejectField(field); + } + } + if (receiver instanceof Class) { + Field staticField = GroovyCallSiteSelector.staticField((Class)receiver, attribute); + if (staticField != null) { + if (whitelist.permitsStaticFieldSet(staticField, value)) { + Checker.preCheckedCast(staticField.getType(), value, false, false, false); + return super.onSetAttribute(invoker, receiver, attribute, value); + } else { + rejector = () -> StaticWhitelist.rejectStaticField(staticField); + } + } } + throw rejector != null ? rejector.reject() : unclassifiedField(receiver, attribute); } @Override public Object onGetArray(Invoker invoker, Object receiver, Object index) throws Throwable { @@ -402,10 +462,10 @@ private interface Rejector { Object[] args = new Object[] {index}; Method method = GroovyCallSiteSelector.method(receiver, "getAt", args); if (method != null) { - if (whitelist.permitsMethod(method, receiver, args)) { + if (permitsMethod(whitelist, method, receiver, args)) { return super.onGetArray(invoker, receiver, index); } else { - throw StaticWhitelist.rejectMethod(method); + throw rejectMethod(method); } } args = new Object[] {receiver, index}; @@ -429,10 +489,10 @@ private interface Rejector { Object[] args = new Object[] {index, value}; Method method = GroovyCallSiteSelector.method(receiver, "putAt", args); if (method != null) { - if (whitelist.permitsMethod(method, receiver, args)) { + if (permitsMethod(whitelist, method, receiver, args)) { return super.onSetArray(invoker, receiver, index, value); } else { - throw StaticWhitelist.rejectMethod(method); + throw rejectMethod(method); } } args = new Object[] {receiver, index, value}; @@ -449,6 +509,40 @@ private interface Rejector { throw new RejectedAccessException("No such putAt method found: putAt method " + EnumeratingWhitelist.getName(receiver) + "[" + EnumeratingWhitelist.getName(index) + "]=" + EnumeratingWhitelist.getName(value)); } + private static void preCheckArgumentCasts(Method method, Object[] args) throws Throwable { + Parameter[] parameters = method.getParameters(); + for (int i = 0; i < parameters.length; i++) { + Parameter parameter = parameters[i]; + if (i == parameters.length - 1 && parameter.isVarArgs()) { + Class componentType = parameter.getType().getComponentType(); + for (int j = i; j < args.length; j++) { + Object arg = args[j]; + Checker.preCheckedCast(componentType, arg, false, false, false); + } + } else { + Object arg = args[i]; + Checker.preCheckedCast(parameter.getType(), arg, false, false, false); + } + } + } + + /** + * Check if the specified method defined on the receiver is synthetic. + * + * If {@code receiver} is a {@link GroovyObject} with a synthetically generated implementation of + * {@link GroovyObject#getProperty} or {@link GroovyObject#setProperty}, then we do not care about intercepting + * that method call since we handle known cases, and we specifically do not want missing properties to be rejected + * because of the existence of the method. + */ + private static boolean isSyntheticMethod(Object receiver, Method method) { + try { + return receiver.getClass().getDeclaredMethod(method.getName(), String.class, Object.class).isSynthetic(); + } catch (NoSuchMethodException e) { + // Some unusual case, e.g. the method is defined in a superclass, so we return false and intercept the call just in case. + } + return false; + } + private static String printArgumentTypes(Object[] args) { StringBuilder b = new StringBuilder(); for (Object arg : args) { @@ -472,4 +566,39 @@ private static String printArgumentTypes(Object[] args) { } } + private static boolean permitsFieldGet(@NonNull Whitelist whitelist, @NonNull Field field, @NonNull Object receiver) { + if (Modifier.isStatic(field.getModifiers())) { + return whitelist.permitsStaticFieldGet(field); + } + return whitelist.permitsFieldGet(field, receiver); + } + + private static boolean permitsFieldSet(@NonNull Whitelist whitelist, @NonNull Field field, @NonNull Object receiver, @CheckForNull Object value) { + if (Modifier.isStatic(field.getModifiers())) { + return whitelist.permitsStaticFieldSet(field, value); + } + return whitelist.permitsFieldSet(field, receiver, value); + } + + private static boolean permitsMethod(@NonNull Whitelist whitelist, @NonNull Method method, @NonNull Object receiver, @NonNull Object[] args) { + if (Modifier.isStatic(method.getModifiers())) { + return whitelist.permitsStaticMethod(method, args); + } + return whitelist.permitsMethod(method, receiver, args); + } + + public static RejectedAccessException rejectMethod(@NonNull Method m) { + if (Modifier.isStatic(m.getModifiers())) { + return StaticWhitelist.rejectStaticMethod(m); + } + return StaticWhitelist.rejectMethod(m); + } + + public static RejectedAccessException rejectField(@NonNull Field f) { + if (Modifier.isStatic(f.getModifiers())) { + return StaticWhitelist.rejectStaticField(f); + } + return StaticWhitelist.rejectField(f); + } + } diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxResolvingClassLoader.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxResolvingClassLoader.java index a6bf27147..a0935ba7e 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxResolvingClassLoader.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxResolvingClassLoader.java @@ -3,10 +3,22 @@ import com.github.benmanes.caffeine.cache.Cache; import com.github.benmanes.caffeine.cache.Caffeine; import com.github.benmanes.caffeine.cache.LoadingCache; +import com.github.benmanes.caffeine.cache.Scheduler; +import groovy.lang.GroovyClassLoader; import groovy.lang.GroovyShell; +import hudson.util.ClassLoaderSanityThreadFactory; +import hudson.util.DaemonThreadFactory; +import hudson.util.NamingThreadFactory; import java.net.URL; +import java.security.AccessControlContext; +import java.security.ProtectionDomain; import java.time.Duration; import java.util.Optional; +import java.util.concurrent.Executor; +import java.util.concurrent.Executors; +import java.util.concurrent.ForkJoinPool; +import java.util.concurrent.ForkJoinWorkerThread; +import java.util.concurrent.ThreadFactory; import java.util.function.Supplier; import java.util.logging.Level; import java.util.logging.Logger; @@ -23,6 +35,51 @@ class SandboxResolvingClassLoader extends ClassLoader { private static final Logger LOGGER = Logger.getLogger(SandboxResolvingClassLoader.class.getName()); + /** + * Care must be taken to avoid leaking instances of {@link GroovyClassLoader} when computing the cached value. + * This can happen in several ways, depending on the Caffeine configuration: + * + *

    + *
  • In its default configuration, Caffeine uses {@link ForkJoinPool#commonPool} as its {@link Executor}. + * As of recent Java versions, {@link ForkJoinPool} can capture a reference to {@link GroovyClassLoader} by + * creating a {@link ForkJoinWorkerThread} whose {@link Thread#inheritedAccessControlContext} refers to an + * {@link AccessControlContext} whose {@link ProtectionDomain} refers to {@link GroovyClassLoader}. + *
  • When Caffeine is configured with an {@link Executor} returned by {@link Executors#newCachedThreadPool}, + * that {@link Executor} can capture a reference to {@link GroovyClassLoader} by creating a {@link Thread} + * whose {@link Thread#inheritedAccessControlContext} refers to an {@link AccessControlContext} whose {@link + * ProtectionDomain} refers to {@link GroovyClassLoader}. Additionally, when the thread pool's {@link + * ThreadFactory} is not wrapped by {@link ClassLoaderSanityThreadFactory}, the {@link Executor} can sometimes + * create {@link Thread} instances whose {@link Thread#contextClassLoader} refers to {@link + * GroovyClassLoader}. + *
+ * + * As of JEP-411, {@link Thread#inheritedAccessControlContext} is + * deprecated for removal, but in the meantime we must contend with this issue. We therefore create a dedicated + * {@link Executors#newSingleThreadExecutor}, which is safe for use with Caffeine from a memory perspective because: + * + *
    + *
  • In contrast to {@link ForkJoinPool#commonPool}, the thread is eagerly created and avoids references to + * {@link GroovyClassLoader} in {@link Thread#inheritedAccessControlContext}. + *
  • In contrast to {@link Executors#newCachedThreadPool}, the thread is eagerly created and avoids references + * to {@link GroovyClassLoader} in {@link Thread#inheritedAccessControlContext}. + *
  • In contrast to {@link Executors#newCachedThreadPool}, the thread is eagerly created and avoids references + * to {@link GroovyClassLoader} in {@link Thread#contextClassLoader}, thereby avoiding the need for {@link + * ClassLoaderSanityThreadFactory}. + *
+ * + * A single-threaded {@link Executor} is safe for use with Caffeine from a CPU perspective because the cache's work is implemented with cheap O(1) algorithms. + * + *

In the medium term, once {@link Thread#inheritedAccessControlContext} is removed upstream, we could possibly + * switch to a combination of {@link Executors#newCachedThreadPool} and {@link ClassLoaderSanityThreadFactory}. + * + *

In the long term, a listener should be added to inform this class when dynamically installed plugins become + * available, as described in the comments to {@link #makeParentCache(boolean)}, in which case the use of Caffeine + * could possibly be removed entirely. + */ + private static final Executor cacheExecutor = Executors.newSingleThreadExecutor(new NamingThreadFactory( + new DaemonThreadFactory(), SandboxResolvingClassLoader.class.getName() + ".cacheExecutor")); + static final LoadingCache>> parentClassCache = makeParentCache(true); static final LoadingCache>> parentResourceCache = makeParentCache(false); @@ -98,16 +155,35 @@ private static T load(LoadingCache> cache, Str private static LoadingCache> makeParentCache(boolean weakValuesInnerCache) { // The outer cache has weak keys, so that we do not leak class loaders, but strong values, because the // inner caches are only referenced by the outer cache internally. - Caffeine outerBuilder = Caffeine.newBuilder().recordStats().weakKeys(); + Caffeine outerBuilder = Caffeine.newBuilder() + .executor(cacheExecutor) + .scheduler(Scheduler.systemScheduler()) + .recordStats() + .weakKeys(); // The inner cache has strong keys, since they are just strings, and expires entries 15 minutes after they are // added to the cache, so that classes defined by dynamically installed plugins become available even if there - // were negative cache hits prior to the installation (ideally this would be done with a listener). The values - // for the inner cache may be weak if needed, for example parentClassCache uses weak values to avoid leaking - // classes and their loaders. - Caffeine innerBuilder = Caffeine.newBuilder().recordStats().expireAfterWrite(Duration.ofMinutes(15)); + // were negative cache hits prior to the installation (ideally this would be done with a listener, in which case + // this two-level Caffeine cache could possibly be replaced by something based on ClassValue, like + // org.kohsuke.stapler.ClassLoaderValue). The values for the inner cache may be weak if needed; for example, + // parentClassCache uses weak values to avoid leaking classes and their loaders. + Caffeine innerBuilder = Caffeine.newBuilder() + .executor(cacheExecutor) + .scheduler(Scheduler.systemScheduler()) + .recordStats() + .expireAfterWrite(Duration.ofMinutes(15)); if (weakValuesInnerCache) { innerBuilder.weakValues(); } + // In both cases above, note that by default Caffeine does not perform cleanup and evict values "automatically" + // or instantly after a value expires. Instead, it performs small amounts of maintenance work after write + // operations (or occasionally after read operations if writes are rare). When Caffeine is configured with its + // default Executor of ForkJoinPool#commonPool, it immediately schedules an asynchronous eviction event after + // such write operations; however, when using a custom executor, a scheduler is required in order to run the + // maintenance activity in the near future rather than deferring it to a subsequent cache operation. Since + // Caffeine does not define a default scheduler, we explicitly configure its scheduler to the recommended + // dedicated system-wide Scheduler#systemScheduler. This preserves, as closely as possible, Caffeine's behavior + // when using ForkJoinPool#commonPool. See + // com.github.benmanes.caffeine.cache.BoundedLocalCache#rescheduleCleanUpIfIncomplete for details. return outerBuilder.build(parentLoader -> innerBuilder.build()); } diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript.java index 6a7ca1e72..dcf42ec3f 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript.java @@ -36,11 +36,9 @@ import hudson.model.Run; import hudson.model.TaskListener; import hudson.util.FormValidation; -import io.jenkins.lib.versionnumber.JavaSpecificationVersion; import java.beans.Introspector; import java.io.Serializable; -import java.lang.ref.Reference; import java.lang.ref.WeakReference; import java.lang.reflect.Field; import java.lang.reflect.Method; @@ -52,9 +50,7 @@ import java.util.HashSet; import java.util.Iterator; import java.util.List; -import java.util.Map; import java.util.Set; -import java.util.concurrent.ConcurrentMap; import java.util.logging.Level; import java.util.logging.Logger; import edu.umd.cs.findbugs.annotations.CheckForNull; @@ -121,7 +117,7 @@ public boolean isSandbox() { } public @NonNull List getClasspath() { - return classpath != null ? classpath : Collections.emptyList(); + return classpath != null ? classpath : Collections.emptyList(); } public String getOldScript() { @@ -208,10 +204,8 @@ private static void cleanUpClass(Class clazz, Set encounteredLoa if (encounteredClasses.add(clazz)) { LOGGER.log(Level.FINER, "found {0}", clazz.getName()); Introspector.flushFromCaches(clazz); - cleanUpClassInfoCache(clazz); cleanUpGlobalClassSet(clazz); cleanUpClassHelperCache(clazz); - cleanUpObjectStreamClassCaches(clazz); cleanUpLoader(clazz.getClassLoader(), encounteredLoaders, encounteredClasses); } } @@ -269,45 +263,6 @@ private static void cleanUpGlobalClassValue(@NonNull ClassLoader loader) throws } } - private static void cleanUpClassInfoCache(Class clazz) { - JavaSpecificationVersion current = JavaSpecificationVersion.forCurrentJVM(); - if (current.isNewerThan(new JavaSpecificationVersion("1.8")) - && current.isOlderThan(new JavaSpecificationVersion("16"))) { - try { - // TODO Work around JDK-8231454. - Class classInfoC = Class.forName("com.sun.beans.introspect.ClassInfo"); - Field cacheF = classInfoC.getDeclaredField("CACHE"); - try { - cacheF.setAccessible(true); - } catch (RuntimeException e) { // TODO Java 9+ InaccessibleObjectException - /* - * Not running with "--add-opens java.desktop/com.sun.beans.introspect=ALL-UNNAMED". - * Until core adds this to its --add-opens configuration, and until that core - * change is widely adopted, avoid unnecessary log spam and return early. - */ - if (LOGGER.isLoggable(Level.FINER)) { - LOGGER.log(Level.FINER, "Failed to clean up " + clazz.getName() + " from ClassInfo#CACHE. A metaspace leak may have occurred.", e); - } - return; - } - Object cache = cacheF.get(null); - Class cacheC = Class.forName("com.sun.beans.util.Cache"); - if (LOGGER.isLoggable(Level.FINER)) { - LOGGER.log(Level.FINER, "Cleaning up " + clazz.getName() + " from ClassInfo#CACHE."); - } - Method removeM = cacheC.getMethod("remove", Object.class); - removeM.invoke(cache, clazz); - } catch (ReflectiveOperationException e) { - /* - * Should never happen, but if it does, ensure the failure is isolated to this - * method and does not prevent other cleanup logic from executing. - */ - LOGGER.log(Level.WARNING, "Failed to clean up " + clazz.getName() + " from ClassInfo#CACHE. A metaspace leak may have occurred.", e); - } - } - } - - private static void cleanUpGlobalClassSet(@NonNull Class clazz) throws Exception { Class classInfoC = Class.forName("org.codehaus.groovy.reflection.ClassInfo"); // or just ClassInfo.class, but unclear whether this will always be there Field globalClassSetF = classInfoC.getDeclaredField("globalClassSet"); @@ -350,28 +305,6 @@ private static void cleanUpClassHelperCache(@NonNull Class clazz) throws Exce classCache.getClass().getMethod("remove", Object.class).invoke(classCache, clazz); } - private static void cleanUpObjectStreamClassCaches(@NonNull Class clazz) throws Exception { - Class cachesC = Class.forName("java.io.ObjectStreamClass$Caches"); - for (String cacheFName : new String[] {"localDescs", "reflectors"}) { - Field cacheF = cachesC.getDeclaredField(cacheFName); - cacheF.setAccessible(true); - Object cache = cacheF.get(null); - if (cache instanceof ConcurrentMap) { - // Prior to JDK-8277072 - Iterator>, ?>> iterator = ((ConcurrentMap) cache).entrySet().iterator(); - while (iterator.hasNext()) { - if (iterator.next().getKey().get() == clazz) { - iterator.remove(); - if (LOGGER.isLoggable(Level.FINER)) { - LOGGER.log(Level.FINER, "cleaning up {0} from ObjectStreamClass.Caches.{1}", new Object[]{clazz.getName(), cacheFName}); - } - break; - } - } - } - } - } - /** @deprecated use {@link #evaluate(ClassLoader, Binding, TaskListener)} */ @Deprecated public Object evaluate(ClassLoader loader, Binding binding) throws Exception { @@ -524,6 +457,7 @@ private final class CleanClassCollector extends ClassCollector { @Extension public static final class DescriptorImpl extends Descriptor { + @NonNull @Override public String getDisplayName() { return ""; // not intended to be displayed on its own } diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/AbstractWhitelist.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/AbstractWhitelist.java index b8d89eadf..a80fafb0a 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/AbstractWhitelist.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/AbstractWhitelist.java @@ -27,6 +27,8 @@ import java.lang.reflect.Constructor; import java.lang.reflect.Field; import java.lang.reflect.Method; + +import edu.umd.cs.findbugs.annotations.NonNull; import org.jenkinsci.plugins.scriptsecurity.sandbox.Whitelist; /** @@ -36,31 +38,31 @@ */ public abstract class AbstractWhitelist extends Whitelist { - @Override public boolean permitsMethod(Method method, Object receiver, Object[] args) { + @Override public boolean permitsMethod(@NonNull Method method, @NonNull Object receiver, @NonNull Object[] args) { return false; } - @Override public boolean permitsConstructor(Constructor constructor, Object[] args) { + @Override public boolean permitsConstructor(@NonNull Constructor constructor, @NonNull Object[] args) { return false; } - @Override public boolean permitsStaticMethod(Method method, Object[] args) { + @Override public boolean permitsStaticMethod(@NonNull Method method, @NonNull Object[] args) { return false; } - @Override public boolean permitsFieldSet(Field field, Object receiver, Object value) { + @Override public boolean permitsFieldSet(@NonNull Field field, @NonNull Object receiver, Object value) { return false; } - @Override public boolean permitsFieldGet(Field field, Object receiver) { + @Override public boolean permitsFieldGet(@NonNull Field field, @NonNull Object receiver) { return false; } - @Override public boolean permitsStaticFieldSet(Field field, Object value) { + @Override public boolean permitsStaticFieldSet(@NonNull Field field, Object value) { return false; } - @Override public boolean permitsStaticFieldGet(Field field) { + @Override public boolean permitsStaticFieldGet(@NonNull Field field) { return false; } diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/AclAwareWhitelist.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/AclAwareWhitelist.java index a978ff916..0e2eb323f 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/AclAwareWhitelist.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/AclAwareWhitelist.java @@ -24,6 +24,7 @@ package org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists; +import edu.umd.cs.findbugs.annotations.NonNull; import hudson.security.ACL; import java.lang.reflect.Constructor; import java.lang.reflect.Field; @@ -32,10 +33,10 @@ import org.jenkinsci.plugins.scriptsecurity.sandbox.Whitelist; /** - * Delegating whitelist which allows certain calls to be made only when a non-{@link ACL#SYSTEM} user is making them. + * Delegating whitelist which allows certain calls to be made only when a non-{@link ACL#SYSTEM2} user is making them. *

First there is a list of unrestricted signatures; these can always be run. *

Then there is a (probably much smaller) list of restricted signatures. - * These can be run only when the {@linkplain Jenkins#getAuthentication current user} is a real user or even {@linkplain Jenkins#ANONYMOUS}, but not when {@link ACL#SYSTEM}. + * These can be run only when the {@linkplain Jenkins#getAuthentication2 current user} is a real user or even {@linkplain Jenkins#ANONYMOUS2}, but not when {@link ACL#SYSTEM2}. * Restricted methods should be limited to those which actually perform a permissions check, typically using {@link ACL#checkPermission}. * Allowing the system pseudo-user to run these would be dangerous, since we do not know “on whose behalf” a script is running, and this “user” is permitted to do anything. */ @@ -50,41 +51,38 @@ public class AclAwareWhitelist extends Whitelist { */ public AclAwareWhitelist(Whitelist unrestricted, Whitelist restricted) { this.unrestricted = unrestricted; - if (this.unrestricted instanceof EnumeratingWhitelist) { - ((EnumeratingWhitelist) this.unrestricted).precache(); - } this.restricted = restricted; } private static boolean authenticated() { - return !ACL.SYSTEM.equals(Jenkins.getAuthentication()); + return !ACL.SYSTEM2.equals(Jenkins.getAuthentication2()); } - @Override public boolean permitsMethod(Method method, Object receiver, Object[] args) { + @Override public boolean permitsMethod(@NonNull Method method, @NonNull Object receiver, @NonNull Object[] args) { return unrestricted.permitsMethod(method, receiver, args) || authenticated() && restricted.permitsMethod(method, receiver, args); } - @Override public boolean permitsConstructor(Constructor constructor, Object[] args) { + @Override public boolean permitsConstructor(@NonNull Constructor constructor, @NonNull Object[] args) { return unrestricted.permitsConstructor(constructor, args) || authenticated() && restricted.permitsConstructor(constructor, args); } - @Override public boolean permitsStaticMethod(Method method, Object[] args) { + @Override public boolean permitsStaticMethod(@NonNull Method method, @NonNull Object[] args) { return unrestricted.permitsStaticMethod(method, args) || authenticated() && restricted.permitsStaticMethod(method, args); } - @Override public boolean permitsFieldGet(Field field, Object receiver) { + @Override public boolean permitsFieldGet(@NonNull Field field, @NonNull Object receiver) { return unrestricted.permitsFieldGet(field, receiver); } - @Override public boolean permitsFieldSet(Field field, Object receiver, Object value) { + @Override public boolean permitsFieldSet(@NonNull Field field, @NonNull Object receiver, Object value) { return unrestricted.permitsFieldSet(field, receiver, value); } - @Override public boolean permitsStaticFieldGet(Field field) { + @Override public boolean permitsStaticFieldGet(@NonNull Field field) { return unrestricted.permitsStaticFieldGet(field); } - @Override public boolean permitsStaticFieldSet(Field field, Object value) { + @Override public boolean permitsStaticFieldSet(@NonNull Field field, Object value) { return unrestricted.permitsStaticFieldSet(field, value); } diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/AnnotatedWhitelist.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/AnnotatedWhitelist.java index 282a0788d..992c3b696 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/AnnotatedWhitelist.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/AnnotatedWhitelist.java @@ -60,31 +60,31 @@ private boolean allowed(@NonNull AccessibleObject o) { return ann.restricted() == restricted; } - @Override public boolean permitsMethod(Method method, Object receiver, Object[] args) { + @Override public boolean permitsMethod(@NonNull Method method, @NonNull Object receiver, @NonNull Object[] args) { return allowed(method); } - @Override public boolean permitsConstructor(Constructor constructor, Object[] args) { + @Override public boolean permitsConstructor(@NonNull Constructor constructor, @NonNull Object[] args) { return allowed(constructor); } - @Override public boolean permitsStaticMethod(Method method, Object[] args) { + @Override public boolean permitsStaticMethod(@NonNull Method method, @NonNull Object[] args) { return allowed(method); } - @Override public boolean permitsFieldGet(Field field, Object receiver) { + @Override public boolean permitsFieldGet(@NonNull Field field, @NonNull Object receiver) { return allowed(field); } - @Override public boolean permitsFieldSet(Field field, Object receiver, Object value) { + @Override public boolean permitsFieldSet(@NonNull Field field, @NonNull Object receiver, Object value) { return allowed(field); } - @Override public boolean permitsStaticFieldGet(Field field) { + @Override public boolean permitsStaticFieldGet(@NonNull Field field) { return allowed(field); } - @Override public boolean permitsStaticFieldSet(Field field, Object value) { + @Override public boolean permitsStaticFieldSet(@NonNull Field field, Object value) { return allowed(field); } diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/BlanketWhitelist.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/BlanketWhitelist.java index 7779e2249..4a8b7d965 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/BlanketWhitelist.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/BlanketWhitelist.java @@ -27,6 +27,8 @@ import java.lang.reflect.Constructor; import java.lang.reflect.Field; import java.lang.reflect.Method; + +import edu.umd.cs.findbugs.annotations.NonNull; import org.jenkinsci.plugins.scriptsecurity.sandbox.Whitelist; /** @@ -35,31 +37,31 @@ */ public final class BlanketWhitelist extends Whitelist { - @Override public boolean permitsMethod(Method method, Object receiver, Object[] args) { + @Override public boolean permitsMethod(@NonNull Method method, @NonNull Object receiver, @NonNull Object[] args) { return true; } - @Override public boolean permitsConstructor(Constructor constructor, Object[] args) { + @Override public boolean permitsConstructor(@NonNull Constructor constructor, @NonNull Object[] args) { return true; } - @Override public boolean permitsStaticMethod(Method method, Object[] args) { + @Override public boolean permitsStaticMethod(@NonNull Method method, @NonNull Object[] args) { return true; } - @Override public boolean permitsFieldSet(Field field, Object receiver, Object value) { + @Override public boolean permitsFieldSet(@NonNull Field field, @NonNull Object receiver, Object value) { return true; } - @Override public boolean permitsFieldGet(Field field, Object receiver) { + @Override public boolean permitsFieldGet(@NonNull Field field, @NonNull Object receiver) { return true; } - @Override public boolean permitsStaticFieldSet(Field field, Object value) { + @Override public boolean permitsStaticFieldSet(@NonNull Field field, Object value) { return true; } - @Override public boolean permitsStaticFieldGet(Field field) { + @Override public boolean permitsStaticFieldGet(@NonNull Field field) { return true; } diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/EnumeratingWhitelist.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/EnumeratingWhitelist.java index 26e14674f..98ec62b1a 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/EnumeratingWhitelist.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/EnumeratingWhitelist.java @@ -24,6 +24,11 @@ package org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists; +import com.github.benmanes.caffeine.cache.Caffeine; +import com.github.benmanes.caffeine.cache.LoadingCache; +import edu.umd.cs.findbugs.annotations.CheckForNull; +import edu.umd.cs.findbugs.annotations.NonNull; +import java.lang.reflect.AccessibleObject; import java.lang.reflect.Array; import java.lang.reflect.Constructor; import java.lang.reflect.Field; @@ -31,13 +36,11 @@ import java.lang.reflect.Modifier; import java.util.Arrays; import java.util.List; -import java.util.concurrent.ConcurrentHashMap; - +import java.util.function.BiPredicate; +import java.util.function.Predicate; +import java.util.function.Supplier; import org.jenkinsci.plugins.scriptsecurity.sandbox.Whitelist; -import edu.umd.cs.findbugs.annotations.CheckForNull; -import edu.umd.cs.findbugs.annotations.NonNull; - /** * A whitelist based on listing signatures and searching them. Lists of signatures should not change * from invocation to invocation. @@ -57,130 +60,58 @@ public abstract class EnumeratingWhitelist extends Whitelist { protected abstract List staticFieldSignatures(); - ConcurrentHashMap permittedCache = new ConcurrentHashMap<>(); // Not private to facilitate testing - - @SafeVarargs - private final void cacheSignatureList(List ...sigs) { - for (List list : sigs) { - for (Signature s : list) { - if (!s.isWildcard()) { // Cache entries for wildcard signatures will never be accessed and just waste space - permittedCache.put(s.toString(), Boolean.TRUE); + private final Predicate methodCache = cache(this::methodSignatures, MethodSignature::matches); + private final Predicate> constructorCache = cache(this::newSignatures, NewSignature::matches); + private final Predicate staticMethodCache = cache(this::staticMethodSignatures, MethodSignature::matches); + private final Predicate fieldCache = cache(this::fieldSignatures, FieldSignature::matches); + private final Predicate staticFieldCache = cache(this::staticFieldSignatures, FieldSignature::matches); + + private static Predicate cache(Supplier> signatures, BiPredicate matches) { + // Unfortunately xxxSignatures() will return empty if called from superclass constructor, + // since subclass constructors initialize lists, thus the need for Supplier. + // Would be cleaner for EnumeratingWhitelist to take all signatures in its constructor, + // and for StaticWhitelist to just be a utility with static constructor methods rather than a subclass. + LoadingCache cache = Caffeine.newBuilder().weakKeys().build((M m) -> { + for (S s : signatures.get()) { + if (matches.test(s, m)) { + return true; } } - } - } - - /** Prepopulates the "permitted" cache, resetting if populated already. Should be called when method signatures change or after initialization. */ - final void precache() { - if (!permittedCache.isEmpty()) { - this.permittedCache.clear(); // No sense calling clearCache - } - cacheSignatureList((List)methodSignatures(), (List)(newSignatures()), - (List)(staticMethodSignatures()), (List)(fieldSignatures()), - (List)(staticFieldSignatures())); - } - - /** Frees up nearly all memory used for the cache. MUST BE CALLED if you change the result of the xxSignatures() methods. */ - final void clearCache() { - this.permittedCache.clear(); - this.permittedCache = new ConcurrentHashMap<>(); - } - - @Override public final boolean permitsMethod(Method method, Object receiver, Object[] args) { - String key = canonicalMethodSig(method); - Boolean b = permittedCache.get(key); - if (b != null) { - return b; - } - - boolean output = false; - for (MethodSignature s : methodSignatures()) { - if (s.matches(method)) { - output = true; - break; + return false; + }); + return m -> { + if (signatures.get().isEmpty()) { + return false; // shortcut } - } - permittedCache.put(key, output); - return output; + return cache.get(m); + }; } - @Override public final boolean permitsConstructor(Constructor constructor, Object[] args) { - String key = canonicalConstructorSig(constructor); - Boolean b = permittedCache.get(key); - if (b != null) { - return b; - } - - boolean output = false; - for (NewSignature s : newSignatures()) { - if (s.matches(constructor)) { - output = true; - break; - } - } - permittedCache.put(key, output); - return output; + @Override public final boolean permitsMethod(@NonNull Method method, @NonNull Object receiver, @NonNull Object[] args) { + return methodCache.test(method); } - @Override public final boolean permitsStaticMethod(Method method, Object[] args) { - String key = canonicalStaticMethodSig(method); - Boolean b = permittedCache.get(key); - if (b != null) { - return b; - } - - boolean output = false; - for (MethodSignature s : staticMethodSignatures()) { - if (s.matches(method)) { - output = true; - break; - } - } - permittedCache.put(key, output); - return output; + @Override public final boolean permitsConstructor(@NonNull Constructor constructor, @NonNull Object[] args) { + return constructorCache.test(constructor); } - @Override public final boolean permitsFieldGet(Field field, Object receiver) { - String key = canonicalFieldSig(field); - Boolean b = permittedCache.get(key); - if (b != null) { - return b; - } + @Override public final boolean permitsStaticMethod(@NonNull Method method, @NonNull Object[] args) { + return staticMethodCache.test(method); + } - boolean output = false; - for (FieldSignature s : fieldSignatures()) { - if (s.matches(field)) { - output = true; - break; - } - } - permittedCache.put(key, output); - return output; + @Override public final boolean permitsFieldGet(@NonNull Field field, @NonNull Object receiver) { + return fieldCache.test(field); } - @Override public final boolean permitsFieldSet(Field field, Object receiver, Object value) { + @Override public final boolean permitsFieldSet(@NonNull Field field, @NonNull Object receiver, Object value) { return permitsFieldGet(field, receiver); } - @Override public final boolean permitsStaticFieldGet(Field field) { - String key = canonicalStaticFieldSig(field); - Boolean b = permittedCache.get(key); - if (b != null) { - return b; - } - - boolean output = false; - for (FieldSignature s : staticFieldSignatures()) { - if (s.matches(field)) { - output = true; - break; - } - } - permittedCache.put(key, output); - return output; + @Override public final boolean permitsStaticFieldGet(@NonNull Field field) { + return staticFieldCache.test(field); } - @Override public final boolean permitsStaticFieldSet(Field field, Object value) { + @Override public final boolean permitsStaticFieldSet(@NonNull Field field, Object value) { return permitsStaticFieldGet(field); } @@ -218,7 +149,7 @@ public static abstract class Signature implements Comparable { } abstract boolean exists() throws Exception; /** opposite of {@link #getName(Class)} */ - static final Class type(String name) throws Exception { + static Class type(String name) throws Exception { // ClassUtils.getClass is too lax: permits Outer.Inner where we require Outer$Inner. if (name.endsWith("[]")) { // https://stackoverflow.com/q/1679421/12916; TODO Java 12+ use Class.arrayType @@ -261,7 +192,7 @@ public boolean isWildcard() { } // Utility methods for creating canonical string representations of the signature - static final StringBuilder joinWithSpaces(StringBuilder b, String[] types) { + static StringBuilder joinWithSpaces(StringBuilder b, String[] types) { for (String type : types) { b.append(' ').append(type); } @@ -276,6 +207,8 @@ static String[] argumentTypes(Class[] argumentTypes) { return s; } + // TODO move all these to StaticWhitelist + /** Canonical name for a field access. */ static String canonicalFieldString(@NonNull Field field) { return getName(field.getDeclaringClass()) + ' ' + field.getName(); @@ -378,14 +311,14 @@ static class StaticMethodSignature extends MethodSignature { public static final class NewSignature extends Signature { private final String type; private final String[] argumentTypes; - public NewSignature(String type, String[] argumentTypes) { + public NewSignature(String type, String... argumentTypes) { this.type = type; this.argumentTypes = argumentTypes.clone(); } public NewSignature(Class type, Class... argumentTypes) { this(getName(type), argumentTypes(argumentTypes)); } - boolean matches(Constructor c) { + boolean matches(Constructor c) { return getName(c.getDeclaringClass()).equals(type) && Arrays.equals(argumentTypes(c.getParameterTypes()), argumentTypes); } @Override String signaturePart() { @@ -424,8 +357,7 @@ boolean matches(Field f) { } @Override boolean exists() throws Exception { try { - type(type).getField(field); - return true; + return !Modifier.isStatic(type(type).getField(field).getModifiers()); } catch (NoSuchFieldException x) { return false; } @@ -444,6 +376,13 @@ static class StaticFieldSignature extends FieldSignature { @Override public String toString() { return "staticField " + signaturePart(); } + @Override boolean exists() throws Exception { + try { + return Modifier.isStatic(type(type).getField(field).getModifiers()); + } catch (NoSuchFieldException x) { + return false; + } + } } } diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/GenericWhitelist.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/GenericWhitelist.java index c0bc22f99..94cc374b8 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/GenericWhitelist.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/GenericWhitelist.java @@ -24,16 +24,13 @@ package org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists; -import hudson.Extension; import java.io.IOException; -import org.kohsuke.accmod.Restricted; -import org.kohsuke.accmod.restrictions.NoExternalUse; /** - * Includes entries useful for general kinds of scripts. + * @deprecated replaced by {@link StaticWhitelist#stockWhitelists}, now used only in tests */ -@Restricted(NoExternalUse.class) -@Extension public final class GenericWhitelist extends ProxyWhitelist { +@Deprecated +public final class GenericWhitelist extends ProxyWhitelist { public GenericWhitelist() throws IOException { super(StaticWhitelist.from(GenericWhitelist.class.getResource("generic-whitelist"))); diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/JenkinsWhitelist.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/JenkinsWhitelist.java deleted file mode 100644 index fcab76841..000000000 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/JenkinsWhitelist.java +++ /dev/null @@ -1,42 +0,0 @@ -/* - * The MIT License - * - * Copyright 2014 CloudBees, Inc. - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - * THE SOFTWARE. - */ - -package org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists; - -import hudson.Extension; -import java.io.IOException; -import org.kohsuke.accmod.Restricted; -import org.kohsuke.accmod.restrictions.NoExternalUse; - -/** - * Includes entries useful for scripts accessing the Jenkins API, such as model objects. - */ -@Restricted(NoExternalUse.class) -@Extension public final class JenkinsWhitelist extends ProxyWhitelist { - - public JenkinsWhitelist() throws IOException { - super(StaticWhitelist.from(JenkinsWhitelist.class.getResource("jenkins-whitelist"))); - } - -} diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/ProxyWhitelist.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/ProxyWhitelist.java index fe9bb6810..94b7eb33b 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/ProxyWhitelist.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/ProxyWhitelist.java @@ -24,273 +24,111 @@ package org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists; +import edu.umd.cs.findbugs.annotations.NonNull; import java.lang.reflect.Constructor; import java.lang.reflect.Field; import java.lang.reflect.Method; - -import net.jcip.annotations.GuardedBy; -import org.jenkinsci.plugins.scriptsecurity.sandbox.Whitelist; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.WeakHashMap; -import java.util.concurrent.locks.ReentrantReadWriteLock; +import org.jenkinsci.plugins.scriptsecurity.sandbox.Whitelist; /** * Aggregates several whitelists. */ public class ProxyWhitelist extends Whitelist { - @GuardedBy("lock") - private Collection originalDelegates; - - @GuardedBy("lock") - final List delegates = new ArrayList<>(); - - @GuardedBy("lock") - private final List methodSignatures = new ArrayList<>(); - - @GuardedBy("lock") - private final List newSignatures = new ArrayList<>(); - - @GuardedBy("lock") - private final List staticMethodSignatures = new ArrayList<>(); - @GuardedBy("lock") - private final List fieldSignatures = new ArrayList<>(); - - @GuardedBy("lock") - private final List staticFieldSignatures = new ArrayList<>(); - - /** anything wrapping us, so that we can propagate {@link #reset} calls up the chain */ - @GuardedBy("lock") - private final Map wrappers = new WeakHashMap<>(); - - // TODO Consider StampedLock when we switch to Java8 for better performance - https://dzone.com/articles/a-look-at-stampedlock - private ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); + private volatile Whitelist[] delegates; public ProxyWhitelist(Collection delegates) { reset(delegates); } - private void addWrapper(ProxyWhitelist wrapper) { - // This method should only be called from {@link ProxyWhitelist#reset(Collection)} - // where this thread already holds a write lock on the wrapper. - assert wrapper.lock.writeLock().isHeldByCurrentThread(); - lock.writeLock().lock(); - try { - wrappers.put(wrapper, null); - // The rest of the method only reads the current instance's fields - // So we downgrade this lock from write to read to reduce contention - lock.readLock().lock(); - } finally { - lock.writeLock().unlock(); - } - try { - for (Whitelist subdelegate : delegates) { - if (subdelegate instanceof EnumeratingWhitelist) { - // Discard any cache that is not the top-level cache - ((EnumeratingWhitelist) subdelegate).clearCache(); - } else { - wrapper.delegates.add(subdelegate); - } - } - wrapper.methodSignatures.addAll(methodSignatures); - wrapper.newSignatures.addAll(newSignatures); - wrapper.staticMethodSignatures.addAll(staticMethodSignatures); - wrapper.fieldSignatures.addAll(fieldSignatures); - wrapper.staticFieldSignatures.addAll(staticFieldSignatures); - } finally { - lock.readLock().unlock(); - } - } - public final void reset(Collection delegates) { - lock.writeLock().lock(); - - try { - // store the incoming delegates for use during this reset - // and during future reset propagation - originalDelegates = delegates; - reset(); - } finally { - lock.writeLock().unlock(); - } - } - - private void reset() { - lock.writeLock().lock(); - - try { - delegates.clear(); - methodSignatures.clear(); - newSignatures.clear(); - staticMethodSignatures.clear(); - fieldSignatures.clear(); - staticFieldSignatures.clear(); - - // Make the first delegate an adapter that points the fields of this proxy instance - delegates.add(new EnumeratingWhitelist() { - @Override protected List methodSignatures() { - return methodSignatures; - } - @Override protected List newSignatures() { - return newSignatures; - } - @Override protected List staticMethodSignatures() { - return staticMethodSignatures; - } - @Override protected List fieldSignatures() { - return fieldSignatures; - } - @Override protected List staticFieldSignatures() { - return staticFieldSignatures; - } - }); - for (Whitelist delegate : originalDelegates) { - if (delegate instanceof EnumeratingWhitelist) { - EnumeratingWhitelist ew = (EnumeratingWhitelist) delegate; - methodSignatures.addAll(ew.methodSignatures()); - newSignatures.addAll(ew.newSignatures()); - staticMethodSignatures.addAll(ew.staticMethodSignatures()); - fieldSignatures.addAll(ew.fieldSignatures()); - staticFieldSignatures.addAll(ew.staticFieldSignatures()); - ew.clearCache(); - } else if (delegate instanceof ProxyWhitelist) { - ProxyWhitelist pw = (ProxyWhitelist) delegate; - pw.addWrapper(this); - } else { - Objects.requireNonNull(delegate); - this.delegates.add(delegate); - } - } - for (ProxyWhitelist wrapper : wrappers.keySet()) { - wrapper.reset(); - } - if (wrappers.isEmpty()) { - // The first delegate is always an adapter pointing the fields of this proxy instance - EnumeratingWhitelist adapter = (EnumeratingWhitelist)delegates.get(0); - // Top-level ProxyWhitelist should be the only cache - // and should precache the statically permitted signatures - adapter.precache(); - } - } finally { - lock.writeLock().unlock(); - } + this.delegates = delegates.toArray(Whitelist[]::new); } public ProxyWhitelist(Whitelist... delegates) { this(Arrays.asList(delegates)); } - @Override public final boolean permitsMethod(Method method, Object receiver, Object[] args) { - lock.readLock().lock(); - try { - for (Whitelist delegate : delegates) { - if (delegate.permitsMethod(method, receiver, args)) { - return true; - } + /** + * Called before {@link #permitsMethod} and similar methods. + * May call {@link #reset(Collection)}. + */ + protected void beforePermits() {} + + @Override public final boolean permitsMethod(@NonNull Method method, @NonNull Object receiver, @NonNull Object[] args) { + beforePermits(); + for (Whitelist delegate : delegates) { + if (delegate.permitsMethod(method, receiver, args)) { + return true; } - } finally { - lock.readLock().unlock(); } return false; } - @Override public final boolean permitsConstructor(Constructor constructor, Object[] args) { - lock.readLock().lock(); - try { - for (Whitelist delegate : delegates) { - if (delegate.permitsConstructor(constructor, args)) { - return true; - } + @Override public final boolean permitsConstructor(@NonNull Constructor constructor, @NonNull Object[] args) { + beforePermits(); + for (Whitelist delegate : delegates) { + if (delegate.permitsConstructor(constructor, args)) { + return true; } - } finally { - lock.readLock().unlock(); } return false; } - @Override public final boolean permitsStaticMethod(Method method, Object[] args) { - lock.readLock().lock(); - try { - for (Whitelist delegate : delegates) { - if (delegate.permitsStaticMethod(method, args)) { - return true; - } + @Override public final boolean permitsStaticMethod(@NonNull Method method, @NonNull Object[] args) { + beforePermits(); + for (Whitelist delegate : delegates) { + if (delegate.permitsStaticMethod(method, args)) { + return true; } - } finally { - lock.readLock().unlock(); } return false; } - @Override public final boolean permitsFieldGet(Field field, Object receiver) { - lock.readLock().lock(); - try { - for (Whitelist delegate : delegates) { - if (delegate.permitsFieldGet(field, receiver)) { - return true; - } + @Override public final boolean permitsFieldGet(@NonNull Field field, @NonNull Object receiver) { + beforePermits(); + for (Whitelist delegate : delegates) { + if (delegate.permitsFieldGet(field, receiver)) { + return true; } - } finally { - lock.readLock().unlock(); } return false; } - @Override public final boolean permitsFieldSet(Field field, Object receiver, Object value) { - lock.readLock().lock(); - try { - for (Whitelist delegate : delegates) { - if (delegate.permitsFieldSet(field, receiver, value)) { - return true; - } + @Override public final boolean permitsFieldSet(@NonNull Field field, @NonNull Object receiver, Object value) { + beforePermits(); + for (Whitelist delegate : delegates) { + if (delegate.permitsFieldSet(field, receiver, value)) { + return true; } - } finally { - lock.readLock().unlock(); } - return false; } - @Override public final boolean permitsStaticFieldGet(Field field) { - lock.readLock().lock(); - try { - for (Whitelist delegate : delegates) { - if (delegate.permitsStaticFieldGet(field)) { - return true; - } + @Override public final boolean permitsStaticFieldGet(@NonNull Field field) { + beforePermits(); + for (Whitelist delegate : delegates) { + if (delegate.permitsStaticFieldGet(field)) { + return true; } - } finally { - lock.readLock().unlock(); } return false; } - @Override public final boolean permitsStaticFieldSet(Field field, Object value) { - lock.readLock().lock(); - try { - for (Whitelist delegate : delegates) { - if (delegate.permitsStaticFieldSet(field, value)) { - return true; - } + @Override public final boolean permitsStaticFieldSet(@NonNull Field field, Object value) { + beforePermits(); + for (Whitelist delegate : delegates) { + if (delegate.permitsStaticFieldSet(field, value)) { + return true; } - } finally { - lock.readLock().unlock(); } return false; } @Override public String toString() { - lock.readLock().lock(); - try { - return super.toString() + delegates; - } finally { - lock.readLock().unlock(); - } + return super.toString() + Arrays.toString(delegates); } } diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/StaticWhitelist.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/StaticWhitelist.java index 8efab7bb6..3bfb5192d 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/StaticWhitelist.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/StaticWhitelist.java @@ -46,9 +46,13 @@ import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import hudson.Extension; +import org.apache.commons.io.input.SequenceReader; import org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException; +import org.jenkinsci.plugins.scriptsecurity.sandbox.Whitelist; import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval; import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.DoNotUse; import org.kohsuke.accmod.restrictions.NoExternalUse; /** @@ -204,6 +208,17 @@ public static StaticWhitelist from(URL definition) throws IOException { } } + @Restricted(DoNotUse.class) + @Extension public static Whitelist stockWhitelists() throws IOException { + try (InputStream gis = StaticWhitelist.class.getResourceAsStream("generic-whitelist"); + Reader gr = new InputStreamReader(gis, StandardCharsets.UTF_8); + InputStream jis = StaticWhitelist.class.getResourceAsStream("jenkins-whitelist"); + Reader jr = new InputStreamReader(jis, StandardCharsets.UTF_8); + Reader r = new SequenceReader(gr, jr)) { + return new StaticWhitelist(r); + } + } + @Override protected List methodSignatures() { return methodSignatures; } diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ApprovalContext.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ApprovalContext.java index 9d82d415b..240a8ff4e 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ApprovalContext.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ApprovalContext.java @@ -58,19 +58,19 @@ public static ApprovalContext create() { /** * Creates a context with a specified user ID. - * ({@link ACL#SYSTEM} is automatically ignored.) + * ({@link ACL#SYSTEM2} is automatically ignored.) */ public ApprovalContext withUser(@CheckForNull String user) { - return new ApprovalContext(ACL.SYSTEM.getName().equals(user) ? null : user, item, key); + return new ApprovalContext(ACL.SYSTEM2.getName().equals(user) ? null : user, item, key); } /** * Creates a context with the user associated with the current thread. - * ({@link ACL#SYSTEM} is automatically ignored, but the user might be {@link Jenkins#ANONYMOUS}.) + * ({@link ACL#SYSTEM2} is automatically ignored, but the user might be {@link Jenkins#ANONYMOUS2}.) */ public ApprovalContext withCurrentUser() { User u = User.current(); - return withUser(u != null ? u.getId() : Jenkins.ANONYMOUS.getName()); + return withUser(u != null ? u.getId() : Jenkins.ANONYMOUS2.getName()); } /** diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntry.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntry.java index 9ad7f4867..1a59b008d 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntry.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ClasspathEntry.java @@ -197,6 +197,7 @@ public boolean equals(Object obj) { @Extension public static class DescriptorImpl extends Descriptor { + @NonNull @Override public String getDisplayName() { return "ClasspathEntry"; diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java index 8a0f2f1c5..264b89fcc 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java @@ -25,6 +25,9 @@ package org.jenkinsci.plugins.scriptsecurity.scripts; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import hudson.model.BallColor; +import hudson.model.PageDecorator; +import hudson.security.ACLContext; import jenkins.model.GlobalConfiguration; import jenkins.model.GlobalConfigurationCategory; import jenkins.model.ScriptListener; @@ -37,7 +40,6 @@ import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.ProxyWhitelist; import org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException; import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.StaticWhitelist; -import org.jenkinsci.plugins.scriptsecurity.sandbox.Whitelist; import org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.GroovySandbox; import hudson.Extension; import hudson.ExtensionList; @@ -51,17 +53,18 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; -import java.io.UnsupportedEncodingException; import java.net.URL; +import java.nio.charset.StandardCharsets; import java.security.DigestInputStream; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.util.ArrayList; -import java.util.Collections; import java.util.Comparator; +import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashSet; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.Stack; import java.util.TreeSet; @@ -69,29 +72,54 @@ import java.util.logging.Level; import java.util.logging.Logger; import java.util.regex.Pattern; +import java.util.stream.Collectors; + import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; import jenkins.model.Jenkins; import net.sf.json.JSON; -import org.acegisecurity.context.SecurityContext; -import org.acegisecurity.context.SecurityContextHolder; +import org.jenkinsci.plugins.scriptsecurity.sandbox.Whitelist; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.DataBoundSetter; +import org.kohsuke.stapler.QueryParameter; import org.kohsuke.stapler.bind.JavaScriptMethod; +import org.kohsuke.stapler.verb.POST; /** * Manages approved scripts. */ @Symbol("scriptApproval") @Extension -public class ScriptApproval extends GlobalConfiguration implements RootAction { +public final class ScriptApproval extends GlobalConfiguration implements RootAction { + /** + * SECURITY-2450: Since 1172.v35f6a_0b_8207e, unmodified, unsandboxed scripts are no longer automatically approved + * when administrators submit job configuration forms. + *

+ * This flag restores the previous behavior when set to {@code true}. + *

+ * + * @see 1172.v35f6a_0b_8207e changelog + */ @SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "for script console") public static /* non-final */ boolean ADMIN_AUTO_APPROVAL_ENABLED = SystemProperties.getBoolean(ScriptApproval.class.getName() + ".ADMIN_AUTO_APPROVAL_ENABLED"); + /** + * SECURITY-3103: Since 1265.va_fb_290b_4b_d34, administrators saving jobs (e.g., when copying existing jobs with + * unapproved scripts) will no longer result in unapproved scripts in those configurations being approved. + *

+ * This flag restores the previous behavior when set to {@code true}. + *

+ * + * @see 1265.va_fb_290b_4b_d34 changelog + */ + @SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "for script console") + public static /* non-final */ boolean ALLOW_ADMIN_APPROVAL_ENABLED = + SystemProperties.getBoolean(ScriptApproval.class.getName() + ".ALLOW_ADMIN_APPROVAL_ENABLED"); + private static final Logger LOG = Logger.getLogger(ScriptApproval.class.getName()); private static final XStream2 XSTREAM2 = new XStream2(); @@ -113,6 +141,7 @@ protected XmlFile getConfigFile() { return new XmlFile(XSTREAM2, new File(Jenkins.get().getRootDir(),getUrlName() + ".xml")); } + @NonNull @Override public GlobalConfigurationCategory getCategory() { return GlobalConfigurationCategory.get(GlobalConfigurationCategory.Security.class); @@ -170,8 +199,82 @@ boolean isClassDirectory() { return hash.compareTo(o.hash); } } - - /** All scripts which are already approved, via {@link #hash}. */ + + enum Hasher { + SHA512 { + final Pattern shaPattern = Pattern.compile("SHA512:[a-fA-F0-9]{128}"); + @Override + String prefix() { + return "SHA512:"; + } + + @Override + MessageDigest digest() throws NoSuchAlgorithmException { + return MessageDigest.getInstance("SHA-512"); + } + + @Override + Pattern pattern() { + return shaPattern; + } + }, + @Deprecated + SHA1 { + final Pattern shaPattern = Pattern.compile("[a-fA-F0-9]{40}"); + @Override + String prefix() { + return ""; + } + @Override + MessageDigest digest() throws NoSuchAlgorithmException { + return MessageDigest.getInstance("SHA-1"); + } + + @Override + Pattern pattern() { + return shaPattern; + } + }; + String hash(String script, String language) { + try { + MessageDigest digest = digest(); + digest.update(language.getBytes(StandardCharsets.UTF_8)); + digest.update((byte) ':'); + digest.update(script.getBytes(StandardCharsets.UTF_8)); + return prefix() + Util.toHexString(digest.digest()); + } catch (NoSuchAlgorithmException x) { + throw new AssertionError(x); + } + } + + /** + * Creates digest of JAR contents. + * Package visibility to be used in tests. + */ + String hashClasspathEntry(URL entry) throws IOException { + try { + MessageDigest digest = digest(); + try (InputStream is = entry.openStream(); BufferedInputStream bis = new BufferedInputStream(is); DigestInputStream input = new DigestInputStream(bis, digest)) { + byte[] buffer = new byte[1024]; + while (input.read(buffer) != -1) { + // discard + } + return prefix() + Util.toHexString(digest.digest()); + } + } catch (NoSuchAlgorithmException x) { + throw new AssertionError(x); + } + } + abstract String prefix(); + abstract MessageDigest digest() throws NoSuchAlgorithmException; + abstract Pattern pattern(); + } + + static final Hasher DEFAULT_HASHER = Hasher.SHA512; + + private transient Thread convertDeprecatedApprovedClasspathEntriesThread = null; + + /** All scripts which are already approved, via {@link Hasher#hash(String, String)}. */ private final TreeSet approvedScriptHashes = new TreeSet<>(); /** All sandbox signatures which are already whitelisted, in {@link StaticWhitelist} format. */ @@ -183,12 +286,78 @@ boolean isClassDirectory() { /** All external classpath entries allowed used for scripts. */ private /*final*/ TreeSet approvedClasspathEntries; - /* for test */ void addApprovedClasspathEntry(ApprovedClasspathEntry acp) { + /* for test */ synchronized void addApprovedClasspathEntry(ApprovedClasspathEntry acp) { approvedClasspathEntries.add(acp); } - public boolean isScriptApproved(String script, Language language) { - return this.isScriptHashApproved(hash(script, language.getName())); + public boolean isScriptApproved(@NonNull String script, @NonNull Language language) { + for (Hasher hasher : Hasher.values()) { //Default Hasher should be first in the array + String hash = hasher.hash(script, language.getName()); + if (this.isScriptHashApproved(hash)) { + return true; + } + } + return false; + } + + static class ConversionCheckResult { + final String oldHash; + final String newHash; + final boolean approved; + final boolean converted; + + public ConversionCheckResult(final String oldHash, final String newHash, final boolean approved, final boolean converted) { + this.oldHash = oldHash; + this.newHash = newHash; + this.approved = approved; + this.converted = converted; + } + } + + @Restricted(NoExternalUse.class) @NonNull + private synchronized ConversionCheckResult checkAndConvertApprovedScript(@NonNull String script, @NonNull Language language) { + String hash = DEFAULT_HASHER.hash(script, language.getName()); + if (approvedScriptHashes.contains(hash)) { + return new ConversionCheckResult(hash, hash, true, false); + } + for (Hasher hasher : Hasher.values()) { + if (hasher != DEFAULT_HASHER) { + String oldHash = hasher.hash(script, language.getName()); + if (approvedScriptHashes.contains(oldHash)) { + LOG.fine("A script is approved with an old hash algorithm. " + + "Converting now, this may cause performance issues until all old hashes has been converted or removed."); + approvedScriptHashes.remove(oldHash); + approvedScriptHashes.add(hash); + save(); + return new ConversionCheckResult(oldHash, hash, true, true); + } + } + } + return new ConversionCheckResult(hash, hash, false, false); + } + + @Restricted(NoExternalUse.class) @NonNull + private synchronized ConversionCheckResult checkAndConvertApprovedClasspath(@NonNull URL url) throws IOException { + String hash = DEFAULT_HASHER.hashClasspathEntry(url); + ApprovedClasspathEntry acp = new ApprovedClasspathEntry(hash, url); + if (approvedClasspathEntries.contains(acp)) { + return new ConversionCheckResult(hash, hash, true, false); + } + for (Hasher hasher : Hasher.values()) { + if (hasher != DEFAULT_HASHER) { + String oldHash = hasher.hashClasspathEntry(url); + ApprovedClasspathEntry oacp = new ApprovedClasspathEntry(oldHash, url); + if (approvedClasspathEntries.contains(oacp)) { + LOG.fine("A classpath is approved with an old hash algorithm. " + + "Converting now, this may cause performance issues until all old hashes has been converted or removed."); + approvedClasspathEntries.remove(oacp); + approvedClasspathEntries.add(acp); + save(); + return new ConversionCheckResult(oldHash, hash, true, true); + } + } + } + return new ConversionCheckResult(hash, hash, false, false); } @Restricted(NoExternalUse.class) // for use from Jelly @@ -227,7 +396,7 @@ public static final class PendingScript extends PendingThing { this.language = language.getName(); } public String getHash() { - return hash(script, language); + return DEFAULT_HASHER.hash(script, language); } public Language getLanguage() { for (Language l : ExtensionList.lookup(Language.class)) { @@ -236,9 +405,11 @@ public Language getLanguage() { } } return new Language() { + @NonNull @Override public String getName() { return language; } + @NonNull @Override public String getDisplayName() { return ""; } @@ -300,9 +471,7 @@ public static final class PendingClasspathEntry extends PendingThing implements PendingClasspathEntry(@NonNull String hash, @NonNull URL url, @NonNull ApprovalContext context) { super(context); - /** - * hash should be stored as files located at the classpath can be modified. - */ + // hash should be stored as files located at the classpath can be modified. this.hash = hash; this.url = url; } @@ -325,9 +494,7 @@ public static final class PendingClasspathEntry extends PendingThing implements } public static @NonNull PendingClasspathEntry searchKeyFor(@NonNull String hash) { - final PendingClasspathEntry entry = new PendingClasspathEntry(hash, - SEARCH_APPROVAL_URL, SEARCH_APPROVAL_CONTEXT); - return entry; + return new PendingClasspathEntry(hash, SEARCH_APPROVAL_URL, SEARCH_APPROVAL_CONTEXT); } } @@ -354,31 +521,99 @@ private PendingClasspathEntry getPendingClasspathEntry(@NonNull String hash) { @DataBoundConstructor public ScriptApproval() { load(); + } + + @Override + public synchronized void load() { + clear(); + super.load(); + // Check for loaded class directories + boolean changed = false; + int dcp = 0; + for (Iterator i = approvedClasspathEntries.iterator(); i.hasNext();) { + final ApprovedClasspathEntry entry = i.next(); + if (entry.isClassDirectory()) { + i.remove(); + changed = true; + } + if (!DEFAULT_HASHER.pattern().matcher(entry.hash).matches()) { + dcp++; + } + } + int dsh = countDeprecatedApprovedScriptHashes(); + if (dcp > 0 || dsh > 0) { + LOG.log(Level.WARNING, "There are {0} deprecated approved script hashes " + + "and {1} deprecated approved classpath hashes. " + + "They will be rehashed upon next use and that may cause performance issues " + + "until all of them are converted or removed.", new Object[]{dsh, dcp}); + } + if (changed) { + save(); + } + try { + configurationChanged(); + } catch (IOException x) { + LOG.log(Level.SEVERE, "Malformed signature entry in scriptApproval.xml: '" + x.getMessage() + "'"); + } + } + + private void clear() { + approvedScriptHashes.clear(); + approvedSignatures.clear(); + pendingScripts.clear(); + pendingSignatures.clear(); /* can be null when upgraded from old versions.*/ if (aclApprovedSignatures == null) { aclApprovedSignatures = new TreeSet<>(); + } else { + aclApprovedSignatures.clear(); } if (approvedClasspathEntries == null) { approvedClasspathEntries = new TreeSet<>(); + } else { + approvedClasspathEntries.clear(); } if (pendingClasspathEntries == null) { pendingClasspathEntries = new TreeSet<>(); + } else { + pendingClasspathEntries.clear(); } - // Check for loaded class directories - boolean changed = false; - for (Iterator i = approvedClasspathEntries.iterator(); i.hasNext();) { - if (i.next().isClassDirectory()) { - i.remove(); - changed = true; + } + + @Restricted(NoExternalUse.class) + public synchronized boolean hasDeprecatedApprovedScriptHashes() { + return countDeprecatedApprovedScriptHashes() > 0; + } + + @Restricted(NoExternalUse.class) + public synchronized int countDeprecatedApprovedScriptHashes() { + int dsh = 0; + for (String hash : approvedScriptHashes) { + if (!DEFAULT_HASHER.pattern().matcher(hash).matches()) { + dsh++; } } - if (changed) { - save(); + return dsh; + } + + @Restricted(NoExternalUse.class) + public synchronized int countDeprecatedApprovedClasspathHashes() { + int dcp = 0; + for (ApprovedClasspathEntry entry : approvedClasspathEntries) { + if (!DEFAULT_HASHER.pattern().matcher(entry.getHash()).matches()) { + dcp++; + } } + return dcp; + } + + @Restricted(NoExternalUse.class) + public synchronized boolean hasDeprecatedApprovedClasspathHashes() { + return countDeprecatedApprovedClasspathHashes() > 0; } /** Nothing has ever been approved or is pending. */ - boolean isEmpty() { + synchronized boolean isEmpty() { return approvedScriptHashes.isEmpty() && approvedSignatures.isEmpty() && aclApprovedSignatures.isEmpty() && @@ -388,53 +623,13 @@ boolean isEmpty() { pendingClasspathEntries.isEmpty(); } - private static String hash(String script, String language) { - try { - MessageDigest digest = MessageDigest.getInstance("SHA-1"); - digest.update(language.getBytes("UTF-8")); - digest.update((byte) ':'); - digest.update(script.getBytes("UTF-8")); - return Util.toHexString(digest.digest()); - } catch (NoSuchAlgorithmException | UnsupportedEncodingException x) { - throw new AssertionError(x); - } - } - - /** - * Creates digest of JAR contents. - * Package visibility to be used in tests. - */ - static String hashClasspathEntry(URL entry) throws IOException { - InputStream is = entry.openStream(); - try { - DigestInputStream input = null; - try { - MessageDigest digest = MessageDigest.getInstance("SHA-1"); - input = new DigestInputStream(new BufferedInputStream(is), digest); - byte[] buffer = new byte[1024]; - while (input.read(buffer) != -1) { - // discard - } - return Util.toHexString(digest.digest()); - } catch (NoSuchAlgorithmException x) { - throw new AssertionError(x); - } finally { - if (input != null) { - input.close(); - } - } - } finally { - is.close(); - } - } - /** * Used when someone is configuring a script. * Typically you would call this from a {@link DataBoundConstructor}. * It should also be called from a {@code readResolve} method (which may then simply return {@code this}), * so that administrators can for example POST to {@code config.xml} and have their scripts be considered approved. *

If the script has already been approved, this does nothing. - * Otherwise, if this user has the {@link Jenkins#ADMINISTER} permission (and is not {@link ACL#SYSTEM}) + * Otherwise, if this user has the {@link Jenkins#ADMINISTER} permission (and is not {@link ACL#SYSTEM2}) * and a corresponding flag is set to {@code true}, or Jenkins is running without security, it is added to the approved list. * Otherwise, it is added to the pending list. * @param script the text of a possibly novel script @@ -444,13 +639,15 @@ static String hashClasspathEntry(URL entry) throws IOException { * @return {@code script}, for convenience */ public synchronized String configuring(@NonNull String script, @NonNull Language language, @NonNull ApprovalContext context, boolean approveIfAdmin) { - final String hash = hash(script, language.getName()); - if (!approvedScriptHashes.contains(hash)) { + final ConversionCheckResult result = checkAndConvertApprovedScript(script, language); + if (!result.approved) { if (!Jenkins.get().isUseSecurity() || - ((Jenkins.getAuthentication() != ACL.SYSTEM && Jenkins.get().hasPermission(Jenkins.ADMINISTER)) - && (ADMIN_AUTO_APPROVAL_ENABLED || approveIfAdmin))) { - approvedScriptHashes.add(hash); - removePendingScript(hash); + (ALLOW_ADMIN_APPROVAL_ENABLED && + ((Jenkins.getAuthentication2() != ACL.SYSTEM2 && Jenkins.get().hasPermission(Jenkins.ADMINISTER)) + && (ADMIN_AUTO_APPROVAL_ENABLED || approveIfAdmin)))) { + approvedScriptHashes.add(result.newHash); + //Pending scripts are not stored with a precalculated hash, so no need to remove any old hashes + removePendingScript(result.newHash); } else { String key = context.getKey(); if (key != null) { @@ -499,10 +696,10 @@ public synchronized String using(@NonNull String script, @NonNull Language langu return script; } - String hash = hash(script, language.getName()); - if (!approvedScriptHashes.contains(hash)) { + ConversionCheckResult result = checkAndConvertApprovedScript(script, language); + if (!result.approved) { // Probably need not add to pendingScripts, since generally that would have happened already in configuring. - throw new UnapprovedUsageException(hash); + throw new UnapprovedUsageException(result.newHash); } ScriptListener.fireScriptEvent(script, origin, null); return script; @@ -531,29 +728,29 @@ public synchronized void configuring(@NonNull ClasspathEntry entry, @NonNull App } //TODO: better error propagation URL url = entry.getURL(); - String hash; + ConversionCheckResult result; try { - hash = hashClasspathEntry(url); + result = checkAndConvertApprovedClasspath(url); } catch (IOException x) { // This is a case the path doesn't really exist LOG.log(Level.WARNING, null, x); return; } - - ApprovedClasspathEntry acp = new ApprovedClasspathEntry(hash, url); - if (!approvedClasspathEntries.contains(acp)) { + + if (!result.approved) { boolean shouldSave = false; - PendingClasspathEntry pcp = new PendingClasspathEntry(hash, url, context); + PendingClasspathEntry pcp = new PendingClasspathEntry(result.newHash, url, context); if (!Jenkins.get().isUseSecurity() || - ((Jenkins.getAuthentication() != ACL.SYSTEM && Jenkins.get().hasPermission(Jenkins.ADMINISTER)) + ((Jenkins.getAuthentication2() != ACL.SYSTEM2 && Jenkins.get().hasPermission(Jenkins.ADMINISTER)) && (ADMIN_AUTO_APPROVAL_ENABLED || entry.isShouldBeApproved() || !StringUtils.equals(entry.getOldPath(), entry.getPath())))) { - LOG.log(Level.FINE, "Classpath entry {0} ({1}) is approved as configured with ADMINISTER permission.", new Object[] {url, hash}); + LOG.log(Level.FINE, "Classpath entry {0} ({1}) is approved as configured with ADMINISTER permission.", new Object[] {url, result.newHash}); + ApprovedClasspathEntry acp = new ApprovedClasspathEntry(result.newHash, url); pendingClasspathEntries.remove(pcp); approvedClasspathEntries.add(acp); shouldSave = true; } else { if (pendingClasspathEntries.add(pcp)) { - LOG.log(Level.FINE, "{0} ({1}) is pending", new Object[] {url, hash}); + LOG.log(Level.FINE, "{0} ({1}) is pending", new Object[] {url, result.newHash}); shouldSave = true; } } @@ -593,25 +790,24 @@ public synchronized FormValidation checking(@NonNull ClasspathEntry entry) { */ public synchronized void using(@NonNull ClasspathEntry entry) throws IOException, UnapprovedClasspathException { URL url = entry.getURL(); - String hash = hashClasspathEntry(url); - - if (!approvedClasspathEntries.contains(new ApprovedClasspathEntry(hash, url))) { - // Don't add it to pending if it is a class directory - if (entry.isClassDirectory()) { - LOG.log(Level.WARNING, "Classpath {0} ({1}) is a class directory, which are not allowed.", new Object[] {url, hash}); - throw new UnapprovedClasspathException("classpath entry %s is a class directory, which are not allowed.", url, hash); - } else { - // Never approve classpath here. - ApprovalContext context = ApprovalContext.create(); - if (pendingClasspathEntries.add(new PendingClasspathEntry(hash, url, context))) { - LOG.log(Level.FINE, "{0} ({1}) is pending.", new Object[] {url, hash}); - save(); - } + // Don't add it to pending if it is a class directory + if (entry.isClassDirectory()) { + LOG.log(Level.WARNING, "Classpath {0} is a class directory, which are not allowed.", url); + throw new UnapprovedClasspathException("classpath entry %s is a class directory, which are not allowed.", url, ""); + } + ConversionCheckResult result = checkAndConvertApprovedClasspath(url); + + if (!result.approved) { + // Never approve classpath here. + ApprovalContext context = ApprovalContext.create(); + if (pendingClasspathEntries.add(new PendingClasspathEntry(result.newHash, url, context))) { + LOG.log(Level.FINE, "{0} ({1}) is pending.", new Object[]{url, result.newHash}); + save(); } - throw new UnapprovedClasspathException(url, hash); + throw new UnapprovedClasspathException(url, result.newHash); } - LOG.log(Level.FINER, "{0} ({1}) had been approved", new Object[] {url, hash}); + LOG.log(Level.FINER, "{0} ({1}) had been approved", new Object[] {url, result.newHash}); } /** @@ -629,23 +825,32 @@ public synchronized FormValidation checking(@NonNull String script, @NonNull Lan if (StringUtils.isEmpty(script)) { return FormValidation.ok(); } - if (approvedScriptHashes.contains(hash(script, language.getName()))) { + final ConversionCheckResult result = checkAndConvertApprovedScript(script, language); + if (result.approved) { return FormValidation.okWithMarkup("The script is already approved"); } if (!Jenkins.get().hasPermission(Jenkins.ADMINISTER)) { return FormValidation.warningWithMarkup("A Jenkins administrator will need to approve this script before it can be used"); } else { - if (willBeApproved || ADMIN_AUTO_APPROVAL_ENABLED) { - return FormValidation.ok("The script has not yet been approved, but it will be approved on save"); + if ((ALLOW_ADMIN_APPROVAL_ENABLED && (willBeApproved || ADMIN_AUTO_APPROVAL_ENABLED)) || !Jenkins.get().isUseSecurity()) { + return FormValidation.okWithMarkup("The script has not yet been approved, but it will be approved on save."); } - + String approveScript = "Approve script"; return FormValidation.okWithMarkup("The script is not approved and will not be approved on save. " + - "Modify the script to approve it on save, or approve it explicitly on the " + - "Script Approval Configuration page"); + "Either modify the script to match an already approved script, approve it explicitly on the " + + "Script Approval Configuration page after save, or approve this version of the script. " + + approveScript); } } + @Restricted(NoExternalUse.class) + @POST + // can not call this method doApproveScript as that collides with the javascript binding in #approveScript + public synchronized void doApproveScriptHash(@QueryParameter(required=true) String hash) throws IOException { + approveScript(hash); + } + /** * @deprecated Use {@link #checking(String, Language, boolean)} instead */ @@ -656,7 +861,8 @@ public synchronized FormValidation checking(@NonNull String script, @NonNull Lan synchronized boolean isClasspathEntryApproved(URL url) { try { - return approvedClasspathEntries.contains(new ApprovedClasspathEntry(hashClasspathEntry(url), url)); + final ConversionCheckResult result = checkAndConvertApprovedClasspath(url); + return result.approved; } catch (IOException e) { return false; } @@ -671,7 +877,7 @@ synchronized boolean isClasspathEntryApproved(URL url) { * @return {@code script}, for convenience */ public synchronized String preapprove(@NonNull String script, @NonNull Language language) { - approvedScriptHashes.add(hash(script, language.getName())); + approvedScriptHashes.add(DEFAULT_HASHER.hash(script, language.getName())); return script; } @@ -734,7 +940,7 @@ public synchronized void setApprovedSignatures(String[] signatures) throws IOExc goodSignatures.add(signature); } catch (IOException e) { LOG.warning("Ignoring malformed signature: " + signature - + " (Occurred exception: " + e.toString() + ")"); + + " (Occurred exception: " + e + ")"); } } approvedSignatures.addAll(goodSignatures); @@ -765,14 +971,27 @@ public synchronized String[] getAclApprovedSignatures() { @DataBoundSetter public synchronized void setApprovedScriptHashes(String[] scriptHashes) throws IOException { - Jenkins.getInstance().checkPermission(Jenkins.RUN_SCRIPTS); + Jenkins.get().checkPermission(Jenkins.ADMINISTER); approvedScriptHashes.clear(); - Pattern sha1Pattern = Pattern.compile("[a-fA-F0-9]{40}"); for (String scriptHash : scriptHashes) { - if (scriptHash != null && sha1Pattern.matcher(scriptHash).matches()) { - approvedScriptHashes.add(scriptHash); - } else { - LOG.warning(() -> "Ignoring malformed script hash: " + scriptHash); + if (StringUtils.isNotEmpty(scriptHash)) { + if (DEFAULT_HASHER.pattern().matcher(scriptHash).matches()) { + approvedScriptHashes.add(scriptHash); + } else { + boolean allowed = false; + for (Hasher hasher : Hasher.values()) { + if (hasher != DEFAULT_HASHER && hasher.pattern().matcher(scriptHash).matches()) { + allowed = true; + break; + } + } + if (allowed) { + LOG.warning(() -> "Adding deprecated script hash that will be converted on next use: " + scriptHash); + approvedScriptHashes.add(scriptHash); + } else { + LOG.warning(() -> "Ignoring malformed script hash: " + scriptHash); + } + } } } save(); @@ -784,23 +1003,33 @@ public synchronized String[] getApprovedScriptHashes() { return approvedScriptHashes.toArray(new String[approvedScriptHashes.size()]); } + private synchronized void configurationChanged() throws IOException { + // Do not use lookupSingleton: ScriptApprovalLoadingTest.dynamicLoading + ApprovedWhitelist instance = ExtensionList.lookup(Whitelist.class).get(ApprovedWhitelist.class); + if (instance == null) { + throw new IllegalStateException("Failed to find ApprovedWhitelist"); + } + LOG.fine("resetting"); + synchronized (instance) { + instance.pendingDelegate = new AclAwareWhitelist(new StaticWhitelist(approvedSignatures), new StaticWhitelist(aclApprovedSignatures)); + } + } + @Restricted(NoExternalUse.class) // implementation @Extension public static final class ApprovedWhitelist extends ProxyWhitelist { - public ApprovedWhitelist() { - try { - reconfigure(); - } catch (IOException e) { - LOG.log(Level.SEVERE, "Malformed signature entry in scriptApproval.xml: '" + e.getMessage() + "'"); - } - } - String[][] reconfigure() throws IOException { - ScriptApproval instance = ScriptApproval.get(); - synchronized (instance) { - reset(Collections.singleton(new AclAwareWhitelist(new StaticWhitelist(instance.approvedSignatures), new StaticWhitelist(instance.aclApprovedSignatures)))); - return new String[][] {instance.getApprovedSignatures(), instance.getAclApprovedSignatures(), instance.getDangerousApprovedSignatures()}; + private @CheckForNull Whitelist pendingDelegate; + + @Override protected synchronized void beforePermits() { + if (pendingDelegate != null) { + LOG.fine("refreshing"); + reset(Set.of(pendingDelegate)); + pendingDelegate = null; + } else { + LOG.finer("no need to refresh"); } } + } @Override public String getIconFileName() { @@ -824,13 +1053,11 @@ public Set getPendingScripts() { removePendingScript(hash); save(); } - SecurityContext orig = ACL.impersonate(ACL.SYSTEM); - try { + + try (ACLContext ctx = ACL.as2(ACL.SYSTEM2)) { for (ApprovalListener listener : ExtensionList.lookup(ApprovalListener.class)) { listener.onApproved(hash); } - } finally { - SecurityContextHolder.setContext(orig); } } @@ -859,19 +1086,93 @@ synchronized void removePendingScript(String hash) { save(); } + + /** + * Clears {@link #approvedScriptHashes} from all entries not matching {@link #DEFAULT_HASHER}. + * @throws IOException if so when saving to disk. + */ + @Restricted(NoExternalUse.class) // for use from AJAX + @JavaScriptMethod public synchronized void clearDeprecatedApprovedScripts() throws IOException { + Jenkins.get().checkPermission(Jenkins.ADMINISTER); + approvedScriptHashes.removeIf(s -> !DEFAULT_HASHER.pattern().matcher(s).matches()); + save(); + } + + @Restricted(NoExternalUse.class) + public String getSpinnerIconClassName() { + return BallColor.GREY_ANIME.getIconClassName(); + } + + /** + * Schedules a {@link Thread} task that rehashes/converts all approved classpath entries + * that are hashed not using {@link #DEFAULT_HASHER}. + */ + @Restricted(NoExternalUse.class) // for use from AJAX + @JavaScriptMethod public synchronized void convertDeprecatedApprovedClasspathEntries() { + Jenkins.get().checkPermission(Jenkins.ADMINISTER); + if (!isConvertingDeprecatedApprovedClasspathEntries()) { + final List entries = approvedClasspathEntries.stream() + .filter(e -> !DEFAULT_HASHER.pattern().matcher(e.getHash()).matches()) + .collect(Collectors.toList()); + if (!entries.isEmpty()) { + LOG.log(Level.INFO, "Scheduling conversion of {0} deprecated approved classpathentry hashes.", entries.size()); + convertDeprecatedApprovedClasspathEntriesThread = new Thread(() -> { + final Map result = new HashMap<>(); + for (int i = 0; i < entries.size(); i++) { + final ApprovedClasspathEntry entry = entries.get(i); + final URL entryURL = entry.getURL(); + LOG.log(Level.INFO, String.format("Converting %s\t(%d/%d)", entryURL, i + 1, entries.size())); + try { + final String hash = DEFAULT_HASHER.hashClasspathEntry(entryURL); + result.put(entryURL.toExternalForm(), new ApprovedClasspathEntry(hash, entryURL)); + } catch (Throwable e) { + LOG.log(Level.WARNING, "Failed to convert " + entryURL, e); + } + Thread.yield(); //Technically not needed as there is plenty of IO happening in this thread. + } + synchronized (ScriptApproval.this) { + approvedClasspathEntries.removeIf(e -> result.containsKey(e.getURL().toExternalForm())); + approvedClasspathEntries.addAll(result.values()); + try { + save(); + } catch (Exception e) { + LOG.log(Level.WARNING, "Failed to store conversion result.", e); + } + } + LOG.info("Conversion done."); + synchronized (ScriptApproval.this) { + convertDeprecatedApprovedClasspathEntriesThread = null; + } + }, "Approved Classpaths rehasher"); + convertDeprecatedApprovedClasspathEntriesThread.setDaemon(true); + convertDeprecatedApprovedClasspathEntriesThread.start(); + LOG.fine("Background conversion task scheduled."); + } else { + LOG.info("Nothing to convert."); + } + } else { + LOG.fine("Background conversion task already running."); + } + } + + /** + * Checks if {@link #convertDeprecatedApprovedClasspathEntriesThread} is active. + * @return true if so. + */ + @Restricted(NoExternalUse.class) + public synchronized boolean isConvertingDeprecatedApprovedClasspathEntries() { + return convertDeprecatedApprovedClasspathEntriesThread != null + && convertDeprecatedApprovedClasspathEntriesThread.isAlive(); + } + @Restricted(NoExternalUse.class) // for use from Jelly public Set getPendingSignatures() { return pendingSignatures; } - // Added to account for new findbugs annotations in ExtensionList#get (PCT scenario) - private String[][] reconfigure() throws IOException { - final ApprovedWhitelist awl = ExtensionList.lookup(Whitelist.class).get(ApprovedWhitelist.class); - if (awl != null) { - return awl.reconfigure(); - } else { - return new String[][] {new String[0], new String[0], new String[0]}; - } + private synchronized String[][] reconfigure() throws IOException { + configurationChanged(); + return new String[][] {getApprovedSignatures(), getAclApprovedSignatures(), getDangerousApprovedSignatures()}; } @Restricted(NoExternalUse.class) // for use from AJAX @@ -935,14 +1236,14 @@ private String[][] reconfigure() throws IOException { @Restricted(NoExternalUse.class) public synchronized List getApprovedClasspathEntries() { ArrayList r = new ArrayList<>(approvedClasspathEntries); - Collections.sort(r, Comparator.comparing(o -> o.url.toString())); + r.sort(Comparator.comparing(o -> o.url.toString())); return r; } @Restricted(NoExternalUse.class) public synchronized List getPendingClasspathEntries() { List r = new ArrayList<>(pendingClasspathEntries); - Collections.sort(r, Comparator.comparing(o -> o.url.toString())); + r.sort(Comparator.comparing(o -> o.url.toString())); return r; } @@ -962,7 +1263,7 @@ public JSON getClasspathRenderInfo() { @Restricted(NoExternalUse.class) // for use from AJAX @JavaScriptMethod - public JSON approveClasspathEntry(String hash) throws IOException { + public synchronized JSON approveClasspathEntry(String hash) throws IOException { Jenkins.get().checkPermission(Jenkins.ADMINISTER); URL url = null; synchronized (this) { @@ -975,13 +1276,10 @@ public JSON approveClasspathEntry(String hash) throws IOException { } } if (url != null) { - SecurityContext orig = ACL.impersonate(ACL.SYSTEM); - try { + try (ACLContext ctx = ACL.as2(ACL.SYSTEM2)) { for (ApprovalListener listener : ExtensionList.lookup(ApprovalListener.class)) { listener.onApprovedClasspathEntry(hash, url); } - } finally { - SecurityContextHolder.setContext(orig); } } return getClasspathRenderInfo(); @@ -989,7 +1287,7 @@ public JSON approveClasspathEntry(String hash) throws IOException { @Restricted(NoExternalUse.class) // for use from AJAX @JavaScriptMethod - public JSON denyClasspathEntry(String hash) throws IOException { + public synchronized JSON denyClasspathEntry(String hash) throws IOException { Jenkins.get().checkPermission(Jenkins.ADMINISTER); PendingClasspathEntry cp = getPendingClasspathEntry(hash); if (cp != null) { @@ -1001,7 +1299,7 @@ public JSON denyClasspathEntry(String hash) throws IOException { @Restricted(NoExternalUse.class) // for use from AJAX @JavaScriptMethod - public JSON denyApprovedClasspathEntry(String hash) throws IOException { + public synchronized JSON denyApprovedClasspathEntry(String hash) throws IOException { Jenkins.get().checkPermission(Jenkins.ADMINISTER); if (approvedClasspathEntries.remove(new ApprovedClasspathEntry(hash, null))) { save(); @@ -1018,4 +1316,7 @@ public synchronized JSON clearApprovedClasspathEntries() throws IOException { return getClasspathRenderInfo(); } + @Restricted(NoExternalUse.class) + @Extension + public static class FormValidationPageDecorator extends PageDecorator {} } diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalLink.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalLink.java index 484f4444b..38e73c99f 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalLink.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalLink.java @@ -24,9 +24,11 @@ package org.jenkinsci.plugins.scriptsecurity.scripts; +import edu.umd.cs.findbugs.annotations.NonNull; import hudson.Extension; import hudson.model.ManagementLink; import hudson.security.Permission; +import jenkins.management.Badge; import jenkins.model.Jenkins; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; @@ -38,7 +40,7 @@ if (ScriptApproval.get().isEmpty()) { return null; } - return "notepad.svg"; + return "symbol-edit-note"; } @Override public String getUrlName() { @@ -71,13 +73,60 @@ return message; } + @NonNull @Override public Permission getRequiredPermission() { return Jenkins.ADMINISTER; } - // TODO: Override `getCategory` instead using `Category.SECURITY` when minimum core version is 2.226+, see https://github.com/jenkinsci/jenkins/commit/6de7e5fc7f6fb2e2e4cb342461788f97e3dfd8f6. - protected String getCategoryName() { - return "SECURITY"; + @NonNull + @Override + public Category getCategory() { + return Category.SECURITY; } + @Override + public Badge getBadge() { + int pendingScripts = ScriptApproval.get().getPendingScripts().size(); + int pendingSignatures = ScriptApproval.get().getPendingSignatures().size(); + int pendingClasspathEntries = ScriptApproval.get().getPendingClasspathEntries().size(); + int dangerous = ScriptApproval.get().getDangerousApprovedSignatures().length; + int total = 0; + StringBuilder toolTip = new StringBuilder(); + if (pendingScripts > 0) { + total += pendingScripts; + toolTip.append(Messages.ScriptApprovalLink_outstandingScript(pendingScripts)); + } + if (pendingSignatures > 0) { + if (total > 0) { + toolTip.append("\n"); + } + toolTip.append(Messages.ScriptApprovalLink_outstandingSignature(pendingSignatures)); + total += pendingSignatures; + } + if (pendingClasspathEntries > 0) { + if (total > 0) { + toolTip.append("\n"); + } + toolTip.append(Messages.ScriptApprovalLink_outstandingClasspath(pendingClasspathEntries)); + total += pendingClasspathEntries; + } + if (total > 0 || dangerous > 0) { + StringBuilder text = new StringBuilder(); + if (total > 0) { + text.append(total); + } + if (dangerous > 0) { + if (total > 0) { + toolTip.append("\n"); + text.append("/"); + } + text.append(dangerous); + toolTip.append(Messages.ScriptApprovalLink_dangerous(dangerous)); + } + Badge.Severity severity = dangerous > 0 ? Badge.Severity.DANGER : Badge.Severity.WARNING; + return new Badge(text.toString(), toolTip.toString(), severity); + } + + return null; + } } diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/languages/GroovyLanguage.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/languages/GroovyLanguage.java index 83d9c932d..5298d809f 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/languages/GroovyLanguage.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/languages/GroovyLanguage.java @@ -24,6 +24,7 @@ package org.jenkinsci.plugins.scriptsecurity.scripts.languages; +import edu.umd.cs.findbugs.annotations.NonNull; import org.jenkinsci.plugins.scriptsecurity.scripts.Language; import hudson.Extension; import hudson.ExtensionList; @@ -37,10 +38,12 @@ public static Language get() { return ExtensionList.lookup(Language.class).get(GroovyLanguage.class); } + @NonNull @Override public String getName() { return "groovy"; } + @NonNull @Override public String getDisplayName() { return "Groovy"; } diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/languages/GroovyShellLanguage.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/languages/GroovyShellLanguage.java index ab96a3550..ba7fbcfe6 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/languages/GroovyShellLanguage.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/languages/GroovyShellLanguage.java @@ -24,6 +24,7 @@ package org.jenkinsci.plugins.scriptsecurity.scripts.languages; +import edu.umd.cs.findbugs.annotations.NonNull; import org.jenkinsci.plugins.scriptsecurity.scripts.Language; import hudson.Extension; import hudson.ExtensionList; @@ -40,10 +41,12 @@ public static Language get() { return ExtensionList.lookup(Language.class).get(GroovyShellLanguage.class); } + @NonNull @Override public String getName() { return "groovy-sh"; } + @NonNull @Override public String getDisplayName() { return "Groovy Template for Shell"; } diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/languages/GroovyXmlLanguage.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/languages/GroovyXmlLanguage.java index 173875f2a..f65dc54a0 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/languages/GroovyXmlLanguage.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/languages/GroovyXmlLanguage.java @@ -24,6 +24,7 @@ package org.jenkinsci.plugins.scriptsecurity.scripts.languages; +import edu.umd.cs.findbugs.annotations.NonNull; import org.jenkinsci.plugins.scriptsecurity.scripts.Language; import hudson.Extension; import hudson.ExtensionList; @@ -37,10 +38,12 @@ public static Language get() { return ExtensionList.lookup(Language.class).get(GroovyXmlLanguage.class); } + @NonNull @Override public String getName() { return "groovy-xml"; } + @NonNull @Override public String getDisplayName() { return "Groovy Template for XML"; } diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/languages/JellyLanguage.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/languages/JellyLanguage.java index 9fac14047..4c2074318 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/languages/JellyLanguage.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/languages/JellyLanguage.java @@ -24,6 +24,7 @@ package org.jenkinsci.plugins.scriptsecurity.scripts.languages; +import edu.umd.cs.findbugs.annotations.NonNull; import org.jenkinsci.plugins.scriptsecurity.scripts.Language; import hudson.Extension; import hudson.ExtensionList; @@ -37,10 +38,12 @@ public static Language get() { return ExtensionList.lookup(Language.class).get(JellyLanguage.class); } + @NonNull @Override public String getName() { return "jelly"; } + @NonNull @Override public String getDisplayName() { return "Jelly"; } diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/languages/JexlLanguage.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/languages/JexlLanguage.java index a4f5542ce..6bb892df6 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/languages/JexlLanguage.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/languages/JexlLanguage.java @@ -24,6 +24,7 @@ package org.jenkinsci.plugins.scriptsecurity.scripts.languages; +import edu.umd.cs.findbugs.annotations.NonNull; import org.jenkinsci.plugins.scriptsecurity.scripts.Language; import hudson.Extension; import hudson.ExtensionList; @@ -37,10 +38,12 @@ public static Language get() { return ExtensionList.lookup(Language.class).get(JexlLanguage.class); } + @NonNull @Override public String getName() { return "jexl"; } + @NonNull @Override public String getDisplayName() { return "JEXL"; } diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/languages/SystemCommandLanguage.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/languages/SystemCommandLanguage.java index e08383df9..82378aa57 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/languages/SystemCommandLanguage.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/languages/SystemCommandLanguage.java @@ -24,6 +24,7 @@ package org.jenkinsci.plugins.scriptsecurity.scripts.languages; +import edu.umd.cs.findbugs.annotations.NonNull; import hudson.Extension; import hudson.ExtensionList; import hudson.Util; @@ -40,11 +41,13 @@ public static Language get() { return ExtensionList.lookup(Language.class).get(SystemCommandLanguage.class); } + @NonNull @Override public String getName() { return "system-command"; } + @NonNull @Override public String getDisplayName() { return "System Commands"; diff --git a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript/JENKINS-15604.js b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript/JENKINS-15604.js index d3a0e6401..a196cfc08 100644 --- a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript/JENKINS-15604.js +++ b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript/JENKINS-15604.js @@ -1,5 +1,5 @@ // https://issues.jenkins-ci.org/browse/JENKINS-15604 workaround: function cmChange(editor, change) { editor.save(); - $$('.validated').each(function (e) {e.onchange();}); + document.querySelectorAll('.validated').forEach(function (e) {e.onchange();}); } diff --git a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/generic-whitelist b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/generic-whitelist index a13119ef6..8828d126f 100644 --- a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/generic-whitelist +++ b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/generic-whitelist @@ -31,12 +31,16 @@ method groovy.lang.Closure ncurry int java.lang.Object method groovy.lang.Closure ncurry int java.lang.Object[] method groovy.lang.Closure setDelegate java.lang.Object method groovy.lang.Closure setResolveStrategy int +new groovy.lang.EmptyRange java.lang.Comparable method groovy.lang.GString plus java.lang.String +new groovy.lang.IntRange boolean int int method groovy.lang.Range getFrom method groovy.lang.Range getTo method groovy.lang.Range step int method groovy.lang.Range step int groovy.lang.Closure +new groovy.lang.Script method groovy.lang.Script getBinding +new groovy.lang.Script groovy.lang.Binding staticMethod groovy.xml.XmlUtil escapeXml java.lang.String method java.io.Flushable flush @@ -267,6 +271,7 @@ method java.lang.String split java.lang.String method java.lang.String split java.lang.String int method java.lang.String startsWith java.lang.String method java.lang.String startsWith java.lang.String int +method java.lang.String stripIndent method java.lang.String substring int method java.lang.String substring int int method java.lang.String toCharArray @@ -299,6 +304,9 @@ method java.lang.Throwable getMessage method java.lang.Throwable printStackTrace java.io.PrintStream method java.lang.Throwable printStackTrace java.io.PrintWriter method java.math.BigDecimal multiply java.math.BigDecimal +method java.math.BigDecimal negate +method java.math.BigInteger negate +method java.math.BigInteger not new java.net.MalformedURLException java.lang.String new java.net.URL java.lang.String staticMethod java.net.URLDecoder decode java.lang.String java.lang.String @@ -480,6 +488,8 @@ staticField java.time.format.DateTimeFormatter ISO_WEEK_DATE staticField java.time.format.DateTimeFormatter ISO_ZONED_DATE_TIME staticField java.time.format.DateTimeFormatter RFC_1123_DATE_TIME staticMethod java.time.format.DateTimeFormatter ofPattern java.lang.String +staticMethod java.time.format.DateTimeFormatter ofPattern java.lang.String java.util.Locale +method java.time.format.DateTimeFormatter parse java.lang.CharSequence staticField java.time.temporal.ChronoUnit CENTURIES staticField java.time.temporal.ChronoUnit DAYS staticField java.time.temporal.ChronoUnit DECADES @@ -784,11 +794,16 @@ method java.util.random.RandomGenerator nextInt int method java.util.random.RandomGenerator nextLong method java.util.regex.MatchResult end method java.util.regex.MatchResult end int +method java.util.regex.MatchResult end java.lang.String method java.util.regex.MatchResult group method java.util.regex.MatchResult group int +method java.util.regex.MatchResult group java.lang.String method java.util.regex.MatchResult groupCount +method java.util.regex.MatchResult hasMatch +method java.util.regex.MatchResult namedGroups method java.util.regex.MatchResult start method java.util.regex.MatchResult start int +method java.util.regex.MatchResult start java.lang.String method java.util.regex.Matcher appendReplacement java.lang.StringBuffer java.lang.String method java.util.regex.Matcher appendTail java.lang.StringBuffer method java.util.regex.Matcher end java.lang.String @@ -843,9 +858,11 @@ staticMethod org.codehaus.groovy.runtime.DateGroovyMethods getTimeString java.ut staticMethod org.codehaus.groovy.runtime.DateGroovyMethods minus java.util.Calendar java.util.Calendar staticMethod org.codehaus.groovy.runtime.DateGroovyMethods minus java.util.Date int staticMethod org.codehaus.groovy.runtime.DateGroovyMethods minus java.util.Date java.util.Date +staticMethod org.codehaus.groovy.runtime.DateGroovyMethods next java.sql.Date staticMethod org.codehaus.groovy.runtime.DateGroovyMethods next java.util.Calendar staticMethod org.codehaus.groovy.runtime.DateGroovyMethods next java.util.Date staticMethod org.codehaus.groovy.runtime.DateGroovyMethods plus java.util.Date int +staticMethod org.codehaus.groovy.runtime.DateGroovyMethods previous java.sql.Date staticMethod org.codehaus.groovy.runtime.DateGroovyMethods previous java.util.Calendar staticMethod org.codehaus.groovy.runtime.DateGroovyMethods previous java.util.Date staticMethod org.codehaus.groovy.runtime.DateGroovyMethods putAt java.util.Calendar int int @@ -861,9 +878,30 @@ staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods and java.lang.Bool staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods any java.lang.Iterable groovy.lang.Closure staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods any java.lang.Object groovy.lang.Closure staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods any java.util.Map groovy.lang.Closure +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods asBoolean boolean[] +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods asBoolean byte[] +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods asBoolean char[] +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods asBoolean double[] +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods asBoolean float[] +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods asBoolean int[] +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods asBoolean java.lang.Boolean staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods asBoolean java.lang.CharSequence +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods asBoolean java.lang.Character +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods asBoolean java.lang.Number staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods asBoolean java.lang.Object +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods asBoolean java.lang.Object[] +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods asBoolean java.util.Collection +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods asBoolean java.util.Enumeration +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods asBoolean java.util.Iterator +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods asBoolean java.util.Map +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods asBoolean java.util.regex.Matcher +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods asBoolean long[] +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods asBoolean short[] staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods asImmutable java.util.Map +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods bitwiseNegate java.lang.CharSequence +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods bitwiseNegate java.lang.Number +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods bitwiseNegate java.lang.String +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods bitwiseNegate java.util.BitSet staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods capitalize java.lang.String staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods collate java.lang.Iterable int staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods collate java.lang.Iterable int boolean @@ -1067,7 +1105,10 @@ staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods minus java.util.So staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods minus java.util.SortedSet java.lang.Object staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods minus java.util.SortedSet java.util.Collection staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods multiply java.lang.String java.lang.Number +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods next java.lang.CharSequence +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods next java.lang.Character staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods next java.lang.Number +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods next java.lang.String staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods or java.lang.Boolean java.lang.Boolean staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods padLeft java.lang.CharSequence java.lang.Number staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods padLeft java.lang.CharSequence java.lang.Number java.lang.CharSequence @@ -1115,7 +1156,10 @@ staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods power java.lang.Lo staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods power java.lang.Number java.lang.Number staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods power java.math.BigDecimal java.lang.Integer staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods power java.math.BigInteger java.lang.Integer +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods previous java.lang.CharSequence +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods previous java.lang.Character staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods previous java.lang.Number +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods previous java.lang.String staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods push java.util.List java.lang.Object staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods putAll java.util.Map java.util.Collection staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods putAt java.util.List int java.lang.Object @@ -1185,6 +1229,8 @@ staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods times java.lang.Nu staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods toBoolean java.lang.Boolean staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods toBoolean java.lang.String staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods toDouble java.lang.String +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods toFloat java.lang.String +staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods toInteger java.lang.Number staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods toInteger java.lang.String staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods toList boolean[] staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods toList byte[] @@ -1248,7 +1294,6 @@ staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethodsSupport createSimil staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethodsSupport createSimilarCollection java.util.Collection int staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethodsSupport createSimilarMap java.util.Map staticMethod org.codehaus.groovy.runtime.InvokerHelper asIterator java.lang.Object -staticMethod org.codehaus.groovy.runtime.ScriptBytecodeAdapter bitwiseNegate java.lang.Object staticMethod org.codehaus.groovy.runtime.ScriptBytecodeAdapter compareEqual java.lang.Object java.lang.Object staticMethod org.codehaus.groovy.runtime.ScriptBytecodeAdapter compareGreaterThan java.lang.Object java.lang.Object staticMethod org.codehaus.groovy.runtime.ScriptBytecodeAdapter compareGreaterThanEqual java.lang.Object java.lang.Object @@ -1258,10 +1303,13 @@ staticMethod org.codehaus.groovy.runtime.ScriptBytecodeAdapter compareLessThanEq staticMethod org.codehaus.groovy.runtime.ScriptBytecodeAdapter compareNotEqual java.lang.Object java.lang.Object staticMethod org.codehaus.groovy.runtime.ScriptBytecodeAdapter compareNotIdentical java.lang.Object java.lang.Object staticMethod org.codehaus.groovy.runtime.ScriptBytecodeAdapter compareTo java.lang.Object java.lang.Object -staticMethod org.codehaus.groovy.runtime.ScriptBytecodeAdapter createRange java.lang.Object java.lang.Object boolean staticMethod org.codehaus.groovy.runtime.ScriptBytecodeAdapter findRegex java.lang.Object java.lang.Object staticMethod org.codehaus.groovy.runtime.ScriptBytecodeAdapter isCase java.lang.Object java.lang.Object staticMethod org.codehaus.groovy.runtime.ScriptBytecodeAdapter matchRegex java.lang.Object java.lang.Object +staticMethod org.codehaus.groovy.runtime.StringGroovyMethods asBoolean java.lang.CharSequence +staticMethod org.codehaus.groovy.runtime.StringGroovyMethods asBoolean java.util.regex.Matcher +staticMethod org.codehaus.groovy.runtime.StringGroovyMethods bitwiseNegate java.lang.CharSequence +staticMethod org.codehaus.groovy.runtime.StringGroovyMethods bitwiseNegate java.lang.String staticMethod org.codehaus.groovy.runtime.StringGroovyMethods capitalize java.lang.CharSequence staticMethod org.codehaus.groovy.runtime.StringGroovyMethods capitalize java.lang.String staticMethod org.codehaus.groovy.runtime.StringGroovyMethods center java.lang.CharSequence java.lang.Number @@ -1311,6 +1359,8 @@ staticMethod org.codehaus.groovy.runtime.StringGroovyMethods minus java.lang.Str staticMethod org.codehaus.groovy.runtime.StringGroovyMethods minus java.lang.String java.util.regex.Pattern staticMethod org.codehaus.groovy.runtime.StringGroovyMethods multiply java.lang.CharSequence java.lang.Number staticMethod org.codehaus.groovy.runtime.StringGroovyMethods multiply java.lang.String java.lang.Number +staticMethod org.codehaus.groovy.runtime.StringGroovyMethods next java.lang.CharSequence +staticMethod org.codehaus.groovy.runtime.StringGroovyMethods next java.lang.String staticMethod org.codehaus.groovy.runtime.StringGroovyMethods normalize java.lang.CharSequence staticMethod org.codehaus.groovy.runtime.StringGroovyMethods normalize java.lang.String staticMethod org.codehaus.groovy.runtime.StringGroovyMethods padLeft java.lang.CharSequence java.lang.Number @@ -1321,12 +1371,16 @@ staticMethod org.codehaus.groovy.runtime.StringGroovyMethods padRight java.lang. staticMethod org.codehaus.groovy.runtime.StringGroovyMethods padRight java.lang.CharSequence java.lang.Number java.lang.CharSequence staticMethod org.codehaus.groovy.runtime.StringGroovyMethods padRight java.lang.String java.lang.Number staticMethod org.codehaus.groovy.runtime.StringGroovyMethods padRight java.lang.String java.lang.Number java.lang.String +staticMethod org.codehaus.groovy.runtime.StringGroovyMethods previous java.lang.CharSequence +staticMethod org.codehaus.groovy.runtime.StringGroovyMethods previous java.lang.String staticMethod org.codehaus.groovy.runtime.StringGroovyMethods readLines java.lang.CharSequence staticMethod org.codehaus.groovy.runtime.StringGroovyMethods readLines java.lang.String staticMethod org.codehaus.groovy.runtime.StringGroovyMethods replaceAll java.lang.CharSequence java.lang.CharSequence groovy.lang.Closure staticMethod org.codehaus.groovy.runtime.StringGroovyMethods replaceAll java.lang.CharSequence java.util.regex.Pattern groovy.lang.Closure +staticMethod org.codehaus.groovy.runtime.StringGroovyMethods replaceAll java.lang.CharSequence java.util.regex.Pattern java.lang.CharSequence staticMethod org.codehaus.groovy.runtime.StringGroovyMethods replaceAll java.lang.String java.lang.String groovy.lang.Closure staticMethod org.codehaus.groovy.runtime.StringGroovyMethods replaceAll java.lang.String java.util.regex.Pattern groovy.lang.Closure +staticMethod org.codehaus.groovy.runtime.StringGroovyMethods replaceAll java.lang.String java.util.regex.Pattern java.lang.String staticMethod org.codehaus.groovy.runtime.StringGroovyMethods replaceFirst java.lang.CharSequence java.lang.CharSequence groovy.lang.Closure staticMethod org.codehaus.groovy.runtime.StringGroovyMethods replaceFirst java.lang.CharSequence java.util.regex.Pattern groovy.lang.Closure staticMethod org.codehaus.groovy.runtime.StringGroovyMethods replaceFirst java.lang.String java.lang.String groovy.lang.Closure @@ -1345,5 +1399,8 @@ staticMethod org.codehaus.groovy.runtime.StringGroovyMethods unexpand java.lang. staticMethod org.codehaus.groovy.runtime.StringGroovyMethods unexpand java.lang.CharSequence int staticMethod org.codehaus.groovy.runtime.StringGroovyMethods unexpandLine java.lang.CharSequence int -# Currently called indirectly from Builder.sandboxCast; cf. comment in SandboxInvokerTest.typeCoercion: +# Needed for normal Pipeline script instantiation. TODO: Add @Whitelisted annotation to the constructor and remove this entry. +new org.jenkinsci.plugins.workflow.cps.CpsScript + +# No longer needed except for Pipelines serialized before groovy-cps changes for SECURITY-2824, so wait for most users to update and then remove it. staticMethod org.kohsuke.groovy.sandbox.impl.Checker checkedCast java.lang.Class java.lang.Object boolean boolean boolean diff --git a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/jenkins-whitelist b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/jenkins-whitelist index 5d9897c6b..5e8ce3fb0 100644 --- a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/jenkins-whitelist +++ b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/jenkins-whitelist @@ -28,11 +28,12 @@ method hudson.scm.ChangeLogSet$Entry getMsg method hudson.scm.ChangeLogSet$Entry getMsgAnnotated method hudson.scm.ChangeLogSet$Entry getMsgEscaped method hudson.scm.ChangeLogSet$Entry getTimestamp -field hudson.scm.EditType ADD -field hudson.scm.EditType DELETE -field hudson.scm.EditType EDIT +staticField hudson.scm.EditType ADD +staticField hudson.scm.EditType DELETE +staticField hudson.scm.EditType EDIT method hudson.scm.EditType getName method hudson.tools.ToolInstallation getHome method hudson.tools.ToolInstallation getName method jenkins.model.CauseOfInterruption getShortDescription method jenkins.model.CauseOfInterruption$UserInterruption getUserId +staticField jenkins.model.Jenkins VERSION diff --git a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/Messages.properties b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/Messages.properties index 522fb4e9e..2d79849a4 100644 --- a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/Messages.properties +++ b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/Messages.properties @@ -2,3 +2,7 @@ ClasspathEntry.path.notExists=Specified path does not exist ClasspathEntry.path.notApproved=This classpath entry is not approved. Require an approval before execution. ClasspathEntry.path.noDirsAllowed=Class directories are not allowed as classpath entries. ScriptApprovalNote.message=Administrators can decide whether to approve or reject this signature. +ScriptApprovalLink.outstandingScript={0} scripts pending approval +ScriptApprovalLink.outstandingSignature={0} signatures pending approval +ScriptApprovalLink.outstandingClasspath={0} classpath entries pending approval +ScriptApprovalLink.dangerous={0} approved dangerous signatures \ No newline at end of file diff --git a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval/FormValidationPageDecorator/header.jelly b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval/FormValidationPageDecorator/header.jelly new file mode 100644 index 000000000..f9cca940b --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval/FormValidationPageDecorator/header.jelly @@ -0,0 +1,4 @@ + + + + \ No newline at end of file diff --git a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval/FormValidationPageDecorator/validate.js b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval/FormValidationPageDecorator/validate.js new file mode 100644 index 000000000..b5bf0698b --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval/FormValidationPageDecorator/validate.js @@ -0,0 +1,15 @@ +Behaviour.specify('.script-approval-approve-link', 'ScriptApproval.FormValidationPageDecorator', 0, function (element) { + element.onclick = function (ev) { + const approvalUrl = ev.target.dataset.baseUrl; + const hash = ev.target.dataset.hash; + const xmlhttp = new XMLHttpRequest(); + xmlhttp.onload = function () { + alert('Script approved'); + }; + xmlhttp.open('POST', approvalUrl + "/approveScriptHash"); + const data = new FormData(); + data.append('hash', hash); + xmlhttp.setRequestHeader(crumb.fieldName, crumb.value); + xmlhttp.send(data); + } +}); \ No newline at end of file diff --git a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval/index.jelly b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval/index.jelly index 0b0cac0bf..cb2ca82a3 100644 --- a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval/index.jelly +++ b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval/index.jelly @@ -30,7 +30,7 @@ THE SOFTWARE. + + + + + +

You can also remove all previous classpath entry approvals: diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyCallSiteSelectorTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyCallSiteSelectorTest.java index 402b89745..344c7153e 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyCallSiteSelectorTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyCallSiteSelectorTest.java @@ -24,9 +24,7 @@ package org.jenkinsci.plugins.scriptsecurity.sandbox.groovy; -import groovy.lang.Binding; import groovy.lang.GString; -import groovy.lang.Script; import hudson.EnvVars; import hudson.model.BooleanParameterValue; import hudson.model.Hudson; @@ -42,11 +40,9 @@ import hudson.model.ParametersAction; import hudson.model.StringParameterValue; import jenkins.model.Jenkins; -import org.apache.commons.io.output.NullOutputStream; import org.codehaus.groovy.runtime.GStringImpl; import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.EnumeratingWhitelist; import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.EnumeratingWhitelistTest; -import org.jenkinsci.plugins.scriptsecurity.scripts.ApprovalContext; import static org.junit.Assert.*; import org.junit.Test; import org.jvnet.hudson.test.Issue; @@ -56,7 +52,7 @@ public class GroovyCallSiteSelectorTest { @Test public void arrays() throws Exception { Method m = EnumeratingWhitelistTest.C.class.getDeclaredMethod("m", Object[].class); assertEquals("literal call", m, GroovyCallSiteSelector.method(new EnumeratingWhitelistTest.C(), "m", new Object[] {new Object[] {"a", "b"}})); - assertEquals("we assume the interceptor has dealt with varargs", null, GroovyCallSiteSelector.method(new EnumeratingWhitelistTest.C(), "m", new Object[] {"a", "b"})); + assertNull("we assume the interceptor has dealt with varargs", GroovyCallSiteSelector.method(new EnumeratingWhitelistTest.C(), "m", new Object[]{"a", "b"})); assertEquals("array cast", m, GroovyCallSiteSelector.method(new EnumeratingWhitelistTest.C(), "m", new Object[] {new String[] {"a", "b"}})); } @@ -98,13 +94,6 @@ public static void m2(long x) {} assertEquals(Hudson.class.getMethod("getInstance"), GroovyCallSiteSelector.staticMethod(Hudson.class, "getInstance", new Object[0])); } - @Issue("JENKINS-38908") - @Test public void main() throws Exception { - Script receiver = (Script) new SecureGroovyScript("def main() {}; this", true, null).configuring(ApprovalContext.create()).evaluate(GroovyCallSiteSelectorTest.class.getClassLoader(), new Binding(), null); - assertEquals(receiver.getClass().getMethod("main"), GroovyCallSiteSelector.method(receiver, "main", new Object[0])); - assertEquals(receiver.getClass().getMethod("main", String[].class), GroovyCallSiteSelector.method(receiver, "main", new Object[] {"somearg"})); - } - @Issue("JENKINS-45117") @Test public void constructorVarargs() throws Exception { assertEquals(EnvVars.class.getConstructor(), GroovyCallSiteSelector.constructor(EnvVars.class, new Object[0])); diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptorTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptorTest.java index 281fccbbd..5cfb15d3f 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptorTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptorTest.java @@ -24,6 +24,7 @@ package org.jenkinsci.plugins.scriptsecurity.sandbox.groovy; +import edu.umd.cs.findbugs.annotations.NonNull; import groovy.json.JsonBuilder; import groovy.json.JsonDelegate; import groovy.lang.GString; @@ -40,6 +41,8 @@ import groovy.text.Template; import groovy.transform.ASTTest; import hudson.Functions; +import java.io.File; +import java.io.IOException; import java.lang.reflect.Constructor; import java.lang.reflect.Field; @@ -52,10 +55,10 @@ import java.util.Collections; import java.util.Date; import java.util.HashMap; +import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Properties; -import java.util.concurrent.Callable; import java.util.regex.Pattern; import org.apache.commons.io.IOUtils; @@ -68,12 +71,10 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.*; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException; import org.jenkinsci.plugins.scriptsecurity.sandbox.Whitelist; @@ -137,7 +138,7 @@ public class SandboxInterceptorTest { assertEvaluate(new ProxyWhitelist( new AbstractWhitelist() { @Override - public boolean permitsMethod(Method method, Object receiver, Object[] args) { + public boolean permitsMethod(@NonNull Method method, @NonNull Object receiver, @NonNull Object[] args) { if (method.getName().equals("invokeMethod") && receiver instanceof JsonBuilder) return true; if (method.getName().equals("invokeMethod") && receiver instanceof JsonDelegate) @@ -199,14 +200,14 @@ public boolean permitsMethod(Method method, Object receiver, Object[] args) { assertRejected(new ProxyWhitelist(), "staticMethod jenkins.model.Jenkins getInstance", "class X {Object x = jenkins.model.Jenkins.instance}; new X().x"); assertRejected(new ProxyWhitelist(), "staticMethod jenkins.model.Jenkins getInstance", "class X {Object x; {x = jenkins.model.Jenkins.instance}}; new X().x"); try { - errors.checkThat(evaluate(new ProxyWhitelist(), "class X {static Object x = jenkins.model.Jenkins.instance}; X.x"), is((Object) "should be rejected")); + errors.checkThat(evaluate(new ProxyWhitelist(), "class X {static Object x = jenkins.model.Jenkins.instance}; X.x"), is("should be rejected")); } catch (ExceptionInInitializerError x) { errors.checkThat(x.getMessage(), ((RejectedAccessException) x.getCause()).getSignature(), is("staticMethod jenkins.model.Jenkins getInstance")); } catch (Throwable t) { errors.addError(t); } try { - errors.checkThat(evaluate(new ProxyWhitelist(), "class X {static Object x; static {x = jenkins.model.Jenkins.instance}}; X.x"), is((Object) "should be rejected")); + errors.checkThat(evaluate(new ProxyWhitelist(), "class X {static Object x; static {x = jenkins.model.Jenkins.instance}}; X.x"), is("should be rejected")); } catch (ExceptionInInitializerError x) { errors.checkThat(x.getMessage(), ((RejectedAccessException) x.getCause()).getSignature(), is("staticMethod jenkins.model.Jenkins getInstance")); } catch (Throwable t) { @@ -235,14 +236,12 @@ public boolean permitsMethod(Method method, Object receiver, Object[] args) { assertRejected(new StaticWhitelist("new " + clazz), "method " + clazz + " isProp3", "new " + clazz + "().prop3"); assertEvaluate(new StaticWhitelist("staticMethod " + clazz + " isProp4"), true, clazz + ".prop4"); assertRejected(new StaticWhitelist(), "staticMethod " + clazz + " isProp4", clazz + ".prop4"); - final RejectedAccessException x = assertThrows(RejectedAccessException.class, + final MissingPropertyException x = assertThrows(MissingPropertyException.class, () -> evaluate(new StaticWhitelist("new " + clazz), "new " + clazz + "().nonexistent")); - assertNull(x.getSignature()); assertEquals("No such field found: field " + clazz + " nonexistent", x.getMessage()); - final RejectedAccessException x2 = assertThrows(RejectedAccessException.class, + final MissingPropertyException x2 = assertThrows(MissingPropertyException.class, () -> evaluate(new StaticWhitelist("new " + clazz), "new " + clazz + "().nonexistent = 'edited'")); - assertNull(x.getSignature()); assertEquals("No such field found: field " + clazz + " nonexistent", x2.getMessage()); assertRejected(new StaticWhitelist("new " + clazz), "method " + clazz + " getProp5", "new " + clazz + "().prop5"); @@ -279,7 +278,7 @@ public static final class Clazz { } } private String quoteSingle(Object o) { - return "'" + String.valueOf(o) + "'"; + return "'" + o + "'"; } @Whitelisted static long incr(long x) { return x + 1; @@ -388,6 +387,20 @@ public static final class Special { @Issue("JENKINS-34741") @Test public void structConstructor() throws Exception { assertEvaluate(new StaticWhitelist(), "ok", "class C {String f}; new C(f: 'ok').f"); + // Map literals are equivalent to the named argument syntax. + assertEvaluate(new StaticWhitelist(), "ok", "class C {String f}; new C([f: 'ok']).f"); + // The map does not have to be a literal. + assertEvaluate(new StaticWhitelist(), "ok", "class C {String f}; def map = [f: 'ok']; new C(map).f"); + // Make sure that we do not assign properties more than once. + assertEvaluate(new StaticWhitelist(), 2, + "class Global { static int x = 0 }\n" + + "class C { def y; def setY(def y) { Global.x += y } }\n" + + "new C(y: 2); Global.x"); + // Make sure that we do not instantiate the object more than once. + assertEvaluate(new StaticWhitelist(), 1, + "class Global { static int x = 0 }\n" + + "class C { def y; C() { Global.x += 1 } }\n" + + "new C(y: 2); Global.x"); } @Test public void defSyntax() throws Exception { @@ -569,7 +582,10 @@ public String toString() { @Test public void templates() throws Exception { final GroovyShell shell = new GroovyShell(GroovySandbox.createSecureCompilerConfiguration()); - final Template t = new SimpleTemplateEngine(shell).createTemplate("goodbye <%= aspect.toLowerCase() %> world"); + final Template t; + try (GroovySandbox.Scope scope = new GroovySandbox().withWhitelist(new GenericWhitelist()).enter()) { + t = new SimpleTemplateEngine(shell).createTemplate("goodbye <%= aspect.toLowerCase() %> world"); + } assertEquals("goodbye cruel world", GroovySandbox.runInSandbox(() -> t.make(new HashMap(Collections.singletonMap("aspect", "CRUEL"))).toString(), new ProxyWhitelist(new StaticWhitelist("method java.lang.String toLowerCase"), new GenericWhitelist()))); @@ -590,10 +606,10 @@ public String toString() { cc.setScriptBaseClass(SpecialScript.class.getName()); GroovyShell shell = new GroovyShell(cc); Whitelist wl = new AbstractWhitelist() { - @Override public boolean permitsMethod(Method method, Object receiver, Object[] args) { + @Override public boolean permitsMethod(@NonNull Method method, @NonNull Object receiver, @NonNull Object[] args) { return method.getDeclaringClass() == GroovyObject.class && method.getName().equals("getProperty") && receiver instanceof SpecialScript && args[0].equals("magic"); } - @Override public boolean permitsConstructor(Constructor constructor, Object[] args) { + @Override public boolean permitsConstructor(@NonNull Constructor constructor, @NonNull Object[] args) { return constructor.getDeclaringClass() == SpecialScript.class; } }; @@ -641,24 +657,24 @@ public static abstract class SpecialScript extends Script { @Issue("SECURITY-580") @Test public void positionalConstructors() throws Exception { - assertRejected(new ProxyWhitelist(), "new java.lang.Boolean java.lang.String", "['true'] as Boolean"); - assertEvaluate(new StaticWhitelist("new java.lang.Boolean java.lang.String"), true, "['true'] as Boolean"); - String cc = "staticMethod org.kohsuke.groovy.sandbox.impl.Checker checkedCast java.lang.Class java.lang.Object boolean boolean boolean"; - assertRejected(new StaticWhitelist(cc), "new java.lang.Boolean java.lang.String", "Boolean x = ['true']; x"); - assertEvaluate(new StaticWhitelist(cc, "new java.lang.Boolean java.lang.String"), true, "Boolean x = ['true']; x"); + assertRejected(new ProxyWhitelist(), "new java.lang.Exception java.lang.String", "['true'] as Exception"); + assertEvaluate(new StaticWhitelist("new java.lang.Exception java.lang.String", "method java.lang.Throwable getMessage"), + "true", "(['true'] as Exception).getMessage()"); + assertRejected(new ProxyWhitelist(), "new java.lang.Exception java.lang.String", "Exception x = ['true']; x.getMessage()"); + assertEvaluate(new StaticWhitelist("new java.lang.Exception java.lang.String", "method java.lang.Throwable getMessage"), + "true", "Exception x = ['true']; x.getMessage()"); assertRejected(new ProxyWhitelist(), "new java.util.TreeMap java.util.Map", "[k: 1] as TreeMap"); assertEvaluate(new StaticWhitelist("new java.util.TreeMap java.util.Map"), Collections.singletonMap("k", 1), "[k: 1] as TreeMap"); - assertRejected(new StaticWhitelist(cc), "new java.util.TreeMap java.util.Map", "TreeMap x = [k: 1]; x"); - assertEvaluate(new StaticWhitelist(cc, "new java.util.TreeMap java.util.Map"), Collections.singletonMap("k", 1), "TreeMap x = [k: 1]; x"); + assertRejected(new ProxyWhitelist(), "new java.util.TreeMap java.util.Map", "TreeMap x = [k: 1]; x"); + assertEvaluate(new StaticWhitelist("new java.util.TreeMap java.util.Map"), Collections.singletonMap("k", 1), "TreeMap x = [k: 1]; x"); // These go through a different code path: - assertEvaluate(new ProxyWhitelist(), Arrays.asList(1), "[1] as LinkedList"); - assertEvaluate(new ProxyWhitelist(), Arrays.asList("v"), "['v'] as LinkedList"); - assertEvaluate(new StaticWhitelist(cc), Arrays.asList(1), "LinkedList x = [1]; x"); - assertEvaluate(new StaticWhitelist(cc), Arrays.asList("v"), "LinkedList x = ['v']; x"); - assertEvaluate(new ProxyWhitelist(), Arrays.asList(1), "int[] a = [1]; a as LinkedList"); - assertEvaluate(new ProxyWhitelist(), Arrays.asList("v"), "String[] a = ['v']; a as LinkedList"); - assertEvaluate(new StaticWhitelist(cc), Arrays.asList("v"), "String[] a = ['v']; LinkedList x = a; x"); - assertEvaluate(new StaticWhitelist(cc), Arrays.asList("v"), "String[] a = ['v']; LinkedList x = a; x"); + assertEvaluate(new ProxyWhitelist(), Collections.singletonList(1), "[1] as LinkedList"); + assertEvaluate(new ProxyWhitelist(), Collections.singletonList("v"), "['v'] as LinkedList"); + assertEvaluate(new ProxyWhitelist(), Collections.singletonList(1), "LinkedList x = [1]; x"); + assertEvaluate(new ProxyWhitelist(), Collections.singletonList("v"), "LinkedList x = ['v']; x"); + assertEvaluate(new ProxyWhitelist(), Collections.singletonList(1), "int[] a = [1]; a as LinkedList"); + assertEvaluate(new ProxyWhitelist(), Collections.singletonList("v"), "String[] a = ['v']; a as LinkedList"); + assertEvaluate(new ProxyWhitelist(), Collections.singletonList("v"), "String[] a = ['v']; LinkedList x = a; x"); /* TODO casting arrays is not yet supported: assertRejected(new StaticWhitelist(cc), "new java.lang.Boolean java.lang.String", "String[] a = ['true']; Boolean x = a; x"); assertEvaluate(new StaticWhitelist(cc, "new java.lang.Boolean java.lang.String"), true, "String[] a = ['true']; Boolean x = a; x"); @@ -673,7 +689,7 @@ public static abstract class SpecialScript extends Script { @Issue("kohsuke/groovy-sandbox #16") @Test public void infiniteLoop() throws Exception { - assertEvaluate(new BlanketWhitelist(), "abc", "def split = 'a b c'.split(' '); def b = new StringBuilder(); for (i = 0; i < split.length; i++) {println(i); b.append(split[i])}; b.toString()"); + assertEvaluate(new BlanketWhitelist(), "abc", "def split = 'a b c'.split(' '); def b = new StringBuilder(); for (i = 0; i < split.length; i++) {b.append(split[i])}; b.toString()"); } @Issue("JENKINS-25118") @@ -806,6 +822,9 @@ public void setSecure(boolean x) {} assertRejected(wl, "method " + e + " explode", e + ".TWO.explode()"); assertEvaluate(wl, "TWO", e + ".TWO.name()"); assertRejected(wl, "staticField " + e + " ONE", e + ".ONE.name()"); + // Seems undesirable, but this is the current behavior. Requires new java.util.LinkedHashMap and staticMethod ImmutableASTTransformation checkPropNames. + errors.checkThrows(ExceptionInInitializerError.class, () -> evaluate(new GenericWhitelist(), + "enum Thing { ONE, TWO }; Thing.ONE.toString()")); } public enum E { ONE(1), @@ -866,7 +885,14 @@ private static Object evaluate(Whitelist whitelist, String script) { } catch (IllegalAccessException | NoSuchFieldException e) { throw new AssertionError("Groovy class loader fields have changed", e); } - Object actual = new GroovySandbox().withWhitelist(whitelist).runScript(shell, script); + // Not all tests use GenericWhitelist, so we always force allow `new Script(Binding)`. + Whitelist baseWhitelist; + try { + baseWhitelist = new ProxyWhitelist(whitelist, new StaticWhitelist("new groovy.lang.Script groovy.lang.Binding")); + } catch (IOException e) { + throw new AssertionError(e); + } + Object actual = new GroovySandbox().withWhitelist(baseWhitelist).runScript(shell, script); if (actual instanceof GString) { actual = actual.toString(); // for ease of comparison } @@ -903,6 +929,15 @@ public static void assertRejected(Whitelist whitelist, String expectedSignature, try { Object actual = evaluate(whitelist, script); errors.checkThat(actual, is((Object) "should be rejected")); + } catch (GroovyRuntimeException x) { + // Exceptions during script parsing and instantiation are typically wrapped in GroovyRuntimeException, and + // we cannot modify our code to directly throw the cause without breaking other plugins that only catch + // GroovyRuntimeException. + if (x.getCause() instanceof RejectedAccessException) { + errors.checkThat(x.getMessage(), ((RejectedAccessException)x.getCause()).getSignature(), is(expectedSignature)); + } else { + errors.addError(x); + } } catch (RejectedAccessException x) { errors.checkThat(x.getMessage(), x.getSignature(), is(expectedSignature)); } catch (Throwable t) { @@ -1282,7 +1317,7 @@ public void blockIllegalAnnotationsInAnnotations() throws Exception { @Issue("SECURITY-1754") @Test public void blockDirectCallsToSyntheticConstructors() throws Exception { // Not ok, the call to super() in the synthetic constructor for Subclass cannot be intercepted. - final SecurityException e = assertThrows(SecurityException.class, + SecurityException e = assertThrows(SecurityException.class, () -> evaluate(new GenericWhitelist(), "class Superclass { }\n" + "class Subclass extends Superclass { }\n" + @@ -1290,6 +1325,22 @@ public void blockIllegalAnnotationsInAnnotations() throws Exception { assertThat(e.getMessage(), equalTo( "Rejecting illegal call to synthetic constructor: private Subclass(org.kohsuke.groovy.sandbox.impl.Checker$SuperConstructorWrapper). " + "Perhaps you meant to use one of these constructors instead: public Subclass()")); + e = assertThrows(SecurityException.class, () -> evaluate(new GenericWhitelist(), + "class Superclass { Superclass(String x) { } }\n" + + "class Subclass extends Superclass {\n" + + " def wrapper\n" + + " Subclass() { super('secret.key'); def $cw = $cw; wrapper = $cw }\n" + + "}\n" + + "def wrapper = new Subclass().wrapper\n" + + "class MyFile extends File {\n" + + " MyFile(String path) {\n" + + " super(path)\n" + + " }\n" + + "}\n" + + "new MyFile(wrapper, 'unused')")); + assertThat(e.getMessage(), equalTo( + "Rejecting illegal call to synthetic constructor: private MyFile(org.kohsuke.groovy.sandbox.impl.Checker$SuperConstructorWrapper,java.lang.String). " + + "Perhaps you meant to use one of these constructors instead: public MyFile(java.lang.String)")); } @Issue("SECURITY-1754") @@ -1352,6 +1403,16 @@ public void blockIllegalAnnotationsInAnnotations() throws Exception { "new Subclass().class.simpleName"); } + @Test public void blockCallsToSyntheticConstructorsViaCasts() throws Exception { + // Not ok, the call to super() in the synthetic constructor for Subclass cannot be intercepted. + assertRejected(new GenericWhitelist(), "new Subclass org.kohsuke.groovy.sandbox.impl.Checker$SuperConstructorWrapper", + "class Superclass { }\n" + + "class Subclass extends Superclass {\n" + + " Subclass() { }\n" + + "}\n" + + "Subclass x = [null]"); + } + @Issue("SECURITY-1754") @Test public void groovyInterceptable() throws Throwable { assertRejected(new GenericWhitelist(), "method groovy.lang.GroovyObject invokeMethod java.lang.String java.lang.Object", @@ -1381,7 +1442,7 @@ public void blockIllegalAnnotationsInAnnotations() throws Exception { "}\n" + "new Test()"); // Test.equals and Test.getClass are inherited and not sandbox-transformed, so they can be called outside of the sandbox. - assertFalse(result.equals(new Object())); + assertNotEquals(result, new Object()); assertThat(result.getClass().getSimpleName(), equalTo("Test")); // Test.toString is defined in the sandbox, so it cannot be called outside of the sandbox. result.toString(); @@ -1389,6 +1450,317 @@ public void blockIllegalAnnotationsInAnnotations() throws Exception { assertThat(e.getMessage(), equalTo("Rejecting unsandboxed static method call: jenkins.model.Jenkins.get()")); } + @Issue("SECURITY-2824") + @Test public void blockUnsafeImplicitCastsPropertiesAndAttributes() throws Throwable { + // Instance properties + assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", + "class Test { File file }\n" + + "def t = new Test()\n" + + "t.file = ['secret.key']\n"); + // Static properties + assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", + "class Test { static File file }\n" + + "Test.file = ['secret.key']\n"); + // Instance attributes + assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", + "class Test { File file }\n" + + "def t = new Test()\n" + + "t.@file = ['secret.key']\n"); + // Static attributes + assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", + "class Test { static File file }\n" + + "Test.@file = ['secret.key']\n"); + } + + @Issue("SECURITY-2824") + @Test public void blockUnsafeImplicitCastsPropertySetterParameters() throws Throwable { + assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", + "class Test {\n" + + " Object file\n" + + " def setFile(File file) {\n" + + " this.file = file\n" + + " }\n" + + "}\n" + + "def t = new Test()\n" + + "t.file = ['secret.key']\n " + + "t.file.class"); + } + + @Issue("SECURITY-2824") + @Test public void blockUnsafeImplicitCastsPropertySettersWithUnusualCapitalization() throws Throwable { + assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", + "class Test {\n" + + " String aProp\n" + + " def setAProp(List prop) {\n" + + " this.aProp = 'unused'\n" + + " }\n" + + " def setaProp(File prop) {\n" + + " this.aProp = /got a ${prop.class}/\n" + + " }\n" + + "}\n" + + "def t = new Test()\n" + + "t.aProp = ['secret.key']\n " + + "t.aProp"); + } + + @Issue("SECURITY-2824") + @Test public void propertiesWithMultipleSetters() throws Throwable { + // If you have multiple setters but one of them matches the argument type exactly, that is the one that gets used. + assertEvaluate(new GenericWhitelist(), "list overload", + "class Test {\n" + + " Object prop\n" + + " def setProp(File prop) {\n" + + " this.prop = 'file overload'\n" + + " }\n" + + " def setProp(String prop) {\n" + + " this.prop = 'string overload'\n" + + " }\n" + + " def setProp(List prop) {\n" + + " this.prop = 'list overload'\n" + + " }\n" + + "}\n" + + "def t = new Test()\n" + + "t.prop = ['secret.key']\n " + + "t.prop"); + // In this case, somewhere in the bowels of ScriptBytecodeAdapter.setProperty / MetaClass.invokeMethod + // the list is converted to an array which is then used directly as the arguments array when invoking the setter, + // so the String overload is chosen. SandboxInterceptor doesn't understand this case and intercepts the field... + assertEvaluate(new GenericWhitelist(), "string overload", + "class Test {\n" + + " Object prop\n" + + " def setProp(File prop) {\n" + + " this.prop = 'file overload'\n" + + " }\n" + + " def setProp(String prop) {\n" + + " this.prop = 'string overload'\n" + + " }\n" + + "}\n" + + "def t = new Test()\n" + + "t.prop = ['secret.key']\n" + + "t.prop"); + // ... so if the field name does not match the setter names, SandboxInterceptor will reject it. + errors.checkThrows(MissingPropertyException.class, () -> evaluate(new GenericWhitelist(), + "class Test {\n" + + " Object prop2\n" + + " def setProp(File prop) {\n" + + " this.prop2 = prop\n" + + " }\n" + + " def setProp(String prop) {\n" + + " this.prop2 = prop\n" + + " }\n" + + " def getProp() { this.prop2 }\n" + + "}\n" + + "def t = new Test()\n" + + "t.prop = ['secret.key']\n" + + "t.prop")); + // If none of the overloads match directly, and there is no overload that matches the list if it is converted + // to an array, then a MissingPropertyException gets thrown. + errors.checkThrows(MissingMethodException.class, () -> evaluate(new GenericWhitelist(), + "class Test {\n" + + " Object prop\n" + + " def setProp(File prop) {\n" + + " this.prop = prop\n" + + " }\n" + + " def setProp(Integer prop) {\n" + + " this.prop = prop\n" + + " }\n" + + "}\n" + + "def t = new Test()\n" + + "t.prop = ['secret.key']\n " + + "t.prop")); + // Methods with more than one parameter are not setters and must be ignored when intercepting property assignment. + assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String int", + "class Test {\n" + + " Object prop\n" + + " def setProp(File prop) {\n" + + " this.prop = 'file overload'\n" + + " }\n" + + " def setProp(String prop1, Integer prop2) {\n" + + " this.prop = 'multi-param overload'\n" + + " }\n" + + "}\n" + + "def t = new Test()\n" + + "t.prop = ['secret.key', 1]\n" + + "t.prop"); + // ... but if you have multiple setters as well as a method with the same name that has multiple parameters, + // then Groovy will invoke the multi-parameter method if it is the best match for the arguments. + assertEvaluate(new GenericWhitelist(), "multi-param overload", + "class Test {\n" + + " Object prop\n" + + " def setProp(File prop) {\n" + + " this.prop = 'file overload'\n" + + " }\n" + + " def setProp(Boolean prop) {\n" + + " this.prop = 'boolean overload'\n" + + " }\n" + + " def setProp(String prop1, Integer prop2) {\n" + + " this.prop = 'multi-param overload'\n" + + " }\n" + + "}\n" + + "def t = new Test()\n" + + "t.prop = ['secret.key', 1]\n" + + "t.prop"); + } + + @Issue("SECURITY-2824") + @Test public void blockUnsafeImplicitCastsMetaClassModification() throws Throwable { + assertRejected(new GenericWhitelist(), "method groovy.lang.GroovyObject getMetaClass", + "class Parent {\n" + + " public File prop\n" + + "}\n" + + "class Child extends Parent {\n" + + " Object prop // property\n" + + "}\n" + + "def p = new Parent()\n" + + "def c = new Child()\n" + + "c.metaClass = p.metaClass\n" + + "c.prop = ['secret.key']\n" + + "c.prop"); + } + + @Issue("SECURITY-2824") + @Test public void blockUnsafeImplicitCastsInitialParameterExpressions() throws Throwable { + assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", + "def method(File file = ['secret.key']) { file }; method()"); + assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", + "({ File file = ['secret.key'] -> file })()"); + assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", + "class Test {\n" + + " def x\n" + + " Test(File file = ['secret.key']) {\n" + + " x = file\n" + + " }\n" + + "}\n" + + "new Test().x"); + } + + @Issue("SECURITY-2824") + @Test public void blockUnsafeImplicitCastsFields() throws Throwable { + assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", + "class Test {\n" + + " File file = ['secret.key']\n" + + "}\n" + + "new Test().file"); + assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", + "@groovy.transform.Field File file = ['secret.key']\n" + + "file"); + } + + @Test public void blockSyntheticConstructorsFieldsAndMethods() throws Throwable { + assertRejected(new GenericWhitelist(), "method groovy.lang.GroovyObject getMetaClass", + "class Test {}; new Test().metaClass"); + assertRejected(new GenericWhitelist(), "field Test metaClass", + "class Test {}; new Test().@metaClass"); + assertRejected(new GenericWhitelist(), "staticMethod Script1 $getCallSiteArray", + "getClass().$getCallSiteArray()"); + assertRejected(new GenericWhitelist(), "method Script1 $getStaticMetaClass", + "$getStaticMetaClass()"); + errors.checkThrows(SecurityException.class, () -> evaluate(new GenericWhitelist(), + "class Parent {" + + " Parent(String value) { }" + + "}\n" + + "class Child extends Parent {\n" + + " def wrapper\n" + + " Child(String value) {\n" + + " super(value)\n" + + " def $cw = $cw\n" + + " wrapper = $cw\n" + + " }\n" + + "}\n" + + "def cw = new Child('secret.key').wrapper\n" + + "class MyFile extends File { public MyFile(String path) { super(path) } }\n" + + "def f = new MyFile(cw, 'unused')\n" + + "[f, f.class]")); + } + + @Test + public void booleanCasts() throws Throwable { + assertEvaluate(new GenericWhitelist(), null, "null as Boolean"); + assertEvaluate(new GenericWhitelist(), true, "true as Boolean"); + assertEvaluate(new GenericWhitelist(), false, "[:] as Boolean"); + assertEvaluate(new GenericWhitelist(), false, "[] as Boolean"); + assertEvaluate(new GenericWhitelist(), true, "([false] as boolean[]) as Boolean"); + assertEvaluate(new GenericWhitelist(), true, "['false'] as Boolean"); + assertEvaluate(new GenericWhitelist(), true, "class Test { }; new Test() as Boolean"); + assertEvaluate(new GenericWhitelist(), false, "class Test { }; new Test() { boolean asBoolean() { false } } as Boolean"); + assertEvaluate(new GenericWhitelist(), true, "('a' =~ '.*') as Boolean"); + } + + @Test + public void staticAttributesAreNotShadowedByClassFields() throws Throwable { + assertEvaluate(new GenericWhitelist(), "foo", "class MyClass { static String name = 'foo' }; MyClass.@name"); + assertEvaluate(new GenericWhitelist(), "foo", "class MyClass { static String name }; MyClass.@name = 'foo'"); + } + + @Issue("JENKINS-42214") + @Test + public void accessStaticMembersViaInstance() throws Throwable { + String fqcn = HasStaticMembers.class.getName(); + // Make sure that we report the correct signature in RejectedAccessException. + assertRejected(new AnnotatedWhitelist(), "staticField " + fqcn + " FOO", "def o = new " + fqcn + "(); o.FOO"); + assertRejected(new AnnotatedWhitelist(), "staticMethod " + fqcn + " getBAR", "def o = new " + fqcn + "(); o.BAR"); + assertRejected(new AnnotatedWhitelist(), "staticMethod " + fqcn + " isBAZ", "def o = new " + fqcn + "(); o.BAZ"); + assertRejected(new AnnotatedWhitelist(), "staticField " + fqcn + " FOO", "def o = new " + fqcn + "(); o.FOO = 1"); + assertRejected(new AnnotatedWhitelist(), "staticMethod " + fqcn + " setBAR int", "def o = new " + fqcn + "(); o.BAR = 2"); + assertRejected(new AnnotatedWhitelist(), "staticField " + fqcn + " FOO", "def o = new " + fqcn + "(); o.@FOO"); + assertRejected(new AnnotatedWhitelist(), "staticField " + fqcn + " FOO", "def o = new " + fqcn + "(); o.@FOO = 1"); + assertRejected(new AnnotatedWhitelist(), "staticMethod " + fqcn + " method", "def o = new " + fqcn + "(); o.method()"); + assertRejected(new AnnotatedWhitelist(), "staticMethod " + fqcn + " getAt int", "def o = new " + fqcn + "(); o[0]"); + assertRejected(new AnnotatedWhitelist(), "staticMethod " + fqcn + " putAt int java.lang.Object", "def o = new " + fqcn + "(); o[0] = null"); + // Make sure that we check for the correct signature in allowlists. + assertEvaluate(HasStaticMembers.allowlist("staticField " + fqcn + " FOO"), 1, + "def o = new " + fqcn + "(); o.FOO = o.FOO"); + assertEvaluate(HasStaticMembers.allowlist("staticField " + fqcn + " FOO"), 1, + "def o = new " + fqcn + "(); o.@FOO = o.@FOO"); + assertEvaluate(HasStaticMembers.allowlist("staticMethod " + fqcn + " getBAR", "staticMethod " + fqcn + " setBAR int"), 2, + "def o = new " + fqcn + "(); o.BAR = o.BAR"); + assertEvaluate(HasStaticMembers.allowlist("staticMethod " + fqcn + " isBAZ"), true, + "def o = new " + fqcn + "(); o.BAZ"); + assertEvaluate(HasStaticMembers.allowlist("staticMethod " + fqcn + " method"), 3, + "def o = new " + fqcn + "(); o.method()"); + assertEvaluate(HasStaticMembers.allowlist("staticMethod " + fqcn + " getAt int", "staticMethod " + fqcn + " putAt int java.lang.Object"), 3, + "def o = new " + fqcn + "(); o[0] = o[3]"); + } + + public static class HasStaticMembers { + @Whitelisted public HasStaticMembers() { } + public static int FOO = 1; // Groovy will access the field directly + public static int BAR = 2; // Groovy will access the field via getBAR and setBAR + public static int getBAR() { return BAR; } + public static void setBAR(int BAR) { HasStaticMembers.BAR = BAR; } + public static boolean BAZ = true; // Groovy will read the field via isBAZ + public static boolean isBAZ() { return BAZ; } + public static int method() { return 3; } + public static int getAt(int index) { return index; } + public static void putAt(int index, Object value) { } + public static Whitelist allowlist(String... signatures) throws Exception { + List signaturesList = new ArrayList<>(Arrays.asList(signatures)); + signaturesList.add("new " + HasStaticMembers.class.getName()); + return new StaticWhitelist(signaturesList); + } + } + + @Issue("SECURITY-3016") + @Test public void blockUnsafeCastsPropertyAssignmentViaImplicitMapConstructor() throws Throwable { + // Map constructors are supported when using new, but these property assignments are unsafe. + assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", + "class Test { File f }; new Test(f: ['secret.key'])"); + assertRejected(new GenericWhitelist(), "new java.io.File java.lang.String", + "class Test { File f; int x }; new Test(x: 1, f: ['secret.key'])"); + // Map constructors are not supported when casting, regardless of whether the property assignments are safe or not. + errors.checkThrows(UnsupportedOperationException.class, () -> evaluate(new GenericWhitelist(), + "class Test { File f }; Test t = [f: ['secret.key']]")); + errors.checkThrows(UnsupportedOperationException.class, () -> evaluate(new GenericWhitelist(), + "class Test { File f; int x }; Test t = [x: 1, f: ['secret.key']]")); + errors.checkThrows(UnsupportedOperationException.class, () -> evaluate(new GenericWhitelist(), + "class Test { File f }; [f: ['secret.key']] as Test")); + errors.checkThrows(UnsupportedOperationException.class, () -> evaluate(new GenericWhitelist(), + "class Test { File f }; (Test)[f: ['secret.key']]")); + // Map constructors are not currently supported when constructing inner classes. + errors.checkThrows(SecurityException.class, () -> evaluate(new GenericWhitelist(), + "class Outer { class Inner { File f }; def makeInner() { new Inner(f: ['secret.key']) } }; new Outer().makeInner().f")); + } + /** * Checks that the annotation is blocked from being used in the provided script whether it is imported or used via * fully-qualified class name. diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxResolvingClassLoaderTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxResolvingClassLoaderTest.java index 6436973d9..695d2a9ba 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxResolvingClassLoaderTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxResolvingClassLoaderTest.java @@ -34,7 +34,6 @@ import static org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SandboxResolvingClassLoader.CLASS_NOT_FOUND; import static org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SandboxResolvingClassLoader.parentClassCache; import static org.junit.Assert.assertThrows; -import static org.junit.Assert.fail; public class SandboxResolvingClassLoaderTest { diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java index 76dd85257..7849ce289 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java @@ -24,17 +24,20 @@ package org.jenkinsci.plugins.scriptsecurity.sandbox.groovy; -import com.gargoylesoftware.htmlunit.html.HtmlCheckBoxInput; -import com.gargoylesoftware.htmlunit.html.HtmlInput; +import org.htmlunit.CollectingAlertHandler; +import org.htmlunit.html.HtmlCheckBoxInput; +import org.htmlunit.html.HtmlInput; import groovy.lang.Binding; +import groovy.lang.Script; import hudson.remoting.Which; +import hudson.security.ACLContext; import org.apache.tools.ant.AntClassLoader; import org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException; import org.jenkinsci.plugins.scriptsecurity.scripts.ClasspathEntry; -import com.gargoylesoftware.htmlunit.html.HtmlForm; -import com.gargoylesoftware.htmlunit.html.HtmlFormUtil; -import com.gargoylesoftware.htmlunit.html.HtmlPage; -import com.gargoylesoftware.htmlunit.html.HtmlTextArea; +import org.htmlunit.html.HtmlForm; +import org.htmlunit.html.HtmlFormUtil; +import org.htmlunit.html.HtmlPage; +import org.htmlunit.html.HtmlTextArea; import hudson.model.FreeStyleProject; import hudson.model.FreeStyleBuild; import hudson.model.Item; @@ -44,14 +47,15 @@ import hudson.security.Permission; import hudson.tasks.BuildStepDescriptor; import hudson.tasks.Publisher; +import hudson.util.VersionNumber; import java.io.File; import java.io.IOException; import java.net.URISyntaxException; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Set; import jenkins.model.Jenkins; -import jenkins.security.NotReallyRoleSensitiveCallable; import org.apache.commons.io.FileUtils; import org.apache.commons.lang.StringUtils; import org.apache.tools.ant.DirectoryScanner; @@ -60,7 +64,14 @@ import org.jenkinsci.plugins.scriptsecurity.scripts.UnapprovedUsageException; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.arrayWithSize; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.emptyArray; +import static org.hamcrest.Matchers.is; +import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.Whitelisted; +import org.jenkinsci.plugins.scriptsecurity.scripts.ApprovalContext; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; @@ -68,23 +79,38 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import org.jenkinsci.plugins.scriptsecurity.scripts.languages.GroovyLanguage; +import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; +import org.jvnet.hudson.test.BuildWatcher; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.MockAuthorizationStrategy; import org.kohsuke.groovy.sandbox.impl.Checker; +import static org.junit.Assert.assertEquals; public class SecureGroovyScriptTest { + @ClassRule public static BuildWatcher WATCHER = new BuildWatcher(); + @Rule public JenkinsRule r = new JenkinsRule(); @Rule public TemporaryFolder tmpFolderRule = new TemporaryFolder(); + private void addPostBuildAction(HtmlPage page) throws IOException { + String displayName = r.jenkins.getExtensionList(BuildStepDescriptor.class).get(TestGroovyRecorder.DescriptorImpl.class).getDisplayName(); + if (Jenkins.getVersion().isOlderThan(new VersionNumber("2.422"))) { + page.getAnchorByText(displayName).click(); + } else { + HtmlForm config = page.getFormByName("config"); + r.getButtonByCaption(config, displayName).click(); + } + + } + /** * Basic approval test where the user doesn't have ADMINISTER privs but has unchecked * the sandbox checkbox. Should result in script going to pending approval. @@ -105,7 +131,7 @@ public class SecureGroovyScriptTest { HtmlPage page = wc.getPage(p, "configure"); HtmlForm config = page.getFormByName("config"); HtmlFormUtil.getButtonByCaption(config, "Add post-build action").click(); // lib/hudson/project/config-publishers2.jelly - page.getAnchorByText(r.jenkins.getExtensionList(BuildStepDescriptor.class).get(TestGroovyRecorder.DescriptorImpl.class).getDisplayName()).click(); + addPostBuildAction(page); wc.waitForBackgroundJavaScript(10000); List scripts = config.getTextAreasByName("_.script"); // Get the last one, because previous ones might be from Lockable Resources during PCT. @@ -118,7 +144,7 @@ public class SecureGroovyScriptTest { List sandboxes = config.getInputsByName("_.sandbox"); // Get the last one, because previous ones might be from Lockable Resources during PCT. HtmlCheckBoxInput sandboxRB = (HtmlCheckBoxInput) sandboxes.get(sandboxes.size() - 1); - assertEquals(true, sandboxRB.isChecked()); // should be checked + assertTrue(sandboxRB.isChecked()); // should be checked sandboxRB.setChecked(false); // uncheck sandbox mode => forcing script approval r.submit(config); @@ -158,7 +184,7 @@ public class SecureGroovyScriptTest { /** - * Test where the user has ADMINISTER privs, default to non sandbox mode. + * Test where the user has ADMINISTER privs, default to non sandbox mode, but require approval */ @Test public void testSandboxDefault_with_ADMINISTER_privs() throws Exception { r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); @@ -177,7 +203,7 @@ public class SecureGroovyScriptTest { HtmlPage page = wc.getPage(p, "configure"); HtmlForm config = page.getFormByName("config"); HtmlFormUtil.getButtonByCaption(config, "Add post-build action").click(); // lib/hudson/project/config-publishers2.jelly - page.getAnchorByText(r.jenkins.getExtensionList(BuildStepDescriptor.class).get(TestGroovyRecorder.DescriptorImpl.class).getDisplayName()).click(); + addPostBuildAction(page); wc.waitForBackgroundJavaScript(10000); List scripts = config.getTextAreasByName("_.script"); // Get the last one, because previous ones might be from Lockable Resources during PCT. @@ -193,12 +219,12 @@ public class SecureGroovyScriptTest { // The user has ADMINISTER privs => should default to non sandboxed assertFalse(publisher.getScript().isSandbox()); - // Because it has ADMINISTER privs, the script should not have ended up pending approval + // even though it has ADMINISTER privs, the script should still require approval Set pendingScripts = ScriptApproval.get().getPendingScripts(); - assertEquals(0, pendingScripts.size()); + assertEquals(1, pendingScripts.size()); // Test that the script is executable. If it's not, we will get an UnapprovedUsageException - assertEquals(groovy, ScriptApproval.get().using(groovy, GroovyLanguage.get(), "Testing")); + assertThrows(UnapprovedUsageException.class, () -> ScriptApproval.get().using(groovy, GroovyLanguage.get(), "Testing")); } /** @@ -220,7 +246,7 @@ public class SecureGroovyScriptTest { HtmlPage page = wc.getPage(p, "configure"); HtmlForm config = page.getFormByName("config"); HtmlFormUtil.getButtonByCaption(config, "Add post-build action").click(); // lib/hudson/project/config-publishers2.jelly - page.getAnchorByText(r.jenkins.getExtensionList(BuildStepDescriptor.class).get(TestGroovyRecorder.DescriptorImpl.class).getDisplayName()).click(); + addPostBuildAction(page); wc.waitForBackgroundJavaScript(10000); List scripts = config.getTextAreasByName("_.script"); // Get the last one, because previous ones might be from Lockable Resources during PCT. @@ -887,7 +913,7 @@ public void testScriptApproval() throws Exception { JenkinsRule.WebClient wc = r.createWebClient(); - // If configured by a user with ADMINISTER script is approved if edited by that user + // If configured by a user with ADMINISTER script is not approved and approval is requested { wc.login("admin"); HtmlForm config = wc.getPage(p, "configure").getFormByName("config"); @@ -898,8 +924,9 @@ public void testScriptApproval() throws Exception { script.setText(groovy); r.submit(config); - assertTrue(ScriptApproval.get().isScriptApproved(groovy, GroovyLanguage.get())); - + assertFalse(ScriptApproval.get().isScriptApproved(groovy, GroovyLanguage.get())); + assertEquals(1, ScriptApproval.get().getPendingScripts().size()); + // clean up for next tests ScriptApproval.get().preapproveAll(); ScriptApproval.get().clearApprovedScripts(); @@ -924,11 +951,14 @@ public void testScriptApproval() throws Exception { ScriptApproval.get().clearApprovedScripts(); } - // If configured by a user with ADMINISTER while escape hatch is on script is approved upon save + // If configured by a user with ADMINISTER while escape hatches are on script is approved upon save { wc.login("admin"); - boolean original = ScriptApproval.ADMIN_AUTO_APPROVAL_ENABLED; + boolean originalAdminAutoApprove = ScriptApproval.ADMIN_AUTO_APPROVAL_ENABLED; ScriptApproval.ADMIN_AUTO_APPROVAL_ENABLED = true; + boolean originalAllowAdminApproval = ScriptApproval.ALLOW_ADMIN_APPROVAL_ENABLED; + ScriptApproval.ALLOW_ADMIN_APPROVAL_ENABLED = true; + try { HtmlForm config = wc.getPage(p, "configure").getFormByName("config"); List scripts = config.getTextAreasByName("_.script"); @@ -943,15 +973,18 @@ public void testScriptApproval() throws Exception { ScriptApproval.get().preapproveAll(); ScriptApproval.get().clearApprovedScripts(); } finally { - ScriptApproval.ADMIN_AUTO_APPROVAL_ENABLED = original; + ScriptApproval.ADMIN_AUTO_APPROVAL_ENABLED = originalAdminAutoApprove; + ScriptApproval.ALLOW_ADMIN_APPROVAL_ENABLED = originalAllowAdminApproval; } } // If configured by a user without ADMINISTER while escape hatch is on script is not approved { wc.login("devel"); - boolean original = ScriptApproval.ADMIN_AUTO_APPROVAL_ENABLED; + boolean originalAdminAutoApprove = ScriptApproval.ADMIN_AUTO_APPROVAL_ENABLED; ScriptApproval.ADMIN_AUTO_APPROVAL_ENABLED = true; + boolean originalAllowAdminApproval = ScriptApproval.ALLOW_ADMIN_APPROVAL_ENABLED; + ScriptApproval.ALLOW_ADMIN_APPROVAL_ENABLED = true; try { r.submit(wc.getPage(p, "configure").getFormByName("config")); @@ -961,7 +994,8 @@ public void testScriptApproval() throws Exception { ScriptApproval.get().preapproveAll(); ScriptApproval.get().clearApprovedScripts(); } finally { - ScriptApproval.ADMIN_AUTO_APPROVAL_ENABLED = original; + ScriptApproval.ADMIN_AUTO_APPROVAL_ENABLED = originalAdminAutoApprove; + ScriptApproval.ALLOW_ADMIN_APPROVAL_ENABLED = originalAllowAdminApproval; } } } @@ -1006,24 +1040,18 @@ public void testSandboxClassResolution() throws Exception { p.getPublishersList().add(new TestGroovyRecorder( new SecureGroovyScript("class Test { public void finalize() { } }; null", false, null))); - ACL.impersonate(User.getById("dev", true).impersonate(), new NotReallyRoleSensitiveCallable() { - public Void call() throws Exception { - FreeStyleBuild b = r.assertBuildStatus(Result.FAILURE, p.scheduleBuild2(0)); - r.assertLogContains("UnapprovedUsageException", b); - return null; - } - }); + try (ACLContext ctx = ACL.as(User.getById("dev", true))) { + FreeStyleBuild b = r.assertBuildStatus(Result.FAILURE, p.scheduleBuild2(0)); + r.assertLogContains("UnapprovedUsageException", b); + } Set ps = ScriptApproval.get().getPendingScripts(); assertEquals(1, ps.size()); ScriptApproval.get().approveScript(ps.iterator().next().getHash()); - ACL.impersonate(User.getById("dev", true).impersonate(), new NotReallyRoleSensitiveCallable() { - public Void call() throws Exception { - r.assertBuildStatus(Result.SUCCESS, p.scheduleBuild2(0)); - return null; - } - }); + try (ACLContext ctx = ACL.as(User.getById("dev", true))) { + r.assertBuildStatus(Result.SUCCESS, p.scheduleBuild2(0)); + } } @Issue("SECURITY-1292") @@ -1246,4 +1274,90 @@ public void testScriptAtFieldInitializers() throws Exception { FreeStyleBuild b2 = r.assertBuildStatus(Result.FAILURE, p2.scheduleBuild2(0)); r.assertLogContains("staticField jenkins.YesNoMaybe YES", b2); } + + @Issue("SECURITY-2848") + @Test public void blockScriptClassesWithMainMethods() throws Exception { + FreeStyleProject p = r.createFreeStyleProject(); + SecureGroovyScript s = new SecureGroovyScript( + "class Test extends org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SecureGroovyScriptTest$HasMainMethod { }", true, null); + p.getPublishersList().add(new TestGroovyRecorder(s)); + FreeStyleBuild b = r.buildAndAssertStatus(Result.FAILURE, p); + r.assertLogContains("method groovy.lang.GroovyObject invokeMethod java.lang.String java.lang.Object (Test main)", b); + assertNull(r.jenkins.getSystemMessage()); + } + + @Issue("SECURITY-2824") + @Test public void blockCastingBindingValues() throws Exception { + FreeStyleProject p = r.createFreeStyleProject(); + SecureGroovyScript s = new SecureGroovyScript( + "class Test { File list }", true, null); + TestGroovyRecorder recorder = new TestGroovyRecorder(s); + Binding binding = new Binding(); + binding.setProperty("list", Collections.singletonList("secret.key")); + recorder.setBinding(binding); + p.getPublishersList().add(recorder); + FreeStyleBuild b = r.buildAndAssertStatus(Result.FAILURE, p); + r.assertLogContains("new java.io.File java.lang.String", b); + } + + @Test public void testApprovalFromFormValidation() throws Exception { + r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); + MockAuthorizationStrategy mockStrategy = new MockAuthorizationStrategy(); + mockStrategy.grant(Jenkins.ADMINISTER).everywhere().to("admin"); + for (Permission p : Item.PERMISSIONS.getPermissions()) { + mockStrategy.grant(p).everywhere().to("admin"); + } + r.jenkins.setAuthorizationStrategy(mockStrategy); + + FreeStyleProject p = r.createFreeStyleProject("p"); + try (JenkinsRule.WebClient wc = r.createWebClient()) { + CollectingAlertHandler altertHandler = new CollectingAlertHandler(); + wc.setAlertHandler(altertHandler); + + wc.login("admin"); + HtmlPage page = wc.getPage(p, "configure"); + HtmlForm config = page.getFormByName("config"); + HtmlFormUtil.getButtonByCaption(config, "Add post-build action").click(); // lib/hudson/project/config-publishers2.jelly + addPostBuildAction(page); + wc.waitForBackgroundJavaScript(10000); + List scripts = config.getTextAreasByName("_.script"); + // Get the last one, because previous ones might be from Lockable Resources during PCT. + HtmlTextArea script = scripts.get(scripts.size() - 1); + String groovy = "build.externalizableId"; + script.setText(groovy); + // nothing is approved or pending (no save) + assertThat(ScriptApproval.get().getPendingScripts(), is(empty())); + assertThat(ScriptApproval.get().getApprovedScriptHashes(), is(emptyArray())); + + wc.waitForBackgroundJavaScript(10_000); // FormValidation to display + + page.getAnchorByText("Approve script").click(); + + wc.waitForBackgroundJavaScript(10_000); // wait for the ajax approval to complete + + assertThat(altertHandler.getCollectedAlerts(), contains("Script approved")); + // script is approved + assertThat(ScriptApproval.get().getPendingScripts(), is(empty())); + assertThat(ScriptApproval.get().getApprovedScriptHashes(), is(arrayWithSize(1))); + + r.submit(config); + + FreeStyleBuild b = r.assertBuildStatus(Result.SUCCESS, p.scheduleBuild2(0)); + } + } + + public static class HasMainMethod { + @Whitelisted + public HasMainMethod() { } + public static void main(String[] args) throws IOException { + Jenkins.get().setSystemMessage("SECURITY-2848"); + } + } + + @Issue("JENKINS-38908") + @Test public void groovyCallSiteSelectorMain() throws Exception { + Script receiver = (Script) new SecureGroovyScript("def main() {}; this", true, null).configuring(ApprovalContext.create()).evaluate(GroovyCallSiteSelectorTest.class.getClassLoader(), new Binding(), null); + assertEquals(receiver.getClass().getMethod("main"), GroovyCallSiteSelector.method(receiver, "main", new Object[0])); + assertEquals(receiver.getClass().getMethod("main", String[].class), GroovyCallSiteSelector.method(receiver, "main", new Object[] {"somearg"})); + } } diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/TestGroovyRecorder.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/TestGroovyRecorder.java index 4071a376d..6b0e8b645 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/TestGroovyRecorder.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/TestGroovyRecorder.java @@ -24,6 +24,7 @@ package org.jenkinsci.plugins.scriptsecurity.sandbox.groovy; +import edu.umd.cs.findbugs.annotations.NonNull; import groovy.lang.Binding; import hudson.Extension; import hudson.Launcher; @@ -46,6 +47,7 @@ public final class TestGroovyRecorder extends Recorder { private final SecureGroovyScript script; + private transient Binding binding; @DataBoundConstructor public TestGroovyRecorder(SecureGroovyScript script) { this.script = script.configuringWithKeyItem(); @@ -54,10 +56,16 @@ public final class TestGroovyRecorder extends Recorder { public SecureGroovyScript getScript() { return script; } - + + public void setBinding(Binding binding) { + this.binding = binding; + } + @Override public boolean perform(AbstractBuild build, Launcher launcher, BuildListener listener) throws InterruptedException, IOException { try { - Binding binding = new Binding(); + if (binding == null) { + binding = new Binding(); + } binding.setVariable("build", build); build.setDescription(String.valueOf(script.evaluate(Jenkins.get().getPluginManager().uberClassLoader, binding, listener))); } catch (Exception x) { @@ -71,7 +79,8 @@ public SecureGroovyScript getScript() { } @Extension public static final class DescriptorImpl extends BuildStepDescriptor { - + + @NonNull @Override public String getDisplayName() { return "Test Groovy Recorder"; } diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/EnumeratingWhitelistTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/EnumeratingWhitelistTest.java index c0c9957d1..534c78eb7 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/EnumeratingWhitelistTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/EnumeratingWhitelistTest.java @@ -33,6 +33,7 @@ import org.junit.Assert; import org.junit.Test; +import org.jvnet.hudson.test.Issue; import static org.junit.Assert.*; public class EnumeratingWhitelistTest { @@ -136,51 +137,22 @@ public void caching() throws Exception { Field f = Fancy.class.getField("myF"); Field staticF = Fancy.class.getField("myStaticF"); - EnumeratingWhitelist.MethodSignature mSig = new EnumeratingWhitelist.MethodSignature(Fancy.class, "m", Object[].class); - String[] obString = {Object.class.getName()}; - EnumeratingWhitelist.StaticMethodSignature staticMSig = new EnumeratingWhitelist.StaticMethodSignature(Fancy.class.getName(), "staticM", obString); - EnumeratingWhitelist.NewSignature conSig = new EnumeratingWhitelist.NewSignature(Fancy.class); - EnumeratingWhitelist.FieldSignature fSig = new EnumeratingWhitelist.FieldSignature(Fancy.class, "myF"); - EnumeratingWhitelist.FieldSignature staticFSig = new EnumeratingWhitelist.StaticFieldSignature(Fancy.class.getName(), "myStaticF"); - // Each tested twice to allow for once with and once without caching // And we're checking cache entries and equality of canonical method name and signatures assertFalse(myList.permitsMethod(m, new Fancy(), null)); assertFalse(myList.permitsMethod(m, new Fancy(), null)); - assertEquals(Boolean.FALSE, myList.permittedCache.get(mSig.toString())); assertFalse(myList.permitsStaticMethod(staticM, null)); assertFalse(myList.permitsStaticMethod(staticM, null)); - assertEquals(Boolean.FALSE, myList.permittedCache.get(staticMSig.toString())); assertFalse(myList.permitsConstructor(con, null)); assertFalse(myList.permitsConstructor(con, null)); - assertEquals(Boolean.FALSE, myList.permittedCache.get(conSig.toString())); assertFalse(myList.permitsFieldGet(f, new Fancy())); assertFalse(myList.permitsFieldGet(f, new Fancy())); - assertEquals(Boolean.FALSE, myList.permittedCache.get(fSig.toString())); assertFalse(myList.permitsStaticFieldGet(staticF)); assertFalse(myList.permitsStaticFieldGet(staticF)); - assertEquals(Boolean.FALSE, myList.permittedCache.get(staticFSig.toString())); - - // Now we clear, add matching signatures, and precache - myList.clearCache(); - assertTrue(myList.permittedCache.isEmpty()); - myList.methodSignatures.add(mSig); - myList.staticMethodSignatures.add(staticMSig); - myList.newSignatures.add(conSig); - myList.fieldSignatures.add(fSig); - myList.staticFieldSignatures.add(staticFSig); - myList.precache(); - - // Just confirms we cached right - assertEquals(Boolean.TRUE, myList.permittedCache.get(mSig.toString())); - assertEquals(Boolean.TRUE, myList.permittedCache.get(staticMSig.toString())); - assertEquals(Boolean.TRUE, myList.permittedCache.get(conSig.toString())); - assertEquals(Boolean.TRUE, myList.permittedCache.get(fSig.toString())); - assertEquals(Boolean.TRUE, myList.permittedCache.get(staticFSig.toString())); } @Test @@ -189,12 +161,17 @@ public void testCachingWithWildcards() throws Exception { Field f = C.class.getField("myField"); myList.fieldSignatures.add(new EnumeratingWhitelist.FieldSignature(C.class, "*")); - myList.precache(); - assertTrue(myList.permittedCache.isEmpty()); // We cannot cache Wildcard matches directly, only when we see a method they permit - assertTrue(myList.permitsFieldGet(f, new C())); // No cache, so we fall back to direct search and hit the wildcard, then cache permitted assertTrue(myList.permitsFieldGet(f, new C())); // Should hit cache for that specific method - assertEquals(Boolean.TRUE, myList.permittedCache.get(EnumeratingWhitelist.canonicalFieldSig(f))); // Verifies it can cache that + } + + @Issue("JENKINS-42214") + @Test public void fieldExists() throws Exception { + assertTrue(new EnumeratingWhitelist.FieldSignature("hudson.model.Result", "color").exists()); + assertTrue(new EnumeratingWhitelist.StaticFieldSignature("hudson.model.Result", "ABORTED").exists()); + + assertFalse(new EnumeratingWhitelist.StaticFieldSignature("hudson.model.Result", "color").exists()); + assertFalse(new EnumeratingWhitelist.FieldSignature("hudson.model.Result", "ABORTED").exists()); } } diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/GenericWhitelistTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/GenericWhitelistTest.java index 0d438a5a9..77a63a3a3 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/GenericWhitelistTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/GenericWhitelistTest.java @@ -36,7 +36,7 @@ public class GenericWhitelistTest { @Rule public ErrorCollector errors = new ErrorCollector(); @Test public void sanity() throws Exception { - StaticWhitelistTest.sanity(GenericWhitelist.class.getResource("generic-whitelist")); + StaticWhitelistTest.sanity(StaticWhitelist.class.getResource("generic-whitelist")); } @Issue("SECURITY-538") diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/JenkinsWhitelistTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/JenkinsWhitelistTest.java index c7b01621a..5f5bfacdd 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/JenkinsWhitelistTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/JenkinsWhitelistTest.java @@ -29,7 +29,7 @@ public class JenkinsWhitelistTest { @Test public void sanity() throws Exception { - StaticWhitelistTest.sanity(JenkinsWhitelist.class.getResource("jenkins-whitelist")); + StaticWhitelistTest.sanity(StaticWhitelist.class.getResource("jenkins-whitelist")); } } diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/ProxyWhitelistTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/ProxyWhitelistTest.java index 32fa7f343..680a79589 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/ProxyWhitelistTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/ProxyWhitelistTest.java @@ -51,7 +51,7 @@ public class ProxyWhitelistTest { pw1.reset(Collections.singleton(new StaticWhitelist(new StringReader("method java.lang.String length\nmethod java.lang.Object hashCode")))); assertTrue(pw2.permitsMethod(String.class.getMethod("length"), "x", new Object[0])); assertTrue(pw2.permitsMethod(Object.class.getMethod("hashCode"), "x", new Object[0])); - pw1.reset(Collections.emptySet()); + pw1.reset(Collections.emptySet()); assertFalse(pw2.permitsMethod(String.class.getMethod("length"), "x", new Object[0])); assertFalse(pw2.permitsMethod(Object.class.getMethod("hashCode"), "x", new Object[0])); } @@ -60,21 +60,10 @@ public class ProxyWhitelistTest { ProxyWhitelist pw1 = new ProxyWhitelist(new StaticWhitelist(new StringReader("staticField java.util.Collections EMPTY_LIST"))); ProxyWhitelist pw2 = new ProxyWhitelist(pw1); assertTrue(pw2.permitsStaticFieldGet(Collections.class.getField("EMPTY_LIST"))); - pw1.reset(Collections.emptySet()); + pw1.reset(Collections.emptySet()); assertFalse(pw2.permitsStaticFieldGet(Collections.class.getField("EMPTY_LIST"))); } - /** Ensures we cache at the top-level ProxyWhitelist */ - @Test - public void caching() throws Exception { - ProxyWhitelist pw1 = new ProxyWhitelist(new StaticWhitelist(new StringReader("method java.lang.String length"))); - assertEquals(1, ((EnumeratingWhitelist)pw1.delegates.get(0)).permittedCache.size()); - - ProxyWhitelist pw2 = new ProxyWhitelist(pw1, new StaticWhitelist(new StringReader("method java.lang.String trim"))); - assertEquals(0, ((EnumeratingWhitelist)pw1.delegates.get(0)).permittedCache.size()); - assertEquals(2, ((EnumeratingWhitelist)pw2.delegates.get(0)).permittedCache.size()); - } - /** * Test concurrent modification of delegates when initializing a ProxyWhitelist. This may cause concurrent threads * to enter an infinite loop when using a {@link java.util.WeakHashMap} to hold the delegates as it is not diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/StaticWhitelistTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/StaticWhitelistTest.java index 470eabf9e..ab035ac9e 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/StaticWhitelistTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/StaticWhitelistTest.java @@ -40,8 +40,11 @@ import java.util.List; import java.util.Random; import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.MatchResult; import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.EnumeratingWhitelist.MethodSignature; +import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.EnumeratingWhitelist.NewSignature; import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.EnumeratingWhitelist.Signature; import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.EnumeratingWhitelist.StaticMethodSignature; import org.junit.Assert; @@ -58,7 +61,7 @@ public class StaticWhitelistTest { static void sanity(URL definition) throws Exception { StaticWhitelist wl = StaticWhitelist.from(definition); - List sigs = new ArrayList(); + List sigs = new ArrayList<>(); try (InputStream is = definition.openStream(); InputStreamReader isr = new InputStreamReader(is, StandardCharsets.UTF_8); BufferedReader br = new BufferedReader(isr)) { String line; while ((line = br.readLine()) != null) { @@ -113,6 +116,8 @@ static void sanity(URL definition) throws Exception { * on the test environment. */ private static final Set KNOWN_GOOD_SIGNATURES = new HashSet<>(Arrays.asList( + // From workflow-cps, which is not a dependency of this plugin. + new NewSignature("org.jenkinsci.plugins.workflow.cps.CpsScript"), // From workflow-support, which is not a dependency of this plugin. new MethodSignature("org.jenkinsci.plugins.workflow.support.steps.build.RunWrapper", "getRawBuild"), // From groovy-cps, which is not a dependency of this plugin. @@ -120,8 +125,9 @@ static void sanity(URL definition) throws Exception { "java.util.Iterator", "groovy.lang.Closure"), // Overrides CharSequence.isEmpty in Java 15+. new MethodSignature(String.class, "isEmpty"), - // Does not exist until Java 15. + // Do not exist until Java 15. new MethodSignature(CharSequence.class, "isEmpty"), + new MethodSignature(String.class, "stripIndent"), // Override the corresponding RandomGenerator methods in Java 17+. new MethodSignature(Random.class, "nextBoolean"), new MethodSignature(Random.class, "nextBytes", byte[].class), @@ -139,7 +145,17 @@ static void sanity(URL definition) throws Exception { new MethodSignature("java.util.random.RandomGenerator", "nextGaussian"), new MethodSignature("java.util.random.RandomGenerator", "nextInt"), new MethodSignature("java.util.random.RandomGenerator", "nextInt", "int"), - new MethodSignature("java.util.random.RandomGenerator", "nextLong") + new MethodSignature("java.util.random.RandomGenerator", "nextLong"), + // Override the corresponding MatchResult methods in Java 20+. + new MethodSignature(Matcher.class, "end", String.class), + new MethodSignature(Matcher.class, "group", String.class), + new MethodSignature(Matcher.class, "start", String.class), + // Do not exist until Java 20. + new MethodSignature(MatchResult.class, "end", String.class), + new MethodSignature(MatchResult.class, "group", String.class), + new MethodSignature(MatchResult.class, "hasMatch"), + new MethodSignature(MatchResult.class, "namedGroups"), + new MethodSignature(MatchResult.class, "start", String.class) )); @Test public void sanity() throws Exception { diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/EntryApprovalTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/EntryApprovalTest.java index 807c436f2..bf9ce0fea 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/EntryApprovalTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/EntryApprovalTest.java @@ -100,7 +100,7 @@ static final class Entry extends Approvable { this.entry = entry; ScriptApproval.get().configuring(entry, ApprovalContext.create()); // If configure is successful, calculate the hash - this.hash = ScriptApproval.hashClasspathEntry(entry.getURL()); + this.hash = ScriptApproval.DEFAULT_HASHER.hashClasspathEntry(entry.getURL()); } @Override diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/HasherScriptApprovalTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/HasherScriptApprovalTest.java new file mode 100644 index 000000000..ad10842e9 --- /dev/null +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/HasherScriptApprovalTest.java @@ -0,0 +1,172 @@ +package org.jenkinsci.plugins.scriptsecurity.scripts; + +import org.hamcrest.Matcher; +import org.jenkinsci.plugins.scriptsecurity.scripts.languages.GroovyLanguage; +import edu.umd.cs.findbugs.annotations.NonNull; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.Issue; +import org.jvnet.hudson.test.JenkinsSessionRule; +import org.jvnet.hudson.test.LoggerRule; + +import java.io.IOException; +import java.net.MalformedURLException; +import java.net.URL; +import java.util.logging.Level; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInRelativeOrder; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.not; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +public class HasherScriptApprovalTest { + @Rule + public JenkinsSessionRule session = new JenkinsSessionRule(); + @Rule + public LoggerRule log = new LoggerRule(); + + @Test + @Issue("SECURITY-2564") + public void hasherMatchesItsOwnHashes() throws Throwable { + session.then(r -> { + for (ScriptApproval.Hasher hasher : ScriptApproval.Hasher.values()) { + assertTrue(hasher.pattern().matcher(hasher.hash("Hello World", "Text")).matches()); + } + }); + } + + @Test + @Issue("SECURITY-2564") + public void warnsAndClearsDeprecatedScriptHashes() throws Throwable { + session.then(r -> { + final ScriptApproval approval = ScriptApproval.get(); + approval.approveScript(ScriptApproval.Hasher.SHA1.hash("Hello World", "Text")); + approval.approveScript(ScriptApproval.Hasher.SHA1.hash("node { echo 'Hello World' }", "Groovy")); + approval.approveScript(ScriptApproval.DEFAULT_HASHER.hash("have you tried it sometime?", "Text")); + }); + log.record(ScriptApproval.class.getName(), Level.FINE).capture(10000); + session.then(r -> { + final ScriptApproval approval = ScriptApproval.get(); + assertEquals(2, approval.countDeprecatedApprovedScriptHashes()); + assertThat(log.getMessages(), hasItem( + containsString("There are 2 deprecated approved script hashes " + + "and 0 deprecated approved classpath hashes."))); + approval.clearDeprecatedApprovedScripts(); + assertEquals(0, approval.countDeprecatedApprovedScriptHashes()); + }); + } + + @Test + @Issue("SECURITY-2564") + public void convertsScriptApprovalsOnUse() throws Throwable { + final String script = "node { echo 'Hello World' }"; + final Matcher> logMatcher = containsInRelativeOrder( + containsString("A script is approved with an old hash algorithm. Converting now, ")); + session.then(r -> { + final ScriptApproval approval = ScriptApproval.get(); + approval.approveScript(ScriptApproval.Hasher.SHA1.hash("Hello World", "Text")); + approval.approveScript(ScriptApproval.Hasher.SHA1.hash(script, GroovyLanguage.get().getName())); + approval.approveScript(ScriptApproval.DEFAULT_HASHER.hash("have you tried it sometime?", "Text")); + }); + log.record(ScriptApproval.class.getName(), Level.FINE).capture(10000); + session.then(r -> { + final ScriptApproval approval = ScriptApproval.get(); + assertEquals(2, approval.countDeprecatedApprovedScriptHashes()); + approval.using(script, GroovyLanguage.get()); + assertEquals(1, approval.countDeprecatedApprovedScriptHashes()); + assertThat(log.getMessages(), logMatcher); + }); + log.capture(10000); + session.then(r -> { + final ScriptApproval approval = ScriptApproval.get(); + assertEquals(1, approval.countDeprecatedApprovedScriptHashes()); + approval.using(script, GroovyLanguage.get()); + assertEquals(1, approval.countDeprecatedApprovedScriptHashes()); + assertThat(log.getMessages(), not(logMatcher)); + }); + } + + @Test + @Issue("SECURITY-2564") + public void testConvertApprovedClasspathEntries() throws Throwable { + session.then(r -> { + final ScriptApproval approval = ScriptApproval.get(); + addApprovedClasspathEntries(approval); + assertEquals(2, approval.countDeprecatedApprovedClasspathHashes()); + }); + log.record(ScriptApproval.class.getName(), Level.FINE).capture(10000); + session.then(r -> { + final ScriptApproval approval = ScriptApproval.get(); + assertEquals(2, approval.countDeprecatedApprovedClasspathHashes()); + + assertThat(log.getMessages(), hasItem( + containsString("There are 0 deprecated approved script hashes " + + "and 2 deprecated approved classpath hashes."))); + + approval.convertDeprecatedApprovedClasspathEntries(); + assertThat(log.getMessages(), containsInRelativeOrder( + containsString("Scheduling conversion of 2 deprecated approved classpathentry hashes."), + containsString("Background conversion task scheduled."))); + try { + while (approval.isConvertingDeprecatedApprovedClasspathEntries()) { + Thread.sleep(500); + } + } catch (InterruptedException ignored) { + } + assertEquals(0, approval.countDeprecatedApprovedClasspathHashes()); + }); + } + + @Test + @Issue("SECURITY-2564") + public void testClasspathEntriesConvertedOnUse() throws Throwable { + session.then(r -> { + final ScriptApproval approval = ScriptApproval.get(); + addApprovedClasspathEntries(approval); + assertEquals(2, approval.countDeprecatedApprovedClasspathHashes()); + }); + log.record(ScriptApproval.class.getName(), Level.FINE).capture(10000); + session.then(r -> { + final ScriptApproval approval = ScriptApproval.get(); + assertEquals(2, approval.countDeprecatedApprovedClasspathHashes()); + URL url = getJar("org/apache/commons/lang3/StringUtils.class"); + approval.using(new ClasspathEntry(url.toString())); + assertEquals(1, approval.countDeprecatedApprovedClasspathHashes()); + final Matcher> logMatcher = containsInRelativeOrder( + containsString("A classpath is approved with an old hash algorithm. Converting now, ")); + assertThat(log.getMessages(), logMatcher); + log.capture(1000); + approval.using(new ClasspathEntry(url.toString())); //Using it again should not convert it again. + assertThat(log.getMessages(), not(logMatcher)); + assertEquals(1, approval.countDeprecatedApprovedClasspathHashes()); + }); + } + + private void addApprovedClasspathEntries(final ScriptApproval approval) throws IOException { + URL url = getJar("org/apache/commons/lang3/StringUtils.class"); + ScriptApproval.ApprovedClasspathEntry acp = new ScriptApproval.ApprovedClasspathEntry( + ScriptApproval.Hasher.SHA1.hashClasspathEntry(url), + url + ); + approval.addApprovedClasspathEntry(acp); + + url = getJar("net/sf/json/JSON.class"); + acp = new ScriptApproval.ApprovedClasspathEntry( + ScriptApproval.Hasher.SHA1.hashClasspathEntry(url), + url + ); + approval.addApprovedClasspathEntry(acp); + approval.save(); + } + + @NonNull + private URL getJar(final String resource) throws MalformedURLException { + URL url = getClass().getClassLoader().getResource(resource); + String path = url.getPath(); + path = path.substring(0, path.indexOf('!')); + return new URL(path); + } +} diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/JcascTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/JcascTest.java index b562f911f..67763a443 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/JcascTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/JcascTest.java @@ -8,27 +8,41 @@ import io.jenkins.plugins.casc.model.CNode; import org.junit.ClassRule; import org.junit.Test; +import org.jvnet.hudson.test.LoggerRule; + +import java.util.logging.Level; import static io.jenkins.plugins.casc.misc.Util.getSecurityRoot; import static io.jenkins.plugins.casc.misc.Util.toStringFromYamlFile; import static io.jenkins.plugins.casc.misc.Util.toYamlString; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.collection.IsIterableContainingInAnyOrder.containsInAnyOrder; +import static org.hamcrest.core.StringContains.containsString; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; public class JcascTest { - @ClassRule + @ClassRule(order = 1) + public static LoggerRule logger = new LoggerRule().record(ScriptApproval.class.getName(), Level.WARNING) + .capture(100); + + @ClassRule(order = 2) @ConfiguredWithCode("smoke_test.yaml") public static JenkinsConfiguredWithCodeRule j = new JenkinsConfiguredWithCodeRule(); + + @Test public void smokeTestEntry() throws Exception { String[] approved = ScriptApproval.get().getApprovedSignatures(); - assertTrue(approved.length == 1); + assertEquals(1, approved.length); assertEquals(approved[0], "method java.net.URI getHost"); String[] approvedScriptHashes = ScriptApproval.get().getApprovedScriptHashes(); - assertTrue(approvedScriptHashes.length == 1); + assertEquals(1, approvedScriptHashes.length); assertEquals(approvedScriptHashes[0], "fccae58c5762bdd15daca97318e9d74333203106"); + assertThat(logger.getMessages(), containsInAnyOrder( + containsString("Adding deprecated script hash " + + "that will be converted on next use: fccae58c5762bdd15daca97318e9d74333203106"))); } @Test diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/Manager.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/Manager.java index 0a468a0f2..cffabb6fa 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/Manager.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/Manager.java @@ -24,11 +24,10 @@ package org.jenkinsci.plugins.scriptsecurity.scripts; -import com.gargoylesoftware.htmlunit.ConfirmHandler; -import com.gargoylesoftware.htmlunit.Page; -import com.gargoylesoftware.htmlunit.html.DomElement; -import com.gargoylesoftware.htmlunit.html.HtmlElement; -import com.gargoylesoftware.htmlunit.html.HtmlPage; +import org.htmlunit.ConfirmHandler; +import org.htmlunit.html.DomElement; +import org.htmlunit.html.HtmlElement; +import org.htmlunit.html.HtmlPage; import org.jvnet.hudson.test.JenkinsRule; import java.io.IOException; diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalLoadingTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalLoadingTest.java new file mode 100644 index 000000000..4fc7c14e1 --- /dev/null +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalLoadingTest.java @@ -0,0 +1,66 @@ +/* + * The MIT License + * + * Copyright 2024 CloudBees, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package org.jenkinsci.plugins.scriptsecurity.scripts; + +import java.io.File; +import jenkins.RestartRequiredException; +import org.apache.commons.io.FileUtils; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.arrayContaining; +import org.junit.AssumptionViolatedException; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.RealJenkinsRule; + +public final class ScriptApprovalLoadingTest { + + @Rule public final RealJenkinsRule rr = new RealJenkinsRule(); + + @Test public void dynamicLoading() throws Throwable { + rr.then(ScriptApprovalLoadingTest::_dynamicLoading1); + rr.then(ScriptApprovalLoadingTest::_dynamicLoading2); + } + + private static void _dynamicLoading1(JenkinsRule r) throws Throwable { + File plugin = new File(r.jenkins.root, "plugins/script-security.jpl"); + FileUtils.copyFile(plugin, new File(plugin + ".bak")); + r.jenkins.pluginManager.getPlugin("script-security").doDoUninstall(); + } + + private static void _dynamicLoading2(JenkinsRule r) throws Throwable { + File plugin = new File(r.jenkins.root, "plugins/script-security.jpl"); + FileUtils.copyFile(new File(plugin + ".bak"), plugin); + try { + r.jenkins.pluginManager.dynamicLoad(plugin); + } catch (RestartRequiredException x) { + throw new AssumptionViolatedException("perhaps running in PCT, where this cannot be tested", x); + } + ScriptApproval sa = ScriptApproval.get(); + sa.approveSignature("method java.lang.Object wait"); + assertThat(sa.getApprovedSignatures(), arrayContaining("method java.lang.Object wait")); + } + +} diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalNoteTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalNoteTest.java index a603ebec7..c46a0171c 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalNoteTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalNoteTest.java @@ -24,9 +24,9 @@ package org.jenkinsci.plugins.scriptsecurity.scripts; -import com.gargoylesoftware.htmlunit.TextPage; -import com.gargoylesoftware.htmlunit.html.DomNodeUtil; -import com.gargoylesoftware.htmlunit.html.HtmlPage; +import org.htmlunit.TextPage; +import org.htmlunit.html.DomNodeUtil; +import org.htmlunit.html.HtmlPage; import hudson.model.FreeStyleBuild; import hudson.model.FreeStyleProject; import hudson.model.Item; diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java index 0f189d3cb..a3fe3b092 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java @@ -24,8 +24,8 @@ package org.jenkinsci.plugins.scriptsecurity.scripts; -import com.gargoylesoftware.htmlunit.html.HtmlPage; -import com.gargoylesoftware.htmlunit.html.HtmlTextArea; +import org.htmlunit.html.HtmlPage; +import org.htmlunit.html.HtmlTextArea; import hudson.model.FreeStyleProject; import hudson.model.Result; import hudson.util.VersionNumber; @@ -49,6 +49,8 @@ import java.util.logging.Level; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.hasItemInArray; +import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; @@ -56,7 +58,7 @@ public class ScriptApprovalTest extends AbstractApprovalTest { @Rule - public LoggerRule logging = new LoggerRule(); + public LoggerRule logging = new LoggerRule().record(ScriptApproval.class, Level.FINER).capture(100); private static final String CLEAR_ALL_ID = "approvedScripts-clear"; @@ -74,15 +76,9 @@ public class ScriptApprovalTest extends AbstractApprovalTest r.getMessage()).toArray(String[]::new), + hasItemInArray("Malformed signature entry in scriptApproval.xml: ' new java.lang.Exception java.lang.String'")); } @Test @LocalData("dangerousApproved") public void dangerousApprovedSignatures() { @@ -176,6 +172,29 @@ public void upgradeSmokes() throws Exception { r.assertBuildStatus(Result.SUCCESS, p.scheduleBuild2(0).get())); } + @LocalData // Some scriptApproval.xml with existing signatures approved + @Test + public void reload() throws Exception { + configureSecurity(); + ScriptApproval sa = ScriptApproval.get(); + + FreeStyleProject p = r.createFreeStyleProject(); + p.getPublishersList().add(new TestGroovyRecorder( + new SecureGroovyScript("jenkins.model.Jenkins.instance", true, null))); + p.getPublishersList().add(new TestGroovyRecorder( + new SecureGroovyScript("println(jenkins.model.Jenkins.instance.getLabels())", false, null))); + r.assertLogNotContains("org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException: " + + "Scripts not permitted to use staticMethod jenkins.model.Jenkins getInstance", + r.assertBuildStatus(Result.SUCCESS, p.scheduleBuild2(0).get())); + r.assertLogNotContains("org.jenkinsci.plugins.scriptsecurity.scripts.UnapprovedUsageException: script not yet approved for use", + r.assertBuildStatus(Result.SUCCESS, p.scheduleBuild2(0).get())); + + ScriptApproval.get().getConfigFile().delete(); + sa.load(); + r.assertLogContains("org.jenkinsci.plugins.scriptsecurity.scripts.UnapprovedUsageException: script not yet approved for use", + r.assertBuildStatus(Result.FAILURE, p.scheduleBuild2(0).get())); + } + private Script script(String groovy) { return new Script(groovy); } diff --git a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/TestGroovyRecorder/config.jelly b/src/test/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/TestGroovyRecorder/config.jelly similarity index 100% rename from src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/TestGroovyRecorder/config.jelly rename to src/test/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/TestGroovyRecorder/config.jelly diff --git a/src/test/resources/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest/reload/scriptApproval.xml b/src/test/resources/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest/reload/scriptApproval.xml new file mode 100644 index 000000000..b15a50a3d --- /dev/null +++ b/src/test/resources/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest/reload/scriptApproval.xml @@ -0,0 +1,14 @@ + + + + fccae58c5762bdd15daca97318e9d74333203106 + + + staticMethod jenkins.model.Jenkins getInstance + + + + + + + \ No newline at end of file