diff --git a/README.md b/README.md index f23cbb7..cb74423 100644 --- a/README.md +++ b/README.md @@ -36,8 +36,10 @@ understand Spring semantics — not regex. own configuration metadata for Boot 2.0 through 3.5. - **CI-friendly** — stable exit codes and SARIF 2.1.0 output for GitHub code scanning. -- **Zero false-positive noise** — default `--fail-on HIGH`, and every rule - ships an explicit false-positive rationale. +- **False positives are treated as bugs** — every rule ships an explicit + false-positive rationale with a fixture behind it, and a rule that fires on + something it cannot reach gets narrowed rather than documented around. + SPR-CONFIG-001 lost its `shutdown` token that way. Default `--fail-on HIGH`. ## Getting started @@ -97,6 +99,7 @@ sprig version Print version | SPR-CONFIG-003 | MEDIUM | config | Cookie `http-only`/`secure` explicitly disabled | | SPR-CONFIG-004 | HIGH | config | CORS wildcard + credentials on Actuator or GraphQL | | SPR-CONFIG-005 | LOW | config | Spring Security logging at `DEBUG` / `TRACE` | +| SPR-CONFIG-006 | CRITICAL | config | Actuator `shutdown` / `heapdump` both exposed and opened by `access` or `enabled` | Each rule has a doc with detection details and a false-positive rationale under [`docs/rules/`](docs/rules/). diff --git a/docs/rules/SPR-CONFIG-001.md b/docs/rules/SPR-CONFIG-001.md index 9979435..3da3796 100644 --- a/docs/rules/SPR-CONFIG-001.md +++ b/docs/rules/SPR-CONFIG-001.md @@ -8,20 +8,45 @@ ## Description -`management.endpoints.web.exposure.include: "*"` (or `env`, `heapdump`, -`shutdown`) exposes Spring Boot Actuator over HTTP. The classic CVE pattern: -`/actuator/env` leaks environment variables (secrets, credentials), and -`/actuator/heapdump` leaks process memory — including tokens and keys. +`management.endpoints.web.exposure.include: "*"` (or `env`, `heapdump`) puts +Spring Boot Actuator on the application's own HTTP connector. `/actuator/env` +leaks environment variables, which is where credentials live, and +`/actuator/configprops` leaks the resolved configuration alongside it. + +What the wildcard reaches depends on the Boot version, and the difference is +worth stating because the older behaviour is the more serious one. Measured +against running apps with `include: "*"` and nothing else set: + +| Endpoint | 2.3.12 | 3.3.13 | 3.5.16 | +|---|---|---|---| +| `env`, `configprops`, `beans`, `mappings`, `threaddump`, `loggers`, `metrics` | 200 | 200 | 200 | +| `heapdump` | 200 | 200 | **404** | +| `shutdown` | 404 | 404 | 404 | + +- **Through Boot 3.3**, the wildcard alone serves `/actuator/heapdump`: a HPROF + of the live process, carrying every credential and session token in memory. +- **From Boot 3.4**, the access gate holds `heapdump` at 404 until a second + property opens it. On those versions this finding marks the exposure, and + [SPR-CONFIG-006](SPR-CONFIG-006.md) is what reports the disclosure. +- **On every version**, the information leak through `env` and its neighbours + is real and unauthenticated. ## Detection -Fires when `management.endpoints.web.exposure.include` contains `*`, `env`, -`heapdump`, or `shutdown` (comma-separated or YAML list). +Fires when `management.endpoints.web.exposure.include` contains `*`, `env`, or +`heapdump` (comma-separated or YAML list). ## False-positive rationale - Safe, explicit lists (`health`, `info`) are never flagged. - JMX exposure (`management.endpoints.jmx.*`) is not in scope. +- **`shutdown` in the exposure list is not flagged**, and used to be. Exposure + does not reach it: `POST /actuator/shutdown` answered 404 under `include: "*"` + on 2.3.12, 3.3.13 and 3.5.16 alike, because the endpoint ships switched off in + every release. It takes `management.endpoint.shutdown.enabled: true` or, from + 3.4, `management.endpoint.shutdown.access: unrestricted` to open, which is + SPR-CONFIG-006's subject. Flagging the bare token was a HIGH finding with + nothing behind it (#45). ## Remediation diff --git a/docs/rules/SPR-CONFIG-006.md b/docs/rules/SPR-CONFIG-006.md new file mode 100644 index 0000000..256be5f --- /dev/null +++ b/docs/rules/SPR-CONFIG-006.md @@ -0,0 +1,135 @@ +# SPR-CONFIG-006 — Actuator shutdown or heapdump both exposed and switched on + +| | | +|---|---| +| Severity | CRITICAL | +| Kind | CONFIG | +| Tags | `actuator`, `config` | + +## Description + +Spring Boot ships `shutdown` and `heapdump` switched off. Reaching either takes +two properties, not one: the endpoint has to be in the web exposure list **and** +its access gate has to be open. This rule fires only when both are true in the +same file, and what it reports is not a possibility but a request that succeeds: + +- `POST /actuator/shutdown` returns 200 and the process exits. +- `GET /actuator/heapdump` returns a HPROF of the live heap. Everything the + application holds in memory is in it: datasource passwords, session tokens, + signing keys. + +Neither gate is enough alone. Measured on Boot 3.5.16, with +`exposure.include: health,info` and `management.endpoint.shutdown.access: +unrestricted` set, `POST /actuator/shutdown` still answers 404. Add the id to +the exposure list and the same request answers 200. + +## What reaches what + +Every cell below is an observed HTTP status against a running app +(`spring-boot-starter-web` + `spring-boot-starter-actuator`, no security +starter), with `management.endpoints.web.exposure.include: "*"`. + +Each cell is `heapdump` / `shutdown`. + +| Configuration | 2.3.12 | 3.3.13 | 3.5.16 | +|---|---|---|---| +| exposure only | 200 / 404 | 200 / 404 | **404** / 404 | +| `endpoint..enabled: true` | 200 / **200** | 200 / **200** | **200** / **200** | +| `endpoints.enabled-by-default: true` | 200 / **200** | 200 / **200** | **200** / **200** | +| `endpoint..access: unrestricted` | 200 / 404 | 200 / 404 | **200** / **200** | +| `endpoint..access: read-only` | 200 / 404 | 200 / 404 | **200** / 404 | +| `endpoints.access.default: unrestricted` | 200 / 404 | 200 / 404 | **200** / **200** | +| `endpoints.access.max-permitted` | 200 / 404 | 200 / 404 | **200** / 404 | + +Where a row says ``, the id measured was `shutdown` on 2.3.12 and 3.3.13 and +both ids on 3.5.16. `heapdump` needs no help before 3.4, which is the point of +the first row; on 3.3.13 setting `heapdump.enabled: false` takes it from 200 to +404, so the property binds there as well. + +The whole `access` family is inert before 3.4, measured rather than assumed. +The pre-3.4 columns of the last row were taken at `max-permitted: none`, the +strictest value there is, and it still left `heapdump` answering 200; the 3.5.16 +column is at `read-only`. Everything reading 200 in those two columns is what +exposure alone already gives, which is why `heapdump` on Boot 2.x and 3.3 +belongs to SPR-CONFIG-001 rather than here. + +Three things fall out of that table and are built into the rule: + +- **`read-only` is enough to leak.** `heapdump` is a GET, so read-only access + serves it. Only `shutdown`, a POST, needs `unrestricted`. +- **Both spellings are live.** `enabled` works from 2.x through 3.5; + `access` arrived in 3.4 and does nothing before it. Neither replaced the + other on any version measured. +- **`max-permitted` caps writes only.** `read-only` takes `shutdown` back to + 404 and leaves `heapdump` serving a real dump. It caps both spellings: + `shutdown.enabled: true` under a `read-only` cap is 404 on 3.5.16. + +sprig does not read `pom.xml` or `build.gradle` and so has no idea which Boot +version a scanned project builds against. It reports either spelling and leaves +the version question to this table. + +## Detection + +Fires when, **within one file**, `management.endpoints.web.exposure.include` +contains `*` or the endpoint id, and one of the following opens that endpoint: + +| Property | Opens `shutdown` at | Opens `heapdump` at | +|---|---|---| +| `management.endpoint..enabled` | `true` | `true` | +| `management.endpoint..access` | `unrestricted` | `unrestricted`, `read-only` | +| `management.endpoints.enabled-by-default` | `true` | `true` | +| `management.endpoints.access.default` | `unrestricted` | `unrestricted`, `read-only` | + +The finding points at the property that opens the gate, because that is the line +a reader has to change. + +## False-positive rationale + +- **Exposure without access is not a finding.** An open gate on an endpoint + missing from the exposure list reaches nothing, and neither does exposure + without the gate. Both are fixtures. +- **A per-endpoint setting beats the blanket one.** `access.default: + unrestricted` with `shutdown.access: none` leaves shutdown at 404, and the + rule stays quiet about shutdown while still reporting `heapdump`. +- **`max-permitted` is honoured**, and only against write access, which is + where a simpler reading would produce a wrong answer in both directions. + + This is the one place the rule knowingly trades a miss for the quiet. The cap + arrived in 3.4 and does nothing before it: on 3.3.13, `shutdown.enabled: true` + under `max-permitted: read-only` still answers 200, and the rule stays silent. + Reading the cap as inert instead would put a CRITICAL finding on every + correctly capped 3.4+ project, and a 3.4-only property written into a pre-3.4 + project is already a line that does nothing on its own terms. +- **Both keys must be in the same file.** Base and profile files are never + merged, matching SPR-CONFIG-004. +- **`env`, `configprops`, `beans`, `mappings`, `threaddump` are not in scope.** + All five answered 200 on all three versions from exposure alone, so an + `access` property that opens one of them adds no reachability that the + exposure list has not already granted. Exposing them is SPR-CONFIG-001's + finding, and #45 proposed covering them here before that was measured. + +## Remediation + +Take the endpoint out of the exposure list: + +```yaml +management: + endpoints: + web: + exposure: + include: health,info +``` + +Or close the gate explicitly, which is what to reach for when the wildcard has +to stay: + +```yaml +management: + endpoint: + shutdown: + access: none # Boot 3.4+ + enabled: false # honoured on 2.3.12, 3.3.13 and 3.5.16 +``` + +If the endpoint is genuinely needed, move management to its own port behind +authentication rather than leaving it on the application connector. diff --git a/src/main/java/io/sprig/rule/rules/ActuatorEndpointAccessRule.java b/src/main/java/io/sprig/rule/rules/ActuatorEndpointAccessRule.java new file mode 100644 index 0000000..223687d --- /dev/null +++ b/src/main/java/io/sprig/rule/rules/ActuatorEndpointAccessRule.java @@ -0,0 +1,224 @@ +package io.sprig.rule.rules; + +import io.sprig.model.FindingCollector; +import io.sprig.model.Severity; +import io.sprig.rule.Rule; +import io.sprig.rule.RuleContext; +import io.sprig.rule.RuleKind; +import io.sprig.scan.ConfigEntry; +import java.nio.file.Path; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +/** + * SPR-CONFIG-006 — an Actuator endpoint that Spring Boot disables by default has been switched back + * on and exposed over HTTP. {@code shutdown} stops the application; {@code heapdump} returns a + * HPROF of the live process, which contains every credential and session token in memory. + * + *

