As Security Products have evolved independently over the last year we have stacked some technical debt and have now discrepancies in our architecture. To avoid mistakes and improve maintenance we need to align them. This also helps providing a similar usage, making it easier for our users to integrate these features. Migrating from current state to our final goal has several implications over the whole stack (Rails BE/FE, wrapper project, common library) and it’s not easy to follow them. This is even more important than some steps are required to be released synchronously or happen in a specific order. ## Current state - DAST uses the ZAProxy json format with some added fields (spider, urlsOutOfScope etc) - Other Secure products: SAST, DS, CS etc use the Common Report Format that is built by the [Common Project](https://gitlab.com/gitlab-org/security-products/analyzers/common) - The GitLab Rails codebase needs to understand how to read these different formats ## Proposal - DAST should use the Common Report Format instead of the modified ZAProxy format. - DAST should build the Common Report Format independently of the Common Project. - A JSON schema should be used that defines the Common Report Format. This could be included in the Common Project. - When DAST starts to use more than one analyzer, the decision about whether or not to use the Common Project should be revisited. ## Why was this decision made? There has been back and forth about whether or not we should use the Go analyzer and the Common Project to build the DAST Common Report Format. The decision not to use the Common Project right now in DAST comes down to the following reasons: - The DAST team would need to use Ruby, Python and Go. - The DAST Python layer already manipulates the ZAProxy JSON, turning it into the Common Report Format is deemed a trivial task. - The DAST analyze script would have to be rewritten in Go. - Some Common Project features are not applicable to DAST and would need to be turned off (e.g. search). - The migration from modified ZAProxy format to Common Report Format is more difficult as the Common Project cannot build a single JSON file containing both formats. - 31 configuration parameters would need to be parsed both in the Go analyzer and in Python DAST. - As more analyzers are added to DAST, it may become clear that DAST does not belong in the Common Project at all. We can delay this decision till we have more information. ## To be ```plantuml Entrypoint -> "/analyze" : -t SITE "/analyze" -> "/analyze" : Tail logs (ZAP and DAST) "/analyze" -> "DAST Python" : New scan (zap-{baseline,full-scan}.py) Entrypoint -> "DAST Python" : -t SITE (zap-{baseline,full-scan}.py) "DAST Python" -> "DAST Python" : Parse CLI configuration "DAST Python" -> "SITE" : Wait till ready (http) "DAST Python" -> "SITE" : Verify domain (http) "DAST Python" -> "ZAP Python" : New scan (Python import) "ZAP Python" -> "ZAP Server" : Start Server (cli) "ZAP Python" -> "DAST Python" : Hooks (Python import) "DAST Python" -> "ZAP Server" : New ZAP session (REST API) "DAST Python" -> "SITE" : Login (Selenium) "DAST Python" -> "ZAP Server" : New Web Session (REST API) "ZAP Python" -> "ZAP Server" : Orchestrate scan (REST API) "ZAP Server" -> "File System" : Save results file "ZAP Python" -> "ZAP Server" : Poll till complete (REST API) "ZAP Python" -> "DAST Python" : Hooks (Python import) "DAST Python" -> "File System" : Edit results file, convert to Common Report Format "ZAP Python" -> "ZAP Server" : Shutdown (REST API) "/analyze" -> "/analyze" : Stop virtual framebuffer ``` ## Migration Path * [ ] DAST should create a `gl-dast-report.json` with modified ZAProxy fields and Common Report Format fields * [ ] The GitLab rails codebase should move to depend on the Common Report Format fields when parsing the `gl-dast-report.json` * [ ] The ZAProxy fields should be removed from the `gl-dast-report.json` report <details> <summary>Previous Go analyzer Migration path</summary> ## Migration path 1. * [x] %"11.9" - Put the job definition into the vendored include: https://gitlab.com/gitlab-org/gitlab-ee/issues/8663 1. * [x] %"11.9" - Deprecate the job definition directly using ZAProxy in favor of the vendored include: https://gitlab.com/gitlab-org/gitlab/issues/8663 1. * [x] %"12.0" - Remove Feature Flag for parse_dast_reports: https://gitlab.com/gitlab-org/gitlab-ee/issues/9636 1. * [x] %"12.0" - Drop support for the very old job definition using ZAProxy with the `artifacts` syntax (not the `reports` one). 1. * [x] %"12.1" - Integrate DAST properties into the Security Products Common format: https://gitlab.com/gitlab-org/gitlab-ee/issues/9635 1. * [ ] %"12.5" Use ~backend parser to process raw artifacts and provide nice and standardized API to ~frontend for Merge Request Widget and Pipeline view. https://gitlab.com/gitlab-org/gitlab/issues/11976 and https://gitlab.com/gitlab-org/gitlab/issues/12003 1. * [ ] %"12.5" Create a ZAProxy analyzer (Golang project leveraging the common library) generating a report compatible with our common format by leveraging the common library. This requires to support both new and old report format in the rails backend. https://gitlab.com/gitlab-org/gitlab/issues/14053 1. * [ ] %"12.5"? - Replace the DAST bash wrapper with the ZAProxy analyzer by updating [the DAST vendored template](https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/ci/templates/Security/DAST.gitlab-ci.yml): TODO create issue 1. * [ ] %"13.0" - Drop support for the old job definition using ZAProxy with the `reports` syntax (producing the raw ZAProxy scanner report format). At this point, we only accept reports with the common format. This is considered the final stage to reach. </details> <details> <summary>Previous current state</summary> ## Current state - ~frontend fetches raw ZAProxy report (vendor format) from artifacts for Project Level Security Dashboard, Merge Request Widget and Pipeline View. - ~backend parses raw ZAProxy report (vendor format) from artifacts to store vulnerabilities in the database for reports executed on the `default_branch`. - actual job definition uses ZAProxy (extended via a [hook script](https://github.com/zaproxy/zaproxy/blob/develop/docker/docs/scan-hooks.md)) directly with the new `reports` syntax - previous job definition using ZAProxy (extended via a [hook script](https://github.com/zaproxy/zaproxy/blob/develop/docker/docs/scan-hooks.md)) directly with the OLD `artifacts` syntax has been deprecated in `11.5`, to be removed in `12.0`. - there is a brand new [DAST wrapper project](https://gitlab.com/gitlab-org/security-products/dast) but it currently still produces the raw ZAProxy report (vendor format). </details> <p> <details> <summary>Old plan</summary> 1. * [x] %"11.9" - [Put the job definition into the vendored include](https://gitlab.com/gitlab-org/gitlab-ee/issues/8663) 1. * [x] Deprecate the job definition directly using ZAProxy in favor of the vendored include: https://gitlab.com/gitlab-org/gitlab/issues/8663 1. * [ ] [Make the DAST wrapper generating a report following the common format](https://gitlab.com/gitlab-org/gitlab-ee/issues/14053). But it's not supposed to use the `common` library (go) yet. Using this new format means we must make ~backend and ~frontend supporting the new format and staying backward compatible with the old one 1. * [ ] Update the vendored job definition to use this wrapper instead of ZAProxy. As we already deprecated the non-vendored job definition, we may not announce the move to use the wrapper instead of ZAProxy in the job definition. This is debatable. 1. * [x] %"12.1" - [Integrate DAST properties into the Security Products Common format](https://gitlab.com/gitlab-org/gitlab-ee/issues/9635) 1. * [ ] [Remove Feature Flag for parse_dast_reports](https://gitlab.com/gitlab-org/gitlab-ee/issues/9636) 1. * [ ] Drop support for the very old job definition using ZAProxy with the `artifacts` syntax (not the `reports` one). 1. * [ ] Use ~backend parser to process raw artifacts and provide nice and standardized API to ~frontend for Project Level Security Dashboard, Merge Request Widget and Pipeline view. 1. * [ ] Drop support for the old job definition using ZAProxy with the `reports` syntax (producing the raw ZAProxy scanner report format). At this point, we only accept reports with the common format. This is considered the final stage to reach (at least for now). An optional step that can be placed at any position after 3: * [ ] [Make the DAST wrapper a Golang project leveraging the common library](https://gitlab.com/gitlab-org/gitlab-ee/issues/14053) instead of being just a shell script.</details> </p>