diff --git a/CHANGELOG.md b/CHANGELOG.md index b104aca..ebc6ecc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,22 @@ CHANGELOG for 1.x =================== +## v1.8.0 - (2026-09-23) +### TODO When updating +The DAST scans (OWASP ZAP) moved to the stack-agnostic Standard Security Gitlab Package +(see [ADR](adr/20260923-dast-extraction-to-standard-security.md)). `composer recipes:install smartbooster/standard-bundle --reset --force` +does not remove the files copied by previous versions, so on a project that used them : +- Install standard-security to keep the `dast-*` targets (see its README), then add `include .standard-security/security.mk` to the project's `Makefile` +- Delete the files copied by the previous recipe : `git rm make/security.mk dast_scan/build-plan.sh && git rm -r dast_scan/scripts` +- Keep `dast_scan/targets.json` and `dast_scan/alert-filters.json`, their format is unchanged +- On a project that did not use the DAST scans, just delete `make/security.mk`, `dast_scan/build-plan.sh` and `dast_scan/scripts/` + +### Removed +- `make/security.mk`, `dast_scan/build-plan.sh`, `dast_scan/scripts/` and `docs/security.md`, now shipped by Standard Security +- recipe `smartbooster.standard-bundle.1.8.json` : no longer copies `make/security.mk`, `dast_scan/build-plan.sh` and `dast_scan/scripts/` + +### Added +- `adr/20260923-dast-extraction-to-standard-security.md` + ## v1.7.2 - (2026-09-23) ### Changed - `docs/front_code_validation.md` : label each `eslint.config.mjs`/`prettier.config.mjs` code block with its file name, add the missing diff --git a/README.md b/README.md index a864cf7..9aa08bd 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,6 @@ Bundle grouping all dev vendor that we use for testing and coding with the SMART - All the development vendors that we use daily to check our code quality: - static analysis with [PHPStan](docs/phpstan.md), - security taint analysis with [Psalm](docs/psalm.md), - - blackbox DAST scans of the running app with [OWASP ZAP](docs/security.md), - checkstyle and coding standards with [PHP-CS-Fixer](https://github.com/php-cs-fixer/php-cs-fixer) - native linters (php, twig, yaml, sf container) - tests bundle configuration for [PHPUnit](docs/phpunit.md) diff --git a/adr/20260923-dast-extraction-to-standard-security.md b/adr/20260923-dast-extraction-to-standard-security.md new file mode 100644 index 0000000..634dd8c --- /dev/null +++ b/adr/20260923-dast-extraction-to-standard-security.md @@ -0,0 +1,28 @@ +# Extraction of the DAST scans (OWASP ZAP) to `smartbooster/standard-security` + +**Goal**: stop shipping the blackbox DAST scans from this bundle, and ship them instead from a package that any project can install, +whatever its stack. + +**Context**: the DAST scans (`make/security.mk`, `dast_scan/build-plan.sh`, `dast_scan/scripts/*.js`, `docs/security.md`) were added in +v1.6.0 and hardened in v1.7.x. Unlike the rest of the bundle, nothing in them is PHP or Symfony specific: a set of shell/JS scripts and +config driven by `make` and run in Docker against a URL. Front-only projects (VitePress, VuePress, Astro sites ...) need the same scans but cannot install +a Symfony bundle, so the files ended up copied by hand into those projects, and already started to drift from the bundle version. + +**Selected solution**: move the DAST scans to a dedicated internal package repository on Gitlab, +a stack-agnostic package installed by copying its `packages/` folder into the project's `.standard-security/` directory at a given git tag, +with a `standard-security.lock` tracking the installed version and a `make security-fetch` target to update it (same approach as the +`symfony-docker.lock` / `make docker-fetch` of smartbooster/symfony-docker). + +Discarded alternatives: +- keeping the files in this bundle and copying them by hand into non-Symfony projects: two copies to maintain, which already diverged; +- one package per stack (composer + npm): the same scripts published twice, for tooling that has no language dependency at all; +- git submodule / subtree, or a templating tool (copier, cruft): extra tooling or workflow for files that are never customised per project + (the project-specific part is data, `dast_scan/targets.json` and `dast_scan/alert-filters.json`, not a templated file). + +## Impact + +- `make/security.mk`, `dast_scan/build-plan.sh`, `dast_scan/scripts/` and `docs/security.md` removed from the bundle +- new recipe manifest `smartbooster.standard-bundle.1.8.json`, without the three DAST entries of `copy-from-package` +- the recipe reset does not remove files copied by a previous version: projects have to delete them by hand and install + standard-security to keep the scans (see the v1.8.0 entry of the CHANGELOG) +- project-owned files (`dast_scan/targets.json`, `dast_scan/alert-filters.json`) keep their format and location diff --git a/dast_scan/build-plan.sh b/dast_scan/build-plan.sh deleted file mode 100755 index 21ff44c..0000000 --- a/dast_scan/build-plan.sh +++ /dev/null @@ -1,281 +0,0 @@ -#!/bin/sh -# -# Generates a ZAP Automation Framework plan on standard output. -# -# Usage: build-plan.sh [targets.json] [report-prefix] -# -# Three levels of increasing invasiveness, each one including the previous: -# passive : replays the declared "paths", with GET. No side effect. -# crawl : + spider and AJAX spider, without form submission. -# active : + form submission and active scanner (real attacks), -# excludePaths then taking the sensitive routes out of scope. -# -# Schema of targets.json (dast_scan/targets.json by default): -# environments : one environment per key, with baseUrl (where {sub} is replaced by -# the subdomain), the duration caps in minutes spiderMaxMins, -# spiderAjaxMaxMins and activeScanMaxMins, and reportRisks (levels -# kept in the reports, all of them by default). -# sites : scanned subdomains; paths = routes replayed with GET, -# ajaxSpider = site additionally explored by the AJAX spider. -# excludePaths : regexes taken out of scope in active mode, for the routes that are -# destructive or that trigger emails. -# -# Two more files shape the plan: -# dast_scan/alert-filters.json : the baseline, turned into an alertFilter job so that an -# arbitrated alert stops failing the build while staying in the report. -# dast_scan/scripts/*.js : home-made scan rules, inlined below. -# -set -eu - -mode=${1:?"missing mode (passive|crawl|active)"} -env_name=${2:?"missing environment (see .environments in targets.json)"} -targets=${3:-"$(dirname "$0")/targets.json"} -scripts_dir="$(dirname "$0")/scripts" -filters_file="$(dirname "$0")/alert-filters.json" - -command -v jq >/dev/null 2>&1 || { echo "build-plan.sh: jq is required" >&2; exit 1; } -[ -f "$targets" ] || { echo "build-plan.sh: targets file not found ($targets)" >&2; exit 1; } -jq empty "$targets" 2>/dev/null || { echo "build-plan.sh: invalid JSON ($targets)" >&2; exit 1; } - -case "$mode" in passive|crawl|active) ;; *) echo "build-plan.sh: unknown mode ($mode)" >&2; exit 1 ;; esac - -base_url=$(jq -r --arg env "$env_name" '.environments[$env].baseUrl // empty' "$targets") -[ -n "$base_url" ] || { - echo "build-plan.sh: unknown environment ($env_name). Known: $(jq -r '.environments | keys | join(", ")' "$targets")" >&2 - exit 1 -} - -spider_max_mins=$(jq -r --arg env "$env_name" '.environments[$env].spiderMaxMins // 5' "$targets") -ajax_max_mins=$(jq -r --arg env "$env_name" '.environments[$env].spiderAjaxMaxMins // 2' "$targets") -active_max_mins=$(jq -r --arg env "$env_name" '.environments[$env].activeScanMaxMins // 10' "$targets") -report_risks=$(jq -r --arg env "$env_name" '.environments[$env].reportRisks // [] | join(", ")' "$targets") -fail_on_risk=$(jq -r --arg env "$env_name" '.environments[$env].failOnRisk // "Low"' "$targets") -prefix=${4:-"blackbox-$env_name-$mode"} -context="blackbox" - -# The only two switches of the plan: crawl or not, attack or not. -if [ "$mode" = passive ]; then crawl=false; else crawl=true; fi -if [ "$mode" = active ]; then attack=true; else attack=false; fi - -site_urls() { - jq -r --arg base "$base_url" '.sites[] | .sub as $sub | ($base | gsub("\\{sub\\}"; $sub))' "$targets" -} - -ajax_urls() { - jq -r --arg base "$base_url" '.sites[] | select(.ajaxSpider) | .sub as $sub | ($base | gsub("\\{sub\\}"; $sub))' "$targets" -} - -exclude_paths() { jq -r '.excludePaths[]?' "$targets"; } - -# In active mode the excludePaths take routes out of scope: they are then neither attacked -# nor passively analysed, so there is no point in replaying them. ZAP regexes must match -# the whole URL, hence the anchoring. -requestor_urls() { - jq -r --arg base "$base_url" --argjson filter "$attack" ' - (if $filter then (.excludePaths // []) else [] end) as $excluded - | .sites[] as $site - | ($base | gsub("\\{sub\\}"; $site.sub)) as $root - | $site.paths[]? - | ($root + .) as $url - | select([$excluded[] | . as $regex | select($url | test("^" + $regex + "$"))] | length == 0) - | $url' "$targets" -} - -[ -n "$(site_urls)" ] || { echo "build-plan.sh: no entry in .sites" >&2; exit 1; } -[ -n "$(requestor_urls)" ] || { echo "build-plan.sh: no route to replay ($targets: empty \"paths\", or everything excluded by excludePaths)" >&2; exit 1; } - -# The context is limited to the declared urls: everything below each of them is included. -cat < carrying no rel at all - the exact pattern -// Qualys reports as QID 150222 - never triggers it, at any threshold. - -const ScanRuleMetadata = Java.type("org.zaproxy.addon.commonlib.scanrules.ScanRuleMetadata"); - -function getMetadata() { - return ScanRuleMetadata.fromYaml(` -id: 9000002 -name: Link to another domain opened with target="_blank" and no rel="noopener" -description: > - The page links to another domain with target="_blank" but without rel="noopener" nor - rel="noreferrer". The opened page can then reach the opener through window.opener and - redirect the original tab, for instance to a phishing page (reverse tabnabbing). -solution: > - Add rel="noopener noreferrer" to every link carrying target="_blank". -references: - - https://cwe.mitre.org/data/definitions/1022.html -risk: LOW -confidence: HIGH -cweId: 1022 -wascId: 15 -alertTags: - QUALYS_QID: "150222" -status: alpha -`); -} - -const ANCHOR_PATTERN = /<(?:a|area)\b[^>]*>/gi; - -function attribute(tag, name) { - const match = new RegExp(name + '\\s*=\\s*("([^"]*)"|\'([^\']*)\'|([^\\s">]+))', "i").exec(tag); - if (match === null) { - return null; - } - const value = match[2] !== undefined ? match[2] : (match[3] !== undefined ? match[3] : match[4]); - return value === undefined ? null : value; -} - -function hostOf(url) { - const match = /^https?:\/\/([^/?#]+)/i.exec(url); - return match === null ? null : match[1].toLowerCase(); -} - -function scan(ps, msg, src) { - if (!msg.getResponseHeader().isHtml()) { - return; - } - - const host = String(msg.getRequestHeader().getHostName()).toLowerCase(); - const body = String(msg.getResponseBody().toString()); - const reported = {}; - - ANCHOR_PATTERN.lastIndex = 0; - let match; - while ((match = ANCHOR_PATTERN.exec(body)) !== null) { - const tag = match[0]; - - const target = (attribute(tag, "target") || "").toLowerCase(); - if (target !== "_blank") { - continue; - } - - const rel = (attribute(tag, "rel") || "").toLowerCase(); - if (rel.indexOf("noopener") !== -1 || rel.indexOf("noreferrer") !== -1) { - continue; - } - - // Relative links stay on the same origin: only absolute links to another host matter. - const href = attribute(tag, "href"); - if (href === null) { - continue; - } - const linkHost = hostOf(href); - if (linkHost === null || linkHost === host) { - continue; - } - - // One alert per target URL is enough, a menu can repeat the same link many times. - if (reported[href]) { - continue; - } - reported[href] = true; - - ps.newAlert() - .setParam(href) - .setEvidence(tag) - .setOtherInfo("Qualys reports this as QID 150222 (CWE-1022).") - .setMessage(msg) - .raise(); - } -} diff --git a/dast_scan/scripts/client-side-cookies.js b/dast_scan/scripts/client-side-cookies.js deleted file mode 100644 index 135f8b3..0000000 --- a/dast_scan/scripts/client-side-cookies.js +++ /dev/null @@ -1,95 +0,0 @@ -// Passive scan rule: cookies written by client-side JavaScript. -// -// A cookie written through document.cookie never appears in a Set-Cookie response header, so -// the native rules 10010 (HttpOnly) and 10011 (Secure) are structurally blind to it - as is any -// scanner that only looks at responses. Qualys sees those cookies because its crawler reads them -// from the browser, and reports them as QID 150122 (no Secure) and QID 150123 (no HttpOnly). -// -// This rule works on what is always observable: the JavaScript actually served. A cookie written -// there can never be HttpOnly, and its Secure attribute depends solely on client code, so it is -// worth an explicit review - and the rule fires whether or not the crawl happened to trigger the -// write, which is what makes it reliable. -// -// Only writes naming the cookie are reported. A bare "document.cookie = someVariable" is the -// plumbing of every cookie library and would alert on each bundle embedding one. A name built -// from a template literal - cookies.set(`${s}`, ...), as shipped by swagger-ui - is the same -// case: what the regex captures is the template, not a cookie name. - -const ScanRuleMetadata = Java.type("org.zaproxy.addon.commonlib.scanrules.ScanRuleMetadata"); - -function getMetadata() { - return ScanRuleMetadata.fromYaml(` -id: 9000003 -name: Cookie written by client-side JavaScript -description: > - The served JavaScript writes a cookie from the browser. Such a cookie cannot carry the HttpOnly - attribute (it would break the code reading it back), and its Secure attribute is set by the - client code alone, not by the server: both have to be reviewed in the code rather than assumed. -solution: > - Keep client-side cookies free of any sensitive value, and make sure the code setting them forces - the Secure attribute when the page is served over HTTPS. Move to a server-set cookie whenever the - value does not have to be read by JavaScript. -references: - - https://cwe.mitre.org/data/definitions/1004.html - - https://cwe.mitre.org/data/definitions/614.html -risk: LOW -confidence: HIGH -cweId: 1004 -wascId: 13 -alertTags: - QUALYS_QID: "150122 / 150123" -status: alpha -`); -} - -// cookies.set("name", ...) / $cookies.set('name') / Cookies.set(`name`) - the call sites of the -// usual libraries (vue-cookies, js-cookie...), which survive minification. -const LIBRARY_WRITE_PATTERN = /(?:\$?cookies|cookieStore)\s*\.\s*set\s*\(\s*["'`]([^"'`]+)["'`]/gi; - -// document.cookie = "name=..." - the raw write, when the name is a literal. -const RAW_WRITE_PATTERN = /document\s*\.\s*cookie\s*=\s*["'`]([^="'`]+)=/gi; - -function isJavaScript(msg) { - const contentType = msg.getResponseHeader().getHeader("Content-Type"); - return contentType !== null && String(contentType).toLowerCase().indexOf("javascript") !== -1; -} - -// A ${...} left in the captured text means the name is assembled at runtime: nothing to report. -function isDynamic(name) { - return name.indexOf("${") !== -1; -} - -function collect(body, pattern, names) { - pattern.lastIndex = 0; - let match; - while ((match = pattern.exec(body)) !== null) { - const name = match[1].trim(); - if (name.length > 0 && !isDynamic(name) && names.indexOf(name) === -1) { - names.push(name); - } - } -} - -function scan(ps, msg, src) { - if (!msg.getResponseHeader().isHtml() && !isJavaScript(msg)) { - return; - } - - const body = String(msg.getResponseBody().toString()); - const names = []; - collect(body, LIBRARY_WRITE_PATTERN, names); - collect(body, RAW_WRITE_PATTERN, names); - - for (let i = 0; i < names.length; i++) { - ps.newAlert() - .setParam(names[i]) - .setEvidence(names[i]) - .setOtherInfo( - "Cookie written from JavaScript, so it can carry neither HttpOnly (Qualys QID 150123) " - + "nor a server-controlled Secure attribute (Qualys QID 150122). Check the code that " - + "sets it." - ) - .setMessage(msg) - .raise(); - } -} diff --git a/dast_scan/scripts/form-autocomplete.js b/dast_scan/scripts/form-autocomplete.js deleted file mode 100644 index 0ea5f3e..0000000 --- a/dast_scan/scripts/form-autocomplete.js +++ /dev/null @@ -1,114 +0,0 @@ -// Passive scan rule: sensitive form field left with browser autocompletion enabled. -// -// ZAP retired its own rule (10012, "Password Autocomplete in browser") in pscanrules 22 -// (2018, issue 4215) because browsers ignore autocomplete="off" on password fields. -// Qualys still reports it as QID 150112, category Vulnerabilities, so the rule is kept here -// for audit parity: the point is to be warned before an external audit is, not to claim the -// finding is exploitable. -// -// A field is considered covered when either the field itself or its enclosing form carries -// an autocomplete value that turns the feature off. -// -// Only type="password" is reported. Guessing sensitivity from the field name was tried and -// dropped: a Symfony form scopes its field names, so the plain e-mail input of the "forgot -// password" form is named forgot_password[email] and matched every "password" heuristic. - -const ScanRuleMetadata = Java.type("org.zaproxy.addon.commonlib.scanrules.ScanRuleMetadata"); - -function getMetadata() { - return ScanRuleMetadata.fromYaml(` -id: 9000001 -name: Sensitive form field has not disabled autocomplete -description: > - A password input is served without an autocomplete attribute that turns browser autocompletion - off, neither on the field nor on its enclosing form. On a shared workstation the browser may - offer to store and replay the value. -solution: > - Add autocomplete="off" (or "new-password" / "current-password" where a password manager is - wanted) to the field, or autocomplete="off" to the form that contains it. -references: - - https://cwe.mitre.org/data/definitions/200.html -risk: LOW -confidence: HIGH -cweId: 200 -wascId: 15 -alertTags: - QUALYS_QID: "150112" -status: alpha -`); -} - -const FORM_OPEN_PATTERN = /]*>/gi; -const INPUT_PATTERN = /]*>/gi; - -// Values that actually disable the browser's own storage of the value. -const SAFE_VALUES = ["off", "new-password", "current-password"]; - -function attribute(tag, name) { - const match = new RegExp(name + '\\s*=\\s*("([^"]*)"|\'([^\']*)\'|([^\\s">]+))', "i").exec(tag); - if (match === null) { - return null; - } - const value = match[2] !== undefined ? match[2] : (match[3] !== undefined ? match[3] : match[4]); - return value === undefined ? null : value; -} - -function isSensitive(tag) { - return (attribute(tag, "type") || "").toLowerCase() === "password"; -} - -function autocompleteDisabled(tag) { - const value = (attribute(tag, "autocomplete") || "").toLowerCase(); - return SAFE_VALUES.indexOf(value) !== -1; -} - -// Splits the body into segments, each one carrying the
tag it belongs to (null outside -// of any form), so that an autocomplete="off" set on the form covers the fields it contains. -function segments(body) { - const result = []; - let lastIndex = 0; - let lastForm = null; - - FORM_OPEN_PATTERN.lastIndex = 0; - let match; - while ((match = FORM_OPEN_PATTERN.exec(body)) !== null) { - result.push({ form: lastForm, html: body.substring(lastIndex, match.index) }); - lastForm = match[0]; - lastIndex = FORM_OPEN_PATTERN.lastIndex; - } - result.push({ form: lastForm, html: body.substring(lastIndex) }); - - return result; -} - -function scan(ps, msg, src) { - if (!msg.getResponseHeader().isHtml()) { - return; - } - - const body = String(msg.getResponseBody().toString()); - const parts = segments(body); - - for (let i = 0; i < parts.length; i++) { - const part = parts[i]; - if (part.form !== null && autocompleteDisabled(part.form)) { - continue; - } - - INPUT_PATTERN.lastIndex = 0; - let match; - while ((match = INPUT_PATTERN.exec(part.html)) !== null) { - const tag = match[0]; - if (!isSensitive(tag) || autocompleteDisabled(tag)) { - continue; - } - - ps.newAlert() - .setParam(attribute(tag, "name") || attribute(tag, "id") || "") - .setEvidence(tag) - .setOtherInfo("Qualys reports this as QID 150112 (CWE-200).") - .setMessage(msg) - .raise(); - } - } -} diff --git a/dast_scan/scripts/phpinfo.js b/dast_scan/scripts/phpinfo.js deleted file mode 100644 index a23af94..0000000 --- a/dast_scan/scripts/phpinfo.js +++ /dev/null @@ -1,81 +0,0 @@ -// Passive scan rule: detects a phpinfo() output in the body of a response. -// -// The native Hidden File Finder rule only probes four known paths (phpinfo.php, -// info.php, i.php, test.php). This one analyses the content, so it applies to any URL -// reached by the scan, including a plain application route. -// -// Injected into the plan by dast_scan/build-plan.sh, enabled in the three modes. - -const ScanRuleMetadata = Java.type("org.zaproxy.addon.commonlib.scanrules.ScanRuleMetadata"); - -function getMetadata() { - return ScanRuleMetadata.fromYaml(` -id: 9000004 -name: Exposed phpinfo() output -description: > - The response contains the output of the PHP phpinfo() function. It discloses the PHP - version, the server configuration, the absolute filesystem paths, the loaded extensions - and sometimes environment variables, which makes it easier to target a later attack. -solution: > - Remove the call to phpinfo() from the publicly exposed code. If the page is still needed - for diagnostics, restrict it to authenticated access and to internal IP addresses. -references: - - https://www.php.net/manual/en/function.phpinfo.php -risk: HIGH -confidence: HIGH -cweId: 200 -wascId: 13 -status: alpha -`); -} - -// The title alone is enough, it is phpinfo()'s signature. Otherwise the PHP version plus -// two labels of its configuration table are required, so as not to alert on a page that -// would merely mention phpinfo(). -const TITLE_PATTERN = /\s*phpinfo\(\)\s*<\/title>/i; -const VERSION_PATTERN = /PHP Version\s*(?:<\/[a-z0-9]+>\s*)*[0-9]+\.[0-9]+/i; -const CONFIG_MARKERS = [ - "Configuration File (php.ini) Path", - "Loaded Configuration File", - "Server API", - "Build Date", -]; - -function findEvidence(body) { - const title = TITLE_PATTERN.exec(body); - if (title !== null) { - return title[0]; - } - - const version = VERSION_PATTERN.exec(body); - if (version === null) { - return null; - } - - let markers = 0; - for (let i = 0; i < CONFIG_MARKERS.length; i++) { - if (body.indexOf(CONFIG_MARKERS[i]) !== -1) { - markers++; - } - } - return markers >= 2 ? version[0] : null; -} - -function scan(ps, msg, src) { - // phpinfo() produces HTML: this avoids analysing assets, images and fonts. - const contentType = msg.getResponseHeader().getHeader("Content-Type"); - if (contentType !== null && String(contentType).toLowerCase().indexOf("html") === -1) { - return; - } - - const body = String(msg.getResponseBody().toString()); - const evidence = findEvidence(body); - if (evidence === null) { - return; - } - - ps.newAlert() - .setEvidence(evidence) - .setOtherInfo("Detected by analysing the response body, regardless of the URL.") - .raise(); -} diff --git a/docs/security.md b/docs/security.md deleted file mode 100644 index 4ad85b9..0000000 --- a/docs/security.md +++ /dev/null @@ -1,281 +0,0 @@ -# Security — DAST blackbox scans (OWASP ZAP) - -[PHPStan](phpstan.md) and [Psalm](psalm.md) read the **code** (SAST). This documentation covers the other half: scanning the **running -application** (DAST) with [OWASP ZAP](https://www.zaproxy.org/), through the targets shipped by `make/security.mk`. - -Scans are **blackbox**: no authentication, only the public routes of the project (the ones behind a `PUBLIC_ACCESS` `access_control` in -`config/packages/security.yaml`). ZAP runs in Docker, against a URL, and knows nothing about the code. - -> The Makefile contains **no URL**: everything is declared in `dast_scan/targets.json` specific to the project, and the ZAP -> [Automation Framework](https://www.zaproxy.org/docs/automate/automation-framework/) plan is generated on the fly by `dast_scan/build-plan.sh`. - -## What the recipe ships - -| File | Role | -|--------------------------------|--------------------------------------------------------------------------------------------------------------------------| -| `make/security.mk` | the `dast-*` make targets, the Docker run and the `DAST_*` variables | -| `dast_scan/build-plan.sh` | generates the ZAP plan (contexts, jobs, reports) from `targets.json`, on standard output | -| `dast_scan/scripts/*.js` | custom passive scan rules, injected inline into every generated plan (see [Custom passive rules](#custom-passive-rules)) | -| `dast_scan/targets.json` | **not shipped — each project writes its own** (see [Setup](#setup-mandatory-after-a-recipe-reset)) | -| `dast_scan/alert-filters.json` | **not shipped — optional, each project writes its own** (see [The baseline](#the-baseline-dast_scanalert-filtersjson)) | - -## Setup (mandatory after a recipe reset) - -```bash -composer update smartbooster/standard-bundle -composer recipes:install smartbooster/standard-bundle --reset --force -``` - -The reset copies `make/security.mk`, `dast_scan/build-plan.sh` and `dast_scan/scripts/`, but **never `dast_scan/targets.json`**: base URLs, -subdomains and public routes are project-specific, and a shipped example would silently overwrite the project's own file on every reset. - -**Writing `dast_scan/targets.json` is therefore the first step after the recipe install**, otherwise every target fails immediately: - -``` -build-plan.sh: targets file not found (dast_scan/targets.json) -``` - -Then complete the setup: - -1. Ignore the generated artefacts in the project `.gitignore` (the plans and reports are rebuilt on each run): - ```gitignore - /dast_scan/generated - /dast_scan/report - ``` -2. Check the prerequisites on the host running `make`: **Docker** (the ZAP image is pulled on first run) and **`jq`** (used by - `build-plan.sh` to read `targets.json`). -3. Make sure the application answers on the declared `baseUrl` (the container runs with `--network host`, so `http://{sub}.localhost` works - against a local stack). -4. Optionally write `dast_scan/alert-filters.json`, the project's arbitrated baseline — see - [The baseline](#the-baseline-dast_scanalert-filtersjson). Absent, the plan simply carries no `alertFilter` job. - -## `dast_scan/targets.json` - -```json -{ - "environments": { - "local": { - "baseUrl": "http://{sub}.localhost", - "spiderMaxMins": 5, - "spiderAjaxMaxMins": 2, - "activeScanMaxMins": 10 - }, - "ci": { - "baseUrl": "https://{sub}.my-project.integration.example.com", - "spiderMaxMins": 5, - "spiderAjaxMaxMins": 2, - "activeScanMaxMins": 30, - "reportRisks": ["info", "low", "medium", "high"], - "failOnRisk": "Low" - } - }, - "sites": [ - { - "sub": "admin", - "ajaxSpider": true, - "paths": ["/login", "/forgot_password", "/reset_password"] - }, - { - "sub": "app", - "ajaxSpider": true, - "paths": ["/login", "/register"] - }, - { - "sub": "api", - "ajaxSpider": false, - "paths": ["/docs"] - }, - ], - "excludePaths": [ - ".*/logout.*", - ".*/_wdt/.*", - ".*/_profiler.*", - ".*/delete$", - ".*/batch.*", - ".*/forgot_password.*", - ] -} -``` - -### `environments` - -One entry per environment, addressed by the make target (`local`, `ci`, …). An unknown name lists the available ones and stops. - -| Key | Default | Role | -|---------------------|---------|------------------------------------------------------------------------------------------| -| `baseUrl` | — | root URL of each site; `{sub}` is replaced by the `sub` of every entry of `sites` | -| `spiderMaxMins` | `5` | cap, in minutes, of the classic spider (`crawl` and `active` modes) | -| `spiderAjaxMaxMins` | `2` | cap, in minutes, of the AJAX spider, **per site** having `ajaxSpider: true` | -| `activeScanMaxMins` | `10` | cap, in minutes, of the active scanner (`active` mode) | -| `reportRisks` | all | risk levels kept in the reports, e.g. `["medium", "high"]` to drop `info`/`low` noise | -| `failOnRisk` | `Low` | lowest risk making the build fail, see [The gate](#the-gate-failonrisk) — `High`, `Medium`, `Low` or `Informational` | - -### `sites` - -The scanned subdomains. They define the ZAP **context**: everything below these URLs is in scope, everything else (CDN, fonts, external -icons loaded by the pages) is neither crawled, attacked nor reported (can have some trace on insight due to spider crawl). - -| Key | Role | -|--------------|--------------------------------------------------------------------------------------------------------------------| -| `sub` | subdomain injected into `baseUrl` in place of `{sub}` | -| `paths` | routes replayed with `GET` at the start of every scan, so passive analysis always sees them even without crawling | -| `ajaxSpider` | `true` to additionally explore the site with the AJAX spider (headless Firefox) — reserve it for JS-heavy interfaces | - -Routes with parameters are declared with a dummy value (`/register?token=test`): the point is to reach the -controller and have ZAP analyse the response, not to hit real data. - -### `excludePaths` - -Regexes taken out of scope, **in `active` mode only** — those routes are then neither attacked, nor replayed, nor passively analysed. They -must match the **whole URL** (`build-plan.sh` anchors them with `^…$` when filtering the replayed routes). - -Exclude anything an attack payload must not reach: destructive routes (`.*/delete$`, `.*/batch.*`), routes sending emails -(`.*/forgot_password.*`, `.*/inscription.*`), payment callbacks (`.*/payment-ipn.*`), the profiler (`.*/_wdt/.*`, `.*/_profiler.*` when using it on dev mode) and the -logout, which would only kill the session. - -## The gate (`failOnRisk`) - -ZAP **always exits `0`**, whatever it finds. What turns an alert into a red pipeline is the `exitStatus` job, appended to every generated -plan with `errorLevel` set to the environment's `failOnRisk` (default `Low`) and `warnLevel` to `Informational`. It is declared -`alwaysRun: true`, so the status is still set when an earlier job interrupts the plan. - -Two consequences worth knowing before raising the bar: - -- `failOnRisk` filters on the **risk**, not on the report: an alert dropped from the reports by `reportRisks` still fails the build if its - risk reaches `failOnRisk`. Keep `reportRisks` at least as wide as `failOnRisk`, otherwise the CI fails on an alert that appears nowhere. -- the job skips every alert requalified as `False Positive` (ZAP's `ExitStatusJob` ignores `CONFIDENCE_FALSE_POSITIVE`) — which is exactly - what makes the baseline below work. - -## The baseline (`dast_scan/alert-filters.json`) - -A scan that fails on `Low` only stays useful if the alerts already arbitrated stop failing it. `dast_scan/alert-filters.json` is that -baseline: `build-plan.sh` turns it into an `alertFilter` job, emitted **before** any job generating traffic (a filter only applies to the -alerts raised after it is registered). The file is optional and, like `targets.json`, **never shipped by the recipe**: it holds project -arbitrations, and a shipped example would overwrite the project's own file on every reset. - -```json -{ - "filters": [ - { - "ruleId": "10096", - "url": "https://admin\\.[^/]+(/.*)?", - "urlRegex": true, - "newRisk": "False Positive", - "note": "Timestamp Disclosure on admin: the integers are the Sonata asset cache-busters (?v=1767630660), not a server clock." - }, - { - "ruleId": "40040-2", - "url": "https://api\\.[^/]+(?:/(?!doc).*)?", - "urlRegex": true, - "newRisk": "Info", - "note": "QID 150631 - outside /doc, reflecting the Origin is accepted: the api is stateless (Bearer token, no session cookie)." - } - ] -} -``` - -| Key | Role | -|----------------------------------|---------------------------------------------------------------------------------------------------------------------------------------| -| `ruleId` | rule id, or `alertRef` to target a single alert of a multi-alert rule (`10055-4` is one variant of the CSP rule, `10055` all of them) | -| `newRisk` | `False Positive` (skipped by the gate, still visible in the report) or `Info` / `Low` / `Medium` / `High` to requalify | -| `url` + `urlRegex` | restrict the filter to an URL, literal or regex | -| `parameter` + `parameterRegex` | restrict it to a parameter (cookie name, field name…) | -| `attack`/`evidence` (+ `…Regex`) | restrict it to the payload sent or to the evidence found | -| `methods`, `context` | restrict it to HTTP methods or to a ZAP context | -| `note` | **ours, not ZAP's**: stripped from the job and rendered as a YAML comment above the filter it documents | - -Two traps: - -- ZAP matches with `String.matches()`, so a regex must describe the **whole** URL. `"https://api\\.[^/]+/(?!doc)"` filters nothing; - `"https://api\\.[^/]+/(?!doc).*"` does (JSON, hence the doubled backslash). -- a filter that matches nothing is **silent** — no warning in the report, no line in the log. After adding one, re-run the scan and check - the alert really moved. - -Prefer scoping a filter by what makes it a false positive (the parameter, the evidence, the rule) rather than by the URL that happened to -raise it: a filter pinned on URLs silently stops covering the next route, while a filter pinned on the rule keeps the arbitration explicit. -Always fill `note` — it ends up in the generated plan, which is the artefact the next audit reads. - -## Commands - -Three levels of increasing invasiveness, each one including the previous: - -| Command | Mode | What it adds | Where it can run | -|-------------------------------|-----------|-----------------------------------------------------------|---------------------------------------| -| `make dast-blackbox-passive` | `passive` | replays `paths` with `GET`, passive analysis only | anywhere, prod included (if needed) | -| `make dast-blackbox-crawl` | `crawl` | + spider and AJAX spider, no form submitted, no attack | anywhere, prod included (if needed) | -| `make dast-blackbox-active` | `active` | + form submission and real attacks (SQLi, XSS, traversal) | **disposable env only** | -| `make dast-blackbox-ci` | `active` | same, on the `ci` environment of `targets.json` | integration only | -| `make dast-plan` | — | prints the generated plan, runs **no** scan | anywhere | - -`dast-blackbox-passive`, `-crawl` and `-active` all target the `local` environment. `dast-plan` accepts the two of them as variables: - -```bash -make dast-plan DAST_MODE=active DAST_ENV=ci -``` - -> `active` submits forms and sends attack payloads: it can create accounts, trigger emails and write junk data. - -## Add-ons (`DAST_ADDONS`) - -The official ZAP image ships the **release** add-ons only. `DAST_ADDONS` (default `ascanrulesBeta`) lists the extra ones installed into the -container before each run — `ascanrulesBeta` is what brings the CORS active scan rule (alert `40040`, the equivalent of Qualys' QID 150631), -along with `40025` *Proxy Disclosure* and `40038` *Bypassing 403*. Add-on ids are **case sensitive**; set `DAST_ADDONS=` to install none. - -The install runs as a **separate `zap.sh` invocation** inside the same container, before the one doing `-autorun`. Installing in the same run -does not work: resolving `ascanrulesBeta` pulls a newer `commonlib`, and swapping it while ZAP is up breaks the custom scripts that build -their metadata from it (`Could not initialize class …ScanRuleMetadata`). - -## Output - -Every run writes three things, all named after the make target: - -- the plan actually executed, in `dast_scan/generated/<target>.yaml` — regenerated on each run, useful to check the scope before attacking - and archivable by the CI; -- the reports, in `dast_scan/report/<target>-<timestamp>.html` and `.json` (`traditional-html-plus` and `traditional-json` templates), - restricted to the declared sites and to `reportRisks`; -- ZAP's own log, copied out of the container into `dast_scan/report/<target>.log`. In `-cmd` mode the log never reaches stdout, so this is - the only way to read the warnings counted by the report's `insight.log.warn`. The copy is unconditional (the exit code is saved - beforehand), so a **failing** scan — the case where the log matters most — still yields it. - -The `-plus` HTML template also lists the rules that ran **without** finding anything, the statistics, the parameters and the -request/response of each alert instance: the report says what was analysed, not only what alerted. - -## Custom passive rules - -`dast_scan/scripts/*.js` holds project-independent passive scan rules, copied **inline** into the plan (not mounted into the container): the -plan stays self-contained, and the one archived by the CI holds the rules that actually ran. They are registered before any traffic-generating -job and enabled in the three modes. - -Four are shipped. The last three exist because Qualys reports vulnerabilities ZAP has no native equivalent for: the point is to be warned by -our own CI before an external audit is, hence the `QUALYS_QID` alert tag carried by each of them. - -| Script | Id | What it catches | -|--------------------------|-----------|-----------------------------------------------------------------------------------------------------------------------| -| `phpinfo.js` | `9000004` | a `phpinfo()` output in **any** response body, where the native *Hidden File Finder* rule only probes four known paths (`phpinfo.php`, `info.php`, `i.php`, `test.php`) | -| `form-autocomplete.js` | `9000001` | a `type="password"` input whose autocompletion is disabled neither on the field nor on its form (QID 150112). ZAP retired its own rule `10012` in 2018 because browsers ignore `autocomplete="off"` on passwords; Qualys still reports it | -| `blank-target-links.js` | `9000002` | a cross-domain link with `target="_blank"` and no `rel="noopener"` (QID 150222). The native rule `10108` only alerts when `rel` explicitly contains `opener`, so a link with no `rel` at all never triggers it, at any threshold | -| `client-side-cookies.js` | `9000003` | a cookie written by the served JavaScript (QID 150122 / 150123). Such a cookie never appears in a `Set-Cookie` header, so the native rules `10010`/`10011` — and any scanner reading responses only — are structurally blind to it | - -The last three deliberately report from the **served code** rather than from observed traffic: they fire whether or not the crawl happened to -reach the page, which is what makes them reliable in CI. Each carries in its header the false positives that were tried and dropped -(name-based heuristics on Symfony scoped field names, `document.cookie = someVariable` in every cookie library…) — read it before widening a -pattern. - -To add one, drop a `.js` file in `dast_scan/scripts/` — `build-plan.sh` picks up every file of the directory automatically. It must export a -`getMetadata()` returning a `ScanRuleMetadata.fromYaml(...)` and a `scan(ps, msg, src)` raising the alert. Take the next id in the -`9000001+` range reserved here for home-made rules, well clear of the ids ZAP's own add-ons use. - -## Troubleshooting - -| Symptom | Cause / fix | -|----------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------| -| `build-plan.sh: targets file not found` | `dast_scan/targets.json` was not written after the recipe reset (see [Setup](#setup-mandatory-after-a-recipe-reset)) | -| `build-plan.sh: unknown environment (x). Known: …` | the make target asks for an environment absent from `.environments` | -| `build-plan.sh: no route to replay` | `paths` is empty, or `excludePaths` excludes all of them in `active` mode | -| `make: *** [dast-…] Error 137` | the JVM got `SIGKILL`ed: increase the value of the `DAST_JVM_MEM` variable | -| `AccessDeniedException` on the reports | the report directory must be writable by uid 1000 (the target already `chmod 777` it, check a stale root-owned dir) | -| ZAP finds nothing but the pages exist | check the `baseUrl` from **inside** the container (`--network host`), and that the routes are really public | -| `make: *** [dast-…] Error 1` with no error in the output | nominal: the `exitStatus` job found an alert at or above `failOnRisk`. Read the report, then fix it or arbitrate it in `alert-filters.json` | -| a filter of `alert-filters.json` changes nothing | the regex must match the **whole** URL (`String.matches()`); a filter matching nothing is silent. Check the emitted job with `make dast-plan` | -| `Could not initialize class …ScanRuleMetadata` | an add-on was installed in the same `zap.sh` run as the scan, swapping `commonlib` under the scripts — keep the two invocations separate | -| an alert is absent from the report but fails the build | `reportRisks` is narrower than `failOnRisk` (see [The gate](#the-gate-failonrisk)) | diff --git a/make/security.mk b/make/security.mk deleted file mode 100644 index 74b73a6..0000000 --- a/make/security.mk +++ /dev/null @@ -1,74 +0,0 @@ -## -## Application security (OWASP ZAP) -## -------------------------------- -# Blackbox DAST scans: no authentication, only the public routes of the 4 subdomains -# (see the PUBLIC_ACCESS access_control in config/packages/security.yaml). -# The targets are described in dast_scan/targets.json, the ZAP plan is generated on the -# fly by dast_scan/build-plan.sh. This Makefile knows no URL. -# -# Three levels of increasing invasiveness, each one including the previous: -# passive : declared routes, with GET -> no side effect, prod included -# crawl : + spider and AJAX spider -> no form submitted, no attack -# active : + forms and active scanner -> disposable environment only - -DAST_IMAGE ?= ghcr.io/zaproxy/zaproxy:stable -DAST_TARGETS ?= dast_scan/targets.json -DAST_PLAN_DIR ?= dast_scan/generated -DAST_REPORT_DIR ?= dast_scan/report - -# zap.sh sizes its heap from /proc/meminfo, and only reads the container limit through -# cgroup v1: on a cgroup v2 host it sees the RAM of the machine and reserves a quarter of -# it, well beyond what the CI grants it (SIGKILL, make Error 137). An explicit -Xmx passed -# to zap.sh takes precedence over that computation. -DAST_JVM_MEM ?= 2g - -# The official image ships the release add-ons only: the CORS active scan rule (Zap Alert Id 40040, equivalent of the 150631 from -# Qualys) lives in ascanrulesBeta, which is downloaded into the container. Add-on ids are case sensitive. -DAST_ADDONS ?= ascanrulesBeta - -# Installing in the same run as -autorun does not work: resolving ascanrulesBeta pulls a newer -# commonlib, and swapping it while ZAP is up breaks the scripts that build their metadata from it -# ("Could not initialize class ...ScanRuleMetadata"). Hence two invocations in the same container: -# the first one installs, the second one starts with everything already in place. -ifeq ($(strip $(DAST_ADDONS)),) -DAST_ADDON_CMD := -else -DAST_ADDON_CMD := zap.sh -cmd $(foreach addon,$(DAST_ADDONS),-addoninstall $(addon)) && -endif - -# Generates the plan, named after the make target, then has ZAP execute it. -# $(1) = mode, $(2) = environment declared in targets.json. -# The chmod is there for the CI: ZAP runs under uid 1000 and writes its reports into the -# mounted volume, which the runner creates as root (AccessDeniedException otherwise). The -# plans directory stays untouched, it is mounted read-only. -# zap.log is copied out of the container before it is discarded (--rm): in -cmd mode the log -# never reaches stdout, so it is the only way to read the warnings counted by the report's -# "insight.log.warn". The copy runs after a ";" and not a "&&", and the exit code is saved -# beforehand, so a failing scan - the case where the log matters most - still yields it. -define dast_run - @mkdir -p $(CURDIR)/$(DAST_PLAN_DIR) $(CURDIR)/$(DAST_REPORT_DIR) - @chmod 777 $(CURDIR)/$(DAST_REPORT_DIR) - @$(CURDIR)/dast_scan/build-plan.sh $(1) $(2) $(DAST_TARGETS) $@ \ - > $(CURDIR)/$(DAST_PLAN_DIR)/$@.yaml - docker run --rm --network host \ - -v $(CURDIR)/$(DAST_PLAN_DIR):/zap/plans:ro \ - -v $(CURDIR)/$(DAST_REPORT_DIR):/zap/wrk:rw \ - $(DAST_IMAGE) sh -c '$(DAST_ADDON_CMD) zap.sh -Xmx$(DAST_JVM_MEM) -cmd -autorun /zap/plans/$@.yaml; RC=$$?; cp -f /home/zap/.ZAP/zap.log /zap/wrk/$@.log 2>/dev/null; exit $$RC' -endef - -.PHONY: dast-blackbox-passive dast-blackbox-crawl dast-blackbox-active dast-blackbox-ci dast-plan - -dast-blackbox-passive: ## PASSIVE blackbox DAST scan, locally: replays the routes of targets.json with GET and analyses the responses. No side effect, can be run anywhere. - $(call dast_run,passive,local) - -dast-blackbox-crawl: ## EXPLORATORY blackbox DAST scan, locally: adds spider and AJAX spider to cover assets and XHR calls. No attack, no form submitted. - $(call dast_run,crawl,local) - -dast-blackbox-active: ## ACTIVE blackbox DAST scan, locally: adds form submission and attacks (SQLi, XSS, path traversal). /!\ Disposable environment only. - $(call dast_run,active,local) - -dast-blackbox-ci: ## ACTIVE blackbox DAST scan on the integration environment. /!\ Never on staging nor prod. - $(call dast_run,active,ci) - -dast-plan: ## Prints the generated ZAP plan without running any scan. E.g.: make dast-plan DAST_MODE=active DAST_ENV=ci - @$(CURDIR)/dast_scan/build-plan.sh $(or $(DAST_MODE),passive) $(or $(DAST_ENV),local) $(DAST_TARGETS) diff --git a/recipes.json b/recipes.json index de3f045..1410eb1 100644 --- a/recipes.json +++ b/recipes.json @@ -15,7 +15,8 @@ "1.4.2", "1.5", "1.6", - "1.7" + "1.7", + "1.8" ] }, "branch": "main", diff --git a/smartbooster.standard-bundle.1.8.json b/smartbooster.standard-bundle.1.8.json new file mode 100644 index 0000000..16150d6 --- /dev/null +++ b/smartbooster.standard-bundle.1.8.json @@ -0,0 +1,29 @@ +{ + "manifests": { + "smartbooster/standard-bundle": { + "manifest": { + "bundles": { + "DAMA\\DoctrineTestBundle\\DAMADoctrineTestBundle": ["test"], + "Liip\\TestFixturesBundle\\LiipTestFixturesBundle": ["test"], + "Smart\\StandardBundle\\SmartStandardBundle": ["dev", "test"] + }, + "copy-from-package": { + "config/packages/dama_doctrine_test_bundle.yaml": "%CONFIG_DIR%/packages/test/dama_doctrine_test_bundle.yaml", + "config/packages/liip_test_fixtures.yaml": "%CONFIG_DIR%/packages/test/liip_test_fixtures.yaml", + "make/dev.mk": "make/dev.mk", + "make/qualimetry.mk": "make/qualimetry.mk", + "make/test.mk": "make/test.mk", + "tests/bootstrap.php": "tests/bootstrap.php", + ".php-cs-fixer.dist.php": ".php-cs-fixer.dist.php", + "phpstan.neon": "phpstan.neon", + "phpstan-baseline.neon": "phpstan-baseline.neon", + "psalm.xml": "psalm.xml", + "psalm-taint-stubs.php": "psalm-taint-stubs.php", + "psalm-taint-baseline.xml": "psalm-taint-baseline.xml", + "phpunit.xml.dist": "phpunit.xml.dist" + } + }, + "ref": "99b6caae20c5aa9c2dbdce55f05fc568c193b1c0" + } + } +}