Both halves are required, and that is the whole point of this rule. Exposure alone never + * reaches either endpoint: measured on Boot 2.3.12, 3.3.13 and 3.5.16, {@code exposure.include: + * "*"} leaves {@code POST /actuator/shutdown} at 404. Access alone never reaches them either: with + * {@code exposure.include: health,info}, setting {@code + * management.endpoint.shutdown.access=unrestricted} leaves it at 404. Name the endpoint in the + * exposure list and open the gate, and the same request returns 200 and the process exits. + * + *

Boot spells the gate two ways and honours both on current versions. {@code + * management.endpoint..enabled} was honoured on 2.3.12, 3.3.13 and 3.5.16 alike. {@code + * management.endpoint..access} arrived in 3.4 and is inert before it. Since sprig never reads + * the project's Boot version, it reports either spelling and leaves the version question to + * docs/rules/SPR-CONFIG-006.md. + */ +public final class ActuatorEndpointAccessRule implements Rule { + + /** The exposure list. An endpoint absent from it is unreachable whatever its access says. */ + private static final String EXPOSURE = "management.endpoints.web.exposure.include"; + + private static final String ACCESS_DEFAULT = "management.endpoints.access.default"; + private static final String MAX_PERMITTED = "management.endpoints.access.max-permitted"; + private static final String ENABLED_BY_DEFAULT = "management.endpoints.enabled-by-default"; + + /** + * The endpoints Boot ships switched off, and the access level each one needs before it answers. + * {@code shutdown} is a POST, so read-only access leaves it at 404; {@code heapdump} is a GET + * and read-only is enough to serve it. + */ + private static final Map GATED_ENDPOINTS = + Map.of("shutdown", Boolean.TRUE, "heapdump", Boolean.FALSE); + + private static final Set CONFIG_KEYS = + Stream.concat( + Stream.of(EXPOSURE, ACCESS_DEFAULT, MAX_PERMITTED, ENABLED_BY_DEFAULT), + GATED_ENDPOINTS.keySet().stream() + .flatMap( + id -> + Stream.of( + "management.endpoint." + id + ".access", + "management.endpoint." + + id + + ".enabled"))) + .collect(Collectors.toUnmodifiableSet()); + + @Override + public String id() { + return "SPR-CONFIG-006"; + } + + @Override + public String name() { + return "actuator-endpoint-access"; + } + + @Override + public String description() { + return "An Actuator endpoint disabled by default (shutdown, heapdump) is both exposed over HTTP and switched back on."; + } + + @Override + public String remediation() { + return "Remove the endpoint from management.endpoints.web.exposure.include, or set management.endpoint..access=none (Boot 3.4+) or management.endpoint..enabled=false. If the endpoint is genuinely needed, put management on a separate port behind authentication."; + } + + @Override + public Severity severity() { + return Severity.CRITICAL; + } + + @Override + public Set tags() { + return Set.of("actuator", "config"); + } + + @Override + public RuleKind kind() { + return RuleKind.CONFIG; + } + + @Override + public Set configKeys() { + return CONFIG_KEYS; + } + + @Override + public boolean appliesTo(RuleContext ctx) { + return ctx.config() != null && !ctx.config().isEmpty(); + } + + @Override + public void analyze(RuleContext ctx, FindingCollector findings) { + for (Path file : ctx.config().files()) { + Map entries = ctx.config().entriesFor(file); + ConfigEntry exposure = entries.get(EXPOSURE); + if (exposure == null) { + continue; + } + List exposed = tokens(exposure.asString()); + for (Map.Entry gated : GATED_ENDPOINTS.entrySet()) { + String id = gated.getKey(); + if (!exposed.contains("*") && !exposed.contains(id)) { + continue; + } + ConfigEntry opener = openerFor(entries, id, gated.getValue()); + if (opener != null) { + findings.add( + this, + opener.source(), + opener.line(), + "Actuator '" + + id + + "' is exposed over HTTP and switched on by " + + opener.key() + + "=" + + opener.asString() + + ". " + + effect(id), + opener.key()); + } + } + } + } + + /** + * The property that opens {@code id}, or null if nothing does. Returns the entry rather than a + * boolean so the finding can point at the line a reader has to change. + */ + private static ConfigEntry openerFor( + Map entries, String id, boolean needsWrite) { + if (capped(entries, needsWrite)) { + return null; + } + ConfigEntry access = entries.get("management.endpoint." + id + ".access"); + ConfigEntry enabled = entries.get("management.endpoint." + id + ".enabled"); + if (access != null || enabled != null) { + // An explicit per-endpoint setting decides on its own: access=none beats a blanket + // access.default=unrestricted, which is measured behaviour on 3.5.16. + if (enabled != null && isTrue(enabled.asString())) { + return enabled; + } + if (access != null && grants(access.asString(), needsWrite)) { + return access; + } + return null; + } + ConfigEntry blanketEnabled = entries.get(ENABLED_BY_DEFAULT); + if (blanketEnabled != null && isTrue(blanketEnabled.asString())) { + return blanketEnabled; + } + ConfigEntry blanketAccess = entries.get(ACCESS_DEFAULT); + if (blanketAccess != null && grants(blanketAccess.asString(), needsWrite)) { + return blanketAccess; + } + return null; + } + + /** + * Whether {@code management.endpoints.access.max-permitted} caps the endpoint below what it + * needs. Measured on 3.5.16: {@code read-only} takes shutdown back to 404 while heapdump, a + * GET, still returns a real HPROF, and the cap holds against the legacy {@code enabled: true} + * spelling too. + * + *

