-
Notifications
You must be signed in to change notification settings - Fork 2
Document web application vulnerability scanning procedure (#7021) #7096
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Document web application vulnerability scanning procedure (#7021) #7096
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #7096 +/- ##
========================================
Coverage 85.24% 85.24%
========================================
Files 152 152
Lines 22060 22060
========================================
Hits 18804 18804
Misses 3256 3256 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1023862
to
634e24a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, small nits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the intended category of ZAP application usage.
In that spirit, I'd rearrange the sections, moving the ZAP Setup last, and maybe the authenticated scan preceding it, but I'll leave that to your discretion. Also, consider having a small/medium explanatory paragraph in the beginning regarding the how long to expect for a scan to run and any network/connectivity delays that may be experienced while running this without request limit filters (if any).
5fc1864
to
ac189e9
Compare
I think it makes sense to have the setup first, since it has options that should be set before any scans are run.
I've expanded a few of the sections to make them more descriptive, and added a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved ✅
OPERATOR.rst
Outdated
|
||
- From the menu, select ``Tools`` -> ``Options``: | ||
|
||
- -> ``Network`` -> ``Rate Limit``, add and enable a 3 request per second rule |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No double bullets, please.
- -> ``Network`` -> ``Rate Limit``, add and enable a 3 request per second rule | |
- Select ``Network`` -> ``Rate Limit``, add and enable a 3 request per second rule |
OPERATOR.rst
Outdated
|
||
The process for running an authenticated scan is to first obtain an Azul | ||
authentication token, and launch the ZAP application with the token set as | ||
an environment variable. See |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OPERATOR.rst
Outdated
|
||
- -> ``Check for Updates``: | ||
|
||
- Check for updates on startup: Checked |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Check for updates on startup: Checked | |
- Check the *Check for updates on startup* option |
Use italics when you quote from a UI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also ask for another peer review from Abraham asking him to walk through these steps in order to verify them.
OPERATOR.rst
Outdated
environment variable. See the `ZAP documentation | ||
<https://www.zaproxy.org/docs/getting-further/authentication/handling-auth-yourself/>`_ | ||
for more information. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For long URLs, separate the link and the target definition as outlined in the Sphinx documentation I linked to in my previous review.
OPERATOR.rst
Outdated
- From the popup, select the *No, I do not want to persis this session at this | ||
moment in time* option and click *Start* | ||
|
||
- From the menu, select *Edit* -> *ZAP Mode* -> *Standard Mode* (This should be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use em dash, please.
- From the menu, select *Edit* -> *ZAP Mode* -> *Standard Mode* (This should be | |
- From the menu, select *Edit* – *ZAP Mode* – *Standard Mode* (This should be |
OPERATOR.rst
Outdated
- Using the Swagger UI, excecute an enpoint such as ``/index/catalogs`` | ||
|
||
- Note the example ``curl`` command, and copy the token from the | ||
``Authorization`` header option (e.g. "Bearer ya29.a0…") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
``Authorization`` header option (e.g. "Bearer ya29.a0…") | |
``Authorization`` header option (e.g. ``Bearer ya29.a0…``) |
OPERATOR.rst
Outdated
new scan. Failure to do so can pollute the scan results with the findings from | ||
the previous scan. | ||
|
||
If you are promopted with options to persist the ZAP session, select the *No, I |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you are promopted with options to persist the ZAP session, select the *No, I | |
If you are prompted with options to persist the ZAP session, select the *No, I |
OPERATOR.rst
Outdated
|
||
- Check the *Check for updates to the add-ons you have installed* option | ||
|
||
Running an authenticated scan |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sounds like it is optional. It should state clearly that all scans should be authenticated, or, if only some should be authenticated, which ones.
7fe1f01
to
c473359
Compare
@achave11-ucsc, Please peer review, and attempt to follow the guide in order to successfully run scans and generate reports. A full list of the URLs to scan is available from the issue template for the monthly scan (template is not yet merged PR #7121). Since completing all 8 scans would take a ~24 hours, I suggest running one scan fully (the Azul indexer scans complete the quickest), generating a report, and comparing the results to the last triaged scan results on this Google Sheet. For the other scans, getting the scan to start and seeing findings start to trickle in should suffice. Feel free to reach out if you have any questions, however (hopefully) the guide will give you everything you need. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM ✅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PL please.
2d10b12
to
bcf2b25
Compare
bcf2b25
to
9d9da68
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we discussed in PL using a numbered list for the sequence of steps. Please also flatten the list structure so that it becomes a simple sequence of steps. Make sure that list items are complete sentences.
Connected issues: #7021
Checklist
Author
develop
issues/<GitHub handle of author>/<issue#>-<slug>
1 when the issue title describes a problem, the corresponding PR
title is
Fix:
followed by the issue titleAuthor (partiality)
p
tag to titles of partial commitspartial
or completely resolves all connected issuespartial
labelAuthor (chains)
base
or this PR is not chained to another PRchained
or is not chained to another PRAuthor (reindex, API changes)
r
tag to commit title or the changes introduced by this PR will not require reindexing of any deploymentreindex:dev
or the changes introduced by it will not require reindexing ofdev
reindex:anvildev
or the changes introduced by it will not require reindexing ofanvildev
reindex:anvilprod
or the changes introduced by it will not require reindexing ofanvilprod
reindex:prod
or the changes introduced by it will not require reindexing ofprod
reindex:partial
and its description documents the specific reindexing procedure fordev
,anvildev
,anvilprod
andprod
or requires a full reindex or carries none of the labelsreindex:dev
,reindex:anvildev
,reindex:anvilprod
andreindex:prod
API
or this PR does not modify a REST APIa
(A
) tag to commit title for backwards (in)compatible changes or this PR does not modify a REST APIapp.py
or this PR does not modify a REST APIAuthor (upgrading deployments)
make docker_images.json
and committed the resulting changes or this PR does not modifyazul_docker_images
, or any other variables referenced in the definition of that variableu
tag to commit title or this PR does not require upgrading deploymentsupgrade
or does not require upgrading deploymentsdeploy:shared
or does not modifydocker_images.json
, and does not require deploying theshared
component for any other reasondeploy:gitlab
or does not require deploying thegitlab
componentdeploy:runner
or does not require deploying therunner
imageAuthor (hotfixes)
F
tag to main commit title or this PR does not include permanent fix for a temporary hotfixanvilprod
andprod
) have temporary hotfixes for any of the issues connected to this PRAuthor (before every review)
develop
, squashed old fixupsmake requirements_update
or this PR does not modifyrequirements*.txt
,common.mk
,Makefile
andDockerfile
R
tag to commit title or this PR does not modifyrequirements*.txt
reqs
or does not modifyrequirements*.txt
make integration_test
passes in personal deployment or this PR does not modify functionality that could affect the IT outcomePeer reviewer (after approval)
System administrator (after approval)
demo
orno demo
no demo
no sandbox
N reviews
label is accurateOperator (before pushing merge the commit)
reindex:…
labels andr
commit title tagno demo
develop
_select dev.shared && CI_COMMIT_REF_NAME=develop make -C terraform/shared apply_keep_unused
or this PR is not labeleddeploy:shared
_select dev.gitlab && CI_COMMIT_REF_NAME=develop make -C terraform/gitlab apply
or this PR is not labeleddeploy:gitlab
_select anvildev.shared && CI_COMMIT_REF_NAME=develop make -C terraform/shared apply_keep_unused
or this PR is not labeleddeploy:shared
_select anvildev.gitlab && CI_COMMIT_REF_NAME=develop make -C terraform/gitlab apply
or this PR is not labeleddeploy:gitlab
deploy:gitlab
deploy:gitlab
System administrator
dev.gitlab
are complete or this PR is not labeleddeploy:gitlab
anvildev.gitlab
are complete or this PR is not labeleddeploy:gitlab
Operator (before pushing merge the commit)
_select dev.gitlab && make -C terraform/gitlab/runner
or this PR is not labeleddeploy:runner
_select anvildev.gitlab && make -C terraform/gitlab/runner
or this PR is not labeleddeploy:runner
sandbox
label or PR is labeledno sandbox
dev
or PR is labeledno sandbox
anvildev
or PR is labeledno sandbox
sandbox
deployment or PR is labeledno sandbox
anvilbox
deployment or PR is labeledno sandbox
sandbox
deployment or PR is labeledno sandbox
anvilbox
deployment or PR is labeledno sandbox
sandbox
or this PR does not remove catalogs or otherwise causes unreferenced indices indev
anvilbox
or this PR does not remove catalogs or otherwise causes unreferenced indices inanvildev
sandbox
or this PR is not labeledreindex:dev
anvilbox
or this PR is not labeledreindex:anvildev
sandbox
or this PR is not labeledreindex:dev
anvilbox
or this PR is not labeledreindex:anvildev
p
if the PR is also labeledpartial
Operator (chain shortening)
develop
or this PR is not labeledbase
chained
label from the blocked PR or this PR is not labeledbase
base
base
label from this PR or this PR is not labeledbase
Operator (after pushing the merge commit)
dev
anvildev
dev
dev
anvildev
anvildev
_select dev.shared && make -C terraform/shared apply
or this PR is not labeleddeploy:shared
_select anvildev.shared && make -C terraform/shared apply
or this PR is not labeleddeploy:shared
dev
anvildev
Operator (reindex)
dev
or this PR is neither labeledreindex:partial
norreindex:dev
anvildev
or this PR is neither labeledreindex:partial
norreindex:anvildev
dev
or this PR is neither labeledreindex:partial
norreindex:dev
anvildev
or this PR is neither labeledreindex:partial
norreindex:anvildev
dev
or this PR is neither labeledreindex:partial
norreindex:dev
anvildev
or this PR is neither labeledreindex:partial
norreindex:anvildev
dev
or this PR does not require reindexingdev
anvildev
or this PR does not require reindexinganvildev
dev
or this PR does not require reindexingdev
anvildev
or this PR does not require reindexinganvildev
dev
or this PR does not require reindexingdev
anvildev
or this PR does not require reindexinganvildev
dev
or this PR does not require reindexingdev
dev
or this PR does not require reindexingdev
deploy_browser
job in the GitLab pipeline for this PR indev
or this PR does not require reindexingdev
anvildev
or this PR does not require reindexinganvildev
deploy_browser
job in the GitLab pipeline for this PR inanvildev
or this PR does not require reindexinganvildev
Operator
deploy:shared
,deploy:gitlab
,deploy:runner
,API
,reindex:partial
,reindex:anvilprod
andreindex:prod
labels to the next promotion PRs or this PR carries none of these labelsdeploy:shared
,deploy:gitlab
,deploy:runner
,API
,reindex:partial
,reindex:anvilprod
andreindex:prod
labels, from the description of this PR to that of the next promotion PRs or this PR carries none of these labelsShorthand for review comments
L
line is too longW
line wrapping is wrongQ
bad quotesF
other formatting problem