The cap is a 3.4+ property and does nothing before it, so on 3.3 a project that set both + * {@code shutdown.enabled: true} and this cap would still be reachable while this rule stays + * quiet. That is accepted rather than fixed: writing a 3.4-only property into a pre-3.4 project + * is a configuration that does nothing on its own terms, and treating the cap as inert would + * put a CRITICAL finding on every correctly capped 3.4+ project instead. + */ + private static boolean capped(Map entries, boolean needsWrite) { + ConfigEntry max = entries.get(MAX_PERMITTED); + return max != null && !grants(max.asString(), needsWrite); + } + + /** Whether an access level reaches the endpoint. {@code none} never does. */ + private static boolean grants(String value, boolean needsWrite) { + if (value == null) { + return false; + } + String level = value.trim().toLowerCase(Locale.ROOT); + return needsWrite + ? level.equals("unrestricted") + : level.equals("unrestricted") || level.equals("read-only"); + } + + private static boolean isTrue(String value) { + return value != null && value.trim().toLowerCase(Locale.ROOT).equals("true"); + } + + private static String effect(String id) { + return id.equals("shutdown") + ? "POST /actuator/shutdown stops the application." + : "GET /actuator/heapdump returns a dump of process memory, including credentials and session tokens."; + } + + private static List tokens(String value) { + if (value == null) { + return List.of(); + } + return Stream.of(value.split(",")) + .map(t -> t.trim().toLowerCase(Locale.ROOT)) + .filter(t -> !t.isEmpty()) + .collect(Collectors.toList()); + } +} diff --git a/src/main/java/io/sprig/rule/rules/ActuatorExposureRule.java b/src/main/java/io/sprig/rule/rules/ActuatorExposureRule.java index e0c7da0..2119e5b 100644 --- a/src/main/java/io/sprig/rule/rules/ActuatorExposureRule.java +++ b/src/main/java/io/sprig/rule/rules/ActuatorExposureRule.java @@ -13,14 +13,24 @@ /** * SPR-CONFIG-001 — Actuator exposes a sensitive endpoint. The infamous {@code - * management.endpoints.web.exposure.include: "*"} leaks environment variables and heap dumps; - * {@code env} and {@code heapdump} alone are just as dangerous. + * management.endpoints.web.exposure.include: "*"} serves {@code env}, {@code configprops}, {@code + * beans} and {@code mappings} on every version measured, and {@code /actuator/env} alone is enough + * to hand over credentials. + * + *

{@code shutdown} used to be in this list and is not a token this rule can act on. Exposure + * never reaches it: {@code POST /actuator/shutdown} answered 404 under {@code include: "*"} on Boot + * 2.3.12, 3.3.13 and 3.5.16 alike, because the endpoint is switched off by default in every + * release. It takes a second property to open, which is SPR-CONFIG-006's subject. + * + *

{@code heapdump} stays, and what it means depends on the version. Through Boot 3.3 exposure + * alone serves a real HPROF. From 3.4 the access gate holds it at 404 until something opens it, so + * on those versions this finding marks the exposure rather than the disclosure. */ public final class ActuatorExposureRule implements Rule { private static final String KEY = "management.endpoints.web.exposure.include"; - private static final Set SENSITIVE = Set.of("*", "env", "heapdump", "shutdown"); + private static final Set SENSITIVE = Set.of("*", "env", "heapdump"); @Override public String id() { @@ -34,7 +44,7 @@ public String name() { @Override public String description() { - return "Spring Boot Actuator exposes sensitive endpoint(s): *, env, heapdump, or shutdown."; + return "Spring Boot Actuator exposes sensitive endpoint(s): *, env, or heapdump."; } @Override diff --git a/src/main/java/io/sprig/rule/rules/BuiltInRules.java b/src/main/java/io/sprig/rule/rules/BuiltInRules.java index 90c9adf..57f6170 100644 --- a/src/main/java/io/sprig/rule/rules/BuiltInRules.java +++ b/src/main/java/io/sprig/rule/rules/BuiltInRules.java @@ -9,6 +9,7 @@ public final class BuiltInRules { private static final List ALL = List.of( new ActuatorExposureRule(), + new ActuatorEndpointAccessRule(), new HardcodedSecretRule(), new InsecureCookieFlagsRule(), new CorsConfigWildcardCredentialsRule(), diff --git a/src/test/java/io/sprig/report/SarifReporterTest.java b/src/test/java/io/sprig/report/SarifReporterTest.java index 912605c..ed1d674 100644 --- a/src/test/java/io/sprig/report/SarifReporterTest.java +++ b/src/test/java/io/sprig/report/SarifReporterTest.java @@ -44,7 +44,7 @@ void outputIsSchemaValid() throws Exception { void ruleIndexesAreWithinRulesTable() throws Exception { JsonNode document = new ObjectMapper().readTree(render(new SarifReporter())); int ruleCount = document.at("/runs/0/tool/driver/rules").size(); - assertThat(ruleCount).isEqualTo(10); + assertThat(ruleCount).isEqualTo(11); for (JsonNode result : document.at("/runs/0/results")) { assertThat(result.get("ruleIndex").asInt()).isLessThan(ruleCount); } diff --git a/src/test/java/io/sprig/rule/ActuatorEndpointAccessRuleTest.java b/src/test/java/io/sprig/rule/ActuatorEndpointAccessRuleTest.java new file mode 100644 index 0000000..8f2726d --- /dev/null +++ b/src/test/java/io/sprig/rule/ActuatorEndpointAccessRuleTest.java @@ -0,0 +1,71 @@ +package io.sprig.rule; + +import static org.assertj.core.api.Assertions.assertThat; + +import io.sprig.model.Finding; +import io.sprig.model.Severity; +import io.sprig.rule.rules.ActuatorEndpointAccessRule; +import java.util.List; +import org.junit.jupiter.api.Test; + +/** + * Each case here mirrors a request measured against a running Boot app. The fixture comments name + * the version and the status codes, so a change in behaviour is a change to both. + */ +class ActuatorEndpointAccessRuleTest extends RuleTestBase { + + private final ActuatorEndpointAccessRule rule = new ActuatorEndpointAccessRule(); + + @Test + void flagsBothEndpointsWhenExposedAndOpened() { + List findings = findingsFor("actuator-access-open", rule); + assertThat(findings).hasSize(2); + assertThat(findings) + .allSatisfy( + f -> { + assertThat(f.ruleId()).isEqualTo("SPR-CONFIG-006"); + assertThat(f.severity()).isEqualTo(Severity.CRITICAL); + }); + assertThat(findings) + .extracting(Finding::propertyPath) + .containsExactlyInAnyOrder( + "management.endpoint.shutdown.access", + "management.endpoint.heapdump.access"); + assertFindingAt(findings, "application.yml", 8); + assertFindingAt(findings, "application.yml", 10); + } + + /** Read-only is enough for a GET endpoint, and never enough for a POST one. */ + @Test + void readOnlyOpensHeapdumpButNotShutdown() { + List findings = findingsFor("actuator-access-capped", rule); + assertThat(findings).hasSize(1); + assertThat(findings.get(0).propertyPath()).isEqualTo("management.endpoint.heapdump.access"); + assertThat(findings.get(0).message()).contains("dump of process memory"); + } + + /** A blanket default opens what is gated; an explicit per-endpoint setting overrides it. */ + @Test + void blanketDefaultFiresAndPerEndpointAccessNoneDoesNot() { + List findings = findingsFor("actuator-access-blanket", rule); + assertThat(findings).hasSize(1); + assertThat(findings.get(0).propertyPath()).isEqualTo("management.endpoints.access.default"); + assertThat(findings.get(0).message()).contains("heapdump"); + } + + /** The gate is open and the endpoint is not exposed, which reaches nothing. */ + @Test + void doesNotFlagAccessWithoutExposure() { + assertThat(findingsFor("actuator-access-not-exposed", rule)).isEmpty(); + } + + @Test + void doesNotFlagWildcardExposureOnItsOwn() { + assertThat(findingsFor("actuator-exposed", rule)).isEmpty(); + } + + @Test + void doesNotFlagSafeConfiguration() { + assertThat(findingsFor("secure-app", rule)).isEmpty(); + } +} diff --git a/src/test/java/io/sprig/rule/ActuatorExposureRuleTest.java b/src/test/java/io/sprig/rule/ActuatorExposureRuleTest.java index f622dd4..7773d2f 100644 --- a/src/test/java/io/sprig/rule/ActuatorExposureRuleTest.java +++ b/src/test/java/io/sprig/rule/ActuatorExposureRuleTest.java @@ -24,4 +24,14 @@ void flagsWildcardExposure() { void doesNotFlagSafeExposure() { assertThat(findingsFor("secure-app", rule)).isEmpty(); } + + /** + * Exposing {@code shutdown} does not reach it. The endpoint is switched off by default on every + * release measured, so this rule has nothing to say about the token on its own and + * SPR-CONFIG-006 owns the configuration that does open it. + */ + @Test + void doesNotFlagShutdownInTheExposureList() { + assertThat(findingsFor("actuator-shutdown-token", rule)).isEmpty(); + } } diff --git a/src/test/java/io/sprig/rule/ConfigKeyMetadataTest.java b/src/test/java/io/sprig/rule/ConfigKeyMetadataTest.java index 41c19ec..fed43eb 100644 --- a/src/test/java/io/sprig/rule/ConfigKeyMetadataTest.java +++ b/src/test/java/io/sprig/rule/ConfigKeyMetadataTest.java @@ -81,7 +81,25 @@ class ConfigKeyMetadataTest { * it" and "it looks right next to the declared ones" do not clear that bar. Without evidence in * the reason, this map mutes the test instead of recording what was checked. */ - private static final Map ACCEPTED_ABSENCES = Map.of(); + private static final Map ACCEPTED_ABSENCES = + Map.of( + "management.endpoint.shutdown.access", + "Boot 3.5.16, exposure.include=health,info,shutdown: POST" + + " /actuator/shutdown answers 404, and 200 with the process" + + " exiting once this is set to unrestricted. Bound" + + " reflectively per endpoint id, so no endpoint declares it.", + "management.endpoint.shutdown.enabled", + "Boot 2.3.12 and 3.3.13, same exposure: POST /actuator/shutdown answers" + + " 404, and 200 once this is true. Still honoured on 3.5.16.", + "management.endpoint.heapdump.access", + "Boot 3.5.16, exposure.include=health,info,heapdump: GET" + + " /actuator/heapdump answers 404, and 200 with a JAVA PROFILE" + + " 1.0.2 header once this is read-only or unrestricted.", + "management.endpoint.heapdump.enabled", + "Boot 3.5.16, exposure.include=*: GET /actuator/heapdump answers 404," + + " and 200 with a JAVA PROFILE 1.0.2 header once this is true." + + " On 3.3.13 setting it false takes the same request from 200" + + " to 404."); @BeforeAll static void loadIndex() throws IOException { diff --git a/src/test/resources/fixtures/actuator-access-blanket/src/main/resources/application.yml b/src/test/resources/fixtures/actuator-access-blanket/src/main/resources/application.yml new file mode 100644 index 0000000..aeb113b --- /dev/null +++ b/src/test/resources/fixtures/actuator-access-blanket/src/main/resources/application.yml @@ -0,0 +1,14 @@ +# The blanket default opens every gated endpoint, and a per-endpoint setting +# overrides it. Measured on Boot 3.5.16: access.default=unrestricted alone takes +# both /actuator/heapdump and /actuator/shutdown from 404 to 200, and adding +# shutdown.access=none takes shutdown back to 404 while heapdump stays open. +management: + endpoints: + web: + exposure: + include: "*" + access: + default: unrestricted + endpoint: + shutdown: + access: none diff --git a/src/test/resources/fixtures/actuator-access-capped/src/main/resources/application.yml b/src/test/resources/fixtures/actuator-access-capped/src/main/resources/application.yml new file mode 100644 index 0000000..d39cecf --- /dev/null +++ b/src/test/resources/fixtures/actuator-access-capped/src/main/resources/application.yml @@ -0,0 +1,16 @@ +# max-permitted caps write access, and only write access. Measured on Boot +# 3.5.16 with both endpoints exposed: POST /actuator/shutdown drops back to 404 +# under this cap, while GET /actuator/heapdump still returns a real HPROF, +# because read-only is all a GET endpoint needs. +management: + endpoints: + web: + exposure: + include: "*" + access: + max-permitted: read-only + endpoint: + shutdown: + access: unrestricted + heapdump: + access: unrestricted diff --git a/src/test/resources/fixtures/actuator-access-not-exposed/src/main/resources/application.yml b/src/test/resources/fixtures/actuator-access-not-exposed/src/main/resources/application.yml new file mode 100644 index 0000000..54585f8 --- /dev/null +++ b/src/test/resources/fixtures/actuator-access-not-exposed/src/main/resources/application.yml @@ -0,0 +1,15 @@ +# The access gate is open on both endpoints, and neither is reachable, because +# they are not in the exposure list. Measured on Boot 3.5.16: GET +# /actuator/heapdump and POST /actuator/shutdown both answer 404 under exactly +# this configuration, and both answer 200 the moment the ids are added to the +# include list. +management: + endpoints: + web: + exposure: + include: health,info + endpoint: + shutdown: + access: unrestricted + heapdump: + access: unrestricted diff --git a/src/test/resources/fixtures/actuator-access-open/src/main/resources/application.yml b/src/test/resources/fixtures/actuator-access-open/src/main/resources/application.yml new file mode 100644 index 0000000..8032020 --- /dev/null +++ b/src/test/resources/fixtures/actuator-access-open/src/main/resources/application.yml @@ -0,0 +1,10 @@ +management: + endpoints: + web: + exposure: + include: "*" + endpoint: + shutdown: + access: unrestricted + heapdump: + access: read-only diff --git a/src/test/resources/fixtures/actuator-shutdown-token/src/main/resources/application.yml b/src/test/resources/fixtures/actuator-shutdown-token/src/main/resources/application.yml new file mode 100644 index 0000000..85d41b4 --- /dev/null +++ b/src/test/resources/fixtures/actuator-shutdown-token/src/main/resources/application.yml @@ -0,0 +1,10 @@ +# shutdown named in the exposure list and nothing else. Measured on Boot 2.3.12, +# 3.3.13 and 3.5.16: POST /actuator/shutdown answers 404 in all three, because +# the endpoint ships switched off and exposure does not switch it on. Flagging +# this was a false positive; SPR-CONFIG-006 covers the configuration that does +# reach it. +management: + endpoints: + web: + exposure: + include: health,info,shutdown diff --git a/src/test/resources/golden/demo-app.sarif.golden b/src/test/resources/golden/demo-app.sarif.golden index 0f0fb51..0a661cf 100644 --- a/src/test/resources/golden/demo-app.sarif.golden +++ b/src/test/resources/golden/demo-app.sarif.golden @@ -11,10 +11,10 @@ "id" : "SPR-CONFIG-001", "name" : "actuator-exposure", "shortDescription" : { - "text" : "Spring Boot Actuator exposes sensitive endpoint(s): *, env, heapdump, or shutdown." + "text" : "Spring Boot Actuator exposes sensitive endpoint(s): *, env, or heapdump." }, "fullDescription" : { - "text" : "Spring Boot Actuator exposes sensitive endpoint(s): *, env, heapdump, or shutdown." + "text" : "Spring Boot Actuator exposes sensitive endpoint(s): *, env, or heapdump." }, "help" : { "text" : "Limit management.endpoints.web.exposure.include to safe endpoints (health, info) or expose management over a non-public port with authentication." @@ -97,6 +97,24 @@ "precision" : "high", "security-severity" : "3.0" } + }, { + "id" : "SPR-CONFIG-006", + "name" : "actuator-endpoint-access", + "shortDescription" : { + "text" : "An Actuator endpoint disabled by default (shutdown, heapdump) is both exposed over HTTP and switched back on." + }, + "fullDescription" : { + "text" : "An Actuator endpoint disabled by default (shutdown, heapdump) is both exposed over HTTP and switched back on." + }, + "help" : { + "text" : "Remove the endpoint from management.endpoints.web.exposure.include, or set management.endpoint..access=none (Boot 3.4+) or management.endpoint..enabled=false. If the endpoint is genuinely needed, put management on a separate port behind authentication." + }, + "helpUri" : "https://github.com/vianbas/sprig/blob/main/docs/rules/SPR-CONFIG-006.md", + "properties" : { + "tags" : [ "actuator", "config" ], + "precision" : "high", + "security-severity" : "9.0" + } }, { "id" : "SPR-CORS-001", "name" : "cors-wildcard-credentials", @@ -192,7 +210,7 @@ }, "results" : [ { "ruleId" : "SPR-CORS-001", - "ruleIndex" : 5, + "ruleIndex" : 6, "level" : "error", "message" : { "text" : "Cross-origin configured with origins=* and allowCredentials=true." @@ -212,7 +230,7 @@ } }, { "ruleId" : "SPR-SRC-005", - "ruleIndex" : 9, + "ruleIndex" : 10, "level" : "warning", "message" : { "text" : "headers().frameOptions() is disabled: clickjacking protection removed." @@ -232,7 +250,7 @@ } }, { "ruleId" : "SPR-SRC-002", - "ruleIndex" : 6, + "ruleIndex" : 7, "level" : "error", "message" : { "text" : "Insecure password handling: NoOpPasswordEncoder or {noop} plaintext password used." @@ -252,7 +270,7 @@ } }, { "ruleId" : "SPR-SRC-004", - "ruleIndex" : 8, + "ruleIndex" : 9, "level" : "warning", "message" : { "text" : "@EnableWebSecurity without @EnableMethodSecurity: @PreAuthorize/@Secured method annotations are not enforced." @@ -272,7 +290,7 @@ } }, { "ruleId" : "SPR-SRC-003", - "ruleIndex" : 7, + "ruleIndex" : 8, "level" : "error", "message" : { "text" : "SecurityFilterChain permits every request via .anyRequest().permitAll() with no authentication mechanism."